Repository navigation
fix: close the v0.19.0..v0.21.2 audit findings - #768
Merged
Merged
Conversation
…ate severity An existing production equality method whose substantive base body is replaced by a constant `true` was reported as body-replaced-by-stub at the gate's severity up to v0.19.0. Since equality-made-constant took over that case it was capped at warning, so the run passed. A changed method now keeps the gate's severity again; an added equality method, one whose base body was already a bare constant, and one in test code stay capped at warning. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
parse_post_tool_outcome read any payload carrying tool_response, tool_result, toolResult or output without an exit-code field as exit 0. Codex sends tool_response as the string "Exit code: N ..." and Qwen Code puts "Exit Code: N" in tool_response.llmContent, so a failing cargo test was recorded as a passing verification and [hooks.verified-stop] let the stop through. The exit code is now read from numeric fields, from Exit code: N statements in the text result (any non-zero statement wins, so output that prints a passing line cannot mask a failure), from Copilot's <exited with exit code N>, and for Claude Code from a non-interrupted PostToolUse (documented: a failing tool goes to PostToolUseFailure). llmContent is read as output. An unknown exit code is never witnessed; Copilot resultType success alone no longer counts. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…es, heading prefixes, tracked targets truncate_claim sliced the claim at byte 57 and panicked (exit 101) when that byte fell inside a multi-byte character; it now counts characters. A ~~~ fence is a code fence, closed only by the same character at least as many times. A heading that starts with a designated heading (`## Root cause (summary)`, `## 2. Root cause`) stays in scope; `#word` is no longer a heading. A (verified: <path>) target must be tracked in the index: a file only on disk no longer resolves. The per-document check is now judge_document, which self-test runs with a positive and a negative control. GATES.md gains a What it does NOT catch list. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
The equality-made-constant carve-outs matched source text: a comment holding `#[derive(` and `PartialEq`, or the words `import mock` anywhere in the file, exempted a constant equality method. They now read syntax nodes: mock-framework imports from import statements, a mock base class from the class's bases, and generator markers from Rust attribute items and Java annotations. A hand-written equality method is never generated: @DataClass, attrs, pydantic BaseModel, Lombok, a Java record and Kotlin data classes leave a hand-written method in place of the one they would generate, and Rust refuses a hand-written impl beside a derive. Those markers no longer exempt it. A test double in a module named for test doubles (fakes.py, mocks.ts) is carved out, and one in tests/fakes.py keeps the carve-out when the file is judged as production code because it holds no test. Java `return Boolean.TRUE;` and any parenthesised constant (`return (true);`) are trivial bodies, so the Boolean.TRUE and parenthesis branches of the constant-true check are reachable. A self-test case covers a changed method made constant and a generated one. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
In refuse mode, a stop whose refusal could not be recorded against max_per_session (no session id in the payload, a session store that cannot be written, or .git/discipline/sessions/.lock held, a file the agent can create itself) was let through with exit 0. It is now refused, with a line saying why the refusal was not counted. stop_hook_active and agy executionNum are checked first, so the agent's loop guard still bounds the refusals; observe mode only logs. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…ibe the MDD form standard_normal_quantile: the docstring called Acklam's 1.15e-9 bound absolute; it is relative (measured max relative error 1.13e-9 against scipy.stats.norm.ppf; absolute error up to 2.9e-9 for |z| < 3 and 3.8e-8 near p = 1e-256). The tests pinned only fast-path constants, so a changed tail coefficient (C[5] 2.938 -> 2.948) survived the suite. New pins at p = 0.01, 0.99, 1e-6, 1e-12, 0.3, 0.7 (scipy, relative tolerance 2e-9) kill that mutant in test_standard_normal_quantile_pinned_reference_values. sample_size_for_interval: a half-width of 1e-300 gave n = inf, cast to usize::MAX. It now errors when n is not finite or exceeds MAX_SAMPLE_SIZE (1e9); sample_size_for_interval_refuses_an_unbounded_result fails on the old code (unwrap_err on Ok) and passes now. minimum_detectable_difference: the formula is unchanged (the locked pre-registration pins its values). The docstring now says it solves the unpooled Wald-type equation, not the Fleiss pooled-null formula, and states the measured gap (0.91% at n = 141, 0.54% at n = 245, up to about 9% at n = 10). Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…oc audit statistics The script docstring (and the #766 commit message) described a harvester, replay runner, rater packet builder and summary generator. None of that is in the file or anywhere in the repository, so manifest.json, findings.json, adjudication.json and summary.json cannot be regenerated from committed code. The docstring now says so. The sample-size guard of src/guards/perf/bounds.rs is mirrored (Python raised ZeroDivisionError at w = 1e-300). Post-hoc audit, not pre-registered (--post-hoc-audit [--write]): Fleiss pooled-null MDD, the design's pass probability and the n at which a Wilson lower bound clears 0.90, a Newcombe independent-sample interval for the structural-versus-twin difference, and PR-clustered intervals for the structural class (cluster-robust ratio and a PR-cluster bootstrap, seed 20261015, 20000 resamples). Each is pinned in --test against scipy, statsmodels or numpy values, and post-hoc-audit.json is checked against a fresh recomputation. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…n the base tree With [hooks.verified-stop] run_at_stop = true and no command configured, all four defaults (cargo test, pytest, npm test, go test ./...) had to pass, so a Rust-only repository was always refused. The defaults are now the runners whose marker (Cargo.toml; pyproject.toml, setup.py or pytest.ini; package.json; go.mod) is a file at the root of the base tree, so a marker the change adds vouches for nothing. With no vouched command the refusal says so and names the setting. docs/CONFIGURATION.md now lists where vouched commands come from (hooks.verified-stop.commands, [gates.command], [gates.test-floor] test_command, then the defaults) instead of the [commands.<id>] gates it named. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…labels
test_benchmark_summary_results_match_rust_bounds checked summary.json's
intervals and verdicts against its own counts only. It now joins
findings.json to adjudication.json's final labels, tallies them per gate,
per class (from the locked pre-registration) and for the twin, recomputes
the raters' exact agreement and Cohen's kappa from their individual
labels, and requires summary.json and adjudication.json to match (counts
exactly, kappa to 1e-6). Each summary gate row's class must match the
pre-registration.
Discrimination: with ITEM-0003's three labels flipped consistently from
false-positive to true-positive in adjudication.json, the previous test
file passes (6 passed) and this one fails ("twin: labels give 10 true
positives, summary.json says 9"); the data was restored. The new
flipping_one_adjudication_label_breaks_the_summary_cross_check flips a
discipline final label, a twin final label and one rater label in memory
and asserts each is reported.
Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
docs/CONFIGURATION.md said session files are written 0600 and flock-locked. The store wrote them with std::fs::write (0644 under the usual umask) and serialises writers with a create_new lock file that is removed as stale after 30 seconds. Session files and the lock file are now created 0600 on Unix (OpenOptions mode, then an atomic rename), and the docs describe the lock file as it is: an ordering between honest writers, not a security boundary. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
… it leaves out The verified-stop section said a turn is checked when files changed since HEAD; the tree digest diffs the working tree against the merge base of HEAD with the base branch, falling back to HEAD's tree. It now says so, and that files git ignores are excluded from the digest. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
With quote_line = false the label still appended the site's rule text, and for many packs that text runs past the rules: an ESLint description after `--`, a nolint or NOLINT comment body, a pylint or noqa trailing word, a clippy `reason = "..."`, a C# pragma or SuppressMessage justification, a Scala nowarn `msg=` filter, an annotation string with a second word. A token written there was echoed in every report format. The label now holds a fixed tool name and the rule identifiers, each spelled with letters, digits and _@/.:- only; the list stops at a description marker, a named argument, a word spelled otherwise, or a word that follows a rule with no comma (a flake8 code in a noqa excepted). quote_line = true still quotes the line. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
The unfailable-command check was one regex that matched only a trailing
true, exit 0 or :, so cargo test || echo ok, || /bin/true, (cargo test || true),
|| { true; }, || exit 00 and ||true passed. The pipefail check accepted the text
set -o pipefail anywhere, including inside a quoted string, and only pipes into a
fixed list of filters were checked.
A new shared module (src/guards/shell_masking.rs) parses the command with
tree-sitter-bash and reports: an || list whose right side cannot fail; under a
gate command (errexit off) a last command that cannot fail; set +e with no later
status check while errexit is on; and a pipeline with a failable stage before the
last while pipefail is off, whatever the filter. pipefail and errexit are read only
from real set command nodes. A command that does not parse, or nests deeper than
the analysis reads, gets a could-not-verify note.
gate-command-lint uses it through analyze_gate_command; a self-test case runs it
with positive and negative controls. Tests pin || : and ; : (audit M6).
Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…ysis
On GitHub, exit-code-masked was a substring test for '|| true', so ||true,
|| :, || exit 0, || echo failed and || /bin/true passed; on GitLab the list was
'|| true' and '|| :'. Text that only mentioned a mask (a comment, an echo) was
reported. Both paths now use src/guards/shell_masking.rs on the script read with
errexit on, after replacing ${{ }} and $[[ ]] template expressions byte for
byte. The checked set +e exemption is kept (status_checked, shared).
A changed script that does not parse, or a step whose shell is not bash or sh,
gets a could-not-verify note. The finding code and the GitLab directive subject
are unchanged; the GitHub finding is anchored at the masking line.
cmd; true is not reported in a CI script: errexit ends the script at the first
failure before true runs. (audit M7)
Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…_chars; test tracker titles A quoted text with fewer than min_quote_chars non-whitespace characters (new key, default 8) is reported as quote-too-short: a one-character quote was contained in the cited line and verified. A cited commit must be the base or one of its ancestors (GitCtx::is_reachable_from_base; HEAD when the run has no base). A commit only this change, another branch or the object database holds is reported as commit-unresolvable: evidence committed and deleted again inside one pull request verified before. The tracker-title check is now covered by tests over a canned forge (match, mismatch, missing issue, offline note, require_online error). Its line number was one short for a reference at the start of a line; lines are now counted in one pass. DISCIPLINE_NO_NETWORK is read as the forge client reads it, so 0 and false no longer switch the check off. The forge is identified once per run. AGENTS.md 3.3 lists the tracker-title check among the features that read a forge's REST API. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
Vitest's default v8 provider reads `/* v8 ignore next */` and `/* v8 ignore start */`, and Node's test runner reads `/* node:coverage ignore next */` and `node:coverage disable`. The JavaScript pack read only istanbul and c8. The new markers follow the same rule: they must open the comment, so prose that mentions one and a string that holds one are not sites. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
The rounding helper was a lambda behind noqa: E731, which suppression-delta flags; it is a nested function now. Output unchanged (--test passes, including the post-hoc-audit.json freshness check). Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…-hoc audit #482 Part B said "Done" and "human review, blind to the gate", while its own "Ships when" needs every number to resolve to a committed artifact and the harvest/replay/sampling/packet code is not committed. The row is now Partial: numbers resolve to committed JSON, the harness is not committed, rater provenance is unverified. Corrections from the data: dependency-delta has 7 vite false positives (not 6), so the named mechanisms cover 19 of 23; the 43 pii false positives are 21 RFC 1918 literals, 19 macOS home-directory paths in test code (ITEM-0334 among them) and 3 placeholder secrets (line text read at each head commit through the GitHub API). The twin gap is labelled PASS_categorical_by_design: the twin's winning regime (recall) was not measured. New post-hoc audit bullet, labelled not pre-registered: undeclared deviations (version 0.21.1 vs 0.20.3, a non-existent full evaluated_commit SHA, ruff-sh/ruff does not exist, 12 non-PR commits against merged-only, merge-commit head_sha, a wider twin path rule) and the INTERMEDIATE_amended_threshold_redesign label the pre-registration prescribes; PR clustering (design effect 6.0, effective n 24.0, bootstrap and cluster-robust intervals containing 0.90); a Newcombe interval for the twin difference; unverified rater provenance; design power 0.591 at n = 141, smallest clearing n 127, Wilson bound 0.8352 for 216/245. Numbers come from docs/benchmarks/results/post-hoc-audit.json. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
… ci-integrity gate-command-lint: how a command is read, what cannot fail, the forms caught, the could-not-verify note, and a What it does NOT catch list. ci-integrity: exit-code-masked now names the forms read from the script's syntax tree, the cmd; true case under errexit, and its blind spots. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
Since c469122 a changed file whose head side did not parse was noted and skipped, so with max_unsafe = 0 a change adding an unsafe block to a file nested past the parse-depth limit exited 0 once the other parse-strict gates were off. The budget is a count of every unsafe site, and a file with no facts cannot be counted: the gate now returns the parse error, which stops the run with exit 2 (could not check), as it did before c469122. A base side that does not parse is still noted and every head-side site is judged as new. The c469122 tests that expected a note are changed to expect the could-not-check, and the tree-depth and bounded-parse tests no longer list unsafe-budget among the gates that read what they can. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…n paths The gate checked only a changed file's path. git mv secure/policy.py src/ passed while deleting the same file was blocked; a rename is now judged at its source as well, and the finding names where the file moved. changed_files skips symlinks, so a new symlink under a forbidden directory passed. GitCtx::changed_files_including_symlinks lists them for this gate only; every other gate keeps reading changed_files. Plan-mode and path judgement are now planned_output_breaches and changed_file_breaches, which self-test runs with positive and negative controls. GATES.md describes plan mode under What it catches. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…er --disable A gate switched off by --disable was skipped unless the head file said `enabled = false` literally. A change that deleted the gate's `enabled = true` table, so that a gate off by default fell back to off, was then hidden by --disable in CI. The skip now parses the head file with the defaults applied and no command-line switch, and skips only when that file still runs the gate. A read or parse error stops the gate instead of being taken for a skip. GATES.md said --config-override is left out of the diff by default; the diff compares the configuration in force, so an override that loosens a key is reported. The text now says so, and that only --disable alone is left out. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…uns them GATES.md named `cargo fetch --locked` and `npm install --package-lock-only --ignore-scripts`; the presets run `cargo metadata --locked --format-version 1` and `npm ci --ignore-scripts --dry-run`, which does not write the lockfile. Each preset guards its manifest as well as its lockfile. The four presets are the whole set; there is no yarn, pnpm or pip preset. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
hook install never wired hook run --event post-tool, so the witnessed-runs path of [hooks.verified-stop] was unreachable from installed hooks and a stop could only be verified through run_at_stop. Each generated file now runs it after the agent's shell tool, by the names its recorded payloads carry: Bash (Claude Code, Codex), run_shell_command (Qwen Code), bash (Copilot CLI, repository and user-level files), run_command (agy), bash and shell (OpenCode plugin, which keeps 1.x arguments from tool.execute.before by callID). The witness never blocks, in either mode. post-tool returns before reading the base ref or the tree when the working tree's discipline.toml does not mention verified-stop, and the witness returns when the base ref does not enable it. The hook-integrity reader recognises the new entry (JsonCommand::PostTool) and the OpenCode mode reader ignores it like the session-start command. The committed hook files are regenerated with hook install --observe --upgrade. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…and config-integrity
…gnore markers in code (RUN: x), a bare RUN:, (RUN), [READ], `RUN`, **RUN** and -- RUN all satisfied a verification claim. A label is now RUN or READ, then :, an em or en dash, -- or a spaced -, then content naming a command, path or citation. Content is a placeholder when it is empty, a known placeholder word, a template slot such as <command>, a single filler word (done, ok), a first word of fewer than two letters or digits, or fewer than four letters, digits and path characters in all. A marker inside a code span that does not start with it (`echo RUN: x`) is code, not a label. No existing test asserted one of the removed forms; all pass unchanged. docs/benchmarks/claim-labels-precision.md now states that the study is in-sample, that recall is not measured, and that who Reader B was is not stated in the document or its history. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…de-constant direction and the live-session evidence note The v0.21.1 preset row named six resolve-locked-* presets; src/guards/presets.rs defines four (cargo-, npm-, uv-, go-resolve-locked). The v0.20.0 equality-made-constant row said stricter while it capped a changed equality method made constant at warning, which v0.19.0 blocked. The live-session row now names the committed fixtures as its evidence and states that the acted-on step and the Qwen Code 0.25.2 Stop behaviour have no committed artifact. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…liverable report The plugin generator's doc comment said an enforcing setup without tool.hook throws so the plugin fails closed; it only says so on stderr (console.error), which tests/test_agent_hooks.rs asserts (setupThrew == false). The comment now says what happens. withTimeout stops waiting for a command but cannot kill it, because Bun's $ returns a ShellPromise with no handle to the child process; the generated file and the doc comment now say so. In the 2.x execute.after handler, a failed check whose call.result has neither a text part nor a string output dropped the report silently. In enforcing mode it is now said on stderr (console.error). Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
The Package step fixed entry order, mtimes and the gzip header, but the archive still recorded the runner's build user and modes from the runner's umask, so the same commit packed on two runners could differ. Before archiving, directories are set 0755, files 0644 and the binary 0755; tar records owner and group 0 with no names (GNU tar on the Linux runners: --owner=0 --group=0 --numeric-owner; bsdtar on the macOS runners: --uid 0 --gid 0 --uname '' --gname ''). The listing after packing is verbose, so the job log shows the recorded owner and modes. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…nfinement and claim-labels # Conflicts: # src/selftest.rs
The end-to-end test of the Claude Code post-tool entry put the binary's directory on PATH, a spawn tests/test_isolation.rs does not allow outside its list. It now checks that the entry is the missing-binary guard followed by hook run --agent claude-code --event post-tool, and runs those arguments through common::discipline_cmd. Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
…dgers Adds unreleased Behaviour Changes rows for the hook, masking, research-gate, stub-bodies, suppression-delta, unsafe-budget, config-integrity and bounds fixes, and Default Changes rows for stub-bodies (changed equality method made constant) and citation-anchors (min_quote_chars). Signed-off-by: Nicolas Brousse <nicolas@brousse.info>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the defects found by an audit of
v0.19.0..v0.21.2. Closes #769. Refs #482.Hooks and session guards
[hooks.verified-stop]witnessed a failing run as passing when the post-tool payload carried a result but no recognised exit-code field (Codex"Exit code: 101\n..."string, QwenllmContent). A run now counts only when its exit code is stated; any non-zero statement wins.hook installnow wires each agent's shell tool tohook run --event post-tool, so the witness path is reachable from installed hooks.sessions/.lock) is refused instead of let through. The loop guards still apply.0600; the docs describe the real lock (acreate_newlock file, notflock), the merge-base comparison, the excluded gitignored files and the command sources.Exit-code masking
gate-command-lintandci-integrityexit-code-maskedread masking from the bash syntax tree through a sharedsrc/guards/shell_masking.rs.|| echo,|| /bin/true,||true,|| :,|| exit 0,|| exit 00,|| { true; },(cmd || true)andset +o errexit.pipefailcounts only from a realsetcommand.Research gates
mechanism-sections:~~~fences;(verified: …)targets to be tracked.citation-anchors:min_quote_chars(default 8) and finding codequote-too-short;DISCIPLINE_NO_NETWORKlike the forge client;scope-confinementjudges rename sources and symlinks.claim-labelsrequires a separator and real content, and ignores markers in code spans. Its precision study now states that it is in-sample.stub-bodies, suppression-delta, unsafe-budget, config-integrity
Boolean.TRUEand parenthesised constants are recognised.suppression-deltareports rule identifiers only; descriptions and justifications are no longer echoed. It also reads thev8 ignoreandnode:coveragemarkers.unsafe-budgetfails closed (exit 2) on a head side that does not parse.config-integritycompares the head file's effective enabled state under--disable.Bounds and benchmark
bounds.rs:sample_size_for_intervalerrors instead of saturating.adjudication.jsonand recomputes kappa.ROADMAP
unreleased.Verification
Each fix carries a test that was watched failing on the v0.21.2 behaviour and passing with the fix; mutation evidence is in the commit messages.
cargo fmt --check,cargo clippy --all-targets -- -D warnings,discipline docs --checkandscripts/benchmark_precision.py --testpass on the merged head, anddiscipline self-testgives 328 cases, 0 failed.cargo test --no-fail-faston the merged head: 141 test binaries, 4212 passed, 0 failed, 1 ignored.allow-agent-instructions: AGENTS.md lists the citation-anchors tracker-title check among the forge-reading features (§3.3)
allow-agent-instructions: .claude/settings.json gains the generated post-tool entry for Bash
allow-agent-instructions: .agents/hooks.json gains the generated post-tool entry for run_command
allow-agent-instructions: .github/hooks/discipline.json gains the generated post-tool entry for bash
allow-agent-instructions: .opencode/plugins/discipline.js gains the generated post-tool witness for shell tools
allow-swallow: .opencode/plugins/discipline.js the post-tool witness only records a passing run and must never refuse a tool call
allow-assertion-drop: the_opencode_plugin_registers_every_hook_in_both_blocks the plugin runs two more commands for the post-tool witness
allow-assertion-drop: the_opencode_observe_plugin_has_no_throw_and_guards_every_handler two more try/catch blocks for the post-tool witness
allow-assertion-drop: the_opencode_enforcing_plugin_refuses_on_a_non_zero_exit_and_reports_no_registration the witness try/catch and the dropped-report console.error change the counted forms
allow-assertion-drop: the_opencode_observe_plugin_never_throws_whatever_the_command_does two more post-shell handlers in the driver
allow-assertion-drop: positive_control_changed_equality_made_constant the changed equality method made constant is reported at gate severity again, so the run now exits 1 instead of 0