diff --git a/docs/developers/daemon/07-workspace-filesystem.md b/docs/developers/daemon/07-workspace-filesystem.md index a5db05a30d5..a0c2d558543 100644 --- a/docs/developers/daemon/07-workspace-filesystem.md +++ b/docs/developers/daemon/07-workspace-filesystem.md @@ -7,7 +7,7 @@ Daemon HTTP file routes and ordinary delegated ACP `readTextFile` / `writeTextFi - **Path resolution** — canonicalize paths and reject anything escaping the bound workspace, including via symlinks. - **Trust gating** — refuse writes when the workspace is not trusted (`untrusted_workspace`). - **Size & content policy** — full-snapshot/output cap (`MAX_READ_BYTES = 256 KiB`), large-text windows bounded in both output and scan cost (`MAX_TEXT_SCAN_BYTES = 8 MiB`), write cap (`MAX_WRITE_BYTES = 5 MiB`), binary detection. -- **Atomicity** — write-then-rename with target mode preservation and `0o600` default for new files. +- **Atomicity** — write-then-rename with target mode preservation; new files default to `0o600`, or follow the process umask under the factory's `system` new-file mode policy (`QWEN_SERVE_NEW_FILE_MODE`). - **Audit** — every access / denial emits a structured event for `PermissionAuditRing` / monitoring. - **Typed errors** — closed `FsErrorKind` union mapped to HTTP statuses. @@ -29,7 +29,7 @@ That text-read capability slice covers direct `read_file` plus the shared pre-re - Refuse full-snapshot reads above `MAX_READ_BYTES`, while allowing explicit windows with output capped at `MAX_READ_BYTES` and scan cost capped at `MAX_TEXT_SCAN_BYTES`; refuse writes above `MAX_WRITE_BYTES` and binary files (`binary_file`). - Refuse writes/edits when the workspace is untrusted (`untrusted_workspace`) — gated by `assertTrustedForIntent(trusted, intent)`. - Honor `.gitignore` / `.qwenignore` patterns via `shouldIgnore`. -- Perform atomic write-then-rename with target mode preservation; default new file mode is `0o600`. +- Perform atomic write-then-rename with target mode preservation; new files default to `0o600` (umask-derived `0o666 & ~umask` under the `system` new-file mode policy). - Emit `fs.access` / `fs.denied` audit events on every operation. - Map every failure to a `FsError` with kind and HTTP status; route handlers serialize them uniformly. @@ -82,7 +82,7 @@ Two defensive properties the adapter MUST preserve (because the inline proxy is 1. **Reject non-regular files** — sockets / pipes / char devices / procfs / sysfs entries can stream unbounded data despite `stats.size === 0`. The inline path throws with `describeStatKind(stats)` in the message. 2. **Avoid unbounded full-file buffering.** The inline fallback caps a buffered read at `READ_FILE_SIZE_CAP = 100 MiB`. The injected adapter instead applies the stricter WorkspaceFileSystem contract: full snapshots stop at 256 KiB, while larger UTF-8 files require a finite `limit` and are streamed from an inode-bound handle with at most 256 KiB returned. It must not read an entire 500 MB log merely to return `{ line: 1, limit: 10 }`. -The adapter goes further: it uses `WorkspaceFileSystem.writeTextOverwrite` (PR 18 primitive) for workspace writes and a factory-owned equivalent for strictly marked external built-in-tool writes. Both use atomic temporary-file-and-rename writes with mode preservation, `0o600` default, and symlink rejection inside the shared canonical-path lock. This is a **divergence from the pre-F1 inline proxy** which resolved symlinks and wrote through to their target — agents that relied on writing through symlinked dotfiles now have to address the resolved path directly. +The adapter goes further: it uses `WorkspaceFileSystem.writeTextOverwrite` (PR 18 primitive) for workspace writes and a factory-owned equivalent for strictly marked external built-in-tool writes. Both use atomic temporary-file-and-rename writes with mode preservation, new-file mode under the factory's `NewFileModePolicy` (`0o600` default; umask-following under `system`), and symlink rejection inside the shared canonical-path lock. This is a **divergence from the pre-F1 inline proxy** which resolved symlinks and wrote through to their target — agents that relied on writing through symlinked dotfiles now have to address the resolved path directly. ### FsError preservation over the ACP wire @@ -250,7 +250,7 @@ flowchart LR - **Symlinks are rejected, not followed.** This is a divergence from the pre-F1 inline `BridgeClient.writeTextFile` proxy which resolved symlinks. Agents writing through symlinked dotfiles need to address the resolved path directly. - **`io_error` vs `permission_denied` are distinct.** Do not conflate them. Monitoring pipelines key on `errorKind` for alerting — folding ENOSPC into permission_denied would page security responders for `df -h` problems. -- **New file mode defaults to `0o600`, not umask defaults.** The write syscall's `mode` arg bypasses umask. Agents writing public files should explicitly pass a mode override. +- **New file mode defaults to `0o600`, not umask defaults.** The write syscall's `mode` arg bypasses umask. Agents cannot pass a per-write mode override. Operators who want agent-created files to follow the daemon's umask can opt in per daemon with `QWEN_SERVE_NEW_FILE_MODE=system` (existing files still preserve their mode); see [`17-configuration.md`](./17-configuration.md). - **`createServeApp` default `trusted: false`** silently rejects ACP writes with `untrusted_workspace` for embedders that do not inject a custom `fsFactory` or `bridge`. A one-time stderr warning fires the first time; further callers see no reminder. See [`02-serve-runtime.md`](./02-serve-runtime.md). - **Large text requires an explicit window argument**, any of `line` / `limit` / `maxBytes`. A read with none of them stays `file_too_large`, because a caller that believes it holds the whole file may write it back truncated. Windows stream from an inode-bound handle and never return more than `MAX_READ_BYTES`. - **`MAX_READ_BYTES` caps what a read returns; `MAX_TEXT_SCAN_BYTES` caps what it costs.** Line offsets are resolved by scanning from byte 0, so `{ line: 900_000_000, limit: 20 }` returns almost nothing and still walks the file. Past 8 MiB of scanning the read is refused with `file_too_large` pointing at `readBytes`, which reaches any offset in O(1). diff --git a/docs/developers/daemon/17-configuration.md b/docs/developers/daemon/17-configuration.md index 6a0f2ca53bb..b04b3368a0d 100644 --- a/docs/developers/daemon/17-configuration.md +++ b/docs/developers/daemon/17-configuration.md @@ -65,6 +65,7 @@ This page collects every setting that affects the `qwen serve` daemon and its ad | `QWEN_SERVE_RATE_LIMIT_MUTATION` | Env fallback for `--rate-limit-mutation`. | | `QWEN_SERVE_RATE_LIMIT_READ` | Env fallback for `--rate-limit-read`. | | `QWEN_SERVE_RATE_LIMIT_WINDOW_MS` | Env fallback for `--rate-limit-window-ms`. | +| `QWEN_SERVE_NEW_FILE_MODE` | New-file mode policy for daemon text writes: `owner` (default — NEW files are created `0600`, umask-independent) or `system` (NEW files follow `0o666 & ~umask`). Case-insensitive; the literal `0600` is accepted as an alias for `owner` (no other octal modes are supported), and unrecognized values warn on stderr and keep the `0600` default. Existing files always preserve their mode. See [`qwen-serve.md` — New-file mode for agent text writes](../../users/qwen-serve.md#new-file-mode-for-agent-text-writes). | | `QWEN_CODE_MEMORY_PROJECT_SCOPE` | `workspace` keys project memory by the exact workspace dir; `git-root` selects the legacy shared scope. When unset, the daemon injects `workspace`; unrecognized values warn once and retain the legacy `git-root` behavior. Propagates via the runtime base env, not `childEnvOverrides`; `--memory-project-scope` wins. Each workspace remember/forget/dream lane caps pending tasks at `MAX_PENDING = 16`; N workspaces allow up to 16·N queued tasks with no daemon-wide cap. | Blank `QWEN_CODE_MEMORY_PROJECT_SCOPE` values are treated as unset and therefore default to `workspace`; unrecognized non-empty values still warn once and retain the legacy `git-root` behavior. diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 5a31f8545c3..daf03659a36 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -626,7 +626,7 @@ provider decision with their normal tool policy and isolation boundary. - **Chrome extension browser automation is separate from framing.** `qwen serve --allow-origin chrome-extension://` lets the extension frame the Web Shell and connect to the daemon. Console/network/screenshot/click tools require an external CDP MCP adapter command: `QWEN_CDP_MCP_COMMAND=/path/to/cdp-mcp-adapter qwen serve --allow-origin chrome-extension://`. The main CLI package does not bundle a browser automation adapter; clients can check `caps.features.includes('browser_automation_mcp')` before presenting those tools as available. - **A spawned `qwen --acp` child receives its owning runtime's effective environment.** The daemon freezes a process-env base, applies that workspace's settings/env-file overlay to a runtime-local snapshot, and never writes the overlay back to `process.env`; same-named keys in another runtime do not cross over. `QWEN_SERVER_TOKEN` is scrubbed before spawn because the agent does not need the daemon bearer. Loader-affecting variables (`NODE_OPTIONS`, `npm_config_node_options` and npm's config-file redirects, `NODE_PATH`, `OPENSSL_CONF`, `NODE_REPL_EXTERNAL_MODULE`, `npm_config_node_gyp`, `npm_config_init_module`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, exported bash function definitions `BASH_FUNC_*`) are likewise never passed to session subprocesses — the daemon scrubs them from its own `process.env` and from the frozen base env that session-hosting children spawn with (the base env keeps them only under the `DEV=true` harness, whose `.ts` entries still need the tsx loader), and `.env` / `settings.json` `env` sources reject them (see [settings](./configuration/settings.md)); this applies to every session the daemon hosts. Base credentials such as `OPENAI_API_KEY`, `ANTHROPIC_API_KEY`, `QWEN_*`, and `DASHSCOPE_API_KEY` otherwise pass through unless the runtime overlay changes them. **This is intentional, not a sandbox.** The agent runs as the same UID with shell-tool access, so anything in `~/.bashrc`, `~/.aws/credentials`, or `~/.npmrc` is reachable by prompt injection regardless. Environment isolation between runtimes is not an operating-system security boundary; do not run `qwen serve` under an identity that has credentials you would not trust the agent with. - **Agent text reads are child-local and follow the regular CLI permission rules, not the workspace filesystem boundary.** Direct `read_file` can reach host text paths outside every registered workspace: external paths default to confirmation, and allow rules or approval modes may approve them automatically. Approved reads use the configurable CLI output limits rather than the workspace filesystem's returned-output, full-snapshot, and large-text scan caps. This applies to every shared text-read consumer, so the pre-reads performed by write, edit, notebook, sed, and artifact operations lose those caps together with the workspace filesystem's read audit, symlink rejection, and read-side TOCTOU protections — see [the read design](../design/daemon-local-text-reads.md) for the exact list. Because a confirmation payload is built by reading the file, an out-of-workspace diff is fanned out to **every** attached SSE subscriber before anyone approves it — in the interactive CLI that content is seen only by the person at the terminal. Treat authenticated daemon clients as the same security principal. HTTP filesystem routes remain workspace-scoped and agent discovery-tool behavior is unchanged. -- **Approved final writes from built-in text tools have a narrow same-host route.** `write_file`, `edit`, `notebook_edit`, and the shell tool's simulated sed editor attach internal provenance only after the existing permission policy allows execution. Their final ACP text write can therefore target an absolute path outside the owning workspace without a second confirmation; allow rules, AUTO/AUTO_EDIT and YOLO behave like the CLI, while rejection, Plan, Hook/Guard refusal and pre-execution cancellation do not send the final write. Cancellation after a tool has already entered a non-cancellable filesystem operation keeps that tool's existing behavior. Workspace targets still use WFS. External targets use a daemon host writer with the same trust snapshot, 5 MiB encoded limit, leaf-symlink rejection, canonical path lock, atomic rename, mode preservation, `0600` new-file mode, generation guard and filesystem audit. HTTP writes, generic or unmarked ACP writes, injected bridge/workspace-registry/factory integrations and arbitrary shell redirection do not receive this exception. See [the external-write design](../design/daemon-external-tool-text-writes.md). +- **Approved final writes from built-in text tools have a narrow same-host route.** `write_file`, `edit`, `notebook_edit`, and the shell tool's simulated sed editor attach internal provenance only after the existing permission policy allows execution. Their final ACP text write can therefore target an absolute path outside the owning workspace without a second confirmation; allow rules, AUTO/AUTO_EDIT and YOLO behave like the CLI, while rejection, Plan, Hook/Guard refusal and pre-execution cancellation do not send the final write. Cancellation after a tool has already entered a non-cancellable filesystem operation keeps that tool's existing behavior. Workspace targets still use WFS. External targets use a daemon host writer with the same trust snapshot, 5 MiB encoded limit, leaf-symlink rejection, canonical path lock, atomic rename, mode preservation, `0600` new-file mode by default (configurable — see [New-file mode for agent text writes](#new-file-mode-for-agent-text-writes)), generation guard and filesystem audit. HTTP writes, generic or unmarked ACP writes, injected bridge/workspace-registry/factory integrations and arbitrary shell redirection do not receive this exception. See [the external-write design](../design/daemon-external-tool-text-writes.md). - **Per-subscriber bounded SSE queues** — a slow client that overflows its queue gets a `client_evicted` terminal frame and is closed; one stuck consumer can't pin the daemon. - **Per-session prompt admission cap** — defaults to 5 accepted-but-unsettled prompts per session. A buggy client cannot enqueue unbounded prompt promises or temporary SSE waits for one session. - **Graceful shutdown** — SIGINT/SIGTERM drain the agent children before closing the listener (10s deadline per child). @@ -668,6 +668,22 @@ Issue [#4514](https://github.com/QwenLM/qwen-code/issues/4514) T2.9 ships two op Both flags accept a positive integer in milliseconds; `0`, `NaN`, non-integer, or negative values are rejected at boot with a clear error message. CLI flag wins over env var; explicit `ServeOptions` field (embedded callers) wins over env. SDK consumers should pre-flight the matching capability tag before relying on either behavior — daemons predating this PR omit both tags and the request `deadlineMs` field is silently dropped. +### New-file mode for agent text writes + +Agent text writes (`write_file`, `edit`, `notebook_edit`, and the shell tool's simulated sed editor) publish through the daemon's atomic writer, which preserves an existing target's mode and — for **new** files — defaults to owner-only `0600`, ignoring the daemon process's umask. This fail-closed default is intentional: a fresh agent-created file is never group/world readable by accident, no matter how permissive the supervisor umask is. + +Operators whose deployment convention is umask-driven (e.g. a systemd unit with `UMask=0002`, shared-group repositories) can opt new files into the standard POSIX handling with: + +| Env var | Values | Default | What it does | +| -------------------------- | ---------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `QWEN_SERVE_NEW_FILE_MODE` | `owner` \| `system` | `owner` | `system` creates NEW files at `0o666 & ~umask`, so agent-created files follow the daemon process's umask like any other process on the machine. `owner` keeps the umask-independent `0600` default. Values are case-insensitive; the literal `0600` is accepted as an alias for `owner` (no other octal modes are supported), and any other value is rejected with a stderr warning and the `0600` default is kept. | + +Scope and limits: + +- Applies to NEW files created by the text-write routes (workspace targets, the same-host external host writer, and HTTP text writes). Existing files always keep their on-disk mode — editing a `0600` secret keeps it `0600`, an executable keeps `+x`. +- Binary uploads (`POST /file/upload`) always create at `0600` regardless of this setting. +- The daemon reads the variable at workspace-filesystem construction; restart the daemon after changing it. + ## Multi-session & multi-workspace deployment Pass `--workspace` more than once to register several non-overlapping workspaces in one `qwen serve` process. The first path is primary. Each registered workspace owns an isolated runtime boundary, while the daemon-wide listener, authentication policy, and total-session limit are shared. Production attempts to preheat the primary ACP child for compatibility and retries on first use after failure; trusted secondaries start their own child on demand, and untrusted secondaries do not start ACP. Requests may select a registered workspace by canonical `cwd`; requests that omit `cwd` use the primary workspace. Use one daemon per user or security principal; workspace trust is an execution gate, not an ACL. diff --git a/packages/cli/src/config/shared-env-keys.test.ts b/packages/cli/src/config/shared-env-keys.test.ts index 4199fdff84e..73d1a04ab11 100644 --- a/packages/cli/src/config/shared-env-keys.test.ts +++ b/packages/cli/src/config/shared-env-keys.test.ts @@ -86,6 +86,17 @@ describe('PROJECT_ENV_HARDCODED_EXCLUSIONS', () => { expect(PROJECT_ENV_HARDCODED_EXCLUSIONS).toContain('DEV'); }); + // QWEN_SERVE_NEW_FILE_MODE sets the daemon-wide creation mode for + // agent-written NEW files. A project `.env` flipping it to `system` would + // silently widen file visibility (0600 -> umask-derived) for every + // workspace with no warning, so the fail-closed posture stays an operator + // decision made in the daemon's launch env or a home `.env`. + it('excludes QWEN_SERVE_NEW_FILE_MODE so a project .env cannot widen new-file mode', () => { + expect(PROJECT_ENV_HARDCODED_EXCLUSIONS).toContain( + 'QWEN_SERVE_NEW_FILE_MODE', + ); + }); + // The non-Node TLS trust-anchor vars reach the same MITM outcome as // NODE_EXTRA_CA_CERTS for the curl/git/openssl/python tools a session // shells out to; a project .env must not inject an attacker CA. diff --git a/packages/cli/src/config/shared-env-keys.ts b/packages/cli/src/config/shared-env-keys.ts index 0dd7e641b16..19afdbdab60 100644 --- a/packages/cli/src/config/shared-env-keys.ts +++ b/packages/cli/src/config/shared-env-keys.ts @@ -180,6 +180,14 @@ export const PROJECT_ENV_HARDCODED_EXCLUSIONS = [ // operator set in the daemon's launch env still apply. 'QWEN_CDP_MCP_COMMAND', 'QWEN_SERVE_CDP_TUNNEL_OVER_WS', + // QWEN_SERVE_NEW_FILE_MODE decides the creation mode of every agent-written + // NEW file (owner-only 0600 vs umask-derived). A project `.env` flipping it + // to `system` would silently widen file visibility daemon-wide — including + // files written for OTHER workspaces — with no warning, since `system` is a + // valid value. The fail-closed 0600 posture is an operator decision + // (documented as a per-daemon opt-in), so only the daemon's launch + // environment or a home `.env` may set it. + 'QWEN_SERVE_NEW_FILE_MODE', // DEV gates the daemon's inherited-loader-env scrub (run-qwen-serve.ts); // only the dev harness (scripts/dev.js) stamps it into the launch env. A // project file setting it would silently keep loader vars in the base env diff --git a/packages/cli/src/serve/bridge-file-system-adapter.ts b/packages/cli/src/serve/bridge-file-system-adapter.ts index e87600e7b65..50bf6adc516 100644 --- a/packages/cli/src/serve/bridge-file-system-adapter.ts +++ b/packages/cli/src/serve/bridge-file-system-adapter.ts @@ -20,7 +20,9 @@ * - For writes: `wfs.writeTextOverwrite(resolved, content)` — the * primitive that does atomic temp+rename with target-mode * preservation (existing `0o600` survives the edit; new files - * default to `0o600`, NOT umask). Picked over `wfs.writeText` (no + * default to `0o600`, or follow the daemon umask under the + * factory's `'system'` new-file mode policy — see + * `QWEN_SERVE_NEW_FILE_MODE`). Picked over `wfs.writeText` (no * mode handling, non-atomic) and over `wfs.writeTextAtomic` (whose * `expectedHash` CAS gate doesn't map to ACP's hash-less * `WriteTextFileRequest` wire shape). diff --git a/packages/cli/src/serve/fs/index.ts b/packages/cli/src/serve/fs/index.ts index b974675c7ed..c83852bc924 100644 --- a/packages/cli/src/serve/fs/index.ts +++ b/packages/cli/src/serve/fs/index.ts @@ -43,10 +43,13 @@ export { type FsDeniedAuditPayload, } from './audit.js'; export { + OWNER_ONLY_NEW_FILE_MODE, createWorkspaceFileSystemFactory, isContentHash, + resolveNewFileModeBits, type ContentHash, type CreateWorkspaceFileSystemFactoryDeps, + type NewFileModePolicy, type FsEntry, type FsStat, type GlobOptions, 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 4b5f5ffafee..9abb750aae9 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.test.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.test.ts @@ -15,6 +15,8 @@ import { FS_ACCESS_EVENT_TYPE, FS_DENIED_EVENT_TYPE, createWorkspaceFileSystemFactory, + resolveNewFileModeBits, + type NewFileModePolicy, type ResolvedPath, type WorkspaceFileSystem, type WorkspaceFileSystemFactory, @@ -36,6 +38,7 @@ async function makeHarness(opts?: { ignore?: Ignore; includeRawPaths?: boolean; generationGuard?: { assertOpen(): void }; + newFileMode?: NewFileModePolicy; }): Promise { const scratch = await fsp.mkdtemp( path.join(os.tmpdir(), `qwen-wfs-${randomBytes(4).toString('hex')}-`), @@ -51,6 +54,7 @@ async function makeHarness(opts?: { ignore: opts?.ignore, includeRawPaths: opts?.includeRawPaths, generationGuard: opts?.generationGuard, + newFileMode: opts?.newFileMode, }); const fs = factory.forRequest({ originatorClientId: 'client-x', @@ -1709,6 +1713,126 @@ describe('WorkspaceFileSystem - write/edit', () => { }); }); +// New-file mode policy (#9250): the daemon's text writers historically +// created every NEW file at `0o600` regardless of the process umask. +// `QWEN_SERVE_NEW_FILE_MODE=system` lets operators opt into the +// standard `0o666 & ~umask` handling; the default stays owner-only. +describe('WorkspaceFileSystem - new-file mode policy', () => { + const isPosix = process.platform !== 'win32'; + + it('resolveNewFileModeBits: owner policy ignores the umask', () => { + expect(resolveNewFileModeBits('owner', 0o000)).toBe(0o600); + expect(resolveNewFileModeBits('owner', 0o002)).toBe(0o600); + expect(resolveNewFileModeBits('owner', 0o077)).toBe(0o600); + }); + + it('resolveNewFileModeBits: system policy applies 0o666 & ~umask', () => { + expect(resolveNewFileModeBits('system', 0o000)).toBe(0o666); + expect(resolveNewFileModeBits('system', 0o002)).toBe(0o664); + expect(resolveNewFileModeBits('system', 0o022)).toBe(0o644); + expect(resolveNewFileModeBits('system', 0o077)).toBe(0o600); + }); + + it('default (owner) policy ignores a permissive umask', async () => { + if (!isPosix) return; + const h = await makeHarness(); + const prev = process.umask(0o002); + try { + const r = await h.fs.resolve('owner-default.txt', 'write'); + await h.fs.writeTextOverwrite(r, 'x'); + const st = await fsp.lstat(r as string); + expect(st.mode & 0o7777).toBe(0o600); + } finally { + process.umask(prev); + await teardown(h); + } + }); + + it('system policy creates new files at 0o666 & ~umask', async () => { + if (!isPosix) return; + const h = await makeHarness({ newFileMode: 'system' }); + const prev = process.umask(0o002); + try { + const r = await h.fs.resolve('system-new.txt', 'write'); + const out = await h.fs.writeTextOverwrite(r, 'hello\n'); + expect(out.created).toBe(true); + const st = await fsp.lstat(r as string); + expect(st.mode & 0o7777).toBe(0o664); + } finally { + process.umask(prev); + await teardown(h); + } + }); + + it('system policy still preserves an existing target mode', async () => { + if (!isPosix) return; + const h = await makeHarness({ newFileMode: 'system' }); + const prev = process.umask(0o002); + try { + const target = path.join(h.workspace, 'system-secret.txt'); + await fsp.writeFile(target, 'old', { mode: 0o600 }); + await fsp.chmod(target, 0o600); + const r = await h.fs.resolve('system-secret.txt', 'write'); + const out = await h.fs.writeTextOverwrite(r, 'new'); + expect(out.created).toBe(false); + const st = await fsp.lstat(target); + expect(st.mode & 0o7777).toBe(0o600); + } finally { + process.umask(prev); + await teardown(h); + } + }); + + it('system policy applies to writeTextAtomic create', async () => { + if (!isPosix) return; + const h = await makeHarness({ newFileMode: 'system' }); + const prev = process.umask(0o022); + try { + const r = await h.fs.resolve('system-atomic.txt', 'write'); + await h.fs.writeTextAtomic(r, 'a\n', { mode: 'create' }); + const st = await fsp.lstat(r as string); + expect(st.mode & 0o7777).toBe(0o644); + } finally { + process.umask(prev); + await teardown(h); + } + }); + + it('system policy applies to the same-host external tool write route', async () => { + if (!isPosix) return; + const h = await makeHarness({ newFileMode: 'system' }); + const prev = process.umask(0o002); + try { + expect(h.factory.writeSameHostToolText).toBeTypeOf('function'); + const external = path.join(h.scratch, 'external-tool-write.txt'); + await h.factory.writeSameHostToolText?.( + { route: 'TEST same-host', sessionId: 'sess-1' }, + { path: external, content: 'ext\n' }, + ); + const st = await fsp.lstat(external); + expect(st.mode & 0o7777).toBe(0o664); + } finally { + process.umask(prev); + await teardown(h); + } + }); + + it('writeBytesAtomic keeps 0o600 even under the system policy', async () => { + if (!isPosix) return; + const h = await makeHarness({ newFileMode: 'system' }); + const prev = process.umask(0o002); + try { + const r = await h.fs.resolve('system-bytes.bin', 'write'); + await h.fs.writeBytesAtomic(r, Buffer.from('bin')); + const st = await fsp.lstat(r as string); + expect(st.mode & 0o7777).toBe(0o600); + } finally { + process.umask(prev); + await teardown(h); + } + }); +}); + describe('WorkspaceFileSystem - trust gate', () => { let h: Harness; beforeEach(async () => { diff --git a/packages/cli/src/serve/fs/workspace-file-system.ts b/packages/cli/src/serve/fs/workspace-file-system.ts index c90f8a12b86..9d5956879b6 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.ts @@ -200,6 +200,47 @@ export type WriteMode = 'create' | 'replace' | 'overwrite'; */ export type AtomicWriteMode = Exclude; +/** + * Mode policy for NEW files created by text writes + * (`writeTextAtomic` / `writeTextOverwrite` / `edit` / `editAtomic` + * and the same-host built-in-tool write route). + * + * - `'owner'` (default) — owner-only `0o600`, independent of the + * daemon's umask. This is the long-standing fail-closed posture: + * a fresh agent-created file is never world/group readable by + * accident, regardless of how permissive the process umask is. + * - `'system'` — the standard POSIX `0o666 & ~umask` handling, so + * agent-created files follow the daemon process's umask like any + * other process on the machine (e.g. `0664` under `umask 0002`). + * Operators running the daemon under a supervisor that sets + * `UMask=` (systemd drop-ins, containers) can opt in via + * `QWEN_SERVE_NEW_FILE_MODE=system`. + * + * Existing-file mode preservation is unaffected by either policy — + * editing a `0600` secret keeps it `0600`, an executable keeps `+x`. + * Binary uploads (`writeBytesAtomic`) always create at `0o600` + * regardless of this policy. + */ +export type NewFileModePolicy = 'owner' | 'system'; + +/** Owner-only mode bits applied to new files under the default policy. */ +export const OWNER_ONLY_NEW_FILE_MODE = 0o600; + +/** + * Resolve the mode bits for a NEW file under the given policy. + * `umask` is read lazily only when the `system` policy consumes it; the + * default `owner` policy never touches the process umask. Tests pass an + * explicit value to stay deterministic without mutating process state. + */ +export function resolveNewFileModeBits( + policy: NewFileModePolicy, + umask?: number, +): number { + return policy === 'system' + ? 0o666 & ~(umask ?? process.umask()) + : OWNER_ONLY_NEW_FILE_MODE; +} + export interface WriteTextAtomicOptions extends WriteTextFileOptions { mode: AtomicWriteMode; expectedHash?: ContentHash; @@ -262,10 +303,12 @@ export interface WorkspaceFileSystem { /** * Unconditional create-or-overwrite (no `expectedHash` gate). Atomic * temp+rename with target-mode preservation: a `0o600` secret survives - * the edit at `0o600`; a new file is created at `0o600` (NOT umask - * default). Used by protocols whose wire format carries no client-side - * hash — e.g. ACP `WriteTextFileRequest` is just `{path, content, - * sessionId}` so the CAS-gated `writeTextAtomic` doesn't fit. + * the edit at `0o600`. New files are created under the factory's + * `NewFileModePolicy` — `0o600` by default, or the umask-derived + * `0o666 & ~umask` under `'system'`. Used by protocols whose wire + * format carries no client-side hash — e.g. ACP `WriteTextFileRequest` + * is just `{path, content, sessionId}` so the CAS-gated + * `writeTextAtomic` doesn't fit. * * Symlinks at the target are rejected (`symlink_escape`) consistent * with `writeTextAtomic` and HTTP `POST /file`. @@ -346,6 +389,14 @@ export interface CreateWorkspaceFileSystemFactoryDeps { pathLocks?: PathMutexRegistry; /** Runtime-generation guard checked at mutation commit points. */ generationGuard?: Pick; + /** + * Mode policy for NEW files created by text writes. Defaults to + * `'owner'` (`0o600`, umask-independent). `'system'` follows the + * daemon process's umask (`0o666 & ~umask`). Production wiring + * derives this from `QWEN_SERVE_NEW_FILE_MODE`; see + * `NewFileModePolicy`. + */ + newFileMode?: NewFileModePolicy; } /** @@ -391,6 +442,7 @@ export function createWorkspaceFileSystemFactory( }); const lowFs = new StandardFileSystemService(); const pathLocks = deps.pathLocks ?? new PathMutexRegistry(); + const newFileMode: NewFileModePolicy = deps.newFileMode ?? 'owner'; const forRequest = (ctx: RequestContext): WorkspaceFileSystem => new WorkspaceFileSystemImpl({ @@ -402,6 +454,7 @@ export function createWorkspaceFileSystemFactory( lowFs, pathLocks, generationGuard: deps.generationGuard, + newFileMode, }); return { @@ -439,6 +492,7 @@ export function createWorkspaceFileSystemFactory( ctx, pathLocks, generationGuard: deps.generationGuard, + newFileMode, }); return; } catch (outsideErr) { @@ -470,6 +524,8 @@ interface SameHostToolTextWriteDeps { ctx: RequestContext; pathLocks: PathMutexRegistry; generationGuard?: Pick; + /** New-file mode policy for the external host-writer route. */ + newFileMode: NewFileModePolicy; } async function writeSameHostToolTextOutsideWorkspace( @@ -492,6 +548,7 @@ async function writeSameHostToolTextOutsideWorkspace( content, mode: 'overwrite', meta, + newFileModeBits: resolveNewFileModeBits(deps.newFileMode), assertGenerationOpen: () => deps.generationGuard?.assertOpen(), }); deps.audit.recordAccess(deps.ctx, { @@ -598,6 +655,8 @@ interface ImplDeps { lowFs: StandardFileSystemService; pathLocks: PathMutexRegistry; generationGuard?: Pick; + /** Mode policy for NEW files created by text writes. */ + newFileMode: NewFileModePolicy; } function assertNoNestedWorkspaces(workspaces: readonly string[]): void { @@ -1171,6 +1230,7 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { mode: opts.mode, expectedHash: opts.expectedHash, meta, + newFileModeBits: resolveNewFileModeBits(this.deps.newFileMode), assertGenerationOpen: () => this.deps.generationGuard?.assertOpen(), }); const verdict = this.ignoreVerdict(p, 'file'); @@ -1274,6 +1334,7 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { content, mode: 'overwrite', meta, + newFileModeBits: resolveNewFileModeBits(this.deps.newFileMode), assertGenerationOpen: () => this.deps.generationGuard?.assertOpen(), }); const verdict = this.ignoreVerdict(p, 'file'); @@ -1411,6 +1472,7 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { mode: 'replace', expectedHash: opts.expectedHash, meta, + newFileModeBits: resolveNewFileModeBits(this.deps.newFileMode), assertGenerationOpen: () => this.deps.generationGuard?.assertOpen(), }); const verdict = this.ignoreVerdict(p, 'file'); @@ -1482,6 +1544,7 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { content: next, mode: 'overwrite', meta: mergeWriteMeta(snapshot.meta, {}), + newFileModeBits: resolveNewFileModeBits(this.deps.newFileMode), assertGenerationOpen: () => this.deps.generationGuard?.assertOpen(), }); const verdict = this.ignoreVerdict(p, 'file'); @@ -1605,6 +1668,13 @@ interface AtomicWriteTextInput { mode: WriteMode; expectedHash?: ContentHash; meta: ReadMeta; + /** + * Mode bits for a NEW target (existing targets always preserve their + * on-disk mode). Undefined falls back to the owner-only `0o600` + * default; callers wired with a `NewFileModePolicy` pass + * `resolveNewFileModeBits(policy)` here. + */ + newFileModeBits?: number; assertGenerationOpen?: () => void; } @@ -2264,6 +2334,7 @@ async function atomicWriteTextResolvedFile( buf, mode: input.mode, expectedHash: input.expectedHash, + newFileModeBits: input.newFileModeBits, assertGenerationOpen: input.assertGenerationOpen, }); } @@ -2280,6 +2351,12 @@ async function atomicPublishResolvedFile(input: { buf: Buffer; mode: WriteMode; expectedHash?: ContentHash; + /** + * Mode bits for a NEW target. Existing targets preserve their on-disk + * mode regardless. Undefined falls back to the owner-only `0o600` + * default (`OWNER_ONLY_NEW_FILE_MODE`). + */ + newFileModeBits?: number; assertGenerationOpen?: () => void; }): Promise { const target = input.target; @@ -2333,7 +2410,13 @@ async function atomicPublishResolvedFile(input: { mode: input.mode, expectedHash: input.expectedHash, }); - await chmodHandleBestEffort(tempHandle, targetState.mode ?? 0o600); + // Existing targets preserve their on-disk mode; NEW targets get the + // caller's policy bits (umask-following `0o666 & ~umask` under the + // `system` policy) or the owner-only `0o600` default. + await chmodHandleBestEffort( + tempHandle, + targetState.mode ?? input.newFileModeBits ?? OWNER_ONLY_NEW_FILE_MODE, + ); await assertTempPathMatchesStat(tmpPath, tempStat); await tempHandle.close(); tempHandle = undefined; diff --git a/packages/cli/src/serve/process-env-guard.test.ts b/packages/cli/src/serve/process-env-guard.test.ts index f642a02b9a5..dcc7eedfada 100644 --- a/packages/cli/src/serve/process-env-guard.test.ts +++ b/packages/cli/src/serve/process-env-guard.test.ts @@ -168,8 +168,9 @@ const allowedProcessEnvAccesses = normalizeAllowances([ 'packages/cli/src/serve/server/fs-factory.ts', { reason: - 'Embedded server construction keeps a process-environment compatibility fallback.', - accesses: { 'computed:IDE_WORKSPACE_PATH_ENV_VAR': 1 }, + 'Embedded server construction keeps a process-environment compatibility fallback, ' + + 'and the new-file-mode policy parser defaults to the daemon process environment.', + accesses: { 'computed:IDE_WORKSPACE_PATH_ENV_VAR': 1, whole: 1 }, }, ], [ diff --git a/packages/cli/src/serve/server/fs-factory.test.ts b/packages/cli/src/serve/server/fs-factory.test.ts index 814d76cc4c7..2125e10872d 100644 --- a/packages/cli/src/serve/server/fs-factory.test.ts +++ b/packages/cli/src/serve/server/fs-factory.test.ts @@ -7,8 +7,19 @@ import { promises as fsp, realpathSync } from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { resolveBoundWorkspacesFromIdeEnv } from './fs-factory.js'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + parseNewFileModePolicy, + resolveBridgeFsFactory, + resolveBoundWorkspacesFromIdeEnv, +} from './fs-factory.js'; + +const mockWriteStderrLine = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/stdioHelpers.js', () => ({ + writeStderrLine: mockWriteStderrLine, +})); + +const isPosix = process.platform !== 'win32'; const scratches: string[] = []; @@ -184,3 +195,130 @@ describe('resolveBoundWorkspacesFromIdeEnv', () => { ).toEqual([realpathSync.native(primary), realpathSync.native(parent)]); }); }); + +describe('parseNewFileModePolicy (QWEN_SERVE_NEW_FILE_MODE)', () => { + // Earlier suites in this file legitimately warn through the same + // helper; reset before AND after so call-count assertions here only + // see this suite's own invocations. + beforeEach(() => { + mockWriteStderrLine.mockClear(); + }); + afterEach(() => { + mockWriteStderrLine.mockClear(); + }); + + it('defaults to owner when unset or empty', () => { + expect(parseNewFileModePolicy({})).toBe('owner'); + expect(parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: '' })).toBe( + 'owner', + ); + expect(parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: ' ' })).toBe( + 'owner', + ); + expect(mockWriteStderrLine).not.toHaveBeenCalled(); + }); + + it('accepts explicit owner spellings', () => { + expect(parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: 'owner' })).toBe( + 'owner', + ); + expect(parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: '0600' })).toBe( + 'owner', + ); + expect( + parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: ' OWNER ' }), + ).toBe('owner'); + expect(mockWriteStderrLine).not.toHaveBeenCalled(); + }); + + it('accepts system case-insensitively with surrounding whitespace', () => { + expect(parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: 'system' })).toBe( + 'system', + ); + expect( + parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: ' System ' }), + ).toBe('system'); + expect(mockWriteStderrLine).not.toHaveBeenCalled(); + }); + + it('rejects unknown values with a warning and keeps the 0600 default', () => { + expect(parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: '0644' })).toBe( + 'owner', + ); + expect( + parseNewFileModePolicy({ QWEN_SERVE_NEW_FILE_MODE: 'everyone' }), + ).toBe('owner'); + expect(mockWriteStderrLine).toHaveBeenCalledTimes(2); + expect(mockWriteStderrLine.mock.calls[0]?.[0]).toContain( + 'QWEN_SERVE_NEW_FILE_MODE', + ); + expect(mockWriteStderrLine.mock.calls[0]?.[0]).toContain('0600 default'); + }); +}); + +describe('resolveBridgeFsFactory env-var wiring (QWEN_SERVE_NEW_FILE_MODE)', () => { + // Guards the seam between the documented env var and the daemon: every + // production call site omits `newFileMode`, so `resolveBridgeFsFactory` + // must derive the policy from `process.env` itself. A regression that + // hard-codes the default here would silently disable the knob while every + // injected-`newFileMode` unit test stayed green. + it('derives the policy from process.env when newFileMode is not injected', async () => { + if (!isPosix) return; + const scratch = await mkScratch(); + const prevEnv = process.env['QWEN_SERVE_NEW_FILE_MODE']; + const prevUmask = process.umask(0o002); + process.env['QWEN_SERVE_NEW_FILE_MODE'] = 'system'; + try { + const factory = resolveBridgeFsFactory({ + boundWorkspaces: [scratch], + trusted: true, + }); + const fs = factory.forRequest({ route: 'TEST /op' }); + const resolved = await fs.resolve('env-wired.txt', 'write'); + const out = await fs.writeTextOverwrite(resolved, 'hello\n'); + expect(out.created).toBe(true); + const st = await fsp.lstat(resolved as string); + // system policy: 0o666 & ~umask(0o002) = 0o664, not the 0o600 default. + expect(st.mode & 0o7777).toBe(0o664); + } finally { + if (prevEnv === undefined) { + delete process.env['QWEN_SERVE_NEW_FILE_MODE']; + } else { + process.env['QWEN_SERVE_NEW_FILE_MODE'] = prevEnv; + } + process.umask(prevUmask); + } + }); + + it('keeps the fail-closed 0600 default when the env var is unset', async () => { + // Mirror half of the seam guard above: with the variable unset the SAME + // production seam must resolve to the fail-closed `owner` policy. A + // regression that makes the unset default resolve to `system` flips + // every agent-created new file to umask-derived modes (0o664 under + // umask 0o002) with no warning — and only this test catches it. + if (!isPosix) return; + const scratch = await mkScratch(); + const prevEnv = process.env['QWEN_SERVE_NEW_FILE_MODE']; + const prevUmask = process.umask(0o002); + delete process.env['QWEN_SERVE_NEW_FILE_MODE']; + try { + const factory = resolveBridgeFsFactory({ + boundWorkspaces: [scratch], + trusted: true, + }); + const fs = factory.forRequest({ route: 'TEST /op' }); + const resolved = await fs.resolve('default-policy.txt', 'write'); + const out = await fs.writeTextOverwrite(resolved, 'default\n'); + expect(out.created).toBe(true); + const st = await fsp.lstat(resolved as string); + expect(st.mode & 0o7777).toBe(0o600); + } finally { + if (prevEnv === undefined) { + delete process.env['QWEN_SERVE_NEW_FILE_MODE']; + } else { + process.env['QWEN_SERVE_NEW_FILE_MODE'] = prevEnv; + } + process.umask(prevUmask); + } + }); +}); diff --git a/packages/cli/src/serve/server/fs-factory.ts b/packages/cli/src/serve/server/fs-factory.ts index 18481d7c6a2..0a5767e371c 100644 --- a/packages/cli/src/serve/server/fs-factory.ts +++ b/packages/cli/src/serve/server/fs-factory.ts @@ -10,6 +10,7 @@ import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; import { canonicalizeWorkspace, createWorkspaceFileSystemFactory, + type NewFileModePolicy, type WorkspaceFileSystemFactory, } from '../fs/index.js'; import type { PathMutexRegistry } from '../fs/path-mutex-registry.js'; @@ -17,6 +18,40 @@ import type { WorkspaceGenerationGuard } from '../workspace-registry.js'; import { isWithinRoot } from '../../config/path-comparison.js'; const IDE_WORKSPACE_PATH_ENV_VAR = 'QWEN_CODE_IDE_WORKSPACE_PATH'; +const NEW_FILE_MODE_ENV_VAR = 'QWEN_SERVE_NEW_FILE_MODE'; + +/** + * Parse `QWEN_SERVE_NEW_FILE_MODE` into the workspace filesystem's + * new-file mode policy. + * + * Accepted values (case-insensitive, surrounding whitespace ignored): + * - unset / empty / `owner` / `0600` → `'owner'` — new files are + * created owner-only `0600` regardless of the daemon umask (the + * default, preserving the long-standing fail-closed posture). + * - `system` → `'system'` — new files follow the standard POSIX + * `0o666 & ~umask` handling, so agent-created files honor the + * daemon process's umask (e.g. a systemd unit's `UMask=0002`) + * like any other process on the machine. + * + * Any other value is rejected with a stderr warning and falls back to + * `'owner'` — a typo in a security-relevant knob must never silently + * widen file visibility. Mode preservation for existing files is + * unaffected by either policy. + */ +export function parseNewFileModePolicy( + env: NodeJS.ProcessEnv = process.env, +): NewFileModePolicy { + const raw = env[NEW_FILE_MODE_ENV_VAR]; + if (raw === undefined || raw.trim() === '') return 'owner'; + const normalized = raw.trim().toLowerCase(); + if (normalized === 'system') return 'system'; + if (normalized === 'owner' || normalized === '0600') return 'owner'; + writeStderrLine( + `qwen serve: ignoring invalid ${NEW_FILE_MODE_ENV_VAR}=${raw} ` + + `(expected 'system' or 'owner'); new files keep the 0600 default`, + ); + return 'owner'; +} /** * Build a no-op fs-audit emitter that logs a warning every @@ -68,6 +103,11 @@ export function resolveBridgeFsFactory(input: { customIgnoreFiles?: string[]; pathLocks?: PathMutexRegistry; generationGuard?: Pick; + /** + * New-file mode policy for the default factory. Undefined reads + * `QWEN_SERVE_NEW_FILE_MODE` (default `'owner'` = `0600`). + */ + newFileMode?: NewFileModePolicy; }): WorkspaceFileSystemFactory { if (input.injected) return input.injected; return createWorkspaceFileSystemFactory({ @@ -76,6 +116,7 @@ export function resolveBridgeFsFactory(input: { emit: input.emit ?? createDefaultFsAuditEmit(), pathLocks: input.pathLocks, generationGuard: input.generationGuard, + newFileMode: input.newFileMode ?? parseNewFileModePolicy(), ...(input.customIgnoreFiles !== undefined ? { customIgnoreFiles: input.customIgnoreFiles } : {}),