sign.verify_record: harden max_age_seconds/max_future_skew_seconds validation to match provenance.py

[harshnair75567-cloud]

What this changes

provenance.verify_record() validates max_age_seconds and max_future_skew_seconds
via _check_seconds() (added per the review on #164): a negative value is refused with
a named error rather than treated as a stricter bound, and bool is excluded because it
is an int subclass, so True would otherwise silently pass as one second.

sign.verify_record() — the original Trust Record verifier — never received the same
hardening. It checks only max_future_skew_seconds < 0 and compares max_age_seconds
against the record's age with no validation at all. Passing max_age_seconds=-1, a value
a caller might use meaning "no bound" (the documented way to disable the check is None),
rejects every record — including one issued the same second — as record is stale, with
an error naming the record rather than the misconfigured argument.

_check_seconds() moves to sign.py and is shared: sign.verify_record() calls it
directly, and provenance.verify_record() imports it, passing its own ProvenanceError
through a new exc parameter so each keeps its existing public error type.

No behavior changes for any previously-valid input. Reference implementation only —
spec/trace-v0.2.md and spec/server-provenance-v1.md are untouched.

Type of change

• Editorial (typo, link fix, clarification: no normative effect)
• Non-breaking spec change (new optional field, new platform profile, informative addition)
• Breaking spec change (requires 14-day comment period and Project Lead sign-off)
• Schema change
• Example addition

None of the above — this is a reference-library fix (src/agentrust_trace/sign.py,
src/agentrust_trace/provenance.py), not a change to normative spec text. Per
CONTRIBUTING.md, tooling changes are in the no-sponsor-required set.

Spec section

None — implementation only.

Checklist

• DCO sign-off on all commits (git commit -s)
• CHANGELOG.md updated (see entry below — not required for a non-normative change, added anyway since the reference library documents its own bug fixes there)
• Breaking changes marked with <!-- CHANGED: #NNN: description --> in spec text — N/A, not breaking
• Backward compatibility statement included — non-breaking: tightens rejection of already-invalid configuration values only; no previously-valid call changes behavior

[lywinged]

CI is red, and separately the change looks half applied. The code that is here behaves exactly as documented, so both are additions rather than rewrites.

The problem it describes reproduces exactly. On main, a record signed a moment earlier, with max_age_seconds=-1:

ValueError: record is stale: iat is 0s old, exceeds max_age_seconds=-1

0s old is the changelog's "including one issued the same second", and the message does name the record rather than the argument that was wrong.

1. The failing step

House style (no em dashes), which runs tools/check_dashes.py:

CHANGELOG.md:16: em dash (use a colon, a comma, or two sentences)
  ...Passing `max_age_seconds=-1` — a value a caller might use meaning "no boun...
CHANGELOG.md:16: em dash (use a colon, a comma, or two sentences)
  ...is the documented way to disable the check — rejected every record, includ...

2 occurrence(s).

Both are in the changelog entry this adds, and the same script exits 0 on main. EXEMPT_PREFIXES is ("examples/", "spec/trace-v0.1.md"), so CHANGELOG.md is in scope. This is easy to miss: the rule and the script both arrived with #230.

I replaced those two characters with commas and the step exits 0. Nothing else is needed for it.

2. The provenance half is missing

The changelog says:

_check_seconds() is now defined once in sign.py and shared: sign.verify_record() calls it directly, and provenance.verify_record() imports it, passing its own ProvenanceError via a new exc parameter

The docstring on the new function says the same. provenance.py is not in this diff. It still carries its own _check_seconds at line 251 hardcoding ProvenanceError, so the package has two, and nothing anywhere passes exc.

So the exc parameter is right and the sentence describing it is ahead of the code. The missing half is small, and provenance.py line 23 already imports several private names from sign, so there is somewhere to put it:

• delete the local _check_seconds at provenance.py:251
• add _check_seconds to the existing from agentrust_trace.sign import (...)
• pass exc=ProvenanceError at the two call sites on lines 334 and 335

Line numbers are against e23e126. I applied that: 828 passed 1 skipped, ruff check src tests scripts clean, mypy src/agentrust_trace clean, and one _check_seconds in the package. The import runs one way, provenance to sign and not back, so there is no cycle. And exc then earns its place: with the local copy gone, provenance.verify_record still raises ProvenanceError for a string, a bool and a negative, which is the whole point of the parameter and is currently untestable because nothing passes it.

3. What I ran, and what passed

Against e23e126, the four steps in ci.yml verbatim:

step	result
ruff check src tests scripts	passed
python tools/check_dashes.py	exit 1, the two above
mypy src/agentrust_trace	passed
pytest --cov	828 passed, 1 skipped, 95%

And every branch the change adds, on a fresh clone with a signed record:

configuration	outcome
max_future_skew_seconds=-1	ValueError, must be non-negative
max_future_skew_seconds="x"	ValueError, must be an integer
max_future_skew_seconds=True	ValueError, must be an integer
max_age_seconds=-1	ValueError, must be non-negative
max_age_seconds="x"	ValueError, must be an integer
max_age_seconds=True	ValueError, must be an integer
max_age_seconds=None	accepted, the bound stays disabled

All seven behave as the docstring says. The bool exclusion is the part worth having: isinstance(True, int) is true in Python, so True would otherwise have passed as a one second bound.

4. One note, not blocking

The type check is the one branch with nothing behind it. I disabled each of the three branches separately and reran:

branch disabled	tests that fail
isinstance(value, bool) or not isinstance(value, int)	none, 828 still green
value < 0	1, test_verify_record_rejects_negative_future_skew_configuration
optional and value is None	1

The two that fail are caught by tests already on main: the first guards the three lines this replaces, and the second exists because something already relies on max_age_seconds=None disabling the bound. So the < 0 and optional behaviour is held, and the isinstance guard, which is the whole of the new type and boolean handling, is held by nothing. The suite count is unchanged at 828, which is the same reading from the other side.

A few parametrized cases rather than a redesign. The behaviour is already right; what is missing is the part that says so.