Repository navigation
Scope Always allow to a command prefix instead of the whole tool - #1666
Conversation
Accept all on a Claude command used to add a whole-tool session rule, so allowing `npm test` also allowed `rm -rf` for the rest of the session. Allow-always now sends a prefix rule (`npm test:*`) derived from the command that runs, or a rule the user typed; the whole tool is an explicit "Allow any <tool> this session" overflow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
💡 Codex Review
solenta/electron/permissionCommand.js
Lines 130 to 131 in 266ff9f
For commands whose operation appears after a dispatcher or global option, this two-token heuristic creates a much broader grant than the approved command: npm run test becomes npm run:*, while git -C repo status becomes git:*. Claude documents that npm run * matches every npm script and that git * permits every git subcommand, so later deploy, publish, push, or other mutating commands can run without another prompt. Parse through dispatcher verbs and global options, or fall back to a narrower exact rule, before appending the wildcard. Claude Code wildcard-rule documentation
solenta/electron/permissionCommand.js
Lines 122 to 124 in 266ff9f
For a command such as FOO=bar npm test, this removes the assignment and stores npm test:*, but Claude only strips a fixed set of known-safe environment variables when matching allow rules; an arbitrary variable prevents that rule from matching. Consequently, clicking “Always allow” for such a command still produces another prompt on the identical next invocation. Only remove variables from Claude's safe allowlist, or retain unknown assignments in the derived rule. Claude Code wrapper and environment matching documentation
ℹ️ 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".
… n / a on top #1666 landed #479 on main with the same shape as this branch's narrowing (alwaysRule, "*" for the whole tool). Take main's backend, IPC and card UI as-is; this branch now only adds the keyboard layer (#477): y / n / a on permission and plan cards, key hints, the composer marker, and tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… n / a on top #1666 landed #479 on main with the same shape as this branch's narrowing (alwaysRule, "*" for the whole tool). Take main's backend, IPC and card UI as-is; this branch now only adds the keyboard layer (#477): y / n / a on permission and plan cards, key hints, the composer marker, and tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Accepting
npm testwith "Accept all" used to add a whole-tool session rule, so every later Bash call in that session ran unasked,rm -rfincluded. Always allow now grantsBash(npm test:*), which is what the CLIs themselves offer. Allowing the whole tool is an explicit choice you have to pick. Closes #479.Changes
npm test:*. The rule is derived from the command that will actually run, so it updates when you edit the command.npm run *, and holds a red Allow any Bash this session button. That button is the only way to get a whole-tool rule.Allowed for session: Bash: npm test (rule: npm test:*).acceptForSessionalready covers only that exact command.Verification
npm run typecheckpasses andnpx vite buildsucceeds.respondPermission, including the governance, guardrails, codex-approvals and orch-merge tests, passes 177/0 after rebasing onto main.npm test -- foosendsruleContent: "npm test:*"and not a whole-tool rule. They also cover a typed rule,*meaning the whole tool, and an empty typed rule falling back to the prefix.threadView,permissionPromptand the newpermissionRuleParitytests pass. Card tests cover the rule following command edits, a typed rule surviving later edits, an empty rule disabling the button, the overflow sending*, andprofile.askhiding every always-allow option.test:electronrun ended at 4738 passing and 19 failing. The failures were in grok MCP registration, codex usage and worktree tests, none of which load these files. Rerun on their own, those files pass 146/0 both on this branch and on an unmodified HEAD.Notes for reviewers
src/permissionRule.tsis a renderer copy ofcommandPrefixinelectron/permissionCommand.js, so the button can show the rule as you type.test/permissionRuleParity.test.tsfails if they drift.<prefix>", which describes more than is actually granted; the real grant is narrower, never wider.npm test && rm -rfmatchesnpm test:*is decided by the Claude CLI, which splits chained commands. Solenta doesn't check it again.claude-opus-5-5 via Claude Code
🤖 Generated with Claude Code