Skip to content

feat(computer): download generated workspace files - #698

Merged
davidmckayv merged 6 commits into
CopilotKit:mainfrom
Harbor404:feat/370-channel-generated-files
Oct 2, 2026
Merged

davidmckayv merged 6 commits into
CopilotKit:mainfrom
Harbor404:feat/370-channel-generated-files

Conversation

@Harbor404

Copy link
Copy Markdown
Contributor

What this changes

Implements the download path for generated workspace files in #370.

  • Adds raw-byte GET /files/download?path=... to agent-computer; PDFs, images and archives are streamed intact rather than decoded through the 64 KB text-read path.
  • Adds the public, session-authenticated GET /api/computers/:botId/files/download?path=... API.
  • Adds an independent computer_download_file acting tool whose policy intent is download_file. It is not folded into computer_read_file, so a deployment can allow reading while refusing downloads, or the reverse.
  • Enforces the existing workspace path boundary via canonical resolvePath: absolute paths and .. are refused, symlink escapes are refused, and directories are not downloadable.
  • Adds a 100 MiB download ceiling with 413. Files at or below the ceiling are streamed through Bun.file/sendfile, not buffered into a string.
  • Uses one shared response contract: application/octet-stream, Content-Disposition: attachment, X-Content-Type-Options: nosniff, Cache-Control: private, no-store, with CRLF/quote/backslash-safe RFC 5987 filenames.
  • Keeps existing read/list/write APIs unchanged and introduces no new state, table, listener, or permission model.

Where it runs

  • New state that outlives a request? None. The download is a stateless pass-through from the server gateway to the already-selected computer.
  • What happens on the second replica? Any replica can answer. Ownership is checked through the existing canUseBot middleware and the computer is addressed through the existing provider/gateway path.
  • Anything serialised? No read-modify-write state is introduced.
  • Anything fanned out to a browser? No. This is a request/response stream.
  • New listener, port, or schedule? None; the route is under the existing /api/computers ingress.

Boundary and audit

  • Every acting call still goes through the gateway: resolve, decide, audit, then act.
  • New refusals and new failures each write a row. Allowed, refused, and failed download attempts are covered by tests.
  • Nothing new is trusted from the client that the server can resolve itself. The policy sees the server-side file path context and the computer independently confines it to its workspace.
  • Per-Bot ownership is checked before the gateway is asked; denied ownership returns the existing uniform 404.
  • Errors are explicit: 400 malformed path/request, 403 workspace or policy refusal, 404 missing Bot/file, 413 size cap, 500 unexpected failure, with existing 503 for an unavailable computer.

Changelog

  • Added an entry under Unreleased in CHANGELOG.md.

Proof

Local head: 0b75cb4.

  • OPENBOT_FILE_DOWNLOAD_HTTP=1 bun test agent-computer/tests/file-download-http.test.ts — 9 pass, 0 fail. Covers real HTTP process, binary integrity over 64 KB, auth, missing file, directory, traversal, symlink escape, 413, and header injection.
  • bun test agent-computer/tests — 351 pass, 34 skipped, 0 fail.
  • bun test shared/*.test.ts — 92 pass, 0 fail.
  • server/tests/computer-*.test.ts excluding DB-backed *.integration.test.ts — 356 pass, 0 fail.
  • Focused server contracts (computer-download-routes, computer-client, computer-gateway, computer-policy) — 157 pass, 0 fail.
  • bun run typecheck — pass across app/server/worker; agent-computer bun run typecheck — pass.
  • bun run lint — pass.
  • bun run format:check — pass.

Risks / residual verification

  • The 100 MiB cap is intentionally a constant in DEFAULT_WORKSPACE_LIMITS; making it deployment-configurable can be a follow-up if operators need a different ceiling.
  • DB-backed server/tests/computer-*.integration.test.ts were not run locally because no dedicated TEST_DATABASE_URL is configured in this workspace. CI should run them.
  • The full root bun test locally is blocked by the same pre-existing TEST_DATABASE_URL requirement in integration files; all affected non-DB suites above are green.

davidmckayv
davidmckayv previously approved these changes Oct 2, 2026
@davidmckayv
davidmckayv enabled auto-merge (squash) October 2, 2026 03:56
auto-merge was automatically disabled October 2, 2026 04:59

Head branch was pushed to by a user without write access

davidmckayv
davidmckayv previously approved these changes Oct 2, 2026

@davidmckayv davidmckayv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The current head 6f0f48f matches the accepted source disposition from the refreshed triage. Required CI must pass before landing.

@davidmckayv
davidmckayv enabled auto-merge (squash) October 2, 2026 16:52
@Harbor404

Copy link
Copy Markdown
Contributor Author

Hi @davidmckayv — thanks for the approval on 6f0f48f0.

One thing still blocks the merge: the CI workflow run for this head is stuck at action_required with 0 jobs created (run 36966932365), so the required checks never execute and branch protection keeps the PR BLOCKED.

Since this is a first-time-contributor PR, that run needs a maintainer to approve/trigger it (Actions → the run → "Approve and run"). Could you or another maintainer approve it? No code changes are needed on my side — the head is exactly the one you approved.

Thanks!

# Conflicts:
#	CHANGELOG.md
auto-merge was automatically disabled October 2, 2026 18:52

Head branch was pushed to by a user without write access

@Harbor404

Copy link
Copy Markdown
Contributor Author

Conflict resolved by merging the latest main (764a8fb) into this branch in merge commit 05af765. Only CHANGELOG.md conflicted; both the new main entries and this PR's generated-file download entry were retained.

The previous approval may now be stale because the head changed and may need to be renewed. Focused tests and typechecks pass locally, but CI still requires a maintainer to approve the workflow run for this new head.

download_file was not one of the intents the switch gates, so a member
could still download workspace files after an administrator turned
Cloud computer use off. It now needs the same capability as reading or
writing those files.
@davidmckayv

Copy link
Copy Markdown
Contributor

I pushed one commit. download_file was missing from COMPUTER_INTENTS in server/src/admin/controls.ts, so a member could still download workspace files after an administrator turned Cloud computer use off. A download now needs the same capability as reading or writing workspace files. There is a test for it in server/tests/enterprise-controls.test.ts.

@davidmckayv
davidmckayv merged commit a34d254 into CopilotKit:main Oct 2, 2026
19 checks passed
davidmckayv added a commit that referenced this pull request Oct 2, 2026
…723)

#698 made agent-computer/src/index.ts import ../../shared/file-download at
runtime. The computer Dockerfile copies only agent-computer/src, so the
image built and then exited on start with "Cannot find module". The other
shared imports there are type-only and erased, which is why this one was
the first to break. The one file now goes to /shared, where the relative
path resolves from /app/src.

The component image job only built the Dockerfiles, and a build succeeds
with a missing runtime module, so it now also starts the computer image
and waits for /health.
davidmckayv added a commit that referenced this pull request Oct 2, 2026
A keep-both merge left the tail of the routine failure-count entry,
with its migration sentence, stranded under the download entry; it is
back where it belongs. The download entry now says the Cloud computer
use switch refuses downloads, as the maintainer fix to #698 made it, and
the saved group reply entry says interrupted consent cards and handoffs
are still posted, as the fix to #713 made them. The README settings
table gains OPENBOT_SELF_HOST_BANNER.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants