fix: hold the last three entry points to the type they document - #9
Merged
Conversation
Completes what #6 began and deliberately deferred. That change guarded `sign.verify_record` and `sign.jwk_thumbprint` and left four functions of the same class alone, because open PRs agentrust-io#225 and agentrust-io#227 were editing them and carrying the fix there would have collided with work someone else was mid-way through. Those PRs have landed. Re-running the same sweep against upstream `main` shows what they closed and what they did not: provenance.tool_catalog_hash 4 of 12 -> 0 closed by agentrust-io#229 provenance.verify_record 11 of 12 -> 11 not closed provenance.check_tool_catalog 11 of 12 -> 11 not closed content_marking.verify_assertion second argument, not closed `agentrust-io#225` added `_as_object` for `record["identity"]` and `record["tool_catalog"]`. Neither field is reachable until the record itself is a mapping, so a guard inside a record cannot close the record's own type. That was stated in #6's body as a prediction and is now a measurement. `AttributeError` is not the `ProvenanceError` `verify_record` documents, so it passes through a caller's `except ProvenanceError` untouched. `content_marking.verify_assertion` is the one worth naming separately, because it was worse than a crash rather than merely undocumented. It never checked that `record_bytes` were bytes, and `bytes(5)` is five zero bytes, so an int was hashed, failed to match, and the caller was told: the record at <url> does not match the assertion ... The record changed after the asset was signed, or the URL is serving a different one. Nothing was wrong with the record or the URL. A crash says the call was wrong; this said somebody else's server was serving forged evidence, specifically, with a digest attached. `build_assertion` has guarded the same argument all along. After this, every public entry point in the package reports zero leaks under the sweep, including the two `sign.py` functions #6 fixed. Also fixes a sentence in the changelog entry agentrust-io#227 merged. The em dash sweep in agentrust-io#230 removed its punctuation and left "Valid JSON is not always an object that is an array", which states the opposite of the point. `tools/check_dashes.py` cannot catch this: the removal was clean and the sentence is what broke. Thirty-two tests, each shown load-bearing by removing the guard it covers: 12, 11 and 10 failures respectively. 1031 passed, 1 skipped. Ruff clean, and `tools/check_dashes.py` reports nothing in any file this repository shares with upstream. Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
|
❔ Contributor Check: UNKNOWN
Automated check by AgenTrust Contributor Check. |
This was referenced Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes what #6 began and deliberately deferred. That change guarded
sign.verify_recordandsign.jwk_thumbprintand left four functions of the same class alone, because open upstream PRs agentrust-io#225 and agentrust-io#227 were editing them and carrying the fix here would have collided with work someone else was mid-way through.Those PRs have landed. Re-running the same sweep against the merged tree shows what they closed and what they did not:
provenance.tool_catalog_hashprovenance.verify_recordprovenance.check_tool_catalogcontent_marking.verify_assertionagentrust-io#225 added
_as_objectforrecord["identity"]andrecord["tool_catalog"]. Neither field is reachable until the record itself is a mapping, so a guard on a field inside a record cannot close the record's own type. #6's body said that as a prediction; it is now a measurement.AttributeErroris not theProvenanceErrorthatverify_recorddocuments, so it passes straight through a caller'sexcept ProvenanceError.The one that is worse than a crash
content_marking.verify_assertionnever checked thatrecord_byteswere bytes.bytes(5)isb"\x00\x00\x00\x00\x00", so an int was hashed, failed to match, and the caller was told:Nothing was wrong with the record or the URL. A crash says the call was wrong. This said somebody else's server was serving different evidence, specifically, confidently, with a digest attached.
build_assertionhas guarded the same argument all along.test_an_int_no_longer_reports_a_record_mismatchasserts the absence of that diagnosis by name, so it cannot come back quietly.After this
Every public entry point in the package reports zero leaks under the sweep:
sign.sign_recordandprovenance.sign_recordstill answerTypeErrorto a non-mapping. They are producers rather than verifiers, document no error contract, andTypeErroris Python's own answer to that call. #6 left them and so does this.A changelog sentence, fixed because it is in the entry above the one I added
The em dash sweep in agentrust-io#230 removed the punctuation from the entry agentrust-io#227 merged and left:
"an object that is an array" states the opposite of the point, and there is a leftover double space where the second dash was.
tools/check_dashes.pycannot catch this: the removal was clean and the sentence is what broke.Checks
Thirty-two tests, each shown load-bearing by removing the guard it covers:
verify_assertionrecord_bytesprovenance.verify_recordrecordprovenance.check_tool_catalogrecord1031 passed, 1 skipped. Ruff clean.tools/check_dashes.pyreports nothing in any file this repository shares with upstream.This branch also carries a merge of upstream
main(five commits: agentrust-io#225, agentrust-io#227, agentrust-io#228, agentrust-io#229, agentrust-io#230). The conflicts were in four files and all butCHANGELOG.mdwere pure em dash removal; those were resolved by taking this fork's content and adopting upstream's own punctuation choice for each identical sentence rather than inventing one.CHANGELOG.mdwas an additive merge of both sides' entries.Generated by Claude Code