fix(tr-sig): stop publishing a finding whose message names a different code - #76
Draft
lywinged wants to merge 3 commits into
Draft
fix(tr-sig): stop publishing a finding whose message names a different code#76lywinged wants to merge 3 commits into
lywinged wants to merge 3 commits into
Conversation
…ules
Ten statements the documentation makes that the code does not do, across nine
pages. All live on main, none checked by anything.
Codes:
- TR-SIG-005 is carried by every signature finding tr_sig.py produces and is
documented nowhere. It is the signature check outcome: the Ed25519 result, a
signature that cannot be verified, or no signature at all, which is FAIL at
Level 1 and above and UNVERIFIED at Level 0. UNVERIFIED is deliberately not
SKIP, so an unsigned record cannot read as a benign omission.
- TR-SIG-004 was documented as private key material in cnf.jwk. It reports key
type: kty missing, or not in {OKP, EC}. The private-key condition is not
reported under this code, or any code, because the module raises before
producing a finding for it. That is a module bug, fixed separately.
- TR-ANC-002 is documented in three files and named by no module, with two
descriptions that disagree: docs/error-codes.md and docs/modules/tr-anc.md
call it the https-scheme check, which tr_anc.py performs under TR-ANC-001,
while docs/levels.md calls it a missing anchor.leaf_hash. Folded into
TR-ANC-001. Adding a second ANC code instead is a behaviour change.
- TR-ANC-001 was documented as requiring a resolvable URI and as rejecting a
placeholder value. Nothing resolves it, and no placeholder check exists
anywhere under src/. It checks presence, string type, https scheme and a
host. An earlier revision of this branch carried the placeholder claim
forward; it is removed.
- TR-RTE-001 documented `sev-snp`, `tdx` and `opaque` as valid platforms. None
is in _VALID_PLATFORMS, so a reader following the page produced a record
TR-RTE-001 rejects, while eight of the ten registered platforms went
unmentioned. The wrong list appeared in three pages, and docs/levels.md
carried a sample record using one of them.
- TR-RTE-003 was documented as resolving the RIM URI and checking the manifest
behind it. It checks that the string starts with https://, and nothing else.
- TR-ENV-004 was documented as a required-fields gate over the schema's full
required set, positive case "all of: eat_profile, iat, subject, ...". It
checks cnf.jwk.kty, and nothing else.
- TR-POL-002 lost track of `declared`, in four pages. Four values, not three.
- docs/modules.md summarises five of the seven modules as doing work they do
not do: private key leak detection (the path that raises), RIM URI
resolution, SCITT inclusion proof structure, builder URI, and required
fields. Each row now says what its module checks.
Samples:
- docs/levels.md shows an anchor object in its minimum conformant Level 2
record. schemas/trace-claim.json is additionalProperties: false and defines
no anchor property. Measured: valid_level0.json validates, and the same
record with the documented anchor block fails with "Additional properties are
not allowed ('anchor' was unexpected)". anchor.leaf_hash appears nowhere else
in the repository.
tests/test_docs_match_the_modules.py adds two guards.
The code set named by the modules must equal the code set with a row in
docs/error-codes.md. A row, not a mention: a code named in passing is not
documented, and accepting a mention would let the check be satisfied by prose
that tells a reader nothing. Every TR- code in this package lives under
modules/, so that scope is complete.
And every JSON sample under docs/ must agree with the packaged schema. Three
qualify today, all in docs/levels.md, but every .md is scanned rather than a
list kept by hand, because a hand-maintained list of what gets checked is the
same defect this exists to catch. The check also fails if it validated nothing,
so it cannot degrade to a pass over no work when the samples or the fence
change.
Both guards fail on main's documentation and pass on this one. Verified
individually: a code moved from a row into prose fails the first, a changed
fence fails the second, and each sample drift reintroduced alone fails alone.
What the guards do not do, stated in their docstrings. The code check matches
on codes named in module source, not on Finding.code: TR-SIG-003 appears only
inside a message string a TR-SIG-005 finding carries, so matching on
Finding.code would demand deleting a row that documents a real condition. And
neither guard can tell whether a row describes what its code reports, which was
six of the ten defects above; those were checked by reading the rows against
the module.
The sample guard drops `required` before validating, since the samples are
fragments, but leaves `if` and `not` intact. Stripping `required` from an `if`
makes it vacuously true and fires the matching `then` against records the
condition never meant to reach; the schema's `origin` rule does exactly that,
and an earlier draft of this guard reported two valid samples as broken. String
values containing an ellipsis are dropped before validation: a sample signature
written as eyJhbGciOiJFZERTQSJ9... is a reader's placeholder, not a claim about
the format.
Not touched: two sample CI transcripts under docs/tutorials/ quote output the
code no longer produces. Regenerating a transcript is a different kind of
change and they are left rather than edited by hand.
203 passed, 5 xpassed. ruff clean. No broken relative links, nav unchanged.
CI runs neither ruff nor mypy today.
Signed-off-by: LouieLuNZ <48041247+lywinged@users.noreply.github.com>
`runner.run` calls every module directly, with no `try`. A module that raises
ends the run: the caller gets a traceback where a verdict belongs, and the
record is neither passed nor failed. tr_sig did that in five ways.
$ python -m trace_tests.cli verify --record leaked.json
File "src/trace_tests/runner.py", line 38, in run
results["TR-SIG"] = tr_sig.check(trace, record, fmt, level)
File "src/trace_tests/modules/tr_sig.py", line 126, in check
findings.append(Finding(
TypeError: Finding.__init__() got an unexpected keyword argument 'rule'
`Finding` is a dataclass over `code, status, message`. There is no `rule`
parameter, so that call has never produced a finding. It is the check meant to
catch a record that embeds its own private key, and it is the only such check
in the suite. Nothing rejected the record earlier either: the packaged schema
sets no additionalProperties on cnf.jwk and carries no guard against a `d`
member. No test anywhere placed a `d` in a JWK.
The other four are the same shape one level down. `trace.get("cnf", {}).get(...)`
raises AttributeError whenever cnf or cnf.jwk is present but is not an object,
and check_cmcp_runtime reads the same chain three deep. Measured before the
change: a string, a number, null, a list or True in either position ends the
run. `_jwk_of` reads the JWK through isinstance guards and returns an empty
dict when the record does not carry one as an object. Both entry points use it,
and its docstring says which of the two the outer guard is for: cmcp passes
record["trace"] and can hand it anything, while check passes a trace that is
already a dict and reads it directly elsewhere, so that function is not
hardened against a non-dict trace and this helper does not pretend otherwise.
The leak is reported under TR-SIG-004, where docs/error-codes.md documents this
condition, rather than under TR-SIG-002, which the raising call named.
TR-SIG-002 in this module belongs to check_cmcp_runtime and means a key that is
not OKP/Ed25519; reusing it here would give one code two meanings across two
formats.
The leak branch returns early, so nothing else in the module runs. That path
used to raise, so its outcome was unreachable and no consumer had met it.
Making it reachable as it stood would publish a record carrying no signature
verdict of any kind: not pass, not fail, not unverified, which is the
benign-omission reading UNVERIFIED exists to prevent, and one test in this
suite already reads that finding with a bare `next`. The branch now also emits
TR-SIG-005 UNVERIFIED, "signature not checked; cnf.jwk carries private key
material".
tests/test_modules_never_raise.py asserts that no module raises, and that none
returns an empty finding list, over every top-level field replaced with each of
nine junk values, every top-level field removed, the same two classes for
cnf.jwk, and a JWK carrying `d`. False and 0 are among the junk values because
a bare truthiness test reads them as absent, which is a different branch from a
wrong type. The module list and each module's parameters are read from the
signature rather than written down, so a module that gains a parameter is still
exercised instead of dropping out quietly. Six of the seven modules already
guarded their inputs with isinstance and pass unchanged.
That parametrised test passes fmt="trace" throughout, so it never reaches
check_cmcp_runtime. A separate case walks a cmcp envelope with the same junk
values at trace, trace.cnf, trace.cnf.jwk and signature; without it, half of
this change was hardened and untested.
Four further tests assert behaviour rather than the absence of an exception,
because the generic test would pass if the checks were deleted outright: a
record embedding its own private key fails under TR-SIG-004; the same record
still reports whether its signature was checked; runner.run completes on it at
every level, which is the path the traceback came out of; and no finding from
any module repeats the key it found, since report.py publishes every message
into a JSON and an HTML artifact meant to be forwarded. That last one runs over
all seven modules rather than only the one that reports the leak, because a
message anywhere could echo the value. The report itself carries only the
record's digest, verified, so a message is the only place a key could be copied
out.
Verified by reverting each change in turn; each has a test that fails without
it and passes with it, including the cmcp read on its own. The report path was
exercised end to end on the new status combination: JSON, HTML and the badge
all build, level 0 reports 2 failures with 1 unverified, level 1 reports 4 with
the unverified folded in, and no level is marked passed.
TR-SIG FAIL TR-SIG-004: cnf.jwk must not contain private key material
('d' member present in the JWK)
TR-SIG UNVERIFIED TR-SIG-005: signature not checked; cnf.jwk carries
private key material
Result: FAIL (8 checks, 2 failure(s), 0 skipped)
Scope, and what is left. loader.extract_trace returns record["trace"] unchecked
for cmcp-runtime, which reading it alone suggests is a third hole of this kind.
It is not one on the path the tool takes: load_record refuses a cmcp envelope
whose trace is not a dict before extract_trace runs, measured through the CLI on
an absent trace, a string and a list, each reported as an error rather than a
traceback. extract_trace is unexported and runner.run is its only caller, so
reaching it means assembling a record by hand and calling the runner without the
loader.
Two things nearby are real and are not fixed here, because each decides what a
new case reports rather than repairing a call that never worked.
check_cmcp_runtime has no private-key check at all, so a cmcp claim embedding
`d` is detected by nothing. And a signature that fails verification is carried
as a TR-SIG-005 finding whose message names TR-SIG-003, so the code in the
forwarded report and the code in the message disagree.
217 passed, 5 xpassed. ruff reports six findings in tr_sig.py, all identical on
main and none in the lines this changes.
Signed-off-by: LouieLuNZ <48041247+lywinged@users.noreply.github.com>
…t code
report.py publishes both halves: the JSON artifact carries `code` per finding
and the HTML table prints it beside the text. They disagreed. `_verify_ed25519`
returned messages prefixed with TR-SIG-001, TR-SIG-002 and TR-SIG-003, and two
callers attached those messages to findings of their own, `check` under
TR-SIG-005 and `check_cmcp_runtime` under TR-SIG-001. A code written in that
helper could only ever match one of the two, and matched neither.
Measured across every module over a mutation sweep, four distinct instances,
all of them in tr_sig:
code=TR-SIG-005 message names TR-SIG-001: signature verification failed
code=TR-SIG-005 message names TR-SIG-002: invalid public key in cnf.jwk.x
code=TR-SIG-005 message names TR-SIG-003: invalid base64url signature
code=TR-SIG-001 message names TR-SIG-002: invalid public key in cnf.jwk.x
So a malformed signature was forwarded as a TR-SIG-005 finding reading
"TR-SIG-003", naming a code the suite had not used and the docs described as
something else.
The helper's messages now name no code. Nothing else changes about them: the
reason for the failure is still there, and Finding.code is the only place a
code appears. Naming a module's own code in its message is the convention
everywhere else here and is left alone.
TR-SIG-003 had no other user. It was never a Finding.code anywhere; that
message prefix was the whole of its existence, so removing it leaves a code
documented in three pages and emitted by nothing. The rows are removed and the
condition they described, a signature that does not verify, is reported by
TR-SIG-005 for a plain record and TR-SIG-001 for a cmcp claim, both of which
are documented.
That also forced the two rows next to it to be read again. TR-SIG-001 was
documented as "Signature algorithm is not Ed25519", which nothing reports under
that code: in check_cmcp_runtime it means a missing signature or the
verification outcome itself. TR-SIG-002 was "cnf.jwk missing or malformed";
it means a key that is not OKP/Ed25519 or a missing x, and only in the cmcp
path. Both rows now say so, in docs/error-codes.md and docs/modules/tr-sig.md,
and the Level 0 failure list and the tutorial's reference table follow, since
both cited cmcp-only codes for plain records.
tests/test_findings_are_self_consistent.py asserts that no finding's message
names a code other than its own, over both entry points and a mutation set
aimed at the error branches rather than at breadth. Verified by restoring each
of the three prefixes on its own: each fails the guard, and each fails it in
both the plain and the cmcp case.
It counts the findings it looked at and fails on none. Its first version did
not: every record is skipped when a module raises, so a module that raised on
all of them reported a pass over nothing, measured by making check raise before
its first finding. That is the same degradation the sample guard on the
error-code branch was hardened against, reproduced one branch later.
The docs guard from the error-code branch needed strengthening to see any of
this. It matched codes by regex over module file text, so the docstring added
here explaining why TR-SIG-003 must not appear in a message was, on its own,
enough to keep the deleted code looking alive: with the row removed and no
emitter left, the guard still passed. It now reads string literals through
`ast`, skipping module, class and function docstrings, and comments are dropped
by the parse. Verified in both directions: re-adding the dead row fails, and
documenting a code no module emits fails; and on a probe carrying a code in a
module docstring, a function docstring, a comment and a real literal, only the
literal is seen.
Removing the prefix also settles that guard's own rationale, which said the
named set and the Finding.code set differ and gave TR-SIG-003 as the example.
Measured after this change they are identical, so the docstring now says that
and gives the reason the match stays on literals: it reads source text and
cannot tell which literal reaches a Finding.
Unrelated to the codes but found while checking where they appear:
docs/quickstart.md documented an output format the CLI does not produce. Its
sample put the error code in the first column, where the CLI prints the module,
and none of its three messages exists anywhere in the source. Replaced with
output from a real run, and the surrounding text now says where a code does and
does not appear.
This branch is stacked: the error-code documentation branch first, then the
raising-on-a-malformed-record branch, then this one. Each contains the commits
below it.
225 passed, 5 xpassed. The two test files this adds are ruff clean. ruff reports
six findings in tr_sig.py; the rule counts are identical on main, 4 E501, 1
F841 and 1 I001, and none of the six is on a line this changes.
Signed-off-by: LouieLuNZ <48041247+lywinged@users.noreply.github.com>
Contributor
|
❔ Contributor Check: UNKNOWN
Automated check by AgenTrust Contributor Check. |
github-actions
Bot
added
the
needs-review:UNKNOWN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
report.pypublishes both halves: the JSON artifact carriescodeper finding and the HTML table prints it beside the text. They disagreed._verify_ed25519returned messages prefixed withTR-SIG-001,TR-SIG-002andTR-SIG-003, and two callers attached those messages to findings of their own:checkunderTR-SIG-005,check_cmcp_runtimeunderTR-SIG-001. A code written in that helper could only ever match one of the two, and matched neither.Measured across every module over a mutation sweep, four distinct instances, all in
tr_sig:So a malformed signature was forwarded as a TR-SIG-005 finding reading "TR-SIG-003" — a code the suite had not used and the docs described as something else.
The fix
The helper's messages name no code. Nothing else about them changes: the reason for the failure is still there, and
Finding.codebecomes the only place a code appears. Naming a module's own code in its message is the convention everywhere else here and is left alone.What that exposed
TR-SIG-003had no other user. It was never aFinding.codeanywhere; the message prefix was the whole of its existence. Removing it leaves a code documented in three pages and emitted by nothing, so the rows are removed. The condition they described, a signature that does not verify, is reported byTR-SIG-005for a plain record andTR-SIG-001for a cmcp claim, both documented.The two rows beside it were also wrong.
TR-SIG-001was documented as "Signature algorithm is not Ed25519", which nothing reports under that code: incheck_cmcp_runtimeit means a missing signature, or the verification outcome itself.TR-SIG-002was "cnf.jwkmissing or malformed"; it means a key that is not OKP/Ed25519 or a missingx, and only in the cmcp path. Both rows now say so, and the Level 0 failure list and the tutorial's reference table follow, since both cited cmcp-only codes for plain records.The guard
tests/test_findings_are_self_consistent.pyasserts that no finding's message names a code other than its own, over both entry points and a mutation set aimed at the error branches rather than at breadth. Verified by restoring each of the three prefixes on its own: each fails the guard, in both the plain and the cmcp case.It also counts the findings it looked at and fails on none. Its first version did not: every record is skipped when a module raises, so a module that raised on all of them reported a pass over nothing — measured by making
checkraise before its first finding. That is the same degradation the sample guard on the error-code branch was hardened against, reproduced one branch later.The other guard needed strengthening to see any of this
The docs guard from the error-code branch matched codes by regex over module file text. The docstring added here, explaining why
TR-SIG-003must not appear in a message, was on its own enough to keep the deleted code looking alive: with the row removed and no emitter left, that guard still passed.It now reads string literals through
ast, skipping module, class and function docstrings; comments are dropped by the parse. Verified both ways: re-adding the dead row fails, and documenting a code no module emits fails. On a probe carrying a code in a module docstring, a function docstring, a comment and a real literal, only the literal is seen.Removing the prefix also settles that guard's own rationale, which said the named set and the
Finding.codeset differ and gaveTR-SIG-003as the example. Measured after this change they are identical, so the docstring now says that, and gives the reason the match stays on literals: it reads source text and cannot tell which literal reaches aFinding.Unrelated, found while checking where codes appear
docs/quickstart.mddocumented an output format the CLI does not produce. Its sample put the error code in the first column, where the CLI prints the module, and none of its three messages exists anywhere in the source:Replaced with output from a real run, and the surrounding text now says where a code does and does not appear.
Merge order
docs: align the published error codes and record samples with the modulesfix(tr-sig): report a malformed record instead of raising on it#74corrects the TR-SIG rows;#75adds the private-key condition back to TR-SIG-004; this one removes TR-SIG-003 and reclassifies TR-SIG-001 and TR-SIG-002 as cmcp-only. Reading them in that order is also the order they make sense in.225 passed, 5 xpassed. The two test files this adds are
ruffclean.ruffreports six findings intr_sig.py; the rule counts are identical onmain(4 E501, 1 F841, 1 I001) and none of the six is on a line this changes.Generated by Claude Code