Skip to content

transparency: eight round-three findings, including a high that predates everything - #341

Merged
pdbethke merged 1 commit into
mainfrom
fix/transparency-round-three
Sep 12, 2026
Merged

pdbethke merged 1 commit into
mainfrom
fix/transparency-round-three

Conversation

@pdbethke

Copy link
Copy Markdown
Owner

Round three on internal/transparency (reviewer claude-code, verifier antigravity). Eight findings, all eight standing — five pre-existing, three mine.

R1 — high, reproduced, pre-existing since 2026-07-10

The Signed Entry Timestamp was checked only when present. The Merkle proof covers the leaf and the in-tree index — not the global LogIndex, not IntegratedTime. Only the SET does.

So anyone holding a real record could delete the SET and edit both to anything, and certify verify printed the invented values as "verified (publicly witnessed <time>, Rekor #N)". The TUF key validity-window check lives inside VerifySET and went with it.

This is the one finding the verifier said certverify does not blunt: the rekor field is covered by no signature. It is now required and checked first, so the refusal names its own reason. Requiring it rejects nothing legitimate — Rekor returns a SET on every entry; this project's own carries 96 bytes.

Two were mine, from round two

R2 — the signature binding fell back to disclose, don't refuse whenever a parse failed. Right for an unreadable log body; wrong for the envelope, which is attacker input. One non-string sig, or no signatures at all, turned a refused never-logged signature into ok=true — while the reason claimed "unrecognized entry body shape" about a body that read perfectly. The refusal existed for a well-formed envelope and was missing for a malformed one.

R4 — sigstore-go returns a payload hash for intoto v0.0.2, so an intoto entry passed step 3 and skipped step 4 entirely, even though intoto bodies do record signatures, just at spec.content.envelope.signatures. My stated reason for skipping was false for that kind.

R3 refuted my own rationale, with sources

I had skipped comparing the logged envelopeHash on the theory that JSON re-serialization would break it. The reviewer read Rekor's source: it hashes the bytes as submitted, and brain/buildcert.go hands the same envelope variable to Anchor and to Save. I verified that independently. The stored bytes are the submitted bytes, so envelopeHash is now the primary binding.

R5–R8: comments claiming what the code does not do

The CLI printed the rekor detail only on failure, so "never silently so" was kept inside the package and dropped at the door a user reads. doc.go promised verification "without touching the network again" when building the witness does an online TUF refresh. Anchor's comment said it re-fetched "until" the proof appeared, on a single attempt, and grabbed an arbitrary map element on a UUID miss. Logger.Get never checked the entry was dsse or at the index asked for, so a wrong index reported as a hash mismatch — which reads as tampering.

Method

Four negative controls, all firing, with a harness that refuses to score a control whose build is broken (last round's counted a build failure as a pass). The round-two tests built on the refuted contract were deleted rather than patched — a test asserting the wrong contract is worse than no test.

Full suite green, vet clean, security gate OK.

🤖 Generated with Claude Code

https://claude.ai/code/session_011NgkB3eLPBhgfqU1XopR1V

…predates everything

Round three on internal/transparency (reviewer claude-code, verifier
antigravity). Eight findings, all eight standing. Five pre-existing, three
mine.

R1 (HIGH, reproduced, pre-existing since 2026-07-10). The Signed Entry
Timestamp was checked only when present. The Merkle proof covers the leaf and
the IN-TREE index — not the global LogIndex, not IntegratedTime. Only the SET
does. So anyone holding a real record could DELETE the SET and edit both to
anything, and `certify verify` printed the invented values as "verified
(publicly witnessed <time>, Rekor #N)". The TUF key validity-window check lives
inside VerifySET and was skipped with it. This is the only finding in the round
the verifier said certverify does NOT blunt: the rekor field is covered by no
signature. Now required and checked FIRST, so the refusal names its own reason
instead of surfacing as a reconstruction error. Requiring it rejects nothing
legitimate — Rekor returns a SET on every entry, and this project's own carries
96 bytes.

R2 (mine, round two). The signature binding fell back to "disclose, do not
refuse" whenever a parse failed. That is right for an unreadable LOG body and
wrong for the ENVELOPE, which is attacker input: one non-string `sig`, or no
signatures at all, turned a refused never-logged signature into ok=true while
the reason claimed "unrecognized entry body shape" about a body that read
perfectly. The refusal existed for a well-formed envelope and was missing for a
malformed one. Now four distinguishable outcomes, and only an unreadable log
body discloses.

R3 (pre-existing) refuted my own round-two rationale. I skipped comparing the
logged envelopeHash on the theory that JSON re-serialization would break it.
The reviewer read Rekor's source: it hashes the bytes as submitted, and
brain/buildcert.go hands the SAME `envelope` variable to Anchor and to Save.
The stored bytes ARE the submitted bytes. envelopeHash is now the primary
binding, with the signature set as the fallback.

R4 (mine, round two). sigstore-go returns a payload hash for intoto v0.0.2, so
an intoto entry passed step 3 and then skipped step 4 entirely — while intoto
bodies DO record signatures, at spec.content.envelope.signatures. My stated
reason for skipping was false for that kind.

R5–R8, all the same species: a comment claiming what the code does not do.
The CLI printed the rekor detail only on FAILURE, so "never silently so" was
kept inside the package and dropped at the door a user reads. doc.go promised
verification "without touching the network again" when building the witness
does an online TUF refresh. Anchor's comment said it re-fetched "until" the
proof appeared, on a single attempt, and took an arbitrary map element on a
UUID miss. Logger.Get never checked the entry was dsse or at the index asked
for, so a wrong index reported as a hash mismatch — which reads as tampering.

Four negative controls, all firing, and the harness refuses to score one whose
build is broken. The round-two tests built on the refuted contract were
DELETED rather than patched: a test asserting the wrong contract is worse than
no test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NgkB3eLPBhgfqU1XopR1V
@pdbethke
pdbethke merged commit 096bdd1 into main Sep 12, 2026
5 checks passed
@pdbethke
pdbethke deleted the fix/transparency-round-three branch September 12, 2026 19:18
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant