From 58da3973ed83acb263563d40c500631922ae840e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=92=89=E8=90=81?= Date: Mon, 10 Aug 2026 20:17:56 +0800 Subject: [PATCH 01/12] feat: add web shell workspace file uploads --- docs/design/web-shell-file-upload.md | 304 ++++++++++++ .../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 | 133 +++++ .../cli/src/serve/fs/workspace-file-system.ts | 137 ++++- packages/cli/src/serve/routes/capabilities.ts | 4 + .../serve/routes/workspace-file-read.test.ts | 5 + .../serve/routes/workspace-file-write.test.ts | 469 ++++++++++++++++++ .../src/serve/routes/workspace-file-write.ts | 417 ++++++++++++++++ packages/cli/src/serve/server.test.ts | 2 + packages/cli/src/serve/server.ts | 24 + packages/cli/src/serve/server/telemetry.ts | 1 + packages/cli/src/serve/types.ts | 2 + .../serve/workspace-qualified-rest.test.ts | 102 ++++ packages/sdk-typescript/scripts/build.js | 4 +- .../sdk-typescript/src/daemon/DaemonClient.ts | 158 ++++++ packages/sdk-typescript/src/daemon/index.ts | 2 + packages/sdk-typescript/src/daemon/types.ts | 28 ++ .../test/unit/DaemonClient.upload.test.ts | 289 +++++++++++ packages/web-shell/client/App.tsx | 11 + .../client/components/AtMentionPanel.test.tsx | 16 + .../client/components/AtMentionPanel.tsx | 12 +- .../client/components/ChatEditor.module.css | 112 +++++ .../client/components/ChatEditor.test.tsx | 213 +++++++- .../client/components/ChatEditor.tsx | 267 +++++++++- packages/web-shell/client/customization.tsx | 9 + .../client/hooks/useAtMentionMenu.test.tsx | 36 ++ .../client/hooks/useAtMentionMenu.ts | 48 +- .../client/hooks/useComposerCore.dom.test.tsx | 2 + .../web-shell/client/hooks/useComposerCore.ts | 27 +- .../client/hooks/useFileUpload.test.tsx | 366 ++++++++++++++ .../web-shell/client/hooks/useFileUpload.ts | 266 ++++++++++ packages/web-shell/client/i18n.tsx | 20 + 35 files changed, 3473 insertions(+), 37 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 00000000000..af74366ae43 --- /dev/null +++ b/docs/design/web-shell-file-upload.md @@ -0,0 +1,304 @@ +# 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 `writeEncodedTextTemp`, 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: + - 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. + - Builds the first candidate from the captured resolved directory plus basename and resolves that absolute path. Ordinary `fs.resolve(..., 'write')` follows a final-component symlink that stays inside the workspace. If the result differs from the candidate path, the route treats that name as occupied and starts numbering instead of writing beside or through the link target; an escaping link still fails at the boundary. + - Stores the requested basename, resolved parent directory, initially resolved target or occupied marker, route name, fs instance, and `originatorClientId` in a private request context for the handler; later stages do not resolve the original path 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)`, gets the request-scoped fs with the admitted `originatorClientId`, 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 { + client: DaemonClient | WorkspaceDaemonClient; + 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 + error?: string; + resultPath?: string; // server-confirmed final path +} + +interface UseFileUploadReturn { + uploads: FileUploadItem[]; + uploadFiles: ( + files: File[], + targetDir: string, + onUploaded: (path: string) => void, + ) => void; + 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. + +- 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: string)` callback wired in from the composer as a `UseAtMentionMenuOptions` field, and closes the menu. The callback synchronously stores `targetDir` and the current upload `targetKey`, 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. +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 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/packages/cli/src/serve/bridge-file-system-adapter.test.ts b/packages/cli/src/serve/bridge-file-system-adapter.test.ts index f7fbbbdd7a5..a15aad47966 100644 --- a/packages/cli/src/serve/bridge-file-system-adapter.test.ts +++ b/packages/cli/src/serve/bridge-file-system-adapter.test.ts @@ -497,6 +497,7 @@ describe('createBridgeFileSystemAdapter', () => { })), edit: vi.fn(), editAtomic: vi.fn(), + writeBytesAtomic: vi.fn(), }; }, }; @@ -548,6 +549,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 b5709c0bbef..ec312c51383 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -155,6 +155,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?}` — @@ -338,8 +344,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 3fcc24498f4..2ffc9154995 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 472299c8042..a6eca377904 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 03f64acfb7c..47a743d9bd7 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.test.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.test.ts @@ -2283,6 +2283,139 @@ 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('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; + // entry (1), inside-lock (2), pre-publish (3). Close at the + // final publish checkpoint. + if (checks === 3) 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'); + 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([]); + }); +}); + 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 d06fb99b0ee..20583adeaec 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.ts @@ -54,6 +54,7 @@ import { BINARY_PROBE_BYTES, MAX_READ_BYTES, MAX_TEXT_SCAN_BYTES, + MAX_UPLOAD_BYTES, assertTrustedForIntent, enforceReadSize, enforceWriteSize, @@ -274,6 +275,22 @@ 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. `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 }>; } /** @@ -1287,6 +1304,43 @@ 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) { + throw this.recordAndWrap(err, 'write', p as string); + } + } + /** * Coerce an arbitrary thrown value into an `FsError`, emit the * matching `fs.denied` audit event, and return the typed error @@ -1988,6 +2042,38 @@ function mergeWriteMeta( async function atomicWriteTextResolvedFile( input: AtomicWriteTextInput, ): Promise { + const buf = await encodeTextFileContentAsync( + input.target as string, + 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: ResolvedPath; + buf: Buffer; + mode: WriteMode; + expectedHash?: ContentHash; + assertGenerationOpen?: () => void; +}): Promise { const target = input.target as string; const parent = path.dirname(target); const parentStat = await fsp.lstat(parent); @@ -2006,24 +2092,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, @@ -2042,7 +2138,7 @@ async function atomicWriteTextResolvedFile( } tempLive = false; await fsyncParentDirBestEffort(parent); - return encoded; + return written; } catch (err) { await tempHandle?.close().catch(() => undefined); if (tempLive) { @@ -2062,20 +2158,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(); @@ -2093,7 +2186,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 a50fa1fd798..229f90fc98b 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 a03dd5ed761..ef0f6044e6b 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 888d83ea881..6993ec4cd37 100644 --- a/packages/cli/src/serve/routes/workspace-file-write.test.ts +++ b/packages/cli/src/serve/routes/workspace-file-write.test.ts @@ -9,6 +9,7 @@ import { promises as fsp } from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import express from 'express'; import request from 'supertest'; import { createServeApp } from '../server.js'; import { @@ -294,3 +295,471 @@ 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.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)), + }); + }); + + 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 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('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']), + ); + // Both files exist with their own content. + await expect( + fsp.stat(path.join(h.workspace, 'race.bin')), + ).resolves.toBeDefined(); + await expect( + fsp.stat(path.join(h.workspace, 'race (1).bin')), + ).resolves.toBeDefined(); + }); + + 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('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('preserves a generation-closed error from the parent stat', async () => { + await teardown(h); + let checks = 0; + h = await makeHarness({ + token: 'secret', + generationGuard: { + assertOpen() { + checks += 1; + if (checks === 3) { + throw Object.assign(new Error('closed'), { + code: 'workspace_generation_closed', + }); + } + }, + }, + }); + + const res = await upload('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')); + const res = await upload(path.join('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('handles filenames with spaces, non-ASCII, and a literal %', async () => { + const name = 'my 数据 %b.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('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('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('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 to fit 255. + expect(Buffer.byteLength(base, 'utf-8')).toBeLessThanOrEqual(255); + expect(base.endsWith(' (1).txt')).toBe(true); + expect(base).not.toBe(`${stem} (1).txt`); + // The original is untouched. + expect(await fsp.readFile(path.join(h.workspace, name), 'utf-8')).toBe( + 'orig', + ); + }); + + it('returns 409 when all numbered candidates up to the cap are occupied', async () => { + // Occupy a.txt, a (1).txt ... a (999).txt (all 1000 candidates). + await Promise.all( + Array.from({ length: 1000 }, (_, i) => + fsp.writeFile( + path.join(h.workspace, i === 0 ? 'a.txt' : `a (${i}).txt`), + 'x', + ), + ), + ); + const res = await upload('a.txt').send(Buffer.from('y')); + expect(res.status).toBe(409); + expect(res.body.errorKind).toBe('file_already_exists'); + }); +}); + +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. + gate.release(); + gate.release(); + gate.release(); + expect(gate.tryAcquire()).toBe(true); + }); +}); + +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 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; + return hold.then(() => target.writeBytesAtomic(p, data)); + }; + } + 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'); + + release(); + const results = await Promise.all(inFlight.slice(1)); + for (const res of results) { + expect(res.status).toBe(201); + } + + // Slots freed on completion — a subsequent upload now succeeds. + const sixth = await upload('f.bin'); + expect(sixth.status).toBe(201); + expect(sixth.body.path).toBe('f.bin'); + } 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, + }); + }); +}); diff --git a/packages/cli/src/serve/routes/workspace-file-write.ts b/packages/cli/src/serve/routes/workspace-file-write.ts index aba02c17bf2..e36c2410b46 100644 --- a/packages/cli/src/serve/routes/workspace-file-write.ts +++ b/packages/cli/src/serve/routes/workspace-file-write.ts @@ -4,11 +4,17 @@ * 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, isContentHash, + isFsError, type ContentHash, + type ResolvedPath, + type WorkspaceFileSystem, type WorkspaceFileSystemFactory, type WriteMode, } from '../fs/index.js'; @@ -357,3 +363,414 @@ 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; + readonly max: number; +} + +export function createUploadConcurrencyGate( + max: number = MAX_CONCURRENT_UPLOADS, +): UploadConcurrencyGate { + let active = 0; + return { + max, + tryAcquire() { + if (active >= max) return false; + active += 1; + return true; + }, + release() { + if (active > 0) active -= 1; + }, + }; +} + +interface UploadAdmission { + route: string; + fs: WorkspaceFileSystem; + originatorClientId: string | undefined; + basename: string; + resolvedDir: ResolvedPath; +} + +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, + }); + return (req, res, next) => { + raw(req, res, (err?: unknown) => { + if (err) { + if ((err as { status?: number }).status === 413) { + sendUploadTooLarge(res); + 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); + if (basename.length === 0 || basename === '.' || basename === '..') { + sendParseError(res, ROUTE, '`path` must name a file'); + 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, + originatorClientId, + basename, + resolvedDir, + }); + 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 } = admission; + const body = req.body; + const data = + body === undefined || body === null ? Buffer.alloc(0) : (body as Buffer); + const { stem, ext } = splitStemExtension(basename); + const lease = uploadGateLeases.get(req); + if (lease) lease.handlerStarted = true; + try { + 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(candidateAbs, 'write'); + } catch (err) { + // Boundary escapes and other resolution failures stop the loop. + 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 758593730cf..34f5e7100a4 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -459,6 +459,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 f0a1dd63495..87dfa9e0361 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 { @@ -2078,6 +2081,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.ts b/packages/cli/src/serve/server/telemetry.ts index 4818bf46d57..ed73038ceeb 100644 --- a/packages/cli/src/serve/server/telemetry.ts +++ b/packages/cli/src/serve/server/telemetry.ts @@ -544,6 +544,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 16011597d80..83b77a67eba 100644 --- a/packages/cli/src/serve/types.ts +++ b/packages/cli/src/serve/types.ts @@ -462,6 +462,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 cd639449bf8..081c131028e 100644 --- a/packages/cli/src/serve/workspace-qualified-rest.test.ts +++ b/packages/cli/src/serve/workspace-qualified-rest.test.ts @@ -799,6 +799,108 @@ 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('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/sdk-typescript/scripts/build.js b/packages/sdk-typescript/scripts/build.js index c564723003f..91429babb63 100755 --- a/packages/sdk-typescript/scripts/build.js +++ b/packages/sdk-typescript/scripts/build.js @@ -80,7 +80,9 @@ const rootDir = join(__dirname, '..'); // Bumped from 178KB to 184KB for side-task session APIs and source metadata. // Bumped from 184KB to 185KB for the Live Voice lifecycle helpers on both // daemon client classes. -const MAX_DAEMON_BROWSER_BUNDLE_BYTES = 185 * 1024; +// Bumped from 185KB to 187KB for the workspace file-upload surface +// (`uploadWorkspaceFile` + XHR progress) on both daemon client classes. +const MAX_DAEMON_BROWSER_BUNDLE_BYTES = 187 * 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 985315e4f04..61b2a30550c 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -72,6 +72,8 @@ import type { DaemonWorkspaceFileBytes, DaemonWorkspaceFileEditRequest, DaemonWorkspaceFileEditResult, + DaemonWorkspaceFileUploadRequest, + DaemonWorkspaceFileUploadResult, DaemonWorkspaceFileWriteRequest, DaemonWorkspaceFileWriteResult, DaemonWorkspaceAgentDetail, @@ -1899,6 +1901,150 @@ 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); + return (await res.json()) 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) => { + 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) { + 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(new DOMException('The operation was aborted.', 'AbortError')); + }; + if (req.signal) { + if (req.signal.aborted) { + reject( + new DOMException('The operation was aborted.', 'AbortError'), + ); + return; + } + abortListener = () => xhr.abort(); + req.signal.addEventListener('abort', abortListener, { once: true }); + } + xhr.send(req.data as XMLHttpRequestBodyInit); + }, + ); + } + + async uploadWorkspaceFile( + req: DaemonWorkspaceFileUploadRequest, + clientId?: string, + ): Promise { + return await this.uploadFileToPath( + '/file/upload', + 'POST /file/upload', + req, + clientId, + ); + } + // -- Workspace memory (workspace memory/agents) ------------------------------ /** @@ -5862,6 +6008,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 7686d152923..bf081ce0558 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -592,6 +592,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 2a605846b33..e00185192fe 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 { @@ -1963,6 +1965,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/test/unit/DaemonClient.upload.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.upload.test.ts new file mode 100644 index 00000000000..9eaae840919 --- /dev/null +++ b/packages/sdk-typescript/test/unit/DaemonClient.upload.test.ts @@ -0,0 +1,289 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { 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: 'my 数据 %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('my 数据 %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('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), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const ctrl = new AbortController(); + await client.uploadWorkspaceFile({ + path: 'a.bin', + data: new Uint8Array([1]), + signal: ctrl.signal, + }); + expect(calls[0]?.signal).toBeTruthy(); + }); + + it('inherits the client timeout when timeoutMs is omitted', 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(); + } + }); +}); diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index b12cd27e7ea..49c2db48db5 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -722,6 +722,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. */ @@ -1578,6 +1586,7 @@ export function App({ builtinAtProviders, atProviders, composerTagIcons, + fileUploadEnabled, renderToolHeaderExtra, renderWelcomeHeader, renderWelcomeFooter, @@ -1833,6 +1842,7 @@ export function App({ markdownTableMode, markdown, loadingPhrases, + fileUploadEnabled, }), [ composerTagIcons, @@ -1856,6 +1866,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 72a857adbd9..0485b906ddd 100644 --- a/packages/web-shell/client/components/AtMentionPanel.test.tsx +++ b/packages/web-shell/client/components/AtMentionPanel.test.tsx @@ -239,6 +239,22 @@ 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: '', + }, + ]; + mount(menu); + + expect(document.body.textContent).toContain('Upload file'); + 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 c182f8443d9..f3e139e10a1 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 { @@ -443,7 +444,13 @@ export function AtMentionPanel({ <> - {'icon' in row && + {'item' in row && row.item.kind === 'upload' ? ( +