fix(sign): the revocation check failed open on a non-bool answer

[lywinged]

Found by a lens that had not been pointed at this repo: testing what the docstrings claim against what the code does, rather than reading them.

verify_record's docstring says, under a heading that reads Revocation (fail closed):

The trusted key is rejected if it is listed, or if the store cannot answer.

Half of that is implemented.

What fails open

RevocationStore is Container[str] | Callable[[str], bool]. The callable's answer was read by truthiness:

the store returns	before	after
None	accepted	rejected, cannot answer
""	accepted	rejected
0	accepted	rejected
[]	accepted	rejected
"no"	rejected as revoked	rejected, cannot answer
True	rejected as revoked	rejected as revoked
False	accepted	accepted

Truthiness is not a reading of revocation status in either direction. It let a key through on four values and rejected one on the string "no".

None is the case that matters and it is not hypothetical. It is what a CRL, status or SCITT lookup returns when its author handled the 200 and forgot every other response:

def is_revoked(kid):
    resp = requests.get(CRL_URL, params={"kid": kid})
    if resp.status_code == 200:
        return kid in resp.json()["revoked"]
    # every other status falls off the end and returns None

That is exactly the outage the existing except clause was written to survive. It already treats a store that raises as a rejection, on its own stated grounds that an unavailable source is not evidence a key is unrevoked. A store that answers None has supplied no more evidence than one that raises, and was being believed.

The one check in this package that exists to catch a compromised key was the one deciding by truthiness.

It is two entry points, not one

provenance.verify_record imports _check_not_revoked from sign, and its own docstring makes the claim in the same words:

revocation is consulted before the signature is checked, with exactly the semantics sign.verify_record documents ... Both a revoked key and an unreachable store fail closed.

So it failed open identically, and one fix closes both. Verified on this branch, where the refusal now surfaces through that module's own error type:

provenance.verify_record, callable returns None  ->  ProvenanceError: revocation status ... could not be determined
provenance.verify_record, callable returns False ->  accepted

The fix

A callable returning anything other than True or False is treated as unable to answer, and fails closed through the same path and the same message as a store that raises. The two are one fact.

The membership branch is untouched and needs no guard: in yields a real bool whatever __contains__ returns. A test pins that, so it does not acquire one by accident.

The docstring was broader than the code. It now names which answers count as no answer.

Why nothing caught it

Nine revocation tests already existed, covering set stores, a callable returning True, a callable returning False, a callable that raises, public-key objects, underivable keys, and the trusted-key-not-record rule. None of them returned a non-bool. The gap sat between a covered raise and a covered False, which is a shape a test count cannot show.

Measurements

	4b21262	this branch
entry points failing open on a non-bool answer	2	0
suite	1033 passed, 1 skipped	1045 passed, 1 skipped
ruff check src/ tests/	clean	clean
mypy src/	clean	clean
tools/check_dashes.py	clean	clean

12 tests added. 10 of them fail with the guard reverted; the other two are controls that must stay green either way, one asserting True and False still work and one asserting the membership branch is unchanged.

Also checked, and holding

The other four claims in the same two docstrings reproduce exactly: max_age_seconds defaults to 86400 on a Trust Record and to None on a provenance record; max_future_skew_seconds is 300 and is enforced on a provenance record even with no age bound set, which is the #155 defect not recurring; the v0.1 profile identifier is refused before any record is examined; and the profile is read before the signature is checked.

What this does not cover

The same probe found three exported functions leaking AttributeError on their key arguments. That is the type-contract class from #6 and #9, not this one, and it is #13 along with the sweep that should have caught it.

[github-actions]

❔ Contributor Check: UNKNOWN

Check	Result
Profile	UNKNOWN
Credential	LOW
Overall	UNKNOWN

Automated check by AgenTrust Contributor Check.