Repository navigation
fix(trust): per-baseline audit lookup, per-finding override tier gate, fatal optional-pack pin (#1030) - #1094
Open
pengfei-threemoonslab wants to merge 4 commits into
Open
pengfei-threemoonslab wants to merge 4 commits into
pengfei-threemoonslab wants to merge 4 commits into
Conversation
Every baseline in a directory appends to one baseline-audit.log, and verify_baseline compared the file with the log's last row whichever baseline wrote it. Saving baseline-prod.json turned an untouched baseline.json into a critical SHIP-BASELINE-INTEGRITY-MISMATCH, while a copy of the baseline saved last, or a renamed baseline, verified clean. Verification now uses the latest row recorded for that baseline's file name (the final component of baseline_path, either separator, which is how released versions recorded it). A log with rows but none for the baseline is missing_audit_log, titled with the file, what the log records and the baseline save that records it. Released audit logs verify unchanged. The hash reads CRLF and lone CR as LF, as it always has; the docstring that claimed line endings were detected is corrected, and the normalisation is explicit and reads through the static-input snapshot.
…tted at (#1030) The severity-override tier gate compared a downgrade with the catalog default. SHIP-SIDEFX-IDEMPOTENCY-MISSING is critical when a tool's retry policy is known (default high), so an override to high lowered it with no acknowledgement; a deprecated alias whose own default is lower than its expansions did the same. resolve_severity_overrides now takes the scan's pre-override findings. When an override would lower a finding it governs that was emitted above both defaults, that severity is the tier-crossing base. Governance is one rule, governing_configured_check_id, shared with apply_severity_overrides. The base never drops below the defaults.
load_policy_packs downgraded every load error of an optional pack to a warning, so a sha256 pin mismatch, or a pinned pack replaced by a malformed file, a directory or a link, dropped the pack's rules with only a warning. optional now tolerates exactly one failure: nothing exists at the declared path. Anything else is a config error, as for a required pack. Every pack-loading command goes through this loader.
Migration note #trust-config-1030; Baseline Integrity (which row vouches, what the hash covers), Severity-override floor (per-finding tier gate) and the loaded_policy_packs pin line updated.
Contributor
Author
🤖 Engineering review — implementer self-reviewI reviewed Can a baseline still be tampered with undetected?
Can an override still lower a critical side-effect finding another way?
Does any pack-loading path still downgrade a pin mismatch?
Do released baselines and audit logs still work?
Left for the owner
Fixed during self-review
|
This branch has not been deployed
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
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.
Closes #1030
Summary
Three trust-configuration checks compared against the wrong reference. Each now compares against the right one, and each fix extends the mechanism it repairs instead of adding a parallel rule.
core/baseline_audit.py,core/baseline.py). Every baseline in a directory appends to onebaseline-audit.log, andverify_baselinecompared the file with the log's last row, whichever baseline wrote it. Verification now uses the latest row recorded for that baseline. The match is on the final component of the row'sbaseline_path, with either separator, because released versions record the path exactly as the saving command spelled it: relative to that command's working directory, absolute, or Windows-style. If the log has rows but none for this baseline, the result ismissing_audit_log(critical, exit 6 under--strict). Its title names the baseline, the files the log does record, and thebaseline savethat records it. Verification never falls back to another baseline's row.normalize_baseline_line_endings) instead of a side effect ofread_text, and the bytes come through the static-input snapshot, so the hash covers exactly the bytesload_baselineparsed.core/severity_overrides.py,core/check_ids.py,cli/scan/decision.py).resolve_severity_overridestakes this scan's pre-overridefindings. If an override would lower a finding it governs that was emitted above both the catalog default and the dynamic default, that finding's severity becomes the tier-crossing base. This extends the existingmax(catalog, dynamic)base and adds no new rule. "Governs" is a single rule,governing_configured_check_id, whichapply_severity_overridesalso uses: the exact id first, then the first configured alias or umbrella that expands to it. So the gate judges exactly the findings the override will change. The base never drops below the old one, so every override that needed an acknowledgement still needs one. An override that lowers no emitted finding (for example one tocritical) keeps the audit row it had.inputs/policy_packs.py).optional: truenow tolerates only absence: nothing at all exists at the declared path, checked lexically, so a dangling link counts as present. Everything else (a pin mismatch, an unreadable or non-regular file, a path outside the manifest directory, a malformed or invalid pack) raisesConfigError, the same as for a required pack.load_policy_packsis the only loader.scan,verify(head and base),doctor/inspect_sources,baseline saveand the Action (throughscan/verify) all reach it.org status/org policy-packsonly report pin status, as thepolicy_pack_pin_mismatchviolation they already emit.Docs: STABILITY migration note
#trust-config-1030, § Baseline Integrity (which row vouches, what the hash covers), § Severity-override floor (per-finding tier gate), and theloaded_policy_packsline. Alsodocs/baseline.md,docs/policy-packs.md,docs/manifest-v0.1.md,docs/organization.md,docs/checks.mdand the check catalog'sfires_when/evidence_fields, withdocs/checks.jsonandllms-full.txtregenerated. CHANGELOG has an## Unreleased›### Changesentry.Acceptance criteria
baseline verify --stricton the other at exit 0, and tampering with either is still detected.tests/test_baseline_integrity.py::test_two_baselines_in_one_directory_verify_independently: realbaseline save×2 (the second with--owner other-team) on a copy of the support-refund sample. Bothverify --strictexit 0,scan --baseline baseline.jsonemits noSHIP-BASELINE-INTEGRITY-MISMATCH, and tampering with either baseline exits 6 withhash_mismatch. On the unfixed source it fails with[critical] hash_mismatch … exit 6.test_a_sibling_baselines_content_is_not_vouched_for_by_its_row: copying the baseline saved last over another one is nowhash_mismatch. Before the fix it verified clean.samples/large_multi_framework_agent:verify --baseline baseline.json --strictwent from exit 6 to exit 0.test_baseline_hash_reads_line_endings_as_lf_as_its_docstring_stateschecks the docstring's statement and the behaviour together. CRLF and lone-CR conversions verify clean, including--strictexit 0. A content edit made under CRLF is stillhash_mismatch, and a BOM is refused byload_baseline. On the unfixed source the docstring assertion fails.SHIP-SIDEFX-*finding without acknowledgement is rejected.tests/test_severity_override_floor.py::test_scan_rejects_unacknowledged_override_of_dynamically_critical_sidefxruns end to end on the support-refund sample (stripe.create_refundhas a retry policy, so the finding iscritical).SHIP-SIDEFX-IDEMPOTENCY-MISSING: highnow raisesConfigError(exit 2). With anacknowledge_overridesentry the override applies andpolicy_auditrecordscritical → high,downgrade,tier_crossed: true, with the reason. On the unfixed source:DID NOT RAISE ConfigError.test_no_builtin_check_emitted_above_its_default_lowers_without_ack(every built-in check belowcriticalacross the catalog).tests/test_policy_packs.py::test_optional_policy_pack_with_wrong_sha256_is_an_error: a verified pin loads. After an edit that dropsblock: true,scanraises the pinConfigErroranddoctor --jsonexits 2. On the unfixed source:DID NOT RAISE.test_optional_policy_pack_missing_with_a_pin_still_warnsand the existingtest_optional_missing_policy_pack_warnscover the missing case.test_optional_policy_pack_that_exists_but_does_not_load_is_an_error[malformed|directory|dangling_link]covers the rest, pinned and unpinned, and fails on the unfixed source.I proved every acceptance test fails without the fix by checking
src/out at the branch base, rerunning, and restoring. Result: 11 failed, all with the bug's own symptom (no import or attribute errors).Design decisions (please check)
core.autocrlf=trueis the installer default, and GitHub's Windows runners use it) into a critical, unsuppressibleSHIP-BASELINE-INTEGRITY-MISMATCH. That is the same false-critical class this issue fixes. A line-ending conversion cannot change the parsed baseline: JSON treats CR and LF as insignificant whitespace and cannot carry either raw inside a string. It would also break every released Windows-written audit row, because releasedhash_aftervalues are LF-normalised. Every content edit is still detected. If you want bytes anyway, it is a two-line change plusnewline="\n"on the writer. The cost is a documented.gitattributesrequirement (.agents-shipgate/** -text) for Windows adopters.baseline_pathrelative to the saving command's working directory, so nothing richer can be matched across machines without a schema change.BaselineAuditEntryisextra="forbid", so a new row field would break every released reader of a log this version writes. The known limit: two same-named baselines in different directories can share one log only throughbaseline.audit_log, and they are not told apart. When that happens it fails closed as a false mismatch, which is the pre-fix behaviour. It is documented in STABILITY and the docstring.optionaltolerates only a missing file, as the issue's proposed fix says. Narrowing it to "pin mismatch only" would have left the same bypass open through a pinned pack swapped for a symlink, a directory or an oversized file, each of which raised a non-pin error thatoptionaldowngraded. A present-but-malformed optional pack is now exit 2. That is a deliberate break and is in the migration note.verify, a base whose manifest trips the gate makes the base scanscan_failed, which leaves the head gate unchanged (existing handling), so the fixing PR can still go throughverify.Type
No new public surface: no CLI flag, schema version, check ID, control state, or exit code. Two additive evidence keys go on the existing
SHIP-BASELINE-INTEGRITY-MISMATCH(recorded_baselines,latest_audit_baseline_path), listed in the catalog'sevidence_fields. Sample goldens are unchanged (regenerate_goldens.py --check: 24 artifacts, 0 changed), anddocs/distribution-surfaces.mdneeds no row change.Authorship
Verification
Run locally on a scratch venv built from
constraints/dev.txt(--require-hashes, pydantic 2.14.1) pluspip install -e . --no-deps.agents_shipgate.__file__is under this worktree. Branch base:b980d4a981(after #1093).pytest -n auto -m "not perf" --ignore=tests/test_adapter_static_only.pytest_check_unmodelled_host_config_keys.py[local_settings_enabled_plugins-{worktree,git_range}]:git commit -qm basefails because this machine's global gitignore hides the fixture file. Totals: 23316 passed, 14 skipped, 120 xfailed.pytest tests/test_adapter_static_only.pypytest tests/test_latency_budget.py -m perfpyteston every test file that touches overrides, baselines or packs (19 files), rerun after the last gate refinementpytest tests/test_distribution_surface_parity.py tests/test_install_hooks.py tests/test_cold_start_replay.py tests/test_semantic_cold_start.py tests/test_cold_reader_order.py tests/test_public_surface_contract.py tests/test_regenerate_goldens.py tests/test_schema_roundtrip.pyruff check .python -m compileall -q src testspython scripts/regenerate_goldens.py --checkpython scripts/generate_schemas.py --checkdocs/checks.jsonregenerated for the catalog text)python scripts/build-llms-full.pydocs/checks.mdis a source)src/at the branch baseIssue reproductions before → after: #11
verify --stricton the untouchedbaseline.json6 → 0. #37 scan accepted →ConfigErrornaming'critical' → 'high'. #41loaded=0plus a warning →ConfigErrorfor the pin mismatch. #109 is unchanged (exit 0) by design, and the docstring now says so.Release-readiness notes
docs/checks.md(no new IDs; theSHIP-BASELINE-INTEGRITY-MISMATCHtext is updated)STABILITY.md🤖 Generated with Claude Code