Repository navigation
fix(cli): refuse non-object settings; only auto-tighten the default ccxray home - #702
Conversation
…t ccxray home - setup-statusline: a settings.json that parses to null, an array, a number or a string is refused like unparseable JSON (same one-line message, no write, no stack trace); planUninstall and isStatuslineInstalled share the guard. A non-object statusLine is no longer spread into character keys. - tightenCcxrayHome chmods 0700 only the default ~/.ccxray or a custom CCXRAY_HOME that already holds ccxray files (local-secret, hub.json, hub.log); any other custom home gets a one-line warning and keeps its mode. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…rkers Co-Authored-By: Claude Opus 5.5 <[email protected]>
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: 0bccbc45cb
ℹ️ 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 (!isDefault(home) && !holdsCcxrayFiles(home)) { | ||
| if (log) log(`[ccxray] warning: CCXRAY_HOME ${home} is group/world-accessible (${fmt(st.mode & 0o7777)}); ccxray stores secrets and logs there — run chmod 700 on it if it is not shared`); | ||
| return 'warned'; | ||
| } |
There was a problem hiding this comment.
When CCXRAY_HOME is a shared directory, the first run warns and keeps its mode, but auth.readOrCreateEphemeralSecret() then creates local-secret there. On every subsequent invocation this condition treats that self-created file as proof that the entire directory belongs to ccxray and executes chmod 0700, locking other users out of the shared directory—the exact custom-home scenario this change is intended to preserve. Custom homes should continue to warn rather than become eligible for chmod merely because ccxray has used them once.
Useful? React with 👍 / 👎.
…ust be an object - tightenCcxrayHome chmods only the default ~/.ccxray. The local-secret / hub.sock marker exception is gone: ccxray writes local-secret into whatever home it is given, so on the second run a shared dir (or $HOME) became chmod-eligible. A loose custom home now warns on every startup and keeps its mode, and $HOME or any ancestor is refused outright. - setup-statusline / launch prompt / --dry-run: null, [] and other non-object JSON report "must contain a JSON object" instead of "is not valid JSON". - Tests: a pre-existing 0755 custom home stays 0755 after ccxray creates its secret and runs again; CCXRAY_HOME=$HOME is never chmod'ed; the default home still tightens; a PTY launch with non-object settings refuses without asking, rewriting, or writing the declined marker (fails when the launchPromptState refusal is removed from index.js). Co-Authored-By: Claude Opus 5.5 <[email protected]>
tightenCcxrayHome compared lexical paths while statSync/chmodSync follow symlinks, so `~/.ccxray -> <shared 0755 dir>` or `~/.ccxray -> .` let startup chmod the target (including $HOME) to 0700. - lstat the home first; a symlinked default home is warning-only - refuse $HOME/ancestors by canonical (realpath) as well as lexical path, and require realpath(home) === realpath(homedir)/.ccxray to tighten - chmod via an O_NOFOLLOW|O_DIRECTORY descriptor whose dev/ino must match the lstat result, closing the check-then-chmod symlink swap window Regression tests: symlink to a shared dir, `.ccxray -> .` (unit + startup); all three fail on the previous paths.js. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 participant
Summary
Follow-ups to #695 from its security review.
setup-statuslinerefuses non-object settings. Asettings.jsonthat parses tonull, an array, a number, a string or a boolean is refused like unparseable JSON (same one-line message, no write, no stack trace) on install,--removeand the launch prompt. The launch prompt now uses the same reader as setup instead of re-parsing, so it refuses before asking. A non-objectstatusLineis no longer spread into character keys, and a non-stringcommandis not reported as invalid JSON.tightenCcxrayHomechmods 0700 the default~/.ccxray, or a customCCXRAY_HOMEthat already holds a ccxray-unique marker (local-secretorhub.sock, as a regular file/socket owned by the current user, symlinks ignored). Any other custom home gets a one-line warning and keeps its mode, so a shared dir, mount or$HOMEis never locked to 0700.Verification
home-permissions,statusline-setup,setuptests: 129 pass, 4 skipped, 0 fail.