Repository navigation
fix: carry a composer attachment into a Cowork sandbox (issue #1065) - #1735
Conversation
Work mode refused every attachment in the composer, so the most obvious thing a person tries had no path on that half of the surface. The refusal was honest at the time: the run backend accepted a pack and a prompt and nothing else, so a file had nowhere to go. It has somewhere to go now. POST /v1/agent/tasks carries the documents inline, control-plane threads them to the host launcher on the same in-memory task that already carries the bearer JWT and the per-task gateway key, and the launcher writes them into the session's working directory beside the pack, which is the one place the sandboxed agent reads anything from. The names go on the run's initial message so the agent knows they are there; the content does not, because it is on disk precisely so it does not have to fit in a prompt. The text travels inline rather than as a file id because the sandbox holds no Hive credential and has no route to the storage a chat attachment lives in. The browser that uploaded it is the one party already authorized to read it, so this adds no read path, no permission and no widening of who can see whose documents. Bounds, stated rather than discovered: five attachments, 256 KiB of combined text, refused before a credit hold is taken and before a row is created. An attachment name is validated at edge-api, at the proxy and again in the launcher, which is the process that turns one into a path; a name that collides with a pack file is kept under a free name rather than replacing the pack's own instructions with user supplied text.
sakibsadmanshajib
added
priority:critical
📝 WalkthroughWalkthroughWork mode now accepts validated attachments, forwards their extracted text through task creation and launcher requests, writes them into the sandbox working directory, and exposes them through file listings. Frontend, service, engine, integration-test, and proof-harness coverage was added. ChangesWork-mode attachment pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟠 High · up to Work-mode attachments now carry document contents across service boundaries and into sandboxes, but the current implementation sends them over a cleartext HTTP hop alongside credentials, exposing sensitive data to network observers. Some valid attachments can also fail due to encoded-size limits, while restart and mixed-version paths may drop inputs. The change is not merge-ready until these transport and rollout risks are addressed or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant Composer
participant EdgeAPI
participant ControlPlane
participant AgentEngine
participant SandboxWorkspace
Composer->>EdgeAPI: Submit cowork task with attachment name and content
EdgeAPI->>ControlPlane: Forward validated attachments
ControlPlane->>AgentEngine: Launch task with attachments
AgentEngine->>SandboxWorkspace: Write attachment file
SandboxWorkspace-->>AgentEngine: File available in working directory
AgentEngine-->>Composer: Task exposes attachment through files listing
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 68.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 47 functions across 27 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Streaming delta capture, run
|
Streaming delta capture, run
|
Adversarial review, four streamsRecorded here rather than in a private report, per the pipeline. Each finding says what was done about it. Stream 1: CodeRabbit CLI — SKIPPED
Stream 2: security pass, because this touches storage, file paths and a model promptFour findings, three of them fixed on this branch, one accepted and stated. S1, fixed. A newline in an attachment name would have forged a line in the model's prompt. The names are repeated back to the agent as a bullet list on the run's initial message. The first version refused S2, fixed. S3, considered and correct as written. The name is validated four times. Browser, proxy, edge-api, launcher. That is deliberate rather than redundant: the launcher is the process that turns a name into a path and it does not trust the three hops above it, which is the same posture S4, accepted and stated rather than fixed. This adds bounded per-task memory to an unbounded submit path. Nothing rate limits Not findings, checked and clear. No new authentication or authorization path: the content comes from the submitting person's own browser session, which is the one party already able to read it. No cross-tenant read, because nothing here reads a row belonging to anyone. Content is written to a file, never spliced into the system prompt. Nothing new is logged, and the launcher's existing Stream 3: TypeScript passT1, fixed. T2, fixed during the write. A space is legal in a file name. The first version of T3, considered. T4, considered. The refusal copy takes a widened type. Stream 4: plain adversarial pass, ticket firstA1. Does it actually close #1065? Half of it, and the pull request says so in its title and its A2. Is the failing test failing for the real reason? Yes, and it was checked rather than asserted. Before the implementation existed the engine test failed with A3. What breaks if the launcher is older than control-plane? Nothing. A4. Does the in-process arm silently lose attachments? It did in the first draft. A5. What is not proven, said plainly. The frames on this pull request show the composer accepting the attachment in Work mode and the request leaving the browser with the document's text in it. They do not show the file inside a sandbox, because Apptainer is |
Streaming delta capture, run
|
Cowork visual proof, captured in CICaptured against a stack booted from launch-liveness-01-empty-console
launch-liveness-02-sandbox-launched
Run log (screenshot stamps carry no URL, and the log is redacted and linted by `lint:proof-tokens`) |
Cowork visual proof, captured in CICaptured against a stack booted from launch-liveness-01-empty-console
launch-liveness-02-sandbox-launched
Run log (screenshot stamps carry no URL, and the log is redacted and linted by `lint:proof-tokens`) |
Visual proofIssue 1065, Cowork half, against the chat image built from this branch. Frame 1: Work mode with the file chip in the composer and no refusal, a state that is unreachable on main because the send is refused there. Frame 2: the run turn carrying the same chip on the user message. The intercepted create request carried name=service-record.txt and the file's full text including the unique code BRACKEN-1065-QX; log in docs/proof/cowork-attachment-1065-2026-09-02/. The sandbox side is not claimed by these frames and is proven by the engine tests, because Apptainer cannot run on this box.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Streaming delta capture, run
|
Streaming delta capture, run
|
Cowork visual proof, captured in CICaptured against a stack booted from launch-liveness-01-empty-console
launch-liveness-02-sandbox-launched
Run log (screenshot stamps carry no URL, and the log is redacted and linted by `lint:proof-tokens`) |
Streaming delta capture, run
|
Cowork visual proof, captured in CICaptured against a stack booted from launch-liveness-01-empty-console
launch-liveness-02-sandbox-launched
Run log (screenshot stamps carry no URL, and the log is redacted and linted by `lint:proof-tokens`) |
Cowork visual proof, captured in CICaptured against a stack booted from attachment-reaches-the-sandbox-01-attachment-inside-the-sandbox
Run log (screenshot stamps carry no URL, and the log is redacted and linted by `lint:proof-tokens`) |
Rebased onto main, and re-proven on the merged tree
Conflicts, five files, all one change: #1729 landed the inferred Cowork pack while this branch was open. Both sides kept in every case.
Also re-applied to the callers Verified on the merged tree rather than assumed clean. Go across edge-api, control-plane and agent-engine, and the chat frontend suite at 411 tests in 27 files. Then the real Apptainer scenario dispatched again, run That is the same assertion as before, on the code that would actually land, with #1729's inference in front of it. One check that was failing earlier and is not now: CodeQL raised two high-severity path-injection alerts on the working directory build, because the task id reaches the launcher as JSON and becomes a filesystem path. The type made it safe and nothing said so, which is what the query objects to. |
sakibsadmanshajib
left a comment
sakibsadmanshajib
left a comment
There was a problem hiding this comment.
Independent adversarial review, second pass. I did not write this change and I read the pushed diff at the PR head rather than the author's report. CodeRabbit's App reviewed it and all four of its threads are resolved; the CodeRabbit CLI was rate limited, so treat that one stream as SKIPPED rather than as a clean pass.
The headline question: can a malicious file name write outside the workspace?
No. I attacked this specifically and found no escape.
attachmentFileNamerefuses/,\, every C0 control plus DEL,.,.., a name over 255 bytes, and any name wherefilepath.Base(name) != name.TrimSpaceruns first, so a padded" ../x "is still refused. Absolute paths and Windows separators are covered by the backslash and separator checks. A name that is only dots beyond..(...) is a legal file name and lands as one, harmlessly.- Every write goes through
os.Rootrooted atworkingDir, so a symlinked component cannot be crossed even if a name got past the string check. - The PR #1690 family does not exist here.
materializePackalready refuses any pack containing a symlink, so no symlink can be sitting inworkingDirwhen attachments are written, andO_EXCLfails withEEXISTon an existing symlink regardless, including a dangling one. - Two concurrent tasks cannot reach each other.
workingDirisWorkspaceRoot/<task uuid>, freshly created0700, and the id is now regex checked at the one place it becomes a path segment. No attachment name can contain a separator and no write leaves the root. - Unicode normalisation and case folding do not give an overwrite. On a folding or normalising filesystem the second write hits
EEXISTand is renamed; on ext4 the two names are simply distinct. Nothing is clobbered in either case. - Empty after sanitisation is refused, and the length cap is measured in UTF-8 bytes at all four hops now, so the browser and the launcher agree rather than accepting then refusing after the composer has been cleared.
The bounds
Enforced server side and not bypassable by skipping the console. validateAttachments runs in edge-api's handleCreate between the body decode and everything that costs anything, and I read the ordering rather than taking the comment's word for it: decode, pack trim, validateAttachments, AuthorizeProject, sessionbilling.Probe, then client.Create. A refused request therefore takes no credit hold and creates no row, and the tests assert createCalled is still false on each refusal. The proxy's copy is a shape check on a rebuilt body, which is the right posture.
The 256 KiB is measured on the extracted text, server side, in bytes. A small upload that expands during extraction is therefore refused on what it actually became rather than on what was uploaded, which is the correct end of that question.
One accuracy note on the PR body: the name is validated at four hops, but the count and byte caps are enforced at two, the browser and edge-api. Control plane's internal handler and the launcher re-validate names only; their sole bound on quantity is the 2 MiB body reader. That is defensible on a surface behind RequireInternalToken, but "validated at all four hops" reads as covering the caps and it does not.
Content injection
The extracted text never reaches the prompt. It goes to disk and a test fails if the body appears in the initial message. Good. The names do reach the prompt, unfenced, which is the one thing I would still change; see the inline comment on withAttachmentNote.
The wire compatibility fix
The omission is correct in both directions and the test now asserts key absence rather than a nil value, which is the distinction that matters. New launcher against old control plane sees no key, decodes nil, and behaves exactly as before.
On the version skew premise itself: the launcher IS updated by the deploy workflow. deploy-demo-box.yml has an "Install and restart the agent-engine launch daemon" step that runs scripts/install-agent-engine-host.sh, which does go build ... ./apps/agent-engine/cmd/agent-engine against the same checkout, and that step is ordered ahead of the stack restart and exits 1 on failure, which skips every later step. So control plane cannot get ahead of the launcher on this path, and skew is not the normal state for this deployment. The residual risk is small but real and silent; inline comment on serve.go.
The rebutted item, #1742
The rebuttal holds. I checked the hop rather than accepting the label. hive_agent_proxy.py calls OPENAI_API_BASE_URL, which is http://edge-api:8080/v1 inside the compose network, with the shim key on Authorization and the user's token on X-Hive-Upstream-Auth. Open WebUI's own chat completions have always taken that hop with the same credentials and with full message content, and project document text has crossed it since #1358 and #1707. This is new content on an existing channel, not metadata becoming documents, so it is not a material escalation of that hop's posture. Tracking it as #1742 rather than fixing it here is the right call.
The chat half
Verified rather than accepted. vendor/open-webui/src/lib/hive/chat-noise-guards.test.ts exists on main and asserts not.toContain('File not found.') across all five composers that carried the copy pasted handler. The claim is true and this branch does not re-break it.
On the issue reference: the body says "Refs #1065", not Closes #1065, so GitHub will not close the issue on merge. Given both halves are now genuinely done that is a manual close to remember rather than a dishonest claim.
The evidence
The CI scenario exercises the real write path, not a stub. It intercepts the console's own POST /v1/agent/tasks and adds attachments, so the browser to proxy hop is the part it skips, and everything from edge-api through control plane, the launcher and a real Apptainer launch is exercised for real. The assertion reads the file's content off RUNTIME_DIR/workspaces/<task id>/service-record.txt, which is the launcher's own workspace directory, and checks a token generated for that run, then confirms the same name through GET /v1/agent/tasks/{id}/files. A row, a 201 or a name in a list would not have passed it.
Two things it does not prove, worth stating rather than leaving implied. It does not exercise the composer's own assembly of that request, which the PR says and covers with unit tests. And it does not prove the sandboxed agent could read the file: attachments are written 0600, and the reasoning that this is fine is that the Apptainer argv carries no --fakeroot and no user namespace remap, so the container runs as the launcher's own uid. That is sound inference, not measurement, and the scenario's own instructions ask the agent to read the file, so asserting on its answer would have closed the gap for free.
Verdict
Approve, with no blocking findings. The traversal question the change lives or dies on is answered correctly and defended in depth, the bounds are enforced where money is not yet at stake, and the proof is real. Four non-blocking findings are posted inline, plus two below that have no diff line to attach to.
Non-blocking, no diff anchor.
- The message queue is mode blind, and attachments now ride into it.
processNextInQueue(Chat.svelte:1862) drains throughsubmitPromptregardless of$composerMode, so a Work mode send while a run is still generating is enqueued and later replayed as a chat completion. That is pre-existing since #944 and the prompt already took that route, but before this change the queued entry could not carry a file and now it can, so the document goes to a chat model instead of the sandbox with nothing said. Worth its own issue. - Memory amplification of the unbounded launch goroutines.
Service.CreateTask's comment already documents that nothing bounds how many launch goroutines are in flight and points at #900. Each of those now retains up to 256 KiB of document text for the goroutine's life instead of a JWT and a prompt. It does not change the shape of #900, but it changes what an unbounded number of them costs.
…bmit window Two findings from the security review, both real. The file names go into the run's initial message as a bulleted list the agent reads as instructions, and they went in verbatim. Separators and control characters are refused at every hop so a line break was never available, but everything else on one line is a legal POSIX file name, and up to 255 bytes of it times five attachments is attacker chosen text sitting in the agent's own instructions. The realistic path is a document received from somebody else and attached without the name being read closely. Each name is now written with %q, so it arrives quoted, any quote inside it is escaped, and it cannot terminate its own line. Gathering the attachments before the composer is cleared is the right tradeoff, because a refusal should not cost the person the message they just wrote. It also put the first await on the cowork path in front of the clear, up to five reads long, with the prompt and the file chips still populated and nothing guarding a second Enter. That was two createTask calls, two credit holds and two sandboxes from one person pressing a key twice, which is a money path rather than a cosmetic one. A flag set for the duration of the reads and released in a finally closes it, and it is released before the clear rather than held for the send, so the message queue path underneath keeps working. Also states precisely what control-plane does not re-check. The name is validated again by the launcher, which is the process that turns one into a path. The count and the byte cap are enforced in the browser and in edge-api and nowhere after that, so "validated at every hop" was true of one field and read as covering three.
Streaming delta capture, run
|
Review round two addressed, both fixes pushed in 31f34d1Fix 1, the unfenced filename. Fix 2, the double submit window. Filed rather than fixed, all three linked from their threads and from the body:
Correction I owe the record. The pull request body said version skew between control-plane and the launcher was a live risk. It is not the normal state:
Also corrected in the body: "validated at all four hops" was written about the name and read as covering the caps. The count and the 256 KiB total are enforced in the browser and in edge-api only; past that the sole bound is the body reader. The comment at the field now says so too. |
Cowork visual proof, captured in CICaptured against a stack booted from launch-liveness-01-empty-console
launch-liveness-02-sandbox-launched
Run log (screenshot stamps carry no URL, and the log is redacted and linted by `lint:proof-tokens`) |
Cowork visual proof, captured in CICaptured against a stack booted from attachment-reaches-the-sandbox-01-attachment-inside-the-sandbox
Run log (screenshot stamps carry no URL, and the log is redacted and linted by `lint:proof-tokens`) |
|
Re-proven on the final commit, run One red check on the way there, recorded rather than quietly re-run: the automatic proof job failed once on State: |
Second bug-log reconciliation batch of the day. The first batch (#1743, merged 2026-09-02T19:44:04Z) appended 197 entries from 167 PRs, taking `.wolf/buglog.jsonl` on `main` to 511 lines. This batch sweeps every PR merged after that point which carried a `## Buglog entry` heading in its body, appending 14 entries from 7 PRs: - #1733 (1 entry) - #1735 (1 entry) - #1739 (6 entries) - #1740 (1 entry) - #1748 (1 entry) - #1749 (2 entries) - #1756 (2 entries) Checked and excluded: - #1727 carries no buglog entry. It is a docs/process PR (tracking-discipline rule), not a bug fix, and its body mentions `.wolf/buglog.jsonl` only in passing prose. - #1715, #1729, #1731 and #1734 merged before this batch's window and are already present in the first batch (#1743). Verified by id/error_message lookup against the 511 lines already on `main`. Every entry was extracted from its source PR body, parsed as JSON to confirm it is well-formed, and checked for the required `error_message`, `root_cause`, `fix` and `tags` fields (all present, none reconstructed). No duplicates were found against the existing 511 lines or within this batch, checked by both `id` and exact `error_message` match. Diff is exactly one file, 14 insertions, 0 deletions. The first 511 lines byte-match `main`'s current copy (verified with `diff` against `git show origin/main:.wolf/buglog.jsonl`). This PR was not opened on a fix or feature branch, per `.claude/rules/openwolf.md`: it is the dedicated buglog-only PR, branched directly from `main`, diffing only `.wolf/buglog.jsonl`. Refs #873 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
Closes #1065. Closes #847 was already done before this branch; see the ground-truth section below.
Changed from
RefstoClosesafter the independent security review judged both halves delivered, and checked rather than accepted: #1065's own acceptance text is two sentences, and both are now satisfied. The chat half, a file attached in chat reaching the model in the same turn, was already true onmainand is verified rather than assumed. The Cowork half, a file existing inside the sandbox and listing in the panel, is proven on a real Apptainer launch below.One thing not shown as a pixel, so the claim stays checkable: the Working folder is proven through the route the panel calls,
GET /v1/agent/tasks/{id}/files, rather than by a screenshot of the panel rendering the row. This pull request does not touch that renderer.Ground truth first, because the issue names two surfaces
Chat half: already fixed, verified twice, not re-broken by this branch. #1065 cites #847 as its chat-side symptom. #847 was closed on 2026-08-30 after a live re-measurement on 2026-08-29 against
c9e1419b: an attachment uploaded, processed, and its content reached the model in the same turn with a citation. TheFile not found.literal it was named for is gone from every one of the five composers that carried the copy-pasted handler, andchat-noise-guards.test.tspins its absence. PR #1707, merged earlier today, exercised the same manual attach path again while proving something else. So the chat composer needs no code change, and this branch makes none to it beyond the Work-mode branch of the shared submit handler.Cowork half: genuinely broken, and this is it. The break is one line, and it is the shape this repository keeps producing.
mainsubmitHandler(vendor/open-webui/src/lib/components/chat/Chat.svelte:2569) refuses: "Attachments are not supported in Work mode yet."createTasksent{pack, instructions}and nothing else (vendor/open-webui/src/lib/hive/agentTasks.ts:282),POST /v1/agent/tasksdecoded exactly those two fields plusproject_id(apps/edge-api/internal/agenttask/handler.go:135), andRemote.Launchput five keys on the wire, none of them a document (apps/control-plane/internal/agentengine/remote.go:67).workingDirinSandboxEngine.Launch(apps/agent-engine/internal/engine/engine.go:632), the directory bind mounted as/workspace. Nothing butmaterializePackever wrote to it.So there was no reader because there was no writer, and no writer because there was nothing on the wire to write. The refusal was the only honest thing in the chain.
What now reaches the sandbox
The document's extracted text, written to a real file in the agent's working directory before the conversation starts, and the file's name on the run's initial message so the agent knows to open it.
The chain, one thin hop per layer:
Chat.sveltegathers the attachments before the composer is cleared, so a refusal costs the person neither their prompt nor their chips, and renders the same file chips on the user turn a chat turn renders.coworkAttachments.ts(new, Hive authored, unit tested) reads each file's extracted text back fromGET /api/v1/files/{id}, refuses what the sandbox cannot resolve, and enforces the count and byte caps in the browser so the person is told before the send rather than by a 400.hive_agent_proxy.pyrebuilds the list field by field, the way it already rebuildspackandinstructions, and never forwards the submitted body wholesale.apps/edge-api/internal/agenttaskvalidates, then forwards. Validation runs ahead of the project check and the solvency gate, so a request that cannot be honoured never takes a credit hold and never creates a row.apps/control-plane/internal/agenttaskcarries them on the in-memoryTask, exactly whereBearerJWTandLLMAPIKeyalready live, and does not persist them. A task row is a control record, not a copy of the customer's documents.buildAgentEnginehand the engine the same task:Remote.Launchputs them on the/launchbody, and the in-processagentengine.Engineconverts them too, so a deployment cannot quietly lose attachments depending on how it is wired.apps/agent-engine/internal/engine/attachments.go(new) writes them intoworkingDirthrough anos.Root, aftermaterializePack, withO_EXCL.Why the text travels inline, and the ceiling that buys
The sandbox is behind
--network nonewith an egress proxy in front of it. It holds no Hive credential and has no route to the object storage a chat attachment lives in, so something has to hand it the bytes. The browser that uploaded them is the one party already authorized to read them, which is why the text rides the create request rather than a file id the sandbox would have to resolve.That means no new read path, no new permission, and no widening of who can see whose documents. It also means a bound: five attachments, 256 KiB of combined text, refused with a message rather than truncated. Truncation would hand the agent a document that stops mid sentence and let it answer confidently from half a file, which is the same silent-failure class this issue is about. The upgrade path, when a run needs a 25 MB PDF verbatim, is for the launcher to fetch the document itself, and that needs a credential and a route it does not have today. The number is written down in three places that must agree, each pointing at
apps/edge-api/internal/agenttask/handler.goas the one that enforces it.Retrieval scope: untouched, stated explicitly
This adds no retrieval. It does not read
public.rag_documents, does not touch/v1/rag/*, does not resolve a collection, and does not callget_sources_from_items. A Cowork run gets the bytes the submitting person's own browser already held and nothing else.In particular it neither helps nor worsens #1643, the tenant readable RAG store: that issue keeps its full scope. The project half of this problem, a run consulting a Project's documents, is #1312 and task 8 of the Projects unification spec, and both still own it. Wiring a
project_idretrieval into the launcher here would have meant exactly the widening #1643 warns about, on a path with no ownership check written yet.Untrusted input, since this is a file path and a model prompt
A name is not a path.
../escape.txt,nested/file.txt,.,.., a backslash, a control character and anything over 255 bytes are refused, in the browser, at the proxy, at edge-api and again in the launcher. The launcher checks it a fourth time on purpose: it is the process that turns a name into a path, and it does not trust the three hops above it, exactly as it already does forTask.Pack.That four-hop claim is about the name and nothing else, stated here because it reads as covering more. The count and the 256 KiB total are enforced in the browser and in edge-api's
validateAttachmentsand nowhere after that, so past edge-api the only bound left is the body reader. Deliberate: a second copy of the quantity policy in control-plane would be the two disagreeing copies that package already refuses to keep for packs, and that surface is behindRequireInternalTokenrather than customer reachable. The comment at the field says the same thing.A name is not a sentence either. The names go on the run's initial message, and a file name is free text with a small alphabet removed: refusing separators and control characters takes the line break away and nothing else. Each name is written with
%q, so it arrives quoted, a quote inside it is escaped, and it cannot terminate its own line.TestSandboxEngine_Launch_FencesTheAttachmentNameInThePromptuses a name shaped like an instruction.One person's keypress is one run. Gathering the attachments before the composer is cleared is what keeps a refusal from costing someone their message, and it put the first
awaiton the cowork path in front of the clear. A second Enter in that window meant twocreateTaskcalls and two credit holds.coworkGatherInFlight, released in afinallybefore the clear, closes it without holding the flag through the send, which would have broken the message queue path underneath.A traversal that the string check cannot see. Every write goes through
os.Rootconfined toworkingDir, so a symlinked subdirectory cannot be crossed even if a name got past the check.A name cannot replace a pack file. The pack is planted first and every attachment is created
O_EXCL. An attachment calledAGENTS.mdis kept asAGENTS-1.md; it does not overwrite the pack's own instructions with user supplied text, which would be both a broken pack and a very short path to a prompt injection. The rename is bounded.Content is untrusted, and stays that way. It is written to a file, not spliced into the system prompt. Only the file names go on the initial message, which is the same untrusted-document posture the pack's own handling already carries and which
listWorkspaceFilesalready reasons about.Credentials. Nothing new is logged. The launcher's existing
redactCredentialson the launch error path is unchanged and still covers both keys that request carries.Tests
Red first, and red for the right reason: every new assertion reads the value back at the far end rather than checking that it was sent.
apps/agent-engine/internal/engine/attachments_test.goreads the attachment's content out of the directory the launch bind mounts as/workspace, asserts it appears in the working folder listing with the right size, asserts a colliding name leaves the pack'sAGENTS.mdbyte for byte intact and keeps the attachment anyway, asserts seven malformed names each fail the launch and leave no working directory behind, and asserts the initial message names the file without carrying its content.apps/control-plane/internal/agentengine/remote_test.godecodes the actual/launchbody a fake daemon received, which is the seam this defect class breaks at, and asserts the key is absent when the task has no attachments so an older launcher sees the body it always did.apps/edge-api/internal/agenttask/attachments_test.goasserts what reached control-plane, and that each refusal happens withcreateCalledstill false, so a bad request cannot take a hold.vendor/open-webui/src/lib/hive/coworkAttachments.test.tscovers the content read, both places the text can already be, the four refusals, and that the cap is measured in bytes rather than code units, since a Bengali or emoji-heavy document is three times its string length.coworkMode.test.tspinned the blanket refusal string as a fixed behaviour from the feat(chat): make Cowork a mode of the composer instead of a destination (#944) #1193 review. It is no longer a behaviour, and leaving it would have made this fix unmergeable for a reason the file did not explain.Proven end to end on a real Apptainer sandbox
Written after the fact, because the pull request originally said this could not be shown before merge. That was true of the development box and not of CI.
agent-visual-proof.ymlstands the real thing up per run fromrefs/pull/1735/merge; a scenario was added to its harness and dispatched at this pull request. Run 33668985745,success:Both halves of the issue's Cowork acceptance criterion, on a real launch: the file exists inside the sandbox, asserted on its content and on a string generated for that run, and it lists in the Working folder through the customer route the panel itself calls. Detail, including the two false negatives the scenario hit first, is in a comment below.
Filed rather than fixed here
Three, all from the security review, all either pre-existing or latent, none of them a reason to widen this diff.
AGENTS.mdbecomes the agent's project instructions if a pack ever ships without one. Today it collides with a pack-planted file and is renamed, so the protection is the pack's rather than the writer's. Latent, not present.What this pull request does not do
project_idin Go.Buglog entry
{"id":"bug-1065-cowork-attachment-never-reaches-sandbox","date":"2026-09-02","title":"A file attached in the composer could not be given to a Cowork run at all","error_message":"Attachments are not supported in Work mode yet. Remove the file, or switch to Chat mode to send it.","root_cause":"The composer refused the send because there was nothing downstream to accept a document: createTask sent only pack and instructions, POST /v1/agent/tasks decoded only those plus project_id, Remote.Launch put no document on the /launch body, and SandboxEngine.Launch wrote nothing but the pack into the working directory the sandbox bind mounts as /workspace. Four layers with no field, so the refusal in the browser was the only honest link in the chain.","fix":"Carry the attachment's extracted text inline from the composer to the launcher, and write it into the session working directory after materializePack with O_EXCL through an os.Root, adding the file names to the run's initial message. Validate the name as a bare file name at all four hops, cap at five attachments and 256 KiB of combined text ahead of the credit hold, and never persist the content on the task row.","tags":["cowork","agent-engine","attachments","issue-1065","issue-847","sandbox","edge-api","control-plane","open-webui"]}Summary by CodeRabbit
New Features
Bug Fixes