feat(daemon): make serve new-file mode configurable (QWEN_SERVE_NEW_FILE_MODE) - #9364
Conversation
…ILE_MODE)
qwen serve's atomic text writers created every NEW file at 0600
unconditionally, ignoring the daemon process umask with no way to opt
out (issue reporter runs the daemon under a systemd UMask=0002 drop-in
and every agent-created file diverged from the group-readable repo
convention).
Add a NewFileModePolicy ('owner' = 0600 default, 'system' = standard
0o666 & ~umask) on createWorkspaceFileSystemFactory, threaded through
writeTextAtomic / writeTextOverwrite / edit / editAtomic and the
same-host external tool-write route. resolveBridgeFsFactory reads
QWEN_SERVE_NEW_FILE_MODE ('owner' | '0600' | 'system',
case-insensitive); unrecognized values warn on stderr and keep the
fail-closed 0600 default.
Existing-file mode preservation is unchanged, binary uploads stay
0600, and the default behavior is bit-for-bit unchanged.
Closes #9250
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 1c": none — but note I did not execute the new test suites (left to gate/verification stages); my checks above are static cross-file traces..
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/server/fs-factory.ts:110 — [review] newFileMode option is declared and read but never set by any caller (dead switch)docs/developers/daemon/17-configuration.md:68 — [review] QWEN_SERVE_NEW_FILE_MODE missing from 20-quickstart-operations.md §4 env-var tabledocs/developers/daemon/07-workspace-filesystem.md:195 — [probe] writeTextAtomic sequence diagram still asserts the unconditional 0o600 new-file default
中文说明
未探索到全部深度(达到工具调用预算):"agent 1c":none — but note I did not execute the new test suites (left to gate/verification stages); my checks above are static cross-file traces.。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
…'s docs and wiring - process-env guard: register the whole-object process.env access in fs-factory.ts (the parseNewFileModePolicy default parameter) so the serve process.env guard suite passes — the PR-caused CI failure. - resolveNewFileModeBits: read the umask lazily only when the 'system' policy consumes it; the default 'owner' path no longer issues two umask(2) syscalls per write. - docs: state that the literal `0600` is an alias for `owner` (no other octal modes) in both tables; correct the binary-upload route to POST /file/upload; replace the phantom per-write mode-override clause with the factual statement that agents cannot pass one. - test: pin the resolveBridgeFsFactory env seam — with newFileMode uninjected, the policy must come from process.env.QWEN_SERVE_NEW_FILE_MODE (regression-mutates to a hard-coded default are now caught).
|
Closeout for the round-1 findings, pushed in a5a63ac:
Verified: env-guard + fs-factory suites 15/15, workspace-file-system suite green (one pre-existing failure is a root-user environment artifact — the test chmods dirs to 0000 and uid 0 bypasses mode bits; it fails identically without this change), changed files eslint + prettier clean. Deferred to follow-ups (replied in-thread): the boot-warning dedup and the resolved-policy log trail — both need module-level state in the shared parser plus a test reset hook, so they belong in a focused change rather than this closeout pass. All eight threads resolved. |
R2-1: the daemon boot path loads the primary workspace .env into process.env before any fs factory is built, and the new-file-mode key was not in PROJECT_ENV_HARDCODED_EXCLUSIONS — a project-controlled file could flip the documented fail-closed 0600 posture to umask-derived modes daemon-wide with no warning (system is a valid value), widening the visibility of agent-created files on a multi-user host. Register it as a process-scoped operator knob like the other daemon posture keys, with a security test pinning the exclusion.
|
Round-2 closeout (the 19:48Z re-review ran against the pre-fix head; everything below landed after it):
All sixteen threads across both rounds resolved. |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/server/fs-factory.ts:50 — [review] carried-forward R1-2: invalid-value warning fires once per factory construction, not once per daemon (deferred by author)packages/cli/src/serve/server/fs-factory.ts:45 — [review] carried-forward R1-6: effective policy leaves no log trail (deferred by author)
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
…geFsFactory seam The env-wiring test only covered the 'system' half of the seam; the unset-env default (owner -> 0600) had no coverage through the same production path — a regression making the unset default resolve to 'system' would flip every agent-created new file to umask-derived modes with no test failing (mutant verified surviving all 13 prior tests; this mirror test fails it with 0o664 vs 0o600).
|
Bot-review round closeout (pushed 3144475): added the missing fail-closed mirror half of the resolveBridgeFsFactory seam test — env unset → owner → 0600 through the production seam, flip-verified against the 'system' mutant (expected 436 to be 384). fs-factory suite 13/13, prettier + eslint clean. Thread resolved. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
Released in v0.21.14. |
What this PR does
Adds a daemon-level opt-in that lets
qwen servetext writes create NEW files with the standard umask-derived mode instead of the hard-coded owner-only0600. A newNewFileModePolicy('owner'|'system') is threaded from theWorkspaceFileSystemFactorydeps through every text-write publish path (writeTextAtomic,writeTextOverwrite,edit,editAtomic, and the same-host external tool-write route). The shared production wiring (resolveBridgeFsFactory) reads the policy fromQWEN_SERVE_NEW_FILE_MODE(owner/0600/system, case-insensitive); unrecognized values warn on stderr and keep the fail-closed0600default. Existing-file mode preservation, binary uploads (writeBytesAtomic), and the default behavior are all unchanged.Why it's needed
qwen serve's atomic writers created every NEW file at0600unconditionally — the process umask was never consulted and no setting, flag, or env var existed to change it (#9250). Operators running the daemon under a supervisor that setsUMask=(e.g. a systemd drop-in withUMask=0002) saw every agent-created file diverge from the group-readable convention of the rest of the machine, with no escape hatch other than post-write chmod hooks. The0600default stays (it is a defensible fail-closed posture); this PR adds the missing user override requested in the issue, in the env-var form the issue explicitly accepts.Reviewer Test Plan
How to verify
Unit/integration coverage was added rather than a live-daemon repro, because the mode is asserted on real files on disk through the actual publish path:
cd packages/cli && npx vitest run src/serve/fs/workspace-file-system.test.ts src/serve/server/fs-factory.test.ts— new suiteWorkspaceFileSystem - new-file mode policypins: default policy stays0o600even underumask 002;systempolicy creates at0o666 & ~umask(0664 under umask 002, 0644 under umask 022) acrosswriteTextOverwrite,writeTextAtomic, and the same-host external route; existing targets keep their mode undersystem;writeBytesAtomicstays0o600undersystem.parseNewFileModePolicypins env parsing incl. the stderr warning fallback for unknown values.QWEN_SERVE_NEW_FILE_MODE=systemandumask 0002creates agent-written new files at664(stat -c '%a'); without the variable the mode remains600.npx tsc --noEmitinpackages/clipasses;eslint+prettier --checkpass on all touched files.Note:
workspace-file-system.test.ts > multi-root workspaces > throws AggregateError when every workspace root glob failsfails in this sandbox because it runs as root (chmod0000cannot deny root); confirmed pre-existing by re-running it with this PR's changes stashed.Evidence (Before & After)
N/A (non-UI change; behavior is file-mode bits on daemon-written files)
Tested on
Environment (optional)
Node v22.20.0, unit/integration tests only (
vitest), no live daemon session.Risk & Scope
systemmode makes agent-created files group/world readable when the daemon umask is permissive — that is the operator's explicit opt-in; the default remains0600and invalid env values fail closed with a warning.BridgeClient.writeTextFilefallback proxy (Mode A / channels / IDE companion) intentionally keeps its documented0600posture; thesettings.jsonkey form suggested in the issue was not added since the issue accepts an env var; binary uploads stay0600.Linked Issues
Closes #9250
中文说明
这个 PR 做了什么
为
qwen serve增加一个 daemon 级开关:文本写入创建新文件时可以用标准的 umask 推导权限,替代原先硬编码的 owner-only0600。新增NewFileModePolicy('owner'|'system'),从WorkspaceFileSystemFactorydeps 一路贯穿所有文本写入发布路径(writeTextAtomic、writeTextOverwrite、edit、editAtomic以及同宿主外部工具写路由)。生产公共接线(resolveBridgeFsFactory)从QWEN_SERVE_NEW_FILE_MODE读取策略(owner/0600/system,大小写不敏感);无法识别的值会在 stderr 告警并保持 fail-closed 的0600默认值。已有文件的权限保留、二进制上传(writeBytesAtomic)以及默认行为均不变。为什么需要
qwen serve的原子写入器此前无条件把每个新文件创建为0600——从不查询进程 umask,也没有任何配置项、flag 或环境变量可以修改(#9250)。在设置了UMask=的 supervisor 下运行 daemon 的运维者(例如 systemd drop-in 里UMask=0002)会看到 agent 创建的每个文件都与机器上其余部分「组可读」的约定不一致,除了事后 chmod 钩子外没有任何办法。0600默认值保留(这是合理的 fail-closed 姿态);本 PR 按 issue 明确接受的环境变量形式补上缺失的用户覆盖能力。审阅者测试计划
如何验证
用单元/集成覆盖代替真实 daemon 复现,因为权限位是在真实发布路径上对真实落盘文件断言的:
cd packages/cli && npx vitest run src/serve/fs/workspace-file-system.test.ts src/serve/server/fs-factory.test.ts——新套件WorkspaceFileSystem - new-file mode policy固定:默认策略在umask 002下仍为0o600;system策略按0o666 & ~umask创建(umask 002 → 0664,umask 022 → 0644),覆盖writeTextOverwrite、writeTextAtomic与同宿主外部路由;system下已有目标权限保留;writeBytesAtomic在system下仍为0o600。parseNewFileModePolicy固定环境变量解析(含未知值告警回退)。QWEN_SERVE_NEW_FILE_MODE=system且umask 0002启动的 daemon,agent 写出的新文件为664(stat -c '%a');不设该变量时仍为600。packages/cli下npx tsc --noEmit通过;所有改动文件eslint+prettier --check通过。注意:
workspace-file-system.test.ts > multi-root workspaces > throws AggregateError when every workspace root glob fails在本沙箱失败,原因是以 root 运行(chmod0000无法拒绝 root);已通过 stash 掉本 PR 改动后重跑确认为存量问题。前后对比证据
N/A(非 UI 变更;行为是 daemon 写文件的权限位)
测试环境
环境(可选)
Node v22.20.0,仅单元/集成测试(
vitest),未起真实 daemon 会话。风险与范围
system模式下,若 daemon umask 宽松,agent 创建的文件会变为组/全局可读——这是运维者显式选择的结果;默认仍为0600,非法环境变量值会 fail closed 并告警。BridgeClient.writeTextFile内联回退代理(Mode A / channels / IDE companion)有意保留其文档化的0600姿态;未新增 issue 建议的settings.json配置键(issue 已接受环境变量形式);二进制上传保持0o600。关联 Issue
Closes #9250