diff --git a/CHANGELOG.md b/CHANGELOG.md index e0d813e9..b890f082 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - **`SHIP-MCP-ENV-SECRET-PASSTHROUGH` counts a secret an MCP server forwards by name, not only an `env` entry.** A secret-like variable in Codex's `env_vars` list or `env_http_headers`, any `bearer_token_env_var`, and a `${NAME}` reference in a `headers` value (`.mcp.json`) now fire the finding (an editor's `${env:NAME}` is read as `NAME` there and in `env`, and a prompted `${input:…}` names no variable) and add to the server's risk score as an `env` entry always did. A credential-named header (`Authorization`) counts the variable it carries whatever that variable is called, and a value written where a variable name belongs is redacted in the evidence, not echoed. (#1025) - **A `codex_config` source names the other hosts' MCP config files it does not read.** Its coverage said it read `.mcp.json`, `.codex/config.toml` "and the other host MCP config files" under its root; it reads only the first two. A `.cursor/mcp.json` or `.vscode/mcp.json` there is now named in a source warning, so a workspace whose servers live only in one no longer passes silently, and the coverage text in [`docs/determinism-boundary.md`](docs/determinism-boundary.md) names exactly the files read and the files named. Reading those files is #663. (#1025) - **`verify` stops when it has no base, and a deleted manifest is a removed gate.** With neither `--base` nor `--no-base`, a run whose detection found nothing (a default branch other than `main` or `master` with no `origin/HEAD`, or a repository with no remote) read no committed change, published that empty change set as `complete` and granted merge: a pull request deleting `.github/workflows/agents-shipgate.yml` on a `develop` repository exited 0 as `mergeable`, and exited 20 as `blocked` once `--base origin/develop` was named. Detection keeps every base it found before and otherwise follows `check`'s ladder (#649): a default branch already at the head is an empty comparison, and a repository with no remote uses its local `main` or `master`. When nothing resolves, the run exits 2 with `base_status: ref_missing`, a `diff_status` of `unavailable`/`refs_missing` whose detail begins `base_unresolved`, a `fetch_base` next action naming `--base` and `--no-base`, and every permission denied. A change deleting the configured manifest that its merge base (or `HEAD`, for a working-tree run without a base) holds is a human `stop` naming the removal, exit 2, instead of the `configure` and `init --write` route; a missing manifest whose base cannot be resolved fails closed as above, one whose base holds it but cannot be compared (a shallow merge base), or whose named `--base` is not available locally (the Action's `diff_base: target` without `fetch-depth: 0`), fails closed with that diff-input `fetch_base` stop in both CI modes, and a repository that never had one keeps the setup route (with an unavailable named base, only where complete history, neither shallow nor grafted, shows the manifest never existed; a depth-1 checkout with an unfetched `--base` therefore stops in advisory mode too, where it used to exit 0 on the host route). `SHIP-VERIFY-CI-GATE-REMOVED` resolves changed paths against the evaluated tree's root instead of the manifest's directory, so `apps/agent/shipgate.yaml` no longer turns a comment in the root workflow into a critical blocker, nor lets a nested copy hide a root deletion. Every Git archive packs the commit's tree rather than its history, so a depth-1 checkout archives `--head` instead of failing an integrity check that agent mode routed to editing `shipgate.yaml`. `check` leaves verify's default reports directory out of a working-tree change set when it holds only uncommitted Shipgate artifacts, so a check, verify, check loop no longer counts the reports as changed files. **A run that used to report `mergeable` with no base now exits 2**: pass `--base` (in the Action, `diff_base: target` with `fetch-depth: 0`, or `base_ref`) or `--no-base`. No schema, contract or check ID moves. See the `verify stops without a base` migration note in `STABILITY.md`. (#1023) +- **`verify` reads an in-repository `--diff-from` and the exit policy from the base, not from the change under review.** A `--diff-from` path inside the repository was opened in the checkout, took the place of the `--base` scan, and was not a trust root: a pull request that added a destructive tool and regenerated `ci/base-report.json` from its own head compared the head with itself and could come back `mergeable` with `trust_root_touched: false`, and `SHIP-VERIFY-POLICY-WEAKENED` read its base policy from that same file. When a base resolves, such a path is now read as the base commit holds it (a file the base does not hold as a regular file is replaced by a scan of the base, and `base_notes` says which); a path outside the repository is the operator's and is read where it is, as is any path when there is no base (`--no-base`). An in-repository `--diff-from`, `--baseline` or `--policy-pack` file the change edits is a touched trust root (`SHIP-VERIFY-TRUST-ROOT-TOUCHED`, class `shipgate_state` for the first two and `policy` for a pack), unless the trust-root table already classifies it. Without `--ci-mode` and `--fail-on`, `ci.mode` and `ci.fail_on` came from the head's manifest, so changing `mode: strict` to `mode: advisory`, or dropping a severity from `ci.fail_on`, in the pull request turned exit 20 into exit 0 while the verdict stayed `blocked`. With a base and without `--ci-mode`, `ci.mode` now resolves to the stricter of the base and head manifests (`strict` if either declares it, named in `base_notes`); `--ci-mode` remains the operator's override, and a change that adopts `strict` still takes effect in the same pull request. Without `--fail-on`, the base's declared `ci.fail_on` is a floor for the exit code only, with or without `--ci-mode`: an active finding of a severity the base declares exits 20 (`fail_policy.exit_code`, `would_fail_ci`), and `base_notes` names the severities. The blocker severities, the verdict and `fail_policy.fail_on` keep the change's list, so a loosened list stays the human-review route `SHIP-VERIFY-POLICY-WEAKENED` reports (the governance benchmark is unchanged) and fails CI until a human handles it. **A workflow that pointed `--diff-from` at a report regenerated inside the pull request, or that relied on the head manifest to relax `ci.mode` or `ci.fail_on` (including through the Action, which passes `--fail-on` only when its `fail_on` input is set), now sees the base's comparison and the base's exit floor.** No schema, contract or check ID moves. See the `verify reads PR-controlled inputs from the base` migration note in `STABILITY.md`. (#1024) - **`attest`, `org bundle` and the Action validate the verification receipt of current verify output.** All three gated receipt validation on version literals the engine had stopped emitting (verifier `0.5`–`0.7`, `shipgate.verify_run/v3`). On every completed `1.x` verify run, `attest --from agents-shipgate-reports/verifier.json` and the Action's `attestation: true` step crashed with a traceback and exit 1, `org bundle` published a rewritten `receipt_id` or `decision_id` with exit 0, and the Action exported empty `request_id`, `receipt_id`, `decision_id` and `artifact_set_id` beside a `verification_receipt_json` nothing had checked. One predicate, `requires_verification_receipt` in `agents_shipgate.schemas.verification_identity`, now decides for all three: it names the verifier versions written before receipts (`0.1`–`0.4`) and reads every other version, newer ones included, as owing a receipt whenever the verifier states an outcome; the Action, which also publishes `report.json`'s decision, owes one whenever `verifier.json` names a `report.json` as its own, and publishes nothing from a report it does not name (the manifest-free host route leaves an earlier run's in place). A `0.1`–`0.4` label on a payload carrying a receipt-era identity field (`request_id`, `decision_id` and the four others verifier `0.5` added) is not believed. A failed or not-run execution that states none (a missing manifest, the manifest-free host route) writes no receipt, and still publishes, with empty identities. A receipt that is present is always validated: its content addressing, every artifact it binds and the request and decision `verifier.json` names. A missing, tampered or mismatched one exits `3` in `attest` and `org bundle` with one line naming the receipt (plus the agent-mode `next_action` line), and fails the Action's output step. `org bundle` also refuses an `attestation.json` whose receipt identity is not the validated receipt's, or that names one when none was validated. The Action's `verification_receipt_json` is empty unless the receipt validated. No schema version changes. See the [migration note](STABILITY.md#receipt-predicate-1022). (#1022) - **An OpenAPI operation behind a path-item `$ref` is read, and one that cannot be read is named.** The loader skipped every key of a path item that was not an HTTP method, so `/accounts/{id}: {$ref: '#/components/pathItems/Account'}` contributed nothing: its `deleteAccount` was absent from `report.json` and a strict scan passed. A `$ref` to another path item in the same document is now followed (chains included, with the fields written beside the `$ref` read first), and each operation's `source_pointer` and line name where it is written, such as `/components/pathItems/Account/delete`. A path item whose `$ref` leaves the document (a split spec's `./paths/users.yaml`, which is reported and never opened), names nothing, or loops is a source warning and an `adapter_parse` row (`unresolved_path_item_ref`) in `surface_exclusions`, so the scan no longer passes; a referenced operation replaced by one of the same method beside the `$ref` is one too (`shadowed_path_item_operation`). Schema `$ref`s the document resolves no longer warn: the resolver also descends `additionalProperties`, `patternProperties`, `not`, `prefixItems` and list-form `items`, follows a schema that is only a reference, and an unresolved reference is reported once per target rather than once per tool, so four read operations sharing an `additionalProperties` reference no longer read as `insufficient_evidence`. Unquoted YAML status codes (`200:`) select the lowest 2xx response as the output schema instead of the first response listed, and the DELETE proof's pure-Python YAML parse runs only when a document has a DELETE, stopping once the document is past its node budget. **Tool inventories can grow** where a path-item `$ref` hid operations, and a verdict that rested on the hidden part can move from `passed` to `review_required` or `blocked`; one held back only by false unresolved-reference warnings can move to `passed`. The DELETE proof still refuses a path item written as a `$ref`. (#1021) - `diff --application` reads bounded local factories returning OpenAI Agents SDK and Google ADK tool lists. It keeps the returned members and their construction locations, and reports unread constructor identity, import effects, escaping values, or unresolved callers as coverage gaps rather than treating them as established wiring. This is static source analysis; it does not execute a factory or establish runtime enforcement. (#874, #964) diff --git a/STABILITY.md b/STABILITY.md index f418662b..349432e6 100644 --- a/STABILITY.md +++ b/STABILITY.md @@ -347,6 +347,78 @@ the Action tag) for reproducible CI. --- + + +## Migration Note: Unreleased — `verify` reads PR-controlled inputs from the base (#1024) + +**What was wrong.** Two inputs that decide a pull request's verdict or exit +code were read from the pull request's own head. A `--diff-from` path inside +the repository was opened in the checkout, replaced the `--base` scan even when +both were given, and was not a trust root: a change that added a destructive +tool and regenerated `ci/base-report.json` from its own head compared the head +with itself and could come back `mergeable`, with `trust_root_touched: false`, +and `SHIP-VERIFY-POLICY-WEAKENED` read its base policy from that file. Without +`--ci-mode` and `--fail-on`, `ci.mode` and `ci.fail_on` came from the head's +manifest, so a change from `mode: strict` to `mode: advisory`, or one dropping +a severity from `ci.fail_on`, turned exit 20 into exit 0 while the verdict +stayed `blocked`. + +**What changes.** + +- With a base (`--base`, or one detected), an in-repository `--diff-from` is + read as the base commit holds it, through a run-private copy; the checkout's + copy is not read. When the base has no regular file at that path, the run + compares against a scan of the base instead (`base_status` is that scan's, + not `diff_from_provided`), and `base_notes` says which happened. A path + outside the repository is the operator's and is read where it is, as before, + and so is an in-repository path when there is no base (`--no-base`). +- An in-repository `--diff-from`, `--baseline` or `--policy-pack` file the + change edits is a touched trust root: `SHIP-VERIFY-TRUST-ROOT-TOUCHED` with + `trust_root_class` `shipgate_state` (reference and baseline) or `policy` + (policy pack) and `matched_glob` set to the configured path. A path the + trust-root table already classifies (`.agents-shipgate/**`, `policies/**`, + and so on) keeps its existing class and glob, so its finding fingerprint does + not move. +- With a base and without `--ci-mode`, `ci.mode` resolves to the stricter of + the base and head manifests, read from the base commit: `strict` if either + declares it, named in `base_notes` when the base's applied. `--ci-mode` + remains the operator's override, and a change that adopts `strict` takes + effect in the same pull request. The mode decides only the exit code: the + blocker severities are `{critical} ∪ fail_on`, and neither mode's default + `fail_on` adds to `critical`. Without a base, or when the base has no + manifest or one this engine cannot read (named in `base_notes`), the head's + declaration applies as before. `fail_policy.ci_mode`, the exit code and + `verifier.json` `mode` report the resolved mode. +- With a base and without `--fail-on`, the base manifest's declared + `ci.fail_on` is a floor for the **exit code only**, with or without + `--ci-mode` (the Action always passes one). An active finding whose + severity the base declares exits `20`, and `fail_policy.exit_code` / + `would_fail_ci` say so, even when the change's own list leaves that + severity out. The blocker severities, `decision`, `merge_verdict` and + `fail_policy.fail_on` keep the change's list, because `fail_on` decides + which severities block (the release decision truth table): a loosened list + stays the human-review route `SHIP-VERIFY-POLICY-WEAKENED` reports, as the + governance benchmark's `ci-advisory-from-strict` pins, rather than a blocker + under the list it loosens. Such a change therefore fails CI until a human + handles it, and `base_notes` names the severities the base held. + `--fail-on` remains the operator's override. + +**What does not change.** No schema, contract, `base_status` value or check ID +moves. `scan` is unchanged: it has no base. An out-of-repository +`--diff-from`, a run that passes `--fail-on` (with or without `--ci-mode`), +every verdict, and every run whose head declares the same `ci` block as its +base behave as before. + +**Who must act.** A pipeline that points `--diff-from` at a report the pull +request regenerates should drop it (verify then scans the base) or keep the +reference outside the repository. A pipeline that relied on the head manifest +to relax `ci.mode` or `ci.fail_on` passes `--ci-mode` or `--fail-on`. In the +Action, which always passes `--ci-mode`, a pull request that drops a severity +from the base's `ci.fail_on` now fails the job while a finding of that +severity is active; set the Action's `fail_on` input to override. + +--- + ## Migration Note: Unreleased — n8n's `include`, MCP secrets forwarded by name, and the host MCP files a `codex_config` source does not read (#1025) @@ -4558,7 +4630,7 @@ release-gate artifact. | `3` | Input parse error (malformed YAML/JSON, file too large, path traversal blocked) | | `4` | Other Agents Shipgate error | | `6` | Baseline integrity failure (v0.11+) — `agents-shipgate baseline verify --strict` detected `SHIP-BASELINE-INTEGRITY-MISMATCH`. Only the standalone `baseline verify` command emits this code; `scan` continues to use `20` for gate failure regardless of integrity-mode. | -| `20` | Gate failure. Strict-mode scan/verify: ≥ 1 unsuppressed finding hit `fail_on`, or ≥ 1 active unbaselined finding sets `blocks_release`. Also emitted by opt-in governance gate flags (v0.13+): `baseline status --require-owner`/`--require-expiry`/`--max-age-days` on violations, and `audit --host --drift --fail-on-drift` on host-grant drift. Without those flags the commands are advisory and exit `0`. | +| `20` | Gate failure. Strict-mode scan/verify: ≥ 1 unsuppressed finding hit `fail_on`, or ≥ 1 active unbaselined finding sets `blocks_release`. `verify` with a base and without `--fail-on` also exits `20` when an active finding's severity is in the base manifest's declared `ci.fail_on`, in either mode, even where the change's own list (which still sets the verdict) leaves it out (#1024). Also emitted by opt-in governance gate flags (v0.13+): `baseline status --require-owner`/`--require-expiry`/`--max-age-days` on violations, and `audit --host --drift --fail-on-drift` on host-grant drift. Without those flags the commands are advisory and exit `0`. | ### Runtime contract JSON @@ -4943,6 +5015,8 @@ Notation: `fail_on` is `release_decision.fail_policy.fail_on` after `ci_mode` re **Why exit code 20 depends on `--baseline-mode`.** `release_decision.{blockers, review_items}[]` always include the full set computed against `report.findings` (with suppressed excluded). The strict-mode exit code, however, is computed from `baseline_filtered_active(report, new_findings_only=...)` — when `--baseline-mode new-findings` is set (the default for the GitHub Action when `baseline:` is provided), baseline-matched policy and severity blockers are filtered out before the exit check, so exit is `0`. With `new_findings_only=False`, a matched policy blocker still triggers exit 20. The `release_decision` block remains baseline-aware in all cases; only the exit-code path changes mode. +**`verify`'s base `fail_on` floor (#1024).** With a base and without `--fail-on`, `verify` passes the base manifest's declared `ci.fail_on` to the exit computation only. The exit column then also reads `20` for any row whose finding is active after the same baseline filtering and whose severity is in that list, in either CI mode. The category and `rule` columns do not move, because `blocker_severities` stays `{critical} ∪ fail_policy.fail_on`, the change's own list. + Concretely: a scan with one baseline-matched critical and zero new findings produces `summary.status = "release_blockers_detected"` AND `release_decision.decision = "review_required"`. Both are correct under their respective contracts. New consumers should read `release_decision.decision`. #### Evidence-only decision states @@ -5446,6 +5520,20 @@ whose `detail` begins `base_unresolved`, a `fetch_base` next action naming unread change set as complete ([migration note](#verify-base-unresolved-1023)). +The change under review never supplies its own comparison or exit policy +([migration note](#verify-pr-controlled-inputs-1024)). With a base, an +in-repository `--diff-from` is read as the base commit holds it, and a base +without a regular file there is compared through a base scan instead; an +out-of-repository `--diff-from`, or any `--diff-from` with `--no-base`, is read +where it is. Precedence for `ci.mode` is: `--ci-mode` when given; otherwise, +with a base, the stricter of the base and head manifests'; otherwise the head +manifest's. `ci.fail_on` is `--fail-on` when given, otherwise the head +manifest's, and it alone sets the blocker severities and the verdict; with a +base and without `--fail-on`, the base manifest's declared `ci.fail_on` also +fails CI (exit `20`) for any active finding of those severities. An +in-repository `--diff-from`, `--baseline` or `--policy-pack` file the change +edits is reported by `SHIP-VERIFY-TRUST-ROOT-TOUCHED`. + Before any trigger-skip can return success, non-preview `verify` also requires the resolved `--config` path to exist. A missing config is a configuration failure, not a docs-only or no-trigger success: verify writes `verifier.json`, diff --git a/docs/baseline.md b/docs/baseline.md index db6a6f2d..795a09e1 100644 --- a/docs/baseline.md +++ b/docs/baseline.md @@ -163,6 +163,18 @@ load in older package versions. `release_decision.baseline_delta`; `--diff-from` drives `tool_surface_diff` and `action_surface_diff`. +In `verify`, `--diff-from` also stands in for the base scan, so it is the +"before" half of every base-relative comparison, including the base policy +snapshot `SHIP-VERIFY-POLICY-WEAKENED` reads. It is therefore never taken from +the change under review (#1024). With a base (`--base`, or one detected), a +path inside the repository is read as the base commit holds it; if the base has +no regular file at that path, verify scans the base instead and says so in +`base_notes`. A path outside the repository is the operator's and is read where +it is, as is any path with `--no-base`. The `--baseline` file is read from the +evaluated head. An in-repository `--diff-from`, `--baseline` or `--policy-pack` +file that the change edits is reported by `SHIP-VERIFY-TRUST-ROOT-TOUCHED` +(class `shipgate_state`, or `policy` for a pack), so a human reviews the edit. + Finding delta buckets compare **identity**, not whether the PR caused a risk. `new_findings` means an identity was absent from the reference; `resolved_findings` means it is absent from the head; `unchanged_findings` means diff --git a/src/agents_shipgate/checks/verify.py b/src/agents_shipgate/checks/verify.py index 6edd2378..1901310b 100644 --- a/src/agents_shipgate/checks/verify.py +++ b/src/agents_shipgate/checks/verify.py @@ -33,6 +33,7 @@ _LEGACY_TRUST_ROOT_SURFACES, PROTECTED_FILE_EDITS, TRUST_ROOT_SURFACES, + configured_input_trust_root, is_configured_manifest, is_context_configured_manifest, ) @@ -64,7 +65,13 @@ def run(context: ScanContext) -> list[Finding]: if not path or path in seen: continue seen.add(path) - classification = _configured_manifest(context, path) or _classify(path) + # A command-line input (#1024) is classified only where nothing else + # already explains the path, so existing rows keep their fingerprints, + # but it is never cleared by the structural shortcuts below. + configured_input = configured_input_trust_root(context, path) + classification = ( + _configured_manifest(context, path) or _classify(path) or configured_input + ) if path in assessment.openshell_safe_paths and classification is None: continue if classification is None and path in assessment.openshell_paths: @@ -72,6 +79,9 @@ def run(context: ScanContext) -> list[Finding]: if classification is None: continue trust_root_class, matched_glob = classification + if configured_input is not None: + findings.append(_finding(context, path, trust_root_class, matched_glob)) + continue if ( trust_root_class in {"agent_instructions", "tool_surface_decl"} and instruction_profile(path) is not None diff --git a/src/agents_shipgate/ci/release_decision.py b/src/agents_shipgate/ci/release_decision.py index 8672eb87..32c97003 100644 --- a/src/agents_shipgate/ci/release_decision.py +++ b/src/agents_shipgate/ci/release_decision.py @@ -178,9 +178,20 @@ def build_release_decision( new_findings_only: bool, tool_catalog: Sequence[Tool] | None = None, source_recovery_evidence: Sequence[SourceRecoveryEvidence] | None = None, + exit_fail_on_floor: Sequence[Severity] | None = None, ) -> ReleaseDecision: """Compute the release decision. + ``exit_fail_on_floor`` is a set of severities that fail CI in addition to + ``fail_on``, and nothing else (#1024). ``verify`` passes the base + manifest's declared ``ci.fail_on`` so the change under review cannot lower + its own exit code by loosening the list. It reaches the exit code (and so + ``fail_policy.exit_code`` / ``would_fail_ci``) only: the blocker + severities, the decision and ``fail_policy.fail_on`` keep the run's own + list, so a loosened list stays a human-review route, reported by + ``SHIP-VERIFY-POLICY-WEAKENED``, rather than a blocker under the list it + loosens. + ``tools`` is the root-reachable surface — the population every count and every gate reads. ``tool_catalog`` is everything extraction found, reachable or not, and is used for exactly one thing: scaffolding the binding @@ -429,10 +440,20 @@ def build_release_decision( # The canonical decision is computed before FailPolicy. Semantic gaps # remain strict-CI failures even when a higher-precedence named concern # labels the decision review_required instead of insufficient_evidence. + exit_fail_on: list[Severity] | None = fail_on + if exit_fail_on_floor: + exit_fail_on = [ + *fail_on_resolved, + *( + severity + for severity in exit_fail_on_floor + if severity not in fail_on_resolved + ), + ] exit_code = exit_code_for_report( report, ci_mode, - fail_on=fail_on, + fail_on=exit_fail_on, new_findings_only=new_findings_only, release_decision=decision, has_semantic_gaps=has_semantic_gaps or has_binding_gaps or has_policy_gaps, diff --git a/src/agents_shipgate/cli/scan/final_report.py b/src/agents_shipgate/cli/scan/final_report.py index ac114b5f..6c70865a 100644 --- a/src/agents_shipgate/cli/scan/final_report.py +++ b/src/agents_shipgate/cli/scan/final_report.py @@ -19,6 +19,7 @@ ) from agents_shipgate.core.semantic_consistency import validate_semantic_consistency from agents_shipgate.report.json_report import report_json_payload +from agents_shipgate.schemas.common import Severity from agents_shipgate.schemas.manifest import AgentsShipgateManifest, CiConfig from agents_shipgate.schemas.report import ReadinessReport from agents_shipgate.schemas.surfaces import ActionDeclarationFacts @@ -35,6 +36,7 @@ def _build_final_report( plan: _OutputPlan, config_path: Path, declared_ci: CiConfig | None = None, + exit_fail_on_floor: list[Severity] | None = None, ) -> tuple[ReadinessReport, Any]: """Phase 8: hash the run_id, build the ``ReadinessReport`` from the fully sanitized surfaces, run capability-diff enrichment, and @@ -108,6 +110,7 @@ def _build_final_report( policy_evidence_gaps=sanitized.policy_evidence_gaps, source_omissions=sanitized.source_omissions, source_recovery_evidence=sanitized.source_recovery_evidence, + exit_fail_on_floor=exit_fail_on_floor, ) if report.release_decision is not None: reference_facts = ( diff --git a/src/agents_shipgate/cli/scan/orchestrator.py b/src/agents_shipgate/cli/scan/orchestrator.py index 94473324..bbfa575c 100644 --- a/src/agents_shipgate/cli/scan/orchestrator.py +++ b/src/agents_shipgate/cli/scan/orchestrator.py @@ -11,6 +11,7 @@ from agents_shipgate.core.operation_attribution import ReconstructedOperationBase from agents_shipgate.report.human_order import HumanArtifactContext from agents_shipgate.schemas.capabilities import CapabilityLockFileV1 +from agents_shipgate.schemas.common import parse_severity from agents_shipgate.schemas.report import ReadinessReport from agents_shipgate.schemas.verification import VerificationContext @@ -51,6 +52,7 @@ def run_scan( human_context_callback: Callable[[HumanArtifactContext], None] | None = None, manifest_text: str | None = None, operation_base: ReconstructedOperationBase | None = None, + exit_fail_on_floor: list[str] | None = None, ) -> tuple[ReadinessReport, int]: """Run a full scan pipeline. Returns ``(report, exit_code)``. @@ -87,6 +89,7 @@ def run_scan( human_context_callback=human_context_callback, manifest_text=manifest_text, operation_base=operation_base, + exit_fail_on_floor=exit_fail_on_floor, ) except AgentsShipgateError as exc: # Every recovery route that names a file to edit needs the manifest @@ -150,6 +153,7 @@ def _run_scan( human_context_callback: Callable[[HumanArtifactContext], None] | None, manifest_text: str | None, operation_base: ReconstructedOperationBase | None, + exit_fail_on_floor: list[str] | None = None, ) -> tuple[ReadinessReport, int]: """The pipeline itself. Split from :func:`run_scan` so the manifest the run read can be attached to any failure on the way out, in one place.""" @@ -245,6 +249,11 @@ def _run_scan( plan=plan, config_path=config_path, declared_ci=resolved.declared_ci, + exit_fail_on_floor=( + [parse_severity(item) for item in exit_fail_on_floor] + if exit_fail_on_floor + else None + ), ) human_context = human_artifact_context(config_path, verification_context) if human_context_callback is not None: diff --git a/src/agents_shipgate/cli/verify/command.py b/src/agents_shipgate/cli/verify/command.py index 2cbfbee9..8486f446 100644 --- a/src/agents_shipgate/cli/verify/command.py +++ b/src/agents_shipgate/cli/verify/command.py @@ -168,12 +168,18 @@ def verify( ci_mode: str | None = typer.Option( None, "--ci-mode", - help="advisory or strict. Overrides manifest ci.mode for the head scan.", + help=( + "advisory or strict. Overrides manifest ci.mode for the head scan; " + "without it, the stricter of the base and head manifests applies." + ), ), fail_on: str | None = typer.Option( None, "--fail-on", - help="Comma-separated severities that fail CI.", + help=( + "Comma-separated severities that fail CI. Without it, the base " + "manifest's ci.fail_on also fails CI; the verdict follows the head's." + ), ), baseline: Path | None = typer.Option( None, @@ -188,7 +194,10 @@ def verify( diff_from: Path | None = typer.Option( None, "--diff-from", - help="Explicit prior report.json or baseline JSON for head diff.", + help=( + "Explicit prior report.json or baseline JSON for head diff. A path " + "inside the repository is read as committed at the base." + ), ), authorization: Path | None = typer.Option( None, diff --git a/src/agents_shipgate/cli/verify/git.py b/src/agents_shipgate/cli/verify/git.py index 3bc56396..4afbe898 100644 --- a/src/agents_shipgate/cli/verify/git.py +++ b/src/agents_shipgate/cli/verify/git.py @@ -1110,6 +1110,67 @@ def path_present_at_ref(workspace: Path, ref: str, path: Path) -> bool | None: return False +@dataclass(frozen=True) +class FileAtRef: + """One repository file as a commit holds it (#1024). + + ``state`` is ``present`` with ``data`` set, ``absent`` when the commit has + no entry at exactly this path, or ``unreadable`` with ``reason`` naming why + (not a regular file, past the read bound, or Git could not answer). + """ + + state: Literal["present", "absent", "unreadable"] + data: bytes | None = None + reason: str | None = None + + +_REGULAR_FILE_MODES = frozenset({b"100644", b"100755"}) + + +def regular_file_at_ref( + workspace: Path, ref: str, path: Path, *, max_bytes: int +) -> FileAtRef: + """Read exactly ``path`` at ``ref`` as a regular file, following nothing. + + ``read_bytes_at_ref`` answers ``None`` alike for an absent path, an + oversize blob and a Git failure, and ``git show`` of a symbolic link + returns its target text as though it were the file. A comparison reference + has to tell those apart: an absent one can be replaced by a scan of the + same commit, while a link or an unreadable one cannot be read at all. + """ + + commit = commit_sha(workspace, ref) + if commit is None: + return FileAtRef("unreadable", reason=f"{ref!r} does not resolve to a commit") + wanted = path.as_posix() + listing = _run_git_bounded_output( + workspace, + ["--literal-pathspecs", "ls-tree", "-z", "--full-tree", commit, "--", wanted], + max_output_bytes=_MAX_MANIFEST_LISTING_BYTES, + ) + if listing is None: + return FileAtRef("unreadable", reason="Git could not list the path") + for record in (item for item in listing.split(b"\0") if item): + header, separator, encoded_path = record.partition(b"\t") + if not separator or encoded_path != wanted.encode("utf-8"): + continue + fields = header.split() + if len(fields) != 3 or fields[1] != b"blob" or fields[0] not in _REGULAR_FILE_MODES: + return FileAtRef("unreadable", reason="it is not a regular file") + data = _run_git_bounded_output( + workspace, + ["cat-file", "blob", fields[2].decode("ascii")], + max_output_bytes=max_bytes, + ) + if data is None: + return FileAtRef( + "unreadable", + reason=f"it could not be read within {max_bytes} bytes", + ) + return FileAtRef("present", data=data) + return FileAtRef("absent") + + def path_ever_present_from(workspace: Path, ref: str, path: Path) -> bool | None: """Whether any commit reachable from ``ref`` ever held exactly ``path``. diff --git a/src/agents_shipgate/cli/verify/orchestrator.py b/src/agents_shipgate/cli/verify/orchestrator.py index e817a5d7..cc3a743c 100644 --- a/src/agents_shipgate/cli/verify/orchestrator.py +++ b/src/agents_shipgate/cli/verify/orchestrator.py @@ -22,6 +22,7 @@ from agents_shipgate import __version__ from agents_shipgate.checks.verify import PROTECTED_FILE_EDITS from agents_shipgate.checks.verify_policy import touched_policy_surfaces +from agents_shipgate.ci.exit_policy import effective_fail_on from agents_shipgate.ci.release_decision import SUGGESTED_DECLARATIONS_FILENAME from agents_shipgate.cli._artifact_lifecycle import clear_verifier_route_artifacts from agents_shipgate.cli._helpers import _apply_strict_plugins @@ -163,6 +164,7 @@ build_human_authorization_request, ) from agents_shipgate.schemas.human_review_request import HUMAN_REVIEW_REQUEST_FILENAME +from agents_shipgate.schemas.manifest import CiConfig from agents_shipgate.schemas.report import ( ReadinessReport, ReleaseDecision, @@ -227,6 +229,7 @@ read_bytes_at_ref, read_file_at_ref, ref_exists, + regular_file_at_ref, removes_a_yaml_file, removes_any_tracked_path, repository_identity, @@ -555,6 +558,17 @@ def run_verify( if diff_from is not None else None ) + # A reference inside the repository is content the change under review + # can rewrite. It is read as committed at the base, never from the head, and + # editing it is touching a trust root (#1024). One outside the repository + # is the operator's own and is read where it is. + diff_from_relative = _repository_relative_input(git_root, static_diff_from_path) + configured_input_roots = _configured_input_trust_roots( + git_root, + policy_packs=policy_pack_paths or [], + baseline=baseline_path, + diff_from=static_diff_from_path, + ) _reject_output_input_overlap( git_root=git_root, out_dir=out_dir, @@ -1291,12 +1305,24 @@ def capture_human_context(context: HumanArtifactContext) -> None: nonlocal head_human_context head_human_context = context + # With a base to read it from, an in-repository reference is taken from + # the base commit; the checkout's copy is not an input to this run. + diff_from_at_base = diff_from_relative is not None and bool(base) and base_exists + # Nor does the change under review set its own exit code: whatever the + # command line leaves open is floored at what the base declares (#1024). + base_ci: CiConfig | None = None + if base and base_exists and (ci_mode is None or fail_on is None): + base_ci, base_ci_note = _base_ci_policy( + git_root=git_root, base=base, config_relative=config_relative + ) + if base_ci_note is not None: + base_notes.append(base_ci_note) external_snapshot_paths = [ path for path in [ baseline_path, *list(policy_pack_paths or []), - static_diff_from_path, + None if diff_from_at_base else static_diff_from_path, ] if path is not None ] @@ -1348,13 +1374,29 @@ def run_base_report_dir() -> Path: return Path(run_base.name) try: - if diff_from is not None: + base_reference: Path | None = None + if diff_from_at_base: + assert base is not None and diff_from_relative is not None + base_reference, reference_note = _diff_reference_at_base( + git_root=git_root, + base=base, + relative=diff_from_relative, + run_report_dir=run_base_report_dir, + ) + base_notes.append(reference_note) + if diff_from is not None and not diff_from_at_base: base_status = "diff_from_provided" assert static_diff_from_path is not None base_report = static_diff_from_path base_notes.append( f"Using explicit diff reference: {_display_path(base_report, git_root)}" ) + elif base_reference is not None: + base_status = "diff_from_provided" + base_report = base_reference + # Published under the path the operator named; the bytes are the + # base commit's, which the note above says. + base_report_reference = static_diff_from_path elif base and base_exists: ( base_status, @@ -1528,11 +1570,22 @@ def run_base_report_dir() -> Path: native_observation = observe_native(config_path=openshell_proof_config, required=openshell_proof_required, workspace=git_root, input_root=head_input_root) base_notes.append(f"OpenShell native containment: {native_observation.status} ({native_observation.reason_code}); modeled inputs only.") + scan_manifest_text = ( + head_manifest_text if archive_head else worktree_manifest_text + ) + scan_ci_mode, exit_fail_on_floor, ci_notes = _exit_policy_base_floor( + head_manifest_text=scan_manifest_text, + base_ci=base_ci, + base=base, + ci_mode=ci_mode, + fail_on=fail_on, + ) + base_notes.extend(ci_notes) report, head_exit_code = run_scan( config_path=head_config_path, output_dir=out_dir, formats=HEAD_FORMATS, - ci_mode=ci_mode, + ci_mode=scan_ci_mode, fail_on=fail_on, baseline_path=head_baseline_path, diff_from_path=base_report, @@ -1573,16 +1626,16 @@ def run_base_report_dir() -> Path: # (PR #404 review 2). base_comparison_unavailable=base_status in _BASE_COMPARISON_FAILURES, + configured_trust_root_inputs=configured_input_roots, enabled_plugin_hooks=enabled_plugin_hooks, enabled_plugin_hook_issues=tuple(enabled_plugin_hook_issues), openshell_native=native_observation, ), capability_lock_callback=capture_capability_lock, human_context_callback=capture_human_context, - manifest_text=( - head_manifest_text if archive_head else worktree_manifest_text - ), + manifest_text=scan_manifest_text, operation_base=operation_base, + exit_fail_on_floor=exit_fail_on_floor, ) except AgentsShipgateError as exc: # `run_scan` records the manifest it read, and when the head is @@ -2644,6 +2697,183 @@ def _map_optional_tree_path( return tree_dir / relative +def _repository_relative_input(git_root: Path, path: Path | None) -> Path | None: + """The repository path an operator-named input file has, if it has one.""" + + if path is None: + return None + try: + relative = path.relative_to(git_root) + except ValueError: + return None + return relative if relative.parts else None + + +def _configured_input_trust_roots( + git_root: Path, + *, + policy_packs: Sequence[Path], + baseline: Path | None, + diff_from: Path | None, +) -> dict[str, str]: + """The in-repository inputs this run was pointed at, by trust-root class. + + Like the configured manifest, whatever a run reads as its policy, its + accepted debt or its comparison reference governs the gate wherever it + lives, and a change that edits it touches a trust root (#1024). The table + of globs cannot know these paths: they exist only on the command line. + """ + + roots: dict[str, str] = {} + for path, trust_root_class in ( + *((pack, "policy") for pack in policy_packs), + (baseline, "shipgate_state"), + (diff_from, "shipgate_state"), + ): + relative = _repository_relative_input(git_root, path) + if relative is not None: + roots.setdefault(relative.as_posix(), trust_root_class) + return roots + + +_BASE_DIFF_REFERENCE_NAME = "diff-from-reference.json" + + +def _diff_reference_at_base( + *, + git_root: Path, + base: str, + relative: Path, + run_report_dir: Callable[[], Path], +) -> tuple[Path | None, str]: + """Read an in-repository ``--diff-from`` as the base commit holds it (#1024). + + The checkout's copy belongs to the change under review: a pull request that + regenerated the reference from its own head compared against itself. The + base's copy is written where only this run reads it. A base without one, or + with one that is not a readable regular file, has nothing to read, so the + run compares against a scan of the base instead, and says so. + """ + + spelled = relative.as_posix() + found = regular_file_at_ref( + git_root, base, relative, max_bytes=_MAX_CACHED_REPORT_BYTES + ) + if found.state == "present": + assert found.data is not None + destination = run_report_dir() / _BASE_DIFF_REFERENCE_NAME + with destination.open("xb") as handle: + handle.write(found.data) + return destination, ( + f"Using diff reference {spelled} as committed at {base!r}; the copy " + "in the change under review is not read." + ) + why = ( + f"{base!r} does not contain it" + if found.state == "absent" + else f"it could not be read at {base!r}: {found.reason}" + ) + return None, ( + f"Did not use --diff-from {spelled}: {why}. Comparing with a scan of " + f"{base!r} instead; the copy in the change under review is never read." + ) + + +def _base_ci_policy( + *, + git_root: Path, + base: str, + config_relative: Path, +) -> tuple[CiConfig | None, str | None]: + """The ``ci`` block the base commit's manifest declares (#1024). + + ``None`` with no note when the base has no manifest at all: the change + introduces the gate, and its own declaration is the only one there is. + ``None`` with a note when the base manifest cannot be read or validated. + """ + + found = regular_file_at_ref( + git_root, base, config_relative, max_bytes=MAX_WORKTREE_MANIFEST_BYTES + ) + if found.state == "absent": + return None, None + reason = found.reason + if found.state == "present": + assert found.data is not None + try: + return ( + load_manifest_text( + found.data.decode("utf-8"), source=config_relative + ).ci, + None, + ) + except (AgentsShipgateError, UnicodeDecodeError, ValueError) as exc: + reason = str(exc).splitlines()[0] if str(exc) else type(exc).__name__ + return None, ( + f"The exit policy follows the change's {config_relative.as_posix()}: " + f"the copy at {base!r} could not be read ({reason})." + ) + + +def _exit_policy_base_floor( + *, + head_manifest_text: str | None, + base_ci: CiConfig | None, + base: str | None, + ci_mode: str | None, + fail_on: list[str] | None, +) -> tuple[str | None, list[str] | None, list[str]]: + """Floor the exit policy the command line left open at the base (#1024). + + The change under review cannot lower its own exit code. + + - Without ``--ci-mode``, the mode is ``strict`` if the base or the head + declares it. A change that adopts ``strict`` still takes effect in the + same pull request. The mode decides only the exit code: blockers are + ``{critical}`` plus ``fail_on``, and neither mode's default adds to + ``critical``. + - Without ``--fail-on``, the base's declared ``ci.fail_on`` is returned as + an exit-only floor. Every severity in it still fails CI, while the + blocker severities, the decision and ``fail_policy.fail_on`` keep the + head's list: a loosened list stays the human-review route + ``SHIP-VERIFY-POLICY-WEAKENED`` reports, not a blocker under the list it + loosens, and it fails CI until a human handles it. + + An explicit flag remains the operator's override for its own knob. + Returns the mode to scan with (``None`` leaves the head manifest's own), + the exit floor, and a note for each knob the base held. + """ + + if base_ci is None or head_manifest_text is None: + return ci_mode, None, [] + try: + head_ci = load_manifest_text(head_manifest_text).ci + except (AgentsShipgateError, ValueError): + # The head scan reports its own manifest error. + return ci_mode, None, [] + notes: list[str] = [] + resolved_mode = ci_mode + if ci_mode is None and base_ci.mode == "strict" and head_ci.mode != "strict": + resolved_mode = "strict" + notes.append( + f"The change declares ci.mode: {head_ci.mode} and {base!r} declares " + "strict; strict applies. Pass --ci-mode to override." + ) + floor: list[str] | None = None + if fail_on is None and base_ci.fail_on: + floor = list(base_ci.fail_on) + head_list = effective_fail_on(resolved_mode or head_ci.mode, head_ci.fail_on) + added = [severity for severity in floor if severity not in head_list] + if added: + notes.append( + f"The change's ci.fail_on leaves out {', '.join(added)}, which " + f"{base!r} declares; a finding of that severity still fails CI " + "(exit 20), while the verdict follows the change's list. Pass " + "--fail-on to override." + ) + return resolved_mode, floor, notes + + # File names that count as a Shipgate manifest when deciding whether a ref # already carries a gate. ``shipgate.yaml`` is the published default; the # configured name is added at call time so a repository that renamed its diff --git a/src/agents_shipgate/core/findings/report_builder.py b/src/agents_shipgate/core/findings/report_builder.py index ae0f5121..b0f25224 100644 --- a/src/agents_shipgate/core/findings/report_builder.py +++ b/src/agents_shipgate/core/findings/report_builder.py @@ -74,6 +74,7 @@ def build_report( policy_evidence_gaps: list[EvidenceGap] | None = None, source_omissions: list[SourceSurfaceOmission] | None = None, source_recovery_evidence: list[SourceRecoveryEvidence] | None = None, + exit_fail_on_floor: list[Severity] | None = None, ) -> ReadinessReport: report = ReadinessReport( run_id=run_id, @@ -131,6 +132,7 @@ def build_report( fail_on=fail_on, new_findings_only=new_findings_only, source_recovery_evidence=source_recovery_evidence, + exit_fail_on_floor=exit_fail_on_floor, ) # v0.35: built from the decision, not beside it. Accounting asks whether # a gap row names the excluded subject, so the gaps have to exist first — diff --git a/src/agents_shipgate/core/trust_roots.py b/src/agents_shipgate/core/trust_roots.py index c12f1140..04734de3 100644 --- a/src/agents_shipgate/core/trust_roots.py +++ b/src/agents_shipgate/core/trust_roots.py @@ -969,6 +969,27 @@ def configured_manifest_identity(context: object) -> object | None: return getattr(context, "config_path", None) +def configured_input_trust_root(context: object, path: str) -> tuple[str, str] | None: + """Classify a changed path that is one of the run's command-line inputs. + + ``VerificationContext.configured_trust_root_inputs`` names the in-repository + policy packs, baseline and comparison reference a verify run was pointed + at (#1024). Returns ``(trust_root_class, configured_path)`` for a match. + Both sides are normalized, and ASCII case variants match, for the reason + :func:`trust_root_class_for` gives. + """ + + verification = getattr(context, "verification", None) + inputs = getattr(verification, "configured_trust_root_inputs", None) or {} + candidate = _normalized(path).casefold() + if not candidate: + return None + for configured, trust_root_class in sorted(inputs.items()): + if _normalized(configured).casefold() == candidate: + return trust_root_class, configured + return None + + def is_context_configured_manifest(context: object, path: str) -> bool: """Match a changed path against the best identity carried by a scan.""" @@ -1067,6 +1088,7 @@ def is_configured_manifest( "IdentityReadBudgetExceeded", "PROTECTED_FILE_EDITS", "PathIdentityIssue", + "configured_input_trust_root", "configured_manifest_identity", "inspect_lexical_path_identity", "is_context_configured_manifest", diff --git a/src/agents_shipgate/schemas/verification.py b/src/agents_shipgate/schemas/verification.py index 0bc2d671..0984314e 100644 --- a/src/agents_shipgate/schemas/verification.py +++ b/src/agents_shipgate/schemas/verification.py @@ -62,6 +62,13 @@ class VerificationContext(BaseModel): # compare. It never relaxes a verdict; it only stops the head from # claiming a comparison it did not get. base_comparison_unavailable: bool = False + # Repository-relative paths of the in-repository inputs the verifier was + # pointed at on its command line (``--policy-pack``, ``--baseline``, + # ``--diff-from``), each with the trust-root class it governs the gate as. + # Like ``configured_manifest_path``, they exist only in the invocation, so + # the trust-root glob table cannot see them; a change that edits one + # touches a trust root (#1024). + configured_trust_root_inputs: dict[str, str] = Field(default_factory=dict) # The files that hold hooks of a plugin the repository's project settings # enable, read from both compared sides (an ``EnabledPluginHookFiles``), # and the input issues from reading them (``BoundaryInputIssue``s), as diff --git a/tests/shard_seconds.json b/tests/shard_seconds.json index 3299401a..6f20a33a 100644 --- a/tests/shard_seconds.json +++ b/tests/shard_seconds.json @@ -434,7 +434,8 @@ "tests/test_sdk_agent_clones.py": 73.3, "tests/test_memory_bank_effects.py": 2.6, "tests/test_verification_receipt_consumers.py": 3.7, - "tests/test_verify_base_and_gate_removal.py": 17.9 + "tests/test_verify_base_and_gate_removal.py": 17.9, + "tests/test_verify_pr_controlled_inputs.py": 23.4 }, "measured_at": "cbff1ef0b101", "measurement": { @@ -533,6 +534,17 @@ "case_count": 20, "command": "python -m pytest tests/test_verify_base_and_gate_removal.py --junitxml=base-gate.xml", "time_basis": "JUnit testcase time; original full-suite measurements retained" + }, + { + "source": "junit", + "measured_at": "b1ca815e with issue #1024 verify PR-controlled inputs increment", + "report_sha256": "23dd603cea7e91d0bb3c46a1b4360a7d56c164145a17253ecc000ff7e55ebf03", + "files": { + "tests/test_verify_pr_controlled_inputs.py": 23.4 + }, + "case_count": 25, + "command": "python -m pytest tests/test_verify_pr_controlled_inputs.py --junitxml=pr-controlled-inputs.xml", + "time_basis": "JUnit testcase time; original full-suite measurements retained" } ] } diff --git a/tests/test_adapter_static_only.py b/tests/test_adapter_static_only.py index 0719478c..1c74329a 100644 --- a/tests/test_adapter_static_only.py +++ b/tests/test_adapter_static_only.py @@ -263,7 +263,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.Popen", - line=3380, + line=3441, snippet=( "subprocess.Popen(cmd, env=env, stderr=subprocess.PIPE, " "stdin=subprocess.PIPE if input is not None else " @@ -286,7 +286,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.run", - line=3789, + line=3850, snippet=( "subprocess.run(cmd, capture_output=capture_output, check=check, " "env=env, input=input, stderr=stderr, stdin=stdin, stdout=stdout, " diff --git a/tests/test_verify_pr_controlled_inputs.py b/tests/test_verify_pr_controlled_inputs.py new file mode 100644 index 00000000..a22c57dd --- /dev/null +++ b/tests/test_verify_pr_controlled_inputs.py @@ -0,0 +1,614 @@ +"""#1024: the change under review never supplies its own comparison or exit policy. + +Two inputs that decide a pull request's verdict or exit code were read from the +pull request's own head: + +* An in-repository ``--diff-from`` was opened in the checkout, replaced the + ``--base`` scan, and was not a trust root. A change that added a destructive + tool and regenerated ``ci/base-report.json`` from its own head compared + against itself and came back ``mergeable``. It is now read as committed at the + base, and the path is a touched trust root, as are an in-repository + ``--baseline`` and ``--policy-pack``. +* Without ``--ci-mode`` / ``--fail-on``, ``ci.mode`` and ``ci.fail_on`` came + from the head's manifest, so flipping ``mode: strict`` to ``mode: advisory`` + (or dropping a ``fail_on`` severity) in the change turned exit 20 into + exit 0. The mode now resolves to the stricter of the base and head + manifests, and the base's ``ci.fail_on`` is an exit-only floor: the verdict + still follows the head's list. +""" + +from __future__ import annotations + +import json +import os +import re +import subprocess +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from agents_shipgate.cli.main import app +from agents_shipgate.cli.verify.git import FileAtRef, regular_file_at_ref +from agents_shipgate.cli.verify.orchestrator import _exit_policy_base_floor +from agents_shipgate.schemas.baseline import BaselineFile +from agents_shipgate.schemas.manifest import CiConfig + +runner = CliRunner() + +#: Git that ignores the caller's configuration, hooks and exported ``GIT_*``. +_GIT_ENV = { + **{key: value for key, value in os.environ.items() if not key.startswith("GIT_")}, + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_CONFIG_NOSYSTEM": "1", +} + +SAMPLE = Path(__file__).resolve().parents[1] / "samples" / "agent_weakens_gate" +REFERENCE = "ci/base-report.json" +TRUST_ROOT_TOUCHED = "SHIP-VERIFY-TRUST-ROOT-TOUCHED" + +DELETE_TOOL = { + "name": "docs.delete", + "description": "Delete a documentation article permanently.", + "inputSchema": { + "type": "object", + "properties": {"article_id": {"type": "string"}}, + "required": ["article_id"], + "additionalProperties": False, + }, + "annotations": {"destructiveHint": True}, + "auth": {"type": "oauth2", "scopes": ["docs:admin"]}, + "owner": "docs-platform", +} + +REFUND_TOOL = { + "name": "stripe.create_refund", + "description": "Create a refund for a customer charge.", + "inputSchema": { + "type": "object", + "properties": {"charge_id": {"type": "string"}, "amount": {"type": "integer"}}, + "required": ["charge_id", "amount"], + "additionalProperties": False, + }, + "annotations": {"destructiveHint": True}, + "auth": {"type": "oauth2", "scopes": ["payments:write"]}, + "owner": "payments", +} + + +def _git(cwd: Path, *args: str) -> str: + return subprocess.run( + [ + "git", + "-c", "user.name=Test", + "-c", "user.email=test@example.invalid", + "-c", "commit.gpgsign=false", + "-c", f"core.hooksPath={os.devnull}", + *args, + ], + cwd=cwd, + check=True, + capture_output=True, + text=True, + env=_GIT_ENV, + ).stdout.strip() + + +def _commit(repo: Path, message: str) -> str: + _git(repo, "add", "-A") + _git(repo, "commit", "-qm", message) + return _git(repo, "rev-parse", "HEAD") + + +def _scan_report(repo: Path, out: Path) -> bytes: + result = runner.invoke( + app, + ["scan", "-c", str(repo / "shipgate.yaml"), "--out", str(out), "--format", "json"], + ) + assert result.exit_code in {0, 20}, result.output + return (out / "report.json").read_bytes() + + +def _sample_repo(tmp_path: Path, *, mode: str = "advisory") -> Path: + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q", "-b", "main") + (repo / "shipgate.yaml").write_text( + (SAMPLE / "shipgate.yaml").read_text(encoding="utf-8"), encoding="utf-8" + ) + (repo / "tools.json").write_text( + (SAMPLE / "tools.json").read_text(encoding="utf-8"), encoding="utf-8" + ) + (repo / ".gitignore").write_text("agents-shipgate-reports/\n", encoding="utf-8") + _set_ci(repo, mode=mode) + return repo + + +def _set_ci(repo: Path, *, mode: str, fail_on: list[str] | None = None) -> None: + manifest = repo / "shipgate.yaml" + block = f"ci:\n mode: {mode}\n" + if fail_on is not None: + block += f" fail_on: [{', '.join(fail_on)}]\n" + text, count = re.subn( + r"ci:\n mode: \w+\n(?: fail_on: .*\n)?", + block, + manifest.read_text(encoding="utf-8"), + ) + assert count == 1 + manifest.write_text(text, encoding="utf-8") + + +def _add_tool(repo: Path, tool: dict) -> None: + tools = json.loads((repo / "tools.json").read_text(encoding="utf-8")) + tools["tools"].append(tool) + (repo / "tools.json").write_text(json.dumps(tools, indent=2) + "\n", encoding="utf-8") + + +def _add_bound_refund_tool(repo: Path) -> None: + """A destructive financial tool bound to the root agent: a release blocker.""" + + _add_tool(repo, REFUND_TOOL) + manifest = repo / "shipgate.yaml" + manifest.write_text( + manifest.read_text(encoding="utf-8").replace( + " - {tool: docs.lookup, source_id: docs_tools}\n", + " - {tool: docs.lookup, source_id: docs_tools}\n" + " - {tool: stripe.create_refund, source_id: docs_tools}\n", + ), + encoding="utf-8", + ) + + +def _regenerate_reference(repo: Path, tmp_path: Path, name: str) -> bytes: + data = _scan_report(repo, tmp_path / name) + (repo / REFERENCE).parent.mkdir(parents=True, exist_ok=True) + (repo / REFERENCE).write_bytes(data) + return data + + +def _verify(repo: Path, *args: str, out: Path): + result = runner.invoke( + app, + [ + "verify", + "--workspace", str(repo), + "--config", "shipgate.yaml", + "--out", str(out), + "--format", "json", + *args, + ], + ) + return result, json.loads(result.stdout) + + +def _report(out: Path) -> dict: + return json.loads((out / "report.json").read_text(encoding="utf-8")) + + +def _fail_policy(out: Path) -> dict: + return _report(out)["release_decision"]["fail_policy"] + + +def _touched(out: Path) -> dict[str, str]: + return { + finding["evidence"]["changed_file"]: finding["evidence"]["trust_root_class"] + for finding in _report(out)["findings"] + if finding["check_id"] == TRUST_ROOT_TOUCHED + } + + +def _forged_reference_pr(tmp_path: Path) -> tuple[Path, bytes]: + """Base commits a scanned reference; the PR adds docs.delete and rescans.""" + + repo = _sample_repo(tmp_path) + base_bytes = _regenerate_reference(repo, tmp_path, "scan-base") + _commit(repo, "base with a committed comparison reference") + _git(repo, "checkout", "-q", "-b", "pr") + _add_tool(repo, DELETE_TOOL) + head_bytes = _regenerate_reference(repo, tmp_path, "scan-head") + _commit(repo, "add docs.delete and regenerate the reference") + assert head_bytes != base_bytes + return repo, base_bytes + + +# --- an in-repository --diff-from --------------------------------------------- + + +def test_a_regenerated_in_repo_reference_cannot_make_the_change_mergeable( + tmp_path: Path, +) -> None: + repo, base_bytes = _forged_reference_pr(tmp_path) + out = tmp_path / "out" + + result, payload = _verify( + repo, "--base", "main", "--head", "HEAD", "--diff-from", REFERENCE, out=out + ) + + assert result.exit_code == 0, result.output + assert payload["merge_verdict"] != "mergeable" + assert payload["can_merge_without_human"] is False + assert payload["control"]["permissions"]["merge"] is False + assert payload["capability_review"]["trust_root_touched"] is True + assert REFERENCE in payload["changed_files"] + assert _touched(out).get(REFERENCE) == "shipgate_state" + # The comparison is the base's: the regenerated copy is not what was compared. + assert (out / "verification-base-report.json").read_bytes() == base_bytes + assert any( + f"{REFERENCE} as committed at 'main'" in note for note in payload["base_notes"] + ) + + +@pytest.mark.parametrize("named_base", [True, False], ids=["named-base", "detected-base"]) +@pytest.mark.parametrize("committed", [True, False], ids=["head", "worktree"]) +def test_an_in_repo_reference_is_never_read_from_the_change_under_review( + tmp_path: Path, committed: bool, named_base: bool +) -> None: + """With a resolvable base, the head's copy is not even parsed. + + A detected base counts: with no ``--base`` and no remote, verify follows + #1023's ladder to the local ``main``, and the reference is read there. + """ + + repo, base_bytes = _forged_reference_pr(tmp_path) + # Unreadable as a report: reading it would disable the diff with an error. + (repo / REFERENCE).write_text("this is not a report\n", encoding="utf-8") + if committed: + _commit(repo, "replace the reference") + out = tmp_path / "out" + + result, payload = _verify( + repo, + *(["--base", "main"] if named_base else []), + *(["--head", "HEAD"] if committed else []), + "--diff-from", REFERENCE, + out=out, + ) + + assert result.exit_code == 0, result.output + assert payload["base_ref"] == "main" + assert payload["base_status"] == "diff_from_provided" + assert any( + f"{REFERENCE} as committed at 'main'" in note for note in payload["base_notes"] + ) + assert (out / "verification-base-report.json").read_bytes() == base_bytes + report = _report(out) + assert report["tool_surface_diff"]["enabled"] is True + assert "this is not a report" not in json.dumps(report) + assert payload["merge_verdict"] != "mergeable" + assert _touched(out).get(REFERENCE) == "shipgate_state" + + +@pytest.mark.parametrize("held_as", ["absent", "symlink"]) +def test_a_reference_the_base_cannot_supply_is_replaced_by_a_base_scan( + tmp_path: Path, held_as: str +) -> None: + repo = _sample_repo(tmp_path) + if held_as == "symlink": + (repo / "ci").mkdir() + (repo / REFERENCE).symlink_to("../tools.json") + _commit(repo, "base") + _git(repo, "checkout", "-q", "-b", "pr") + if held_as == "symlink": + (repo / REFERENCE).unlink() + _add_tool(repo, DELETE_TOOL) + _regenerate_reference(repo, tmp_path, "scan-head") + _commit(repo, "add docs.delete and a reference generated from it") + out = tmp_path / "out" + + result, payload = _verify( + repo, "--base", "main", "--head", "HEAD", "--diff-from", REFERENCE, out=out + ) + + assert result.exit_code == 0, result.output + # Compared with a scan of the base, as though --diff-from were absent. A + # base tree holding a link cannot be scanned either, and that comparison + # fails closed exactly as it does without --diff-from. + assert payload["base_status"] == ( + "succeeded" if held_as == "absent" else "archive_failed" + ) + reason = ( + "'main' does not contain it" + if held_as == "absent" + else "it could not be read at 'main': it is not a regular file" + ) + assert f"Did not use --diff-from {REFERENCE}: {reason}." in " ".join( + payload["base_notes"] + ) + assert payload["merge_verdict"] != "mergeable" + assert payload["can_merge_without_human"] is False + assert _touched(out).get(REFERENCE) == "shipgate_state" + + +def test_a_reference_outside_the_repository_is_read_where_it_is(tmp_path: Path) -> None: + """The operator's own file, outside anything the change can edit.""" + + repo = _sample_repo(tmp_path) + _commit(repo, "base") + _git(repo, "checkout", "-q", "-b", "pr") + _add_tool(repo, DELETE_TOOL) + _commit(repo, "add docs.delete") + external = tmp_path / "operator" / "base-report.json" + external.parent.mkdir() + external.write_bytes(_scan_report(repo, tmp_path / "scan-operator")) + out = tmp_path / "out" + + result, payload = _verify( + repo, "--base", "main", "--head", "HEAD", "--diff-from", str(external), out=out + ) + + assert result.exit_code == 0, result.output + assert payload["base_status"] == "diff_from_provided" + assert (out / "verification-base-report.json").read_bytes() == external.read_bytes() + assert any( + note.startswith("Using explicit diff reference:") for note in payload["base_notes"] + ) + assert _touched(out) == {} + + +def test_in_repo_baseline_and_policy_pack_are_trust_roots(tmp_path: Path) -> None: + repo = _sample_repo(tmp_path) + baseline = repo / "ci" / "accepted.json" + policy = repo / "rules" / "org.yaml" + baseline.parent.mkdir() + policy.parent.mkdir() + baseline.write_text( + BaselineFile( + created_at="2026-10-01T00:00:00Z", source_report_run_id="base-baseline" + ).model_dump_json(), + encoding="utf-8", + ) + policy.write_text('name: Org policy\nversion: "1"\nrules: []\n', encoding="utf-8") + _commit(repo, "base with configured inputs") + _git(repo, "checkout", "-q", "-b", "pr") + baseline.write_text( + BaselineFile( + created_at="2026-10-02T00:00:00Z", source_report_run_id="head-baseline" + ).model_dump_json(), + encoding="utf-8", + ) + policy.write_text('name: Org policy, edited\nversion: "1"\nrules: []\n', encoding="utf-8") + _commit(repo, "edit both") + out = tmp_path / "out" + + result, payload = _verify( + repo, + "--base", "main", + "--head", "HEAD", + "--baseline", "ci/accepted.json", + "--policy-pack", "rules/org.yaml", + out=out, + ) + + assert result.exit_code == 0, result.output + assert _touched(out) == { + "ci/accepted.json": "shipgate_state", + "rules/org.yaml": "policy", + } + assert payload["capability_review"]["trust_root_touched"] is True + assert payload["can_merge_without_human"] is False + + +def test_a_file_at_a_ref_is_read_only_as_a_regular_file(tmp_path: Path) -> None: + """A link is never followed, and an oversize blob is not an absent one.""" + + repo = _sample_repo(tmp_path) + (repo / "ci").mkdir() + (repo / "ci" / "report.json").write_text('{"ok": true}\n', encoding="utf-8") + (repo / "ci" / "link.json").symlink_to("report.json") + (repo / "ci" / "tree").mkdir() + (repo / "ci" / "tree" / "member.json").write_text("{}\n", encoding="utf-8") + _commit(repo, "inputs") + + def read(path: str, max_bytes: int = 1024) -> FileAtRef: + return regular_file_at_ref(repo, "HEAD", Path(path), max_bytes=max_bytes) + + assert read("ci/report.json") == FileAtRef("present", data=b'{"ok": true}\n') + assert read("ci/missing.json").state == "absent" + assert read("ci/link.json").state == "unreadable" + assert read("ci/link.json").reason == "it is not a regular file" + assert read("ci/tree").state == "unreadable" + assert read("ci/report.json", max_bytes=4).state == "unreadable" + assert read("ci/*.json").state == "absent" + assert regular_file_at_ref( + repo, "no-such-ref", Path("ci/report.json"), max_bytes=1024 + ).state == "unreadable" + + +# --- ci.mode and ci.fail_on ---------------------------------------------------- + + +def _flipped_to_advisory(tmp_path: Path) -> Path: + """Base declares strict; the PR adds a blocking tool and declares advisory.""" + + repo = _sample_repo(tmp_path, mode="strict") + _commit(repo, "strict base") + _git(repo, "checkout", "-q", "-b", "pr") + _add_bound_refund_tool(repo) + _set_ci(repo, mode="advisory") + _commit(repo, "add a refund tool and relax the gate") + return repo + + +def test_a_change_cannot_lower_its_own_ci_mode(tmp_path: Path) -> None: + repo = _flipped_to_advisory(tmp_path) + out = tmp_path / "out" + + result, payload = _verify(repo, "--base", "main", "--head", "HEAD", out=out) + + assert payload["merge_verdict"] == "blocked" + assert result.exit_code == 20, result.output + assert _fail_policy(out)["ci_mode"] == "strict" + assert payload["mode"] == "strict" + assert any( + "declares ci.mode: advisory and 'main' declares strict" in note + for note in payload["base_notes"] + ) + + +def test_an_explicit_ci_mode_remains_the_operators_override(tmp_path: Path) -> None: + repo = _flipped_to_advisory(tmp_path) + out = tmp_path / "out" + + result, payload = _verify( + repo, "--base", "main", "--head", "HEAD", "--ci-mode", "advisory", out=out + ) + + assert result.exit_code == 0, result.output + assert payload["merge_verdict"] == "blocked" + assert _fail_policy(out)["ci_mode"] == "advisory" + assert not any("ci.mode" in note for note in payload["base_notes"]) + + +def test_a_change_can_still_tighten_its_own_ci_mode(tmp_path: Path) -> None: + repo = _sample_repo(tmp_path, mode="advisory") + _commit(repo, "advisory base") + _git(repo, "checkout", "-q", "-b", "pr") + _add_bound_refund_tool(repo) + _set_ci(repo, mode="strict") + _commit(repo, "add a refund tool and adopt strict") + out = tmp_path / "out" + + result, _payload = _verify(repo, "--base", "main", "--head", "HEAD", out=out) + + assert result.exit_code == 20, result.output + assert _fail_policy(out)["ci_mode"] == "strict" + + +@pytest.mark.parametrize( + ("base", "head", "flags", "expected"), + [ + # A head that declares what the base declares is scanned as before. + ({"mode": "strict"}, {"mode": "strict"}, (None, None), (None, None, False)), + ({"mode": "advisory"}, {"mode": "advisory"}, (None, None), (None, None, False)), + ( + {"mode": "advisory", "fail_on": ["high"]}, + {"mode": "advisory", "fail_on": ["high"]}, + (None, None), + (None, ["high"], False), + ), + # Tightening is the head's to do. + ({"mode": "advisory"}, {"mode": "strict"}, (None, None), (None, None, False)), + ( + {"mode": "advisory", "fail_on": ["high"]}, + {"mode": "advisory", "fail_on": ["critical", "high"]}, + (None, None), + (None, ["high"], False), + ), + # Lowering is not. + ({"mode": "strict"}, {"mode": "advisory"}, (None, None), ("strict", None, True)), + ( + {"mode": "advisory", "fail_on": ["critical", "high"]}, + {"mode": "advisory", "fail_on": ["critical"]}, + (None, None), + (None, ["critical", "high"], True), + ), + # The Action's path: --ci-mode is the operator's, ci.fail_on still floors. + ( + {"mode": "strict", "fail_on": ["high"]}, + {"mode": "advisory"}, + ("advisory", None), + ("advisory", ["high"], True), + ), + # --fail-on is the operator's too. + ( + {"mode": "advisory", "fail_on": ["high"]}, + {"mode": "advisory"}, + (None, ["low"]), + (None, None, False), + ), + ], +) +def test_the_exit_policy_floor_holds_what_the_head_would_lower( + base: dict, head: dict, flags: tuple, expected: tuple +) -> None: + def manifest(ci: dict) -> str: + repo_manifest = (SAMPLE / "shipgate.yaml").read_text(encoding="utf-8") + block = f"ci:\n mode: {ci['mode']}\n" + if "fail_on" in ci: + block += f" fail_on: [{', '.join(ci['fail_on'])}]\n" + return re.sub(r"ci:\n mode: \w+\n", block, repo_manifest) + + ci_mode, fail_on = flags + resolved, floor, notes = _exit_policy_base_floor( + head_manifest_text=manifest(head), + base_ci=CiConfig(**base), + base="main", + ci_mode=ci_mode, + fail_on=fail_on, + ) + + expected_mode, expected_floor, noted = expected + assert resolved == (expected_mode if ci_mode is None else ci_mode) + assert floor == expected_floor + assert bool(notes) is noted + + +@pytest.mark.parametrize("operator_mode", [None, "advisory"]) +def test_a_loosened_fail_on_fails_ci_while_the_verdict_stays_review( + tmp_path: Path, operator_mode: str | None +) -> None: + """The base's ci.fail_on floors the exit code and nothing else. + + ``fail_on`` also sets the blocker severities, so the verdict keeps the + head's list: the change's own ``SHIP-VERIFY-POLICY-WEAKENED`` (high) stays + a human-review item, as the governance benchmark's + ``ci-advisory-from-strict`` pins. But the change cannot lower its own exit + code: under the base's list that high finding fails CI until a human + handles it. ``--ci-mode`` (the Action always passes one) does not lift the + floor; ``--fail-on`` does. + """ + + repo = _sample_repo(tmp_path, mode="strict") + _set_ci(repo, mode="strict", fail_on=["critical", "high"]) + _commit(repo, "strict base failing on critical and high") + _git(repo, "checkout", "-q", "-b", "pr") + _set_ci(repo, mode="advisory") + _commit(repo, "relax the gate") + mode_args = ["--ci-mode", operator_mode] if operator_mode else [] + out = tmp_path / "out" + + result, payload = _verify(repo, "--base", "main", "--head", "HEAD", *mode_args, out=out) + + assert payload["merge_verdict"] == "human_review_required" + assert payload["can_merge_without_human"] is False + assert payload["capability_review"]["policy_weakening_proven"] is True + policy = _fail_policy(out) + assert policy["ci_mode"] == (operator_mode or "strict") + assert policy["fail_on"] == (["critical"] if operator_mode is None else []) + assert policy["would_fail_ci"] is True + assert policy["exit_code"] == 20 + assert result.exit_code == 20, result.output + assert any( + "ci.fail_on leaves out" in note and "'main' declares" in note + for note in payload["base_notes"] + ) + + out = tmp_path / "explicit" + result, payload = _verify( + repo, "--base", "main", "--head", "HEAD", *mode_args, "--fail-on", "critical", + out=out, + ) + assert payload["merge_verdict"] == "human_review_required" + assert result.exit_code == 0, result.output + assert not any("ci.fail_on" in note for note in payload["base_notes"]) + + +def test_dropping_fail_on_beside_a_blocking_tool_still_fails_ci(tmp_path: Path) -> None: + """The issue's shape for ``fail_on``: a blocked change cannot exit 0.""" + + repo = _sample_repo(tmp_path) + _set_ci(repo, mode="advisory", fail_on=["high"]) + _commit(repo, "advisory base failing on high") + _git(repo, "checkout", "-q", "-b", "pr") + _add_bound_refund_tool(repo) + _set_ci(repo, mode="advisory") + _commit(repo, "add a refund tool and drop fail_on") + out = tmp_path / "out" + + result, payload = _verify( + repo, "--base", "main", "--head", "HEAD", "--ci-mode", "advisory", out=out + ) + + assert payload["merge_verdict"] == "blocked" + assert result.exit_code == 20, result.output + assert _fail_policy(out)["fail_on"] == []