Repository navigation
feat(panicbot): run the full panic-attack suite unless the directives say otherwise - #671
Conversation
… say otherwise Owner ruling 2026-11-13: panicbot ran `assail` only. It now runs every class (a) panic-attack subcommand (scan/analysis, non-interactive, findings or verdict), inventoried in bots/panicbot/src/suite.rs with the reason for each subcommand that is out (interactive, infrastructure, or held back). - AllowedCommands covers assail, assay, adjudicate, diagnostics, attack, assault, ambush, amuck, abduct, axial; the default is all of them. - Directives: no file gets the full suite. A file keeps exactly what it allows and denies (no `allow` key keeps the static trio it always granted; `deny` now narrows; deny wins). Unknown names are ignored with a printed note. A directive file that does not parse stops the run instead of falling back to the (now full) default. - Each part ends ran / not_applicable / denied / error, never a silent skip. Dynamic modes need an explicit --target (an executable inside the scanned directory); without one they report not_applicable with the reason. They are refused under pull_request_target and workflow_run. - New findings name their file: assay hits (PB-ASSAY-*), dynamic results and the adjudicated verdict (PB-DYN-*, PB-ADJUDICATE-VERDICT) name the target. - A failed part makes `scan --format json` print an error object and exit 3; a run that analysed nothing prints not_run and exits 4. The report-back driver maps both to envelope statuses; a findings array stays a complete run. - Whole-suite budget (240 s, under the driver's 300 s limit); each invocation gets the smaller of its timeout and what is left. panic-attack runs in a scratch cwd so a scan does not write into the repository. - `adjudicate` prints `pass (processed 0, failed 1)` and exits 0 for a missing or unrecognised report; panicbot treats that as an error, not a pass. - `diagnostics` --version had no timeout and could hang the lane; it is timed. Advisory gating is unchanged (owner ruling G2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
|
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9761631e3e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub fn run_assay(target: &Path, timeout: Duration) -> Result<AssayReport> { | ||
| let scratch = Scratch::new()?; | ||
| let t = target.display().to_string(); | ||
| let value = run_to_report(&["assay", &t], &scratch, timeout)?; |
There was a problem hiding this comment.
When dir is relative—especially the advertised default panicbot scan .—run_to_report changes the subprocess working directory to the new scratch directory, so the relative assay target is resolved there instead of against the caller's repository. An empty candidates report is accepted as successful, meaning the suite can mark assay as ran while silently omitting all substitution findings; pass an absolute/canonical target before entering the scratch directory.
Useful? React with 👍 / 👎.
| .collect(); | ||
|
|
||
| // If diagnostics subcommand exists, use it | ||
| match invoke_panic_attack(&["diagnostics"], timeout) { |
There was a problem hiding this comment.
When --version succeeds slowly and the subsequent diagnostics command is also slow, each subprocess receives the full remaining suite timeout. A diagnostics part given the default 240-second remaining budget can therefore run for nearly 480 seconds, violating the suite's 240-second budget and causing the outer 300-second report-back timeout to kill the adapter instead of producing a structured part error; use a shared deadline or pass only the remaining duration to the second call.
Useful? React with 👍 / 👎.
| pub fn parse_dynamic_report(value: serde_json::Value) -> Result<DynamicReport> { | ||
| let report: DynamicReport = serde_json::from_value(value) | ||
| .context("panic-attack dynamic run wrote an unexpected report shape")?; | ||
| if report.attack_results.is_empty() { |
There was a problem hiding this comment.
When panic-attack returns one result per requested axis but marks every result skipped: true—for example because the current platform cannot apply any attack axis—the vector is nonempty, so this validation accepts it and the suite records assault or ambush as Ran. The report is then also passed to adjudication even though no target execution was observed, creating a false successful dynamic-analysis result; require at least one non-skipped axis or classify the part as not applicable/error.
Useful? React with 👍 / 👎.
| let mut out = Vec::new(); | ||
| let mut unlocated = 0; | ||
| for c in &report.candidates { | ||
| if c.status == "already-applied" { |
There was a problem hiding this comment.
When an assay candidate has target hits but its documented status is no-replacement-source, this condition lets it through and emits a Proven-library substitution candidate claiming that a proven drop-in equivalent exists. That produces an unactionable and factually misleading finding for a replacement panic-attack says is unavailable; only offered candidates should use this finding classification, while unavailable statuses should be skipped or reported distinctly.
Useful? React with 👍 / 👎.
Stacks on #670. Owner ruling 2026-11-13: panicbot ran
assailonly; it now runs every class (a) panic-attack subcommand unless the repo's bot directives say otherwise.What changed
bots/panicbot/src/suite.rs: inventory table of every panic-attack subcommand (class a / b / c, with reasons) and the suite runner. Each part endsran,not_applicable(with reason),deniedorerror; nothing is silently skipped.assail,assay,diagnostics(preflight only). Dynamicassault/ambushrun only with an explicit--targetexecutable inside the scanned dir, elsenot_applicable.adjudicateruns only over dynamic reports.attack,amuck,abduct,axialreportnot_applicablewith reasons (no file/exec source).allowkey keeps the old static trio;denynarrows, deny wins). A directive file that fails to parse now stops the run instead of falling back to the (now full) default.pull_request_target/workflow_run. The checked workflows (fleet-report-back-reusable.yml, standardsfork-governance.ymlfleet job) run onpull_requestwithcontents: readand no secrets, and none passes a target.PB-ASSAY-*(hit file),PB-DYN-*andPB-ADJUDICATE-VERDICT(the target). Gating stays advisory (G2).not_runand exits 4. The driver maps both to envelope statuses; evidence classes added forPB-ids.adjudicateprintspass (processed 0, failed 1)and exits 0 for a missing or unrecognised report, which panicbot now treats as an error; thediagnostics--versioncall had no timeout.Held back / for the owner
bridgeis class (a) in kind but held out per D4 (bridge gate stays outside panicbot).panicbot.a2mlis read-only as before.Checks
cargo fmt --check,cargo clippy --all-targets -- -D warnings,cargo testinbots/panicbot;tests/fleet-report-back-test.sh(213 passed) andtests/fleet-report-back-build-test.sh(65 passed) withSTANDARDS_DIRset.Generated by Claude Code