Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Feb 10, 2026

Summary

The zshProcessSubstitution regex in containsDangerousSubstitution() used =\([^)]+\) which incorrectly matched bash/zsh array assignments like files=(a b c) as dangerous zsh process substitutions like =(whoami).

This caused auto-approve to be bypassed (returning ask_user instead of auto_approve) even when the user had * wildcard in their allowed commands list.

Changes

  • src/core/auto-approval/commands.ts: Added negative lookbehind (?<![a-zA-Z0-9_]) to the regex so that =( preceded by a variable name character (array assignment) is not flagged, while standalone =(cmd) process substitution still is
  • src/core/auto-approval/commands.ts: Updated doc comment to note that array assignments are excluded from detection
  • src/core/auto-approval/__tests__/commands.spec.ts: Added 7 test cases:
    • 3 tests for array assignments that should NOT be flagged (files=(a b c), var=(item1 item2), x=(hello))
    • 3 tests for actual zsh process substitution that SHOULD be flagged (=(whoami), =(ls), echo =(cat /etc/passwd))
    • 1 integration test verifying getCommandDecision returns auto_approve for array assignment commands with * wildcard

Important

Fixes regex in commands.ts to prevent array assignments from being flagged as zsh process substitutions, with updated tests in commands.spec.ts.

  • Behavior:
    • Fixes regex in containsDangerousSubstitution() in commands.ts to exclude array assignments from zsh process substitution detection.
    • Updates doc comment in commands.ts to clarify exclusion of array assignments.
  • Tests:
    • Adds 7 test cases in commands.spec.ts to verify correct detection of array assignments and process substitutions.
    • Includes integration test for getCommandDecision to ensure auto_approve for array assignments with wildcard allowlist.

This description was created by Ellipsis for f20ab30. You can customize this summary. It will automatically update as commits are pushed.

…on detection

The zshProcessSubstitution regex in containsDangerousSubstitution() used
=\([^)]+\) which matched bash/zsh array assignments like files=(a b c)
as if they were dangerous zsh process substitutions like =(whoami).

This caused auto-approve to be bypassed (returning 'ask_user') even when
the user had '*' wildcard in their allowed commands list.

Fix: Add negative lookbehind (?<![a-zA-Z0-9_]) so that =( preceded by a
variable name character (array assignment) is not flagged, while standalone
=(cmd) process substitution still is.
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Feb 10, 2026
@roomote
Copy link
Contributor

roomote bot commented Feb 10, 2026

Rooviewer Clock   See task

Review complete. No issues found. The negative lookbehind fix correctly distinguishes array assignments from zsh process substitutions, and the test coverage is solid.

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant