Repository navigation
Conversation
For any identified claim in any credential type, a configuration may now
declare that the claim carries a commitment over a per-subject seed rather
than a value from a data source. The issuer holds the seed; the credential
carries commit(seed) and never the seed.
The point of holding it is reproducing it: a pseudonym a Relying Party
presents can be mapped back to a person, which is what known-offender
matching and account recovery in the EUDI nym_auth rulebook are built on.
nym_auth is the first consumer, not the feature. The claim path, the
credential it belongs to, the digest and the seed length are configuration,
and pkg/credential/seed names no credential type.
**What the issuer does not do**, each refused on purpose rather than left
undone:
- It does not deliver the seed. Only the commitment is written, as a claim.
A derived seed is recomputable whenever it is wanted, so there is no
moment at which one must be handed over to avoid losing it.
- It does not keep a commitment from being disclosed. That is a wallet's
job against a rulebook; a control here could not stop a verifier that
asks, only create the belief that something was.
- It does not check a consumer's byte-level issuance profile.
**The derivation.**
seed = HKDF-SHA256(secret, info = lp("vc/seed/v1") || lp(seed_bytes)
|| lp(context) || lp(subject))
Every field is length-prefixed rather than joined, so no two can be slid
into one another - with a separator, context "a" + subject "b.c" and "a.b"
+ "c" would be one derivation and two people would share a seed. The seed
length is in there too, which it need not be: HKDF expand is a stream, so
a 32-byte seed would otherwise literally begin with the 16-byte seed for
the same subject.
The secret is NOT in the info. It is the HKDF key, it never changes, and
naming it would invite rotating it - which would repseudonymise everyone
ever issued to. There is deliberately no key identifier and no rotation
mechanism, and the documentation says in those words that losing it is
unrecoverable.
**Fail closed, all of it.** A derivation secret that is absent, too short,
or configured twice over; a seed model or a holder source that needs the
store this issuer has not got; a claim naming a seed source that does not
exist, or no subject path, or an unregistered digest - every one of them
refuses at startup rather than at first issuance, when someone is waiting.
At issuance: a subject identifier that is absent, empty or not a string,
and a commitment arriving from upstream in the document data, which would
produce a credential that looks remappable and is not.
**Also here:** pkg/nymauth implements the rulebook's pseudonym-v1-sha256
and its four configurations, and developer_tools/scripts/seed_recover
reproduces a subject's seed, commitment and pseudonym from the issuer's
own configuration, exiting 2 on a mismatch so it can drive a loop over
candidates. The rulebook publishes a vector for the seed/commitment pair
and none for a pseudonym, so that construction is from the specification
text and is marked as unchecked against a reference output.
Refs SUNET#791.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Five findings, all of them real.
**Two signing paths were skipped.** MakeVC20 and MakeJWP signed
req.DocumentData unchanged while startup accepted a commitments block for
any format - so a W3C or BBS scope could configure a commitment claim,
start cleanly and issue credentials without it. Silently, which is the
whole failure class this feature is supposed to close.
Both inject now. A new signing path added later would make the same
omission and no test written against today's handlers would notice, so
TestEverySigningPathInjectsCommitments walks the package AST for methods
on *Client named Make* whose request carries DocumentData, and fails if
one does not call injectCommitments. A position guard, because what it
catches is an absence.
**A literal dotted key shadowed the nested path.** lookupPath tried the
exact key before descending, in every format. In a nested document
"identity.id" and {"identity":{"id":...}} can both be present and carry
different people, so the seed could be derived from the wrong subject with
both lookups succeeding and nothing reporting anything. Flat and nested
are exclusive now, decided by the format, not ordered.
**The derivation was not pinned to anything.** Every test compared Seed
against Seed, which proves consistency with itself: changing the label,
field order, prefix width, hash or HKDF parameters would have left them
all passing while giving every subject a new seed, with no copy of the old
ones anywhere because nothing is stored. There are golden vectors now,
computed from the doc comment's construction by a separate implementation
rather than captured from this one.
**pkg/nymauth accepted any seed length.** The rulebook fixes both
secret_seed and private_seed at 16 bytes; anything else hashes a
concatenation shifted against what the circuit computes, and the output is
32 bytes either way so nothing downstream could tell. Reachable through a
generic seed source configured with a different seed_bytes - the cost of
the primitive not being nym_auth-specific, and now refused.
**The seed-model constants stole BBSConfig's doc comment**, dropping the
software-key/HSM explanation from the generated configuration reference.
Moved above it; a test-time assertion in the same change checks that
BBSConfig, SeedSource and CommitmentClaim each still adjoin their own
comment. docs/CONFIGURATION.md regenerated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Two more findings, both right.
**The redaction did not hold.** String and GoString are consulted only for
the string-capable verbs; a numeric one falls back to reflection, walks
into the struct and prints the secret as decimal bytes. Deriver implements
fmt.Formatter now, so the rendering is the same under every verb -
including ones where it reads oddly, because a redaction that holds for
the verbs someone thought of is not a redaction.
The test enumerates fmt's verb table instead of a hand-picked list, and
each verb is also checked through an enclosing struct, which is how this
would actually reach a log. Removing Formatter makes it report SIX leaking
verbs, not one: %d, %e, %f, %g, %O and %U. The review named %d; the
hand-picked list in the previous commit would have caught none of them.
**A path conflict deleted the claim in the way.** Injecting identity.ssc
into {"identity": "existing"} type-asserted the scalar to a map, failed,
and created a fresh map over it - so the upstream claim was gone from the
document before signing and the credential was issued missing an attribute
nobody asked to remove. Silently, because the injection succeeded.
The nested case delegates to credential.SetNestedValue, which already
refuses this and already has tests. Reimplementing the traversal is what
produced the bug; the error propagates and refuses the issuance.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
…ntry
**A copied Deriver still printed the secret.** Format was on a pointer
receiver, so fmt.Printf("%v", *d) and any struct holding a Deriver by
value fell back to reflection. Value receivers now - a pointer's method
set includes value-receiver methods, not the reverse.
Covering copied values then exposed one more: %p on a value. fmt resolves
%T and %p BEFORE it consults Formatter, and the bad-verb path it lands on
sets its erroring flag, which disables String, GoString and Format
outright - so no method can intercept it. go vet flags %p on a non-pointer,
but "vet would have caught it" is not the same as "the key cannot be
printed". The secret lives behind a pointer field now, which renders as an
address at that depth, so there is nothing to read either way.
**mdoc injected after the status list entry was allocated.** Injection can
refuse a document - no subject identifier, an upstream commitment, a path
conflict - and the entry is allocated before signing and never handed
back, so every such refusal burned a revocation entry marked VALID and
belonging to no credential. The other three signing paths already injected
before their allocation; this one did not, which is my inconsistency
rather than anything about mdoc.
The AST guard now pins the ORDER as well as the presence, so the next
signing path cannot get this wrong either.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
**seed_recover reported a match for every candidate.** -expect without -pseudonym-config returned before computing or comparing anything, and exited 0. The documented use is a loop over candidate subjects driven by the exit code, so an operator remapping a pseudonym would have concluded the first person they tried was the one behind it. The combination is refused now. **An mdoc commitment target the schema does not declare was dropped.** addElements emits the elements the schema declares and ignores anything else in the document, so injection succeeded and the credential was issued with no commitment in it, reporting nothing. Refused in two places, because neither alone is enough: the issuer checks at issuance, since it receives schemas inline per request and has none at startup; the apigw checks at startup, since it does have them and that is where an operator should find out. The namespace-qualified spelling is accepted, because that is how the claim is named everywhere else. **One commitment could derive its seed from another.** The injection loop read subject paths from the same map it was writing into, so a subject naming another commitment's target found the value just injected - a seed derived from a commitment, identifying nobody. Sorting the paths made that deterministic rather than correct, which is worse, because it looks fixed. Every subject is resolved from the document as received before anything is written. Each of the three fails silently rather than loudly, which is what they have in common and why they are worth the churn: a wrong answer in a remapping loop, a credential missing the claim it was issued for, and a seed derived from the wrong thing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
**A namespace-qualified target was written verbatim.** I added that leniency in the previous commit - accept the qualified spelling, since that is how the claim is named everywhere else - and accepted it without normalising it. An mdoc document is flat and keyed by BARE element identifier, so the value was written at a key addElements never reads and dropped. Worse, the pre-existing-value check looked at the qualified path too, so an upstream bare "ssc" went unnoticed and was what got signed: a commitment this issuer did not compute, in a credential that looks remappable and is not. Resolved to the bare element id now, for the lookup, the write and the startup check alike. **encoding: hex against a bstr target signed 48 bytes of nothing.** A bstr element arrives as JSON and mdoc decodes it as standard base64 - and a 64-character hex commitment is itself valid base64, so it does not fail. It decodes to 48 bytes and gets signed, and a 48-byte bstr is perfectly well-formed, so nothing downstream can tell. Refused at startup and at issuance. **Injection rewrote numbers it never touched.** Decoding into map[string]any turns every JSON number into float64, so re-encoding changed any integer past 2^53: 9007199254740993 was signed as ...992. MakeJWP forwarded the document untouched before this feature existed, so this was a silent data change for every number in every document passing through. json.Decoder with UseNumber. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Four findings, three of them regressions from my own last two commits. The
pattern is the point: I had two implementations of the same resolution -
one in the issuer for issuance, one in pkg/configuration for startup - and
every local fix to one of them drifted them further apart. So this is a
pass over the resolution rather than another patch.
CredentialMetadata.ResolveCommitmentTargets is now the only place that
decides where a commitment goes and how it is encoded. Both callers use
it. The schema is a parameter, since the apigw loads one at startup and
the issuer receives one per request - and because taking it from the
struct meant copying a struct that carries a mutex, which go vet was
right about.
What it fixes:
**Two configured paths could alias one element.** "ssc" and
"<namespace>.ssc" both resolve to "ssc"; neither is written when the
existing-value check runs, so both passed it and the second write silently
replaced the first commitment. Refused, naming both paths.
**A nil claim panicked at startup.** The check ran before the struct
validation that would have rejected it, so "commitments: {ssc: null}" took
the service down with a stack trace instead of a message.
**Resolution depended on map order.** A dotted path can be a literal
element identifier in one namespace AND a qualified spelling of a
different element in another. One pass over the map returned whichever
came up first, so startup could validate one target and issuance write
another. Exact matches are now resolved in a full pass before any
qualified matching, and a path qualified in two namespaces is refused
rather than guessed.
**Trailing bytes were accepted.** Last commit replaced json.Unmarshal with
a Decoder to get UseNumber, which is strictly weaker: a Decoder stops at
the end of the first value, so `{"a":1} garbage` decoded and re-marshalled
as valid JSON. MakeVC20 had full-input validation before this feature
existed and the JWP validator still assumes it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
…ents **The trailing-bytes check was unsound.** decoder.Buffered() exposes only what the decoder happened to prefetch, not what is left in the reader - so with enough whitespace between the object and the garbage the buffer holds nothing but whitespace and the check passes on exactly the input it was written to reject. A second Decode required to return io.EOF instead. The test case for it is the one that distinguishes them: with Buffered() the bare word, the second object and the stray brace are all still caught, which is why the original looked right. **A doc comment was displaced again.** Inserting checkCommitmentTargets directly above checkCredentialMetadataEntries left that function's explanation attached to mine. Same mistake as the BBSConfig one earlier in this PR, which I "fixed" with an assertion naming three specific types - a guard that enumerates cases cannot catch the case it did not enumerate. So there is a real one now, in both packages where I made the mistake: it derives the set of declarations from the AST and requires each doc comment to begin with the name of the thing below it. It catches the reinsertion, and it would have caught the first instance. The failure deserves the guard rather than more care: the inserted code compiles, every test passes, and the diff shows the displaced lines as unchanged context. What it breaks is generated documentation describing the wrong thing, which is to say it usually does not surface at all. One pre-existing violation is exempted with its reason - pkg/model's OIDCOP carries a comment naming the type's former name, which is on main and not this PR's to sweep in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
`go build ./developer_tools/scripts/seed_recover/` writes the binary to the working directory, not to bin/, and it was committed three commits ago by a blanket `git add -A`. The Makefile target builds to bin/, which is ignored; this adds the stray path as a backstop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
**The guard was wrong, and I said otherwise.** I claimed it "would have caught the first instance" - the const block inserted under BBSConfig's explanation. It would not have: it skipped parenthesised declarations entirely, so the group went unchecked, and the type below it was by then undocumented so it went unchecked too. Nothing failed. The claim is in the previous commit message and in a review reply, and it was false. Rewritten around NAMES rather than positions: every declaration name in the package is collected, and any doc comment - group or not - that begins with a name belonging to a different declaration is a displaced comment. Verified by reproducing the original mistake rather than by reasoning about it: putting the const block back under BBSConfig's comment now fails with that comment quoted back. **Nested parent and child targets could not both be written.** In a nested document "a" and "a.b" are mutually exclusive - the outer is written as a string, so writing the inner has to descend through it. Startup accepted the configuration and every issuance failed with a path conflict, which is a deterministic configuration error an operator should hear about at startup. Not a relation in a flat mdoc document, where a dotted element identifier is one key. **Only bstr and tstr elements may carry a commitment.** It is injected as a JSON string. int and uint fail conversion after a status list entry has been allocated and never handed back; bool, array and map do not fail at all - convertElementValue passes an unrecognised value_type through, so the commitment is signed as a CBOR text string where the schema promises something else, and only a verifier would ever find out. **The secret file format was documented wrong.** It said "at least 32 bytes" where seedSecret base64url-decodes it first, so an operator following the generated reference supplies 32 raw bytes and gets a decode error or 24 bytes. Now says base64url, decoded value at least 32, with a command that produces one. docs/CONFIGURATION.md regenerated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
…ier ids All three of this round's findings are the same shape: a value that is written correctly and then replaced, or computed correctly from an input that could never have been real. Nothing errors in any of them. **The SD-JWT builder overwrites its envelope claims.** BuildCredentialWithSigner assigns iat, nbf, exp, iss, jti, vct, vct#integrity, cnf and status unconditionally, AFTER the document this feature injects into has been parsed. A commitment configured at one of them was injected perfectly well and then replaced, so the credential carried the envelope value instead of commit(seed) - both writes succeeded, so nothing reported anything. The root segment is what is checked, not the whole path: status is assigned as a whole object, so status.value is destroyed just as completely. **MakeVC20 fills credentialSubject.id from the request.** Same shape, one claim, and also after injection. Both lists are taken from the signing code rather than from a specification, since it is the code that does the overwriting. **nymauth accepted any verifier_id.** The rulebook defines exactly semantic_id:<value> and x509_san_dns:<value>. Hashing "rp.example" succeeds and returns a 32-byte value indistinguishable from a correct pseudonym, so a remapping loop comparing against what a Verifier actually reported never matches - and the operator concludes the subject is not the one they are looking for. A wrong answer in the workflow this exists for, which is the third one of those in this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
SonarCloud's quality gate was failing on duplication - 7.2% against a 3% threshold - and all of it was mine: 125 identical lines, because I put the same guard in two packages rather than extracting it. I had noticed CI was green and not that this check was not. It is a package now, and the two tests are six lines each. The extraction also gives it its own tests, which it should have had from the start: a guard with no test of its own is a guard nobody has checked, and this one was wrong the first time I wrote it. Those tests cover the case that motivated it - a parenthesised group inserted between a type and its explanation - plus displacement between two single declarations, a group comment that merely names nothing, the exemption path, and that test files are skipped. The end-to-end check still holds: putting the seed-model const block back under BBSConfig's comment fails pkg/model's guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
MakeCredential turns a claim into a disclosure only when the VCTM lists its path with sd "always". Anything else is signed in the CLEAR - so a commitment at a path the type metadata does not declare, or declares as "allowed", was not missing from the credential. It was issued, permanently, in a form the holder cannot withhold. That is the one outcome this feature must never produce. The commitment is a stable identifier for its holder - the same value in every credential ever issued to them - and the wallet's ability to keep it back is the entire protection. Issuing it undisclosable hands it to every verifier unconditionally, and nothing anywhere reports that it happened. Checked in the shared resolver, so startup checks it against the configured VCTM and issuance checks it against the one the request carries. The issuer parses that VCTM before injecting rather than trusting configuration, because it is the document actually being signed. That parse now runs before the builder's own, so it reports an invalid VCTM with the wording the builder used - an existing test pins that contract and it should keep holding, rather than the test being updated to match my message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Both findings narrow the disclosure check added in the previous commit, and both let the same thing through: a commitment signed in the clear, which the holder cannot withhold. **The checks only recognised one SD-JWT spelling.** "vc+sd-jwt" is the legacy identifier this repo still issues and "" is an unset Format; both reach the same signing path. pkg/openid4vp's DCQL validation already lists all three, with a comment saying why - "every place that decides what a format MEANS has to agree with every place that decides whether it is CONSTRAINED" - and I had just done the thing it warns about. The review named the disclosure check. The reserved-claim check had the same hole and is now fixed with it: reservedTargets was keyed by format identifier, so a legacy-spelled scope could also put a commitment at cnf or status and have it overwritten by the builder. Both now go through one predicate, and the reserved lists are keyed by format FAMILY rather than by one of a format's spellings. openid4vp.IsSDJWTFormatIdentifier is deliberately not used: it has the same narrow definition, and it has no production callers, so widening it would mean editing a test that pins it in a file this change has no other reason to touch. Noted rather than swept in. **VCTM paths were flattened before comparison.** path ["a.b"] is one top-level key containing a dot; path ["a","b"] is nested. A configured "a.b" is always injected nested, so joining the VCTM segments made a literal top-level declaration satisfy a check for a nested claim - MakeCredential then never turns the nested value into a disclosure, and the commitment stays in the clear. Compared segment by segment now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
…builder The review named _sd_alg. There were two: _sd is written at the payload root as well, since a top-level claim's disclosure digest goes into whatever object its parent is, and for a top-level claim that is the root. Both are now reserved, and reserved beats declared - a VCTM listing either with sd "always" does not help, because the builder assigns them regardless. There is a test pinning that ordering. More to the point, the list is no longer something I maintain by reading. I built it by grepping for one variable name, which is how it came to be missing a claim assigned to a different one and another assigned deeper in. TestReservedSDJWTClaimsCoverTheBuilder parses pkg/sdjwtvc/methods.go and fails if the builder assigns a root claim the list does not contain - so the next claim added there fails this test rather than producing credentials with a commitment nobody can read. If the builder moves, the test fails and says where to point it. That is the intended behaviour: a list derived from code that is no longer there is a list that has quietly stopped being true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
… for For a W3C credential the proof covers the RDF canonical form, where a property the JSON-LD contexts do not define expands to a relative predicate and is DROPPED - and RDFCredential.ToCompactJSON then returns the original JSON verbatim. So a commitment at an undefined term is visible in the issued credential and NOT covered by the proof over it: alterable without invalidating the signature. That is worse than the format not being supported, so it is not supported. Refused at startup, naming the reason. Supporting it properly means resolving commitment paths against the pinned contexts and refusing undefined or non-absolute predicates. That is real work and belongs with the JSON-LD context handling, not bolted onto this. The honest history: mso_mdoc and SD-JWT VC are the formats the design covers and the ones whose signing layers this code has a complete account of. W3C and JWP injection were added reactively in the first review round, to answer a finding about coverage, without that account - and this is what was behind the gap. The first round's finding offered "reject unsupported formats during startup validation" as the alternative, which is what this now does. The blanket refusal subsumes the reserved subject identifier those formats carried, so that entry is gone and the reserved lists have one family left. My own doc guard caught the comment displacement that caused, which is the first time one of these guards has fired on live code rather than on a deliberate probe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
First of two for SUNET#794. The wire model and its conformance tests; no verifier wiring and no configuration, which is PR 2. A circuit_zk query discloses nothing: every value a Verifier wants is the OUTPUT of a function it asks for in `req`, computed inside the circuit and returned under a handle. That makes it unlike mso_mdoc_zk, which is a ZK presentation of an mdoc and still discloses attributes. **The shape is settled by the shared vectors, not by prose.** The rulebook's normative text and its WIP example disagree - `req` with id/return/params against `claims._req` with fn_id/id - so the vendored planner vectors from siros-dc-matcher decide it. This emits the normative form; the WIP form is reachable but not modelled. **Nothing here validates.** The builder is a serialiser: no coercion, no dropping empties, no repair. Two reasons, and either alone would be enough. The verifier is the tool wallets are tested WITH, so a builder that fixed a bad request could not produce the bad requests a negative test needs. And the rulebook, OpenID4VP and the DC API are all moving while several wallet generations sit in the field, so a request shape this build has never been taught still has to go out. Hence extension at every level: `Extra` on the credential query, on a req item and on the circuit_zk client-metadata entry, flattened rather than nested; and `Raw` on the credential query for the case Extra cannot cover, where a member already modelled changes TYPE. The rulebook's WIP example does exactly that to `claims`, and a later generation could do it to `req` or `meta`. One escape hatch covers all of them; a per-member override would be a field this struct grows every time a specification moves. What the tests pin: - Every one of the 51 shared vectors can be emitted byte for byte. 30 through the typed model, 20 through Raw, and a separate guard asserts that every shape a wallet must ACCEPT goes through the typed model - with one recorded exception, the WIP envelope - so the first test cannot pass by everything quietly falling through to Raw. - Adding MarshalJSON to CredentialQuery and VPFormatsSupported changes how every format is encoded, not just this one. With no extension set the output is byte-IDENTICAL to the struct encoding it replaced, compared against a method-less type alias. These bytes are signed, so equivalent is not good enough. - Requests violating rulebook MUSTs - le before ge, two pseudonym items, a duplicate handle, an unknown function, an over-long verifier_context - go out exactly as configured. - What a Go struct cannot say is named rather than left to be discovered: absent and empty are the same for a string field, so four shapes in the shared set reach the wire only through Raw. The vectors are vendored rather than fetched, so the suite needs no network. A snapshot goes stale silently, so refreshing is a command - `make vendor-circuit-zk-vectors` - and SOURCE records the commit it came from. Refs SUNET#794. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Three findings, two of them mistakes I made and fixed in SUNET#793 and then made again here. **copyDCQL dropped every new field.** It reconstructs each credential query member by member, so Req, Extra and Raw simply vanished on TemplateDCQLQuery - the verifier's main template path. Silently: the result is still a valid query, just not the one that was configured, and a circuit_zk request that lost its functions asks the wallet for nothing. The fields are copied now, but the fix that matters is the guard. TestCopyDCQLCarriesEveryCredentialQueryField derives the field set from the type: the fixture must leave no field zero, and the copy must equal it. Adding a field fails it twice - once for not being covered, again if the copier drops it. A list in a comment would have been correct until the next field. **Params lost precision.** Decoding into `any` makes every JSON number a float64, so 9007199254740993 was re-emitted as ...992 - a parameter the verifier never configured, inside a request a wallet is being tested against. That is the same defect as the UseNumber one in SUNET#793, in a new package, written by me after fixing it there. Every decode in this model now goes through decodeExact, nested values included. **The collision check only saw what omitempty left.** It compared Extra against the ENCODED output, so a modelled member whose field happened to be empty could be supplied through Extra after all - quietly, and only sometimes, which is the worst shape a rule can have. It now checks against every member the type declares, derived by reflection over the struct tags rather than listed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
The circuit_zk client-metadata entry carries doc_example tags, so the generated reference moved and config-docs failed. I added the tags and did not run the generator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Extra was tagged yaml:"-", so an operator configuring .verifier.preferred_vp_formats.circuit_zk had its members read and silently dropped - an escape hatch that existed in the generated documentation and not in the product. The envelope form the docs explicitly advertise, fn_id and input, was exactly what could not be expressed. The review named CircuitZkFormat. openid4vp.DCQL is itself loaded from YAML by pkg/configuration's presentation request templates, so CredentialQuery and ReqItem had the same hole and would have produced the same finding against PR 2. All three have MarshalYAML/UnmarshalYAML now, mirroring ZKSystemTypeSpec, which solved this for its own Params in this package. A test asserts YAML and JSON agree about what is modelled and what is an extension: the same request written on either side has to go out the same, or a template behaves differently depending on which form it was authored in. **And the collision check I wrote for YAML had the bug that had just been found in the JSON one.** It compared against the populated map, so a modelled member whose field was empty could be supplied through Extra after all - the same "quiet, and only sometimes" failure, reproduced within the hour while fixing something else. My own test caught it. Both sides now check declaredJSONKeys rather than inspecting output, and the YAML test covers the empty-field cases specifically, since those are the ones a populated-map check passes. docs/CONFIGURATION.md regenerated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Three findings on the YAML commit, all mine. **Raw could not survive the HA cache.** A request object is JSON-encoded into Mongo or Redis and read back by another node, so a raw query the model cannot hold - object-valued claims, say - was written out fine and then failed to load anywhere except the node that built it. An escape hatch that only works on one replica is not one. Decoding is total now: a shape the typed model cannot hold is kept verbatim as Raw rather than refused. That also keeps this serialiser's own promise - refusing to read back something it had just written would be a repair by other means. Invalid JSON is still an error, since the fallback keeps shapes, not rubbish. **The YAML decode went through JSON, and should not have.** The round trip worked, but only because of what the current encoding/json does with yaml.v2's map[any]any: it coerces an integer key to a string and drops a sequence key SILENTLY, and that behaviour has moved between releases. Known members now decode into a methodless alias so yaml.v2 applies the struct tags itself, and unknown members are normalised separately, with a non-string key refused rather than coerced or dropped. The review said this breaks any template with a nested mapping. It does not, on this toolchain - I checked before changing anything, and ordinary meta/req templates load - but the fix is right anyway, because relying on that was not something I had decided to do. **A wrong type was silently emptied.** The comma-ok assertions turned `id: 7` into "". Through the alias, yaml.v2 coerces it to "7" exactly as it does for every other configuration struct in this repository, and a structural mismatch is an error. Coercion rather than refusal is deliberate: diverging here would make this one type behave unlike the whole configuration surface. One test needed correcting rather than extending. The emission guard counted a vector as "handled by the typed model" whenever decoding did not error - which, once decoding became total, was always. It checks Raw is nil now, so the distinction it exists to make still holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Found while wiring the next branch on top of this one, which is where it would have been noticed in production too. A raw query carries no Format - the bytes are the query, and nothing here models them. But an empty Format means SD-JWT to every other reader in this package, so `ValidateCredentialQuery` sent a raw query down the SD-JWT branch and refused it with "vct_values is required". That check stands on `UIInteraction`, which is the one door a browser-built query arrives by, and on both VC 2.0 paths. An escape hatch that cannot be sent is not one. It now declines to judge a raw query. That is the honest answer rather than a convenience: nothing in this package knows what these bytes mean, so there is no rule to apply to them. It still checks they are JSON, because they are emitted verbatim and bad bytes would otherwise surface as an unencodable request object at send time, a long way from the configuration that caused it. Writing the test corrected my own account of the bug. Two of the three shapes I first reached for - a query with no format, and one naming a format this build refuses - do not fall back to raw at all; they are modelled, so they decode typed and are judged exactly as before. The hatch opens only for a shape the model cannot hold, which is a narrower door than I had described, and the cases now pin shapes that actually go through it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Two findings on the previous commit, both right, and both about what making the decode total actually did. **The stack routes by members raw had erased.** `UIInteraction` copies a query's id into the authorization context's scopes and the response path matches a presentation back to it, so a query that arrived raw and kept an empty id was sent correctly and then matched nothing that came back - the request works and the result vanishes, which is the worse half of the two. Format is read the same way by every format decision downstream. Both are now taken out of the raw bytes and kept in the struct, best effort: a member that is absent or of a type this build does not expect is left alone rather than coerced. Raw stays authoritative for the wire; these are a read-only shadow of what is already in it. **And it had become a way around the constraint rules.** Any query with one wrong-typed member fell back to raw, and raw was then not judged at all - so an SD-JWT query with no vct_values passed the ingress and would be signed and served. That is an unconstrained request, which is precisely what those rules exist to refuse; I had written a test asserting the new behaviour, which documented the reversal rather than catching it. A raw query is now judged by its own bytes: format and meta are re-read from them and the normal per-format rule applies. The member that carries the CONSTRAINT usually survives whatever made the query unmodellable - the WIP rulebook shape only changes "claims" - so this is not a consolation check, it is the same check. An unreadable meta yields no verdict, because there is no constraint there to judge; an ABSENT meta is readable and zero, so the unconstrained case is still caught. The review suggested restricting the hatch to circuit_zk instead. That would not fix this - a circuit_zk query can be unconstrained too - and it would break the thing the hatch is for, since a later generation of any format is exactly what cannot be named in advance. Judging by format, whatever the format turns out to be, covers both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Both from the suppressed findings, both in code the reviews had not flagged before, and both the same omission: an extension point that works from JSON and not from the path an operator actually uses. **preferred_vp_formats is loaded from YAML.** VPFormatsSupported.Extra is tagged `yaml:"-"` and the type had only the JSON pair, so a format identifier this build has never heard of was dropped silently at exactly the point it would be configured. The promise held only for metadata this code constructed itself, which is not where a format that postdates it comes from. It has the flattening YAML pair now, like the other bags. **And req params are an input to a proof.** The struct tag puts params in a map[string]any, but yaml.v2 leaves every nested mapping inside it as map[any]any, so params was the one open tree not getting the walk Extra gets. A key with no JSON spelling is then coerced to its rendering, or - for a sequence key - dropped with no error anywhere. A changed parameter produces a different pseudonym, and that failure appears nowhere near the configuration that caused it. Worth being exact about what changed, because one of my tests was not. A nested tree whose keys are all strings already encoded correctly through encoding/json here; the walk changes the non-string-key cases, and makes staying correct this code's decision rather than that package's. The test I first wrote asserted the nested-tree case and passed with the fix reverted. It is still there, renamed to say what it actually pins - that the walk does not break a valid tree, which is the direction a rebuild can break - and the behaviour change is pinned separately. Also in that pair: a sequence key never reaches the walk, because yaml.v2 refuses it first. Asserted on the outcome rather than the wording, since which layer catches it is not the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
Third time on this branch that treating "raw" as "unjudgeable" opened a
door, and this one I had been told about in general terms already: a check
that cannot compare must refuse, not pass.
A raw query whose meta was a shape this build could not read returned
success. So `{"format":"vc+sd-jwt","meta":"not an object"}` reached the
signer with no vct_values - the exact case the format rules exist to
refuse, reached by making the member HARDER TO READ rather than by leaving
it out. A query that is not an object at all went the same way.
An unreadable meta is now judged as an EMPTY one. Downstream, the two are
the same thing: either way nothing has established what the request is
limited to.
Substituting the zero value rather than listing the formats that have
rules keeps the two in step by construction - a zero MetaQuery satisfies
no constraint rule in the switch and passes the permissive default exactly
where there is no rule to fail, so a format that grows one later is
covered with nothing here updated.
The error says what actually happened. "vct_values is required" would send
an operator looking for a member that may well be present, inside the meta
this could not read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
The routing members are populated by UnmarshalJSON, so a query built in
code - CredentialQuery{Raw: ...}, which is how the escape hatch is meant
to be used directly - carried none of them. Validation then saw an empty
Format, which every reader in this package takes for SD-JWT, and refused a
raw circuit_zk query for a vct_values it was never going to have. On every
path that validates.
The bytes are the source of truth for id and format, so validation reads
them from there now and does not trust the struct's copies. Those copies
exist for the routing the rest of the stack does, which is a different
job; reading them back here was me using one as the other.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
The shadow started as "id and format", because those were the two members I had a concrete report about. That was a list, and lists go stale on the next member - which is this one. A raw W3C query with valid meta.type_values passed validation from its own bytes and was then MATCHED against an empty shadow Meta, so it rejected every credential. Explicit multiple and require_cryptographic_holder_binding settings reverted to their defaults the same way. Raw is authoritative for the wire and nothing else; plenty of this stack reads the struct, and it was reading values nobody chose. Every json-tagged member is shadowed now, derived from the struct tags, so a member added to CredentialQuery is covered by the commit that adds it. Still best effort per member: one that is absent, or of a shape this build cannot read, stays at its zero value rather than failing the whole decode - a member this code has never seen is why these bytes are raw at all. A query that is not a JSON object is treated as an unreadable meta rather than a separate case. It carries no members, which is the strongest form of "the constraint cannot be read", and it has to keep failing closed. Also moved here from the branch above, where I had put it by mistake: the vendoring target recorded the matcher's HEAD while copying from its WORKING TREE, so a dirty or partly-untracked checkout produced a SOURCE line naming a commit that cannot reproduce the vendored files. These vectors are the authority this package settles wire-shape questions against, so provenance that is merely usually true is not much use. It now refuses to run unless that directory is clean - unstaged, staged and untracked all checked. That immediately earned its keep: SOURCE named f7d240b (v0.7.0-2) while the matcher is at d68cf52 (v0.8.0). The 51 files are byte-identical between the two, so nothing was wrong with what is vendored - only with what it said about itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
The fallback asked the wrong question. It tried the typed decode and went to Raw only when that ERRORED - but `"req": []` decodes perfectly well into an empty slice, which omitempty then drops, and an item missing "id" decodes to "" and comes back as a member that was not there. Those survived the decode and did not survive the round trip, so the HA cache quietly repaired them. Which is the one thing this serialiser is not allowed to do. Those exact shapes are fixtures for wallet behaviour - every planner rejects them, and sending one is how a negative test is run - so repairing them makes the tests they exist for impossible. The test is now "does it encode back to what arrived", compared as JSON values rather than bytes, since member order and whitespace are not something this type promises to keep and treating them as loss would send every ordinary query down the raw path. Numbers compare by literal, so a value that re-encoded with different precision counts as changed - which, for a req param, is what it is. This retires a test rather than adding one. TestAbsentAndEmptyAreNotDistinguishedInTheTypedModel documented the lossiness as a boundary and said in as many words that if these shapes ever round-tripped it could be deleted. They do now - not because the struct gained a distinction, but because decoding stopped insisting on using it. It is replaced by the opposite assertion, through the cache as well as the serialiser, plus its counterpart: a shape the model DOES represent faithfully must still decode typed, or the distinction stops meaning anything and every query becomes opaque bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
The shadow was filled in on the decode path and, locally, inside
validation - which takes a value, so the caller's query kept its zeros.
The documented way to use the escape hatch, CredentialQuery{Raw: ...},
therefore produced a query that is SENT correctly and then tracked under
defaults nobody chose: UIInteraction records ID in the authorization
context's scopes and the response path matches a presentation back to it.
It fails as a result that cannot be matched, with nothing malformed to
point at.
NewRawCredentialQuery fills the shadow in, copies the bytes so a later
mutation of the caller's slice cannot change what is sent, and refuses
bytes that are not JSON here rather than at send time, where it would
surface as an unencodable request object far from the call that caused it.
"Callers should use the constructor" is advice, and advice does not
survive the next caller - which is how the shadow came to be two members
in the first place. So production code may not write the literal at all:
an AST walk over every non-test file in the repository fails on a
CredentialQuery composite literal with a Raw key outside the one file that
populates the shadow. It fails closed if the walk parses nothing.
Tests may write it, and should - asserting what a hand-built query does is
the subject of several of them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
"Emitted as exactly these bytes" was never true. encoding/json compacts and HTML-escapes whatever a MarshalJSON returns on its way into the enclosing document, so an operator's layout is dropped and an & inside a string is written as its escape. Nothing here can prevent that short of re-implementing the request-object encoder, and there is no reason to: a conformant wallet cannot tell the difference. What is actually guaranteed - every member, its nesting and every value survives unchanged, including members this build has never heard of and numeric literals it would otherwise reformat - is what the hatch is for, so the claim is narrowed to that rather than the mechanism changed. The tests could not have caught the overclaim, because they all compare JSON semantically; that is the right comparison, and it means the byte claim was never under test at all. So there is now one test asserting both halves: that the configured layout and the literal ampersand do NOT reach the wire, and that the value - claims as an object included - comes back exactly, with the escaped ampersand reading as the string that was configured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
leifj
changed the title
Two key names from the v0.4 rename were changed on the Go side and not
in the browser. The first broke the live gdc verifier outright; the
second was behind it and would have broken the wallet round silently.
The item key. req-schema.js validated `typeof item.id === "string"`
while the server publishes `fn_id`, so parsing /ui/metadata threw
"req must be an array of {id, return, params?} items" and the page
never left loading. Server endpoints were all fine, which is why this
read as a front-end mystery rather than as the rename it was.
The req key. The browser copied the preset's `req` into a posted
`req`, and CredentialQuery.Req is tagged `_req`. encoding/json does
not match one to the other - it drops it - so the posted query would
have carried no items at all, and the request would have gone out
asking the wallet for nothing. No error at any layer. Proved with a
probe before fixing: posted "req" -> len(Req)=0, posted "_req" -> 1.
The two keys are not a mistake to unify. `req` is the preset key, this
repository's own configuration surface, and `_req` is v0.4's wire
member; the copy in handleSelectPredefinedPresentationDefinition is
the one place they meet, and both sides now say so in a comment.
The existing JS test passed throughout because its fixture was never
moved with the rename - it still built items with `id` and the old
`pseudonym-v1-sha256`. A fixture that was not updated is not a test
that was not written; it is worse, because it reports success. It now
pins both directions: the published shape is accepted and the shape it
replaced is refused.
Three guards, each proved by restoring the defect:
- the preset round trip drives the REAL /ui/metadata payload through
the browser's copy and into a CredentialQuery, then through
UIInteraction. Written over the published JSON, not the Go structs,
because both key names exist only in the JSON.
- the schema guard now requires `_req` in the DCQL schema, the literal
copy `c._req = cred.req`, and `item.fn_id` in req-schema.js.
- req-schema.test.js refuses an item carrying `id`.
Also finishes the rename in the generated configuration reference,
which still said circuit_zk in 20 places - including
`preferred_vp_formats.circuit_zk: {}` in a worked example an operator
would copy, and which startup now refuses by name. All prose; the only
literals that keep the old spelling are the legacy-key refusals, which
exist to name it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
…ect-handover # Conflicts: # docs/CONFIGURATION.md # pkg/openid4vp/dcql.go # pkg/openid4vp/zkp_req.go
…mauth main gained the seed-commitments work (SUNET#793, 6f4a95c) plus several merge PRs, which put a second copy of pkg/nymauth and seed_recover on main - the copy from before the v0.4 rename and before the verifier_id inversion. Every branch in this stack went CONFLICTING at once. Resolved toward this branch for everything nym_auth, because main's copy predates both decisions, and toward main for everything else. Textual conflicts were small; one was not textual at all: internal/wallet/apiv1/vci_presentation.go is new on main and reads `!query.Multiple`, while this stack made Multiple a *bool so an absent one is distinguishable from an explicit false. git merged the two cleanly and the result did not compile. It now calls AllowsMultiple(), which applies OpenID4VP 1.0 6.1's default of false - the same answer the bool gave, for an absent member and an explicit false alike. Two documents are left saying what this branch's own code says and will be corrected where the rule changed, not here: the seed_recover `-verifier-id` help and SEED_COMMITMENTS.md still describe the BARE verifier_id. That is correct for this branch, whose checkVerifierID still refuses the prefixed form; the inversion lives one branch up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
…onse # Conflicts: # docs/CONFIGURATION.md
The first live Pixel round reached this verifier, was refused for a reason the verifier had decided and could name - the wallet sent the DS leaf plus the root, and these circuits take exactly one key - and the wallet saw HTTP 500 internal_server_error with a bare req_id. So the round was spent looking at the wallet for a decision made here. Every one of the 32 refusal sites in this path returned a bare error, and NewErrorFromError redacts what it does not recognise, which is right for an unexpected failure and wrong for a verdict. Stable codes now, in the detail-free style the shared matcher vectors use, rendered as 400 and logged at WARN. A shape refusal keeps the shared validator's OWN reason code rather than being renamed here, so a wallet author compares one vocabulary against the vectors instead of two. What stays a 500 is the point. A verifier built without the zknative tag, with no trust evaluator, with an allow-all evaluator, with a verifier_id it will not derive from, or with a circuit that will not load has not refused a presentation - it cannot judge one. Telling a wallet "your presentation is bad" when the fault is ours sends its author hunting for a defect that is not theirs, so those keep the redacted 500 they should have. The split is one function, zkpReqError, which every return in verifyCircuitZkScope goes through. proof_time out of range carries the deviation and the window in its detail: without them a wallet cannot tell a wrong clock from a stale proof, and both land on the same code. The key-set-size refusal now says what to send instead - the Document Signer certificate alone - because the prover otherwise has to guess which of its certificates to drop. Tests: one per code, each proved non-vacuous by restoring the defect - a bare error, a 500 status, a renamed shape code. A guard asserts every declared code is exercised, reading both refusal test files because three codes exist only behind the zknative tag; it immediately caught five codes I had not covered, which is the same gap that put the 500 there to begin with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
…ect-handover # Conflicts: # internal/verifier/apiv1/circuit_zk_scope.go
The allowlist was keyed by pseudonym configuration alone, so a second
entry for one configuration was a startup refusal - "a second circuit
is allowed for configuration %q". That made a verifier running BOTH
Longfellow and Vega nym_auth impossible to configure, which is the
shape leifj decided on 2026-10-24: one verifier offers both, and the
wallet picks between them.
So the key gains the proof system a presentation states in zkp_req's
"system_id", and an allowlist entry gains a required system_id naming
it - the catalog alias ("urn:circuits:longfellow:nym_auth:v1",
"urn:circuits:vega:nym_auth:v1"), which is what travels on the wire,
not the catalog id and not the proofSystem name.
Two entries that differ only in system_id are now a verifier offering
both systems rather than a clash; two entries agreeing on both parts
are still refused at startup, for the reason they always were.
Letting the holder's stated system select a build is safe because it
cannot change the answer. The pseudonym's domain separator is built
from the configuration alone - "nym_auth/v1/" + config - so the same
subject under the same configuration has the same pseudonym whichever
system proved it; the system decides only which library checks the
proof. The stated value is also validated against the verifier's own
offered system_id_values before it reaches any of this.
An unknown or missing system_id is refused at construction, beside the
existing configuration check and for the same reason: a system this
build cannot map to a verifier is an artifact with nowhere to go, and
deferring that to the first presentation shows the operator a refused
proof with nothing pointing at the configuration.
BREAKING: every verifier.nym_auth.circuits.allow entry must now name a
system_id. A deployment tracking this branch - gdc - needs the line
added before it will start. Nothing released carries this
configuration; the allowlist does not exist on main.
Proved non-vacuous: with the key reduced to its configuration half the
package still compiles and the three new tests fail, and the
system-selection one fails by RESOLVING - a Vega lookup silently
returned the Longfellow circuit's bytes, which is the defect the
second key part exists to stop. Removing the system_id check leaves
the other nine malformed-entry cases passing and fails only the two
new ones.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
…spatch Carries the allowlist re-key to (system_id, config) forward, and threads the system through this branch's dispatch: the presentation's stated system_id now selects the build beside the configuration, and the loaded-handle cache is keyed by both. The fixture conflict in pkg/configuration kept this branch's newer shape - the PDP and the prefixed verifier_id - and took only the new required system_id member from the other side. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #830 (which is stacked on #829). Like the rest of this stack it targets
main, so its diff shows the commits below it too — review only the last commit,99840fdda. Merge after #830.A zkp_req presentation can now arrive by redirect (a scanned QR or a link, where the wallet POSTs to
response_uri) and not only through the Digital Credentials API.Why this is not a one-line addition
The two handovers are different structures:
A wallet built one of them. Verifying against the other produces an opaque proof failure, which sends whoever debugs it to look at the circuit — the same trap as #652 and #655 on the mdoc ZK path. So the channel is chosen by how the response actually arrived, both channels have an entry in
circuitZkTranscripts, and a channel with no entry is still refused rather than given a transcript of this verifier's invention.MdocSessionTranscriptRedirectis a separate entry point rather than a flag on the existing one, and the CBOR comes frompkg/mdocrather than being re-derived here: a second implementation of a shape is how two of them drift apart.The thumbprint is required here, where DC API allows nil
Over a redirect the response crosses the network, so nym_auth requires
direct_post.jwt: a pseudonym is a stable identifier for a person across that verifier's sessions, and one sent toresponse_uriin the clear is readable by anything on the path. A nil thumbprint means an unencrypted response, which is why it is in the sharedrejects_redirectvectors rather than left to each implementation to decide.The response mode is also checked where the session is known, so an unencrypted redirect request is refused by name instead of surfacing as
the reader key thumbprint is 0 bytesthree calls later.Vectors
Re-vendored from sirosfoundation/zk-cred-vega
274ed845, which addsderive_redirect(6) andrejects_redirect(8). All 14 run through this builder, including OpenID4VP 1.0 appendix B.2.6.1's own non-normative example — the one case whose expected bytes come from the specification rather than from a sibling implementation, so agreeing with it is agreeing with the standard and not merely with ourselves.That commit sits on the sirosfoundation/zk-cred-vega#13 branch and has not reached main;
pkg/nymauth/verifier/testdata/SOURCEsays to re-pin when #13 lands, and the file digest is not expected to change when it does.The reasoning behind the encryption rule is written up in sirosfoundation/nym_auth_rulebook#9, cited in the code as the unmerged draft it is. The enforceable part of it is in the vectors.
Verification
make test-zknativegreen;./internal/verifier/...and./pkg/nymauth/...green.response_urinot required,client_idnot required, redirect routed to the DC API transcript,direct_post.jwtnot required, a missing session passing the mode check, and the two call-site probes below.checkCircuitZkResponseModealone passes whether or notverifyCircuitZkScopeever calls it, soTestCircuitZkRedirectResponseModeIsEnforcedByTheHandlerdrives the fullVerificationDirectPostpath instead.🤖 Generated with Claude Code
https://claude.ai/code/session_01AB3apg5kCEGNPozrhSggum