Skip to content

feat: set credentialStatus on a credential being built; PartialEq on the type - #24

Merged
stormer78 merged 1 commit into
mainfrom
feat/credential-status-setter
Sep 9, 2026
Merged

feat: set credentialStatus on a credential being built; PartialEq on the type#24
stormer78 merged 1 commit into
mainfrom
feat/credential-status-setter

Conversation

@stormer78

Copy link
Copy Markdown
Contributor

The two items from the conformance audit in #10 that the Working Draft 02 work
closed without.

credentialStatus can be set on a credential being built

DTGCommon::credential_status has been modelled since 0.7.0, but only so that an
entry already on the wire survived a parse-then-re-serialise without changing the
credential's digest. A credential built by one of the new_* constructors had no
way to acquire one short of reaching through credential_mut.

with_credential_status and set_credential_status sit alongside the existing
with_id/set_id pair and carry the same before-signing caveat — a Data
Integrity proof covers the credential minus its proof, so a status entry
spliced in after sign() leaves a document whose proof no longer verifies.

It is CONDITIONAL, not REQUIRED — which is why it is a setter

The audit asked for this as a REQUIRED constructor parameter on new_vdc,
reading the VDC draft (dtgwg-cred-spec#19). The text that merged makes it
conditional:

A VDC MUST carry credentialStatus where its validity period exceeds the
freshness window the governing VTC or VTN defines for delegations, and MAY omit
it otherwise. Issuers SHOULD prefer short validity and re-issuance wherever the
delegator is reachable […]

A verifier must be able to establish that an appointment is in force without
contacting the delegator, and two things satisfy that: a validUntil short
enough that expiry alone bounds the exposure, with the delegator withdrawing by
declining to re-issue; or a status entry the verifier can check.

That freshness window is governance this library does not know, so nothing here
can decide which side of the condition a given VDC falls on. Demanding the entry
from every caller would forbid the short-validity case the specification prefers
— and prefers for a stated reason: a status check is a live lookup that reveals
the verification event to whoever hosts the status list. So the constructor still
supplies no entry of its own, and a caller that needs one attaches it.

Worth flagging to whoever runs the VTI-side audit: F6 has this as REQUIRED, and
the same reading may have propagated into the stack.

Still not resolved anywhere

Neither delegation::verify_chain nor authority::verify_chain checks
revocation; both verify structure, scope and validity only. Unchanged here —
revocation remains a live lookup the caller performs.

PartialEq on DTGCredentialType

Unrelated and one line. Consumers had to assert by pattern (matches!);
assert_eq! now works and reports the actual variant on failure. Eq is derived
alongside. The deprecated RCard variant provokes no warning from the derive.

Notes

  • Non-breaking — both changes are additive, so no major bump. Left under
    [Unreleased] at 0.8.0 rather than cutting a release.
  • 121 tests, up from 118: that the constructor invents no status entry (omitting
    is legitimate), that an attached one reaches the wire and survives parsing
    back, that set_ matches the builder, and that types compare by equality. Plus
    a doctest. cargo clippy --all-targets and cargo fmt --check are clean.
  • README gains a "Revocation status" section next to "Credential identifiers".

Refs: #10

… on the type

Both from the conformance audit in #10, which the VDC work closed without them.

`credentialStatus` has been modelled since 0.7.0, but only so that an entry
already on the wire survived a parse-then-re-serialise without changing the
credential's digest. A credential built by one of the `new_*` constructors had
no way to acquire one short of reaching through `credential_mut`, so this adds
`with_credential_status` and `set_credential_status` alongside the existing
`with_id`/`set_id` pair, with the same before-signing caveat: a Data Integrity
proof covers the credential minus its `proof`, so a status entry spliced in
after `sign()` leaves a document whose proof no longer verifies.

The audit asked for it as a REQUIRED constructor parameter on `new_vdc`,
reading the VDC draft. The text that merged makes it CONDITIONAL: a verifier
MUST be able to establish that an appointment is in force without contacting
the delegator, and either of two things satisfies that - a `validUntil` short
enough that expiry alone bounds the exposure, or a status entry it can check.
A VDC MUST carry one where its validity exceeds the freshness window the
governing VTC or VTN defines for delegations, and MAY omit it otherwise.

That window is governance this library does not know, so nothing here can
decide which side of the condition a given VDC falls on. Demanding the entry
from every caller would forbid the short-validity case the specification
prefers, and prefers for a stated reason: a status check is a live lookup that
reveals the verification event to whoever hosts the status list. Hence a setter
rather than a constructor parameter, and the constructor keeps supplying no
entry of its own. A long-lived appointment made in advance of a delegator's
unavailability is the case the property exists for.

Nothing here resolves the entry. Neither `delegation::verify_chain` nor
`authority::verify_chain` checks revocation; both verify structure, scope and
validity only, and that is unchanged.

`PartialEq` (with `Eq`) on `DTGCredentialType` is unrelated and one line:
consumers had to assert by pattern, and `assert_eq!` now reports the actual
variant when it fails. The deprecated `RCard` variant provokes no warning from
the derive.

121 tests, up from 118.

Refs: trustoverip/dtgwg-cred-spec#19
Refs: #10
Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
@stormer78
stormer78 requested a review from a team as a code owner September 9, 2026 12:14
@stormer78
stormer78 merged commit 3253b93 into main Sep 9, 2026
7 checks passed
@stormer78
stormer78 deleted the feat/credential-status-setter branch September 9, 2026 12:21
@stormer78 stormer78 mentioned this pull request Sep 9, 2026
stormer78 added a commit that referenced this pull request Sep 9, 2026
Cuts the two additions merged in #24: `with_credential_status` /
`set_credential_status`, and `PartialEq` on `DTGCredentialType`.

Both are additive - no API breaks, and nothing changes on the wire for a
credential that does not use them - so this is a minor bump rather than the
major the last two releases needed.

Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
@affinidi-appsecurity-bot

Copy link
Copy Markdown

🛡️ AI Agentic Security Code Review

🔎 A manual security review is recommended before merging. Please contact the Security team for specifics and remediation guidance.

ℹ️ Detailed findings are not published on public repositories; the Security team holds the complete report.

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.

2 participants