Conversation
…#405 S1) Restructure the model-initiated bash tool_call tail into an ordered, named policy list (BASH_COMMAND_POLICIES) evaluated by evaluateBashPolicies: first non-undefined verdict wins, undefined defers to the next policy, overall undefined allows. The only entry is the runtime-guardrails adapter over the byte-identical confirmCommand, so every verdict, block reason, emitted event sequence, headless fail-safe, and config precedence is unchanged. Exposed via __testing with order/short-circuit/allow tests. Work unit S1 of Gentleman-Programming#405: no new policy modules, no config schema change, no second tool_call handler.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe bash tool-call path now evaluates an ordered policy list. The initial ChangesBash policy evaluation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant BashToolCallHandler
participant evaluateBashPolicies
participant runtime-guardrails
participant confirmCommand
BashToolCallHandler->>evaluateBashPolicies: evaluate command
evaluateBashPolicies->>runtime-guardrails: evaluate command
runtime-guardrails->>confirmCommand: confirm command
confirmCommand-->>runtime-guardrails: return verdict
runtime-guardrails-->>evaluateBashPolicies: return verdict
evaluateBashPolicies-->>BashToolCallHandler: return first non-undefined verdict
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The policy seam preserves the existing bash guardrail behavior, and the supplied tests cover policy ordering and guardrail decisions. No concrete merge-blocking risk is established. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new policy seam appears to preserve the current command protections. No security regression was identified, but coverage of the wider runtime is incomplete. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
…interface
The seam harness mocked ExtensionAPI["events"] as
{ emit: () => {}, on: () => {} } with an `as` cast, which trips TS2352:
EventBus.on must return an unsubscribe function, and a void-returning
stub does not overlap. Give the mock the real emit/on signatures so it
matches EventBus structurally and the cast disappears entirely, keeping
the typecheck ratchet baseline at 205.
|
Adding review provenance: the branch went through a four-lens native review (risk, resilience, readability, reliability) on top of CI and CodeRabbit: approved with 8 informational advisories and no corrections required. The advisories worth a skim before merge, all non-blocking:
Verification state: 56/56 autonomous-guard tests pass, biome clean, CI green, CodeRabbit reports no actionable comments. No code changed since the last push, so the diff under review is unchanged. |
|
@Alan-TheGentleman consolidated status of my open gentle-shell PRs, since they all sit green and unlabeled and I want to make sure they are on your radar rather than lost. Merge-ready, blocked only on the
Needs triage rather than review: #605 closes #593, which has no labels yet. The PR itself merges clean, so an approval decision on the issue is the only open question. #915, #1009 and #605 were dry-run verified today as zero-conflict against current main. Happy to rebase, split, or close any of them if the direction changed on main. |
…#405 S1) Restructure the model-initiated bash tool_call tail into an ordered, named policy list (BASH_COMMAND_POLICIES) evaluated by evaluateBashPolicies: first non-undefined verdict wins, undefined defers to the next policy, overall undefined allows. The only entry is the runtime-guardrails adapter over the byte-identical confirmCommand, so every verdict, block reason, emitted event sequence, headless fail-safe, and config precedence is unchanged. Exposed via __testing with order/short-circuit/allow tests. Work unit S1 of Gentleman-Programming#405: no new policy modules, no config schema change, no second tool_call handler.
…interface
The seam harness mocked ExtensionAPI["events"] as
{ emit: () => {}, on: () => {} } with an `as` cast, which trips TS2352:
EventBus.on must return an unsubscribe function, and a void-returning
stub does not overlap. Give the mock the real emit/on signatures so it
matches EventBus structurally and the cast disappears entirely, keeping
the typecheck ratchet baseline at 205.
|
Rebased onto current main: both content commits apply cleanly (the 12 commits of drift in gentle-ai.ts did not overlap the seam), and the autonomous-guard suite passes locally 56/56. |
Part of #405 (work unit S1 of 4; the issue stays open until S2-S4 land).
Problem
#405 asks for structured, ordered evaluation of high-risk model-initiated Bash operations. Today the bash
tool_calltail evaluates protections as an ordered if/else chain inside the single handler, which leaves no deterministic place to compose future policy modules (npm/pnpm package acquisition, destructive direct SQL) without duplicating authority or making precedence hard to reason about.What this adds
Unit S1 from the decomposition posted on the issue: a behavior-preserving evaluation seam.
BASH_COMMAND_POLICIES) evaluated byevaluateBashPolicies: first non-undefined verdict wins,undefineddefers to the next policy, overallundefinedallows.confirmCommand, so every verdict, block reason, emitted event sequence, headless fail-safe, and config precedence is unchanged.__testingwith order, short-circuit, and allow tests (tests/autonomous-guard.test.ts).No new policy modules, no second authority, no sandbox semantics; S2 (npm/pnpm recognition), S3 (probes and evidence), and S4 (direct SQL DROP) build on this seam as separate chained units.
extensions/gentle-ai.tsevaluateBashPolicies+ guardrails adapter (+63)tests/autonomous-guard.test.tsVerification
evaluateBashPoliciesseam tests: 5/5.tests/autonomous-guard.test.ts: 56/56.tests/gentle-ai.test.ts(integration, guards the unchanged-behavior claim): 36/36.main(127 commits, clean, no conflicts) withgit range-diffreviewed and all suites re-run green.Label request:
type:feature(pull-only author, maintainer needs to apply it).Summary by CodeRabbit
rm -rf /, remain blocked or require confirmation, while safe commands such asecho hellocan proceed.