Repository navigation
fix: deliver a project's files to every conversation bound to it (issue #1358) - #1707
Conversation
#1358) The Projects page tells the user "Files here are given to every conversation in this project". They never were. `createBoundChat` wrote only the `hiveProject` marker onto the chat blob, and that marker was read nowhere outside the Projects module, so Chat.svelte assembled every request without it and the model never saw a single project document. The obvious fix, writing the attachment onto the chat blob at bind time, does not work, and this is the part worth recording. `sendMessageSocket` prunes chat level files down to those that some message in the branch references before it builds the request: chatFiles = chatFiles.filter((item) => chatMessageFiles.some((messageFile) => messageFile.id === item.id)); A collection attached at chat creation is referenced by no message, so it is dropped on the first send and then written back out of the persisted chat by the next save. The wiring would exist and the value would still never arrive. So the binding is resolved at request assembly instead, which is the one chokepoint submit, regeneration and continue all pass through, and it is resolved as a reference rather than a snapshot, so a file uploaded into the project tomorrow reaches a conversation created today. Retrieval scope is unchanged. `{ type: 'collection', id }` is byte for byte the item the composer's plus menu already produces when the same project is attached by hand, and it lands in the same branch of `get_sources_from_items`, which resolves the collection and then applies the caller's own read access check. No new retrieval path, no new permission, and nothing that widens what one member of a tenant can read.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesProject chat file integration
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to Project-bound conversations now include the project's files while existing access controls remain in place; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Chat.svelte
participant withProjectFiles
participant OpenWebUIRetrieval
Chat.svelte->>Chat.svelte: Load hiveProjectId from chat blob
Chat.svelte->>withProjectFiles: Pass pruned files and hiveProjectId
withProjectFiles-->>Chat.svelte: Return files with collection reference
Chat.svelte->>OpenWebUIRetrieval: Send request files
OpenWebUIRetrieval-->>Chat.svelte: Resolve project collection files
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 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 |
loadChat set hiveProjectId only inside the branch that reads the chat blob. A chat whose blob does not load never reaches that assignment, so the previous conversation's project would have stayed attached to the next one. Cleared at the top of the load instead, with a source guard so a later edit cannot quietly move it back inside the branch.
Adversarial review, four streams
Security: the project id is client controlled, and that is safe hereThe value now read at request assembly comes off the chat's own JSON blob, which the signed in user can write freely through What happens server side is the pre-existing check, unchanged. A user forging a It also does not touch Adversarial finding 1, fixed: a stale binding could follow the user into the next conversation
Fixed in d587c1a by clearing at the top of Adversarial finding 2, accepted and stated rather than fixed
Scope the reviewer should not expect to find here
TestsWritten before the fix and observed red for the right reasons:
|
The New chat button on a project created a chat whose stored blob was the project marker and nothing else. /chats/new stores what it is given verbatim, so loadChat then reached convertMessagesToHistory(undefined) and threw before the page rendered: the conversation could not be opened at all, which is why nobody had ever observed the project's files failing to arrive in one. The blob now carries an empty but well formed conversation. It is also born with a model. loadChat takes the models off the blob and, unlike initNewChat, has no default to fall back on, so a chat created without them opened on Select a model and could not be sent. seedChatModels mirrors initNewChat's own precedence and falls back to the first model the person can see, so the conversation is never born unusable.
Text half of the visual proof. The frames themselves go to the visual-proof-assets release through scripts/post-pr-visual-proof.sh, but the log is committed here because npm run lint:proof-tokens scans docs/proof and nothing else.
Visual proofIssue 1358 proof, captured against a chat image built from this branch (commit 0d2e8c3) on a fresh instance. Frame 1: the project holds rack-register.txt, whose only distinguishing content is the string BRACKEN-7741-QX. Frame 2: a conversation created with the project page's own New chat button answers the question, renders Retrieved 1 source and cites rack-register.txt; its outgoing request carried files=[{type: collection, id: }]. Frame 3: the control, run before the project existed, on the same model, whose request carried files=null and which searched, found nothing and said so. Full log in docs/proof/projects-attach-delivers-1358-2026-09-02/.
|
Review addendum, covering the three commits added after the first pass
The judgement call in
|
sakibsadmanshajib
left a comment
sakibsadmanshajib
left a comment
There was a problem hiding this comment.
Independent security and correctness review
First pass on this pull request that was not written by the implementer. Scope as asked: the retrieval scope claim verified end to end in the backend rather than from the description, the prune ordering verified on every path that reaches the request assembly, and the two incidental fixes checked for whether they are correct or are papering over a deeper state problem.
Substrate note, because it changes what counts as evidence. The backend that runs is the pinned upstream Open WebUI v0.10.2 image plus the exact literal rewrites in deploy/docker/owui-patches/, not vendor/open-webui/backend, which the chat image builds nothing from. I read both. On this path they agree except for apply_retrieval_authz_patch.py, which makes the choke point stricter than the vendored source shows.
1. Retrieval scope: verified independently, and the claim holds
The item this change emits is { type: 'collection', id } with no context, no legacy and no collection_names.
get_sources_from_items,type == 'collection'branch: the first thing it does isKnowledges.get_knowledge_by_id(item['id']). A nonexistent id, a deleted collection, a malformed value or another tenant's id that does not resolve all returnNone, the branch appends nothing,collection_namesstays empty, and the item is skipped with no error and no leak.- If the row does exist, the branch requires admin, or
knowledge_base.user_id == user.id, or anAccessGrantsread grant, before anything else. - With no
context: 'full'and withrag.bypass_embedding_and_retrievalat its default of false (upstream default, not overridden anywhere in compose), the branch takescollection_names.append(item['id']), and every name then passesfilter_accessible_collections. - At that choke point the admin short circuit is flag gated by
apply_retrieval_authz_patch.py, anddocker-compose.ymlsetsBYPASS_ADMIN_ACCESS_CONTROL: "false". So a caller holding the instance admin role, which on this shared instance every tenant owner does, is validated like anyone else: owner or grant, otherwise denied. Names outside[A-Za-z0-9_-]are dropped before the vector store is touched at all.
So a crafted collection id belonging to another tenant, or to another member of the same tenant, is refused, and refused twice. I could not find a branch this item can reach that skips filter_accessible_collections.
The deeper reason the scope argument is sound is not in the description: files on POST /api/chat/completions is client supplied on every request already. A user who can edit their own chat blob to set the marker can send the identical array by hand today. This change moves an existing capability from the composer to the binding. It creates none.
Issue #1643 is about public.rag_documents and /v1/rag/* in edge-api. Nothing in this diff reaches that store. Neither widened nor narrowed, exactly as the description says.
Direct answer to the question this review was asked: no. This pull request cannot expose a document to a user who could not already reach it.
Two qualifications for the record, neither of them blocking.
(a) The binding now travels through chat clone. POST /chats/{id}/clone and /clone/shared copy the blob wholesale ({**chat.chat, ...}), so a recipient's copy carries the original owner's project id and attaches it on every send without the recipient ever choosing to. It resolves to nothing today, because the recipient is neither owner nor grantee and the admin path is closed by the flag above. This is the one place where the "same as the plus menu" equivalence does not hold, because a plus menu attach is an explicit act by the sender and an inherited binding is not. It becomes a silent read only if BYPASS_ADMIN_ACCESS_CONTROL is ever flipped, or if an admin enables rag.bypass_embedding_and_retrieval, which is admin togglable persisted config and whose context: 'full' sub branch of the collection item never reaches filter_accessible_collections at all. Worth a follow up issue to strip hiveProject on clone. Not worth blocking this.
(b) The capture proves delivery, not denial. It ran with WEBUI_AUTH=false, so its only user held the admin role and BYPASS_ADMIN_ACCESS_CONTROL was unset, which falls back to ENABLE_ADMIN_WORKSPACE_CONTENT_ACCESS and defaults true. The capture therefore exercised the fully bypassed path and never touched the denial path. After this pass I agree with the scope claim, but it is code argued, not captured, and the description should say so rather than reading as though the capture established it.
2. Prune order and marker lifecycle: verified
generateOpenAIChatCompletionis called from exactly one place in the chat surface,Chat.svelte:2962, insidesendMessageSocket. The only other call in the tree isNoteEditor.svelte, a different surface with no chat blob. There is no sibling assembly that buildsfilesand skips this.sendMessageSockethas two callers: the send path at 2753, whichsubmitPromptandregenerateResponseboth reach throughsendMessage, andcontinueResponseat 3259. Submit, regeneration and continue all pass through the one line. Confirmed by reading, not by the description.- The call sits after the
chatFilesprune and after the dedupe, and operates on the localfilesclone.withProjectFilesreturns a new array, sochatFilesis never mutated, andsaveChatHandlerandinitChatHandlerboth persistfiles: chatFileswithout the project item. The binding stays a reference and never becomes a stored attachment, which is what the reference semantics test is really protecting. - Worth stating because it is load bearing and unstated: the router's
update_chat_by_idmerges rather than replaces (updated_chat = {**chat.chat, **form_data.chat}), so thehiveProjectkey survives every save even thoughsaveChatHandlernever sends it. A full replace here would have silently unbound every conversation after its first message, and the tests would not have caught it. - Marker clearing: cleared in
initNewChatand at the top ofloadChatbefore the first await. The only way the component takes a different chat id isnavigateHandler, which callsloadChat; the/route callsinitNewChatthrough the page subscription. A blob that fails to load cannot inherit the previous conversation's project. I found no path by which a project file reaches a conversation opened outside that project. - One gap in the delivery claim rather than the scope claim, in the thread on
Chat.svelte: cowork turns never reach this line.
3. Where automated coverage stopped
CodeRabbit reports Review rate limited on this pull request, and the implementer's own addendum records the same across four attempts on the later commits. So d587c1a (marker clearing before the next blob is read), 0d2e8c3 (openable and sendable) and 8767b45 (the capture log) have had no automated review at all. That is the whole of both incidental fixes. This pass read them by hand; the record should show that they were never machine reviewed.
4. Findings
| Severity | Finding | Thread |
|---|---|---|
| MEDIUM | A conversation created from a project is permanently titled "New Chat" and never gets tags, because pre creating the chat makes is_new_chat false forever |
ProjectDetail.svelte:196 |
| LOW | Hidden models are not filtered out of the seed candidate list, unlike initNewChat |
ProjectDetail.svelte:200 |
| LOW | seedChatModels does not actually mirror initNewChat's precedence, in two named ways |
projects.ts:377 |
| LOW | The guard test does not pin the ordering the whole fix turns on | projects.test.ts:360 |
| INFO | "byte for byte the item the plus menu produces" is not literally true; the real relationship is stronger than the claim | projects.ts:311 |
| INFO | Extra billed task model call per turn, and cowork turns bypass the fix entirely | Chat.svelte:2852 |
Verdict: no security blocker, and no widening of retrieval scope. One correctness finding I would fix before merge, or split into a tracked issue with a line in the description, since it lands squarely on the surface this pull request makes usable for the first time and the capture was configured in a way that could not have shown it.
Review response, issue #1358. Pre creating the conversation on the project page made its first request carry a chat_id, so the backend dropped title and tag generation for it (is_new_chat, main.py) and every conversation started from a project would have read "New Chat" forever. The first capture ran with ENABLE_TITLE_GENERATION=false and so could not have shown it. The project page now navigates to /?project=<id> and creates nothing. initNewChat picks the binding off the query string the way it already picks up models and youtube, and the ordinary composer path supplies the blob, the models, the title and the tags. That deletes both of the incidental fixes this branch had added, createBoundChat and seedChatModels, rather than adding a third, and with them the hidden model filter divergence a reviewer found in the second. One correction to the proposed shape, measured rather than assumed: on this fork initChatHandler's createNewChat never runs on the send path. The chat row is created server side from the completion request, which is why the branch that handles the returned chat_id already has to persist chat level params the backend never received. The binding is written there, one line below params, for exactly the same reason. Also widens the ordering guard so an edit that moves the attachment above the chatFiles prune fails, and corrects the "byte for byte" claim in the comment: the emitted item is a strict subset of the manual attachment, and that is what keeps it on the strictest branch of get_sources_from_items.
Visual proofRe-capture after review, on hive-owui:proof-1358-v3 with title generation left ON, which the first capture had disabled and so could not have tested. Frame 1: the project holds rack-register.txt, whose only distinguishing content is BRACKEN-7741-QX. Frame 2: New chat now navigates to /?project=, the composer creates the conversation on the first message, and the answer is grounded in the file with a citation naming it; the request carried files=[{type: collection, id: }]. Frame 3: the project's conversation list afterwards, showing the generated title rather than a permanent New Chat row, which is the MEDIUM finding. Frame 4: the control, run before the project existed, whose request carried files=null and which searched, found nothing and said so.
|
The extra generate_queries completion per turn in a project bound conversation, including while the project holds no files, and the fact that cowork turns return before this line so a run started inside a project receives none of its documents. Both were raised in review and both are accepted rather than fixed here; Work mode retrieval is issue 1312 and task 8 of the Projects unification spec.
Review finding on PR #1730. Dropping the whole builtin tool set while turning native function calling on would have stranded two of Open WebUI's retrieval paths, which inject documents into the request only under legacy and hand the work to builtin knowledge tools under native: a folder's attached files, which become metadata['folder_knowledge'], and a custom model's own attached knowledge. Both would have gone silently missing in a deployment whose interface still offers them. The knowledge tools are now kept on the turns that carry such knowledge and dropped on every other turn, so an ordinary chat still ships two specifications rather than a permanent knowledge tool nobody asked for. A file attached to the message, and a Hive project's files, which PR #1707 appends to the same request list, were never at risk: upstream runs chat_completion_files_handler unconditionally on either path. That claim is now pinned by a test rather than asserted in a comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WbVmp2Uh7FCgnqKB2TuBb5
…1735) Closes #1065. Closes #847 was already done before this branch; see the ground-truth section below. Changed from `Refs` to `Closes` after 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 on `main` and 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. The `File not found.` literal it was named for is gone from every one of the five composers that carried the copy-pasted handler, and `chat-noise-guards.test.ts` pins 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. | Step | State on `main` | |---|---| | A file is attached in the composer while Work mode is selected | Uploads fine. The plus menu, the size cap and the extraction all work; Work mode shares the composer with chat. | | The person presses send | `submitHandler` (`vendor/open-webui/src/lib/components/chat/Chat.svelte:2569`) refuses: *"Attachments are not supported in Work mode yet."* | | The refusal's stated reason | Accurate. `createTask` sent `{pack, instructions}` and nothing else (`vendor/open-webui/src/lib/hive/agentTasks.ts:282`), `POST /v1/agent/tasks` decoded exactly those two fields plus `project_id` (`apps/edge-api/internal/agenttask/handler.go:135`), and `Remote.Launch` put five keys on the wire, none of them a document (`apps/control-plane/internal/agentengine/remote.go:67`). | | Where a document would have to land | `workingDir` in `SandboxEngine.Launch` (`apps/agent-engine/internal/engine/engine.go:632`), the directory bind mounted as `/workspace`. Nothing but `materializePack` ever 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.svelte` gathers 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 from `GET /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.py` rebuilds the list field by field, the way it already rebuilds `pack` and `instructions`, and never forwards the submitted body wholesale. * `apps/edge-api/internal/agenttask` validates, 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/agenttask` carries them on the in-memory `Task`, exactly where `BearerJWT` and `LLMAPIKey` already live, and **does not persist them**. A task row is a control record, not a copy of the customer's documents. * Both arms of `buildAgentEngine` hand the engine the same task: `Remote.Launch` puts them on the `/launch` body, and the in-process `agentengine.Engine` converts 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 into `workingDir` through an `os.Root`, after `materializePack`, with `O_EXCL`. ## Why the text travels inline, and the ceiling that buys The sandbox is behind `--network none` with 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.go` as 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 call `get_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_id` retrieval 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 for `Task.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 `validateAttachments` and 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 behind `RequireInternalToken` rather 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_FencesTheAttachmentNameInThePrompt` uses 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 `await` on the cowork path in front of the clear. A second Enter in that window meant two `createTask` calls and two credit holds. `coworkGatherInFlight`, released in a `finally` before 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.Root` confined to `workingDir`, 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 called `AGENTS.md` is kept as `AGENTS-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 `listWorkspaceFiles` already reasons about. * **Credentials.** Nothing new is logged. The launcher's existing `redactCredentials` on 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.go` reads 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's `AGENTS.md` byte 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.go` decodes the actual `/launch` body 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.go` asserts what reached control-plane, and that each refusal happens with `createCalled` still false, so a bad request cannot take a hold. * `vendor/open-webui/src/lib/hive/coworkAttachments.test.ts` covers 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. * One stale guard retired with its reason recorded: `coworkMode.test.ts` pinned the blanket refusal string as a fixed behaviour from the #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.yml` stands the real thing up per run from `refs/pull/1735/merge`; a scenario was added to its harness and dispatched at this pull request. Run 33668985745, `success`: ``` the sandbox workspace holds service-record.txt carrying HIVE-1065-68985745 GET /v1/agent/tasks/{id}/files answered HTTP 200: {"files":[{"name":".git","size":4096,...},{"name":"service-record.txt","size":66,...}]} scenario attachment-reaches-the-sandbox: ok ``` 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. * **#1750**, an attachment named `AGENTS.md` becomes 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. * **#1751**, the message queue replays a Work mode submission as a chat completion and now replays its attachments with it. The mode blindness is #944 and predates this change; attachments make an existing wrong path visible. * **#1752**, each unbounded launch goroutine (#900) now retains up to 256 KiB. A constant factor on an unbounded count, worth recording for whoever sizes #900. ## What this pull request does not do * It does not give a Cowork run a Project's documents. That is #1312 and needs an ownership check on `project_id` in Go. * It does not move the 25 MB chat upload ceiling into Work mode. See the ceiling section. * It does not change the chat surface's attachment behaviour at all. ## Buglog entry ```json {"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"]} ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Work mode now supports attaching documents to runs. * Attached files are validated, transferred with the run, and made available in the working folder. * Attached files appear on the user message and can be viewed through the working-folder listing. * Supports up to five attachments with a combined text limit of 256 KiB. * Larger requests are supported to accommodate attachment content. * **Bug Fixes** * Attachments are rejected when empty, oversized, invalidly named, or unsupported. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 participant
Closes #1358.
The false claim, and where it broke
ProjectDetail.svelte:308tells the user "No files yet. Files here are given to every conversation in this project." Nothing delivered them.The trace, end to end:
maincreateBoundChat(vendor/open-webui/src/lib/hive/projects/projects.ts:327) writes{ chat: { hiveProject: projectId } }and nothing else.sendMessageSocket(vendor/open-webui/src/lib/components/chat/Chat.svelte:2799) buildsfilesfromchatFilesplus this turn's own attachments. It has never readhiveProject; before this change the key appeared in no file outsidesrc/lib/hive/projects/.filesreaches the request without a single project document in it.So the break point is
Chat.svelte:2828, thefilesassembly insidesendMessageSocket. The value the marker was supposed to carry had no reader.Why the obvious fix does not work
The issue proposes wiring the project onto
chat.filesatcreateBoundChattime. That fails, and the reason is the interesting half of this defect. Immediately above the assembly,sendMessageSocketprunes:An attachment written onto the chat blob at creation is referenced by no message, so it is deleted on the first send, and then
saveChatHandlerpersistsfiles: chatFilesand deletes it from the stored chat as well. The wiring would exist and the value would still never arrive, which is the shape this repository keeps hitting.That is also why a manual attach works today: the plus menu puts the collection on the user message, so a message references it and the prune keeps it.
The fix
Resolve the binding at request assembly, after the prune, in the one function every entry point reaches.
Chat.sveltereadshiveProjectoff the chat blob at load (hiveProjectId), clears it when a new chat starts, and callswithProjectFiles(files, hiveProjectId)at the end of thefilesassembly insendMessageSocket.submitPrompt, regeneration and continue all route through that one function, so no sibling entry point is left broken.withProjectFilesinprojects.tsappends{ type: 'collection', id: projectId }when the chat is bound and the project is not already on the turn, and returns the identical array otherwise.A reference, not a snapshot. The collection is resolved to its current file set on every request, so a file uploaded into the project tomorrow reaches a conversation created today, which is what the sentence on the project page actually claims. A list of file ids captured at bind time would not do that.
The other half: the conversation the project page creates
The project page's New chat used to create the conversation itself. That was wrong twice over, and a review caught the second one.
It created a blob carrying the marker alone, so
loadChatreachedconvertMessagesToHistory(undefined)and threw before the page rendered. And a conversation that already exists makes its first request carry achat_id, so the backend'sis_new_chatis false (main.py:1096) and the title and tag tasks are dropped for it permanently: every conversation in a project would have read "New Chat" forever, and the project's list would have been a column of identical rows.So the page now creates nothing. It navigates to
/?project=<id>,initNewChatpicks the binding off the query string the way it already picks upmodelsandyoutube, and the ordinary composer path supplies the blob, the models, the title and the tags. Two helper functions this branch had added,createBoundChatandseedChatModels, are deleted rather than kept in step with the code they were re-implementing.One measured correction to that shape, because the obvious place to write the binding does not work here.
initChatHandler'screateNewChatnever runs on the send path on this fork: a send produces noPOST /api/v1/chats/newat all, and the stored blob carries noparams, which that payload would have supplied. The chat row is created server side from the completion request. That is why the branch handling the returned id already persists chat level params "that the backend doesn't receive in the chat completion request", and the binding is written one line below it, for the same reason.The tradeoff, stated rather than buried: the conversation appears in the project once it has a first message rather than immediately. An abandoned pre created chat was a permanent empty row, so this is the better behaviour.
Retrieval scope: unchanged, stated explicitly
This touches document retrieval, so the scope question is answered directly rather than by omission.
{ type: 'collection', id }is byte for byte the itemMessageInput/InputMenu/Knowledge.svelte:179already produces when a user attaches the same project by hand. It lands in the existingitem.get('type') == 'collection'branch ofget_sources_from_items(backend/open_webui/retrieval/utils.py:1468), which resolves the collection only after checkinguser.role == 'admin' or knowledge_base.user_id == user.id or AccessGrants.has_access(...), and whose collection names then pass throughfilter_accessible_collections, the choke point PR #1183 and the #1186 patch hardened.So: no new retrieval path, no new permission, no widening. A user who can open a project bound conversation is a user who could already attach that project from the composer and get the same passages. In particular this does not touch
/v1/rag/*, so it neither helps nor worsens #1643, the tenant readable RAG store; that issue keeps its full scope.Known gap, named rather than left quiet
Cowork turns never reach the injection.
submitHandlerreturns aftersubmitCoworkRun, so a run started inside a project receives none of its documents. That is issue #1312 and task 8 of the Projects unification spec, both of which already own it, and closing it here would mean aproject_idonPOST /v1/agent/taskswith an ownership check in Go. It is recorded in the code comment at the injection site so it cannot go quiet, along with the one cost this adds: a non emptyfilesarray makes the backend rungenerate_queries, so a project bound turn spends one extra task model completion even while the project is empty. Retrieval over an empty collection returns no sources and the injection is guarded, so no answer is degraded.Visual proof
Posted on this pull request, four frames from a chat image built from this branch on a fresh instance, plus the log in
docs/proof/projects-attach-delivers-1358-2026-09-02/.Re-captured after review with title generation left at its default, on. The first capture had disabled it, which made it structurally incapable of showing the defect above: a proof configured with the relevant feature off proves that feature's absence, not its correctness.
The load bearing line is not the screenshot, it is the outgoing request body read off the wire in each case:
The document's only distinguishing content is a string that exists nowhere else. In the project the conversation renders "Retrieved 1 source", answers with it, and cites
rack-register.txt. In the control the model searched, found nothing and said so.And the two things the review said this had to show, both from the same run:
What the capture still cannot show is the denial path, because it runs with
WEBUI_AUTH=falseand therefore has one identity. That claim rests on the independent backend review, not on these frames.Tests
vendor/open-webui/src/lib/hive/projects/projects.test.ts, seven new cases, written before the fix and observed red for the right reasons (withProjectFilesabsent, andChat.sveltecontaining no read of the marker and no call at the assembly site):filesarray, which is the case a naive blob attachment fails;typeandidonly, pinning reference semantics rather than a snapshot;Chat.svelte's source for the URL pickup, the write of the binding once the backend returns the new id, the load off an existing blob, and both reset sites;chatFilesprune fails rather than silently restoring the exact bug this description spends half its length on;ProjectDetail.svelte, so the second chat creation path cannot come back.make test-owui-frontend: 380 passed, 25 files. The same sources run again in place during the chat image build (npm run test:frontend), so this is a build time gate, not only a CI one.Relationship to #1595
The Projects unification spec (
spec-2026-08-31-projects-unification, Obsidian vault) moves Projects onto Hive's own/v1/rag/*store, and its task 6 fixes #1358 as a side effect of the new attachment control. Tasks 4, 5 and 7 of that spec, the proxy routes, the repointedprojects.tsand thehive_projectretrieval branch, are not built: nothing onmainreadsrag_projectsfrom the chat surface, and no pull request is open against them.This change is deliberately scoped to the store Projects is bound to today. When task 5 repoints
projects.ts,withProjectFilesis the one line that changes, fromtype: 'collection'totype: 'hive_project', and the call site inChat.svelteand every test above stay as they are. It does not block, duplicate or pre-empt that work; it stops the interface lying in the meantime.Spec acceptance criteria 1, 5 and 6 remain #1595's, and remain unmet: they are about the new store, the
ENABLE_RAGdefault and cross member project isolation on/v1/rag/*, none of which this touches.Buglog entry
{"date":"2026-09-02","issue":1358,"title":"Projects promised files to every conversation and delivered none","error_message":"A conversation created inside a Project answered as if the project's uploaded documents did not exist, while ProjectDetail.svelte stated 'Files here are given to every conversation in this project'.","root_cause":"createBoundChat wrote only the hiveProject marker onto the chat blob and no code outside src/lib/hive/projects read it, so Chat.svelte's sendMessageSocket assembled the request files with no project document in them. The apparent fix, writing the attachment onto chat.files at bind time, also fails: sendMessageSocket prunes chatFiles to only those files some message in the branch references, so a chat level attachment is dropped on the first send and then written out of the persisted chat by the next save.","fix":"Resolve the binding at request assembly instead of persisting it. Chat.svelte reads hiveProject off the chat blob at load, clears it on a new chat, and calls withProjectFiles(files, hiveProjectId) after the prune and dedupe in sendMessageSocket, the one function submit, regeneration and continue all reach. withProjectFiles appends { type: 'collection', id } , the same item the plus menu produces, so retrieval scope and access control are unchanged.","tags":["owui-fork","projects","rag","claimed-state-no-reader","chat-svelte","frontend"]}