feat(upload)!: opt-in form fields via lvt-upload-with

CHANGELOG.md

@@ -5,6 +5,19 @@ All notable changes to @livetemplate/client will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Changed
+
+- **BREAKING** feat(upload): form fields travel with a Proxied upload only when
+  marked `lvt-upload-with`, replacing the previous serialize-everything-except-
+  `type="password"` denylist (livetemplate#452). A Proxied upload auto-fires on
+  file selection, so the old default silently POSTed every co-located field —
+  CSRF tokens, hidden secrets — to the upload endpoint with no submit-time moment
+  for the user to notice. **Migration:** add `lvt-upload-with` to each field an
+  `OnUpload` handler reads (typically a record id); an unmarked field now
+  surfaces as a missing value in the handler rather than as a silent leak.
+
 ## [v0.18.2] - 2026-07-15
 
 ### Changes

tests/upload-handler.test.ts

@@ -261,20 +261,16 @@ describe("UploadHandler", () => {
       );
     });
 
-    it("serializes the triggering input's form fields into a proxied upload, before the file", async () => {
+    // Fires a proxied auto-upload from the input named "doc" inside the given
+    // form markup and returns the FormData that was POSTed.
+    const proxiedUploadFormData = async (formHTML: string): Promise<FormData> => {
       const postMultipart = jest.fn().mockResolvedValue(undefined);
       const proxiedHandler = new UploadHandler(mockSendMessage, {
         postMultipartUpload: postMultipart,
       });
 
-      // The file input lives in a form carrying a record id (a value field) and
-      // a second, unrelated file input that must NOT become a value field.
       const form = document.createElement("form");
-      form.innerHTML =
-        '<input type="hidden" name="id" value="item-42">' +
-        '<input type="password" name="secret" value="hunter2">' +
-        '<input type="file" name="other" lvt-upload="other">' +
-        '<input type="file" name="doc" lvt-upload="doc">';
+      form.innerHTML = formHTML;
       const input = form.querySelector('input[name="doc"]') as HTMLInputElement;
 
       const file = createMockFile("scan.png", "imgdata", "image/png");
@@ -293,20 +289,131 @@ describe("UploadHandler", () => {
       });
       await jest.runAllTimersAsync();
 
-      const fd = postMultipart.mock.calls[0][0] as FormData;
-      // The record id rides along as a value field...
+      return postMultipart.mock.calls[0][0] as FormData;
+    };
+
+    it("serializes only the lvt-upload-with fields into a proxied upload, before the file", async () => {
+      const fd = await proxiedUploadFormData(
+        '<input type="hidden" name="id" value="item-42" lvt-upload-with>' +
+          '<input type="hidden" name="csrf" value="tok-abc">' +
+          '<input type="text" name="note" value="jotting">' +
+          '<input type="password" name="secret" value="hunter2">' +
+          '<input type="file" name="other" lvt-upload="other">' +
+          '<input type="file" name="doc" lvt-upload="doc">'
+      );
+
+      // The marked record id rides along as a value field...
       expect(fd.get("id")).toBe("item-42");
       // ...ordered before the streamed file part, so OnUpload sees it mid-stream.
       const keys = [...fd.keys()];
       expect(keys.indexOf("id")).toBeLessThan(keys.indexOf("doc"));
+
+      // Everything unmarked stays put. This is the assertion the opt-in contract
+      // exists for: under the old denylist the csrf token and the note both
+      // travelled, and only the password was special-cased out.
+      expect(fd.get("csrf")).toBeNull();
+      expect(fd.get("note")).toBeNull();
+      expect(fd.get("secret")).toBeNull();
+
       // The streamed file is still the file part; the other file input is not
       // serialized as a value field, and the action is untouched.
       expect(fd.get("doc")).toBeInstanceOf(File);
       expect(fd.get("other")).toBeNull();
       expect(fd.get("lvt-action")).toBe("upload_doc_complete");
-      // Password inputs are never serialized into the upload POST — a co-located
-      // credential must not ride along with an auto-fired upload.
-      expect(fd.get("secret")).toBeNull();
+    });
+
+    it("sends no form fields at all when a proxied upload's form marks none", async () => {
+      const fd = await proxiedUploadFormData(
+        '<input type="hidden" name="id" value="item-42">' +
+          '<input type="text" name="note" value="jotting">' +
+          '<input type="file" name="doc" lvt-upload="doc">'
+      );
+
+      // Unmarked is the default, so the POST carries the action and the file and
+      // nothing else — no field leaves the page without being asked to.
+      expect([...fd.keys()].sort()).toEqual(["doc", "lvt-action"]);
+    });
+
+    it("ignores lvt-upload-with on the reserved action and on file inputs", async () => {
+      const fd = await proxiedUploadFormData(
+        '<input type="hidden" name="lvt-action" value="hijacked" lvt-upload-with>' +
+          '<input type="file" name="other" lvt-upload="other" lvt-upload-with>' +
+          '<input type="file" name="doc" lvt-upload="doc">'
+      );
+
+      // Marking cannot clobber the action the caller set...
+      expect(fd.getAll("lvt-action")).toEqual(["upload_doc_complete"]);
+      // ...nor smuggle a second file in as a value field.
+      expect(fd.get("other")).toBeNull();
+    });
+
+    it("opts in a whole radio group when any one member is marked", async () => {
+      const fd = await proxiedUploadFormData(
+        '<input type="radio" name="kind" value="scan" lvt-upload-with>' +
+          '<input type="radio" name="kind" value="photo" checked>' +
+          '<input type="checkbox" name="flag" value="on" lvt-upload-with>' +
+          '<input type="radio" name="tier" value="gold" checked>' +
+          '<input type="file" name="doc" lvt-upload="doc">'
+      );
+
+      // Marking is by name, so the checked sibling travels even though the mark
+      // sits on the unchecked one...
+      expect(fd.get("kind")).toBe("photo");
+      // ...the browser's successful-control rules still decide the value, which
+      // is why the marked-but-unchecked box sends nothing...
+      expect(fd.get("flag")).toBeNull();
+      // ...and an entirely unmarked group stays put, checked or not.
+      expect(fd.get("tier")).toBeNull();
+    });
+
+    it("re-sends the marked fields with every file of a multi-file selection", async () => {
+      const postMultipart = jest.fn().mockResolvedValue(undefined);
+      const proxiedHandler = new UploadHandler(mockSendMessage, {
+        postMultipartUpload: postMultipart,
+      });
+
+      const form = document.createElement("form");
+      form.innerHTML =
+        '<input type="hidden" name="id" value="item-42" lvt-upload-with>' +
+        '<input type="file" name="doc" lvt-upload="doc" multiple>';
+      const input = form.querySelector('input[name="doc"]') as HTMLInputElement;
+
+      await proxiedHandler.startUpload(
+        "doc",
+        [
+          createMockFile("one.png", "first", "image/png"),
+          createMockFile("two.png", "second", "image/png"),
+        ],
+        input
+      );
+      await proxiedHandler.handleUploadStartResponse({
+        upload_name: "doc",
+        entries: [
+          {
+            entry_id: "entry-1",
+            client_name: "one.png",
+            valid: true,
+            auto_upload: true,
+            mode: "proxied",
+          },
+          {
+            entry_id: "entry-2",
+            client_name: "two.png",
+            valid: true,
+            auto_upload: true,
+            mode: "proxied",
+          },
+        ],
+      });
+      await jest.runAllTimersAsync();
+
+      // Each file is POSTed separately, and the marked fields ride along with
+      // every request — each one reaches OnUpload on its own and has to carry
+      // enough context to route its bytes.
+      expect(postMultipart).toHaveBeenCalledTimes(2);
+      for (const [fd] of postMultipart.mock.calls) {
+        expect((fd as FormData).get("id")).toBe("item-42");
+      }
     });
 
     it("previews locally without uploading when mode is preview", async () => {

upload/upload-handler.ts

@@ -426,7 +426,7 @@ export class UploadHandler {
 
   /**
    * Upload a file as a single multipart POST to the live URL, carrying the
-   * upload_<name>_complete action and the enclosing form fields. The server
+   * upload_<name>_complete action and any lvt-upload-with fields. The server
    * routes the part by the field's configured mode: a Proxied field streams the
    * bytes to OnUpload (zero local-disk staging), a Volume-with-Dir field stages
    * them to Dir. Independent of the WebSocket — Proxied always uses this, and
@@ -453,9 +453,12 @@ export class UploadHandler {
       // Write value fields BEFORE the file part so the server resolves the
       // action regardless of multipart part ordering.
       formData.set("lvt-action", `upload_${entry.uploadName}_complete`);
-      // Then the triggering input's enclosing form fields (e.g. a record id), so
+      // Then the fields marked to travel with the upload (e.g. a record id), so
       // the server can associate the streamed bytes with a record in OnUpload.
       this.appendFormFields(formData, entry);
+      // set(), not append(): if a marked value field happens to share the upload
+      // field's name, the file replaces it. The file part is the whole point of
+      // the request, so it wins the collision.
       formData.set(entry.uploadName, entry.file, entry.file.name);
 
       await this.postMultipartUpload(formData, entry.abortController.signal);
@@ -476,37 +479,49 @@ export class UploadHandler {
   }
 
   /**
-   * appendFormFields serializes the value fields of the form enclosing the
-   * upload's triggering input into formData, ahead of the file part — so a
-   * Proxied OnUpload handler can read them (e.g. a record id) mid-stream. Uses
-   * the browser's native form serialization (successful controls only), skipping
-   * File values (file inputs are sent as the streaming part) and the reserved
-   * lvt-action field. A no-op when the input is outside a form.
+   * appendFormFields serializes the opted-in value fields of the form enclosing
+   * the upload's triggering input into formData, ahead of the file part — so a
+   * Proxied OnUpload handler can read them (e.g. a record id) mid-stream. A
+   * no-op when the input is outside a form or nothing is marked.
    *
-   * A Proxied upload auto-fires on file selection (not an explicit submit), so it
-   * POSTs the whole enclosing form. Password inputs are excluded by name so
-   * credentials in a co-located form never ride along with an upload the user
-   * didn't deliberately submit; keep other sensitive controls out of a form that
-   * contains an auto-upload file input. (A broader opt-in model — only fields
-   * marked to travel — is tracked upstream as a safer-by-default follow-up.)
+   * Opt-in, not opt-out: only fields carrying `lvt-upload-with` travel. A
+   * Proxied upload auto-fires on file selection rather than on an explicit
+   * submit, so the user never gets a submit-time moment to notice what leaves
+   * the page. A denylist fails open — an unanticipated sensitive control (a CSRF
+   * token, a hidden secret, an `autocomplete="current-password"` text input)
+   * rides along silently. An allowlist fails closed: forget to mark the record
+   * id and OnUpload sees a missing field, which is a visible bug rather than a
+   * silent leak.
+   *
+   * The mark is form-scoped, not upload-scoped: a marked field travels with
+   * every upload fired from its form. Marking is by *name*, so marking any one
+   * member of a radio/checkbox group opts in the whole group — the browser's
+   * own successful-control rules then decide which values are actually sent.
    *
    * Values are read here, at upload time (after the upload_start round-trip), not
    * at file-selection time, so edits made between selecting the file and the
    * upload completing are reflected — the freshest form state wins.
+   *
+   * When one selection carries several files, each is POSTed as its own request,
+   * so the marked fields are re-serialized into every one — each request reaches
+   * OnUpload independently and must be self-describing.
    */
   private appendFormFields(formData: FormData, entry: UploadEntry): void {
     const form = entry.sourceInput?.form;
     if (!form) return;
-    // Collect password field names to exclude. Casting to HTMLInputElement is safe
-    // for the type check: only <input> has type="password"; <select>/<textarea>
-    // simply never match and are serialized normally by FormData below.
-    const passwordFields = new Set<string>();
+    // Collect the opted-in names first, then let FormData do the serializing, so
+    // successful-control semantics (unchecked boxes, disabled controls, multi
+    // selects) stay the browser's job rather than ours.
+    const optedIn = new Set<string>();
     for (const el of Array.from(form.elements)) {
-      const input = el as HTMLInputElement;
-      if (input.type === "password" && input.name) passwordFields.add(input.name);
+      const name = (el as HTMLInputElement).name;
+      if (name && el.hasAttribute("lvt-upload-with")) optedIn.add(name);
     }
+    if (optedIn.size === 0) return;
     for (const [name, value] of new FormData(form).entries()) {
-      if (name === "lvt-action" || value instanceof File || passwordFields.has(name)) {
+      // lvt-action is reserved for the action set by the caller, and a marked
+      // file input would double-send its bytes as a value field.
+      if (!optedIn.has(name) || name === "lvt-action" || value instanceof File) {
         continue;
       }
       formData.append(name, value);