feat: steering-first surface — change-17 user-input phase, change-18 QoL steering packs, change-7 claude-code adapter - #15
Conversation
📝 WalkthroughWalkthroughChangesThis PR adds user-input phase handling to the engine and matcher, introduces nine built-in YAML rule packs (git-safety, modern-cli, package-manager, encryption-tools, env, hardening, private-key, secret-managers, sops) with supporting predicates and a pack loader exported from the package index, adds a Claude Code hook adapter (normalization, response translation, engine wiring, CLI entry) with tests, and adds a gallery-generation script producing docs/packs.md plus a CI step that verifies the doc stays synchronized with pack definitions. Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Stdin
participant main
participant handleHookEvent
participant normalizeHookPayload
participant Engine
participant toHookResponse
Stdin->>main: raw JSON payload
main->>handleHookEvent: rawPayload, engine
handleHookEvent->>normalizeHookPayload: payload
normalizeHookPayload-->>handleHookEvent: ToolCallContext | null
handleHookEvent->>Engine: evaluate(ctx)
Engine-->>handleHookEvent: GuardrailAction | null
handleHookEvent->>toHookResponse: action, payload
toHookResponse-->>main: ClaudeCodeHookResponse
main-->>Stdin: JSON response
Related PRs: None identified from the provided context. Suggested labels: enhancement, documentation, ci Suggested reviewers: None identified from the provided context. Poem: 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/rule-pack-guide.md (1)
202-210: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winScope this override example to overridable rules.
The docs above say built-in rules can be locked with
overridable: false, so “any built-in rule” is inaccurate here. Narrow the example to user-overridable rules only.♻️ Proposed fix
-You can disable or change any built-in rule in `agent-guardrails.json`: +You can disable or change any overridable built-in rule in `agent-guardrails.json`:🤖 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 `@docs/rule-pack-guide.md` around lines 202 - 210, The override example in the rule pack guide is too broad because it says “any built-in rule” even though some built-in rules can be locked with overridable: false. Update the example text near the agent-guardrails.json snippet to refer only to user-overridable or overridable built-in rules, keeping the example aligned with the documented behavior.src/index.ts (1)
36-46: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStale "both phases" doc comments after adding the
user-inputphase.The re-exported JSDoc for
GuardrailAction("Union of all possible guardrail actions across both phases") andGuardrailRule("Union of rules from both phases") wasn't updated when theuser-inputphase was added.src/core/types.tsalready updated its own doc comments on these same types to say "all phases" — these public re-export comments in the barrel file are now inconsistent with the source and understate the union (it'sbefore-tool/after-tool/user-input, i.e., three phases, not two).📝 Proposed fix
- /** Union of all possible guardrail actions across both phases. */ + /** Union of all possible guardrail actions across all phases. */ GuardrailAction, /** Normalized context for a tool call. */ ToolCallContext, /** A declarative condition that describes when a rule fires. */ MatchCondition, - /** Union of rules from both phases. */ + /** Union of rules from all phases. */ GuardrailRule,🤖 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/index.ts` around lines 36 - 46, Update the barrel re-exports in src/index.ts so the JSDoc for GuardrailAction and GuardrailRule matches the current phase model; the comments still say “both phases” and are stale after adding user-input. Align these public docs with the source types in core types by changing the wording to refer to all phases, so the exported API comments correctly reflect before-tool, after-tool, and user-input.
🤖 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 @.github/workflows/ci.yml:
- Around line 25-26: The check job in the CI workflow is using the default
unscoped GITHUB_TOKEN permissions, so add an explicit least-privilege
permissions block for this job or at the workflow level. Update the workflow
around the check job and the new docs/packs.md verification step so it only
requests the minimal read-only access needed, with no write permissions to
contents, issues, or pull requests.
In `@docs/adrs/002-behavior-model.md`:
- Around line 23-33: The behavior model text incorrectly says `block` and
`suggest` work everywhere, which conflicts with the phase matrix and
`validateRule()`. Update the ADR wording in the section defining the three
phases so `block` is described as the safe baseline across all phases, while
`suggest` is only mentioned for `before-tool`; keep the `user-input` and
`after-tool` allowed behaviors aligned with the existing matrix and ADR-010.
In `@docs/adrs/003-public-api-contract.md`:
- Around line 29-32: The public contract entry for Engine.evaluate should not
mention ctx.phase because phase is derived internally from toolName and is not
part of the runtime API shape. Update the ADR table row for Engine.evaluate to
describe the method as taking ToolCallContext without exposing phase, and keep
the wording aligned with the Engine.evaluate and ToolCallContext symbols so the
contract matches the implementation.
- Around line 17-25: The bootstrap example in the public API contract is not
self-contained because it uses registry before defining it. Update the snippet
around createEngine and loadAllRulePacks to either include the registry
initialization in the example or remove the custom-registry argument from
loadAllRulePacks so the code can be pasted and run as shown.
In `@docs/adrs/010-user-input-mediation.md`:
- Around line 27-33: The user-input contract description is inconsistent with
the implementation and should be updated to match the actual symbols and shape
used by ToolCallContext and HarnessCapabilities. In the ADR, replace the “phase:
user-input” wording with the real tool context field naming (`toolName:
'user-input'`) and describe the raw prompt as living in `command`; also note
that `redactUserInput` is optional and defaults to false as defined in
HarnessCapabilities. Keep the matcher guidance aligned with the actual contract
so adapter authors are not told that bash-command matchers are excluded if that
is not true.
In `@README.md`:
- Around line 44-49: The README example uses loadAllRulePacks("./packs",
registry) before defining registry, so make the snippet copy-paste runnable by
adding the PredicateRegistry setup in the example or replacing it with a
self-contained placeholder-free version. Update the
createEngine/loadAllRulePacks example so registry is introduced before it is
passed in, keeping the sample focused on the README usage shown in the import
and engine.evaluate flow.
In `@src/adapters/claude-code/adapter.test.ts`:
- Around line 1-169: Add tests to cover the untested startup failure path in
main() by making createClaudeCodeEngine() or pack loading throw and asserting
the startup behavior is fail-closed as expected, and extend
normalizeHookPayload() coverage for Write/Edit/Read inputs that omit file_path
so the normalized output explicitly handles the missing path case; use the
existing adapter.test.ts patterns around handleHookEvent,
createClaudeCodeEngine, and normalizeHookPayload to place the new cases.
In `@src/adapters/claude-code/hook.ts`:
- Around line 33-55: `main()` currently leaves stdin reading and
`createClaudeCodeEngine()` outside any fail-closed handling, so errors can
escape without producing a block response. Wrap the full CLI flow in
`main()`—including the `for await (const chunk of process.stdin)` loop,
`createClaudeCodeEngine`, and `handleHookEvent`—in a top-level try/catch. On any
exception, write the same structured blocked decision JSON to stdout before
exiting, so the Claude Code hook always fails closed.
- Around line 47-51: The stdin read in main currently waits indefinitely for
EOF, which can stall the hook when input stays open. Add a timeout around the
process.stdin chunk collection so the read fails closed if the payload does not
arrive in time, and make sure the timeout path aborts the hook cleanly with a
clear error.
In `@src/adapters/claude-code/respond.ts`:
- Around line 68-94: The inline comment in userPromptResponse is hard to
understand and should be rewritten for clarity. Update the note around the
redact branch in userPromptResponse to plainly state that user-input only
supports redact, block, and confirm, and that scrubPrompt is applied here when
payload.prompt is present. Keep the explanation brief and directly tied to the
redact case so the intent is obvious when reading the ClaudeCodeHookResponse
mapping.
In `@src/core/types.ts`:
- Around line 30-33: The confirm fallback in UserInputAction still allows
BeforeToolAction recursively, which lets suggest/run actions slip through
user-input validation. Update the UserInputAction definition and the
isUserInputAction() validation path to narrow the nested confirm.fallback type
for user-input rules, or explicitly reject suggest and run when validating
confirm fallbacks. Use the UserInputAction and isUserInputAction symbols to
locate the affected logic.
In `@src/core/validator.ts`:
- Around line 59-65: The user-input confirm fallback is too permissive because
isUserInputAction still routes non-redact actions through isBeforeToolAction,
which lets suggest/run and nested confirm slip through. Update the validation
logic in isUserInputAction to treat confirm.fallback as block-only for
user-input actions, and make malformed fallback shapes fail validation at load
time rather than being accepted by isBeforeToolAction. Keep the fix localized to
the validator flow around isUserInputAction and any confirm-related type checks
it relies on.
In `@src/engine/engine.ts`:
- Around line 51-53: The nested ternary in the command derivation logic is hard
to read and should be refactored for clarity. Extract the logic around the
existing commands assignment in engine.ts into a small helper such as
deriveCommands(ctx, command) or replace it with an if/else chain, preserving the
user-input special case and the splitCommands fallback behavior. Keep the new
logic close to the current command handling in engine/engine.ts so it remains
easy to test and reuse.
In `@src/matcher/matchers.ts`:
- Around line 36-38: Add a matcher test in matchers.test.ts that explicitly
exercises the widened bash-command branch in matchers.ts for ctx.toolName values
of both bash and user-input. Verify that user-input with a command matching the
bash-command pattern is accepted, and include a case where the prompt exceeds
MAX_MATCH_INPUT_LENGTH so the guard still returns false.
In `@src/packs/git-safety.yaml`:
- Around line 18-31: The git-safety.commit-to-main rule is missing the {matched}
placeholder in both its confirmation and block messages, unlike the other
predicate-based rules in this YAML. Update the defaultAction.message and
fallback.message in the commit-to-main entry so they interpolate {matched},
keeping the wording consistent with the other rules and making the matched git
commit command visible to the user.
In `@src/packs/hardening.yaml`:
- Line 12: The hardening wrapper patterns are too broad and match unrelated
filenames because `hardening.eval`, `hardening.shell-c`, and
`hardening.substitution` use raw substrings like `.env`, `.pem`, and `.key`.
Tighten the regexes in the hardening config so these branches only match real
secret/file suffixes using word boundaries or equivalent delimiters, and make
the `sops`-related matching consistent across the three patterns.
In `@src/packs/index.ts`:
- Around line 9-18: The built-in packs are not being shipped with the compiled
package, so `loadBuiltInRulePacks()` ends up loading nothing because
`BUILTIN_PACKS_DIR` points at `dist/packs` while the YAML files stay under
source. Update the build/publish flow to copy the built-in YAML assets alongside
the compiled output in `dist/packs`, and make sure the packaging step includes
them so `loadAllRulePacks()` can find the shipped rule packs at runtime.
In `@src/packs/modern-cli.yaml`:
- Around line 6-16: The `modern-cli.grep` matcher only catches recursive `grep`
flags containing a lowercase `r`, so it misses uppercase `-R` invocations.
Update the bash-command pattern in the `modern-cli.grep` rule to also match `-R`
(while keeping the existing `--recursive` coverage) so calls like `grep -R ...`
trigger the same `rg` suggestion.
- Around line 18-28: The bash-command matcher in modern-cli’s `modern-cli.find`
rule is too restrictive because it only matches `find` invocations with an
explicit path before `-name`. Update the `pattern` so it also matches the common
path-omitted form like `find -name "*.log"` while still catching existing cases,
and keep the `defaultAction` suggestion behavior unchanged.
In `@src/packs/packs.test.ts`:
- Around line 59-92: Add adversarial coverage in packs.test around the matcher
gaps mentioned in the review: extend the existing makeEngine/bash/read
assertions to include positive blocks for FIDO2 private keys like id_ed25519_sk
and id_ecdsa_sk, and add negative/positive cases for hardening.yaml boundary
behavior such as .keys and .environment false positives plus
.ssh/authorized_keys and .ssh/config handling. Use the existing engine.evaluate,
bash, and read tool patterns in packs.test.ts so the tests clearly pin the regex
behavior and prevent regressions.
In `@src/packs/predicates.ts`:
- Line 7: The GIT_COMMIT matcher in predicates.ts is too broad and can fire for
git commit-* plumbing commands like commit-graph or commit-tree. Tighten the
pattern used by onProtectedBranch so it matches only the porcelain git commit
command, not subcommands with a hyphen after commit. Update the GIT_COMMIT regex
in the predicates logic and verify the confirm/block path in the commit-to-main
rule no longer triggers for those unrelated commands.
- Around line 17-20: The npmInPnpmRepo predicate only checks the current cwd for
pnpm-lock.yaml, so it misses pnpm workspaces when the command is run from a
package subdirectory. Update npmInPnpmRepo to walk up from cwd toward the repo
root until it finds pnpm-lock.yaml (or reaches the filesystem root), and use
that result to decide whether to return true, keeping the existing NPM_INSTALL
and command(ctx) check intact.
In `@src/packs/private-key.yaml`:
- Around line 6-16: The bash-command matcher in private-key.cat does not cover
OpenSSH FIDO2 key filenames, so update the pattern in the rule to also match the
`id_ed25519_sk` and `id_ecdsa_sk` variants alongside the existing `id_*` cases.
Keep the change localized to the `match.pattern` in this rule so shell reads
like `cat`, `less`, `head`, and `tail` are blocked consistently for both
traditional private keys and security-key-backed private keys.
- Around line 22-23: The private-key rule in the private-key pack is too broad
under `.ssh` because the current pattern blocks non-secret files like
`known_hosts`, `config`, and `authorized_keys`. Tighten the pattern in the
file-path matcher so it only targets actual private-key material, and keep the
existing `.pem`/`.key` coverage while narrowing the `.ssh` path handling in the
same rule.
---
Outside diff comments:
In `@docs/rule-pack-guide.md`:
- Around line 202-210: The override example in the rule pack guide is too broad
because it says “any built-in rule” even though some built-in rules can be
locked with overridable: false. Update the example text near the
agent-guardrails.json snippet to refer only to user-overridable or overridable
built-in rules, keeping the example aligned with the documented behavior.
In `@src/index.ts`:
- Around line 36-46: Update the barrel re-exports in src/index.ts so the JSDoc
for GuardrailAction and GuardrailRule matches the current phase model; the
comments still say “both phases” and are stale after adding user-input. Align
these public docs with the source types in core types by changing the wording to
refer to all phases, so the exported API comments correctly reflect before-tool,
after-tool, and user-input.
🪄 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 Plus
Run ID: dc6f663f-38f2-45c3-a41a-156d76b30174
📒 Files selected for processing (46)
.github/workflows/ci.yml.gitignoreCONTRIBUTING.mdLIMITATIONS.mdREADME.mdSECURITY.mddocs/adrs/001-layered-architecture.mddocs/adrs/002-behavior-model.mddocs/adrs/003-public-api-contract.mddocs/adrs/005-yaml-rule-packs.mddocs/adrs/006-observability-strategy.mddocs/adrs/007-trust-and-self-protection.mddocs/adrs/008-non-goals.mddocs/adrs/009-adapter-scope-and-tiering.mddocs/adrs/010-user-input-mediation.mddocs/getting-started.mddocs/packs.mddocs/rule-pack-guide.mdpackage.jsonscripts/generate-packs-gallery.mjssrc/adapters/claude-code/adapter.test.tssrc/adapters/claude-code/hook.tssrc/adapters/claude-code/normalize.tssrc/adapters/claude-code/respond.tssrc/core/harness-capabilities.tssrc/core/normalizer.tssrc/core/types.tssrc/core/validator.tssrc/engine/create-engine.tssrc/engine/engine.tssrc/index.tssrc/matcher/matchers.tssrc/packs/encryption-tools.yamlsrc/packs/env.yamlsrc/packs/git-safety.yamlsrc/packs/hardening.yamlsrc/packs/index.tssrc/packs/modern-cli.yamlsrc/packs/package-manager.yamlsrc/packs/packs.test.tssrc/packs/predicates.tssrc/packs/private-key.yamlsrc/packs/secret-managers.yamlsrc/packs/sops.yamlsrc/resolver/action-resolver.test.tssrc/resolver/action-resolver.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**
⚙️ CodeRabbit configuration file
**: # Agent Guardrails
⚠️ Heavy development: Not yet published to npm. Expect breakage. The quick start below shows the expected workflow once the package is available.
What is this?
A pattern-based steering engine for AI coding agent workflows.
Keep your agent shipping — not spilling secrets no one's watching for. Agent Guardrails steers agents away from costly mistakes instead of just stopping them cold.Is this just a set of guards to block bad behaviour?
No — mostly it transforms. Wherever a Safer Alternative exists, that's the default: reading
.envor decrypting a SOPS file resolves tosuggest, handing the agent a redacted command it can retry immediately instead of stalling out.blockis what's left for the cases with nothing saf...
Files:
src/matcher/matchers.tsdocs/adrs/010-user-input-mediation.mdsrc/packs/package-manager.yamldocs/adrs/009-adapter-scope-and-tiering.mdsrc/packs/modern-cli.yamldocs/adrs/005-yaml-rule-packs.mdsrc/packs/index.tssrc/packs/secret-managers.yamlscripts/generate-packs-gallery.mjssrc/packs/git-safety.yamlsrc/packs/env.yamlsrc/packs/private-key.yamlsrc/packs/packs.test.tssrc/engine/create-engine.tssrc/adapters/claude-code/hook.tssrc/packs/encryption-tools.yamlsrc/packs/sops.yamlsrc/adapters/claude-code/normalize.tsdocs/packs.mdSECURITY.mdsrc/core/normalizer.tsLIMITATIONS.mdsrc/packs/hardening.yamlsrc/adapters/claude-code/adapter.test.tssrc/packs/predicates.tssrc/resolver/action-resolver.tspackage.jsonsrc/adapters/claude-code/respond.tsCONTRIBUTING.mddocs/adrs/006-observability-strategy.mdsrc/resolver/action-resolver.test.tssrc/core/harness-capabilities.tsdocs/rule-pack-guide.mddocs/adrs/008-non-goals.mdsrc/core/validator.tsdocs/adrs/001-layered-architecture.mddocs/getting-started.mddocs/adrs/007-trust-and-self-protection.mdsrc/engine/engine.tsdocs/adrs/002-behavior-model.mddocs/adrs/003-public-api-contract.mdsrc/index.tsREADME.mdsrc/core/types.ts
src/matcher/**/*.ts
⚙️ CodeRabbit configuration file
src/matcher/**/*.ts: Flag if: imports from resolver/, engine/, or infrastructure/; side effects or global state in matching logic; missing adversarial test cases for new regex patterns; reintroducing a registry/handler split.
Do NOT flag: command-splitting limitations (variable tracking is explicitly out of scope); theMatchConditiondiscriminated union being a closed set evaluated by a singlematchesMatcher()function.
Files:
src/matcher/matchers.ts
docs/**/*.md
⚙️ CodeRabbit configuration file
Flag if: broken relative links; code examples that don't match actual API signatures; incorrect vocabulary usage (see README.md vocabulary table — Behavior≠Action, RulePack≠package, Harness≠Agent).
Files:
docs/adrs/010-user-input-mediation.mddocs/adrs/009-adapter-scope-and-tiering.mddocs/adrs/005-yaml-rule-packs.mddocs/packs.mddocs/adrs/006-observability-strategy.mddocs/rule-pack-guide.mddocs/adrs/008-non-goals.mddocs/adrs/001-layered-architecture.mddocs/getting-started.mddocs/adrs/007-trust-and-self-protection.mddocs/adrs/002-behavior-model.mddocs/adrs/003-public-api-contract.md
src/**/*.yaml
⚙️ CodeRabbit configuration file
YAML rule pack files. Flag if: non-kebab-case pack id; non-dotted rule id; phase/action mismatch (before-tool: block/suggest/run/confirm, after-tool: redact only); missing {matched} template in messages; suggest action without replacement.
Files:
src/packs/package-manager.yamlsrc/packs/modern-cli.yamlsrc/packs/secret-managers.yamlsrc/packs/git-safety.yamlsrc/packs/env.yamlsrc/packs/private-key.yamlsrc/packs/encryption-tools.yamlsrc/packs/sops.yamlsrc/packs/hardening.yaml
src/**/*.test.ts
⚙️ CodeRabbit configuration file
Flag if: snapshot tests for regex patterns (use inline values); missing adversarial/edge-case coverage for matchers; jest imports (this project uses vitest).
Files:
src/packs/packs.test.tssrc/adapters/claude-code/adapter.test.tssrc/resolver/action-resolver.test.ts
src/engine/**/*.ts
⚙️ CodeRabbit configuration file
Flag if: imports from infrastructure/; orchestration growing beyond ~80 lines (business logic belongs in resolver/ or matcher/); non-fail-closed handling of malformed ToolCallContext.
Files:
src/engine/create-engine.tssrc/engine/engine.ts
src/core/**/*.ts
⚙️ CodeRabbit configuration file
src/core/**/*.ts: Flag if: imports from matcher/, resolver/, engine/, or infrastructure/; use ofanyor!non-null assertion; default exports.
Do NOT flag: the five-value GuardrailBehavior union, the phase-behavior matrix, or fail-at-load-time validation — these are intentional design decisions documented in README.md.
Files:
src/core/normalizer.tssrc/core/harness-capabilities.tssrc/core/validator.tssrc/core/types.ts
src/resolver/**/*.ts
⚙️ CodeRabbit configuration file
src/resolver/**/*.ts: Flag if: imports from matcher/, engine/, or infrastructure/; missing GuardrailAction variant in switch/exhaustive check; new behavior added without updating both core/types.ts and resolveAction().
Do NOT flag: the confirm→suggest→block fallback chain — it is correct as implemented. Do NOT suggest changing theyamlpackage tojs-yaml— this project uses theyamlpackage (v2.4.0).
Files:
src/resolver/action-resolver.tssrc/resolver/action-resolver.test.ts
src/index.ts
⚙️ CodeRabbit configuration file
src/index.ts: Flag if: default exports; new exports without corresponding documentation.
Do NOT flag: re-exports from infrastructure/. The barrel may re-export from any layer.
Files:
src/index.ts
🪛 GitHub Check: SonarCloud Code Analysis
src/adapters/claude-code/respond.ts
[warning] 97-97: Use concise character class syntax '\w' instead of '[A-Za-z0-9_]'.
src/engine/engine.ts
[warning] 53-53: Extract this nested ternary operation into an independent statement.
🪛 LanguageTool
docs/adrs/009-adapter-scope-and-tiering.md
[style] ~27-~27: Consider an alternative for the overused word “exactly”.
Context: ...gin versus out-of-process subprocess is exactly the asymmetry the capability model and ...
(EXACTLY_PRECISELY)
docs/adrs/002-behavior-model.md
[style] ~94-~94: Consider an alternative for the overused word “exactly”.
Context: ...nues. An adversarial wrapper attempt is exactly the case where letting the model "adapt...
(EXACTLY_PRECISELY)
🪛 zizmor (1.26.1)
.github/workflows/ci.yml
[warning] 10-26: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🔇 Additional comments (35)
src/adapters/claude-code/normalize.ts (1)
30-36: 🗄️ Data Integrity & IntegrationMalformed
read/writepayloads are fail-closed in the engine before matching, so this adapter can’t propagate an invalid context downstream.> Likely an incorrect or invalid review comment.docs/adrs/001-layered-architecture.md (1)
40-70: LGTM!docs/adrs/002-behavior-model.md (1)
56-66: 🎯 Functional CorrectnessVerify the Claude Code version floor.
The
>= 2.1.121gate is a version-specific claim about an external hook API. Please confirm it against the adapter docs/schema before freezing it in prose.LIMITATIONS.md (1)
1-40: LGTM!README.md (1)
22-43: LGTM!Also applies to: 51-120
SECURITY.md (1)
1-59: LGTM!docs/getting-started.md (1)
3-7: LGTM!Also applies to: 31-74, 155-168, 187-190, 203-203
docs/rule-pack-guide.md (1)
42-198: LGTM!src/core/types.ts (1)
54-63: LGTM!Also applies to: 86-97, 140-153
src/core/validator.ts (1)
4-10: LGTM!Also applies to: 115-115, 127-141
src/core/normalizer.ts (1)
7-15: LGTM!Also applies to: 51-66
src/core/harness-capabilities.ts (2)
52-60: CustomcompareVersionshandles the common cases correctly.Parses dotted numeric segments and pads missing segments with 0, which correctly orders versions like
2.1.121vs2.1.120or2.1vs2.1.121. No issues found for the version formats used here (x.y.z).
1-50: LGTM!src/index.ts (1)
1-33: LGTM!Also applies to: 73-119
src/engine/create-engine.ts (1)
1-59: LGTM!src/engine/engine.ts (3)
122-142: LGTM!
148-165: LGTM!
104-109: 🩺 Stability & AvailabilityNo action: after-tool contexts don’t reach this path
normalizeHookPayload()returnsnullforPostToolUse, andprocessMatch()only branches onuser-inputvs.before-tool, so there’s no current after-tool context flowing into this matcher.> Likely an incorrect or invalid review comment.src/resolver/action-resolver.ts (2)
31-33: LGTM!
138-175: LGTM!src/resolver/action-resolver.test.ts (3)
176-193: LGTM!
226-237: LGTM!
281-297: LGTM!src/packs/predicates.ts (1)
1-14: LGTM!Also applies to: 34-55
src/packs/modern-cli.yaml (1)
30-40: LGTM!src/packs/package-manager.yaml (1)
1-17: LGTM!src/packs/git-safety.yaml (1)
6-17: LGTM!Also applies to: 32-45
src/packs/env.yaml (1)
1-51: LGTM!src/packs/secret-managers.yaml (1)
1-49: LGTM!src/packs/sops.yaml (1)
1-28: LGTM!src/packs/encryption-tools.yaml (1)
1-38: LGTM!src/packs/packs.test.ts (1)
1-58: LGTM!Also applies to: 94-174
scripts/generate-packs-gallery.mjs (1)
1-51: LGTM!package.json (1)
43-44: LGTM!docs/packs.md (1)
1-98: LGTM!
| - name: Verify pack gallery is up to date | ||
| run: npm run docs:packs && git diff --exit-code docs/packs.md |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
check job runs with default (unscoped) GITHUB_TOKEN permissions.
Static analysis (zizmor) flags this job for lacking an explicit permissions: block. None of the steps in this job — including the new gallery-verification step — need write access to the repo, issues, PRs, etc. Add least-privilege permissions at the job or workflow level.
🔒 Proposed fix
check:
name: Lint · Format · TypeCheck
runs-on: ubuntu-latest
+ permissions:
+ contents: read
steps:As per static analysis hints: "overly broad permissions (excessive-permissions): default permissions used due to no permissions: block".
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 10-26: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 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 @.github/workflows/ci.yml around lines 25 - 26, The check job in the CI
workflow is using the default unscoped GITHUB_TOKEN permissions, so add an
explicit least-privilege permissions block for this job or at the workflow
level. Update the workflow around the check job and the new docs/packs.md
verification step so it only requests the minimal read-only access needed, with
no write permissions to contents, issues, or pull requests.
Source: Linters/SAST tools
| it('blocks private key reads via bash and read tool', () => { | ||
| const { engine } = makeEngine() | ||
| expect(engine.evaluate(bash('cat ~/.ssh/id_rsa'))?.type).toBe('block') | ||
| expect(engine.evaluate({ toolName: 'read', filePath: '/home/u/.ssh/id_ed25519' })?.type).toBe( | ||
| 'block' | ||
| ) | ||
| }) | ||
|
|
||
| it('allows reading SSH public keys', () => { | ||
| const { engine } = makeEngine() | ||
| expect( | ||
| engine.evaluate({ toolName: 'read', filePath: '/home/u/.ssh/id_ed25519.pub' }) | ||
| ).toBeNull() | ||
| }) | ||
|
|
||
| it('blocks secret manager retrieval', () => { | ||
| const { engine } = makeEngine() | ||
| expect(engine.evaluate(bash('pass show work/aws'))?.type).toBe('block') | ||
| expect(engine.evaluate(bash('op read op://vault/item/password'))?.type).toBe('block') | ||
| expect(engine.evaluate(bash('bw get password github'))?.type).toBe('block') | ||
| }) | ||
|
|
||
| it('blocks decrypt commands from encryption tools', () => { | ||
| const { engine } = makeEngine() | ||
| expect(engine.evaluate(bash('gpg --decrypt secrets.gpg'))?.type).toBe('block') | ||
| expect(engine.evaluate(bash('age -d -i key.txt file.age'))?.type).toBe('block') | ||
| expect(engine.evaluate(bash('openssl enc -d -aes256 -in file.enc'))?.type).toBe('block') | ||
| }) | ||
|
|
||
| it('force-blocks adversarial wrappers around sensitive operations', () => { | ||
| const { engine } = makeEngine() | ||
| expect(engine.evaluate(bash('eval "sops -d secrets.yaml"'))?.type).toBe('block') | ||
| expect(engine.evaluate(bash('bash -c "cat .env"'))?.type).toBe('block') | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add adversarial cases for the flagged pattern gaps.
Coverage here doesn't exercise the boundary issues raised in private-key.yaml (FIDO2 id_ed25519_sk/id_ecdsa_sk) or hardening.yaml (.keys/.environment false positives, .ssh/authorized_keys and .ssh/config over-blocking). Adding negative/positive cases for these would catch regressions in the underlying regexes.
As per path instructions for src/**/*.test.ts: "missing adversarial/edge-case coverage for matchers."
🤖 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/packs/packs.test.ts` around lines 59 - 92, Add adversarial coverage in
packs.test around the matcher gaps mentioned in the review: extend the existing
makeEngine/bash/read assertions to include positive blocks for FIDO2 private
keys like id_ed25519_sk and id_ecdsa_sk, and add negative/positive cases for
hardening.yaml boundary behavior such as .keys and .environment false positives
plus .ssh/authorized_keys and .ssh/config handling. Use the existing
engine.evaluate, bash, and read tool patterns in packs.test.ts so the tests
clearly pin the regex behavior and prevent regressions.
Source: Path instructions
| import type { PredicateRegistry } from '../core/predicate-registry.js' | ||
|
|
||
| const NPM_INSTALL = /(^|[;&|]\s*)npm\s+(install|i|add)\b/ | ||
| const GIT_COMMIT = /(^|[;&|]\s*)git\s+commit\b/ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
GIT_COMMIT regex false-positives on other git commit-* subcommands.
\bcommit\b matches "commit" followed by any non-word character, so git commit-graph write or git commit-tree also match (the - after "commit" already satisfies the trailing \b). These are unrelated plumbing/maintenance subcommands, not the git commit porcelain command the git-safety.commit-to-main rule targets, so onProtectedBranch will incorrectly trigger a confirm/block on protected branches for these commands too.
🐛 Proposed fix
-const GIT_COMMIT = /(^|[;&|]\s*)git\s+commit\b/
+const GIT_COMMIT = /(^|[;&|]\s*)git\s+commit(?:\s|$)/Also applies to: 22-32
🤖 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/packs/predicates.ts` at line 7, The GIT_COMMIT matcher in predicates.ts
is too broad and can fire for git commit-* plumbing commands like commit-graph
or commit-tree. Tighten the pattern used by onProtectedBranch so it matches only
the porcelain git commit command, not subcommands with a hyphen after commit.
Update the GIT_COMMIT regex in the predicates logic and verify the confirm/block
path in the commit-to-main rule no longer triggers for those unrelated commands.
| export function npmInPnpmRepo(ctx: ToolCallContext, cwd = process.cwd()): boolean { | ||
| if (!NPM_INSTALL.test(command(ctx))) return false | ||
| return existsSync(join(cwd, 'pnpm-lock.yaml')) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
npmInPnpmRepo only checks cwd, missing workspace subdirectories.
If the agent runs npm install from within a package subdirectory of a pnpm workspace (common in monorepos), pnpm-lock.yaml typically only exists at the repo root, so this predicate will false-negative and let the conflicting npm install through.
♻️ Proposed fix: walk up to find pnpm-lock.yaml
+function findUp(cwd: string, filename: string): boolean {
+ let dir = cwd
+ for (;;) {
+ if (existsSync(join(dir, filename))) return true
+ const parent = dirname(dir)
+ if (parent === dir) return false
+ dir = parent
+ }
+}
+
export function npmInPnpmRepo(ctx: ToolCallContext, cwd = process.cwd()): boolean {
if (!NPM_INSTALL.test(command(ctx))) return false
- return existsSync(join(cwd, 'pnpm-lock.yaml'))
+ return findUp(cwd, 'pnpm-lock.yaml')
}📝 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.
| export function npmInPnpmRepo(ctx: ToolCallContext, cwd = process.cwd()): boolean { | |
| if (!NPM_INSTALL.test(command(ctx))) return false | |
| return existsSync(join(cwd, 'pnpm-lock.yaml')) | |
| } | |
| function findUp(cwd: string, filename: string): boolean { | |
| let dir = cwd | |
| for (;;) { | |
| if (existsSync(join(dir, filename))) return true | |
| const parent = dirname(dir) | |
| if (parent === dir) return false | |
| dir = parent | |
| } | |
| } | |
| export function npmInPnpmRepo(ctx: ToolCallContext, cwd = process.cwd()): boolean { | |
| if (!NPM_INSTALL.test(command(ctx))) return false | |
| return findUp(cwd, 'pnpm-lock.yaml') | |
| } |
🤖 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/packs/predicates.ts` around lines 17 - 20, The npmInPnpmRepo predicate
only checks the current cwd for pnpm-lock.yaml, so it misses pnpm workspaces
when the command is run from a package subdirectory. Update npmInPnpmRepo to
walk up from cwd toward the repo root until it finds pnpm-lock.yaml (or reaches
the filesystem root), and use that result to decide whether to return true,
keeping the existing NPM_INSTALL and command(ctx) check intact.
| - id: private-key.cat | ||
| title: Block shell reads of key files | ||
| description: Catches cat/less/head/tail on .pem/.key/id_* files | ||
| phase: before-tool | ||
| match: | ||
| type: bash-command | ||
| pattern: "(^|[;&|]\\s*)(cat|less|more|head|tail)\\s+[^;&|]*(\\.pem|\\.key|id_(rsa|ed25519|ecdsa|dsa))\\b" | ||
| defaultAction: | ||
| type: block | ||
| message: 'Blocked: `{matched}` would expose private key material.' | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Bash-command rule misses FIDO2/security-key filenames.
OpenSSH FIDO2-backed keys default to id_ed25519_sk / id_ecdsa_sk (underscore-separated). Since _ is a word character, \b right after ed25519/ecdsa in the current alternation won't match before the _sk suffix, so cat ~/.ssh/id_ed25519_sk slips past this rule (it's still caught by the separate .ssh file-path rule for the read tool, but not for equivalent shell access). Consider adding the _sk suffix to keep the two rules consistent in coverage.
🔧 Proposed pattern update
- pattern: "(^|[;&|]\\s*)(cat|less|more|head|tail)\\s+[^;&|]*(\\.pem|\\.key|id_(rsa|ed25519|ecdsa|dsa))\\b"
+ pattern: "(^|[;&|]\\s*)(cat|less|more|head|tail)\\s+[^;&|]*(\\.pem|\\.key|id_(rsa|ed25519|ecdsa|dsa)(_sk)?)\\b"📝 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.
| - id: private-key.cat | |
| title: Block shell reads of key files | |
| description: Catches cat/less/head/tail on .pem/.key/id_* files | |
| phase: before-tool | |
| match: | |
| type: bash-command | |
| pattern: "(^|[;&|]\\s*)(cat|less|more|head|tail)\\s+[^;&|]*(\\.pem|\\.key|id_(rsa|ed25519|ecdsa|dsa))\\b" | |
| defaultAction: | |
| type: block | |
| message: 'Blocked: `{matched}` would expose private key material.' | |
| - id: private-key.cat | |
| title: Block shell reads of key files | |
| description: Catches cat/less/head/tail on .pem/.key/id_* files | |
| phase: before-tool | |
| match: | |
| type: bash-command | |
| pattern: "(^|[;&|]\\s*)(cat|less|more|head|tail)\\s+[^;&|]*(\\.pem|\\.key|id_(rsa|ed25519|ecdsa|dsa)(_sk)?)\\b" | |
| defaultAction: | |
| type: block | |
| message: 'Blocked: `{matched}` would expose private key material.' |
🤖 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/packs/private-key.yaml` around lines 6 - 16, The bash-command matcher in
private-key.cat does not cover OpenSSH FIDO2 key filenames, so update the
pattern in the rule to also match the `id_ed25519_sk` and `id_ecdsa_sk` variants
alongside the existing `id_*` cases. Keep the change localized to the
`match.pattern` in this rule so shell reads like `cat`, `less`, `head`, and
`tail` are blocked consistently for both traditional private keys and
security-key-backed private keys.
| type: file-path | ||
| pattern: "(\\.pem|\\.key)$|/\\.ssh/(?!.*\\.pub$)" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target rule pack and any nearby docs that define its intent.
git ls-files 'src/packs/private-key.yaml' 'docs/rule-pack-guide.md' 'docs/how-matching-works.md' 'docs/getting-started.md' 'docs/adrs/007-trust-and-self-protection.md'
printf '\n--- src/packs/private-key.yaml ---\n'
cat -n src/packs/private-key.yaml
printf '\n--- search for .ssh / private-key docs references ---\n'
rg -n --no-heading '\.ssh|known_hosts|authorized_keys|private key|private-key' src docs README.mdRepository: nimser/agent-guardrails
Length of output: 6462
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- docs/packs.md around private-key ---\n'
sed -n '68,82p' docs/packs.md
printf '\n--- src/packs/packs.test.ts around private-key tests ---\n'
sed -n '54,78p' src/packs/packs.test.ts
printf '\n--- search for any SSH-specific private-key docs/tests ---\n'
rg -n --no-heading 'known_hosts|authorized_keys|config|id_rsa|id_ed25519|\.ssh/' src docs README.mdRepository: nimser/agent-guardrails
Length of output: 5391
private-key.read matches too much under .ssh. /\.ssh/(?!.*\.pub$) also blocks known_hosts, config, and authorized_keys; narrow it if this pack is meant to protect only private-key material.
🤖 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/packs/private-key.yaml` around lines 22 - 23, The private-key rule in the
private-key pack is too broad under `.ssh` because the current pattern blocks
non-secret files like `known_hosts`, `config`, and `authorized_keys`. Tighten
the pattern in the file-path matcher so it only targets actual private-key
material, and keep the existing `.pem`/`.key` coverage while narrowing the
`.ssh` path handling in the same rule.
b577724 to
fe159c2
Compare
nimser
changed the title
…adapter Implements the three active changes on top of the direction alignment: change-18 — nine built-in YAML packs (25 rules; steering: modern-cli, package-manager, git-safety; security: env, sops, private-key, secret-managers, encryption-tools, hardening) with built-in predicates and loadBuiltInRulePacks(); change-17 — user-input phase wired through the engine and matcher with the redactUserInput capability gate; change-7 — Claude Code hook adapter (structured permissionDecision JSON, updatedInput for run, native ask for confirm, UserPromptSubmit scrubbing, unconditional fail-closed wrapper). Plus the auto-generated docs/packs.md gallery with a CI staleness check. Known gaps before this is user-ready (tracked in the PR body): packaging (YAML packs into dist, yaml to dependencies), the CLI installer, hook-schema verification against a live Claude Code, the Pi adapter, and after-tool redact evaluation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUpa7w5hYzrXyCYkK8K1DC
fe159c2 to
22bf897
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/engine/engine.ts (1)
51-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNested ternary still unresolved.
Same nested-ternary flagged by SonarCloud in a prior review remains unfixed.
♻️ Proposed refactor
- // user-input contexts carry prompt text, which is never command-split (ADR-010) - const commands = - ctx.toolName === 'user-input' ? [command ?? ''] : command ? splitCommands(command) : [''] + // user-input contexts carry prompt text, which is never command-split (ADR-010) + const commands = deriveCommands(ctx, command)function deriveCommands(ctx: ToolCallContext, command: string | undefined): string[] { if (ctx.toolName === 'user-input') return [command ?? ''] return command ? splitCommands(command) : [''] }🤖 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/engine/engine.ts` around lines 51 - 53, The nested ternary in the command derivation logic is still present and should be refactored for clarity. Update the command selection in engine.ts around the existing commands assignment by moving the logic into a helper like deriveCommands(ctx, command) or equivalent, using an explicit if branch for the user-input case and a separate fallback for splitCommands(command) versus ['']. Keep the behavior identical while removing the nested ternary from the engine.ts flow.Source: Linters/SAST tools
🤖 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 `@scripts/generate-packs-gallery.mjs`:
- Line 11: The pack categorization in generate-packs-gallery.mjs is hardcoded
via the STEERING Set, which can silently misclassify new packs. Update the
gallery generation logic to read category from each pack’s YAML/schema data
instead of relying on the STEERING allowlist, and use that field when building
the output so new packs don’t default incorrectly. Adjust the related
pack-processing flow around STEERING and the gallery generation path to preserve
existing categories while making the source of truth explicit.
In `@src/adapters/claude-code/adapter.test.ts`:
- Around line 97-106: Add an adversarial test around toHookResponse in
adapter.test.ts that covers a redact action when the prompt contains a secret
shape not included in SECRET_SHAPES. Use the existing UserPromptSubmit/redact
flow, but assert the expected fail-closed or non-leaking behavior for a token
shape that the matcher does not currently recognize, so the test pins down the
divergence referenced by respond.ts.
In `@src/engine/engine.ts`:
- Around line 122-129: Hoist the loop-invariant effectiveCapabilities
computation out of the per-rule matching path in engine.ts so it is created once
per call instead of on every matching rule. Use the ctxPhase-dependent logic
near resolveAction and the surrounding pack.rules iteration to move the
conditional redaction capability selection before the loop, then reuse that same
HarnessCapabilities value inside the rule evaluation flow.
---
Duplicate comments:
In `@src/engine/engine.ts`:
- Around line 51-53: The nested ternary in the command derivation logic is still
present and should be refactored for clarity. Update the command selection in
engine.ts around the existing commands assignment by moving the logic into a
helper like deriveCommands(ctx, command) or equivalent, using an explicit if
branch for the user-input case and a separate fallback for
splitCommands(command) versus ['']. Keep the behavior identical while removing
the nested ternary from the engine.ts flow.
🪄 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 Plus
Run ID: b5e27a67-6db6-4af6-8604-7c24bd900e7e
📒 Files selected for processing (23)
.github/workflows/ci.ymldocs/packs.mdpackage.jsonscripts/generate-packs-gallery.mjssrc/adapters/claude-code/adapter.test.tssrc/adapters/claude-code/hook.tssrc/adapters/claude-code/normalize.tssrc/adapters/claude-code/respond.tssrc/engine/engine.tssrc/index.tssrc/matcher/matchers.tssrc/packs/encryption-tools.yamlsrc/packs/env.yamlsrc/packs/git-safety.yamlsrc/packs/hardening.yamlsrc/packs/index.tssrc/packs/modern-cli.yamlsrc/packs/package-manager.yamlsrc/packs/packs.test.tssrc/packs/predicates.tssrc/packs/private-key.yamlsrc/packs/secret-managers.yamlsrc/packs/sops.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**
⚙️ CodeRabbit configuration file
**: # Agent Guardrails
⚠️ Heavy development: Not yet published to npm. Expect breakage. The quick start below shows the expected workflow once the package is available.
Your agent just tried
cat .env. It got back the keys — not the values — and kept working.That's the whole idea: a steering layer for AI coding agents. Instead of stopping the agent cold, rules hand it a safer or better move it can act on immediately —
suggesta redacted read,runa rewritten command,redacta leaked value,confirmwith a human, orblockwhen nothing safe exists.Batteries included
Rules work out of the box — no config, no flags:
npx agent-guardrails ...
Files:
src/packs/package-manager.yamlsrc/packs/modern-cli.yamlsrc/matcher/matchers.tspackage.jsonsrc/packs/sops.yamlsrc/packs/hardening.yamlsrc/packs/index.tssrc/packs/git-safety.yamlsrc/packs/secret-managers.yamlsrc/packs/private-key.yamlsrc/packs/encryption-tools.yamlscripts/generate-packs-gallery.mjssrc/adapters/claude-code/normalize.tssrc/index.tssrc/packs/env.yamlsrc/packs/packs.test.tssrc/adapters/claude-code/adapter.test.tsdocs/packs.mdsrc/engine/engine.tssrc/packs/predicates.tssrc/adapters/claude-code/respond.tssrc/adapters/claude-code/hook.ts
src/**/*.yaml
⚙️ CodeRabbit configuration file
YAML rule pack files. Flag if: non-kebab-case pack id; non-dotted rule id; phase/action mismatch (before-tool: block/suggest/run/confirm, after-tool: redact only); missing {matched} template in messages; suggest action without replacement.
Files:
src/packs/package-manager.yamlsrc/packs/modern-cli.yamlsrc/packs/sops.yamlsrc/packs/hardening.yamlsrc/packs/git-safety.yamlsrc/packs/secret-managers.yamlsrc/packs/private-key.yamlsrc/packs/encryption-tools.yamlsrc/packs/env.yaml
src/matcher/**/*.ts
⚙️ CodeRabbit configuration file
src/matcher/**/*.ts: Flag if: imports from resolver/, engine/, or infrastructure/; side effects or global state in matching logic; missing adversarial test cases for new regex patterns; reintroducing a registry/handler split.
Do NOT flag: command-splitting limitations (variable tracking is explicitly out of scope); theMatchConditiondiscriminated union being a closed set evaluated by a singlematchesMatcher()function.
Files:
src/matcher/matchers.ts
src/index.ts
⚙️ CodeRabbit configuration file
src/index.ts: Flag if: default exports; new exports without corresponding documentation.
Do NOT flag: re-exports from infrastructure/. The barrel may re-export from any layer.
Files:
src/index.ts
src/**/*.test.ts
⚙️ CodeRabbit configuration file
Flag if: snapshot tests for regex patterns (use inline values); missing adversarial/edge-case coverage for matchers; jest imports (this project uses vitest).
Files:
src/packs/packs.test.tssrc/adapters/claude-code/adapter.test.ts
docs/**/*.md
⚙️ CodeRabbit configuration file
Flag if: broken relative links; code examples that don't match actual API signatures; incorrect vocabulary usage (see README.md vocabulary table — Behavior≠Action, RulePack≠package, Harness≠Agent).
Files:
docs/packs.md
src/engine/**/*.ts
⚙️ CodeRabbit configuration file
Flag if: imports from infrastructure/; orchestration growing beyond ~80 lines (business logic belongs in resolver/ or matcher/); non-fail-closed handling of malformed ToolCallContext.
Files:
src/engine/engine.ts
🪛 GitHub Check: SonarCloud Code Analysis
src/engine/engine.ts
[warning] 53-53: Extract this nested ternary operation into an independent statement.
src/adapters/claude-code/respond.ts
[warning] 97-97: Use concise character class syntax '\w' instead of '[A-Za-z0-9_]'.
🪛 zizmor (1.26.1)
.github/workflows/ci.yml
[warning] 10-26: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🔇 Additional comments (31)
src/engine/engine.ts (2)
104-109: LGTM!
148-165: LGTM!src/matcher/matchers.ts (1)
34-43: LGTM!src/index.ts (1)
24-33: LGTM!Also applies to: 55-58
src/adapters/claude-code/hook.ts (1)
47-55:main()startup path still not fail-closed and stdin read is unbounded.
createClaudeCodeEngine()(pack loading) and thefor await ... process.stdinloop run outside any try/catch, so a startup crash exits without emitting a block decision; the read also waits indefinitely for EOF. Both were raised on prior commits and remain unaddressed.src/adapters/claude-code/normalize.ts (1)
30-36: 🩺 Stability & AvailabilityMissing
filePathis handled safely.matchesMatcher()returnsfalsewhenfilePathis absent, andisMissingRequiredFields()blocksread/writecalls without one, so this path won’t dereferenceundefined.> Likely an incorrect or invalid review comment.src/adapters/claude-code/respond.ts (1)
73-85: 🔒 Security & PrivacyNo leak path here. The only
user-inputredact rule matches the same secret-shaped tokens this adapter scrubs, so this path doesn’t currently let the raw prompt through.> Likely an incorrect or invalid review comment.src/packs/index.ts (2)
9-18: 🩺 Stability & AvailabilityShip YAML packs alongside compiled output.
BUILTIN_PACKS_DIRstill resolves next to the compiled JS, but no build/publish step copiessrc/packs/*.yamlintodist/packs.loadBuiltInRulePacks()will return an empty list once installed from npm.
1-6: LGTM!Also applies to: 20-20
src/packs/predicates.ts (3)
7-7: 🎯 Functional Correctness
GIT_COMMITstill matches unrelatedgit commit-*plumbing subcommands.
17-20: 🎯 Functional Correctness
npmInPnpmRepostill only checkscwd, missing pnpm workspace subdirectories.
1-6: LGTM!Also applies to: 34-55
src/packs/modern-cli.yaml (3)
6-16: 🎯 Functional Correctness
greppattern still misses uppercase-R.
18-28: 🎯 Functional Correctness
findpattern still misses the path-omitted form (find -name ...).
30-40: LGTM!src/packs/git-safety.yaml (3)
18-31: 🎯 Functional Correctness
commit-to-mainmessages still missing{matched}.
6-16: LGTM!
32-44: 🎯 Functional Correctness | ⚡ Quick win
git clean --force(long flag) bypasses the confirm gate.The
cleanalternative-[a-zA-Z]*fonly matches short-flag clusters ending inf(-f,-df,-xdf, …).git clean --force— a valid, common invocation — doesn't match: after the leading-, the next character is-, which fails[a-zA-Z]*, so the destructivecleannever trips the confirm gate.🐛 Proposed fix
- pattern: "(^|[;&|]\\s*)git\\s+(reset\\s+[^;&|]*--hard|clean\\s+[^;&|]*-[a-zA-Z]*f)" + pattern: "(^|[;&|]\\s*)git\\s+(reset\\s+[^;&|]*--hard|clean\\s+[^;&|]*(-[a-zA-Z]*f\\b|--force\\b))"src/packs/private-key.yaml (2)
6-16: 🎯 Functional CorrectnessBash-command rule still misses FIDO2/security-key filenames (
_sksuffix).
17-27: 🎯 Functional Correctness
private-key.readstill over-broadly blocks all of.ssh/(e.g.known_hosts,config,authorized_keys).src/packs/package-manager.yaml (1)
1-17: LGTM!src/packs/secret-managers.yaml (2)
6-16: 🔒 Security & Privacy | ⚡ Quick win
pass grepbypasses this block rule.
pass grep <term>decrypts every password file and prints matching lines — functionally equivalent topass showfor secret exposure — but isn't matched byshow\b|\S*/.🐛 Proposed fix
- pattern: "(^|[;&|]\\s*)pass\\s+(show\\b|\\S*/)" + pattern: "(^|[;&|]\\s*)pass\\s+(show\\b|grep\\b|\\S*/)"
17-49: LGTM!src/packs/encryption-tools.yaml (1)
1-38: LGTM!src/packs/hardening.yaml (1)
6-37: 🎯 Functional CorrectnessWrapper patterns still over-match on unrelated filenames.
hardening.eval/shell-c/substitution(Lines 12, 23, 34) match raw substrings.env/.pem/.keywithout word boundaries, sonotes.keys,backup.keychain,app.environmentstill get force-blocked. Thesopsbranch is also still inconsistent (sops\bimplicit vssops\sin substitution).src/packs/packs.test.ts (1)
59-92: 📐 Maintainability & Code QualityAdversarial/edge-case coverage still missing for flagged pattern gaps.
No tests exercise the FIDO2 key boundary (
id_ed25519_sk/id_ecdsa_sk) or hardening.yaml's.keys/.environment/.ssh/authorized_keys/.ssh/configfalse-positive risks.Source: Path instructions
.github/workflows/ci.yml (1)
25-26: 🔒 Security & Privacy
checkjob still runs with default unscopedGITHUB_TOKENpermissions.The new gallery-verification step doesn't need write access either; add an explicit least-privilege
permissions:block at the job level.Source: Linters/SAST tools
src/packs/env.yaml (1)
1-51: LGTM!src/packs/sops.yaml (1)
1-28: LGTM!package.json (1)
43-44: LGTM!docs/packs.md (1)
1-98: LGTM!
| const root = join(dirname(fileURLToPath(import.meta.url)), '..') | ||
| const packsDir = join(root, 'src', 'packs') | ||
|
|
||
| const STEERING = new Set(['modern-cli', 'package-manager', 'git-safety']) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
Categorization is a hardcoded id list, prone to silent drift.
New packs must be manually added to STEERING (Line 11) or they default to "Security" — miscategorization wouldn't be caught by the CI staleness check since the generated output would still be internally consistent. Consider deriving the category from a category field in the pack YAML/schema instead.
Also applies to: 34-35
🤖 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 `@scripts/generate-packs-gallery.mjs` at line 11, The pack categorization in
generate-packs-gallery.mjs is hardcoded via the STEERING Set, which can silently
misclassify new packs. Update the gallery generation logic to read category from
each pack’s YAML/schema data instead of relying on the STEERING allowlist, and
use that field when building the output so new packs don’t default incorrectly.
Adjust the related pack-processing flow around STEERING and the gallery
generation path to preserve existing categories while making the source of truth
explicit.
| it('rewrites the prompt for user-input redact', () => { | ||
| const r = toHookResponse( | ||
| { type: 'redact', replacement: '[SECRET-REDACTED]' }, | ||
| { | ||
| hook_event_name: 'UserPromptSubmit', | ||
| prompt: 'my key is ghp_abcdefghijklmnopqrstuvwxyz0123456789 ok', | ||
| } | ||
| ) | ||
| expect(r.hookSpecificOutput?.updatedPrompt).toBe('my key is [SECRET-REDACTED] ok') | ||
| }) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Add adversarial coverage for redact vs. SECRET_SHAPES divergence.
The redact test only exercises a ghp_ token that SECRET_SHAPES recognizes. Add a case where the engine returns redact but the prompt holds a secret shape not in SECRET_SHAPES to pin down the leak/fail-closed behavior (see the related concern in respond.ts).
🤖 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/adapters/claude-code/adapter.test.ts` around lines 97 - 106, Add an
adversarial test around toHookResponse in adapter.test.ts that covers a redact
action when the prompt contains a secret shape not included in SECRET_SHAPES.
Use the existing UserPromptSubmit/redact flow, but assert the expected
fail-closed or non-leaking behavior for a token shape that the matcher does not
currently recognize, so the test pins down the divergence referenced by
respond.ts.
| // In the user-input phase, prompt rewriting is gated on the dedicated | ||
| // redactUserInput capability rather than tool-output redact (ADR-010). | ||
| const effectiveCapabilities: HarnessCapabilities = | ||
| ctxPhase === 'user-input' | ||
| ? { ...capabilities, redact: capabilities.redactUserInput === true } | ||
| : capabilities | ||
|
|
||
| const resolved = resolveAction(rule.defaultAction, effectiveCapabilities, { |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
Hoist effectiveCapabilities out of the per-rule loop.
ctxPhase is loop-invariant, so effectiveCapabilities can be computed once before iterating pack.rules instead of being recreated on every matching rule.
♻️ Proposed refactor
function matchPackRulesTraced(
pack: RulePack,
matchCtx: ToolCallContext,
capabilities: HarnessCapabilities,
ctx: ToolCallContext,
cmd: string,
registry: PredicateRegistry
): MatchResult | undefined {
const ctxPhase = ctx.toolName === 'user-input' ? 'user-input' : 'before-tool'
+ const effectiveCapabilities: HarnessCapabilities =
+ ctxPhase === 'user-input'
+ ? { ...capabilities, redact: capabilities.redactUserInput === true }
+ : capabilities
for (const rule of pack.rules) {
...
if (rule.phase !== ctxPhase) continue
if (!matchesMatcher(rule.match, matchCtx, registry)) continue
...
- // In the user-input phase, prompt rewriting is gated on the dedicated
- // redactUserInput capability rather than tool-output redact (ADR-010).
- const effectiveCapabilities: HarnessCapabilities =
- ctxPhase === 'user-input'
- ? { ...capabilities, redact: capabilities.redactUserInput === true }
- : capabilities
-
const resolved = resolveAction(rule.defaultAction, effectiveCapabilities, {📝 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.
| // In the user-input phase, prompt rewriting is gated on the dedicated | |
| // redactUserInput capability rather than tool-output redact (ADR-010). | |
| const effectiveCapabilities: HarnessCapabilities = | |
| ctxPhase === 'user-input' | |
| ? { ...capabilities, redact: capabilities.redactUserInput === true } | |
| : capabilities | |
| const resolved = resolveAction(rule.defaultAction, effectiveCapabilities, { | |
| const effectiveCapabilities: HarnessCapabilities = | |
| ctxPhase === 'user-input' | |
| ? { ...capabilities, redact: capabilities.redactUserInput === true } | |
| : capabilities | |
| const resolved = resolveAction(rule.defaultAction, effectiveCapabilities, { |
🤖 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/engine/engine.ts` around lines 122 - 129, Hoist the loop-invariant
effectiveCapabilities computation out of the per-rule matching path in engine.ts
so it is created once per call instead of on every matching rule. Use the
ctxPhase-dependent logic near resolveAction and the surrounding pack.rules
iteration to move the conditional redaction capability selection before the
loop, then reuse that same HarnessCapabilities value inside the rule evaluation
flow.
1 participant
Part 2 of 2, stacked on #16 (
feat/new-direction-alignment) — merge that first. Implements the three active openspec changes (change-18, change-17, change-7).What's here
src/packs/): steering —modern-cli,package-manager,git-safety; security —env,sops,private-key,secret-managers,encryption-tools,hardening. Built-in predicates (npm-in-pnpm-repo,on-protected-branch,cat-large-file) andloadBuiltInRulePacks()as the zero-config path. Positive and negative match cases tested.user-inputphase wired end to end: engine phase dispatch, prompt-text matching (never command-split),redactUserInputcapability gate withredact → blockfallback.src/adapters/claude-code/): structuredpermissionDecisionJSON only (no exit-code-2),updatedInputforrun, nativeaskforconfirm,UserPromptSubmitscrubbing, unconditional fail-closed wrapper (ADR-007).docs/packs.mdgallery + CI staleness check.Known gaps before this is user-ready (acceptance criteria for follow-ups)
dist/by the tsc build andyamlis a devDependency — a published package would be broken. Fix before any release.npx agent-guardrails install …, change-12) and nobinentry — the adapter'smain()can't reach anyone's hook config yet.updatedPrompt/updatedInput/UserPromptSubmit response shapes are written from research notes and need a smoke test against a live Claude Code before relying on them.236 tests green; lint/format/typecheck/build pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FUpa7w5hYzrXyCYkK8K1DC