Skip to content

fix(action): classify gh's real denial stderr as contradicted, not unavailable - #111

Merged
SollanSystems merged 1 commit into
mainfrom
fix/denial-classification
Jul 30, 2026
Merged

SollanSystems merged 1 commit into
mainfrom
fix/denial-classification

Conversation

@SollanSystems

Copy link
Copy Markdown
Owner

The same-day correction slice 4b's plan mandated, closing the M2 fixture pair with a real
vendor string instead of a paraphrase. Follows #110.

What the post-merge experiment falsified

#110's first attest.yml run (30509952627)
succeeded on the first attempt, and the D1 prediction held both ways — subject[0].digest.sha256
(6530115e…) is not predicate.chain.head (3f0aa6d5…) and is sha256(<the 64 head bytes>). The pinned certificate claim names were right too: all three of
REQUIRED_CERTIFICATE_CLAIMS are present, and both _TRIGGER_CLAIM_ALIASES
(githubWorkflowTrigger and buildTrigger) are present, so the fail-closed
"at least one" rule was satisfied twice over. No correction needed there.

One thing was wrong. Now that a verifiable attestation exists, the denial-shape stderr
could finally be captured — it could not be before, because no attestation this repo had
minted was verifiable at all. Verbatim, from live gh with a deliberately wrong
--signer-workflow:

Error: verifying with issuer "sigstore.dev"

No marker in _CONTRADICTED_MARKERS matched it. The classifier's fail-closed default
therefore reported the most common denial as anchor_attestation_unavailable.

Why this matters, and why it is not a security hole

Both outcomes are non-promoting and both exit 1, so nothing was ever promoted that should
not have been — the fail-closed default did its job. What broke is D5's observability
distinction, in the more misleading direction: "I looked and it said no" was being
reported as "I could not look." An operator reading that log would go looking for an
index outage instead of an unexpected signer.

The pre-merge marker set was a remembered approximation of a vendor string. That is exactly
the failure mode the plan's M2 fixture requirement exists to catch — "a test author's
remembered approximation of a vendor message is exactly the thing that passes review and
fails in production" — and it is why the denial fixture was scheduled for capture the
moment it became possible.

The fix

  • "verifying with issuer" added to _CONTRADICTED_MARKERS, ahead of the pre-merge
    guesses. Ordering is unchanged and still safe: the "nothing found / transport" markers are
    checked first, so a 404 cannot be misread as a denial.
  • scripts/fixtures/gh_attestation_verify/signer_denied.txt — the 45-byte verbatim capture.
  • test_resolve_classifies_the_real_captured_denial_stderr drives the classifier from that
    file and asserts contradicted. Verified to fail with the marker removed, so it is not
    a vacuous pin.

The M2 fixture pair is now complete: both the 404 branch and the denial branch are pinned
against real captured gh output.

Suite (fresh tmpfs worktree): 1568 passed / 18 skipped canonical
(pyyaml+jsonschema+pytest), 1469 passed / 117 skipped structural-fallback.
scripts/test_action_anchor_resolve.py 24 passed. No version bump.

Copilot AI review requested due to automatic review settings July 30, 2026 03:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@SollanSystems
SollanSystems merged commit 919ce52 into main Jul 30, 2026
14 checks passed
@SollanSystems
SollanSystems deleted the fix/denial-classification branch July 30, 2026 03:08
SollanSystems added a commit that referenced this pull request Jul 30, 2026
Collects slice 4a (#106) and slice 4b (#110, #111, #112): loop verdict, opt-in
keyless attestation, --compare, replay-based chain ancestry, anchor@1, signer
trust, and the head-bearing subject. Neither slice shipped on its own; both sat
in Unreleased and go out under one number.

Beyond the version surfaces, this cut carries corrections a mechanical bump
would not have made.

The changelog said the predicate is handed to actions/attest as a subject-path.
It is handed as predicate-path, alongside a SEPARATE subject file, and §23 says
outright that the predicate bytes are deliberately not the subject — doctor
validation_mode and tool.version live inside the predicate, so the same run
projects different bytes in different environments. A reader following that
sentence would hash the wrong file and fail every verification.

"the three attestations minted before this release": the count was right, the
scope word was wrong. attest.yml shipped inside this same window, so nothing
predates the release; the three carrying the retired subject form are the pushes
through c493804. Re-scoped to match what attest.yml and reference §24 already
said.

ADR 0002 decision 6 is withdrawn, in a new dated amendment. It described
code-owner review as in force once the ruleset required it. The ruleset cannot
require it: one collaborator, no self-approval, bypass_actors empty,
current_user_can_bypass never, and no classic protection underneath. Flipping it
would invert the intent rather than merely fail — maintainer-authored pull
requests become unmergeable while bot-authored ones become gated, and agent work
here lands under the maintainer's account. CODEOWNERS is demoted in place to
what it honestly is, a record of the gate-defining surface, across the ADR, the
changelog, reference §23 and §24, and the file's own header.

test_docs_version.py's changelog check now ADDS 0.12.0 and keeps 0.11.0, and
restores 0.10.0 — the previous cut replaced its predecessor's assertion instead
of adding to it, leaving that heading with no cover.

Baselines, live checkout: 1577 passed / 16 skipped with pyyaml+jsonschema+pytest,
1478 / 115 with pyyaml+pytest. Both legs +7 against pre-cut, which is the CLI
guard's new tests plus the retired version-bump pin offset by the new code-owner
pin. CI additionally installs hypothesis, which moves the number.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants