From a8bcaefea72daa528faba0b7aa1189e4a695cf01 Mon Sep 17 00:00:00 2001 From: ytahdn <1294726970@qq.com> Date: Thu, 13 Aug 2026 06:42:07 +0000 Subject: [PATCH] feat(web-shell): support workspace file uploads (#8874) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add web shell workspace file uploads * fix(serve): accept plain string targets in shared atomic publisher (#8874) * fix(review): bound web shell related paths * fix(review): address web shell file upload review findings (#8874) * test(serve): include upload capability in baseline * fix(review): pin workspace_file_upload in the serve capabilities integration baseline (#8874) * fix(review): address round-2 web shell file upload review findings (#8874) * fix(review): address round-3 web shell file upload review findings (#8874) * fix(review): address round-4 web shell file upload review findings (#8874) * fix(review): address round-5 web shell file upload review findings (#8874) * fix(review): address remaining file upload findings (#8874) * fix(review): address round-6 web shell file upload review findings (#8874) --------- Co-authored-by: 钉萁 Co-authored-by: qwen-code-dev-bot Co-authored-by: Shaojin Wen Co-authored-by: qwen-code-dev-bot --- docs/design/web-shell-file-upload.md | 310 +++++ .../daemon/07-workspace-filesystem.md | 21 +- .../daemon/11-capabilities-versioning.md | 2 +- docs/developers/qwen-serve-protocol.md | 8 +- .../cli/qwen-serve-routes.test.ts | 1 + .../serve/bridge-file-system-adapter.test.ts | 2 + packages/cli/src/serve/capabilities.ts | 10 +- packages/cli/src/serve/fs/index.ts | 1 + packages/cli/src/serve/fs/policy.ts | 11 + .../serve/fs/workspace-file-system.test.ts | 214 ++- .../cli/src/serve/fs/workspace-file-system.ts | 162 ++- packages/cli/src/serve/routes/capabilities.ts | 4 + .../serve/routes/workspace-file-read.test.ts | 5 + .../serve/routes/workspace-file-write.test.ts | 1146 +++++++++++++++- .../src/serve/routes/workspace-file-write.ts | 477 +++++++ packages/cli/src/serve/server.test.ts | 2 + packages/cli/src/serve/server.ts | 24 + .../cli/src/serve/server/telemetry.test.ts | 18 + packages/cli/src/serve/server/telemetry.ts | 1 + packages/cli/src/serve/types.ts | 2 + .../serve/workspace-qualified-rest.test.ts | 219 ++- .../cli/src/ui/hooks/use-effort-command.ts | 9 +- packages/sdk-typescript/scripts/build.js | 6 +- .../sdk-typescript/src/daemon/DaemonClient.ts | 185 +++ packages/sdk-typescript/src/daemon/index.ts | 2 + packages/sdk-typescript/src/daemon/types.ts | 28 + packages/sdk-typescript/src/index.ts | 2 + .../test/unit/DaemonClient.upload.test.ts | 722 ++++++++++ packages/web-shell/client/App.test.tsx | 24 +- packages/web-shell/client/App.tsx | 11 + .../client/components/AtMentionPanel.test.tsx | 20 + .../client/components/AtMentionPanel.tsx | 15 +- .../client/components/ChatEditor.module.css | 120 ++ .../client/components/ChatEditor.test.tsx | 1178 +++++++++++++++-- .../client/components/ChatEditor.tsx | 403 +++++- packages/web-shell/client/customization.tsx | 15 + .../client/hooks/useAtMentionMenu.test.tsx | 352 ++++- .../client/hooks/useAtMentionMenu.ts | 98 +- .../client/hooks/useComposerCore.dom.test.tsx | 124 ++ .../web-shell/client/hooks/useComposerCore.ts | 82 +- .../client/hooks/useFileUpload.test.tsx | 1012 ++++++++++++++ .../web-shell/client/hooks/useFileUpload.ts | 330 +++++ packages/web-shell/client/i18n.tsx | 30 + 43 files changed, 7243 insertions(+), 165 deletions(-) create mode 100644 docs/design/web-shell-file-upload.md create mode 100644 packages/sdk-typescript/test/unit/DaemonClient.upload.test.ts create mode 100644 packages/web-shell/client/hooks/useFileUpload.test.tsx create mode 100644 packages/web-shell/client/hooks/useFileUpload.ts diff --git a/docs/design/web-shell-file-upload.md b/docs/design/web-shell-file-upload.md new file mode 100644 index 0000000000..86bd6c9d1f --- /dev/null +++ b/docs/design/web-shell-file-upload.md @@ -0,0 +1,310 @@ +# Web Shell File Upload + +## Problem + +The Web Shell composer allows referencing workspace files via `@path/to/file`, but the file must already exist in the workspace. Users frequently need to bring local files (screenshots, data files, configs) into the workspace to reference them in prompts. The current workflow requires manually saving files via the CLI or another tool before the Web Shell can see them. + +This feature adds direct file upload from the browser to the workspace: + +1. **Drag-and-drop** onto the composer input — uploads to the target workspace root, shows inline progress above the input. +2. **@ panel upload item** — uploads to the currently browsed directory in the @ file picker. +3. After upload, the composer automatically inserts `@filename` so the existing `@` resolver can consume supported files. + +## Out of scope + +- Multipart form parsing, resumable/chunked uploads, folder upload. +- `expectedHash`-gated writes (CAS): the browser cannot cheaply hash a large file before upload. Can be added later if a client needs it. +- In-place overwrite of existing files via upload: **uploads never overwrite**. The server always resolves an ordinary name conflict by auto-numbering. If in-place replacement or fail-on-conflict behavior is ever needed, it should be added only with a concrete client requirement and an explicit contract. +- ACP-HTTP parity (`_qwen/file/upload`): REST-only for v1, see below. +- Configurable size limit (env/flag): hardcoded constant for now, matching existing limit style. + +## Design + +### fs layer: new `writeBytesAtomic` + +`WorkspaceFileSystem` (`packages/cli/src/serve/fs/workspace-file-system.ts`) has byte **reads** (`readBytes` / `readBytesWindow`) but only text **writes** (`writeTextAtomic` / `writeTextOverwrite` / `writeText` / `edit*`), all of which apply encoding/BOM/line-ending normalization that would corrupt binary content. This feature therefore adds a symmetric binary write method to the interface first: + +```typescript +writeBytesAtomic( + p: ResolvedPath, + data: Buffer, +): Promise<{ sizeBytes: number; hash: ContentHash }>; +``` + +The method is a single-purpose no-clobber create primitive; it cannot modify or replace existing file content. Posture mirrors the existing `writeTextAtomic({ mode: 'create' })` publication semantics: + +- Add `MAX_UPLOAD_BYTES = 50 * 1024 * 1024` to `fs/policy.ts` and export it through `fs/index.ts`. `writeBytesAtomic` enforces `enforceWriteSize(data.length, MAX_UPLOAD_BYTES)`; existing text writes continue using the default `MAX_WRITE_BYTES = 5 * 1024 * 1024`. The upload limit is a distinct binary-ingress policy, not an increase to agent text-write limits. +- `writeBytesAtomic` enforces the trust boundary itself with `assertTrustedForIntent(..., 'write')`; HTTP admission is only an early-rejection optimization. It checks the generation guard at entry, again inside the path lock before temp-file publication, and at the existing final publish checkpoint so a draining/removed runtime cannot commit after admission. +- Atomic temp-file + publish: an interrupted or canceled upload never exposes a partial target. +- An existing target throws `FsError('file_already_exists')` (409), including an external writer racing the final no-clobber publication. +- Symlinks at the target are rejected (`symlink_escape`), consistent with the text writes; boundary resolution goes through the existing `resolve(path, 'write')`. +- A new file is created at `0o600` (not umask default). +- The implementation reuses the existing path lock, temp-file reservation, no-clobber create publication, generation guard, audit, and cleanup machinery. Generalize the current atomic publisher to accept an already validated `Buffer`; do not copy a second binary-specific atomic-write implementation. The byte path must not pass through `atomicWriteTextResolvedFile`, whose internal `enforceWriteSize(buf.length)` intentionally applies the 5 MiB text default. Each public write path validates its final byte buffer with its own policy before calling the shared publisher. + +### Daemon: new `POST /file/upload` endpoint + +Extend `routes/workspace-file-write.ts`, which already owns the workspace file mutation routes and its private `getFsFactory` / `parseClientId` / `resolveOriginatorClientId` machinery. Keeping upload registration there avoids cloning the trust, identity, and workspace-resolution plumbing into a second module. + +**Routes** (both behind `deps.mutate({ strict: true })`): + +- `POST /file/upload` +- `POST /workspaces/:workspace/file/upload` + +Route ownership/scope is identical to `POST /file/write`: workspace-scoped, resolved-runtime. The qualified variant follows the same failure semantics — unknown (including an already removed workspace), untrusted, or non-active workspace states are rejected and never fall back to the primary runtime. + +**Request:** + +``` +Content-Type: application/octet-stream +X-Qwen-Client-Id: + +Query parameters: + path — target file path (relative to workspace root), required, + encoded by URLSearchParams (filenames are frequently non-ASCII); + the server validates Express's already-decoded req.query.path and + must not call decodeURIComponent again + +Body: raw binary bytes +``` + +**Middleware chain:** + +1. `deps.mutate({ strict: true })` — unauthenticated mutations are rejected before any buffering. +2. `fileUploadAdmission` — performs every cheap request-level check before buffering (final-name boundary checks happen in the handler's candidate loop, which runs after buffering): + - Legacy route: verifies the primary workspace is currently trusted through an injected `isWorkspaceTrusted()` dependency. + - Qualified route: `resolveWorkspaceRuntimeFromParam` → `requireTrustedWorkspaceRuntime` → `setWorkspaceRouteContext`. Unknown (including an already removed workspace), untrusted, or draining workspaces stop here and never fall back to the primary runtime. + - Requires `Content-Type: application/octet-stream`; otherwise returns `{ errorKind: 'unsupported_media_type', error: 'File uploads require application/octet-stream', status: 415 }` with status 415. + - Rejects missing/invalid `path` and a requested basename over `MAX_UPLOAD_FILENAME_BYTES` with a standard `parse_error` envelope. + - If a valid `Content-Length` is present and exceeds `MAX_UPLOAD_BYTES`, returns the upload-specific 413 immediately. The raw parser remains authoritative for chunked bodies and clients that omit or understate the header. + - Runs `parseClientId` and `resolveOriginatorClientId` against the selected runtime's bridge. An invalid client id is rejected before buffering. + - Splits `path` into directory + basename, resolves the directory with `fs.resolve(dir, 'write')`, and verifies it is an existing directory with `fs.stat`. Traversal, parent-link escapes, missing/non-directory parents, and other boundary failures are therefore rejected before buffering. The requested final name itself is resolved per candidate in the handler's loop after buffering; an escaping final-component symlink surfaces as the loop's boundary error. + - Stores the requested basename, resolved parent directory, route name, and the per-request fs instance in a private request context for the handler; the handler does not resolve the parent directory again. +3. `fileUploadConcurrencyGate` — admits at most `MAX_CONCURRENT_UPLOADS = 4` requests across the legacy and qualified routes. `createServeApp` creates one shared gate and injects it into both route registrations. A saturated gate returns 429 with `Retry-After: 1` before body parsing. Before the upload handler starts, response `finish` or `close` releases the slot; after the handler starts, the slot remains held until the handler settles so disconnecting clients cannot bypass the memory bound. +4. `fileUploadBodyParser` — wraps `express.raw({ type: 'application/octet-stream', limit: MAX_UPLOAD_BYTES })`. The numeric fs policy constant is the single source of truth for both parser and write limits. Its callback intercepts body-parser `status === 413` and returns the upload-specific `file_too_large` envelope below; other errors call `next(err)`. This prevents the global JSON parser error handler from incorrectly reporting the existing 10 MB JSON limit. +5. Handler: normalizes an absent parsed body for a valid zero-length request to `Buffer.alloc(0)`, takes the admitted request-scoped fs instance, then executes the name-allocation flow below. + +Path traversal and symlink escape are blocked by the same `fs.resolve` boundary guards as `/file/write`. + +**Name allocation:** `WorkspaceFileSystem` only exposes no-clobber byte creation. The route owns the upload-specific naming policy: + +- Try the requested path first, then numbered candidates on `file_already_exists`. Insert ` (N)` before the final extension: `report.pdf → report (1).pdf → report (2).pdf`; no extension: `README → README (1)`; a dotfile with no further extension stays whole: `.env → .env (1)`. The loop makes 1000 attempts total — the requested name plus ` (1)` through ` (999)` — then returns `file_already_exists` if every name is occupied. +- Every numbered candidate is built under the captured resolved directory and independently passes through `fs.resolve(candidate, 'write')`. If resolution produces a different path, that candidate is occupied by an in-workspace symlink and the route continues numbering without calling `writeBytesAtomic`. The no-clobber fs primitive makes concurrent uploads and external writers safe without relying on a route-level lock: if the name is already occupied by any entry, the route tries the next candidate. Boundary and I/O errors stop the loop. +- A route-local `MAX_UPLOAD_FILENAME_BYTES = 255` is the v1 upload filename policy cap, chosen to avoid `ENAMETOOLONG` on common POSIX filesystems; it is not claimed as a complete cross-platform filename validator. When a suffix would exceed the cap, trim only the stem on a Unicode code-point boundary until `stem + suffix + extension` fits; never trim the extension or split a UTF-8 sequence. If the suffix and extension alone cannot fit, return `parse_error`. Platform-specific restrictions such as Windows reserved names remain fs errors from `resolve`/publication. + +**Response:** uploads always create, so the response is always 201. `path` is the final server-confirmed path — a numbered candidate when the requested name was occupied — and clients must use it (not the requested path) for the `@` reference. + +```json +{ + "kind": "file_upload", + "path": "relative/path/to/report (1).pdf", + "sizeBytes": 12345, + "hash": "sha256:<64 lowercase hex>" +} +``` + +The response does not include a redundant `renamed` flag. A client that needs to show an auto-numbering hint compares the requested `path` with the returned `path`. + +Filesystem and upload-specific validation errors use `{ errorKind, error, status, ...details }`: `file_already_exists` 409 when the numbered-candidate cap is exhausted, `parse_error` 400, `unsupported_media_type` 415, `path_outside_workspace` / `symlink_escape` 400, `untrusted_workspace` / `permission_denied` 403, and upload-specific 413: + +```json +{ + "errorKind": "file_too_large", + "error": "Request body too large (max 50 MiB)", + "status": 413, + "maxBytes": 52428800 +} +``` + +The admission check and route-level raw-parser wrapper both emit this response because parser failures occur before the handler and cannot pass through `sendFsError`. Authentication, client-id, and workspace-runtime failures keep their existing daemon envelopes; the SDK's existing `DaemonHttpError` already preserves their status and parsed response body. This route does not duplicate shared validation helpers merely to rename `code` to `errorKind`. + +When all upload slots are occupied, the concurrency gate returns: + +```json +{ + "errorKind": "upload_busy", + "error": "Too many uploads in progress", + "status": 429, + "retryAfterSeconds": 1 +} +``` + +**Limits:** `MAX_UPLOAD_BYTES` is the shared hardcoded 50 MiB policy constant; no separate string-valued route constant or env/flag configurability without a driver. It is sized for screenshots, data files, and configs. Keeping the parser and fs boundary on the same numeric constant prevents requests from being fully buffered under one limit and rejected later under another. Because `express.raw` holds the complete body in memory, relying on the listener's default 256-connection cap would permit roughly 12.5 GiB of upload buffers. The shared four-slot gate instead bounds upload-body buffering to roughly 200 MiB plus normal framework overhead. Make the limit configurable or replace buffering with a streaming fs primitive only if production measurements require a different throughput/memory tradeoff. + +**Capability and limit discovery:** add `workspace_file_upload: { since: 'v1' }` in `capabilities.ts` — convention is new route contract = new tag (same split as `workspace_file_bytes` from `workspace_file_read`). Also add optional `maxWorkspaceFileUploadBytes` to `DaemonCapabilitiesLimits` and advertise `MAX_UPLOAD_BYTES` when the feature is present. Web Shell checks this value before sending and falls back to 50 MiB only if a capability-compatible daemon omits it. Older daemons without the feature tag hide the entry points and return 404 if called directly. A secondary-workspace target additionally requires `workspace_qualified_rest_core`; update that capability's route description to include file upload. + +**ACP-HTTP: out of scope for v1.** `/file/write` also exists as `_qwen/file/write` on the ACP-HTTP surface, but `/file/upload` is REST-only: the Web Shell (the only v1 consumer) talks REST directly, and the ACP-HTTP JSON wire cannot carry raw binary. No entries in `acpRouteTable.ts` / `dispatch.ts`; a base64 `_qwen/file/upload` can follow if a non-browser ACP client ever needs it. + +**Telemetry:** add the `/workspace/file/upload` suffix to the POST allowlist in `server/telemetry.ts` (normalized from `/workspaces/:workspace/file/upload`, next to the existing `/workspace/file/write` entry), otherwise latency lands in the unknown bucket. + +### SDK: `uploadWorkspaceFile()` on both client classes + +Follows the existing request-object signature style (`writeWorkspaceFile(req, clientId?)`). Qualified access goes through the existing `client.workspaceById()` / `workspaceByCwd()` selectors — **no** `uploadWorkspaceQualifiedFile` on `DaemonClient`. + +```typescript +interface DaemonWorkspaceFileUploadRequest { + path: string; + data: ArrayBuffer | Uint8Array | Blob; + signal?: AbortSignal; + /** Omitted inherits the client's default; 0 disables the timeout. */ + timeoutMs?: number; + /** Browser-only: requesting progress without XMLHttpRequest is an error. */ + onProgress?: (event: { loaded: number; total: number }) => void; +} + +interface DaemonWorkspaceFileUploadResult { + kind: 'file_upload'; + path: string; + sizeBytes: number; + hash: DaemonContentHash; +} + +// DaemonClient (legacy-primary), mirrors writeWorkspaceFile +async uploadWorkspaceFile( + req: DaemonWorkspaceFileUploadRequest, + clientId?: string, +): Promise; + +// WorkspaceDaemonClient (workspace-qualified), mirrors its writeWorkspaceFile +async uploadWorkspaceFile( + req: DaemonWorkspaceFileUploadRequest, + clientId?: string, +): Promise; +``` + +Both delegate to one shared internal raw-POST helper on `DaemonClient`, parameterized by URL + route name, the same pairing `WorkspaceDaemonClient` already uses (`/file/write` → `POST /workspaces/:workspace/file/write`). This keeps authentication headers, timeout/abort composition, response parsing, and `DaemonHttpError` construction in one place. Build the URL with `URL.searchParams.set('path', req.path)`; do not pre-encode `path` with `encodeURIComponent`. + +Transport is `XMLHttpRequest` when `onProgress` is provided (`fetch` exposes no upload progress), plain `fetch` otherwise. `onProgress` is explicitly browser-only: if `XMLHttpRequest` is unavailable, fail before sending rather than silently losing progress. Both paths honor `signal`, use the same authentication/client-id headers and `failOnError` response shape, and apply `timeoutMs`. Omission inherits the client's existing timeout; `0` explicitly disables it. The Web Shell passes `timeoutMs: 0` because its per-item `AbortController` owns cancellation and a valid 50 MiB upload can exceed the SDK's general 30-second default. + +### Web Shell: target workspace resolution + +The Web Shell is multi-workspace, so uploads must use the same target as the composer's existing file actions. Do not add a second voice-style resolver: + +- When `useComposerCore` has `workspace` and `atWorkspaceCwd`, use `workspace.client.workspaceByCwd(atWorkspaceCwd).uploadWorkspaceFile(...)`, exactly as its qualified `listDirectory` / `globWorkspace` actions do today. This includes a primary workspace addressed through the qualified route. +- Only the existing legacy composer path with no `atWorkspaceCwd` uses `workspace.client.uploadWorkspaceFile(...)`; a modern multi-workspace composer with a missing cwd is unsupported rather than silently targeting the primary workspace. +- Drag-and-drop and the @ panel entry share the selected client. The @ panel additionally supplies a directory within that workspace. +- A legacy target requires `workspace_file_upload`; a cwd-qualified target requires both `workspace_file_upload` and `workspace_qualified_rest_core`. The selected workspace must also be present exactly once and trusted in the capabilities snapshot. Otherwise hide both upload entry points. +- Host control: the web-shell accepts an optional `fileUploadEnabled` prop (threaded through the customization context). It is an additional gate, not a replacement for the capability: `fileUploadEnabled === false` force-hides both entry points even when the daemon advertises `workspace_file_upload`, while `true`/omitted still requires the capability (and the trust / qualified-route checks above). It never bypasses the capability. + +### Upload versus `@` consumption + +The upload endpoint is format-agnostic workspace storage. A successful upload guarantees that the bytes were created atomically at the returned path; it does **not** guarantee that every model/provider can inline or interpret that file. The automatically inserted reference continues through the existing `@` resolver and inherits its limits: + +- Images use the existing image pipeline and its source/decoding limits. +- PDFs use the existing PDF extraction/rendering behavior. +- Text files remain subject to model context and text-processing limits. +- Unsupported binary formats and oversized non-image binaries may upload successfully but fail when the prompt tries to consume them. + +The Web Shell does not duplicate file sniffing or maintain a second format-support matrix. User-facing copy says the file was uploaded and referenced, not that every model can read every format; any consumption failure comes from the existing resolver. E2E verification must exercise actual prompt consumption for a supported text file and image, not only file existence and inserted composer text. + +### Web Shell: `useFileUpload` hook + +New hook at `packages/web-shell/client/hooks/useFileUpload.ts`: + +```typescript +interface UseFileUploadOptions { + /** Structural client; both daemon client classes satisfy it. */ + client: FileUploadClient | undefined; + maxBytes: number; + targetKey: string; +} + +interface FileUploadItem { + id: string; + file: File; + targetPath: string; // requested relative path in the target workspace + status: 'pending' | 'uploading' | 'done' | 'error'; + progress: number; // 0–1 + /** Locally classified failures; the render site localizes them. */ + errorCode?: 'tooLarge' | 'noDaemon' | 'tooManyFiles'; + error?: string; // raw failure message (server-side errors) + resultPath?: string; // server-confirmed final path + /** Set on a `tooManyFiles` notice row: how many files were not queued. */ + skippedCount?: number; +} + +interface UseFileUploadReturn { + uploads: FileUploadItem[]; + /** True while any item is pending or in flight; gates composer submit. */ + isBusy: boolean; + uploadFiles: ( + files: File[], + targetDir: string, + onUploaded?: (path: string) => void, + ) => number; // returns how many files were actually queued + removeUpload: (id: string) => void; // aborts the in-flight request too +} +``` + +Occupied names are always auto-numbered; safety-boundary failures, candidate exhaustion, and I/O failures still produce an error row. `uploadFiles` stores `onUploaded` with each queued item and invokes it exactly once per successful upload with the server-confirmed final path. A batch accepts at most `MAX_FILES_PER_BATCH = 100` files; the overflow is not queued and surfaces as a single `tooManyFiles` notice row carrying the skipped count, so unbounded drops cannot keep the strictly-sequential queue busy for hours. + +- Done rows display the final file name. If `resultPath !== targetPath`, they additionally show a short auto-numbering hint so the user sees why the name differs from what they dropped. +- Callers pre-flight the target-specific capability set via the same `workspace.capabilities?.features` snapshot `VoiceButton` uses and hide the entry points when unsupported. +- Before queueing, reject files larger than `capabilities.limits.maxWorkspaceFileUploadBytes` (50 MiB fallback) locally with a clear error; the server-side 413 remains authoritative. +- Process each `uploadFiles` batch sequentially in selection order: one item is `uploading`, the rest remain `pending`. A failed or canceled item does not block later items. This keeps browser/daemon memory bounded and makes `@` insertion order deterministic; add concurrency only if measurements justify it later. +- Removing a pending/uploading row aborts the client request. Atomic writes guarantee that a partial target is never exposed, but cancellation is best effort: if the server has already received the body and begun publishing, the complete file may still be written. +- When `targetKey` changes or the hook unmounts, abort and clear the queue. Ignore any late completion from the previous generation so an upload started for workspace A cannot insert a path into workspace B's composer. + +### Web Shell: composer drag-and-drop + +1. Listen for `dragenter` / `dragover` / `dragleave` / `drop` on the composer surface. A batch containing only supported images remains on the existing image-attachment path; ordinary files and mixed batches use workspace upload, so one drop is never handled by both paths. +2. For workspace-upload batches, extract `event.dataTransfer.files` and call `uploadFiles(files, '.', onUploaded)` (target workspace root). +3. Progress UI: a thin strip above the composer input surface, one row per queued/uploading/error file — filename, state or percentage, and remove/cancel action. State text is not color-only, and icon actions have localized accessible names. Completed rows disappear after three seconds; error rows remain until dismissed. +4. On completion, add an inline `kind: 'file'` composer tag whose serialized value is `@`, escaping through the same pipeline existing file items use (`escapeAtReferenceText(sanitizeInsertText(path))`) — screenshot filenames with spaces and non-ASCII characters are common. + +### Web Shell: @ panel upload item + +In `useAtMentionMenu.ts`'s `createFileProvider`, when the files provider is in directory-browse mode: + +1. Prepend a synthetic `AtMentionItem` with a new `kind: 'upload'` at the top of the list. Its label/description use the existing i18n catalog. It appears only when the entry query is empty (the same condition that shows `currentDirectoryItem`) so it does not pollute filtered results, participates in normal keyboard navigation, and is subject to the existing `ITEM_LIMIT` slice. +2. Selecting it removes the mention text that opened the panel, snapshots `fileDirectoryRef.current`, invokes an `onUploadRequest(targetDir, restoreQuery)` callback wired in from the composer as a `UseAtMentionMenuOptions` field, and closes the menu. If upload availability vanished while the menu was open (stale item), the accept closes the menu without removing the text. The callback synchronously stores `targetDir` and the current upload `targetKey`, keeps the `restoreQuery` callback, then calls a mounted hidden `` so the browser treats it as part of the user gesture. This is UI behavior, not a workspace filesystem action, so it does not belong on `AtMentionWorkspaceActions`; the menu hook stays free of `DaemonClient` concerns. +3. The input's change handler uploads the selected files to the captured `targetDir` only if the captured `targetKey` is still current, then clears `input.value` so choosing the same file again fires a new change event. A native `cancel` listener (React only wires `cancel` on ``, and the event does not bubble) invokes the stored `restoreQuery` so a canceled picker gives the removed mention text back. +4. On success, add the same inline file tag used by an existing file-menu selection, directly from the server-confirmed response path. No new cache invalidation API is needed: selecting the upload item closes the menu, and `close()` already replaces `builtinCacheRef.current`; the next open fetches a fresh directory listing. + +Note: uploads to git-ignored paths succeed but remain invisible in the @ listing (`entries.filter((entry) => !entry.ignored)`); the inserted `@` reference still resolves. + +### Data flow summary + +``` +Browser file + ↓ (drag-drop or @ panel upload item) +useFileUpload.uploadFiles() [target workspace resolved] + ↓ (XHR with progress, or fetch) +DaemonClient / WorkspaceDaemonClient.uploadWorkspaceFile() + ↓ +POST /file/upload?path=... (raw octet-stream body) + ↓ mutate gate → workspace/trust/client/metadata admission → concurrency gate → raw parser +route candidate loop + ↓ fs.resolve(candidate, 'write') → fs.writeBytesAtomic (no-clobber create) + ↓ +201 with confirmed (possibly renumbered) path + ↓ +addTags([{ kind: 'file', serialized: '@' }], { placement: 'inline' }) +``` + +## Security and failure behavior + +- The route reuses the strict mutation gate, workspace trust checks, client identity validation, and `fs.resolve` boundary guards from the `workspace-file-write.ts` machinery. +- **Uploads never overwrite existing entries.** Occupied names, including in-workspace final-component symlinks, are auto-numbered without writing through them. No path in this feature modifies or replaces existing content — the candidate loop only ever creates new files. Escaping links and other safety-boundary failures, candidate exhaustion, and I/O failures remain errors. +- Binary writes are atomic (temp + publish): network failures and cancels never expose a partial target. A late client cancellation may still result in the complete file being published. +- The upload is not idempotent: if the server publishes the file but the response is lost, the client cannot know whether creation succeeded. The Web Shell does not automatically retry a request after bytes were sent; a manual retry may intentionally create a numbered copy. +- Wrong Content-Type → 415 before buffering. Zero-byte `application/octet-stream` uploads are valid and produce the SHA-256 of an empty buffer. +- Oversized bodies → the route-specific 413 `file_too_large` envelope; handler/fs failures use `sendFsError`; path escape or an escaping/racing symlink → 400; untrusted workspace → 403. +- The qualified route never falls back to the primary runtime for unknown (including already removed), untrusted, or draining workspaces. +- Upload-body memory is bounded by `MAX_UPLOAD_BYTES × MAX_CONCURRENT_UPLOADS` (about 200 MiB with the v1 constants); auth, workspace resolution, trust, Content-Type, Content-Length, metadata, client identity, and initial path-boundary resolution all run before the concurrency gate and body buffering. + +## Implementation order + +1. **fs layer** — add and export `MAX_UPLOAD_BYTES`, generalize the existing atomic publication internals around an already validated `Buffer`, then add the trust- and generation-gated no-clobber `writeBytesAtomic` create primitive with colocated tests. Preserve the existing 5 MiB text-write policy. +2. **Daemon route** — extend `routes/workspace-file-write.ts` with pre-buffer admission, one shared four-slot concurrency gate injected into legacy + qualified registrations, the route-owned numbered-candidate loop, upload-specific raw-parser errors, capability tag, and telemetry entry; keep route tests colocated in `workspace-file-write.test.ts`, with qualified cases in `workspace-qualified-rest.test.ts`. +3. **SDK** — add `maxWorkspaceFileUploadBytes` capability typing plus `uploadWorkspaceFile()` on `DaemonClient` and `WorkspaceDaemonClient` with the shared raw-POST helper, browser progress, timeout, and abort support, tests. +4. **`useFileUpload` hook** — standalone sequential queue with local size preflight and target-generation cancellation, testable without UI. +5. **Composer drag-and-drop** — hook + progress strip + reference insertion. +6. **@ panel upload item** — synthetic item + target-directory callback wiring; reuse the menu's existing cache reset on close. + +## Test plan + +- **fs layer**: byte-identical round-trip of binary fixtures, including an empty buffer; a payload greater than 5 MiB and at most `MAX_UPLOAD_BYTES` succeeds, proving the text-write default is not applied to the byte path; a direct `writeBytesAtomic` call above `MAX_UPLOAD_BYTES` fails with `file_too_large`; existing text writes above `MAX_WRITE_BYTES` remain rejected. Trust/generation: a direct untrusted call fails with `untrusted_workspace`; a generation closed after method entry but before publication leaves no target. Atomicity: interrupted write leaves no partial target; an external create racing the no-clobber publish still yields `file_already_exists`; symlink target rejected; new file created at `0o600`. +- **Daemon route**: correct bytes written with correct hash and size; zero-byte octet-stream → 201 with the empty-buffer hash; wrong or missing Content-Type → the exact 415 `unsupported_media_type` envelope before buffering; an upload greater than 5 MiB and at most `MAX_UPLOAD_BYTES` succeeds; an oversized declared `Content-Length` is rejected immediately, while a chunked or understated body above `MAX_UPLOAD_BYTES` is rejected by the raw parser before the handler/fs write; both use the exact upload-specific 413 envelope (`errorKind`, `status`, and `maxBytes` included, with no "10 MB" message). Missing/invalid `path`, a requested basename over 255 UTF-8 bytes, invalid client id, missing/non-directory parents, and boundary escapes are rejected before buffering. Paths containing spaces, non-ASCII, `%`, and `#` decode exactly once; a name occupied by a file, directory, or in-workspace final-component symlink → 201 with a numbered `path`, with no write through the existing entry; an escaping symlink remains a boundary error. Numbering preserves the final extension, handles no-extension and dotfile names, skips taken candidates, trims a long Unicode stem to the 255-byte policy cap, and fails at the 1000-candidate cap; auto-numbering never modifies the requested target; concurrent same-name uploads land on distinct candidates. Four admitted uploads may buffer concurrently across both route forms; a fifth receives the exact 429 `upload_busy` response and `Retry-After`, and disconnect/parser-error paths release their slot. The response has no derived `renamed` flag. Capability tag and `limits.maxWorkspaceFileUploadBytes` are advertised. Qualified route: untrusted, unknown (including already removed), and draining workspaces are rejected before buffering and never fall back to the primary runtime. +- **SDK**: progress callbacks fire in a browser; requesting progress without `XMLHttpRequest` fails before sending; omitted timeout inherits the client default, `timeoutMs: 0` disables it, and an explicit timeout or abort signal cancels the request; filesystem errors expose `errorKind` while other daemon errors preserve their existing parsed bodies; both legacy-primary and workspace-qualified clients. +- **Web Shell hook/UI**: a file above the advertised limit is rejected without an HTTP request; a batch above 100 files queues the first 100 and renders one `tooManyFiles` notice row with the skipped count; a batch runs one request at a time in selection order; failure/cancel does not block the next item; removing a pending item prevents it from starting; a late response after abort does not invoke `onUploaded`; changing the target workspace aborts and clears the old queue and ignores late completions; each successful final path creates exactly one inline file tag; removing the last tag restores the placeholder; completed rows disappear after three seconds. Pure supported-image drops stay on the image-attachment path, while ordinary files and mixed batches upload without leaving drag-active styling behind. +- **Web Shell E2E**: drag a file onto the composer → progress strip appears above the input surface → file exists in the workspace → an inline file tag appears (include filenames with spaces/non-ASCII and a literal `%` to cover escaping); drop a file whose requested name is occupied, including by an in-workspace symlink → upload succeeds as `name (1).ext` with an auto-numbering hint derived from the differing paths, the existing entry untouched, and the tag uses the final name; batch drop preserves upload/tag order. @ panel: browse into a nested directory, select upload, choose a file → the trigger `@` is removed, the captured directory receives the file, and an inline file tag appears; reopening the menu fetches a fresh listing without a public cache API; selecting the same local file twice still fires two uploads. Entry points are hidden when either the upload capability or the required qualified-route capability is absent. Submit prompts that reference one uploaded text file and one uploaded image and verify the existing resolver supplies their content; an unsupported/oversized binary surfaces the resolver's existing readable error rather than being described as universally consumable. diff --git a/docs/developers/daemon/07-workspace-filesystem.md b/docs/developers/daemon/07-workspace-filesystem.md index 82594a70ce..a5db05a30d 100644 --- a/docs/developers/daemon/07-workspace-filesystem.md +++ b/docs/developers/daemon/07-workspace-filesystem.md @@ -40,7 +40,7 @@ That text-read capability slice covers direct `read_file` plus the shared pre-re | File | Purpose | | -------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `paths.ts` | `canonicalizeWorkspace`, `resolveWithinWorkspace`, `hasSuspiciousPathPattern`, branded `ResolvedPath`, `Intent` union (`read \| write \| list \| stat \| glob`). | -| `policy.ts` | `MAX_READ_BYTES`, `MAX_TEXT_SCAN_BYTES`, `MAX_WRITE_BYTES`, `BINARY_PROBE_BYTES`, `assertTrustedForIntent`, `detectBinary`, `enforceReadBytesSize`, `enforceReadSize`, `enforceWriteSize`, `shouldIgnore`. | +| `policy.ts` | `MAX_READ_BYTES`, `MAX_TEXT_SCAN_BYTES`, `MAX_WRITE_BYTES`, `MAX_UPLOAD_BYTES`, `BINARY_PROBE_BYTES`, `assertTrustedForIntent`, `detectBinary`, `enforceReadBytesSize`, `enforceReadSize`, `enforceWriteSize`, `shouldIgnore`. | | `audit.ts` | `FS_ACCESS_EVENT_TYPE`, `FS_DENIED_EVENT_TYPE`, `createAuditPublisher`, audit payload types. | | `errors.ts` | `FsError` class, `isFsError`, `FsErrorKind` union (14 kinds), `FsErrorStatus` union (`400 / 403 / 404 / 409 / 413 / 422 / 500 / 503`). | | `workspace-file-system.ts` | `createWorkspaceFileSystemFactory`, `WorkspaceFileSystem` (the orchestrator that reads/writes/lists), `WriteMode`, `ContentHash`, `FsEntry`, `FsStat`, `ListOptions`, `GlobOptions`, `ReadTextOptions`, `ReadBytesOptions`, `WriteTextAtomicOptions`. | @@ -235,15 +235,16 @@ flowchart LR ## Configuration -| Source | Knob | Effect | -| ------------------------------------------------- | --------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | -| `WorkspaceFileSystemFactoryDeps.trusted: boolean` | Constructor input | Whether writes are allowed; defaults to `true` from `runQwenServe`, `false` from `createServeApp` (with warning). | -| Constant | `MAX_READ_BYTES = 256 KiB` | Full-snapshot and returned-text cap; larger text requires an explicit window argument. | -| Constant | `MAX_TEXT_SCAN_BYTES = 8 MiB` | Bytes a large-text read may scan to locate a line offset; past it, `file_too_large`. | -| Constant | `MAX_WRITE_BYTES = 5 MiB` | Write cap; sized below `express.json({ limit: '10mb' })`. | -| Constant | `BINARY_PROBE_BYTES = 4096` | Sample size for content-based binary detection. | -| Capability tags | `workspace_file_read`, `workspace_file_bytes`, `workspace_file_write` | See [`11-capabilities-versioning.md`](./11-capabilities-versioning.md). | -| Workspace files | `.gitignore`, `.qwenignore` | Ignored paths surface as `ignored: true` from `shouldIgnore`. | +| Source | Knob | Effect | +| ------------------------------------------------- | ---------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | +| `WorkspaceFileSystemFactoryDeps.trusted: boolean` | Constructor input | Whether writes are allowed; defaults to `true` from `runQwenServe`, `false` from `createServeApp` (with warning). | +| Constant | `MAX_READ_BYTES = 256 KiB` | Full-snapshot and returned-text cap; larger text requires an explicit window argument. | +| Constant | `MAX_TEXT_SCAN_BYTES = 8 MiB` | Bytes a large-text read may scan to locate a line offset; past it, `file_too_large`. | +| Constant | `MAX_WRITE_BYTES = 5 MiB` | Write cap; sized below `express.json({ limit: '10mb' })`. | +| Constant | `MAX_UPLOAD_BYTES = 50 MiB` | Binary upload cap for `POST /file/upload`; uploads never overwrite and auto-number occupied names. | +| Constant | `BINARY_PROBE_BYTES = 4096` | Sample size for content-based binary detection. | +| Capability tags | `workspace_file_read`, `workspace_file_bytes`, `workspace_file_write`, `workspace_file_upload` | See [`11-capabilities-versioning.md`](./11-capabilities-versioning.md). | +| Workspace files | `.gitignore`, `.qwenignore` | Ignored paths surface as `ignored: true` from `shouldIgnore`. | ## Caveats & Known Limits diff --git a/docs/developers/daemon/11-capabilities-versioning.md b/docs/developers/daemon/11-capabilities-versioning.md index 0a147c19d4..fc87c2ff35 100644 --- a/docs/developers/daemon/11-capabilities-versioning.md +++ b/docs/developers/daemon/11-capabilities-versioning.md @@ -122,7 +122,7 @@ Extension management: `extension_management_v2` adds the global `/extensions/*` Workspace-qualified session reads: `workspace_persisted_transcript`, `workspace_session_export`, `workspace_archived_session_export`. The active and archived export tags are independent from each other and from `session_export` and `workspace_qualified_rest_core`, so clients must pre-flight the exact storage state they intend to export. Persisted transcript paging permits an untrusted secondary under its bounded read policy; both full export paths remain trusted-only. -Workspace mutation (Wave 4+): `workspace_memory`, `workspace_agents`, `workspace_agent_generate`, `workspace_acp_preheat`, `workspace_tool_toggle`, **`workspace_settings`** (conditional), `workspace_permissions`, `workspace_init`, `workspace_github_setup`, `workspace_trust`, `workspace_mcp_restart`, `workspace_mcp_manage`, `workspace_file_read`, `workspace_file_bytes`, `workspace_file_read_cursor`, `workspace_file_write`, **`workspace_reload`** (conditional). +Workspace mutation (Wave 4+): `workspace_memory`, `workspace_agents`, `workspace_agent_generate`, `workspace_acp_preheat`, `workspace_tool_toggle`, **`workspace_settings`** (conditional), `workspace_permissions`, `workspace_init`, `workspace_github_setup`, `workspace_trust`, `workspace_mcp_restart`, `workspace_mcp_manage`, `workspace_file_read`, `workspace_file_bytes`, `workspace_file_read_cursor`, `workspace_file_write`, `workspace_file_upload`, **`workspace_reload`** (conditional). MCP guardrails: **`mcp_guardrails`** (`modes: ['warn', 'enforce']`), `mcp_guardrail_events`, `mcp_server_runtime_mutation`, **`mcp_workspace_pool`** (conditional), **`mcp_pool_restart`** (conditional). diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 9835f42323..6b67f2cdca 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -195,6 +195,7 @@ registry. Clients **must** gate UI off `features`, not off `mode` (per design 'workspace_mcp_manage', 'mcp_guardrail_events', 'mcp_server_runtime_mutation', 'workspace_file_read', 'workspace_file_bytes', 'workspace_file_write', + 'workspace_file_upload', 'session_approval_mode_control', 'workspace_tool_toggle', 'workspace_skill_toggle', 'workspace_skill_batch_toggle', 'workspace_settings', 'workspace_init', 'workspace_mcp_restart', @@ -277,8 +278,13 @@ the hash-aware text mutation routes (`POST /file/write`, `POST /file/edit`). The write tag means the route contract exists; it does not mean the current deployment is open for anonymous mutation. Write/edit are strict mutation routes and require a configured bearer token even on loopback. +`workspace_file_upload` covers `POST /file/upload`, the binary ingress route: +an `application/octet-stream` body capped at `MAX_UPLOAD_BYTES` (50 MiB) is +written into the workspace without ever overwriting — an occupied name is +auto-numbered (`name (1).ext`, `name (2).ext`, ...). It is also a strict +mutation route. -When `workspace_qualified_rest_core` is advertised, the same file surface is also available at `/workspaces/:workspace/file`, `/workspaces/:workspace/file/bytes`, `/workspaces/:workspace/stat`, `/workspaces/:workspace/list`, `/workspaces/:workspace/glob`, `/workspaces/:workspace/file/write`, and `/workspaces/:workspace/file/edit`. +When `workspace_qualified_rest_core` is advertised, the same file surface is also available at `/workspaces/:workspace/file`, `/workspaces/:workspace/file/bytes`, `/workspaces/:workspace/stat`, `/workspaces/:workspace/list`, `/workspaces/:workspace/glob`, `/workspaces/:workspace/file/write`, `/workspaces/:workspace/file/edit`, and `/workspaces/:workspace/file/upload`. The same tag also exposes workspace-qualified project-agent CRUD at `/workspaces/:workspace/agents` and `/workspaces/:workspace/agents/:agentType`. These plural routes only read or mutate project-level agents for the selected workspace; `global` and `user` scope requests return `400 { code: "global_scope_not_supported_for_workspace_route" }`. Workspace-less `/workspace/agents` routes retain their existing primary-workspace behavior and remain the only REST surface for user-level agent scope. diff --git a/integration-tests/cli/qwen-serve-routes.test.ts b/integration-tests/cli/qwen-serve-routes.test.ts index 90402f5758..be73188017 100644 --- a/integration-tests/cli/qwen-serve-routes.test.ts +++ b/integration-tests/cli/qwen-serve-routes.test.ts @@ -364,6 +364,7 @@ describe('qwen serve — capabilities envelope', () => { 'workspace_file_bytes', 'workspace_file_read_cursor', 'workspace_file_write', + 'workspace_file_upload', 'session_approval_mode_control', 'workspace_tool_toggle', 'workspace_skill_toggle', diff --git a/packages/cli/src/serve/bridge-file-system-adapter.test.ts b/packages/cli/src/serve/bridge-file-system-adapter.test.ts index d6330c6f3c..ffa515aad1 100644 --- a/packages/cli/src/serve/bridge-file-system-adapter.test.ts +++ b/packages/cli/src/serve/bridge-file-system-adapter.test.ts @@ -979,6 +979,7 @@ describe('createBridgeFileSystemAdapter', () => { })), edit: vi.fn(), editAtomic: vi.fn(), + writeBytesAtomic: vi.fn(), }; }, }; @@ -1030,6 +1031,7 @@ describe('createBridgeFileSystemAdapter', () => { })), edit: vi.fn(), editAtomic: vi.fn(), + writeBytesAtomic: vi.fn(), }; }, }; diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index d8eec7b13d..39a60718cb 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -162,6 +162,12 @@ export const SERVE_CAPABILITY_REGISTRY = { // gate. Clients should still pre-flight `require_auth` separately for // deployment posture; this tag only means the route contract exists. workspace_file_write: { since: 'v1' }, + // Daemon hosts binary file upload (`POST /file/upload`) behind the strict + // mutation gate. Uploads never overwrite; occupied names auto-number. New + // route contract = new tag (same split as `workspace_file_bytes` from + // `workspace_file_read`). The advertised upload byte cap is surfaced via + // `limits.maxWorkspaceFileUploadBytes`. + workspace_file_upload: { since: 'v1' }, // Daemon hosts the session-level approval-mode // control route `POST /session/:id/approval-mode` (gated by the // mutation gate, strict). The route accepts `{mode, persist?}` — @@ -345,8 +351,8 @@ export const SERVE_CAPABILITY_REGISTRY = { scratch_workspace_registration: { since: 'v1' }, workspace_runtime_removal: { since: 'v1' }, // Workspace-qualified core REST routes under `/workspaces/:workspace/...`. - // Covers core file/status/permissions/trust/lifecycle/MCP/tool, memory, - // workspace agent CRUD, and persisted session organization surfaces. + // Covers core file read/write/upload, status/permissions/trust/lifecycle/MCP/tool, + // memory, workspace agent CRUD, and persisted session organization surfaces. // Workspace-qualified settings also require the existing // `workspace_settings` tag because that surface depends on settings // persistence. ACP/WebSocket and auth stay outside this core tag; diff --git a/packages/cli/src/serve/fs/index.ts b/packages/cli/src/serve/fs/index.ts index 81547f8528..b974675c7e 100644 --- a/packages/cli/src/serve/fs/index.ts +++ b/packages/cli/src/serve/fs/index.ts @@ -21,6 +21,7 @@ export { export { MAX_READ_BYTES, MAX_WRITE_BYTES, + MAX_UPLOAD_BYTES, BINARY_PROBE_BYTES, assertTrustedForIntent, detectBinary, diff --git a/packages/cli/src/serve/fs/policy.ts b/packages/cli/src/serve/fs/policy.ts index 472299c804..a6eca37790 100644 --- a/packages/cli/src/serve/fs/policy.ts +++ b/packages/cli/src/serve/fs/policy.ts @@ -60,6 +60,17 @@ export const MAX_TEXT_SCAN_BYTES = 8 * 1024 * 1024; */ export const MAX_WRITE_BYTES = 5 * 1024 * 1024; +/** + * Maximum bytes accepted by the binary upload write path + * (`writeBytesAtomic`). This is a distinct binary-ingress policy, NOT an + * increase to the agent text-write limit: text writes keep + * `MAX_WRITE_BYTES`. Sized for screenshots, data files, and configs that a + * user drags into the Web Shell. The daemon's upload route and the fs + * boundary share this single constant so a request buffered under the parser + * cap is never rejected later under a different limit. + */ +export const MAX_UPLOAD_BYTES = 50 * 1024 * 1024; + /** * Sample size used for content-based binary detection. Aligned with * `isBinaryFile` from `packages/core/src/utils/fileUtils.ts:414` so diff --git a/packages/cli/src/serve/fs/workspace-file-system.test.ts b/packages/cli/src/serve/fs/workspace-file-system.test.ts index 03f64acfb7..4b5f5ffafe 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.test.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.test.ts @@ -5,7 +5,7 @@ */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { promises as fsp } from 'node:fs'; +import { promises as fsp, writeFileSync } from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { createHash, randomBytes } from 'node:crypto'; @@ -2283,6 +2283,218 @@ describe('WorkspaceFileSystem - multi-root workspaces', () => { }); }); +describe('WorkspaceFileSystem - writeBytesAtomic', () => { + let h: Harness; + beforeEach(async () => { + h = await makeHarness(); + }); + afterEach(async () => teardown(h)); + + it('round-trips arbitrary bytes byte-identically', async () => { + const data = randomBytes(4096); + const r = await h.fs.resolve('blob.bin', 'write'); + const out = await h.fs.writeBytesAtomic(r, data); + expect(out.sizeBytes).toBe(data.length); + expect(out.hash).toBe(rawHash(data)); + expect(await fsp.readFile(r as string)).toEqual(data); + }); + + it('accepts an empty buffer and hashes it', async () => { + const r = await h.fs.resolve('empty.bin', 'write'); + const out = await h.fs.writeBytesAtomic(r, Buffer.alloc(0)); + expect(out.sizeBytes).toBe(0); + expect(out.hash).toBe(rawHash(Buffer.alloc(0))); + expect((await fsp.stat(r as string)).size).toBe(0); + }); + + it('accepts a payload above the 5 MiB text cap (binary policy applies)', async () => { + // 6 MiB > MAX_WRITE_BYTES (5 MiB) but <= MAX_UPLOAD_BYTES (50 MiB): + // proves the byte path does not reuse the text-write default. + const data = Buffer.alloc(6 * 1024 * 1024, 7); + const r = await h.fs.resolve('big.bin', 'write'); + const out = await h.fs.writeBytesAtomic(r, data); + expect(out.sizeBytes).toBe(data.length); + expect((await fsp.stat(r as string)).size).toBe(data.length); + }); + + it('rejects a payload above MAX_UPLOAD_BYTES with file_too_large', async () => { + const data = Buffer.alloc(50 * 1024 * 1024 + 1); + const r = await h.fs.resolve('too-big.bin', 'write'); + const err = await h.fs.writeBytesAtomic(r, data).catch((e: unknown) => e); + expect(isFsError(err)).toBe(true); + expect((err as { kind: string }).kind).toBe('file_too_large'); + await expect(fsp.stat(r as string)).rejects.toMatchObject({ + code: 'ENOENT', + }); + }); + + it('rejects an existing target with file_already_exists', async () => { + await fsp.writeFile(path.join(h.workspace, 'taken.bin'), 'x'); + const r = await h.fs.resolve('taken.bin', 'write'); + const err = await h.fs + .writeBytesAtomic(r, Buffer.from('y')) + .catch((e: unknown) => e); + expect(isFsError(err)).toBe(true); + expect((err as { kind: string }).kind).toBe('file_already_exists'); + // The existing content is untouched. + expect( + await fsp.readFile(path.join(h.workspace, 'taken.bin'), 'utf-8'), + ).toBe('x'); + }); + + it('does not audit a no-clobber collision as fs.denied', async () => { + // Collisions are the upload route's expected candidate-loop control + // flow; monitoring keyed on fs.denied volume must not see them. + await fsp.writeFile(path.join(h.workspace, 'taken.bin'), 'x'); + const r = await h.fs.resolve('taken.bin', 'write'); + const deniedBefore = h.events.filter( + (e) => e.type === FS_DENIED_EVENT_TYPE, + ).length; + await expect( + h.fs.writeBytesAtomic(r, Buffer.from('y')), + ).rejects.toMatchObject({ kind: 'file_already_exists' }); + expect( + h.events.filter((e) => e.type === FS_DENIED_EVENT_TYPE), + ).toHaveLength(deniedBefore); + }); + + it('rejects an escaping symlink at the boundary', async () => { + const outside = path.join(h.scratch, 'outside.txt'); + await fsp.writeFile(outside, 'external'); + const link = path.join(h.workspace, 'evil-link'); + await fsp.symlink(outside, link); + const err = await h.fs.resolve('evil-link', 'write').catch((e) => e); + expect(isFsError(err)).toBe(true); + expect((err as { kind: string }).kind).toBe('symlink_escape'); + expect(await fsp.readFile(outside, 'utf-8')).toBe('external'); + }); + + it('rejects a direct symlink target with symlink_escape', async () => { + // Bypasses `resolve`'s realpath-follow to exercise the defensive + // lstat branch in the no-clobber create path. The route never hands + // `writeBytesAtomic` an in-workspace symlink (it numbers instead), but + // the fs primitive must still refuse to publish through one. + const real = path.join(h.workspace, 'real.bin'); + await fsp.writeFile(real, 'orig'); + const link = path.join(h.workspace, 'link.bin'); + await fsp.symlink(real, link); + const err = await h.fs + .writeBytesAtomic(link as ResolvedPath, Buffer.from('overwrite?')) + .catch((e: unknown) => e); + expect(isFsError(err)).toBe(true); + expect((err as { kind: string }).kind).toBe('symlink_escape'); + expect(await fsp.readFile(real, 'utf-8')).toBe('orig'); + }); + + it('creates new files at 0o600', async () => { + if (process.platform === 'win32') return; + const r = await h.fs.resolve('secret.bin', 'write'); + await h.fs.writeBytesAtomic(r, Buffer.from('s3cret')); + const mode = (await fsp.stat(r as string)).mode & 0o777; + expect(mode).toBe(0o600); + }); + + it('fails with untrusted_workspace on an untrusted factory', async () => { + await teardown(h); + h = await makeHarness({ trusted: false }); + const r = await h.fs.resolve('nope.bin', 'write'); + const err = await h.fs + .writeBytesAtomic(r, Buffer.from('x')) + .catch((e: unknown) => e); + expect(isFsError(err)).toBe(true); + expect((err as { kind: string }).kind).toBe('untrusted_workspace'); + }); + + it('leaves no target when the generation closes before publication', async () => { + let checks = 0; + await teardown(h); + h = await makeHarness({ + generationGuard: { + assertOpen() { + checks += 1; + // resolve() consumes calls 1-2; writeBytesAtomic entry (3) and + // the inside-lock re-check (4) precede the pre-publish checkpoint + // (5). Close at the final publish checkpoint, when the temp file + // already exists. + if (checks === 5) throw new Error('generation closed'); + }, + }, + }); + const target = path.join(h.workspace, 'gen-closed.bin'); + const r = await h.fs.resolve(target, 'write'); + await expect( + h.fs.writeBytesAtomic(r, Buffer.from('must not land')), + ).rejects.toThrow('generation closed'); + expect(checks).toBe(5); + await expect(fsp.stat(target)).rejects.toMatchObject({ code: 'ENOENT' }); + // No stray temp files left behind in the workspace. + const leftover = (await fsp.readdir(h.workspace)).filter((n) => + n.endsWith('.tmp'), + ); + expect(leftover).toEqual([]); + }); + + it('never clobbers when an external writer wins the pre-publish race', async () => { + let checks = 0; + let target = ''; + await teardown(h); + h = await makeHarness({ + generationGuard: { + assertOpen() { + checks += 1; + // At the pre-publish checkpoint (after the entry/lock checks and + // the temp write), an external writer grabs the name. + if (checks === 5) writeFileSync(target, 'external'); + }, + }, + }); + target = path.join(h.workspace, 'race.bin'); + const r = await h.fs.resolve('race.bin', 'write'); + const err = await h.fs + .writeBytesAtomic(r, Buffer.from('upload')) + .catch((e: unknown) => e); + expect(isFsError(err)).toBe(true); + expect((err as { kind: string }).kind).toBe('file_already_exists'); + // The external writer's content is untouched. + expect(await fsp.readFile(target, 'utf-8')).toBe('external'); + const leftover = (await fsp.readdir(h.workspace)).filter((n) => + n.endsWith('.tmp'), + ); + expect(leftover).toEqual([]); + }); + + it('records audit access on success and denial', async () => { + const ignore = new Ignore().add(['*.log']); + await teardown(h); + h = await makeHarness({ ignore }); + + const data = randomBytes(64); + const r = await h.fs.resolve('audit.log', 'write'); + await h.fs.writeBytesAtomic(r, data); + const access = h.events.find( + (e) => + e.type === FS_ACCESS_EVENT_TYPE && + (e.data as { intent: string }).intent === 'write', + ); + expect(access).toBeDefined(); + expect((access!.data as { sizeBytes: number }).sizeBytes).toBe(data.length); + expect((access!.data as { matchedIgnore?: string }).matchedIgnore).toBe( + 'file', + ); + + const tooBig = await h.fs.resolve('audit-big.bin', 'write'); + await h.fs + .writeBytesAtomic(tooBig, Buffer.alloc(50 * 1024 * 1024 + 1)) + .catch(() => {}); + const denied = h.events.find( + (e) => + e.type === FS_DENIED_EVENT_TYPE && + (e.data as { errorKind: string }).errorKind === 'file_too_large', + ); + expect(denied).toBeDefined(); + }); +}); + describe('WorkspaceFileSystem - factory', () => { it('canonicalizes the workspace once at factory build', async () => { const scratch = await fsp.mkdtemp( diff --git a/packages/cli/src/serve/fs/workspace-file-system.ts b/packages/cli/src/serve/fs/workspace-file-system.ts index d6fe81899f..c90f8a12b8 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.ts @@ -38,7 +38,12 @@ import { type AuditPublisher, createAuditPublisher, } from './audit.js'; -import { FsError, wrapAsFsError, type FsErrorKind } from './errors.js'; +import { + FsError, + isFsError, + wrapAsFsError, + type FsErrorKind, +} from './errors.js'; import { assertCursorMatchesFile, decodeTextCursor, @@ -55,6 +60,7 @@ import { BINARY_PROBE_BYTES, MAX_READ_BYTES, MAX_TEXT_SCAN_BYTES, + MAX_UPLOAD_BYTES, assertTrustedForIntent, enforceReadSize, enforceWriteSize, @@ -286,6 +292,23 @@ export interface WorkspaceFileSystem { newText: string, opts: { expectedHash: ContentHash }, ): Promise; + /** + * Single-purpose no-clobber binary create. Writes `data` atomically + * (temp + publish) at `p`; it cannot modify or replace existing file + * content. An existing target (including a final-component symlink) + * throws `file_already_exists` (`symlink_escape` for a symlink). The + * caller is responsible for choosing a free name; the upload route owns + * the numbered-candidate policy; the collision is expected control flow + * there and emits no `fs.denied` audit event. `data` is size-checked + * against `MAX_UPLOAD_BYTES` here — the binary-ingress policy, NOT the + * `MAX_WRITE_BYTES` text default. Trust and generation guards are enforced + * at entry, inside the path lock, and at the final publish checkpoint. + * New files are created at `0o600`. + */ + writeBytesAtomic( + p: ResolvedPath, + data: Buffer, + ): Promise<{ sizeBytes: number; hash: ContentHash }>; } /** @@ -1476,6 +1499,48 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { } } + async writeBytesAtomic( + p: ResolvedPath, + data: Buffer, + ): Promise<{ sizeBytes: number; hash: ContentHash }> { + const start = performance.now(); + try { + this.deps.generationGuard?.assertOpen(); + assertTrustedForIntent(this.deps.trusted, 'write'); + enforceWriteSize(data.length, MAX_UPLOAD_BYTES); + const out = await this.deps.pathLocks.runExclusive( + p as string, + async () => { + await assertCreateTargetAbsent(p as string); + this.deps.generationGuard?.assertOpen(); + const result = await atomicPublishResolvedFile({ + target: p, + buf: data, + mode: 'create', + assertGenerationOpen: () => this.deps.generationGuard?.assertOpen(), + }); + const verdict = this.ignoreVerdict(p, 'file'); + this.deps.audit.recordAccess(this.deps.ctx, { + intent: 'write', + absolute: p, + durationMs: performance.now() - start, + sizeBytes: result.sizeBytes, + matchedIgnore: verdict.ignored ? verdict.category : undefined, + }); + return { sizeBytes: result.sizeBytes, hash: result.hash }; + }, + ); + return out; + } catch (err) { + // A no-clobber collision is the upload route's expected candidate-loop + // outcome (it numbers on), not a boundary policy denial; auditing it + // would emit one `fs.denied` per skipped occupied name. + throw this.recordAndWrap(err, 'write', p as string, { + audit: !(isFsError(err) && err.kind === 'file_already_exists'), + }); + } + } + /** * Coerce an arbitrary thrown value into an `FsError`, emit the * matching `fs.denied` audit event, and return the typed error @@ -1489,7 +1554,12 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { * - routes can still rely on `instanceof FsError` * for their `sendFsError` serializer. */ - private recordAndWrap(err: unknown, intent: Intent, input: string): Error { + private recordAndWrap( + err: unknown, + intent: Intent, + input: string, + opts?: { audit?: boolean }, + ): Error { if ( err instanceof Error && 'code' in err && @@ -1498,6 +1568,9 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { return err; } const fs = wrapAsFsError(err); + if (opts?.audit === false) { + return fs; + } this.deps.audit.recordDenied(this.deps.ctx, { intent, input, @@ -2177,7 +2250,39 @@ function mergeWriteMeta( async function atomicWriteTextResolvedFile( input: AtomicWriteTextInput, ): Promise { - const target = input.target as string; + const buf = await encodeTextFileContentAsync( + input.target, + input.content, + buildWriteMeta(input.meta), + ); + // Text writes keep the `MAX_WRITE_BYTES` policy. The byte upload path + // validates its own buffer against `MAX_UPLOAD_BYTES` before calling the + // shared publisher, so the two policies stay distinct. + enforceWriteSize(buf.length); + return atomicPublishResolvedFile({ + target: input.target, + buf, + mode: input.mode, + expectedHash: input.expectedHash, + assertGenerationOpen: input.assertGenerationOpen, + }); +} + +/** + * Shared atomic temp+publish core. `buf` MUST already be size-validated by + * the caller against its own policy (`MAX_WRITE_BYTES` for text, + * `MAX_UPLOAD_BYTES` for binary uploads). This function does not re-check + * size; it only handles the filesystem mechanics: parent validation, temp + * reservation, precondition checks, generation gating, and publication. + */ +async function atomicPublishResolvedFile(input: { + target: string; + buf: Buffer; + mode: WriteMode; + expectedHash?: ContentHash; + assertGenerationOpen?: () => void; +}): Promise { + const target = input.target; const parent = path.dirname(target); const parentStat = await fsp.lstat(parent); // Defense-in-depth against a parent-symlink swap. A full fix requires @@ -2195,24 +2300,34 @@ async function atomicWriteTextResolvedFile( `parent path is not a directory: ${parent}`, ); } - const tmpPath = path.join( - parent, - `.${path.basename(target)}.${process.pid}.${randomBytes(6).toString('hex')}.tmp`, - ); + const tmpSuffix = `.${process.pid}.${randomBytes(6).toString('hex')}.tmp`; + // The temp name is `.`. When the target basename is itself + // near the filesystem NAME_MAX (255 bytes) — a 255-byte upload, say — the + // untruncated temp name would exceed it and `open()` would fail ENAMETOOLONG + // before the atomic publish could run. Cap the basename portion (UTF-8-safe). + const tmpNameMaxBytes = 255; + const tmpBaseMaxBytes = + tmpNameMaxBytes - 1 - Buffer.byteLength(tmpSuffix, 'utf-8'); + let tmpBase = path.basename(target); + if (Buffer.byteLength(tmpBase, 'utf-8') > tmpBaseMaxBytes) { + tmpBase = safeUtf8Truncate( + Buffer.from(tmpBase, 'utf-8'), + tmpBaseMaxBytes, + ).toString('utf-8'); + } + const tmpPath = path.join(parent, `.${tmpBase}${tmpSuffix}`); let tempLive = false; let tempHandle: Awaited> | undefined; let tempStat: Awaited> | undefined; try { tempHandle = await reserveTempFile(tmpPath); tempLive = true; - const encoded = await writeEncodedTextTemp({ - targetPath: target, + const written = await writeBufferToTemp({ tmpPath, - content: input.content, - meta: input.meta, + buf: input.buf, handle: tempHandle, }); - tempStat = encoded.stat; + tempStat = written.stat; const targetState = await assertAtomicTargetPrecondition({ target, mode: input.mode, @@ -2231,7 +2346,7 @@ async function atomicWriteTextResolvedFile( } tempLive = false; await fsyncParentDirBestEffort(parent); - return encoded; + return written; } catch (err) { await tempHandle?.close().catch(() => undefined); if (tempLive) { @@ -2251,20 +2366,17 @@ async function reserveTempFile( return fsp.open(tmpPath, 'wx', 0o600); } -async function writeEncodedTextTemp(input: { - targetPath: string; +/** + * Write an already-validated buffer to the reserved temp handle and verify + * the handle still names the same regular file. Shared by the text and + * binary publishers; size policy is enforced by the caller, not here. + */ +async function writeBufferToTemp(input: { tmpPath: string; - content: string; - meta: ReadMeta; + buf: Buffer; handle: Awaited>; }): Promise { - const buf = await encodeTextFileContentAsync( - input.targetPath, - input.content, - buildWriteMeta(input.meta), - ); - enforceWriteSize(buf.length); - await input.handle.writeFile(buf); + await input.handle.writeFile(input.buf); await syncHandleBestEffort(input.handle); const st = await fsp.lstat(input.tmpPath); const opened = await input.handle.stat(); @@ -2282,7 +2394,7 @@ async function writeEncodedTextTemp(input: { `temporary path is not a regular file: ${input.tmpPath}`, ); } - return { sizeBytes: buf.length, hash: hashBuffer(buf), stat: st }; + return { sizeBytes: input.buf.length, hash: hashBuffer(input.buf), stat: st }; } async function assertCreateTargetAbsent(target: string): Promise { diff --git a/packages/cli/src/serve/routes/capabilities.ts b/packages/cli/src/serve/routes/capabilities.ts index a50fa1fd79..229f90fc98 100644 --- a/packages/cli/src/serve/routes/capabilities.ts +++ b/packages/cli/src/serve/routes/capabilities.ts @@ -8,6 +8,7 @@ import type { Application } from 'express'; import type { AcpSessionBridge } from '../acp-session-bridge.js'; import { getServeProtocolVersions } from '../capabilities.js'; import type { getAdvertisedServeFeatures } from '../capabilities.js'; +import { MAX_UPLOAD_BYTES } from '../fs/index.js'; import { advertisedMaxPendingPromptsPerSession, advertisedMaxSessions, @@ -70,6 +71,9 @@ export function registerCapabilitiesRoutes( deps.maxPendingPromptsPerSession, ), sessionRestoreTimeoutMs: deps.sessionRestoreTimeoutMs, + ...(features.includes('workspace_file_upload') + ? { maxWorkspaceFileUploadBytes: MAX_UPLOAD_BYTES } + : {}), ...(multiWorkspace ? { maxSessionsPerWorkspace: advertisedMaxSessions( diff --git a/packages/cli/src/serve/routes/workspace-file-read.test.ts b/packages/cli/src/serve/routes/workspace-file-read.test.ts index a03dd5ed76..ef0f6044e6 100644 --- a/packages/cli/src/serve/routes/workspace-file-read.test.ts +++ b/packages/cli/src/serve/routes/workspace-file-read.test.ts @@ -665,6 +665,11 @@ describe('capability advertisement', () => { expect(res.body.features).toContain('workspace_file_read'); expect(res.body.features).toContain('workspace_file_bytes'); expect(res.body.features).toContain('workspace_file_write'); + expect(res.body.features).toContain('workspace_file_upload'); + // The upload byte cap is advertised alongside the feature. + expect(res.body.limits?.maxWorkspaceFileUploadBytes).toBe( + 50 * 1024 * 1024, + ); } finally { await teardown(h); } diff --git a/packages/cli/src/serve/routes/workspace-file-write.test.ts b/packages/cli/src/serve/routes/workspace-file-write.test.ts index 888d83ea88..2d68f0d3de 100644 --- a/packages/cli/src/serve/routes/workspace-file-write.test.ts +++ b/packages/cli/src/serve/routes/workspace-file-write.test.ts @@ -6,9 +6,13 @@ import { createHash, randomBytes } from 'node:crypto'; import { promises as fsp } from 'node:fs'; +import * as http from 'node:http'; +import type { AddressInfo } from 'node:net'; import * as os from 'node:os'; import * as path from 'node:path'; +import { gzipSync } from 'node:zlib'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import express from 'express'; import request from 'supertest'; import { createServeApp } from '../server.js'; import { @@ -35,6 +39,7 @@ async function makeHarness(opts?: { trusted?: boolean; token?: string; generationGuard?: { assertOpen(): void }; + workspaceName?: string; }): Promise { const scratch = await fsp.mkdtemp( path.join( @@ -42,7 +47,7 @@ async function makeHarness(opts?: { `qwen-write-routes-${randomBytes(4).toString('hex')}-`, ), ); - const wsDir = path.join(scratch, 'ws'); + const wsDir = path.join(scratch, opts?.workspaceName ?? 'ws'); await fsp.mkdir(wsDir); const workspace = canonicalizeWorkspace(wsDir); const events: BridgeEvent[] = []; @@ -72,6 +77,82 @@ function rawHash(data: string | Buffer): `sha256:${string}` { return `sha256:${createHash('sha256').update(data).digest('hex')}`; } +/** + * Upload `totalBytes` with chunked transfer encoding, i.e. without a + * Content-Length header. The admission pre-check cannot see the size, so the + * request reaches the concurrency gate and the raw parser — pinning their + * order and the pre-handler slot release on parser rejection. + */ +async function sendChunkedUpload( + app: ReturnType, + targetPath: string, + totalBytes: number, +): Promise<{ status: number; body: string }> { + const server = app.listen(0, '127.0.0.1'); + await new Promise((resolve) => server.once('listening', resolve)); + const port = (server.address() as AddressInfo).port; + try { + return await new Promise<{ status: number; body: string }>( + (resolvePromise) => { + let settled = false; + const settle = (status: number, body: string) => { + if (!settled) { + settled = true; + resolvePromise({ status, body }); + } + }; + const req = http.request( + { + host: '127.0.0.1', + port, + method: 'POST', + path: `/file/upload?path=${encodeURIComponent(targetPath)}`, + headers: { + Host: loopbackHost(), + Authorization: 'Bearer secret', + 'Content-Type': 'application/octet-stream', + 'Transfer-Encoding': 'chunked', + }, + }, + (res) => { + let body = ''; + res.on('data', (chunk) => (body += String(chunk))); + res.on('end', () => settle(res.statusCode ?? 0, body)); + // The server rejects before draining the body; the socket can + // be cut before the response stream reports `end`. + res.on('close', () => settle(res.statusCode ?? 0, body)); + }, + ); + // The server may destroy the connection mid-body once it rejects. + // Settle even when the socket dies before any response: otherwise + // the promise never resolves and the test hangs into the suite + // timeout instead of failing fast on the status assertion. + req.on('error', () => settle(0, '')); + const chunk = Buffer.alloc(4 * 1024 * 1024, 1); + let remaining = totalBytes; + const writeNext = (): void => { + while (remaining > 0) { + const size = Math.min(chunk.length, remaining); + remaining -= size; + const ok = req.write( + size === chunk.length ? chunk : chunk.subarray(0, size), + ); + if (!ok) { + req.once('drain', writeNext); + return; + } + } + req.end(); + }; + writeNext(); + }, + ); + } finally { + server.closeAllConnections?.(); + await new Promise((resolve) => server.close(() => resolve())); + } +} + describe('POST /file/write', () => { let h: Harness; beforeEach(async () => { @@ -294,3 +375,1066 @@ describe('POST /file/edit', () => { expect(await fsp.readFile(outside, 'utf-8')).toBe('foo=1\n'); }); }); + +describe('POST /file/upload', () => { + let h: Harness; + beforeEach(async () => { + h = await makeHarness({ token: 'secret' }); + }); + afterEach(async () => teardown(h)); + + const upload = (pathParam: string) => + request(h.app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'application/octet-stream') + .query({ path: pathParam }); + + it('writes bytes atomically and returns the confirmed path, size, hash', async () => { + const data = randomBytes(256); + const res = await upload('blob.bin').send(data); + expect(res.status).toBe(201); + expect(res.headers['cache-control']).toBe('no-store'); + expect(res.headers['x-content-type-options']).toBe('nosniff'); + expect(res.body).toMatchObject({ + kind: 'file_upload', + path: 'blob.bin', + sizeBytes: data.length, + hash: rawHash(data), + }); + expect(res.body).not.toHaveProperty('renamed'); + expect(await fsp.readFile(path.join(h.workspace, 'blob.bin'))).toEqual( + data, + ); + }); + + it('accepts a zero-byte octet-stream upload', async () => { + const res = await upload('empty.bin').send(Buffer.alloc(0)); + expect(res.status).toBe(201); + expect(res.body).toMatchObject({ + kind: 'file_upload', + path: 'empty.bin', + sizeBytes: 0, + hash: rawHash(Buffer.alloc(0)), + }); + // The empty file must actually materialize on disk. + await expect( + fsp.readFile(path.join(h.workspace, 'empty.bin')), + ).resolves.toEqual(Buffer.alloc(0)); + }); + + it('rejects a wrong Content-Type with 415 before buffering', async () => { + const res = await request(h.app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'text/plain') + .query({ path: 'a.txt' }) + .send('not binary'); + expect(res.status).toBe(415); + expect(res.body).toMatchObject({ + errorKind: 'unsupported_media_type', + status: 415, + }); + }); + + it('rejects a missing Content-Type with the same 415 envelope', async () => { + const res = await request(h.app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .query({ path: 'a.txt' }) + .send(Buffer.from('not binary')); + expect(res.status).toBe(415); + expect(res.body).toMatchObject({ + errorKind: 'unsupported_media_type', + status: 415, + }); + }); + + it('rejects a missing path with parse_error', async () => { + const res = await request(h.app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'application/octet-stream') + .send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); + + it('rejects an oversized declared Content-Length with the upload 413 envelope', async () => { + // Declare a Content-Length above the cap while sending a tiny body. The + // admission gate rejects on the header alone, before buffering, so no + // 50 MiB transfer (and no client EPIPE) is needed to exercise the path. + const res = await request(h.app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'application/octet-stream') + .set('Content-Length', String(50 * 1024 * 1024 + 1)) + .query({ path: 'big.bin' }) + .send(Buffer.from('x')); + expect(res.status).toBe(413); + expect(res.body).toMatchObject({ + errorKind: 'file_too_large', + status: 413, + maxBytes: 50 * 1024 * 1024, + }); + expect(res.body.error).not.toContain('10 MB'); + await expect( + fsp.stat(path.join(h.workspace, 'big.bin')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('numbers dotfiles as whole names (.env -> .env (1))', async () => { + await fsp.writeFile(path.join(h.workspace, '.env'), 'orig'); + const res = await upload('.env').send(Buffer.from('new')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('.env (1)'); + expect(await fsp.readFile(path.join(h.workspace, '.env'), 'utf-8')).toBe( + 'orig', + ); + }); + + it('auto-numbers when the requested name is occupied by a file', async () => { + await fsp.writeFile(path.join(h.workspace, 'report.pdf'), 'orig'); + const res = await upload('report.pdf').send(Buffer.from('new')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('report (1).pdf'); + expect( + await fsp.readFile(path.join(h.workspace, 'report.pdf'), 'utf-8'), + ).toBe('orig'); + expect( + await fsp.readFile(path.join(h.workspace, 'report (1).pdf'), 'utf-8'), + ).toBe('new'); + }); + + it('auto-numbers past several taken candidates', async () => { + await fsp.writeFile(path.join(h.workspace, 'a.txt'), '0'); + await fsp.writeFile(path.join(h.workspace, 'a (1).txt'), '1'); + const res = await upload('a.txt').send(Buffer.from('2')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('a (2).txt'); + }); + + it('lands concurrent same-name uploads on distinct candidates', async () => { + const [first, second] = await Promise.all([ + upload('race.bin').send(Buffer.from('one')), + upload('race.bin').send(Buffer.from('two')), + ]); + expect(first.status).toBe(201); + expect(second.status).toBe(201); + // The no-clobber create guarantees the two uploads never share a path. + expect(first.body.path).not.toBe(second.body.path); + expect(new Set([first.body.path, second.body.path])).toEqual( + new Set(['race.bin', 'race (1).bin']), + ); + // Each response path holds exactly one of the two bodies — a + // misrouted write (same bytes twice, or an empty file) fails here. + const contents = new Set( + await Promise.all( + [first.body.path, second.body.path].map((p) => + fsp.readFile(path.join(h.workspace, p), 'utf-8'), + ), + ), + ); + expect(contents).toEqual(new Set(['one', 'two'])); + }); + + it('numbers a name occupied by a directory', async () => { + await fsp.mkdir(path.join(h.workspace, 'data')); + const res = await upload('data').send(Buffer.from('x')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('data (1)'); + }); + + it('numbers instead of writing through an in-workspace symlink', async () => { + await fsp.writeFile(path.join(h.workspace, 'real.bin'), 'orig'); + await fsp.symlink( + path.join(h.workspace, 'real.bin'), + path.join(h.workspace, 'link.bin'), + ); + const res = await upload('link.bin').send(Buffer.from('new')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('link (1).bin'); + // The symlink target is untouched and no file was written through it. + expect( + await fsp.readFile(path.join(h.workspace, 'real.bin'), 'utf-8'), + ).toBe('orig'); + }); + + it('numbers instead of materializing a symlink whose target is absent', async () => { + await fsp.symlink( + path.join(h.workspace, 'fresh.bin'), + path.join(h.workspace, 'link.bin'), + ); + const res = await upload('link.bin').send(Buffer.from('new')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('link (1).bin'); + // The dangling symlink is untouched and its target was not created. + expect(await fsp.readlink(path.join(h.workspace, 'link.bin'))).toBe( + path.join(h.workspace, 'fresh.bin'), + ); + await expect( + fsp.stat(path.join(h.workspace, 'fresh.bin')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('rejects an escaping symlink at the boundary', async () => { + const outside = path.join(h.scratch, 'outside.bin'); + await fsp.writeFile(outside, 'external'); + await fsp.symlink(outside, path.join(h.workspace, 'evil.bin')); + const res = await upload('evil.bin').send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('symlink_escape'); + expect(await fsp.readFile(outside, 'utf-8')).toBe('external'); + }); + + it('rejects a missing parent directory before buffering', async () => { + const res = await upload('no/such/dir/a.txt').send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); + + it('rejects a non-directory parent before buffering', async () => { + await fsp.writeFile(path.join(h.workspace, 'file.txt'), 'x'); + const res = await upload('file.txt/a.txt').send(Buffer.from('y')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + expect( + await fsp.readFile(path.join(h.workspace, 'file.txt'), 'utf-8'), + ).toBe('x'); + }); + + it('rejects a ../ boundary escape before buffering', async () => { + const res = await upload('../escape.txt').send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('path_outside_workspace'); + await expect( + fsp.stat(path.join(h.scratch, 'escape.txt')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it.each(['report.', 'report ', 'CON.txt'])( + 'rejects the suspicious basename %s before buffering', + async (name) => { + // `docs/` does not exist: rejection must come from the basename + // pre-check, before parent resolution, gate slots, and body buffering. + // Both branches answer with the same parse_error envelope, so only the + // admission-specific message pins which check rejected. + const res = await upload(`docs/${name}`).send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + expect(res.body.error).toContain('suspicious pattern'); + await expect( + fsp.stat(path.join(h.workspace, name)), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }, + ); + + it('rejects a symlinked parent that escapes the workspace', async () => { + const outsideDir = path.join(h.scratch, 'outside-dir'); + await fsp.mkdir(outsideDir); + await fsp.symlink(outsideDir, path.join(h.workspace, 'escape-dir'), 'dir'); + const res = await upload('escape-dir/a.txt').send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('symlink_escape'); + await expect( + fsp.stat(path.join(outsideDir, 'a.txt')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('preserves a generation-closed error from the parent stat', async () => { + // Throw from an instrumented stat (same Proxy technique as the + // disconnect test) so the pin is stage-aware: it fails if the error no + // longer originates at the admission parent stat. + const realFactory = createWorkspaceFileSystemFactory({ + boundWorkspaces: [h.workspace], + trusted: true, + emit: () => {}, + }); + const statFactory = { + assertCanWrite: () => {}, + forRequest: (ctx: { originatorClientId?: string; route: string }) => { + const realFs = realFactory.forRequest(ctx); + return new Proxy(realFs, { + get(target, prop, receiver) { + if (prop === 'stat') { + return (_resolved: Parameters[0]) => { + throw Object.assign(new Error('closed'), { + code: 'workspace_generation_closed', + }); + }; + } + const value = Reflect.get(target, prop, receiver); + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + }, + }; + const app = createServeApp( + { ...baseOpts, workspace: h.workspace, token: 'secret' }, + undefined, + { fsFactory: statFactory as never }, + ); + const res = await request(app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'application/octet-stream') + .query({ path: 'a.txt' }) + .send(Buffer.from('x')); + expect(res.status).toBe(503); + expect(res.headers['retry-after']).toBe('1'); + expect(res.body.code).toBe('workspace_runtime_unavailable'); + }); + + it('uploads into an existing subdirectory', async () => { + await fsp.mkdir(path.join(h.workspace, 'sub')); + // A literal forward-slash path: browsers always send POSIX separators; + // path.join would emit a backslash on the Windows gate. + const res = await upload('sub/file.txt').send(Buffer.from('hi')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('sub/file.txt'); + expect( + await fsp.readFile(path.join(h.workspace, 'sub', 'file.txt'), 'utf-8'), + ).toBe('hi'); + }); + + it('uploads into a workspace whose root path trips the suspicious-pattern check', async () => { + // A canonical workspace root containing a trailing-dot segment is legal + // on POSIX but matches hasSuspiciousPathPattern. Candidates must be + // re-resolved from the workspace-relative admission dir, not from the + // absolute root, or every upload fails with 'suspicious pattern' while + // /file/write on the same workspace works normally. + await teardown(h); + h = await makeHarness({ token: 'secret', workspaceName: 'my proj.' }); + const first = await upload('report.txt').send(Buffer.from('hi')); + expect(first.status).toBe(201); + expect(first.body.path).toBe('report.txt'); + expect( + await fsp.readFile(path.join(h.workspace, 'report.txt'), 'utf-8'), + ).toBe('hi'); + // Numbered candidates take the same re-resolution path. + const second = await upload('report.txt').send(Buffer.from('v2')); + expect(second.status).toBe(201); + expect(second.body.path).toBe('report (1).txt'); + }); + + it('handles filenames with spaces, non-ASCII, and literal % and #', async () => { + // `#` travels as %23 and must survive exactly one server-side decode; + // a double decode or raw-query parse would corrupt the name. + const name = 'my 数据 %b #1.txt'; + const res = await upload(name).send(Buffer.from('v')); + expect(res.status).toBe(201); + expect(res.body.path).toBe(name); + expect(await fsp.readFile(path.join(h.workspace, name), 'utf-8')).toBe('v'); + }); + + it('rejects a requested basename over 255 UTF-8 bytes', async () => { + const longName = 'あ'.repeat(100) + '.txt'; // 100*3 + 4 = 304 bytes + const res = await upload(longName).send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); + + it('accepts a 255-byte basename and rejects a 256-byte one', async () => { + const atCap = 'a'.repeat(251) + '.txt'; // exactly 255 bytes + expect(Buffer.byteLength(atCap, 'utf-8')).toBe(255); + const ok = await upload(atCap).send(Buffer.from('x')); + expect(ok.status).toBe(201); + expect(ok.body.path).toBe(atCap); + + const overCap = 'a'.repeat(252) + '.txt'; // 256 bytes + const bad = await upload(overCap).send(Buffer.from('x')); + expect(bad.status).toBe(400); + expect(bad.body.errorKind).toBe('parse_error'); + }); + + it('returns parse_error when numbering cannot fit the 255-byte cap', async () => { + // A 1-byte stem plus a 254-byte extension fills the whole cap, so no + // ' (n)' suffix fits and fitFilenameToByteCap's null branch must 400. + const name = `a.${'x'.repeat(253)}`; + expect(Buffer.byteLength(name, 'utf-8')).toBe(255); + await fsp.writeFile(path.join(h.workspace, name), 'orig'); + const res = await upload(name).send(Buffer.from('new')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); + + it('rejects an unknown client id before buffering', async () => { + const res = await request(h.app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'application/octet-stream') + .set('X-Qwen-Client-Id', 'not-a-real-client') + .query({ path: 'a.bin' }) + .send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_client_id'); + }); + + it('returns 403 untrusted_workspace on an untrusted workspace', async () => { + await teardown(h); + h = await makeHarness({ trusted: false, token: 'secret' }); + const res = await upload('a.bin').send(Buffer.from('x')); + expect(res.status).toBe(403); + expect(res.body.errorKind).toBe('untrusted_workspace'); + }); + + it('requires a token', async () => { + await teardown(h); + h = await makeHarness(); + const res = await request(h.app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Content-Type', 'application/octet-stream') + .query({ path: 'a.bin' }) + .send(Buffer.from('x')); + expect(res.status).toBe(401); + }); + + it('rejects "." and ".." basenames with parse_error', async () => { + const dot = await upload('.').send(Buffer.from('x')); + expect(dot.status).toBe(400); + expect(dot.body.errorKind).toBe('parse_error'); + const dotdot = await upload('sub/..').send(Buffer.from('x')); + expect(dotdot.status).toBe(400); + expect(dotdot.body.errorKind).toBe('parse_error'); + }); + + it.each(['assets/', 'sub/dir/'])( + 'rejects the directory-shaped trailing-slash path %s before buffering', + async (pathParam) => { + const res = await upload(pathParam).send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }, + ); + + it('rejects a trailing-slash path even when a same-named directory exists', async () => { + await fsp.mkdir(path.join(h.workspace, 'assets')); + const res = await upload('assets/').send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + // No auto-numbered FILE may appear beside the directory. + await expect( + fsp.stat(path.join(h.workspace, 'assets (1)')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it.each(['notes\u0000.txt', 'foo\u0000/x.txt'])( + 'rejects the NUL-bearing path with parse_error and no denied event', + async (pathParam) => { + const deniedBefore = h.events.filter( + (e) => e.type === 'fs.denied', + ).length; + const res = await upload(pathParam).send(Buffer.from('x')); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + expect(h.events.filter((e) => e.type === 'fs.denied')).toHaveLength( + deniedBefore, + ); + }, + ); + + it('rejects an encoded request body instead of silently decoding it', async () => { + const res = await request(h.app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'application/octet-stream') + .set('Content-Encoding', 'gzip') + .query({ path: 'gz.bin' }) + .send(gzipSync(Buffer.from('decoded payload'))); + expect(res.status).toBe(415); + expect(res.body.errorKind).toBe('unsupported_media_type'); + await expect( + fsp.stat(path.join(h.workspace, 'gz.bin')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('numbers past a symlink cycle occupying the requested name', async () => { + await fsp.symlink( + path.join(h.workspace, 'loop.txt'), + path.join(h.workspace, 'loop.txt'), + ); + const res = await upload('loop.txt').send(Buffer.from('x')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('loop (1).txt'); + expect( + await fsp.readFile(path.join(h.workspace, 'loop (1).txt'), 'utf-8'), + ).toBe('x'); + }); + + it('numbers past a two-hop symlink cycle occupying the requested name', async () => { + await fsp.symlink( + path.join(h.workspace, 'b.txt'), + path.join(h.workspace, 'a.txt'), + ); + await fsp.symlink( + path.join(h.workspace, 'a.txt'), + path.join(h.workspace, 'b.txt'), + ); + const res = await upload('a.txt').send(Buffer.from('x')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('a (1).txt'); + }); + + it('uploads through a symlinked directory whose target trips the pattern check', async () => { + // `aux` matches the DOS-device pattern; `docs -> aux` is a legal POSIX + // pair. Candidates re-resolve from the literal request dir, so the + // canonical `aux` segment never enters the pattern check. + await fsp.mkdir(path.join(h.workspace, 'aux')); + await fsp.symlink( + path.join(h.workspace, 'aux'), + path.join(h.workspace, 'docs'), + 'dir', + ); + const res = await upload('docs/report.txt').send(Buffer.from('hi')); + expect(res.status).toBe(201); + expect( + await fsp.readFile(path.join(h.workspace, 'aux', 'report.txt'), 'utf-8'), + ).toBe('hi'); + }); + + it('emits no fs.denied events for a successful auto-numbered upload', async () => { + await fsp.writeFile(path.join(h.workspace, 'shot.png'), '0'); + await fsp.writeFile(path.join(h.workspace, 'shot (1).png'), '1'); + await fsp.writeFile(path.join(h.workspace, 'shot (2).png'), '2'); + const deniedBefore = h.events.filter((e) => e.type === 'fs.denied').length; + const res = await upload('shot.png').send(Buffer.from('3')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('shot (3).png'); + expect(h.events.filter((e) => e.type === 'fs.denied')).toHaveLength( + deniedBefore, + ); + }); + + it('accepts an upload above the 5 MiB text cap (binary policy at HTTP)', async () => { + // 6 MiB > MAX_WRITE_BYTES (5 MiB) but <= MAX_UPLOAD_BYTES: proves the + // route applies the binary-ingress cap, not the text-write default. + const data = Buffer.alloc(6 * 1024 * 1024, 7); + const res = await upload('big.bin').send(data); + expect(res.status).toBe(201); + expect(res.body.sizeBytes).toBe(data.length); + expect((await fsp.stat(path.join(h.workspace, 'big.bin'))).size).toBe( + data.length, + ); + }); + + it('accepts an upload of exactly MAX_UPLOAD_BYTES', async () => { + // The inclusive acceptance boundary across all three size checks: + // admission Content-Length pre-check, express.raw limit, and the fs + // layer's enforceWriteSize all use strict `>`. + const data = Buffer.alloc(50 * 1024 * 1024, 3); + const res = await upload('exact.bin').send(data); + expect(res.status).toBe(201); + expect(res.body.sizeBytes).toBe(data.length); + expect((await fsp.stat(path.join(h.workspace, 'exact.bin'))).size).toBe( + data.length, + ); + }, 30_000); + + it('trims a long stem on numbering to stay within the 255-byte cap', async () => { + // 249-byte stem + '.txt' = 253-byte basename (passes the admission cap). + const stem = 'a'.repeat(249); + const name = `${stem}.txt`; + await fsp.writeFile(path.join(h.workspace, name), 'orig'); + const res = await upload(name).send(Buffer.from('new')); + expect(res.status).toBe(201); + const base = path.posix.basename(res.body.path); + // Numbering adds ' (1)' (4 bytes) -> 257, so the stem is trimmed by + // exactly two bytes (the minimal trim for 1-byte code points). + expect(Buffer.byteLength(base, 'utf-8')).toBe(255); + expect(base.endsWith(' (1).txt')).toBe(true); + expect(base).toBe(`${'a'.repeat(247)} (1).txt`); + // The trimmed name is the real on-disk name with the new content. + expect(await fsp.readFile(path.join(h.workspace, base), 'utf-8')).toBe( + 'new', + ); + // The original is untouched. + expect(await fsp.readFile(path.join(h.workspace, name), 'utf-8')).toBe( + 'orig', + ); + }); + + it('trims a multi-byte stem on a code-point boundary when numbering', async () => { + // 83 * 3 = 249-byte stem + '.txt' = 253 bytes (passes admission). + const stem = '数'.repeat(83); + const name = `${stem}.txt`; + await fsp.writeFile(path.join(h.workspace, name), 'orig'); + const res = await upload(name).send(Buffer.from('new')); + expect(res.status).toBe(201); + const base = path.posix.basename(res.body.path); + // 249 + 4 (' (1)') + 4 ('.txt') = 257 > 255, so one 3-byte code point + // is dropped: exactly 82 chars + suffix = 254 bytes, whole code points. + expect(base).toBe(`${'数'.repeat(82)} (1).txt`); + expect(Buffer.byteLength(base, 'utf-8')).toBe(254); + // The API response path is the exact on-disk name. + expect(await fsp.readFile(path.join(h.workspace, base), 'utf-8')).toBe( + 'new', + ); + }); + + it('lands on the 1000th candidate, then 409s when all are occupied', async () => { + // Occupy a.txt, a (1).txt ... a (998).txt: candidate 999 (the 1000th + // attempt) must still land — pinning the cap against shrinkage. + await Promise.all( + Array.from({ length: 999 }, (_, i) => + fsp.writeFile( + path.join(h.workspace, i === 0 ? 'a.txt' : `a (${i}).txt`), + 'x', + ), + ), + ); + const lands = await upload('a.txt').send(Buffer.from('y')); + expect(lands.status).toBe(201); + expect(lands.body.path).toBe('a (999).txt'); + + // With all 1000 candidates now occupied the route gives up with 409. + const res = await upload('a.txt').send(Buffer.from('z')); + expect(res.status).toBe(409); + expect(res.body.errorKind).toBe('file_already_exists'); + }, 30_000); + + it('publishes no file when the client disconnects during admission', async () => { + // Hold the admission parent-stat so the client can disconnect after its + // full body was sent: the body parser then skips parsing and continues + // with `req.body === undefined`. The handler must not coerce that to an + // empty Buffer and publish a phantom 0-byte file nobody requested. + const realFactory = createWorkspaceFileSystemFactory({ + boundWorkspaces: [h.workspace], + trusted: true, + emit: () => {}, + }); + let statCalls = 0; + let writeCalls = 0; + let releaseStat: () => void = () => {}; + const statHold = new Promise((resolve) => { + releaseStat = resolve; + }); + const hangingFactory = { + assertCanWrite: () => {}, + forRequest: (ctx: { originatorClientId?: string; route: string }) => { + const realFs = realFactory.forRequest(ctx); + return new Proxy(realFs, { + get(target, prop, receiver) { + if (prop === 'stat') { + return (p: Parameters[0]) => { + statCalls += 1; + return statCalls === 1 + ? statHold.then(() => target.stat(p)) + : target.stat(p); + }; + } + if (prop === 'writeBytesAtomic') { + return ( + p: Parameters[0], + data: Buffer, + ) => { + writeCalls += 1; + return target.writeBytesAtomic(p, data); + }; + } + const value = Reflect.get(target, prop, receiver); + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + }, + }; + const app = createServeApp( + { ...baseOpts, workspace: h.workspace, token: 'secret' }, + undefined, + { fsFactory: hangingFactory as never }, + ); + const server = app.listen(0, '127.0.0.1'); + await new Promise((resolve) => server.once('listening', resolve)); + const port = (server.address() as AddressInfo).port; + try { + let resolveDisconnect!: () => void; + const disconnected = new Promise((resolve) => { + resolveDisconnect = resolve; + }); + const req = http.request( + { + host: '127.0.0.1', + port, + method: 'POST', + path: '/file/upload?path=photo.jpg', + headers: { + Host: loopbackHost(), + Authorization: 'Bearer secret', + 'Content-Type': 'application/octet-stream', + 'Content-Length': '5', + }, + }, + (res) => { + res.resume(); + res.on('end', resolveDisconnect); + res.on('close', resolveDisconnect); + }, + ); + req.on('error', () => {}); + req.on('close', resolveDisconnect); + req.write('hello'); + req.end(); + // Cut the connection while admission is suspended on the held stat. + // Awaited so a timeout fails THIS test instead of escaping as an + // unhandled rejection attributed to whatever test runs next. + await vi.waitFor( + () => { + expect(statCalls).toBe(1); + }, + { timeout: 5000 }, + ); + req.destroy(); + // Give the server time to process the disconnect (req becomes + // aborted / res closed) before admission resumes. + await new Promise((r) => setTimeout(r, 100)); + releaseStat(); + await disconnected; + // Let the resumed pipeline reach the aborted-request guard and release + // its gate slot before the follow-ups probe the gate. + await new Promise((r) => setTimeout(r, 150)); + + // Four follow-up uploads through the same app: they succeed only if the + // aborted request released its gate slot, and they pin the write count + // (a phantom write for photo.jpg would make it five). + const followUp = (name: string) => + request(app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'application/octet-stream') + .query({ path: name }) + .send(Buffer.from('x')); + const after = await Promise.all([ + followUp('f.bin'), + followUp('g.bin'), + followUp('h.bin'), + followUp('i.bin'), + ]); + for (const res of after) { + expect(res.status).toBe(201); + } + expect(writeCalls).toBe(4); + await expect( + fsp.stat(path.join(h.workspace, 'photo.jpg')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + } finally { + server.closeAllConnections?.(); + await new Promise((resolve) => server.close(() => resolve())); + } + }); + + it('frees gate slots when clients disconnect mid body-buffering', async () => { + // A slow chunked body passes admission and holds its gate slot while the + // raw parser buffers. Destroying the socket before any response must + // free the slot via the gate's pre-handler `close` listener — without it + // four such disconnects saturate the process-global gate until restart. + const server = h.app.listen(0, '127.0.0.1'); + await new Promise((resolve) => server.once('listening', resolve)); + const port = (server.address() as AddressInfo).port; + const openSlowUpload = (name: string) => { + const req = http.request({ + host: '127.0.0.1', + port, + method: 'POST', + path: `/file/upload?path=${encodeURIComponent(name)}`, + headers: { + Host: loopbackHost(), + Authorization: 'Bearer secret', + 'Content-Type': 'application/octet-stream', + 'Transfer-Encoding': 'chunked', + }, + }); + req.on('error', () => {}); + // A single chunk with no end: the parser keeps buffering forever. + req.write(Buffer.from('x')); + return req; + }; + try { + // Five slow uploads for a four-slot gate: four hold slots while the + // fifth is rejected busy (no slot consumed). The spare keeps the + // saturation probe from squeezing a slow request out of a slot. + const slow = [ + openSlowUpload('slow-a.bin'), + openSlowUpload('slow-b.bin'), + openSlowUpload('slow-c.bin'), + openSlowUpload('slow-d.bin'), + openSlowUpload('slow-e.bin'), + ]; + + // Let every slow request finish admission and reach the gate before + // probing: an early probe would itself occupy the fourth slot and + // push a still-admitting slow request out of the gate. Probe until + // the next upload is busy. + await new Promise((r) => setTimeout(r, 500)); + await vi.waitFor( + async () => { + const res = await upload('probe.bin').send(Buffer.from('x')); + expect(res.status).toBe(429); + }, + { timeout: 10_000 }, + ); + + for (const req of slow) req.destroy(); + + // All four slots must come back from the pre-handler `close` release: + // a leaked slot would surface as a 429 in every retry of this burst. + await vi.waitFor( + async () => { + const after = await Promise.all([ + upload('f.bin').send(Buffer.from('x')), + upload('g.bin').send(Buffer.from('x')), + upload('h.bin').send(Buffer.from('x')), + upload('i.bin').send(Buffer.from('x')), + ]); + for (const res of after) { + expect(res.status).toBe(201); + } + }, + { timeout: 10_000 }, + ); + } finally { + server.closeAllConnections?.(); + await new Promise((resolve) => server.close(() => resolve())); + } + }, 30_000); + + it('frees gate slots when oversized chunked bodies are rejected after admission', async () => { + // A chunked body carries no Content-Length, so it passes the admission + // pre-check, acquires a gate slot, and is rejected by the raw parser. + // Five sequential rejections would exhaust the four-slot gate if the + // pre-handler finish/close release leaked on the parser-413 path. + for (let i = 0; i < 5; i++) { + const res = await sendChunkedUpload( + h.app, + `big-${i}.bin`, + 50 * 1024 * 1024 + 1, + ); + expect(res.status).toBe(413); + expect(JSON.parse(res.body)).toMatchObject({ + errorKind: 'file_too_large', + status: 413, + }); + } + const ok = await upload('small.bin').send(Buffer.from('x')); + expect(ok.status).toBe(201); + }, 60_000); +}); + +describe('upload concurrency gate', () => { + it('admits up to the cap and rejects the next until a slot frees', async () => { + const { createUploadConcurrencyGate } = await import( + './workspace-file-write.js' + ); + const gate = createUploadConcurrencyGate(2); + expect(gate.tryAcquire()).toBe(true); + expect(gate.tryAcquire()).toBe(true); + expect(gate.tryAcquire()).toBe(false); + gate.release(); + expect(gate.tryAcquire()).toBe(true); + // Release is idempotent and never goes negative: after over-releasing + // from empty, probing to the cap admits exactly `max` more acquires + // (an underflowed counter would admit more). + gate.release(); + gate.release(); + gate.release(); + expect(gate.tryAcquire()).toBe(true); + expect(gate.tryAcquire()).toBe(true); + expect(gate.tryAcquire()).toBe(false); + }); +}); + +describe('POST /file/upload HTTP concurrency gate (end-to-end)', () => { + it('holds a slot through a disconnected write, then frees it on completion', async () => { + const scratch = await fsp.mkdtemp( + path.join( + os.tmpdir(), + `qwen-upload-gate-${randomBytes(4).toString('hex')}-`, + ), + ); + const wsDir = path.join(scratch, 'ws'); + await fsp.mkdir(wsDir); + const workspace = canonicalizeWorkspace(wsDir); + const realFactory = createWorkspaceFileSystemFactory({ + boundWorkspaces: [workspace], + trusted: true, + emit: () => {}, + }); + // Hold every writeBytesAtomic until released, counting how many uploads + // have reached the write step (= have already acquired a gate slot). + let started = 0; + let release: () => void = () => {}; + const hold = new Promise((resolve) => { + release = resolve; + }); + const writePromises: Array> = []; + const hangingFactory = { + assertCanWrite: () => {}, + forRequest: (ctx: { originatorClientId?: string; route: string }) => { + const realFs = realFactory.forRequest(ctx); + // A Proxy (not a spread) so prototype methods like resolve/stat are + // preserved; only writeBytesAtomic is intercepted to hold the slot. + return new Proxy(realFs, { + get(target, prop, receiver) { + if (prop === 'writeBytesAtomic') { + return ( + p: Parameters[0], + data: Buffer, + ) => { + started += 1; + const write = hold.then(() => target.writeBytesAtomic(p, data)); + writePromises.push(write); + return write; + }; + } + const value = Reflect.get(target, prop, receiver); + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + }, + }; + const app = createServeApp( + { ...baseOpts, workspace, token: 'secret' }, + undefined, + { fsFactory: hangingFactory as never }, + ); + const upload = (name: string) => + request(app) + .post('/file/upload') + .set('Host', loopbackHost()) + .set('Authorization', 'Bearer secret') + .set('Content-Type', 'application/octet-stream') + .query({ path: name }) + .send(Buffer.from('x')); + + try { + const inFlight = [ + upload('a.bin'), + upload('b.bin'), + upload('c.bin'), + upload('d.bin'), + ]; + // Supertest Tests are lazy thenables — attach a catch to actually send + // each request without awaiting it (they hang until `release()`). + inFlight.forEach((p) => void p.catch(() => {})); + // Wait until all four have acquired a gate slot (reached the write step). + await vi.waitFor(() => { + expect(started).toBe(4); + }); + + // Disconnect one client after its full body has reached the held write. + // The server still owns that Buffer and write task, so the slot must not + // be released until writeBytesAtomic settles. + const firstSettled = inFlight[0].then( + () => undefined, + () => undefined, + ); + inFlight[0].abort(); + await firstSettled; + + const fifth = await upload('e.bin').timeout({ + response: 500, + deadline: 1_000, + }); + expect(fifth.status).toBe(429); + expect(fifth.body).toMatchObject({ + errorKind: 'upload_busy', + status: 429, + retryAfterSeconds: 1, + }); + expect(fifth.headers['retry-after']).toBe('1'); + + // Order probe: a chunked over-limit body carries no Content-Length, + // so admission cannot pre-check it. The saturated gate must reject it + // (429) before the parser buffers it; a parser-first middleware order + // would answer the upload-specific 413 instead. + const chunked = await sendChunkedUpload( + app, + 'chunked.bin', + 50 * 1024 * 1024 + 1, + ); + expect(chunked.status).toBe(429); + expect(JSON.parse(chunked.body)).toMatchObject({ + errorKind: 'upload_busy', + status: 429, + }); + + release(); + const results = await Promise.all(inFlight.slice(1)); + for (const res of results) { + expect(res.status).toBe(201); + } + + // The disconnected a.bin write still completed server-side. Await the + // intercepted write itself: the aborted client has no response to + // synchronize on, and nothing else orders its rename before this stat. + await Promise.all(writePromises); + expect((await fsp.stat(path.join(wsDir, 'a.bin'))).size).toBe(1); + + // All four slots must be free again: four concurrent uploads all + // succeed — exactly one leaked slot would surface as one 429 here. + const after = await Promise.all([ + upload('f.bin'), + upload('g.bin'), + upload('h.bin'), + upload('i.bin'), + ]); + for (const res of after) { + expect(res.status).toBe(201); + } + } finally { + release(); + await fsp.rm(scratch, { recursive: true, force: true }); + } + }); +}); + +describe('fileUploadBodyParser 413 (oversized buffered body)', () => { + it('maps a body-parser 413 to the upload-specific envelope', async () => { + // Exercises the raw-parser branch (not the admission Content-Length + // pre-check): a body that is actually larger than MAX_UPLOAD_BYTES is + // buffered and rejected by express.raw, and the wrapper converts that 413 + // into the upload-specific `file_too_large` envelope with `maxBytes`. + const { fileUploadBodyParser } = await import('./workspace-file-write.js'); + const app = express(); + app.post('/upload', fileUploadBodyParser(), (req, res) => { + res.status(200).json({ ok: true, size: (req.body as Buffer).length }); + }); + const oversized = Buffer.alloc(50 * 1024 * 1024 + 1); + const res = await request(app) + .post('/upload') + .set('Content-Type', 'application/octet-stream') + .send(oversized); + expect(res.status).toBe(413); + expect(res.body).toMatchObject({ + errorKind: 'file_too_large', + status: 413, + maxBytes: 50 * 1024 * 1024, + }); + }); + + it('passes non-413 body-parser errors through next(err)', async () => { + // An unsupported Content-Encoding makes body-parser throw a 415 + // `encoding.unsupported` — the wrapper must forward it via `next(err)` + // instead of misreporting an oversized body. + const { fileUploadBodyParser } = await import('./workspace-file-write.js'); + const app = express(); + app.post('/upload', fileUploadBodyParser(), (req, res) => { + res.status(200).json({ ok: true }); + }); + const res = await request(app) + .post('/upload') + .set('Content-Type', 'application/octet-stream') + .set('Content-Encoding', 'zstd') + .send(Buffer.from('x')); + expect(res.status).toBe(415); + expect(res.text).not.toContain('file_too_large'); + }); +}); diff --git a/packages/cli/src/serve/routes/workspace-file-write.ts b/packages/cli/src/serve/routes/workspace-file-write.ts index aba02c17bf..20b0225c00 100644 --- a/packages/cli/src/serve/routes/workspace-file-write.ts +++ b/packages/cli/src/serve/routes/workspace-file-write.ts @@ -4,11 +4,18 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as path from 'node:path'; import type { Application, Request, RequestHandler, Response } from 'express'; +import express from 'express'; import type { AcpSessionBridge } from '../acp-session-bridge.js'; import { + MAX_UPLOAD_BYTES, + hasSuspiciousPathPattern, isContentHash, + isFsError, type ContentHash, + type ResolvedPath, + type WorkspaceFileSystem, type WorkspaceFileSystemFactory, type WriteMode, } from '../fs/index.js'; @@ -357,3 +364,473 @@ export function registerWorkspaceQualifiedFileWriteRoutes( }, ); } + +// --------------------------------------------------------------------------- +// File upload (`POST /file/upload`) +// +// Binary ingress into the workspace. Uploads NEVER overwrite: an occupied +// name (file, directory, or in-workspace final-component symlink) is +// auto-numbered (`name (1).ext`, `name (2).ext`, ...). The fs layer only +// exposes a no-clobber byte create; the numbered-candidate policy lives here. +// --------------------------------------------------------------------------- + +const MAX_CONCURRENT_UPLOADS = 4; +const MAX_UPLOAD_FILENAME_BYTES = 255; +const NUMBERED_CANDIDATE_CAP = 1000; + +interface UploadGateLease { + handlerStarted: boolean; + release(): void; +} + +const uploadGateLeases = new WeakMap(); + +export interface UploadConcurrencyGate { + tryAcquire(): boolean; + release(): void; +} + +export function createUploadConcurrencyGate( + max: number = MAX_CONCURRENT_UPLOADS, +): UploadConcurrencyGate { + let active = 0; + return { + tryAcquire() { + if (active >= max) return false; + active += 1; + return true; + }, + release() { + if (active > 0) active -= 1; + }, + }; +} + +interface UploadAdmission { + route: string; + fs: WorkspaceFileSystem; + basename: string; + resolvedDir: ResolvedPath; + /** + * The literal request directory admission validated with + * `fs.resolve(dir, 'write')`. Candidates are re-resolved from THIS string, + * never from the canonicalized `resolvedDir`: resolution re-runs + * `hasSuspiciousPathPattern` on its whole input, so canonical segments the + * client never wrote — the workspace root's own path when it trips the + * pattern check, or a symlinked parent whose TARGET name does + * (`docs -> aux`) — would otherwise fail every upload into the directory. + */ + queryDir: string; +} + +const uploadAdmissions = new WeakMap(); + +function splitStemExtension(basename: string): { stem: string; ext: string } { + // A leading dot (`.env`) is part of the stem, not an extension separator. + const lastDot = basename.lastIndexOf('.'); + if (lastDot <= 0) return { stem: basename, ext: '' }; + return { stem: basename.slice(0, lastDot), ext: basename.slice(lastDot) }; +} + +/** + * Trim only the stem — on a Unicode code-point boundary — until + * `stem + suffix + ext` fits `capBytes`. Never trims the extension and never + * splits a UTF-8 sequence. Returns null when suffix + ext alone cannot fit. + */ +function fitFilenameToByteCap( + stem: string, + suffix: string, + ext: string, + capBytes: number, +): string | null { + if (Buffer.byteLength(suffix + ext, 'utf-8') > capBytes) return null; + let chars = Array.from(stem); + while (Buffer.byteLength(chars.join('') + suffix + ext, 'utf-8') > capBytes) { + if (chars.length === 0) return null; + chars = chars.slice(0, -1); + } + return chars.join('') + suffix + ext; +} + +function sendUploadTooLarge(res: Response): void { + applyReadHeaders(res); + res.status(413).json({ + errorKind: 'file_too_large', + error: `Request body too large (max ${MAX_UPLOAD_BYTES / (1024 * 1024)} MiB)`, + status: 413, + maxBytes: MAX_UPLOAD_BYTES, + }); +} + +function fileUploadConcurrencyGate( + gate: UploadConcurrencyGate, +): RequestHandler { + return (req, res, next) => { + if (!gate.tryAcquire()) { + applyReadHeaders(res); + res.status(429).set('Retry-After', '1').json({ + errorKind: 'upload_busy', + error: 'Too many uploads in progress', + status: 429, + retryAfterSeconds: 1, + }); + return; + } + let released = false; + const release = () => { + if (released) return; + released = true; + gate.release(); + res.off('finish', releaseBeforeHandler); + res.off('close', releaseBeforeHandler); + uploadGateLeases.delete(req); + }; + const releaseBeforeHandler = () => { + if (!lease.handlerStarted) release(); + }; + const lease: UploadGateLease = { handlerStarted: false, release }; + uploadGateLeases.set(req, lease); + res.once('finish', releaseBeforeHandler); + res.once('close', releaseBeforeHandler); + next(); + }; +} + +// `express.raw` rejects only when the buffered body EXCEEDS `limit`, so a body +// of exactly MAX_UPLOAD_BYTES passes and MAX_UPLOAD_BYTES+1 is rejected with +// the upload-specific 413 envelope below. Using MAX (not MAX+1) keeps both the +// parser and `writeBytesAtomic` on the same cap, so no body can slip past the +// parser and then surface a generic, `maxBytes`-less 413 from the fs layer. +export function fileUploadBodyParser(): RequestHandler { + const raw = express.raw({ + type: 'application/octet-stream', + limit: MAX_UPLOAD_BYTES, + // The endpoint contract is raw octets: decoding a Content-Encoding would + // publish bytes the client never sent (and hash/size of the decoded form). + inflate: false, + }); + return (req, res, next) => { + raw(req, res, (err?: unknown) => { + if (err) { + const status = (err as { status?: number }).status; + if (status === 413) { + sendUploadTooLarge(res); + return; + } + if (status === 415) { + applyReadHeaders(res); + res.status(415).json({ + errorKind: 'unsupported_media_type', + error: 'File uploads do not support encoded request bodies', + status: 415, + }); + return; + } + // A client aborting mid-body is routine for large uploads; the gate + // slot is already released by the pre-handler response-close + // listener, so the abort must not surface as an unhandled error. + if ((err as { code?: string }).code === 'ECONNABORTED' || req.aborted) { + return; + } + next(err); + return; + } + next(); + }); + }; +} + +function fileUploadAdmission( + deps: RegisterDeps & { workspaceRegistry?: WorkspaceRegistry }, + opts: { + qualified: boolean; + isWorkspaceTrusted?: () => boolean; + }, +): RequestHandler { + return (req, res, next) => { + void (async () => { + const ROUTE = opts.qualified + ? 'POST /workspaces/:workspace/file/upload' + : 'POST /file/upload'; + try { + if (opts.qualified) { + const registry = deps.workspaceRegistry; + if (!registry) { + throw new Error('workspace registry is not configured'); + } + const runtime = resolveWorkspaceRuntimeFromParam(registry, req, res); + if (!runtime) return; + if (!requireTrustedWorkspaceRuntime(runtime, res)) return; + setWorkspaceRouteContext(req, { + runtime, + routePrefix: 'POST /workspaces/:workspace', + }); + } else if (opts.isWorkspaceTrusted?.() === false) { + applyReadHeaders(res); + res.status(403).json({ + errorKind: 'untrusted_workspace', + error: 'workspace is not trusted; write operations are forbidden', + status: 403, + }); + return; + } + + const contentType = (req.headers['content-type'] ?? '') + .split(';')[0] + .trim() + .toLowerCase(); + if (contentType !== 'application/octet-stream') { + applyReadHeaders(res); + res.status(415).json({ + errorKind: 'unsupported_media_type', + error: 'File uploads require application/octet-stream', + status: 415, + }); + return; + } + + const queryPath = req.query['path']; + if (typeof queryPath !== 'string' || queryPath.length === 0) { + sendParseError(res, ROUTE, '`path` query parameter is required'); + return; + } + const dir = path.dirname(queryPath); + const basename = path.basename(queryPath); + // `path.dirname`/`path.basename` silently normalize a trailing slash, + // so a directory-shaped path must be rejected before they run. + if ( + queryPath.endsWith('/') || + basename.length === 0 || + basename === '.' || + basename === '..' + ) { + sendParseError(res, ROUTE, '`path` must name a file'); + return; + } + // Reject statically detectable bad names before taking a gate slot + // and buffering the body; fs.resolve would throw on them anyway. + if (queryPath.includes('\0')) { + sendParseError(res, ROUTE, 'path must not contain null bytes'); + return; + } + if (hasSuspiciousPathPattern(basename)) { + sendParseError(res, ROUTE, 'filename contains a suspicious pattern'); + return; + } + if (Buffer.byteLength(basename, 'utf-8') > MAX_UPLOAD_FILENAME_BYTES) { + sendParseError( + res, + ROUTE, + `filename exceeds ${MAX_UPLOAD_FILENAME_BYTES} bytes`, + ); + return; + } + + const contentLength = req.headers['content-length']; + if (contentLength !== undefined) { + const declared = Number(contentLength); + if (Number.isFinite(declared) && declared > MAX_UPLOAD_BYTES) { + sendUploadTooLarge(res); + return; + } + } + + const clientId = deps.parseClientId(req, res); + if (clientId === null) return; + const originatorClientId = resolveOriginatorClientId( + clientId, + deps, + res, + req, + ); + if (originatorClientId === null) return; + + const factory = getFsFactory(req, res); + if (!factory) return; + const fs = factory.forRequest({ + originatorClientId, + route: ROUTE, + }); + + const resolvedDir = await fs.resolve(dir, 'write'); + let dirStat; + try { + dirStat = await fs.stat(resolvedDir); + } catch (err) { + if (isFsError(err) && err.kind === 'path_not_found') { + sendParseError(res, ROUTE, 'parent directory does not exist'); + return; + } + throw err; + } + if (dirStat.kind !== 'directory') { + sendParseError(res, ROUTE, 'parent path is not a directory'); + return; + } + + uploadAdmissions.set(req, { + route: ROUTE, + fs, + basename, + resolvedDir, + queryDir: dir, + }); + next(); + } catch (err) { + sendFsError( + res, + err, + opts.qualified + ? 'POST /workspaces/:workspace/file/upload' + : 'POST /file/upload', + ); + } + })(); + }; +} + +async function handlePostFileUpload( + req: Request, + res: Response, +): Promise { + const admission = uploadAdmissions.get(req); + if (!admission) { + applyReadHeaders(res); + res.status(500).json({ + errorKind: 'internal_error', + error: 'upload admission context is missing', + status: 500, + }); + return; + } + const { route, fs, basename, resolvedDir, queryDir } = admission; + const { stem, ext } = splitStemExtension(basename); + const lease = uploadGateLeases.get(req); + if (lease) lease.handlerStarted = true; + try { + // A client disconnect during the async admission window lets the body + // parser continue with `req.body === undefined`; writing anyway would + // publish a phantom 0-byte file nobody requested. + if (req.aborted || res.closed) return; + const body = req.body; + const data = + body === undefined || body === null ? Buffer.alloc(0) : (body as Buffer); + for (let n = 0; n < NUMBERED_CANDIDATE_CAP; n++) { + const candidateBasename = + n === 0 + ? basename + : fitFilenameToByteCap( + stem, + ` (${n})`, + ext, + MAX_UPLOAD_FILENAME_BYTES, + ); + if (candidateBasename === null) { + sendParseError( + res, + route, + `filename cannot fit within ${MAX_UPLOAD_FILENAME_BYTES} bytes`, + ); + return; + } + const candidateAbs = path.join(resolvedDir as string, candidateBasename); + let resolved: ResolvedPath; + try { + resolved = await fs.resolve( + path.join(queryDir, candidateBasename), + 'write', + ); + } catch (err) { + // A symlink CYCLE occupying the candidate (ELOOP) is merely an + // occupied name — number on like the other occupied cases. Boundary + // escapes and other resolution failures stop the loop. + if ( + isFsError(err) && + err.kind === 'symlink_escape' && + (err.cause as NodeJS.ErrnoException | undefined)?.code === 'ELOOP' + ) { + continue; + } + sendFsError(res, err, route); + return; + } + if ((resolved as string) !== candidateAbs) { + // An in-workspace symlink already occupies this name; number on. + continue; + } + try { + const out = await fs.writeBytesAtomic(resolved, data); + applyReadHeaders(res); + res.status(201).json({ + kind: 'file_upload', + path: workspaceRelative(req, resolved as string), + sizeBytes: out.sizeBytes, + hash: out.hash, + }); + return; + } catch (err) { + if (isFsError(err) && err.kind === 'file_already_exists') { + continue; + } + sendFsError(res, err, route); + return; + } + } + applyReadHeaders(res); + res.status(409).json({ + errorKind: 'file_already_exists', + error: `could not allocate a free filename for "${basename}"`, + status: 409, + }); + } catch (err) { + sendFsError(res, err, route); + } finally { + uploadAdmissions.delete(req); + lease?.release(); + } +} + +export interface FileUploadLegacyDeps extends RegisterDeps { + uploadGate: UploadConcurrencyGate; + isWorkspaceTrusted?: () => boolean; +} + +export interface FileUploadQualifiedDeps extends RegisterDeps { + uploadGate: UploadConcurrencyGate; + workspaceRegistry: WorkspaceRegistry; +} + +export function registerWorkspaceFileUploadRoutes( + app: Application, + deps: FileUploadLegacyDeps, +): void { + app.post( + '/file/upload', + deps.mutate({ strict: true }), + fileUploadAdmission(deps, { + qualified: false, + isWorkspaceTrusted: deps.isWorkspaceTrusted, + }), + fileUploadConcurrencyGate(deps.uploadGate), + fileUploadBodyParser(), + (req, res) => { + void handlePostFileUpload(req, res); + }, + ); +} + +export function registerWorkspaceQualifiedFileUploadRoutes( + app: Application, + deps: FileUploadQualifiedDeps, +): void { + app.post( + '/workspaces/:workspace/file/upload', + deps.mutate({ strict: true }), + fileUploadAdmission(deps, { qualified: true }), + fileUploadConcurrencyGate(deps.uploadGate), + fileUploadBodyParser(), + (req, res) => { + void handlePostFileUpload(req, res); + }, + ); +} diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index b0b4609441..f06d884d28 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -468,6 +468,8 @@ const EXPECTED_STAGE1_FEATURES = [ 'workspace_file_bytes', 'workspace_file_read_cursor', 'workspace_file_write', + // Binary file upload (never overwrites; auto-numbers occupied names). + 'workspace_file_upload', // Mutation control routes (approval mode, workspace tool/skill toggles, // init scaffold, and MCP server restart). 'session_approval_mode_control', diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index f98f6d955d..5eef6f0325 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -98,8 +98,11 @@ import { registerWorkspaceQualifiedFileReadRoutes, } from './routes/workspace-file-read.js'; import { + createUploadConcurrencyGate, registerWorkspaceFileWriteRoutes, + registerWorkspaceFileUploadRoutes, registerWorkspaceQualifiedFileWriteRoutes, + registerWorkspaceQualifiedFileUploadRoutes, } from './routes/workspace-file-write.js'; import { registerWorkspaceSetupGithubRoutes } from './routes/workspace-setup-github.js'; import { @@ -2080,6 +2083,27 @@ export function createServeApp( safeBody, workspaceRegistry, }); + // One shared four-slot gate bounds upload-body buffering across both the + // legacy and workspace-qualified upload routes. + const uploadGate = createUploadConcurrencyGate(); + registerWorkspaceFileUploadRoutes(app, { + bridge: primaryBridge, + mutate, + parseClientId: parseClientIdHeader, + safeBody, + uploadGate, + ...(primaryRuntimeTrustAuthoritative + ? { isWorkspaceTrusted: isPrimaryWorkspaceTrusted } + : {}), + }); + registerWorkspaceQualifiedFileUploadRoutes(app, { + bridge: primaryBridge, + mutate, + parseClientId: parseClientIdHeader, + safeBody, + uploadGate, + workspaceRegistry, + }); registerWorkspaceSetupGithubRoutes(app, { boundWorkspace: primaryBoundWorkspace, bridge: primaryBridge, diff --git a/packages/cli/src/serve/server/telemetry.test.ts b/packages/cli/src/serve/server/telemetry.test.ts index 654d753885..2cd6608259 100644 --- a/packages/cli/src/serve/server/telemetry.test.ts +++ b/packages/cli/src/serve/server/telemetry.test.ts @@ -387,6 +387,24 @@ describe('daemonTelemetryMiddleware — recordRequest seam', () => { } }); + it('normalizes the plural workspace upload route to a stable route label', () => { + const mw = daemonTelemetryMiddleware(() => '/ws'); + const res = mockRes(200); + mw( + mockReq('POST', '/workspaces/ws-secondary/file/upload'), + res, + vi.fn() as unknown as NextFunction, + ); + res.emit('finish'); + expect(coreMocks.withDaemonRequestSpan).toHaveBeenLastCalledWith( + expect.objectContaining({ + method: 'POST', + route: 'POST /workspace/file/upload', + }), + expect.any(Function), + ); + }); + it('attributes plural workspace voice requests to the selected workspace', () => { const mw = daemonTelemetryMiddleware(() => '/workspace/secondary'); for (const [method, path, route] of [ diff --git a/packages/cli/src/serve/server/telemetry.ts b/packages/cli/src/serve/server/telemetry.ts index 4baece2e1f..6fa40f1044 100644 --- a/packages/cli/src/serve/server/telemetry.ts +++ b/packages/cli/src/serve/server/telemetry.ts @@ -556,6 +556,7 @@ export function resolveDaemonTelemetryRoute( suffix === '/workspace/reload' || suffix === '/workspace/file/write' || suffix === '/workspace/file/edit' || + suffix === '/workspace/file/upload' || suffix === '/workspace/mcp/servers' || suffix === '/workspace/memory' || suffix === '/workspace/agents' || diff --git a/packages/cli/src/serve/types.ts b/packages/cli/src/serve/types.ts index e655a6b902..fd1416bcab 100644 --- a/packages/cli/src/serve/types.ts +++ b/packages/cli/src/serve/types.ts @@ -464,6 +464,8 @@ export interface CapabilitiesEnvelope { maxSessionsPerWorkspace?: number | null; maxTotalSessions?: number | null; sessionRestoreTimeoutMs?: number; + /** Present when `workspace_file_upload` is advertised. */ + maxWorkspaceFileUploadBytes?: number; }; /** * Language codes accepted by `POST /session/:id/language`. diff --git a/packages/cli/src/serve/workspace-qualified-rest.test.ts b/packages/cli/src/serve/workspace-qualified-rest.test.ts index cd639449bf..0b7b373518 100644 --- a/packages/cli/src/serve/workspace-qualified-rest.test.ts +++ b/packages/cli/src/serve/workspace-qualified-rest.test.ts @@ -218,6 +218,7 @@ async function makeHarness(opts?: { secondaryDirName?: string; token?: string; persistSetting?: boolean; + primaryWriteHold?: { hold: Promise; onWriteStart?: () => void }; }) { const scratch = await fsp.mkdtemp( path.join(os.tmpdir(), 'qwen-workspace-qualified-rest-'), @@ -231,11 +232,40 @@ async function makeHarness(opts?: { await fsp.writeFile(path.join(primaryCwd, 'target.txt'), 'primary'); await fsp.writeFile(path.join(secondaryCwd, 'target.txt'), 'secondary'); - const primaryFsFactory = createWorkspaceFileSystemFactory({ + const primaryFsFactoryBase = createWorkspaceFileSystemFactory({ boundWorkspaces: [primaryCwd], trusted: true, emit: () => {}, }); + // A Proxy (not a spread) so prototype methods like resolve/stat are + // preserved; only writeBytesAtomic is intercepted to hold the gate slot. + const primaryFsFactory = opts?.primaryWriteHold + ? { + assertCanWrite: () => {}, + forRequest: ( + ctx: Parameters[0], + ) => { + const realFs = primaryFsFactoryBase.forRequest(ctx); + return new Proxy(realFs, { + get(target, prop, receiver) { + if (prop === 'writeBytesAtomic') { + return ( + p: Parameters[0], + data: Buffer, + ) => { + opts.primaryWriteHold?.onWriteStart?.(); + return opts.primaryWriteHold!.hold.then(() => + target.writeBytesAtomic(p, data), + ); + }; + } + const value = Reflect.get(target, prop, receiver); + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + }, + } + : primaryFsFactoryBase; const secondaryFsFactory = createWorkspaceFileSystemFactory({ boundWorkspaces: [secondaryCwd], trusted: true, @@ -799,6 +829,193 @@ describe('workspace-qualified core REST', () => { } }); + it('routes workspace-qualified file uploads and never falls back to primary', async () => { + const h = await makeHarness({ token: 'secret' }); + try { + const data = Buffer.from([1, 2, 3, 4]); + const res = await request(h.app) + .post(`/workspaces/${encodeURIComponent(h.secondaryId)}/file/upload`) + .set('Authorization', 'Bearer secret') + .set('Host', host()) + .set('Content-Type', 'application/octet-stream') + .query({ path: 'blob.bin' }) + .send(data); + expect(res.status).toBe(201); + expect(res.body.path).toBe('blob.bin'); + // Landed in the SECONDARY workspace, not the primary. + await expect( + fsp.readFile(path.join(h.secondaryCwd, 'blob.bin')), + ).resolves.toEqual(data); + await expect( + fsp.stat(path.join(h.primaryCwd, 'blob.bin')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + } finally { + await fsp.rm(h.scratch, { recursive: true, force: true }); + } + + // Untrusted secondary: 403, and nothing is written to primary either. + const untrusted = await makeHarness({ + secondaryTrusted: false, + token: 'secret', + }); + try { + const res = await request(untrusted.app) + .post( + `/workspaces/${encodeURIComponent(untrusted.secondaryId)}/file/upload`, + ) + .set('Authorization', 'Bearer secret') + .set('Host', host()) + .set('Content-Type', 'application/octet-stream') + .query({ path: 'blocked.bin' }) + .send(Buffer.from('x')); + expect(res.status).toBe(403); + expect(res.body.code).toBe('untrusted_workspace'); + await expect( + fsp.stat(path.join(untrusted.primaryCwd, 'blocked.bin')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + } finally { + await fsp.rm(untrusted.scratch, { recursive: true, force: true }); + } + }); + + it('uploads into a workspace whose root path trips the suspicious-pattern check', async () => { + // A canonical root with a trailing-dot segment matches + // hasSuspiciousPathPattern; candidates must re-resolve from the + // workspace-relative admission dir rather than the absolute root. + const h = await makeHarness({ + token: 'secret', + secondaryDirName: 'my proj.', + }); + try { + const res = await request(h.app) + .post(`/workspaces/${encodeURIComponent(h.secondaryId)}/file/upload`) + .set('Authorization', 'Bearer secret') + .set('Host', host()) + .set('Content-Type', 'application/octet-stream') + .query({ path: 'report.txt' }) + .send(Buffer.from('hi')); + expect(res.status).toBe(201); + expect(res.body.path).toBe('report.txt'); + await expect( + fsp.readFile(path.join(h.secondaryCwd, 'report.txt'), 'utf-8'), + ).resolves.toBe('hi'); + } finally { + await fsp.rm(h.scratch, { recursive: true, force: true }); + } + }); + + it('shares one upload gate between legacy and qualified routes', async () => { + let started = 0; + let releaseWrites: () => void = () => {}; + const hold = new Promise((resolve) => { + releaseWrites = resolve; + }); + const h = await makeHarness({ + token: 'secret', + primaryWriteHold: { hold, onWriteStart: () => (started += 1) }, + }); + try { + const legacyUpload = (name: string) => + request(h.app) + .post('/file/upload') + .set('Authorization', 'Bearer secret') + .set('Host', host()) + .set('Content-Type', 'application/octet-stream') + .query({ path: name }) + .send(Buffer.from('x')); + + const inFlight = [ + legacyUpload('a.bin'), + legacyUpload('b.bin'), + legacyUpload('c.bin'), + legacyUpload('d.bin'), + ]; + // Supertest Tests are lazy thenables — attach a catch to actually + // send each request without awaiting it (they hang in the held write). + inFlight.forEach((p) => void p.catch(() => {})); + await vi.waitFor(() => { + expect(started).toBe(4); + }); + + // All four gate slots are held through the LEGACY route; a qualified + // upload must draw from the same shared gate and be turned away. + const qualified = await request(h.app) + .post(`/workspaces/${encodeURIComponent(h.secondaryId)}/file/upload`) + .set('Authorization', 'Bearer secret') + .set('Host', host()) + .set('Content-Type', 'application/octet-stream') + .query({ path: 'q.bin' }) + .send(Buffer.from('x')); + expect(qualified.status).toBe(429); + expect(qualified.body).toMatchObject({ + errorKind: 'upload_busy', + status: 429, + }); + + releaseWrites(); + const results = await Promise.all(inFlight); + for (const res of results) { + expect(res.status).toBe(201); + } + } finally { + releaseWrites(); + await fsp.rm(h.scratch, { recursive: true, force: true }); + } + }); + + it('rejects qualified uploads for unknown, draining, and already removed workspaces', async () => { + const h = await makeHarness({ token: 'secret' }); + try { + const unknown = await request(h.app) + .post('/workspaces/does-not-exist/file/upload') + .set('Authorization', 'Bearer secret') + .set('Host', host()) + .set('Content-Type', 'application/octet-stream') + .query({ path: 'a.bin' }) + .send(Buffer.from('x')); + expect(unknown.status).toBe(400); + expect(unknown.body.code).toBe('workspace_mismatch'); + await expect( + fsp.stat(path.join(h.primaryCwd, 'a.bin')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + + const secondaryRuntime = h.workspaceRegistry.getByWorkspaceId( + h.secondaryId, + ); + expect(secondaryRuntime).toBeDefined(); + expect(h.workspaceRegistry.beginDrain(secondaryRuntime!)).toBe(true); + + const draining = await request(h.app) + .post(`/workspaces/${encodeURIComponent(h.secondaryId)}/file/upload`) + .set('Authorization', 'Bearer secret') + .set('Host', host()) + .set('Content-Type', 'application/octet-stream') + .query({ path: 'b.bin' }) + .send(Buffer.from('x')); + expect(draining.status).toBe(503); + expect(draining.body.code).toBe('workspace_runtime_unavailable'); + await expect( + fsp.stat(path.join(h.primaryCwd, 'b.bin')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + + h.workspaceRegistry.completeDrain(secondaryRuntime!); + const removed = await request(h.app) + .post(`/workspaces/${encodeURIComponent(h.secondaryId)}/file/upload`) + .set('Authorization', 'Bearer secret') + .set('Host', host()) + .set('Content-Type', 'application/octet-stream') + .query({ path: 'c.bin' }) + .send(Buffer.from('x')); + expect(removed.status).toBe(400); + expect(removed.body.code).toBe('workspace_mismatch'); + await expect( + fsp.stat(path.join(h.primaryCwd, 'c.bin')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + } finally { + await fsp.rm(h.scratch, { recursive: true, force: true }); + } + }); + it('routes workspace-qualified lifecycle mutations and trust-gates them', async () => { const h = await makeHarness({ token: 'secret' }); try { diff --git a/packages/cli/src/ui/hooks/use-effort-command.ts b/packages/cli/src/ui/hooks/use-effort-command.ts index 403168ea57..d2ea5fad59 100644 --- a/packages/cli/src/ui/hooks/use-effort-command.ts +++ b/packages/cli/src/ui/hooks/use-effort-command.ts @@ -50,11 +50,10 @@ export const useEffortCommand = ( // for future sessions, but say it won't take effect until thinking is // re-enabled. if (addItem) { - const feedbackItem: HistoryItemWithoutId & Record = - { - type: MessageType.INFO, - text: formatEffortChangeMessage(config, effort), - }; + const feedbackItem: HistoryItemWithoutId & Record = { + type: MessageType.INFO, + text: formatEffortChangeMessage(config, effort), + }; addItem(feedbackItem, Date.now()); config.getChatRecordingService?.()?.recordSlashCommand({ phase: 'result', diff --git a/packages/sdk-typescript/scripts/build.js b/packages/sdk-typescript/scripts/build.js index 2d2b3cd4b2..c75a4e3a65 100755 --- a/packages/sdk-typescript/scripts/build.js +++ b/packages/sdk-typescript/scripts/build.js @@ -81,7 +81,11 @@ const rootDir = join(__dirname, '..'); // Bumped from 184KB to 185KB for the Live Voice lifecycle helpers on both // daemon client classes. // Bumped from 185KB to 186KB for daemon-owned mid-turn message APIs. -const MAX_DAEMON_BROWSER_BUNDLE_BYTES = 186 * 1024; +// Bumped from 186KB to 188KB for the workspace file-upload surface +// (`uploadWorkspaceFile` + XHR progress) on both daemon client classes. +// Bumped from 188KB to 189KB for the session reasoning-effort config option +// APIs merged in from main. +const MAX_DAEMON_BROWSER_BUNDLE_BYTES = 189 * 1024; // The opt-in `daemon/transports` browser bundle legitimately ships the concrete // ACP transports (AcpHttpTransport/AcpWsTransport/AutoReconnect + negotiate), so // it's larger than the default barrel — but still budgeted so a future PR can't diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 53c50db98a..32f8a6d0e9 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -73,6 +73,8 @@ import type { DaemonWorkspaceFileBytes, DaemonWorkspaceFileEditRequest, DaemonWorkspaceFileEditResult, + DaemonWorkspaceFileUploadRequest, + DaemonWorkspaceFileUploadResult, DaemonWorkspaceFileWriteRequest, DaemonWorkspaceFileWriteResult, DaemonWorkspaceAgentDetail, @@ -1901,6 +1903,177 @@ export class DaemonClient { ); } + /** + * Upload binary bytes to the workspace. Shared raw-POST core used by both + * the legacy-primary `uploadWorkspaceFile` and the workspace-qualified + * variant, parameterized by URL path + route label. Keeps auth headers, + * timeout/abort composition, progress transport, and `DaemonHttpError` + * construction in one place. + * + * Uses `XMLHttpRequest` when `req.onProgress` is provided (`fetch` exposes + * no upload progress); plain `fetch` otherwise. Progress is browser-only: + * requesting it where `XMLHttpRequest` is unavailable fails before sending. + * + * @internal + */ + async uploadFileToPath( + uploadPath: string, + label: string, + req: DaemonWorkspaceFileUploadRequest, + clientId?: string, + ): Promise { + const target = new URL(`${this.baseUrl}${uploadPath}`); + target.searchParams.set('path', req.path); + const url = target.toString(); + const headers = this.headers( + { 'Content-Type': 'application/octet-stream' }, + clientId, + ); + if (req.onProgress) { + return await this.uploadWithProgress(url, label, req, headers); + } + return await this.fetchWithTimeout( + url, + { + method: 'POST', + headers, + body: req.data, + ...(req.signal ? { signal: req.signal } : {}), + }, + async (res) => { + if (!res.ok) throw await this.failOnError(res, label); + const text = await res.text(); + let body: unknown; + try { + body = text ? JSON.parse(text) : undefined; + } catch { + body = text; + } + // Match the XHR path's parse-then-shape-check so the two transports + // fail identically on malformed 2xx bodies. + if (!body || typeof body !== 'object' || !('path' in body)) { + throw new Error(`${label}: invalid upload response body`); + } + return body as DaemonWorkspaceFileUploadResult; + }, + req.timeoutMs, + 'rest', + ); + } + + private async uploadWithProgress( + url: string, + label: string, + req: DaemonWorkspaceFileUploadRequest, + headers: Record, + ): Promise { + if (typeof XMLHttpRequest === 'undefined') { + throw new Error( + `${label}: upload progress requires XMLHttpRequest (browser only)`, + ); + } + let effectiveTimeoutMs = this.fetchTimeoutMs; + if ( + req.timeoutMs !== undefined && + Number.isFinite(req.timeoutMs) && + req.timeoutMs >= 0 + ) { + effectiveTimeoutMs = req.timeoutMs; + } + const onProgress = req.onProgress; + return await new Promise( + (resolve, reject) => { + if (req.signal?.aborted) { + reject( + req.signal.reason ?? + new DOMException('The operation was aborted.', 'AbortError'), + ); + return; + } + const xhr = new XMLHttpRequest(); + let abortListener: (() => void) | undefined; + // Detach the abort listener once the request settles so a long-lived + // signal does not retain a reference to this XHR after completion. + const cleanup = () => { + if (abortListener && req.signal) { + req.signal.removeEventListener('abort', abortListener); + } + }; + xhr.open('POST', url); + for (const [name, value] of Object.entries(headers)) { + xhr.setRequestHeader(name, value); + } + if (effectiveTimeoutMs > 0) xhr.timeout = effectiveTimeoutMs; + xhr.upload.onprogress = (event) => { + if (event.lengthComputable && onProgress) { + onProgress({ loaded: event.loaded, total: event.total }); + } + }; + xhr.onload = () => { + cleanup(); + let body: unknown; + try { + body = xhr.responseText ? JSON.parse(xhr.responseText) : undefined; + } catch { + body = xhr.responseText; + } + if (xhr.status >= 200 && xhr.status < 300) { + // The fetch path rejects non-JSON 2xx bodies (`res.json()` + // throws); match it so the two transports fail identically. + if (!body || typeof body !== 'object' || !('path' in body)) { + reject(new Error(`${label}: invalid upload response body`)); + return; + } + resolve(body as DaemonWorkspaceFileUploadResult); + return; + } + const detail = + body && typeof body === 'object' && 'error' in body + ? String((body as { error: unknown }).error) + : `HTTP ${xhr.status}`; + reject(new DaemonHttpError(xhr.status, body, `${label}: ${detail}`)); + }; + xhr.onerror = () => { + cleanup(); + reject(new Error(`${label}: network request failed`)); + }; + xhr.ontimeout = () => { + cleanup(); + reject(new DOMException('timeout', 'TimeoutError')); + }; + xhr.onabort = () => { + cleanup(); + reject( + req.signal?.reason ?? + new DOMException('The operation was aborted.', 'AbortError'), + ); + }; + if (req.signal) { + abortListener = () => xhr.abort(); + req.signal.addEventListener('abort', abortListener, { once: true }); + } + try { + xhr.send(req.data as XMLHttpRequestBodyInit); + } catch (error) { + cleanup(); + reject(error); + } + }, + ); + } + + async uploadWorkspaceFile( + req: DaemonWorkspaceFileUploadRequest, + clientId?: string, + ): Promise { + return await this.uploadFileToPath( + '/file/upload', + 'POST /file/upload', + req, + clientId, + ); + } + // -- Workspace memory (workspace memory/agents) ------------------------------ /** @@ -5908,6 +6081,18 @@ export class WorkspaceDaemonClient { ); } + uploadWorkspaceFile( + req: DaemonWorkspaceFileUploadRequest, + clientId?: string, + ): Promise { + return this.client.uploadFileToPath( + `/workspaces/${this.workspaceSelector}/file/upload`, + 'POST /workspaces/:workspace/file/upload', + req, + clientId, + ); + } + workspaceSettings(opts?: { clientId?: string; }): Promise { diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index 9a06d0eb07..a8ab64b470 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -594,6 +594,8 @@ export type { DaemonWorkspaceFileBytes, DaemonWorkspaceFileEditRequest, DaemonWorkspaceFileEditResult, + DaemonWorkspaceFileUploadRequest, + DaemonWorkspaceFileUploadResult, DaemonWorkspaceFileWriteRequest, DaemonWorkspaceFileWriteResult, DaemonWorkspaceMcpServerStatus, diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index f3687ce52d..42977ca115 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -26,6 +26,8 @@ export interface DaemonCapabilitiesLimits { maxTotalSessions?: number | null; /** Server-side deadline for ACP session load/resume. */ sessionRestoreTimeoutMs?: number; + /** Present when `workspace_file_upload` is advertised. */ + maxWorkspaceFileUploadBytes?: number; } export interface DaemonWorkspaceCapability { @@ -1967,6 +1969,32 @@ export interface DaemonWorkspaceFileEditResult { matchedIgnore: 'file' | 'directory' | null; } +/** + * Binary file upload request. The bytes are sent as + * `application/octet-stream`; `path` is the target relative to the workspace + * root. Uploads never overwrite — an occupied name is auto-numbered by the + * daemon, and the returned `path` is the final server-confirmed name. + */ +export interface DaemonWorkspaceFileUploadRequest { + path: string; + data: ArrayBuffer | Uint8Array | Blob; + signal?: AbortSignal; + /** Omitted inherits the client's default; `0` disables the timeout. */ + timeoutMs?: number; + /** + * Browser-only upload progress. Requesting progress where + * `XMLHttpRequest` is unavailable throws before sending. + */ + onProgress?: (event: { loaded: number; total: number }) => void; +} + +export interface DaemonWorkspaceFileUploadResult { + kind: 'file_upload'; + path: string; + sizeBytes: number; + hash: DaemonContentHash; +} + /** * Subagent CRUD types. `agentType` on the wire is * the `name` field from the agent's frontmatter (case-insensitive); diff --git a/packages/sdk-typescript/src/index.ts b/packages/sdk-typescript/src/index.ts index dca7a42b46..93b707c064 100644 --- a/packages/sdk-typescript/src/index.ts +++ b/packages/sdk-typescript/src/index.ts @@ -253,6 +253,8 @@ export { type DaemonWorkspaceFileBytes, type DaemonWorkspaceFileEditRequest, type DaemonWorkspaceFileEditResult, + type DaemonWorkspaceFileUploadRequest, + type DaemonWorkspaceFileUploadResult, type DaemonWorkspaceFileWriteRequest, type DaemonWorkspaceFileWriteResult, type DaemonWorkspaceMemoryDreamOptions, diff --git a/packages/sdk-typescript/test/unit/DaemonClient.upload.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.upload.test.ts new file mode 100644 index 0000000000..537a7cb60b --- /dev/null +++ b/packages/sdk-typescript/test/unit/DaemonClient.upload.test.ts @@ -0,0 +1,722 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest'; +import { + DaemonClient, + DaemonHttpError, +} from '../../src/daemon/DaemonClient.js'; +import type { DaemonTransport } from '../../src/daemon/DaemonTransport.js'; + +function jsonResponse(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} + +interface CapturedRequest { + url: string; + method: string; + headers: Record; + body: unknown; + signal?: AbortSignal | null; +} + +function recordingFetch( + reply: (req: CapturedRequest) => Response | Promise, +): { fetch: typeof globalThis.fetch; calls: CapturedRequest[] } { + const calls: CapturedRequest[] = []; + const fetchImpl = vi.fn( + async (input: RequestInfo | URL, init?: RequestInit) => { + const url = + typeof input === 'string' + ? input + : input instanceof URL + ? input.toString() + : input.url; + const headers: Record = {}; + if (init?.headers) { + const h = new Headers(init.headers); + h.forEach((v, k) => (headers[k.toLowerCase()] = v)); + } + const captured: CapturedRequest = { + url, + method: init?.method ?? 'GET', + headers, + body: init?.body ?? null, + signal: init?.signal ?? null, + }; + calls.push(captured); + return reply(captured); + }, + ) as unknown as typeof globalThis.fetch; + return { fetch: fetchImpl, calls }; +} + +describe('uploadWorkspaceFile', () => { + const uploadResult = { + kind: 'file_upload', + path: 'blob.bin', + sizeBytes: 4, + hash: `sha256:${'d'.repeat(64)}`, + }; + + it('POSTs octet-stream bytes with the path in the query string', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect( + client.uploadWorkspaceFile( + { path: 'blob.bin', data: new Uint8Array([1, 2, 3, 4]) }, + 'client-1', + ), + ).resolves.toEqual(uploadResult); + expect(calls[0]?.method).toBe('POST'); + expect(calls[0]?.url).toBe('http://daemon/file/upload?path=blob.bin'); + expect(calls[0]?.headers['content-type']).toBe('application/octet-stream'); + expect(calls[0]?.headers['x-qwen-client-id']).toBe('client-1'); + }); + + it('uses direct REST when an ACP transport is configured', async () => { + const { fetch: restFetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + const transportFetch = vi.fn(async () => + jsonResponse(404, { error: 'ACP route not found' }), + ); + const transport: DaemonTransport = { + type: 'acp-http', + supportsReplay: true, + connected: true, + restFetch, + fetch: transportFetch, + async *subscribeEvents() {}, + dispose() {}, + }; + const client = new DaemonClient({ baseUrl: 'http://daemon', transport }); + + await expect( + client.uploadWorkspaceFile({ + path: 'blob.bin', + data: new Uint8Array([1]), + }), + ).resolves.toEqual(uploadResult); + expect(calls[0]?.url).toBe('http://daemon/file/upload?path=blob.bin'); + expect(transportFetch).not.toHaveBeenCalled(); + }); + + it('URL-encodes the path query parameter exactly once', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await client.uploadWorkspaceFile({ + path: 'a&b+c=d #1 数据 %b.txt', + data: new Uint8Array([0]), + }); + const url = new URL(calls[0]!.url); + expect(url.pathname).toBe('/file/upload'); + expect(url.searchParams.get('path')).toBe('a&b+c=d #1 数据 %b.txt'); + }); + + it('sends the raw bytes as the request body', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const data = new Uint8Array([7, 8, 9]); + await client.uploadWorkspaceFile({ path: 'a.bin', data }); + expect(calls[0]?.body).toBe(data); + }); + + it('sends Blob bodies untouched on the fetch path', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const data = new Blob(['blob-bytes']); + await client.uploadWorkspaceFile({ path: 'a.bin', data }); + expect(calls[0]?.body).toBe(data); + }); + + it('rejects a 2xx fetch response whose JSON body is missing path', async () => { + const { fetch } = recordingFetch(() => jsonResponse(200, {})); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect( + client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + }), + ).rejects.toThrow(/invalid upload response body/); + }); + + it('rejects a non-JSON 2xx fetch body with the same labeled error as XHR', async () => { + // Proxy/captive-portal interstitials answer 200 + HTML; both transports + // must reject with the labeled error (not a bare SyntaxError on fetch). + const { fetch } = recordingFetch( + () => new Response('interstitial', { status: 200 }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect( + client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + }), + ).rejects.toThrow(/invalid upload response body/); + }); + + it('uses the workspace-qualified route via workspaceByCwd', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await client + .workspaceByCwd('/repo') + .uploadWorkspaceFile({ path: 'a.bin', data: new Uint8Array([9]) }); + expect(calls[0]?.url).toBe( + 'http://daemon/workspaces/%2Frepo/file/upload?path=a.bin', + ); + }); + + it('preserves the upload 413 error body', async () => { + const body = { + errorKind: 'file_too_large', + error: 'Request body too large (max 50 MiB)', + status: 413, + maxBytes: 52428800, + }; + const { fetch } = recordingFetch(() => jsonResponse(413, body)); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const err = await client + .uploadWorkspaceFile({ path: 'big.bin', data: new Uint8Array(1) }) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(DaemonHttpError); + expect((err as DaemonHttpError).status).toBe(413); + expect((err as DaemonHttpError).body).toEqual(body); + }); + + it('fails before sending when progress is requested without XMLHttpRequest', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect( + client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + onProgress: () => {}, + }), + ).rejects.toThrow(/XMLHttpRequest/); + expect(calls).toHaveLength(0); + }); + + it('forwards the abort signal to the request', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + // `fetchTimeoutMs: 0` (the upload production config) skips the timeout + // signal composition, so the captured signal is exactly the caller's — + // a dropped `req.signal` would surface as `null` here. + const client = new DaemonClient({ + baseUrl: 'http://daemon', + fetch, + fetchTimeoutMs: 0, + }); + const ctrl = new AbortController(); + await client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + signal: ctrl.signal, + }); + expect(calls[0]?.signal).toBe(ctrl.signal); + }); + + it( + 'inherits the client timeout when timeoutMs is omitted', + // The mock fetch settles only via the abort signal; under the exact + // regression this test guards (no timeout armed, no signal composed) + // nothing would abort and the promise would hang into the package + // testTimeout. A per-test budget fails that shape fast. + { timeout: 5_000 }, + async () => { + vi.useFakeTimers(); + try { + const fetch = vi.fn( + async (_input: RequestInfo | URL, init?: RequestInit) => + await new Promise((_resolve, reject) => { + init?.signal?.addEventListener( + 'abort', + () => reject(init.signal?.reason), + { once: true }, + ); + }), + ) as unknown as typeof globalThis.fetch; + const client = new DaemonClient({ + baseUrl: 'http://daemon', + fetch, + fetchTimeoutMs: 25, + }); + const result = client + .uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + }) + .catch((error: unknown) => error); + + await vi.advanceTimersByTimeAsync(25); + await expect(result).resolves.toMatchObject({ name: 'TimeoutError' }); + } finally { + vi.useRealTimers(); + } + }, + ); + + it('allows timeoutMs 0 to disable the client timeout', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(201, uploadResult), + ); + const client = new DaemonClient({ + baseUrl: 'http://daemon', + fetch, + fetchTimeoutMs: 25, + }); + await client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + timeoutMs: 0, + }); + expect(calls[0]?.signal).toBeNull(); + }); + + it('applies an explicit timeout to progress uploads', async () => { + class FakeXMLHttpRequest { + static latest: FakeXMLHttpRequest | undefined; + timeout = 0; + status = 0; + responseText = ''; + upload = { onprogress: null as ((event: ProgressEvent) => void) | null }; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + ontimeout: (() => void) | null = null; + onabort: (() => void) | null = null; + + constructor() { + FakeXMLHttpRequest.latest = this; + } + + open = vi.fn(); + setRequestHeader = vi.fn(); + abort() { + this.onabort?.(); + } + send() { + this.ontimeout?.(); + } + } + + vi.stubGlobal('XMLHttpRequest', FakeXMLHttpRequest); + try { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + const error = await client + .uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + timeoutMs: 17, + onProgress: () => {}, + }) + .catch((caught: unknown) => caught); + + expect(FakeXMLHttpRequest.latest?.timeout).toBe(17); + expect(error).toMatchObject({ name: 'TimeoutError' }); + } finally { + vi.unstubAllGlobals(); + } + }); + + it('keeps the XHR timeout disabled for timeoutMs 0 despite the client default', async () => { + class FakeXMLHttpRequest { + static latest: FakeXMLHttpRequest | undefined; + timeout = 0; + status = 201; + responseText = JSON.stringify(uploadResult); + upload = { onprogress: null as ((event: ProgressEvent) => void) | null }; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + ontimeout: (() => void) | null = null; + onabort: (() => void) | null = null; + + constructor() { + FakeXMLHttpRequest.latest = this; + } + + open() {} + setRequestHeader() {} + abort() {} + send() { + this.onload?.(); + } + } + + vi.stubGlobal('XMLHttpRequest', FakeXMLHttpRequest); + try { + const client = new DaemonClient({ + baseUrl: 'http://daemon', + fetchTimeoutMs: 30_000, + }); + await client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + timeoutMs: 0, + onProgress: () => {}, + }); + expect(FakeXMLHttpRequest.latest?.timeout).toBe(0); + } finally { + vi.unstubAllGlobals(); + } + }); + + describe('progress uploads over XMLHttpRequest', () => { + class FakeXMLHttpRequest { + static latest: FakeXMLHttpRequest | undefined; + static sendHook: ((xhr: FakeXMLHttpRequest) => void) | undefined; + timeout = 0; + status = 0; + responseText = ''; + sentBody: unknown = undefined; + upload = { onprogress: null as ((event: ProgressEvent) => void) | null }; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + ontimeout: (() => void) | null = null; + onabort: (() => void) | null = null; + open = vi.fn(); + setRequestHeader = vi.fn(); + + constructor() { + FakeXMLHttpRequest.latest = this; + } + + abort() { + this.onabort?.(); + } + send(body?: unknown) { + this.sentBody = body; + FakeXMLHttpRequest.sendHook?.(this); + } + } + + beforeEach(() => { + FakeXMLHttpRequest.latest = undefined; + FakeXMLHttpRequest.sendHook = undefined; + vi.stubGlobal('XMLHttpRequest', FakeXMLHttpRequest); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('builds the request and maps upload progress events', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + const data = new Uint8Array([1, 2, 3]); + const progress: Array<{ loaded: number; total: number }> = []; + + const promise = client.uploadWorkspaceFile( + { + path: 'a.bin', + data, + onProgress: (event) => progress.push(event), + }, + 'client-1', + ); + const xhr = FakeXMLHttpRequest.latest!; + expect(xhr.open).toHaveBeenCalledWith( + 'POST', + 'http://daemon/file/upload?path=a.bin', + ); + expect(xhr.setRequestHeader).toHaveBeenCalledWith( + 'Content-Type', + 'application/octet-stream', + ); + expect(xhr.setRequestHeader).toHaveBeenCalledWith( + 'X-Qwen-Client-Id', + 'client-1', + ); + expect(xhr.sentBody).toBe(data); + + xhr.upload.onprogress?.({ + lengthComputable: true, + loaded: 2, + total: 4, + } as ProgressEvent); + expect(progress).toEqual([{ loaded: 2, total: 4 }]); + xhr.upload.onprogress?.({ + lengthComputable: false, + loaded: 9, + total: 0, + } as ProgressEvent); + expect(progress).toHaveLength(1); + + xhr.status = 201; + xhr.responseText = JSON.stringify(uploadResult); + xhr.onload?.(); + await expect(promise).resolves.toEqual(uploadResult); + }); + + it('rejects non-2xx responses with a parsed DaemonHttpError', async () => { + const body = { + errorKind: 'file_too_large', + error: 'Request body too large (max 50 MiB)', + status: 413, + maxBytes: 52428800, + }; + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + + const promise = client + .uploadWorkspaceFile({ + path: 'big.bin', + data: new Uint8Array([1]), + onProgress: () => {}, + }) + .catch((caught: unknown) => caught); + const xhr = FakeXMLHttpRequest.latest!; + xhr.status = 413; + xhr.responseText = JSON.stringify(body); + xhr.onload?.(); + + const error = await promise; + expect(error).toBeInstanceOf(DaemonHttpError); + expect((error as DaemonHttpError).status).toBe(413); + expect((error as DaemonHttpError).body).toEqual(body); + }); + + it('rejects network failures', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + + const promise = client + .uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + onProgress: () => {}, + }) + .catch((caught: unknown) => caught); + FakeXMLHttpRequest.latest!.onerror?.(); + + const error = await promise; + expect(error).toMatchObject({ + message: expect.stringContaining('network request failed'), + }); + }); + + it('aborts on signal cancellation and detaches the abort listener', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + const ctrl = new AbortController(); + const addSpy = vi.spyOn(ctrl.signal, 'addEventListener'); + const removeSpy = vi.spyOn(ctrl.signal, 'removeEventListener'); + + const promise = client + .uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + signal: ctrl.signal, + onProgress: () => {}, + }) + .catch((caught: unknown) => caught); + + ctrl.abort(); + await expect(promise).resolves.toMatchObject({ name: 'AbortError' }); + const registered = addSpy.mock.calls.find( + ([type]) => type === 'abort', + )?.[1]; + expect(registered).toBeTypeOf('function'); + expect(removeSpy).toHaveBeenCalledWith('abort', registered); + }); + + it('rejects and cleans up when xhr.send throws synchronously', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + FakeXMLHttpRequest.sendHook = () => { + throw new Error('detached buffer'); + }; + const ctrl = new AbortController(); + const addSpy = vi.spyOn(ctrl.signal, 'addEventListener'); + const removeSpy = vi.spyOn(ctrl.signal, 'removeEventListener'); + + const error = await client + .uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + signal: ctrl.signal, + onProgress: () => {}, + }) + .catch((caught: unknown) => caught); + + expect(error).toMatchObject({ message: 'detached buffer' }); + const registered = addSpy.mock.calls.find( + ([type]) => type === 'abort', + )?.[1]; + expect(registered).toBeTypeOf('function'); + expect(removeSpy).toHaveBeenCalledWith('abort', registered); + }); + + it('detaches the abort listener after a successful upload settles', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + const ctrl = new AbortController(); + const addSpy = vi.spyOn(ctrl.signal, 'addEventListener'); + const removeSpy = vi.spyOn(ctrl.signal, 'removeEventListener'); + + const promise = client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + signal: ctrl.signal, + onProgress: () => {}, + }); + const xhr = FakeXMLHttpRequest.latest!; + xhr.status = 201; + xhr.responseText = JSON.stringify(uploadResult); + xhr.onload?.(); + + await expect(promise).resolves.toEqual(uploadResult); + const registered = addSpy.mock.calls.find( + ([type]) => type === 'abort', + )?.[1]; + expect(registered).toBeTypeOf('function'); + expect(removeSpy).toHaveBeenCalledWith('abort', registered); + }); + + it('rejects a pre-aborted signal before constructing an XHR', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + const ctrl = new AbortController(); + ctrl.abort(); + + await expect( + client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + signal: ctrl.signal, + onProgress: () => {}, + }), + ).rejects.toMatchObject({ name: 'AbortError' }); + expect(FakeXMLHttpRequest.latest).toBeUndefined(); + }); + + it('propagates the caller abort reason on a pre-aborted signal', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + const sentinel = new Error('sentinel'); + const ctrl = new AbortController(); + ctrl.abort(sentinel); + + await expect( + client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + signal: ctrl.signal, + onProgress: () => {}, + }), + ).rejects.toBe(sentinel); + }); + + it('propagates the caller abort reason through xhr abort', async () => { + // Matches the fetch transport (AbortSignal.any carries the reason): + // callers keying on `err === reason` must see the same rejection + // whether or not progress reporting selected the XHR path. + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + const sentinel = new Error('sentinel'); + const ctrl = new AbortController(); + + const promise = client + .uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + signal: ctrl.signal, + onProgress: () => {}, + }) + .catch((caught: unknown) => caught); + expect(FakeXMLHttpRequest.latest).toBeDefined(); + + ctrl.abort(sentinel); + await expect(promise).resolves.toBe(sentinel); + }); + + it('rejects a 2xx response with a non-JSON body like the fetch path', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + + const promise = client + .uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + onProgress: () => {}, + }) + .catch((caught: unknown) => caught); + const xhr = FakeXMLHttpRequest.latest!; + xhr.status = 200; + xhr.responseText = 'interstitial'; + xhr.onload?.(); + + const error = await promise; + expect(error).toMatchObject({ + message: expect.stringContaining('invalid upload response body'), + }); + }); + + it('rejects a 2xx response whose JSON body is missing path', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + + const promise = client + .uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + onProgress: () => {}, + }) + .catch((caught: unknown) => caught); + const xhr = FakeXMLHttpRequest.latest!; + xhr.status = 200; + xhr.responseText = JSON.stringify({}); + xhr.onload?.(); + + const error = await promise; + expect(error).toMatchObject({ + message: expect.stringContaining('invalid upload response body'), + }); + }); + + it('sends Blob bodies untouched on the XHR path', async () => { + const client = new DaemonClient({ baseUrl: 'http://daemon' }); + const data = new Blob(['blob-bytes']); + + const promise = client.uploadWorkspaceFile({ + path: 'a.bin', + data, + onProgress: () => {}, + }); + const xhr = FakeXMLHttpRequest.latest!; + expect(xhr.sentBody).toBe(data); + xhr.status = 201; + xhr.responseText = JSON.stringify(uploadResult); + xhr.onload?.(); + await expect(promise).resolves.toEqual(uploadResult); + }); + + it('inherits the client timeout on the XHR when timeoutMs is omitted', async () => { + const client = new DaemonClient({ + baseUrl: 'http://daemon', + fetchTimeoutMs: 30_000, + }); + + const promise = client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + onProgress: () => {}, + }); + const xhr = FakeXMLHttpRequest.latest!; + xhr.status = 201; + xhr.responseText = JSON.stringify(uploadResult); + xhr.onload?.(); + + await expect(promise).resolves.toEqual(uploadResult); + expect(xhr.timeout).toBe(30_000); + }); + }); +}); diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 97b07f652d..a61ef8b57a 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -539,6 +539,7 @@ vi.mock('./utils/systemInfo', () => ({ vi.mock('./components/ChatEditor', async () => { const React = await import('react'); + const { useWebShellCustomization } = await import('./customization'); return { ChatEditor: React.memo( React.forwardRef(function ChatEditor( @@ -563,6 +564,7 @@ vi.mock('./components/ChatEditor', async () => { testState.chatEditorRenderCount += 1; testState.latestChatEditorProps = props; const { onAttachmentsChange } = props; + const customization = useWebShellCustomization(); React.useEffect(() => { onAttachmentsChange?.( Boolean( @@ -608,7 +610,13 @@ vi.mock('./components/ChatEditor', async () => { })); return React.createElement( 'div', - { 'data-web-shell-composer': '' }, + { + 'data-web-shell-composer': '', + 'data-file-upload-enabled': + customization.fileUploadEnabled === undefined + ? undefined + : String(customization.fileUploadEnabled), + }, React.createElement( 'button', { @@ -19252,3 +19260,17 @@ describe('App manual-run orchestration (scheduled tasks)', () => { expect(editorInsertText).not.toHaveBeenCalled(); // but priming skipped }); }); + +describe('fileUploadEnabled customization plumbing', () => { + it('reaches the composer customization when the host disables upload', () => { + const { container } = renderApp({ fileUploadEnabled: false }); + const composer = container.querySelector('[data-web-shell-composer]'); + expect(composer?.getAttribute('data-file-upload-enabled')).toBe('false'); + }); + + it('leaves the customization unset when the prop is omitted', () => { + const { container } = renderApp({}); + const composer = container.querySelector('[data-web-shell-composer]'); + expect(composer?.hasAttribute('data-file-upload-enabled')).toBe(false); + }); +}); diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index c57725d1ca..99d4dd035f 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -931,6 +931,14 @@ export interface WebShellProps { onSlashCommand?: WebShellSlashCommandHandler; /** Built-in @ mention providers to enable. Defaults to all built-ins. */ builtinAtProviders?: WebShellBuiltinAtProvidersConfig; + /** + * Controls whether the composer's file-upload entry points (drag-and-drop + * and the @ panel upload item) are enabled. Works alongside the daemon's + * `workspace_file_upload` capability, not instead of it: `false` force- + * disables upload even when the daemon advertises the capability, while + * `true`/omitted still requires the capability to be satisfied. + */ + fileUploadEnabled?: boolean; /** Additional @ mention categories shown alongside built-in files/extensions. */ atProviders?: readonly WebShellAtProvider[]; /** Icon URLs for custom composer tag kinds used by @ mention chips. */ @@ -1798,6 +1806,7 @@ export function App({ builtinAtProviders, atProviders, composerTagIcons, + fileUploadEnabled, renderToolHeaderExtra, renderWelcomeHeader, renderWelcomeFooter, @@ -2054,6 +2063,7 @@ export function App({ markdownTableMode, markdown, loadingPhrases, + fileUploadEnabled, }), [ composerTagIcons, @@ -2077,6 +2087,7 @@ export function App({ markdownTableMode, markdown, loadingPhrases, + fileUploadEnabled, ], ); const CustomFooter = renderFooter; diff --git a/packages/web-shell/client/components/AtMentionPanel.test.tsx b/packages/web-shell/client/components/AtMentionPanel.test.tsx index 72a857adbd..f44733d62d 100644 --- a/packages/web-shell/client/components/AtMentionPanel.test.tsx +++ b/packages/web-shell/client/components/AtMentionPanel.test.tsx @@ -239,6 +239,26 @@ describe('AtMentionPanel', () => { expect(onSelectTab).toHaveBeenCalledWith('hg'); }); + it('renders the upload item with an upload icon', () => { + const menu = itemsMenu(); + menu.items = [ + { + id: 'upload-file', + label: 'Upload file', + kind: 'upload', + insertText: '', + description: 'Upload a file into this folder', + }, + ]; + mount(menu); + + expect(document.body.textContent).toContain('Upload file'); + expect(document.body.textContent).toContain( + 'Upload a file into this folder', + ); + expect(document.body.querySelector('svg.lucide-upload')).not.toBeNull(); + }); + it('guards image icon sources', () => { const menu = itemsMenu(); menu.items = [ diff --git a/packages/web-shell/client/components/AtMentionPanel.tsx b/packages/web-shell/client/components/AtMentionPanel.tsx index c182f8443d..fc63afaee6 100644 --- a/packages/web-shell/client/components/AtMentionPanel.tsx +++ b/packages/web-shell/client/components/AtMentionPanel.tsx @@ -8,6 +8,7 @@ import { type ReactNode, } from 'react'; import { createPortal } from 'react-dom'; +import { UploadIcon } from 'lucide-react'; import { useI18n } from '../i18n'; import { useWebShellPortalRoot } from '../portalRoot'; import { @@ -175,7 +176,8 @@ export function AtMentionPanel({ labelTitle: item.label, subtitle: item.subtitle, description: - menu.selectedProviderId === FILE_PROVIDER_ID + menu.selectedProviderId === FILE_PROVIDER_ID && + item.kind !== 'upload' ? undefined : (item.description ?? item.detail), icon: item.icon, @@ -443,7 +445,13 @@ export function AtMentionPanel({ <> - {'icon' in row && + {'item' in row && row.item.kind === 'upload' ? ( +