fix(#457): opencode rename pre-create newDir at 0700 before cpSync (close umask 022 timing window) - #483
Conversation
Under standard umask 022, `cpSync(oldDir, newDir, {recursive:true})`
creates the top-level newDir with default umask permissions (0755),
which subsequently fails the opencode-preset 0700 predjection at PHASE 3
wiring with "directory mode must be 0700 — rolling back".
Node 20/22 fs.cp preserves the mode of a pre-existing dest directory
(verified: `mkdirSync(dst,{mode:0o700})` + `cpSync(src,dst)` leaves dst
at 0700 even when src is 0755). Pre-creating newDir at 0700 is a
structural fix — the directory is born at 0700 in a single atomic
mkdirSync call, with no TOCTOU window between creation and the
predjection.
No post-cpSync chmod: that pattern would be TOCTOU-flavored and defeat
the预检's purpose (identity-bound fchmod via opened fd is the only safe
chmod path, and it's already gated by the preset's own `created=true`
branch which callers deliberately opt into).
Non-copresence rename paths (claude-code-cli, codex-sdk, etc.) are
unaffected — those flows do not invoke the opencode-preset predjection,
and mkdirSync at 0700 is functionally equivalent to letting cpSync
create the dir at umask default (all files/subdirs continue to be
copied recursively as before, only the top-level mode differs).
Closes #457.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd483b2857
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Node 20.20.0: `mkdirSync(dst,{mode:0o700})` + `cpSync(src,dst)` leaves | ||
| // dst at 0700 even when src is 0755. Structural fix, no post-cpSync chmod | ||
| // (would be TOCTOU vs the预检's own identity-bound fchmod branch). | ||
| mkdirSync(newDir, { mode: 0o700, recursive: false }); |
There was a problem hiding this comment.
Run the OpenCode rename Docker E2E before merging
This permission change directly affects the OpenCode rename path exercised by test384 L8, but the commit contains no test/report update and the existing docs/tests/report-test384.txt predates this change and stops after L7. Without running the full Docker flow, the actual anet node rename interaction with profile validation, binding migration, and rollback remains unverified; run the relevant Docker E2E suite and record its passing result before merging.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
|
独立复核结论:PASS。 复核方逐条核实,未采信作者描述:
复核笔记留存于 合入后 #457 关闭,发版门禁的这一项解除。 |
Summary
Fixes #457 — under standard umask 022 (Docker/CI/most shells),
anet node renameon any opencode-cli node fails PHASE 1 with:Root cause (
agent-network/bin/cli.ts:5911):cpSync(oldDir, newDir, {recursive:true})creates the top-levelnewDirwith default umask permissions (0755 under umask 022). The subsequent opencode-preset predjection (assertPrivateDirectoryatopencode-preset.ts:190-191) rejects any mode other than 0o700 — throw and rollback.Fix (
+8/-0lines, one-hunk): pre-createnewDirwithmkdirSync(newDir, {mode:0o700, recursive:false})beforecpSync. Node 20/22fs.cppreserves the mode of an existing dest directory (validated empirically; see evidence). No post-cpSync chmod (that pattern would be TOCTOU-flavored and defeat the预检's purpose).Test plan
Verify 1 — witnessed_red → green (cpSync semantics)
cpSync(0755-src, dst_not_exist)→ dst = 0755 (repros [bug][runtime][P1] opencode node rename unusable under umask 022 — rename 自建 workDir 过不了自家 0700 preset 预检(PHASE 1 自拒回滚) #457)mkdirSync(dst,{mode:0o700})+cpSync(0755-src, dst_existing)→ dst = 0700 ✓Verify 2 — 5 fast + 5 slow (bash -x) iterations
Verify 6 — 0755 source directory (real-world scenario for pre-fix nodes)
Verify — rollback safety (
mkdirSyncEEXIST)Verify — non-opencode regression (analytical + semantic)
grep -rn 0o755on agent-network src returns only descriptive comments (no code path asserts newDir mode == 0755)Verify 3 — test384 full L8⚠️ NOT ATTEMPTED
Verify 4 — Docker only ✓
p457-baseline:test(test384 Dockerfile), never touched host. Umask 022 explicitly printed in evidence.Evidence
Evidence tar.gz:
/tmp/p457-evidence-20260729T131528Z.tar.gzsha256:
668e7fa3fc8a45824e8e2f758f0a3cb3a6a78758b056df5f9bfb998f806f7ffbContents:
00-provenance-manifest.md— HEAD SHA, tree hash, status--porcelain=0, image sha256, test switches (none)cpsync-semantics.log— direct Node fs.cpSync mode preservation proofiterations-fast-slow.log— 5+5 iteration matrix baseline vs fixedrollback-test.log— EEXIST rollback safetyregression-non-opencode.log— non-opencode paths unaffectedrename-repro.sh/rename-repro-with-hub.sh— full-flow repros (hub setup flake noted)Timing window note
The issue reports "run1/run3 fail, run2 (bash -x) pass" — my BASELINE cpSync repro shows deterministic 0755 (5/5 fast + 5/5 slow all fail same way). The reported inconsistency was likely anet-side (a race between the failed rename and OTHER chmod code paths); my structural fix eliminates any race by making the dir 0o700 at creation atomically. Fix is race-free by design.
AWAITING INDEPENDENT VALIDATION
Do NOT merge without 通信龙 sign. Fork does not have merge authority. Independent validator (通信龙 route, ~30min turnaround per commitment) will run full E2E in their environment; PR merges only after their PASS + coordinator co-sign.
Closes #457.
🤖 Generated with Claude Code