Skip to content

test(provenance): honor declared digest algorithm in depth oracle - #243

Merged
imran-siddique merged 1 commit into
agentrust-io:mainfrom
solloek369-arch:test/provenance-depth-sha384-oracle
Aug 31, 2026
Merged

imran-siddique merged 1 commit into
agentrust-io:mainfrom
solloek369-arch:test/provenance-depth-sha384-oracle

Conversation

@solloek369-arch

Copy link
Copy Markdown
Contributor

What this changes

build_provenance.digest supports SHA-256 and SHA-384, but the informative provenance-depth oracle previously inspected only subject.digest.sha256 when checking the SLSA attestation subject.

This patch selects the subject digest using the algorithm declared by build_provenance.digest. It adds a positive SHA-384 control with an irrelevant nonmatching SHA-256 member, plus a negative control with the wrong SHA-384 value. The existing SHA-256 corpus remains green.

No schema, wire, normative specification, or production-verifier behavior changes.

Refs #66

Type of change

  • Test/tooling-only (no normative effect)
  • 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

Spec section

No specification text changes. The informative oracle exercises the existing build-provenance subject-binding behavior in §3.3.1.

Verification

  • pytest -q tests/test_build_provenance_depth_vectors.py: 48 passed
  • pytest -q: 830 passed, 1 skipped
  • ruff check src tests: passed
  • mypy src/agentrust_trace: passed

Checklist

  • DCO sign-off on all commits (git commit -s)
  • CHANGELOG.md updated (not applicable: no normative change)
  • Breaking changes marked with <!-- CHANGED: #NNN: description --> (not applicable)
  • Backward compatibility statement included (not applicable: no breaking change)

Signed-off-by: Loek <solloek369@gmail.com>

@lywinged lywinged left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The fix is right and the defect was fully latent: all six vectors in examples/build-provenance-depth/ declare sha256, so nothing in the set could reach the hardcoded lookup. Adding the first sha384 case is what makes it reachable. Four CI steps green here, 830 passed.

One gap, and it is the half the title names.

I deleted and substituted the rule rather than reading it:

_attestation_subject_mismatch replaced by tests failing
the old hardcoded sha256 lookup 1
wanted in entry.get("digest", {}).values() 0
return False 3
return True 12

The second row is a verifier that ignores the declared algorithm entirely and accepts a match found under any label. Nothing in the file catches it. "Honor the declared algorithm" has two halves: use the one that was declared, and do not accept a value declared under a different one. This pins the first and leaves the second open.

To be clear about whose gap it is: the same substitution on main also fails nothing, 46 passed. This branch inherits the hole rather than introducing it, so it is a follow-up and not a reason to hold anything up.

The case that closes it holds the value fixed and moves only the label:

def _mislabelled_control() -> dict[str, Any]:
    """Record declares sha384:X; the subject carries the same hex under `sha256`."""
    vector = copy.deepcopy(CONTROL)
    hexadecimal = "a" * 96
    digest = "sha384:" + hexadecimal
    vector["build_provenance"]["digest"] = digest
    vector["context"]["artifact_digest"] = digest
    statement = _attestation(vector)
    assert statement is not None
    statement["subject"][0]["digest"] = {"sha256": hexadecimal}
    return vector


def test_a_hex_declared_under_another_algorithm_does_not_bind() -> None:
    assert verify(_mislabelled_control(), "builder")["failures"] == ["attestation_subject_mismatch"]

It passes on this branch as submitted and fails under the permissive substitution above, which is the property the other two cannot report on.

Related, and the reason I looked: of the two tests added here, only test_sha384_subject_binding_selects_the_declared_algorithm separates the fixed rule from the broken one. test_sha384_subject_binding_rejects_the_wrong_digest passes under the old hardcoded lookup too, because that lookup rejects every sha384 record and the test expects a rejection. It is a sound control in the accept and reject sense and it is not evidence for this change.

Nothing else: _hex had two occurrences and none remain, and no other rule in the file reads an algorithm from a literal.

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and the shape of the defect is worth naming: a check that reads one algorithm's field while the schema permits two is not a partial check, it is a check that silently passes for every record using the other one. build_provenance.digest allows SHA-256 and SHA-384; the oracle only ever looked at subject.digest.sha256.

Selecting the subject digest by the algorithm the record itself declares is the right fix, rather than trying both.

The two controls are what make it a test rather than an assertion: a positive SHA-384 case carrying an irrelevant non-matching SHA-256 member proves the selection happens, and a negative with the wrong SHA-384 value proves it still fails when it should. The first of those is the one most people leave out, and it is the one that would catch a future "try both and accept either".

Non-normative, no wire or verifier change, existing SHA-256 corpus untouched. Merging.

@imran-siddique
imran-siddique merged commit 3fd9036 into agentrust-io:main Aug 31, 2026
5 of 6 checks passed
lywinged pushed a commit that referenced this pull request Sep 6, 2026
Commit the regression control supplied verbatim by @lywinged in the approving review of #243. The local work adds current replay, mutation proof, and delivery packaging around that contributed control.

Review: #243 (review)

Signed-off-by: Loek <solloek369@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants