Repository navigation
fix(launcher): verify PATH ancestry and reject temporary app bundles - #7022
Conversation
Validate original and resolved POSIX PATH ownership, modes and symlink hops before executing a Bun candidate. Reject raw and canonical temporary app bundles while retaining permanent installations beneath ordinary TMPDIR overrides. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
|
✅ Deterministic PR hygiene checks passed. |
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. |
There was a problem hiding this comment.
🔇 Additional comments (8)
desktop/src-tauri/src/cli_command_posix.rs (1)
97-97: LGTM!Also applies to: 108-108, 129-129, 138-138, 196-196, 197-197, 198-198, 206-206, 254-254
structure/desktop-terminal-command.md (1)
30-33: LGTM!src/lib/bun-path-runtime.mjs (3)
35-38: Handle..after a symlink expansion with the parent of the expanded path.Line 36 sets
currenttoposix.dirname(current). A symlink is expanded into components that restart from/, socurrentis the physical walked path at that point. POSIX semantics resolve..against the physical directory, and this code does the same. The walk therefore matches kernel resolution for... Line 69 also checks the walked path againstrealpath. A divergence would cause rejection, so the check fails closed.
114-123: LGTM!
21-75: 🎯 Functional CorrectnessThe repository defines rejection of group/world-writable ancestors, including shared sticky directories, as intentional. The comment’s Homebrew/Nix examples do not establish a supported layout that violates this policy. No change is supported by the inspected evidence.
src/lib/bun-path-runtime.d.mts (1)
8-17: LGTM!structure/runtime.md (1)
123-123: LGTM!tests/ci-workflows/bun-runtime.test.ts-28-47 (1)
28-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Duplicate fixture-root helper.
trustedPathRoot(Lines 35-46) repeatspathFixtureRootintests/cli/ocx-launcher-runtime.test.ts(Lines 398-406). Both helpers use the same base path and mode. Extract one shared helper intotests/helpers/so that the two trust assumptions cannot drift apart.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
d896d8aa-d6bb-4f9a-8443-7d86b0ad5d9f
📒 Files selected for processing (7)
desktop/src-tauri/src/cli_command_posix.rssrc/lib/bun-path-runtime.d.mtssrc/lib/bun-path-runtime.mjsstructure/desktop-terminal-command.mdstructure/runtime.mdtests/ci-workflows/bun-runtime.test.tstests/cli/ocx-launcher-runtime.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a40fb0370
ℹ️ 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".
|
Thanks @luvs01! Reviewed: PATH Bun fallback now validates every original and symlink-expanded ancestor (root/effective-user owned, not group/world writable), and Desktop refuses persistent terminal-command registration from temporary or translocated bundles. Users with a group-writable PATH ancestor can still set OPENCODEX_BUN_PATH; installation docs explain the rule. Maintainer integration into |
Summary
TMPDIRdoes not by itself make a permanent installation temporary.48c6928e75697ab2d10888bc69ae6b3f6370e463ontodevcommit6da2774369aa32552e318e333902499683d053bc, preserving the launcher contract and regression coverage.Verification
bun test --isolate --parallel=1 tests/cli/ocx-launcher-runtime.test.ts tests/ci-workflows/bun-runtime.test.tsinitially produced 87 pass, 6 POSIX-only skip and 6 timeout failures with the default 5-second test limit.--timeout 60000. Re-running the affectedNode launcher PATH fallback activationgroup with that same setting passed all 7 cases. Product probe deadlines and test assertions were unchanged. All 93 applicable cases therefore passed across these scoped runs; this is not a claim of one clean full-suite run.bun run typecheck,bun run structure:check, andgit diff --checkpassed.bun run privacy:scanpassed for the materialized tracked files, including every changed file; the local checkout is sparse.Checklist
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Summary by CodeRabbit
PATHnow undergo stricter checks of file ownership, permissions, and symlink paths before they can be used.TMPDIRlocations. Ordinary application folders remain eligible with unrelatedTMPDIRoverrides.