Skip to content

feat(protocol): attach private artifacts to file inputs - #169

Merged
SarthakWade merged 7 commits into
mainfrom
feat/artifact-upload
Sep 12, 2026
Merged

feat(protocol): attach private artifacts to file inputs#169
SarthakWade merged 7 commits into
mainfrom
feat/artifact-upload

Conversation

@yashranaway

@yashranaway yashranaway commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds headless upload for attaching a file that already exists in Headless's private artifact store to a visible, enabled file input. Agents may reference only a validated artifact basename.

Arbitrary local-path ingest is intentionally not exposed through CLI, protocol, or MCP. File bytes and source paths never enter protocol parameters, logs, flows, snapshots, diagnostics, or errors. Downloads remain denied.

Security

  • rejects traversal, symlinks, directories, and files outside the private artifact store
  • rejects hidden, clipped, off-screen, obscured, disabled, detached, and non-file targets
  • pins the validated Chromium DOM object so page mutation cannot retarget the upload
  • preserves the existing same-UID trust boundary documented in SECURITY.md

Trusted user-mediated staging of new local files remains tracked by #168 and is not completed by this PR.

Verification

  • 61/61 protocol and security tests
  • agent runtime and benchmark tests
  • secure terminal prompt and MCP integration tests
  • macOS production build
  • macOS conformance reached successfully locally; the full GUI suite is required in CI via the macos-e2e label

@yashranaway yashranaway self-assigned this Sep 10, 2026
@yashranaway

Copy link
Copy Markdown
Collaborator Author

Merge after #170. Both PRs started from the same main; they overlap on Protocol.swift, CLI.swift, HostCore.swift, AgentRuntime.js, tests, and docs. Allowlist is the independent security boundary. Rebase this branch onto #170 after that merges.

Architecture decision numbering: this PR is 23. #170 is 22.

Agents can copy a local fixture into the private artifact store
(`artifacts add` / `artifact.add`) and attach it to a file input
(`upload`) by basename. File bytes never travel on the socket.
Linux Chromium uses DOM.setFileInputFiles; WebKit returns
UNSUPPORTED_CAPABILITY. Downloads stay denied.

Closes #168
@yashranaway

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main (settings, auth, isolated sessions). Mergeable with main. If #170 merges first, this will likely need another rebase because both still touch CLI/protocol/docs.

@SarthakWade SarthakWade left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR must not merge in its current design because it crosses the repository's local-file security boundary.

artifact.add is an agent and MCP protocol command that accepts any absolute regular file readable by the host. Only the destination basename and extension are constrained. An agent can therefore ingest a sensitive local file under an allowed .txt or .json name, navigate to a page, and attach it through Linux upload. Keeping bytes off the socket does not prevent exfiltration. This directly contradicts SECURITY.md, which classifies reading outside the artifact store as a vulnerability, so architecture decision 23 needs to be revised before implementation. Ingest should require trusted user mediation, be limited to user-approved roots with descriptor-relative no-follow access, or remain outside the agent/MCP surface.

Additional blockers are an unbounded read before the 5 MiB check and ambiguous completion semantics after DOM.setFileInputFiles. There are also target-state and capability-hint gaps described inline. The existing pathname handoff to Chromium is not identity-pinned after the store lock is released; while same-UID filesystem access is a documented limitation, the design should explicitly document that remaining race or introduce a brokered descriptor-safe handoff where the engine permits it.

GitHub currently reports the branch as mergeable with no conflicts and directly based on current main. The required Linux E2E check is failing at authentication-storage-state with a closed DevTools pipe before the new upload scenario runs, so this head has no successful Linux upload E2E result. Other checks passed and macOS E2E was skipped.

Comment thread apps/headless/Sources/HeadlessProtocol/Protocol.swift Outdated
Comment thread apps/headless/Sources/HeadlessProtocol/Artifacts.swift Outdated
Comment thread apps/headless/LinuxHost/BrowserProcess.swift Outdated
Comment thread apps/headless/Sources/HeadlessProtocol/Resources/AgentRuntime.js
Comment thread apps/headless/Sources/HeadlessProtocol/Resources/AgentRuntime.js Outdated
Ingest is a local CLI/TTY operator path, never a protocol or MCP command.
Upload still attaches store basenames only. Bound the ingest reader,
treat successful CDP attachment as completion, revalidate file inputs
before attach, and suppress WebKit upload hints.
@yashranaway

Copy link
Copy Markdown
Collaborator Author

Addressed the CHANGES_REQUESTED review on 848aaf0.

P0 artifact.add (Protocol.swift ~284)
Ingest is no longer a protocol command. artifacts add parses as LocalCommand.artifactsAdd, runs ArtifactStore.ingest in the CLI process, and sends nothing over the Unix socket. MCP rejects it the same way it rejects credentials. artifacts list and upload stay on the protocol; upload still names a store basename only. A raw artifact.add request fails to decode. Architecture decision 23 is revised: the agent cannot read outside the artifact store.

P1 readToEnd (Artifacts.swift ~291)
Ingest still does lstat / O_RDONLY|O_NOFOLLOW|O_CLOEXEC / fstat regular-file checks, but st_size is not the read bound. Bytes are read in 64 KiB chunks and stopped as soon as the next chunk would exceed 5 MiB. The protocol suite covers a 5 MiB + 1 file on that path.

P1 DOM.setFileInputFiles (BrowserProcess.swift ~571)
fileInputPrepare captures {ref, role, name} before attach. A successful CDP setFileInputFiles is completion; the node is not queried again. linux-e2e covers an input whose change handler removes it and one that navigates away.

P2 fileInput (AgentRuntime.js ~616)
Immediately before attachment the target must still be an enabled, visible input[type=file] (non-zero box, not display:none / visibility:hidden). Hidden and disabled uploads fail in jsdom and linux-e2e, including a previously issued ref after the input is hidden.

P2 actionHints (AgentRuntime.js ~89)
Hosts inject globalThis.__headlessFileUpload (true on Linux Chromium, false on WebKit). Inspect advertises upload only when that flag is true. macos-e2e asserts WebKit inspect does not advertise it.

Protocol tests 60/60, MCP stdio, pnpm test:runtime, and Linux Docker E2E all passed on this commit.

@yashranaway yashranaway added the macos-e2e Run the macOS WKWebView E2E suite label Sep 12, 2026
# Conflicts:
#	apps/headless/Tests/fixture-server.mjs
#	apps/headless/Tests/linux-e2e.sh
#	docs/roadmap/architecture-decisions.md
@SarthakWade SarthakWade changed the title feat(protocol): add artifact ingest and file upload feat(protocol): attach private artifacts to file inputs Sep 12, 2026

@SarthakWade SarthakWade left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the final implementation and verified the complete CI matrix. Uploads are limited to validated files already inside the private artifact store, target state is revalidated, Linux pins the validated DOM object through attachment, and the remaining same-UID pathname limitation is documented. Protocol, runtime, Linux E2E, macOS build, and macOS E2E checks all pass.

@SarthakWade
SarthakWade merged commit ab4e3e8 into main Sep 12, 2026
8 checks passed
@SarthakWade
SarthakWade deleted the feat/artifact-upload branch September 12, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macos-e2e Run the macOS WKWebView E2E suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants