Repository navigation
fix(update): admit umask-restricted release payloads and re-sync orca-plugin.json - #2821
Conversation
…-plugin.json `genie update` failed on every host whose umask strips group/other bits (e.g. 077) with "admitted install payload content does not match the authenticated source". `tar -xzf` extracts the Genie binary as 0700 there; admission digested the external payload (mode-bearing digest, 0700), copied it, then fchmod'ed the held copy to 0755 — so the two digests could never agree. Normalize the external binary to 0755 through an owned O_NOFOLLOW descriptor *before* the authenticated digest is taken; the external root is our own private extraction sandbox and the copy was going to be 0755 anyway. Regression test reproduces the exact failure; two existing tests that assumed a permissive umask now chmod the bits they assert. Also bump plugins/genie/orca-plugin.json to 5.260829.8: main's version.yml (the copy workflow_run actually executes) still syncs seven JSON files and leaves orca-plugin.json behind, which is what broke the dev tarball build and this PR's CI. The workflow fix on dev only takes effect once promoted to main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013gGxGgKskzyzr1HRUB6cV3
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f1c2f782b
ℹ️ 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".
| if (expectedVersion === null) { | ||
| throw new InstallPromotionError('external install staging requires an expectedVersion to authenticate VERSION'); | ||
| } | ||
| normalizeExternalBinaryMode(externalStagingRoot); |
There was a problem hiding this comment.
When install.sh runs as a non-root user with umask 077, the extracted binary is 0700; installPromoteCommand records that exact identity at src/genie-commands/install-promote.ts:268, but this call changes its mode to 0755. The verifier at lines 318–324 then compares the mutated file with the pre-normalization identity (which includes mode and digest) and rejects it, so the documented install.sh bootstrap path for users affected by the old updater still fails. Normalize before capturing mutation authority, or refresh the bound identity without weakening the inode/content checks.
Useful? React with 👍 / 👎.
…ves umask 077 genie update / install.sh extracted the signed tarball with a bare 'tar -xzf'. As non-root, tar applies the caller umask to every member, so under umask 077 the archived 0755 'genie' landed as 0700 and every skill, plugin file and script lost its group/other bits too. #2821 made admission tolerate the binary case by normalizing the external binary to 0755 before digesting. This closes the root cause one layer up: extract with 'tar -xzpf' in both extractors so the signed archive's recorded modes are reproduced for the whole payload regardless of the caller's umask. - regression test: extraction keeps 0755/0644 under umask 077 - install.sh extract_and_link body must carry -p (static assertion) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018QrkgYMEEhrWTUo5E7Nkjg
…ves umask 077 (#2824) genie update / install.sh extracted the signed tarball with a bare 'tar -xzf'. As non-root, tar applies the caller umask to every member, so under umask 077 the archived 0755 'genie' landed as 0700 and every skill, plugin file and script lost its group/other bits too. #2821 made admission tolerate the binary case by normalizing the external binary to 0755 before digesting. This closes the root cause one layer up: extract with 'tar -xzpf' in both extractors so the signed archive's recorded modes are reproduced for the whole payload regardless of the caller's umask. - regression test: extraction keeps 0755/0644 under umask 077 - install.sh extract_and_link body must carry -p (static assertion) Claude-Session: https://claude.ai/code/session_018QrkgYMEEhrWTUo5E7Nkjg Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Problem
genie update --dev(5.260816.2 → 5.260829.4) fails on any host whose umask strips group/other bits (e.g.0077):Root cause (
src/lib/install-promotion.ts→admitExternalInstallStaging):tar -xzfextractsgenieas0700under that umask. Admission digests the external payload (the digest bindsmode & 0o777, so it records0700),cpSyncs it into the held stage, thenfchmods the copy to0755— and only then compares digests. They can never agree. The tarballs themselves are fine (./genieis0755in every published release); the bug has shipped since v5.260720.1.Fix
Normalize the external
genieto0755through an ownedO_NOFOLLOWdescriptor before the authenticated digest is taken (normalizeExternalBinaryMode). The external root is our own private extraction sandbox and the copy was always going to be0755, so this changes no security posture — it moves an existing normalization ahead of the hash.Verified against the real
genie-5.260829.4-linux-x64-glibc.tar.gzextracted under umask 077: unpatched module → exact error above; patched → admitted +verifyAdmittedInstallStagingPayloadpasses.Tests
genieat0700→ admission succeeds, both sides end at0755, digest guard holds. Fails ondevwith the exact user-facing error.mkdirSync({mode:0o755}),writeFileSync({mode:0o644})are umask-filtered); they nowchmodthe bits they assert so the suite is umask-independent.Also:
plugins/genie/orca-plugin.json→ 5.260829.8This PR's CI (and the dev tarball build for v5.260829.8) fails with
committed version mismatch in plugins/genie/orca-plugin.json: expected 5.260829.8, got 5.260829.7.version.ymlis aworkflow_runworkflow, so GitHub executes main's copy — which still syncs "the exact seven version fields" and doesn't knoworca-plugin.json(dev's copy from #2812/#2816 lists nine). Every dev bump will keep leaving it behind until main's workflow is updated; #2816 hand-patched .7 for the same reason. This PR hand-patches .8 so CI is green now; the durable fix is promoting the workflow change to main (see companion PR againstmain).Note for operators on affected hosts
The installed 5.260816.2 binary runs the buggy admission itself, so
genie updatefrom it will still fail even after this ships. Useinstall.sh(it promotes via the new binary's__install-promote) to get onto the first release containing this fix; subsequentgenie updateruns are then fine.🤖 Generated with Claude Code
https://claude.ai/code/session_013gGxGgKskzyzr1HRUB6cV3