fix: merge latest dev updates into main

[nadav-node9]

Auto-generated PR

Merge latest dev changes into main to trigger a release.

⚠️ Important: When you click Squash and Merge, ensure the commit message starts with:

• fix: to publish a Patch release (0.0.X)
• feat: to publish a Minor release (0.X.0)
  If it starts with chore:, no NPM package will be published!

[github-actions]

🤖 Claude Code Review

Review

Semicolon regex change — correctness concern (high priority)

The most significant change is loosening ;\\s*\\S to just ; in the chain-command guard. The original pattern required a non-whitespace character after the semicolon (i.e., it only blocked ; when followed by an actual command). The new pattern blocks any semicolon, including:

• for i in 1 2 3; do echo $i; done — legitimate shell loops
• echo "hello; world" — semicolons in string arguments
• git commit -m "fix: edge case; closes #123"

This will generate significant false positives on the critical path, where every approval prompt adds friction. The original intent was to catch chained commands, not semicolons in general. Consider at minimum ;\\s*\\S+ or anchoring to end-of-token boundaries. A test covering the loop/string cases should be added before merging.

dangerousWords: ["rm", ...] — overly broad (medium priority)

Adding "rm" as a dangerous word will match substrings like chmod, fromenv, npm, yarn remove, terraform, etc., depending on how dangerousWords matching is implemented. If it's a simple includes() or regex without word boundaries, this is a high false-positive risk. Needs a word-boundary guard (\brm\b) or explicit documentation that the implementation already handles this.

Secrets rule rename — minor, but test coverage gap

The rename from review-secrets-write to flag-secrets-access and broadening to read operations is reasonable. The new .env.bak test is a good addition. However, there's no test covering:

• A read operation on .env (e.g., read_file with file_path: '/project/.env') — the primary motivation for the rename
• source .env via bash — would this be caught by a different rule or fall through?

Type safety / test alignment

The test regex change to '(&&|\\|\\||;)' must match the config example exactly — they do here, which is good. No drift observed.

What looks good

• Renaming the rule and updating reason string is clean and accurate
• The notmy.env.bak test comment clarifying the anchor behavior is helpful
• Config and test changes are kept in sync

Summary

The semicolon regex change is the blocking issue — it trades a real bypass risk for a high false-positive rate on legitimate commands. The rm dangerous word needs boundary-check verification. Everything else is acceptable to merge with the caveats noted.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Review

Semicolon regex change — correctness concern (high priority)

The change from (&&|\|\||;\s*\S) to (&&|\|\||;) is overly broad. A bare ; now blocks legitimate inputs like for i in a b c; do, heredocs, or any semicolon in a string argument (e.g., echo "foo; bar"). The original pattern required at least one non-whitespace character after ;, which was a reasonable heuristic to distinguish chained commands from trailing semicolons. This lookahead was imprecise but the new version is strictly worse — it will produce false positives on common shell idioms, degrading UX and trust in the tool. Needs discussion before merging.

dangerousWords: ["rm", ...] — false positive risk (medium)

Adding "rm" as a dangerous word will match substrings in benign tool arguments: remove, firmware, npm, yarn remove, permissions, etc., depending on how dangerousWords matching is implemented. If it's a substring match rather than a word-boundary match, this will be noisy. Verify the matching logic in core.ts uses \brm\b or equivalent, and document the intent here.

Rule rename is fine

review-secrets-write → flag-secrets-access is semantically correct given the broadened scope to reads. No issues. Reason string update is accurate.

New test — .env.bak coverage

The new test for .env.bak is a good addition. However, there's still no test covering read operations on secrets files (e.g., cat /project/.env), which is the primary motivation for renaming the rule from write to access. That gap should be filled.

Missing negative test

There's no test asserting that rm in dangerousWords does not flag firmware_update or permissions. Given the substring risk above, add one.

Config example / test symmetry

The regex in the example config and in the test are now in sync — good. But the config example has no corresponding test for the && and || cases post-change; those pre-existing tests should still pass and appear to, so this is fine.

Summary

Two issues need resolution before merge:

1. The bare ; regex is too broad and will cause false positives on legitimate shell syntax.
2. The rm dangerous word needs verification of word-boundary matching and a corresponding negative test.

---

Automated review by Claude Sonnet

[nadav-node9]

🎉 This PR is included in version 1.0.11 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀