Fix credential_selection field-id ambiguity in two-VP and intra-PD binding

[stevenvegt]

Problem Statement

Two interacting issues in the current Presentation Definition (PD) field-id handling surface as opaque token-request failures and silently inconsistent credential selections.

1. Cross-VP credential_selection is over-strict. In the two-VP RFC 7523 flow, the wallet automatically captures every id-bearing field value from the organization VP and merges them into the credential_selection used to build the service-provider VP. The SP-side selector then validates that every key in the merged selection must correspond to a field id in the SP PD. Captured keys missing from the SP side fail loudly. Declaring useful selection ids on the org PD (e.g. patient_bsn, delegating_uzi) breaks any caller that doesn't also declare those ids on the SP PD, even though the caller never asked for SP-side filtering on those fields.

2. Same-id fields across descriptors don't constrain values. Within a single PD, when the same id name appears on multiple field-objects (e.g. org_ura on the healthcare-provider descriptor and on the professional-delegation descriptor), a PD author naturally expects "the chosen credentials must agree on this id's value". The current selector treats each descriptor independently, so it can select an HCP VC with org_ura=A and a delegation VC with org_ura=B. The id is effectively decorative within one VP.

A third concern: the matching logic is currently spread across vcr/pe, vcr/holder, and auth/client/iam. Reasoning about the combined behavior requires reading multiple packages and several layers of indirection. The rules cannot be exercised as a single unit.

Solution

• A PD author can declare id-bearing fields freely on either side of a two-VP flow. Cross-VP capture stays internal and is silently filtered down to ids that exist on the target PD before being applied.
• When the same id appears across multiple descriptors in one PD, the wallet finds an assignment where every shared id resolves to the same value across the chosen credentials. The search uses DFS with forward checking on id-bearing fields, falling back to skipping optional descriptors only after all candidate combinations are exhausted.
• A PD whose same-id fields have incompatible filters fails to load, so the PD author learns about the inconsistency at boot time rather than at request time.
• PDs that admit more than one consistent assignment are reported as ambiguous (not silently resolved), and the caller is told which descriptors carry the ambiguity so they can add disambiguating selection keys.
• The API caller gets an early OAuth-shaped invalid_request 400 when credential_selection contains a key that doesn't appear in any PD relevant to the call. Typo protection moves to the API entry, where the full set of in-scope PDs is known.
• The matching algorithm is consolidated into a single pure function in vcr/pe, so all the policies above can be exercised by one black-box test, without mocks.
• The engine records, per request, why each candidate credential was or wasn't selected for each input descriptor (a structured MatchReport), so developers can diagnose why a wallet doesn't satisfy a PD. This folds in Facilitate developers with their policy development #4218 and supersedes the debug-logging approach of closed PR Add debug logging to PD matching #4225. The engine is made traceable now (report opt-in, no overhead otherwise); the developer-facing surface that exposes it (a dev endpoint/UI) is a follow-up.

User Stories

1. As a PD author, I want to use the same id on fields in multiple descriptors so that the wallet enforces that all chosen credentials agree on that id's value, without having to express the constraint somewhere else.
2. As a PD author writing a two-VP scope, I want to declare id-bearing fields on the organization PD that don't exist on the service-provider PD, without breaking the token request flow for callers who never asked for those fields to be filtered on the SP side.
3. As a PD author, I want a PD with inconsistent same-id fields to fail to load at startup with a clear error message naming the offending field id and the conflict, so I don't ship a configuration that silently fails at request time.
4. As an API caller using /request-service-access-token, I want unknown keys in my credential_selection to fail fast with a 400 OAuth invalid_request error naming every offending key, so typos surface before any submission is built.
5. As an API caller on a two-VP request, I want credential_selection keys to be validated against the union of in-scope PDs (org and SP), not against each PD independently, so I can pass keys that target only one side without spurious errors.
6. As an API caller, I want a PD that admits more than one consistent assignment of credentials to be reported as ambiguous (not silently resolved), so I can add disambiguation keys deterministically.
7. As an API caller, I want a credential_selection key that matches a real field id but for which no candidate carries the requested value to report ErrNoCredentials, not crash or fall back to a non-matching candidate.
8. As a contributor to nuts-node, I want the selection engine to be a single pure function whose policies are covered by a black-box test in one place, so the rules aren't smeared across the wallet, auth handler, and PD-matcher layers.
9. As a Nuts node operator running the release, I want the release notes to call out the NewFieldSelector semantic change, the new same-id binding behavior, and the new load-time PD validation, with a one-line audit checklist, so I can review my deployed PDs before upgrading.
10. As a developer writing or debugging a policy, I want the engine to record why each candidate credential was or wasn't selected per input descriptor, so I can see why my wallet doesn't satisfy a PD without trimming the policy file by hand (folds in Facilitate developers with their policy development #4218; supersedes Add debug logging to PD matching #4225).

Behavior Specification

Numbered policies. Each policy is the contract for one well-defined situation; the engine implements them together.

Policy 1: user input is validated up front, against the union of every PD that the request touches

A caller's credential_selection key has to mean something. Silently dropping unknown keys at the API surface lets typos through unnoticed.

The API handler for /request-service-access-token (and any other endpoint that accepts credential_selection) builds the union of field ids across all PDs that will be evaluated in this request: one PD for single-VP, two for two-VP. Every supplied key must appear in that union. If any are not, the handler returns 400 invalid_request with all unknown keys named in the error description.

Worked example. Caller sends {"org_did": "...", "favourite_color": "blue"} on a two-VP request. The union is {org_did, org_ura, ..., sp_did}. favourite_color is not there, so the handler returns 400 with error_description: "unknown credential_selection keys: favourite_color".

Policy 2: cross-VP capture is internal plumbing, not a public selection input

The org-side capture exists to bind the SP-side credentials to the org's identity (so the SP delegation's $.issuer equals the HCP's credentialSubject.id). It is a mechanism, not a public contract; non-applicable keys must not poison the SP build.

After the org VP is built, the engine captures resolved id-bearing values from the chosen org credentials. Those values are fed into the SP build as initial bindings (Policy 3). Keys whose names don't appear as field ids on the SP PD are silently dropped before the SP build starts. A conflict between a user-supplied value and a captured value cannot happen, because user-supplied values are already applied as a filter when selecting the org credentials, so captured values agree with user-supplied values for any shared key.

Worked example. Org PD has id_patient_enrollment carrying id: "patient_bsn". The org VP build picks a credential where patient_bsn resolves to 999911234. SP PD has only org_did and sp_did as field ids. The captured map {org_did: "did:web:...", patient_bsn: "999911234"} is filtered to {org_did: "did:web:..."} before being handed to the SP matcher.

Policy 3: inside one PD, an id name is a binding name; same-id fields across descriptors must agree

An id only earns its keep if it carries a single value across the credentials that share it. Otherwise it is decoration.

When matching a PD, the engine walks descriptors in PD order. At each step it tries every candidate that is consistent with the running bindings map (id to resolved value). Picking a candidate adds its id-bearing field resolutions to the bindings; trying the next descriptor's candidates filters against those bindings. Initial bindings (from the API caller's selection or from cross-VP capture) seed the map. Keys in the initial bindings that are not field ids on this PD are silently dropped.

Worked example. PD has id_healthcare_provider (carrying id: "org_ura") and id_professional_delegation (carrying id: "org_ura"). The wallet holds HCP-A with org_ura=1, HCP-B with org_ura=2, and Delegation-X with org_ura=2. The engine picks HCP-A first (bindings {org_ura: 1}); Delegation-X carries org_ura=2, conflict, no other delegation candidate, backtrack. Picks HCP-B (bindings {org_ura: 2}); Delegation-X is consistent. Success: (HCP-B, Delegation-X).

Policy 4: a descriptor with no consistent candidate is skipped only after exhausting alternatives elsewhere

"This descriptor allows zero matches" must not be a license to give up at the first dead end. If a different choice further up the tree would have let it match, the engine takes that choice.

At each descriptor, the engine tries every candidate consistent with current bindings. If none fit and the descriptor is optional (per SubmissionRequirements), it skips it (records VC=nil) and continues to the next descriptor. If none fit and it is required, this branch fails; the search backtracks to the previous decision and tries a different candidate there. Optional never short-circuits the search; it only means "if everything else fails, the descriptor going unmatched is acceptable".

Worked example. Required A: {A1, A2}. Optional B: {B1}. Required C: {C1}. A1's bindings let B1 match but conflict with C1; A2's bindings conflict with B1 but let C1 match. The engine tries A1, then B1, then C1 fails. Backtrack to A1; no other B candidate; B exhausted; skip B (B is optional); C still fails (bindings from A1 still in effect). Backtrack to A. Tries A2; B1 conflicts; B exhausted; skip B; C1 succeeds. Final assignment: A=A2, B=nil, C=C1.

Policy 5: a PD with more than one complete consistent assignment is ambiguous, not lucky

If the wallet can satisfy a PD two different ways, it must not silently pick one. That hides a real under-specification in the PD or in the caller's selection.

The engine does not stop at the first complete assignment. It continues the search until it finds a second consistent assignment, at which point it short-circuits and returns ErrMultipleCredentials. The error description names the descriptors that carry multiple choices, so the caller knows which keys to add to credential_selection. If only one assignment exists, that is the result. Skipping an optional descriptor is not counted as a distinct alternative to picking it: per Policy 4 the engine prefers candidates over skip, and the skip branch is only entered after all candidates are exhausted.

Worked example. Required A: {A1: foo=X, A2: foo=Y}. Required B: {B1: foo=X, B2: foo=Y}. Assignment 1: (A1, B1). Assignment 2: (A2, B2). Two distinct consistent assignments, so the engine returns ErrMultipleCredentials mentioning the foo binding ambiguity. Caller resends with credential_selection: {foo: "X"}; only (A1, B1) survives; success.

Policy 6: optional: true fields that don't resolve don't bind anything

An unresolved optional field must not manufacture a phantom binding entry with a nil value that other descriptors then have to match.

When the engine computes a candidate's resolved id-bearing values, fields whose paths produced no value (legal only under optional: true) are not added to the running bindings. Other descriptors that share the same id name are free to bind it from their own resolutions.

Worked example. Descriptor A has an optional: true field carrying id: "foo" whose path doesn't resolve on candidate A1. Descriptor B has a required field carrying id: "foo" resolving to Z on candidate B1. Bindings start empty. A1 selected, contributes nothing; bindings still empty. B1 selected; bindings become {foo: Z}. No conflict.

Policy 7: a PD with inconsistent same-id fields fails to load

With Policy 3 in force, an inconsistent PD (same id, incompatible filters) is a deterministic failure for every request that uses it. Catching it at boot, when the policy file is read, rather than at request time, spares the PD author hours of confusion.

When the policy backend loads a PresentationDefinition, the engine validates it. For every id used on two or more field objects across descriptors, the field filters must agree:

• All filters with a declared type use the same type.
• All filters with a const use the same const value.
• A field's id must be unique within its own constraints object (per the PEX specification).

If any of these checks fail, policy load fails with an error naming the field id and the specific conflict. The node refuses to apply the policy.

Filter checks are conservative: pattern intersection is undecidable in general and is not enforced; enum overlap is not enforced. Filters that only differ in path are allowed (it is normal for the same id to be resolved from different JSONPaths on different credential types).

Worked example. A PD declares id: "patient_bsn" on two descriptors. One field has filter: {"type": "string"}; the other has filter: {"type": "number"}. Policy load fails with field id "patient_bsn" has conflicting filter types: string vs number.

Implementation Decisions

• Engine surface (new, all in vcr/pe):
  • Select(pd, candidates, opts ...Option) (Result, error) is a pure function (functional options: WithInitialBindings, WithStrategy, WithSelectionTrace). It owns the full matching pipeline: step 1 matches each descriptor on its own (matchConstraint + matchFormat), step 2 searches for a consistent combination via binding DFS (Policies 3-6), and step 3 applies submission-requirements satisfaction by reusing the existing groups()/apply()/deduplicate() helpers (unchanged). Optionality (which descriptors may be left unfilled, for the P4 skip-vs-backtrack decision) is derived internally from pd.SubmissionRequirements; it is not a caller-supplied input. Output: the final chosen credential per descriptor (nil for skipped or dropped-by-rule) plus the captured id-bearing bindings, or an error. Note (revised during implementation): the engine is consolidated rather than layered, because once Select takes the whole PD a caller-derived optionality digest is redundant and splitting submission-requirements knowledge across two functions is worse. Floor-aware pick min>=1 binding cooperation stays out of scope (see "Out of Scope"); Select uses a coarse skip rule.
  • ValidateSelectionKeys(selection, pds ...PresentationDefinition) error returns an error naming every unknown key, evaluated against the union of field ids across the supplied PDs. Encapsulates Policy 1.
  • Validate(pd PresentationDefinition) error checks one PD for the conditions in Policy 7. Encapsulates Policy 7.
  • Result.Bindings exposes the captured id-bearing field values. Two-VP composition (Policy 2) happens in the caller layer: filter orgResult.Bindings to keys present as field ids on the SP PD, then invoke Select(spPD, ..., filteredBindings). No new method on the engine for this composition; it is straight Go at the call site.
• Policy load wiring:
  • The policy backend that reads PD files at startup calls pe.Validate(pd) for each loaded PD. A failure is fatal: the node refuses to apply that policy, surfacing the error to the operator at boot.
  • Existing policy load paths in policy/ (and any equivalent for discovery definitions) get the new validation call. The exact integration point depends on the loader; the engine validation function is the same.
• Existing API preserved as a thin wrapper:
  • matchConstraints, matchBasic, and matchSubmissionRequirements are deleted; their satisfaction logic is consolidated into Select (which reuses the unchanged groups()/apply()/deduplicate() helpers). Match becomes a thin wrapper: call Select, then build the []InputDescriptorMappingObject descriptor map (with dedup and path formatting) from Result.
  • NewFieldSelector becomes lenient: unknown selection keys are silently dropped, no construction error.
  • BuildSubmission (and the holder interface) gains an initialBindings map[string]string parameter, defaulting to nil for callers that don't need it.
• Two-VP caller (auth/client/iam/openid4vp.go) stops merging captured values into credential_selection. Instead it filters orgResult.Bindings to keys present as field ids on the SP PD and passes the result as initialBindings to the SP submission build. applyCapturedFieldsToSelection is removed or shrinks to a one-line filter helper.
• API handlers (auth/api/iam/api.go for request-service-access-token, and auth/api/iam/openid4vp.go for request-credential where applicable) call ValidateSelectionKeys against the union of in-scope PDs before any submission building. On failure: 400 OAuth invalid_request listing all unknown keys.
• Errors:
  • ErrNoCredentials (existing): no consistent assignment exists for some required descriptor. The error wraps the list of descriptors that ended unmatched.
  • ErrMultipleCredentials (existing): a second consistent assignment was found. The error description names the descriptors that carry multiple choices.
  • Invalid selection key from ValidateSelectionKeys surfaces as a 400 OAuth invalid_request. The error description lists all unknown keys.
  • PD validation failure from Validate is returned to the policy loader and surfaces in node startup logs and exit status.

Testing Decisions

A test exercises external behavior, not internal DFS shape: build a PD and a candidate VC list, call the engine, assert on the returned assignment and error.

• Primary site: black-box engine test in vcr/pe. A single new test file exercises every policy against hand-built PDs and VCs. No mocks, no on-disk fixtures. One sub-test per policy (1 through 7) using the worked example from the spec, plus edge cases. One sub-test for the two-VP composition (two Select calls chained through a bindings filter).
• Secondary site: handler wiring tests in auth/api/iam (validation surface) and auth/client/iam (two-VP chain). One happy-path test verifying the handler calls ValidateSelectionKeys and Select with the right inputs and passes the result through; one error-path test for the validation 400 shape.
• Policy load: a test in the policy backend confirms that pe.Validate is called on each loaded PD and that a validation failure causes the load to fail with a useful error.
• Existing tests that call Match (TestMatch, TestPresentationDefinition_MatchWithSelector_SubmissionRequirements, the submission-requirement and PresentationSubmission.Validate suites) continue to pass: Match is now a thin wrapper over Select, and the apply() rule logic is unchanged (still unit-tested in submission_requirement_test.go). The one selector test that asserts strict validation flips its expectation to assert silent drop. That Select reproduces matchBasic (all-required) and matchSubmissionRequirements (all/pick/min/max) is characterized directly in added baseline funcs from old repo #1.

Prior art for the test patterns: existing TestNewFieldSelector and TestPresentationDefinition_match* provide the closest templates.

Impact Assessment

• Security: no new attack surfaces. Up-front validation at the API handler preserves typo protection (now against the union of in-scope PDs). Cross-VP capture remains internal; loosening it removes a false positive, not a check. No authentication or authorization changes. No new sensitive-data handling.
• Backwards compatibility:
  • BuildSubmission (holder interface) gains a parameter. Internal to nuts-node; all in-tree callers are updated together; mocks regenerated.
  • matchConstraints, matchBasic, and matchSubmissionRequirements are deleted and their logic folded into Select. Internal to vcr/pe.
  • NewFieldSelector behavior changes: stops returning a construction error for unknown selection keys. Public function. Callers that relied on this for typo protection migrate to ValidateSelectionKeys.
  • PD evaluation behavior changes for any PD that uses the same id name across multiple descriptors. PDs that previously emitted inconsistent assignments will either produce a consistent one or fail.
  • PD load behavior changes: PDs with inconsistent same-id filters that previously loaded successfully now fail to load. Operators must audit deployed PDs before upgrading.
• Configuration and deployment: no new config options, no environment variables. Deployment must include a pre-upgrade audit of PDs for the load-time validation (see release-notes audit checklist).
• Versioning: minor bump. Release notes include a callout listing the NewFieldSelector leniency, the new same-id binding behavior, and the new load-time PD validation, plus a one-line PD-author audit checklist (search PDs for repeated id names; verify the intended shared-value semantics; confirm filters on shared ids agree on type and const).

Out of Scope

• Two selection refinements: (1) making submission-requirement pick rules with min >= 1 (choose N-of-M from a group) cooperate with the binding search; and (2) binding-aware tie-breakers — when several binding-consistent assignments exist, a strategy that picks a "best" one by a rule (e.g. prefer the newest credential) instead of taking the first (FirstMatch) or returning ErrMultipleCredentials (Strict). Both deferred.
• Cross-PD validation between org and SP PDs (e.g. warning when a captured org id has no matching field on the SP PD). Cross-VP filtering is silent by Policy 2; surfacing it as a developer-time warning is a follow-up.
• pattern-vs-pattern intersection checks in Validate (when there is no const/enum to test the patterns against). Undecidable in general; such a conflict surfaces at request time via the engine instead. (Updated during split: enum overlap and concrete-value-vs-pattern checks ARE in scope, via the value-set-intersection model.)
• Discovery module (discovery/module.go) behavior changes beyond (a) keeping the existing Match call site working with default options and (b) the new load-time pe.Validate on service-definition PDs (Policy 7).
• Bidirectional cross-VP binding: capture stays one-way (org to SP).
• An ergonomic API for callers to enumerate the descriptors that contributed to an ErrMultipleCredentials. The error description carries the names; structured access is a follow-up.
• The developer-facing surface for matching diagnostics (a dev endpoint or UI exposing the MatchReport). The engine produces the report; exposing it is a follow-up to Facilitate developers with their policy development #4218.

Open Questions

• Engine entry-point naming: Select, Match, Resolve, Pick. The spec calls it Select provisionally; reviewers may prefer another name.
• For ErrMultipleCredentials, should the description list the descriptor ids that contributed multiple choices, the id-bearing field that triggered the ambiguity, or both? The spec proposes "descriptor ids"; "both" gives more diagnostic value at the cost of a longer message.
• Should ValidateSelectionKeys deduplicate empty-value entries (where the caller supplied a key with an empty string)? Today the matcher would treat empty string as a literal selection value.
• For Policy 7, should the load-time validation also warn (without failing) on cases that are technically valid but suspicious (e.g. same id with different pattern filters)? The spec defers these to a follow-up; reviewers may prefer warn-but-load behavior.

Implementation Plan

#	Description	PR	Depends on
1	Core selection engine: Select (eligibility + binding DFS + submission-requirements satisfaction) + Result/Bindings + MatchReport diagnostics (Policies 3-6)	#4280	—
2	Engine validators: Validate (P7) + ValidateSelectionKeys (P1)	#4281	—
3	Rewire vcr/pe public API onto Select; delete matchConstraints/matchBasic/matchSubmissionRequirements/MatchWithSelector and the CredentialSelector abstraction	#4284	1
4	Thread initialBindings through BuildSubmission (holder interface + impls + mock)	#4286	3
5	Two-VP caller seeds the SP build via initialBindings (Policy 2); drop applyCapturedFieldsToSelection	#4287	4
6	API key validation: ValidateSelectionKeys -> 400 invalid_request (Policy 1)	#4288	2
7	Fatal load-time PD validation in policy/ + discovery/ (Policy 7)	#4290	2
8	Release notes + policy docs + two-VP same-id integration test	#4291	5, 6, 7

Strategy note (decided during split): the loose/strict ambiguity behavior is declared per-PD in the
policy/discovery config wrapper, defaults to loose (FirstMatch), and flips to strict in a future major.
Same-id agreement (Policy 3) is always on. Load-time validation (Policy 7) is fatal at boot.

[reinkrul]

Maybe good to separate the refactor issue from the changes/fix-issue, because there's just so much fluff in here that you risk hiding the most important part (the 2 problems to be solved).