provenance.build_record coerces malformed issued_at values before validation

[altrudev]

Problem

On current main at a247244fb0bb97aea4e60d655f36d445cca18cd5, provenance.build_record() converts an explicitly supplied issued_at with int(...) before passing it to the shared structural validator:

stamped_at = int(issued_at if issued_at is not None else time.time())
_check_structure(
    ...,
    issued_at=stamped_at,
)

But _check_structure() explicitly intends to enforce the original JSON-integer boundary:

# bool is an int subclass, and True would otherwise pass as a timestamp.
if not isinstance(issued_at, int) or isinstance(issued_at, bool) or issued_at < 0:
    raise ProvenanceError(...)

Because validation happens after coercion, that guard cannot distinguish several malformed caller values from legitimate integers.

Measured directly from the conversion used by the function:

supplied issued_at	value handed to _check_structure
True	1
False	0
1.9	1
"123"	123
-0.5	0

All five therefore satisfy the validator's integer/non-negative test after conversion. The last case is particularly diagnostic: a negative non-integer is turned into an accepted non-negative timestamp.

Why this is distinct from #142 / #146

#142 found that consumer-side verification failed to enforce structural rules that build_record() was assumed to enforce, including the issued_at rule. #146 moved those rules into the shared _check_structure() helper.

This is a different producer-side ordering defect: build_record() transforms the value before the shared rule sees it. The shared validator itself is strict; the caller path defeats it by normalizing first.

Expected invariant

For an explicitly supplied issued_at:

• a non-negative JSON/Python integer is accepted unchanged;
• bool, float, string, null-equivalent explicit values, containers, and negative values are refused with ProvenanceError;
• only the omitted/default path may derive an integer timestamp from time.time().

The emitted record should never contain a timestamp different in primitive/value semantics from the one the caller explicitly supplied.

Bounded fix direction

Validate the explicit issued_at before conversion, or avoid converting explicit values entirely. Keep conversion only for the internally generated default, e.g. conceptually:

stamped_at = int(time.time()) if issued_at is None else issued_at
_check_structure(..., issued_at=stamped_at)

Regression coverage should pin the coercible malformed values above plus an unchanged valid-integer control and the default-timestamp path.

No provenance wire-format, signature, freshness, revocation, catalog, or verifier semantics need to change.

Duplicate / ownership review

I searched the TRACE issue and PR history for provenance.build_record, issued_at, boolean/float/string coercion, and found no existing issue or PR covering this exact pre-validation coercion. #142/#146 are adjacent but address consumer/producer structural-rule symmetry, not this ordering bug.

AI-assistance disclosure: ChatGPT assisted with current-main source review, duplicate search, conversion reproduction, and issue drafting. altrudev reviewed the bounded claim 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]

Right, and reproduced on a247244 with each of your five values: True becomes 1, False becomes 0, 1.9 becomes 1, "123" becomes 123 and -0.5 becomes 0, and every one of them then satisfies the guard at line 202, whose own comment says what it is for: "bool is an int subclass, and True would otherwise pass as a timestamp". The order at line 238 is what defeats it.

Two things the issue does not say that the same line carries. First, int() raises its own exception on a value it cannot convert: issued_at=[1] leaves build_record as a TypeError and issued_at="abc" as a ValueError, where the module's other public functions are held to ProvenanceError by tests/test_public_functions_raise_what_they_document.py. Second, the reason that file never saw either: it lists provenance.build_record under NO_ARGUMENT_TO_SWEEP at lines 99 to 103, because the function has no positional argument, so its keyword arguments were never swept. Your fix direction closes both at once, since handing an explicit value to _check_structure untouched puts isinstance in front of int(). The regression set should carry [1] and "abc" beside the five coercible values, plus the unchanged-integer control and the default path. One more value belongs in it: the guard has no upper bound, so issued_at=2**60 is accepted and sign_record then raises rfc8785's IntegerDomainError rather than ProvenanceError. #219 holds every other signed integer, the Trust Record's and the bridge's included, to the JCS safe-integer range; this one is not held to it, and the same isinstance line can carry the bound.

On #142 and #146: that is the history, and the distinction you draw is the right one. #146 made the rule shared; this is the producer's ordering in front of it, which #146 did not touch.

No call to build_record in the tree passes issued_at outside tests/test_provenance.py, and none passes time.time() as a float, so the change reaches no caller. Welcome as a PR in the form of #318, with a CHANGELOG.md entry under Unreleased, Fixed. Separately, I am bringing a sweep that walks every keyword argument of every public function with the same junk matrix the positional sweep uses, since that exemption is how this stayed unseen; the issued_at cases in it will be marked as expected failures against this issue, so your PR is what turns them green.