fix(audit): ai-safety-003 validate wiring; verify v0.5.1 self-dogfood slice holds - #45
Conversation
The audit kernel's fail-closed validator (registry membership, evidence grounding, recomputed overall, the zero-findings coverage rule) only ran behind `--operation validate`. `--operation render` parsed a report and printed it with no registry, no evidence grounding, and no shape checks, so a fabricated, drifted, or malformed report could reach human-facing markdown or HTML as if it were authoritative (issue #34, ai-safety-003). `render` now requires `--repo` and runs through the same `validate_and_parse` pipeline `validate` uses before it renders anything; a report that fails validation returns the same error `validate` would have produced instead of being rendered. Update the four cli_tests.rs cases that encoded the old "render never needs --repo" contract, and add a case asserting render fails closed on a report that fails validation. Docs updated to match: `--operation render` is now documented as requiring `--repo` and running the validate pipeline first.
… (ai-safety-003) The fail-closed validator existed and was well-tested but ran on no automated path: not CI, not the orchestrator, not the release workflow (issue #34, ai-safety-003). Add an additive "Validate any produced audit report" step, after each self-scan step in both ci.yml jobs and in release.yml, that looks for audit-report.json at the repo root, under orchestration/, and in the self-scan artifact directory, and runs `code-intel audit --operation validate --repo . --report <path>` on any it finds, failing the step on a validation error. Departments are agent-run today, not part of `run execute`, so no workflow currently produces an audit-report.json automatically -- this step is the structural guarantee for whenever one does show up, not an assumption that one exists yet. release.yml's "Validate packaged Skill bootstrap" step also gets an audit-report.json check against the extracted payload using the packaged binary, alongside the existing packaged `sentrux check`/`gate` calls, so release green, self-scan green, and published-artifact green keep referring to the same snapshot for audit evidence too, not just structural evidence.
Issue #14's v0.5.1 acceptance criteria ask for a regression check that fails CI if the dag_run/execution_kernel cycle (removed in #15) comes back, added to the existing cycle-detection mechanism rather than a new one. That mechanism already exists and is already wired into CI: the absolute `max_cycles = 0` rule in .sentrux/rules.toml, evaluated by sentrux_gate.rs's Tarjan-based rust_import_cycles against every push/PR self-scan. Add sentrux_gate::this_repository_has_no_resolved_import_cycles, which calls the same run_check() engine directly against this repository's own source tree as a plain `cargo test`. This gives a reintroduced cycle (this pair or any other) a seconds-fast failure signal instead of waiting for a full release-mode self-scan build. crates/code-intel-cli/src/sentrux_gate.rs is itself pinned by a supply-chain provenance record (orchestration/internalization/sentrux.json, ownedModifications + operationTrace, checked by tests/internalization_record.rs). Editing the file to add the test changes its SHA-256, so the recorded native-gate-source-sha256 digest is updated to match -- the same maintenance step the file's own history (c3a8ca2) took the last time this file changed. This is a provenance identity record, not the structural quality baseline; updating it to reflect a reviewed, intentional, additive source change is expected and is a different action from regenerating .sentrux/baseline.json to paper over a regression.
Document the render/CI/release validate wiring under [Unreleased], and record (with pointers to the evidence, not just an assertion) that the v0.5.1 self-dogfood acceptance criteria from issue #14 -- the dag_run/execution_kernel cycle at zero under an absolute rule, CI and release self-scan against the real compiled binary, and packaged/ self-scan/release snapshot identity -- were already landed by #15 and held through #38/#42, verified against current main rather than re-implemented here.
…tural violation Acceptance criterion 3 from issue #14 (Hospital must name the first failed rule, evidence, target files, and smallest rerun command on a structural failure) was already implemented by hospital_diagnosis.rs's treatment()/render_hospital() -- but no existing test exercised it: the structural() fixture helper in this file only ever seeds `failure:{"kind":"none"}`, even for a "fail" verdict, so the precedence matrix test only checked the diagnosis string, never the rendered text. Add architecture_gate_failure_names_the_rule_targets_and_smallest_rerun_command, which seeds a structural admission carrying the same shape a real max_cycles violation on dag_run.rs/execution_kernel.rs produces, and asserts hospital.md contains the failing rule and message, the target files, a dedicated "## Failing rules" section, and the literal "Rerun the smallest gate: code-intel sentrux --operation check --repo <repo-root>." line -- and that a surgery plan is produced. This closes the coverage gap rather than just re-reading the source to confirm the behavior exists.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
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 |
…rux_gate.rs The cycle regression guard added to sentrux_gate.rs changed its sha256; orchestration/internalization/sentrux.json was re-pinned in the same change but the provider.sentrux-adapt toolchainDigests entry in orchestration/integrations.json was missed, failing test-atomic-capability-contract.ps1 on every platform. Recomputed and verified via the contract test (ok: true, 11 toolchain evidence capabilities checked).
…olution Resolving the sentrux.json conflict with a whole-file --theirs take kept main's post-#45 sentrux_gate.rs digest but discarded this branch's own re-pin for the edited sentrux_analysis.rs, failing internalization_record.rs ticket_r03 on every platform. Recomputed and swept all 35 internalization records for stale digests (only this one). Full suite green locally: 44 suites, 2082 passed, 0 failed.
…aps (#56) * fix(security): resolve tool paths absolutely, close digest evidence gaps security-004: rg/git launched via Command::new(bare-name) with current_dir() set to the repo under analysis, letting a scanned repo's own rg.exe/git.exe win the Windows PATH search. Add crates/code-intel-cli/src/tool_path.rs, a shared absolute-path resolver reusing doctor_adapter.rs's existing tool-inventory candidate/PATH-scan logic, and route every production Command::new("rg")/Command::new("git") call site through it (hardened_git.rs, sentrux_analysis.rs, snapshot.rs, capability_inventory.rs). doctor_adapter.rs's tool_available() now delegates to the same resolver so presence-checking and launch-time resolution can't drift apart. Mirrored the same fix in tools/sentrux-shim/sentrux-shim.ps1's rg/git invocations. supply-chain-008: 4 of 14 orchestration/integrations.json capability entries (doctor.envelope.compat, repository.snapshot.compat, advisory.workflow-recommend.compat, inventory.rg.compat) declared toolchainDigests with no toolchainDigestEvidence.inputs, so they could not be drift-checked. Added evidence.inputs mapping each entry to the actual source file(s) implementing it, and recomputed toolchainDigests via sha256 over those inputs. Because tool_path.rs changed the byte content of doctor_adapter.rs, capability_inventory.rs, and snapshot.rs, their digests moved; that shift cascades through crates/code-intel-cli/tests/artifact_ref.rs and tests/capability_exec.rs (hardcoded request fixtures validated against the registry by capability.rs's cohere()), through orchestration/internalization/{git,rg,ast-grep,sentrux}.json (provenance records whose sha256 fields are live-recomputed and asserted by internalization_record.rs/ast_grep_internalization.rs), and through run-code-intel.ps1 (a hardcoded advisory.workflow-recommend.compat request fixture). All of these were updated to keep cargo test -p code-intel green; none of them are logic changes, only digest-value corrections. supply-chain-009: tools/sentrux-shim/sentrux-shim.ps1 auto-activated a fabricated local Pro-tier license for third-party sentrux on every bootstrapped machine, opt-out only, with no upstream license evidence anywhere in the tree to justify it. No genuine provenance could be found, so auto-activation is now opt-in: it only fires when SENTRUX_AUTO_PRO is explicitly set to an affirmative value ("1"/"true"), default off. Addresses security-004 from #33, supply-chain-008/009 from #35. * docs: align Sentrux Pro docs with opt-in default supply-chain-009 flipped SENTRUX_AUTO_PRO from opt-out to opt-in in this branch; README.md and the install-plan text still described auto-activation as the default. Update both to the opt-in reality. * chore(sentrux): record the tool_path module cost in the ratchet baseline The shared tool_path resolver (security-004) adds one module imported by six call sites, moving coupling 45.13 -> 45.33 and quality 3968 -> 3967. This is the fix's intrinsic structural cost — the same documented re-baseline procedure used for the ast-grep internalization suite (45.07 -> 45.13). Cycles and god files unchanged. Gate and full self-scan verified green locally after the re-baseline. * fix(install): accept free tier when SENTRUX_AUTO_PRO is not opted in Flipping Pro auto-activation to opt-in left three health checks still requiring Tier: pro unconditionally, so a default install now failed its own installer verification, the tool:sentrux-pro required check, and check-code-intel-tools' missing-list (CI Install portable pipeline step red on all four runners). The expected tier now follows the opt-in: pro when SENTRUX_AUTO_PRO is affirmative, pro-or-free otherwise, mirroring the shim's own truthiness set. * fix(provenance): restore sentrux_analysis.rs digest lost in merge resolution Resolving the sentrux.json conflict with a whole-file --theirs take kept main's post-#45 sentrux_gate.rs digest but discarded this branch's own re-pin for the edited sentrux_analysis.rs, failing internalization_record.rs ticket_r03 on every platform. Recomputed and swept all 35 internalization records for stale digests (only this one). Full suite green locally: 44 suites, 2082 passed, 0 failed.
What this PR actually does
This branch was cut from current
main(b73c16a, confirmedHEAD == origin/mainbefore starting). At that commit, four of the five acceptance criteria this PR was scoped to were already satisfied by prior merged work (#15, then hardened through #38/#42) — verified here against the live tree, not re-implemented. Only ai-safety-003 (from #34) was a genuine gap, and that's the real new work in this PR.Verified as already satisfied (evidence, not assertion)
dag_run/execution_kernelcycle removed.grep execution_kernel crates/code-intel-cli/src/dag_run.rsreturns nothing;execution_kernel.rsimportsdag_runone-directionally (use crate::dag_run::{self, DagExecutionRequest};). Ownership was re-partitioned in Self-dogfood release gates: built-in structural engine, cycle removal, actionable Hospital #15 (CLI front-end out ofdag_run.rs,RunErrorto a leaf), keepingexecution_kernelas the execute+publish composition point — not any of the three options this PR's brief offered, per Self-dogfood release gates: built-in structural engine, cycle removal, actionable Hospital #15's own review notes..sentrux/rules.tomlpinsmax_cycles = 0as an absolute rule (not just a baseline ratchet), enforced bysentrux_gate.rs's Tarjan-basedrust_import_cycleson every CI self-scan. Live proof from this session:sentrux_gate::this_repository_has_no_resolved_import_cycles, acargo testthat calls the samerun_check()engine directly against this repo, so a reintroduced cycle (this pair or any other) fails in seconds instead of waiting for a release-mode self-scan build — using the existing mechanism, not a new one, per the issue's own instruction.ci.yml(both jobs) andrelease.ymlrun the actual compiledcode-intel.exe run execute --repo .against the real checkout and hard-fail the job on a nonzero exit.release.ymlpackages viagit archive HEADspecifically to avoid drift from earlier mutating steps, then re-validates the packaged binary against the packaged payload (sentrux --operation check|gate --repo $payload) with an explicit "release green, self-scan green, and published artifact green must refer to the same snapshot" comment already in the file.hospital_diagnosis.rs'streatment()/render_hospital()already name the failing rule, message, targets, and rerun command, but no existing test exercised that rendering (thestructural()fixture helper only ever seedsfailure:{"kind":"none"}, even on a "fail" verdict). Addedarchitecture_gate_failure_names_the_rule_targets_and_smallest_rerun_command, which seeds a violation shaped exactly like a realmax_cycleshit ondag_run.rs/execution_kernel.rsand assertshospital.mdcontains:New work: ai-safety-003 (#34)
The audit kernel's fail-closed validator (
code-intel audit --operation validate) existed, was well-tested, and was called by nothing automated — not CI, not the orchestrator, not the release workflow.--operation renderdidn't even accept--repo, so a malformed or fabricatedaudit-report.jsoncould reach human-facing markdown/HTML unvalidated.crates/code-intel-cli/src/audit_report/cli.rs:--operation rendernow requires--repoand runs the exact samevalidate_and_parsepipeline--operation validateuses (registry load + self-check, evidence grounding, report-shape rules) before producing any output. A report that fails validation returns the same errorvalidatewould.ci.yml(both jobs) andrelease.ymleach gained an additive "Validate any produced audit report" step that looks foraudit-report.jsonat the repo root, underorchestration/, and in the self-scan artifact directory, runningcode-intel audit --operation validateon any it finds and failing the job on a validation error. Departments are agent-run today, not part ofrun execute, so this is a structural guarantee for whenever one shows up, not an assumption one exists yet.release.yml's existing "Validate packaged Skill bootstrap" step (the one that already re-checkssentrux check/gateagainst the packaged payload) also now checks for a packagedaudit-report.json, using the packaged binary against the packaged payload — same snapshot-identity discipline as the structural checks right next to it.Acceptance criteria checklist
git archive HEAD+ packaged-binary-against-packaged-payload re-check).dag_run/execution_kernelcycle removed AND a regression check prevents its return, using the existing mechanism — already true (absolutemax_cycles = 0); added a fastcargo testagainst the same engine for a quicker feedback loop.--repoand validates first — new work, this PR.Verification
cargo fmt -p code-intel -- --check— clean.cargo check— clean (pre-existingdead_codewarnings instaged_artifact.rs, untouched by this PR).cargo test -p code-intel— 1833 passed, 0 failed, 44 suites (full suite, run twice to confirm; one run hit 2 unrelated transient failures inruntime_ci_evidence.rs— a Windows path race in an untouched file — that did not reproduce on rerun).cargo test -p code-intel --test project_orientation_benchmark— 2 passed (mirrors the dedicated CI step).code-intel.exe run execute --repo . ..., same invocation asci.yml): exit 0,evidence.sentruxverdictpass, HospitalStatus: green / Primary diagnosis: clean snapshot / Next protocol: post_op, raw sentrux outputCycles: 0 -> 0,All rules passed.renderwithout--repo->{"error":"--operation render requires --repo","ok":false}(exit 65);render --repo .-> same markdown as before; a corruptedoverallscore -> rejected byvalidate(exit 65) and by the new CI step logic (simulated locally inpwsh).python -c "import yaml; yaml.safe_load(open(...))"on both edited workflow files.A fix this PR needed but wasn't asked for
Editing
sentrux_gate.rs(to add the cycle regression test) changed its SHA-256, which broketicket_r03_sentrux_record_blocks_shim_retirement_on_windows_and_plugin_gaps— a supply-chain provenance test that pins that file's exact digest inorchestration/internalization/sentrux.json. Updated the two recorded digests to match, the same maintenance step the file's own history (c3a8ca2) took the last time this file changed. This is a provenance identity record, not the structural quality baseline (.sentrux/baseline.json) — updating it to reflect a reviewed, intentional, additive source change is expected, and is a different action from regenerating the baseline to paper over a regression.Honesty note on scope
This PR intentionally avoids GitHub closing syntax for the two issues it touches. Issue 14 is a multi-release roadmap (v0.5.1/v0.6.0/v0.7.0) and only the v0.5.1 slice is in scope here, so it should stay open. Issue 34 lists six ai-safety findings and this PR addresses only ai-safety-003; findings ai-safety-002/004/005/006 are separate and fixed elsewhere, so issue 34 should also stay open. This PR implements the ai-safety-003 slice of issue 34 and verifies-and-hardens (rather than re-implements) the v0.5.1 self-dogfood criteria from issue 14 that a prior session already landed.
Files not touched, as instructed:
orchestration/audit/prompts/*,Invoke-ModelChannelDelegate.ps1,crates/code-intel-cli/src/sentrux_analysis.rs,crates/code-intel-cli/src/doctor_adapter.rs.