fix(provenance): validate issued_at before coercion

[altrudev]

Closes #320.

What

provenance.build_record() now passes an explicitly supplied issued_at into the shared structural validator unchanged. Only the internally generated time.time() default is converted to integer seconds.

Previously int(...) ran first, defeating _check_structure()'s own primitive guard: True -> 1, False -> 0, 1.9 -> 1, "123" -> 123, and -0.5 -> 0. Non-convertible values such as [1] and "abc" leaked raw TypeError/ValueError before the module could raise ProvenanceError.

The same structural check now also rejects issued_at values above the JCS safe-integer ceiling. This covers the additional 2**60 case identified in review: it previously passed _check_structure() and then failed later in sign_record() with the canonicalizer's IntegerDomainError.

Regression coverage

The focused matrix covers:

• the five coercible values from provenance.build_record coerces malformed issued_at values before validation #320;
• [1] and "abc", which previously leaked raw exceptions;
• 2**60, which previously escaped the structural boundary;
• an unchanged valid explicit integer; and
• the default timestamp path, with the clock fixed to a fractional value to prove only that internally generated value is converted.

Scope

No provenance wire format, signature algorithm, freshness, revocation, catalog, or consumer-verifier semantics change. This repairs producer-side ordering in front of the shared structural rule introduced by #146.

CHANGELOG.md is updated under Unreleased / Fixed.

Branch base: upstream a247244fb0bb97aea4e60d655f36d445cca18cd5.

DCO sign-off is present. AI-assistance disclosure: ChatGPT assisted with current-main source review, duplicate/ownership review, implementation drafting, and regression construction. altrudev reviewed the bounded change and remains responsible for the contribution.

[github-actions]

🔴 Contributor Check: HIGH

Check	Result
Profile	HIGH
Credential	LOW
Overall	HIGH

Automated check by AgenTrust Contributor Check.

[lywinged]

head ae4e3cd: Request changes

One thing to change, everything else verified.

CHANGELOG.md line 16 holds the new entry and the cnf.jwk entry on one line, joined by a literal \n\n, backslash and letter written out twice, rather than a line break. Rendered as CommonMark that is one bullet with the four characters showing in the middle, and the cnf.jwk entry has lost its own. Split them into two bullets with a blank line between, as the entry in #322 does. Once that is pushed I will approve on the new head.

What holds on ae4e3cd. The five values from #320, True, False, 1.9, "123" and -0.5, plus [1], "abc" and 2**60, are each refused by build_record with ProvenanceError, and the message names issued_at. An explicit 123 comes out as 123, and when the clock is patched to return a non-integer, only the default path converts it. The bound is at the right place: 2**53 - 1 is accepted and 2**53 refused, which matches rfc8785, whose safe domain excludes 2**53 itself. One suggestion beside the required change, not a blocker: the package already names that ceiling as JCS_SAFE_INTEGER in sign.py, and provenance.py already imports from that module, so the guard can read the constant rather than write 2**53 - 1 a third time.

On the verifier. _check_structure is shared, so verify_record now refuses a record carrying issued_at at or above 2**53 with the range message. On a247244 the same wire record was refused too, by the freshness check as dated in the future, so nothing that verified before is refused now and what changes is which error names it. I also built records on a247244 across every parameter of build_record, thirteen in all, and verified each on ae4e3cd: all pass. So the scope statement holds.

The four CI steps with the hash-pinned install, on 3.11 and on 3.12: green, 1412 passed, 1 skipped. With src/ reverted to a247244 and the new tests kept, exactly the eight refusal cases fail; the two controls, the unchanged integer and the default path, pass on both, which is what a control should do.

For sequencing: #325 marks the issued_at cases as strict expected failures against #320. If this merges first, dropping that entry in #325 is on me.

[lywinged]

head bf3237d: Approve

Both changes are in. The CHANGELOG.md entries are two bullets with a blank line between them. Rendered as CommonMark, the same eight lines that gave two items with the four characters showing now give three, one per entry, and none of the characters is left. The guard reads JCS_SAFE_INTEGER from sign.py instead of writing the number a third time, which is the suggestion and not the required change, so thank you for taking it.

Re-verified on bf3237d rather than carried over, since the head moved. The eight values are each refused by build_record with ProvenanceError naming issued_at. An explicit 123 comes back as 123, and with the clock patched to return a non-integer only the default path converts. The bound sits where the constant is: 9007199254740990 and 9007199254740991 are accepted, 9007199254740992 is refused. provenance.py imports the constant from a module it already imports from, and importing agentrust_trace.provenance on its own still works, so nothing circular came in with it.

The verifier side is unchanged in effect. A record carrying issued_at at 2**60 on the wire is refused on a247244 by the freshness check as dated in the future and here by the range message, so no record that verified before is refused now. Thirteen records built on a247244 across every parameter of build_record all verify on this head.

It does one more thing than the body claims, in your favour. On a247244, a caller who widens max_future_skew_seconds past the ceiling gets rfc8785's IntegerDomainError out of verify_record for such a record, not the ProvenanceError the module documents, because the structural check ran with the value already coerced and the canonicalizer met it first. Here the range check refuses it before that line, so the issued_at route into that leak is closed as well. The same leak is reachable through other fields of a record, which is mine and not something to add here; that half is closed on #325's branch as of its current head.

The four CI steps with the hash-pinned install, on 3.11 and on 3.12: green, 1412 passed, 1 skipped. With src/ reverted to a247244 and the new tests kept, exactly the eight refusal cases fail, and both controls pass on either side.

On sequencing, so this does not wait on anything of mine: merged onto #325's head the only conflict is the CHANGELOG.md hunk, the two issued_at cases there flip to passing as intended, and with that expected-failure entry dropped the whole suite is green at 1506. Nothing on my side needs to land first, and the entry in #325 is mine to remove.

[imran-siddique]

Second conflict in an hour, same file, and this one is mine again: merging #322 re-conflicted #318 and #323 on the CHANGELOG.md block. You rebased the first round inside twenty minutes and I am not going to ask you to do that again for the same reason.

Please drop the CHANGELOG.md hunk from #318 and #323 entirely. Nothing else changes. Force-push without it and both become mergeable immediately and stop being able to conflict each other or anything else in the queue.

I will write the changelog entries for all of them in one maintainer commit once they land, using the wording from your PR bodies. You lose nothing: the entries still say what your changes did, and the attribution is in the commits and the PRs.

Why this rather than another rebase. Every open PR in this repository appends to the same block, so each merge conflicts every other one, and the number of rebases grows with the size of the queue rather than with the size of anyone's change. Two of your PRs have now paid for that twice. The durable fix is a fragment directory, one file per change assembled at release, and I am looking at it separately.

intent_bridge.py in #318 and provenance.py in #323 both merge clean against current main. The changelog is the only thing in the way of either.

[lywinged]

head 7257808: Comment, and the recommendation is to close this rather than change it.

Not an approval and not a request for changes. There is no change that would make it worth
merging, and GitHub already reports the branch as dirty so it cannot merge in this state
anyway; a blocking review would only be asking you for a third rebase, which is the opposite
of what I want here. My earlier approval on bf3237d shows as dismissed, so nothing of mine
stands on this head.

#334 merged at 04:31Z today and closed #320, so the defect this PR is titled for is on
main already. provenance.py:243 there reads stamped_at = issued_at if issued_at is not None else int(time.time()), which is this PR's reorder, and the five coercible values,
[1] and "abc" are all refused there.

One thing this branch had that main did not, which is why the recommendation is to close
it rather than that it was wasted.

The safe-integer ceiling is still open on main

Measured on c04d938, today's head:

build_record(issued_at=2**60)            -> built, issued_at=1152921504606846976
sign_record(that record)                 -> rfc8785.IntegerDomainError
build_record(issued_at=9007199254740992) -> built, then the same

_check_structure has no upper bound there, so the producer accepts a timestamp it cannot
sign, and the class that comes out is not the ProvenanceError the module documents. #334
did not carry this half, and it may have been left out deliberately rather than missed: the
proposal for it is in my 09-10 comment on #320 and that issue is now closed, so if the
answer is that the bound is not wanted, the useful thing is a line on #320 saying so,
because an unaddressed case on a closed issue reads the same as a fixed one. Your guard
carries it, in one clause reading JCS_SAFE_INTEGER from sign.py, with the error message
that names the range and the 2**60 case in the matrix. That is the whole of what this
branch still carries that main does not.

Two instruments report it, and they are independent of each other rather than of me: both
are mine, so read them as two routes to the same place and not as a second opinion.

The first is the reproduction above. The second is #325, which sweeps every keyword argument
of every public function and asks, separately, whether a producer emits anything its own
verifier refuses. Rebasing that branch onto today's main, the leak case for
build_record.issued_at flips to XPASS(strict), because #334 closed it. The
producer-and-verifier case does not flip, and the single value it still reports is
10000000000000000000. That sweep files the issued_at leak against #320 and says nothing
about a ceiling; the value it reports is its own output, not a case anyone chose for it.

What the clause costs

Ported onto c04d938 on its own, nothing else from this branch: 1448 passed, 1 skipped,
ruff clean. It is additive on the verifier too rather than a tightening, and the reason is
worth stating because _check_structure is shared. A record on the wire carrying
issued_at above the ceiling is refused either way under the default freshness policy, as
dated in the future. What changes is the case where a caller widens
max_future_skew_seconds past the gap: on main that returns rfc8785's
IntegerDomainError out of verify_record, and with the clause it returns
ProvenanceError. No record that verified before is refused.

Which half of your change was load-bearing

Both, and not equally, which is worth knowing before anything is deleted. Against your own
eight-case matrix:

• the reorder without the ceiling fails 1 of 8, which is 2**60;
• the ceiling without the reorder fails the other 7.

The two sets are disjoint and they cover the matrix, so neither half is doing any of the
other's work, which is #124's definition of independence and is stronger than either half
being merely exercised. The clause is carried by exactly one case, so if 2**60 is dropped
from the matrix nothing is left testing the bound.

Where I would leave this

The clause is now carried on #325, so this is not a request for a third rebase from you
for something I proposed. #325 is where its own instrument reports the gap: that branch
sweeps every keyword argument of every public function, and one of its checks asks whether a
producer emits anything its own verifier refuses. The issued_at entry there was filed as a
strict expected failure against #320, and on rebasing onto today's main the leak case went
XPASS(strict) because #334 closed it, while the producer-and-verifier case did not, still
reporting the single value 10000000000000000000. So the bound is the half that stayed
open, and the branch reporting it is the one that closes it.

That leaves this PR with nothing main or #325 does not already have, which is the whole
of why the recommendation is to close it. Say so if you would rather carry the clause here
instead and I will take it off #325: it is your finding and your issue, and I have no claim
on which branch it lands from.

For what it is worth on the matrix, #334's is wider than this one everywhere except the
bound: it pins the coerced value for each of the five, and it adds b"7", {"t": 1} and
float("nan").

Either way this branch no longer rebases clean: provenance.py conflicts as well as
CHANGELOG.md, because #334 landed in the same function. And if it does stay open, no
CHANGELOG.md hunk, per the instruction on 09-11.

Nothing here needs anything from the maintainer. The close is his call or yours.