docs: Stage-1 Trust & Security explanation set + Relay tutorial refresh#174
docs: Stage-1 Trust & Security explanation set + Relay tutorial refresh#174jeremi wants to merge 8 commits into
Conversation
Adds the trust-spine pilot explanation at explanation/records-stay-home.mdx — the quality exemplar for the documentation effort. It explains how an institution proves facts from registries it already holds without the records leaving: what stays inside the boundary, what crosses it, the three disclosure modes (value / predicate / redacted), and an honest statement of what the design does and does not guarantee. Resolves the shippable frontmatter TODO by setting owner to the registry-docs area (matching sibling explanation pages) and quotes last_reviewed for house-style consistency. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gfv6Eurtn4CzfnxLpNL2gP Signed-off-by: Claude <noreply@anthropic.com>
Make the Related links archive-safe (../../spec/... instead of root- relative /spec/...), unblocking the check:links:built CI gate that rejects archived pages linking outside their version. Tighten two Tier-C security claims to match RS-SEC-G and the page's own honesty thesis: - the published public key verifies a signed credential or signed result, not "a result" in general (default results are provenance- tagged, not signed); - describe the unauthenticated surface accurately (liveness/readiness probes, public verification keys, credential-issuance discovery metadata) while keeping the strong rule that anything touching a record or claim requires authentication. Declare the SD-JWT VC standard in standards_referenced. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gfv6Eurtn4CzfnxLpNL2gP Signed-off-by: Claude <noreply@anthropic.com>
… tutorial Add five reviewer/operator-facing explanation pages and wire them into the sidebar (a new "Trust & security" group, plus the disclosure page and the records-stay-home pilot under Explanation): - Trust posture and security guarantees — a plain-language overview with an at-a-glance posture table. - Disclosure modes and computed answers — the three modes and what each one reveals. - Data minimization and purpose limitation — the privacy / DPO view. - Threat model — boundaries, assets, threats, and residual risks. - Known limitations and non-guarantees — the canonical limits hub the other pages link to instead of restating. Refresh the Relay tutorial (publish-spreadsheet-secured-registry-api) with a clearer framing, the exact problem+json codes (auth.missing_credential / auth.scope_denied / auth.purpose_required), and a "what you built" recap, on top of its existing verified command output. The sh-command count is unchanged, so the tutorial drift check still holds. All pages follow the house style (second person, sentence-case headings, no inline spec citations — the specs are linked at the foot of each page), and every load-bearing security/privacy claim was re-verified against both the RS-* specs and the implementation. The content stages of `npm run check` pass (frontmatter, Vale, markdown, openapi lint, tutorial drift, build, llms, links, seo). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011U1jTj594nGXhR2r9BzWqm Signed-off-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 578cf68116
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| field the holder does not present stays hidden. It is **holder-bound** — tied to the | ||
| holder's key, and not presentable without the matching private key — and the holder chooses |
There was a problem hiding this comment.
Qualify holder binding for optional profiles
This overstates the credential guarantee for direct Notary issuance profiles: HolderBindingConfig::default() sets mode to "none" in crates/registry-notary-core/src/config.rs:6681, and validate_holder_request returns without requiring holder proof when the profile mode is none in crates/registry-notary-server/src/api.rs:7693. As written, users of the default/unbound profile will believe every SD-JWT VC is tied to a holder key and cannot be replayed by another presenter, which the service does not enforce unless that profile explicitly enables holder binding.
Useful? React with 👍 / 👎.
| - **Every person-level request is audited.** An audit record captures at least the caller, | ||
| the scopes exercised, a request id, and the declared purpose where one was supplied. A |
There was a problem hiding this comment.
Do not promise scopes in Notary audit records
This guarantee is too broad for flows that go through Notary: the Notary EvidenceAuditEvent model has principal, request, purpose, and related fields but no scopes field (crates/registry-notary-core/src/model.rs:1591), and the new limitations page separately says Notary audit omits exercised scopes. Leaving this unqualified tells auditors every person-level audit record can reconstruct the authorizing scopes, but claim and credential flows cannot provide that data today.
Useful? React with 👍 / 👎.
| and algorithm). The only routes open to an anonymous caller are health probes and the public | ||
| keys a verifier needs to check a signature. |
There was a problem hiding this comment.
Include all anonymous protocol endpoints
This narrows the public surface too far for Notary deployments with OID4VCI enabled: is_auth_exempt_path exempts not only probes and JWKS but also /.well-known/openid-credential-issuer, /oid4vci/credential-offer, /oid4vci/token, /oid4vci/nonce, /docs, type metadata under /.well-known/vct/, and credential-status reads. A reviewer using this page as the anonymous-route inventory would miss protocol endpoints that are intentionally reachable without the API-key/OIDC middleware.
Useful? React with 👍 / 👎.
| - **One credential format, one binding method when binding is enabled.** Issued Registry Notary credentials are [SD-JWT VC](../../reference/standards/) only. Holder binding is a per-credential-profile setting that defaults to off (`mode: none`), in which case the issued credential is unbound; when a profile turns it on (`mode: did`), `did:jwk` is the single supported proof-of-possession method, and no other format or binding method is issued. The self-attestation (wallet) issuance path requires a profile with binding enabled, so a credential issued down that path is always `did:jwk`-bound with proof-of-possession. | ||
| - **A profiled issuance subset, not a full issuer.** The [OID4VCI](../../reference/standards/) surface is a scoped self-attestation issuance profile — a profiled subset of Draft 13 using the `dc+sd-jwt` format — not a full OID4VCI issuer and not a claim of general external-wallet interoperability. Discovery declares this explicitly as `openid4vci.support: not_full_issuer` — that is, it announces it is not a full issuer. | ||
| - **No delegated-attestation issuance.** Delegated-attestation transaction tokens are rejected by the credential endpoint; this profile issues only for direct self-attestation principals in this version. | ||
| - **No revocation, no issuer-metadata discovery endpoint, no erasure workflow.** At this version there is no credential revocation service, no `/.well-known/jwt-vc-issuer` endpoint, and no built-in data-subject erasure workflow. A rotated-out signing key may remain published for verification, which is not revocation. These are documented pilot limitations, recorded in `SECURITY.md`. |
There was a problem hiding this comment.
Remove the obsolete no-revocation claim
This canonical limitation is no longer true when credential status is enabled: Notary exposes a public GET /v1/credentials/{credential_id}/status and an admin POST /admin/v1/credentials/{credential_id}/status, and the OpenAPI/schema accept revoked as a lifecycle status (crates/registry-notary-server/src/openapi.rs:820 and crates/registry-notary-server/src/openapi.rs:2824). Keeping an absolute "no credential revocation service" statement will make operators ignore the supported status/revocation surface or conclude the API reference is wrong.
Useful? React with 👍 / 👎.
| Registry Notary issues credentials, but the issuance surface is deliberately narrow. Read the full context in the [Registry Notary protocol](../../spec/rs-pr-notary/). | ||
|
|
||
| - **One credential format, one binding method when binding is enabled.** Issued Registry Notary credentials are [SD-JWT VC](../../reference/standards/) only. Holder binding is a per-credential-profile setting that defaults to off (`mode: none`), in which case the issued credential is unbound; when a profile turns it on (`mode: did`), `did:jwk` is the single supported proof-of-possession method, and no other format or binding method is issued. The self-attestation (wallet) issuance path requires a profile with binding enabled, so a credential issued down that path is always `did:jwk`-bound with proof-of-possession. | ||
| - **A profiled issuance subset, not a full issuer.** The [OID4VCI](../../reference/standards/) surface is a scoped self-attestation issuance profile — a profiled subset of Draft 13 using the `dc+sd-jwt` format — not a full OID4VCI issuer and not a claim of general external-wallet interoperability. Discovery declares this explicitly as `openid4vci.support: not_full_issuer` — that is, it announces it is not a full issuer. |
There was a problem hiding this comment.
Stop advertising a missing discovery flag
The OID4VCI metadata does not currently declare openid4vci.support: not_full_issuer: CredentialIssuerMetadata serializes only issuer, endpoints, display, authorization servers, and credential_configurations_supported, and the Notary metadata builder does not add that extension. Clients or reviewers following this text will look for a machine-readable support flag that is absent from the actual issuer metadata.
Useful? React with 👍 / 👎.
| Do not infer their presence from the route catalog of a different build. Admin routes sit on | ||
| a separate optional listener and are not in the public OpenAPI document. |
There was a problem hiding this comment.
Do not say admin routes are absent from OpenAPI
The generated Notary OpenAPI document does include admin routes, including /admin/v1/reload, /admin/v1/capabilities, config apply/verify/dry-run, posture, and /admin/v1/credentials/{credential_id}/status (crates/registry-notary-server/src/openapi.rs:69 and products/notary/openapi/registry-notary.openapi.json:2131). This sentence would mislead operators and API consumers into thinking the published route catalog cannot be used to review admin surfaces.
Useful? React with 👍 / 👎.
|
|
||
| The first place minimization appears is in the unit of data the system is built around. A claim definition describes one decision or one extracted value — not a whole record — so returning a full record would over-collect relative to the question actually asked. The design treats the narrow question, not the record, as the thing to be answered. | ||
|
|
||
| That principle is enforced one layer down, at the binding between a claim and its source. A source binding reads only the fields its rule needs, and a request that supplies an input path outside a declared allow-list is rejected, so a binding cannot over-collect by accident. The allow-list converts "read only what you need" from an aspiration into a gate that refuses out-of-scope inputs before they reach the source. |
There was a problem hiding this comment.
Qualify field minimization as configuration-driven
This states that Notary derives the source projection from the rule, but the runtime projects every configured binding.fields entry plus lookup/freshness fields (projected_source_fields_with_lookup and projected_source_fields_with_query_values in crates/registry-notary-server/src/standalone.rs:7185). If a binding includes extra fields, the software will still request them, so privacy reviewers should be told this minimization depends on correct binding configuration rather than being automatically enforced from rule dependencies.
Useful? React with 👍 / 👎.
Apply seven code-grounded accuracy corrections from review of this PR. Each was verified against the notary crates and the generated OpenAPI before editing: - Trust posture: broaden the anonymous-route list — it also includes credential-issuer discovery, the OID4VCI wallet-flow endpoints, the docs, public credential-status reads, and credential type metadata, not only probes and public keys. - Known limitations: qualify the absolute "no credential revocation service" — the RS-* specs define no revocation flow, but an optional, off-by-default credential-status surface can mark a credential revoked; and name the not_full_issuer flag to the /.well-known/evidence-service capability-discovery document rather than the OID4VCI credential-issuer metadata. - Threat model: admin routes ARE documented in the generated OpenAPI (the prior "not in the public OpenAPI document" claim was wrong); standalone mode still returns HTTP 501 for runtime reload. - Data minimization: field minimization is driven by the configured binding.fields (plus lookup/freshness fields), not auto-derived from the rule. - records-stay-home (pilot): the audit-record claim no longer implies Notary records exercised scopes (Relay does; Notary's record does not), and holder binding is qualified as per-profile (default unbound; the wallet path binds). check:style, check:markdown, build, and check:links all pass. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011U1jTj594nGXhR2r9BzWqm Signed-off-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98867c9897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| responding. The only unauthenticated surfaces are probes and public JWKS / `did:web` | ||
| discovery. |
There was a problem hiding this comment.
Include all auth-exempt Notary protocol endpoints
This narrows the threat model's anonymous/API-key-exempt surface too far. I checked Notary standalone's is_auth_exempt_path and it also exempts OID4VCI issuer metadata, credential-offer/token/nonce flows, docs, credential-status reads, and type metadata (plus federation's JWS-verified path), not just probes and JWKS/did:web; auditors using this section as the route inventory would miss protocol endpoints intentionally reachable without the normal API-key/OIDC middleware.
Useful? React with 👍 / 👎.
|
|
||
| - **No data-subject erasure or right-to-be-forgotten workflow.** There is no built-in erasure or deletion flow anywhere in the design, so it does not satisfy erasure obligations on its own. As noted above, the read-only design cannot mutate source records, so erasure, where it is required, remains an operation on the source registry outside this system. | ||
| - **No rectification or data-subject-rights flow.** Beyond erasure, there is no rectification or general data-subject-rights mechanism. | ||
| - **No credential revocation service.** There is no revocation flow. Key rotation exists — a rotated-out key may remain published so existing results stay verifiable — but that is not a way to revoke an already-issued credential. Don't count on revocation as a supported capability. |
There was a problem hiding this comment.
Qualify revocation as optional instead of absent
This absolute limit is now misleading for deployments that enable credential status: Notary exposes public GET /v1/credentials/{credential_id}/status and admin POST /admin/v1/credentials/{credential_id}/status, and the update path accepts revoked as a lifecycle status. Saying there is no revocation service/flow and that operators should not count on revocation will make them overlook the supported optional status surface; this should be framed as optional/off-by-default or not spec-level, rather than absent.
Useful? React with 👍 / 👎.
|
|
||
| ## How Registry Notary controls what leaves the service | ||
|
|
||
| Registry Notary is the component that evaluates a claim and decides what the caller receives back. It controls the answer through three **disclosure modes**: `value`, `predicate`, and `redacted`. They are the only modes — there is no fourth mode and no gradation between them. |
There was a problem hiding this comment.
Qualify value mode for field-level redaction
This says there is no gradation between the three modes, but object-valued claims can still be partially redacted while staying in value mode: view_claim handles DisclosureProfile::Value with supported matching.redaction_fields by removing those top-level fields and returning the remaining object plus redacted_fields. For privacy reviews of claims that configure object field redaction, this page will incorrectly suggest the only alternatives are full value, boolean, or fully redacted output.
Useful? React with 👍 / 👎.
| The mode is policy-bound, not caller's choice: a claim defines an `allowed` set and a | ||
| `default`, the service refuses a mode outside the allowed set, and every result records |
There was a problem hiding this comment.
Acknowledge allowed caller disclosure choices
This overstates who selects the disclosure mode for claims whose policy allows more than one mode. The runtime uses the caller's request.disclosure when present and only rejects or downgrades it if it is outside the claim's allowed set; when no disclosure is requested it falls back to the claim default. For profiles that allow both value and predicate, reviewers would incorrectly conclude the caller cannot choose the more revealing allowed mode.
Useful? React with 👍 / 👎.
Apply nine code-verified corrections so four claim-classes read consistently and accurately across the trust/privacy pages (and the records-stay-home pilot): - Revocation: requalify the absolute "no revocation service" as no spec-defined revocation flow plus an optional, off-by-default credential-status surface (public GET + admin POST status endpoints) that can mark a credential `revoked`. (data-minimization, threat-model, records-stay-home; matches the earlier known-limitations fix.) - Anonymous surface: broaden "only probes + keys" to the full auth-exempt set (probes, public verification keys, credential-issuer discovery, OID4VCI wallet-flow endpoints, docs, public credential-status reads, type metadata), and note the federation route is exempt from the API-key/OIDC middleware but still requires a verified peer-signed JWS, so it is not anonymous. (threat-model, records-stay-home.) - value-mode gradation: keep "exactly three modes, no fourth", but note that within `value` mode an object-valued claim can have configured fields redacted (`redaction_fields`), so `value` is not always all-or-nothing. (disclosure-modes.) - Caller choice: the policy bounds which modes are allowed; within the allowed set the caller can request one (refused with claim.disclosure_not_allowed otherwise), and the default applies when none is requested — corrects the "never the caller's choice" overstatement. (disclosure-modes, trust-posture, records-stay-home.) check:style, check:markdown, build, and check:links all pass. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011U1jTj594nGXhR2r9BzWqm Signed-off-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f6e72b244
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **One credential format, one binding method when binding is enabled.** Issued Registry Notary credentials are [SD-JWT VC](../../reference/standards/) only. Holder binding is a per-credential-profile setting that defaults to off (`mode: none`), in which case the issued credential is unbound; when a profile turns it on (`mode: did`), `did:jwk` is the single supported proof-of-possession method, and no other format or binding method is issued. The self-attestation (wallet) issuance path requires a profile with binding enabled, so a credential issued down that path is always `did:jwk`-bound with proof-of-possession. | ||
| - **A profiled issuance subset, not a full issuer.** The [OID4VCI](../../reference/standards/) surface is a scoped self-attestation issuance profile — a profiled subset of Draft 13 using the `dc+sd-jwt` format — not a full OID4VCI issuer and not a claim of general external-wallet interoperability. The capability-discovery document (`/.well-known/evidence-service`) declares `openid4vci.support: not_full_issuer` — announcing it is not a full issuer; this flag lives there rather than in the OID4VCI credential-issuer metadata. | ||
| - **No delegated-attestation issuance.** Delegated-attestation transaction tokens are rejected by the credential endpoint; this profile issues only for direct self-attestation principals in this version. | ||
| - **No revocation flow, no issuer-metadata discovery endpoint, no erasure workflow.** The RS-* specs define no revocation flow. The credential-status surface is optional and off by default; when an operator enables it, a public status endpoint and an admin status endpoint can mark a credential `revoked`. There is no `/.well-known/jwt-vc-issuer` endpoint and no built-in data-subject erasure workflow. A rotated-out signing key may remain published for verification, which is not revocation. These are documented pilot limitations, recorded in `SECURITY.md`. |
There was a problem hiding this comment.
Distinguish status reads from revocation updates
For credential-status-enabled deployments, only the admin update route changes lifecycle state: get_credential_status just reads the record (crates/registry-notary-server/src/api.rs:3149-3170), while update_credential_status requires registry:notary:admin before calling update_status (crates/registry-notary-server/src/api.rs:3400-3441). As written, this says the public status endpoint can mark credentials revoked, overstating what the anonymous surface can do and making the security boundary look broader than it is.
Useful? React with 👍 / 👎.
| |------|--------------------------| | ||
| | `value` | the full evaluated value | | ||
| | `predicate` | only the true/false satisfaction of the rule | | ||
| | `redacted` | no value and no satisfaction outcome | |
There was a problem hiding this comment.
Qualify redacted disclosure for CCCEV renders
This is too broad for redacted evaluations rendered as CCCEV JSON-LD: view_claim sets satisfied to None for redacted results, but render_cccev_evidence_node serializes that as cccev:isConformantTo: false (crates/registry-notary-server/src/runtime.rs:5545-5548 and :5668-5674). In deployments that allow both redacted disclosure and the CCCEV format, callers still receive a yes/no-looking conformance value, so the no-outcome guarantee needs a caveat or the renderer needs to omit that field.
Useful? React with 👍 / 👎.
|
|
||
| - **One credential format, one binding method when binding is enabled.** Issued Registry Notary credentials are [SD-JWT VC](../../reference/standards/) only. Holder binding is a per-credential-profile setting that defaults to off (`mode: none`), in which case the issued credential is unbound; when a profile turns it on (`mode: did`), `did:jwk` is the single supported proof-of-possession method, and no other format or binding method is issued. The self-attestation (wallet) issuance path requires a profile with binding enabled, so a credential issued down that path is always `did:jwk`-bound with proof-of-possession. | ||
| - **A profiled issuance subset, not a full issuer.** The [OID4VCI](../../reference/standards/) surface is a scoped self-attestation issuance profile — a profiled subset of Draft 13 using the `dc+sd-jwt` format — not a full OID4VCI issuer and not a claim of general external-wallet interoperability. The capability-discovery document (`/.well-known/evidence-service`) declares `openid4vci.support: not_full_issuer` — announcing it is not a full issuer; this flag lives there rather than in the OID4VCI credential-issuer metadata. | ||
| - **No delegated-attestation issuance.** Delegated-attestation transaction tokens are rejected by the credential endpoint; this profile issues only for direct self-attestation principals in this version. |
There was a problem hiding this comment.
Narrow delegated-attestation issuance limit
This is only true for the OID4VCI transaction-token path, not for credential issuance overall. When a stored evaluation has access_mode == DelegatedAttestation, /v1/credentials explicitly applies require_delegated_attestation_credential_profile_policy and allows issuance if the relationship allow-lists the profile (crates/registry-notary-server/src/api.rs:6586-6594 and :8884-8921), with config validation supporting delegated claims that reference allowed credential profiles. Leaving the absolute "No delegated-attestation issuance" heading will make operators miss the supported direct materialization path.
Useful? React with 👍 / 👎.
|
|
||
| The redacted mode is worth a reviewer's attention because it shows minimization without loss of accountability: a redacted result carries neither the underlying value nor the satisfaction outcome, yet the evaluation stays referenceable and verifiable through an evaluation identifier, a verification identifier, and a claim hash. You can audit that an evaluation happened and verify it later without the result itself disclosing anything about the person. | ||
|
|
||
| When the system issues a credential rather than returning a direct answer, minimization continues at presentation time. Issued credentials are SD-JWT VC — a verifiable-credential format in which the signed body carries only a SHA-256 digest of each selectively disclosable field, so a field the holder does not present stays hidden, and the holder controls what is revealed. Holder binding — tying the credential to a holder key with `did:jwk` — is a per-profile setting that defaults to off, though the self-attestation (wallet) issuance path requires it; the direct claim or evaluation result is not a credential and is never holder-bound. One caveat for a reviewer: the issuance surface is a profiled, partial subset, not a full credential-issuer implementation, and should not be read as general wallet interoperability or full standards conformance. |
There was a problem hiding this comment.
Qualify SD-JWT disclosure granularity
This overstates the holder's presentation-time control for credentials issued from ordinary stored evaluations. Unless an explicit projection is used, disclosures_for_results creates one SD-JWT disclosure per claim and puts the whole result value inside that disclosure (crates/registry-notary-core/src/sd_jwt.rs:211-224), so an object-valued claim is revealed as a unit rather than field-by-field. Privacy reviewers need this framed as claim/projection-level selective disclosure, not a guarantee that every field can be independently withheld.
Useful? React with 👍 / 👎.
…-granularity claims Four code-verified refinements from continued review: - known limitations (revocation): only the admin status endpoint (requires the registry:notary:admin scope) marks a credential revoked; the public status endpoint reads status. Corrected the wording that implied the public endpoint could revoke. - known limitations (delegated attestation): reframed the absolute "no delegated-attestation issuance" — the OID4VCI transaction-token path is rejected, but direct issuance via /v1/credentials is supported when the stored evaluation is delegated-attestation and the relationship allow-lists the profile. - disclosure modes (redacted + CCCEV): note a current renderer limitation — the CCCEV JSON-LD renderer emits cccev:isConformantTo: false for a redacted result instead of omitting it, so the no-outcome guarantee is not fully upheld in that render. (Tracked as a product issue.) - data minimization (SD-JWT granularity): selective disclosure is at the claim (or configured-projection) level — each claim is one disclosure carrying its whole value, so an object-valued claim is revealed as a unit unless an explicit projection splits it. check:style, check:markdown, build, and check:links all pass. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011U1jTj594nGXhR2r9BzWqm Signed-off-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd06aa9bca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| The minimization and purpose-limitation posture described above is the design default, but the operator owns the configuration. Several of the protections are conditional, and a reviewer should test the deployment, not the design, for each one. | ||
|
|
||
| The clearest example is the fallback when no matching policy is configured. With no matching policy in place, a binding falls back to unrestricted, identifier-only resolution with no purpose gating, no relationship gating, and no input minimization. Equivalently, purpose limitation is supported but partial: a purpose is recorded in audit only where the caller supplies one, and is enforced as a hard gate only where a claim or source binding configures a matching policy. The enforced gate described earlier exists only when someone has configured it. |
There was a problem hiding this comment.
Preserve claim-level purpose gates
In deployments where a source-bound claim sets claim.purpose or the global evidence.allowed_purposes but leaves the binding's matching-purpose list empty, this says there is no purpose gate. The runtime still passes claim_purpose_constraints into matching_pdp_decision for every source binding before read_one_for_context_with_capability is called, and target_matching_contract_test.rs:2473 asserts a claim-purpose mismatch returns pdp.purpose_not_permitted with zero source reads. This wording will make privacy reviewers discount an enforced claim-level purpose control.
Useful? React with 👍 / 👎.
|
|
||
| Federation across institutions is static-peer only. Read the full context in the [Registry Notary protocol](../../spec/rs-pr-notary/) and the [security model](../../spec/rs-sec-g/). | ||
|
|
||
| - **Static peers only.** There is no dynamic trust-chain discovery, no shared replay storage, and no federated credential issuance. All three are out of scope for this version. |
There was a problem hiding this comment.
Qualify federation replay storage support
For multi-instance federation deployments, this absolute “no shared replay storage” limit is no longer accurate: Notary can build a Redis-backed replay store from top-level replay.storage = redis, config validation even allows federation.replay.storage = redis when the top-level store is Redis, and the federation runtime uses that shared ReplayStore for require_replay_insert on federation request jti values. This should be framed as the default in-process/in-memory replay limitation rather than saying shared replay storage is out of scope.
Useful? React with 👍 / 👎.
| - **Feature-gated surfaces.** Several Relay surfaces — the OGC API Features, Records, and EDR adapters, the SP DCI sync adapter, and signed response credentials — are feature-gated and present only in builds configured with the matching Cargo feature. Do not infer their presence in one build from another build's route catalog. | ||
| - **Aggregates are not privacy-budgeted.** As noted stack-wide, Relay aggregate routes provide no longitudinal privacy-budget or cumulative-differencing protection. | ||
| - **Relay does not evaluate or own the manifest.** Relay does not perform claim evaluation or issue credentials — that is Notary's role — and it does not own or version the metadata manifest format, which belongs to Registry Manifest. Relay only serves and scopes compiled artifacts. |
There was a problem hiding this comment.
Do not rule out Relay credential issuance
In Relay deployments with provenance.enabled and a caller requesting Accept: application/vc+jwt, the protected handlers call ProvenanceState::issue and return a compact VC-JWT signed response credential; the generated OpenAPI also mounts the provenance paths when provenance_enabled(config) is true, not behind a Cargo feature. This wording will make operators miss Relay's optional signed-response credential issuer or look for a nonexistent feature gate, even though the page itself distinguishes Relay VC-JWTs from Notary SD-JWT VCs above.
Useful? React with 👍 / 👎.
| - **No revocation flow, no issuer-metadata discovery endpoint, no erasure workflow.** The RS-* specs define no revocation flow. The credential-status surface is optional and off by default; when an operator enables it, the public status endpoint reads a credential's status, and only the admin status endpoint — which requires the `registry:notary:admin` scope — can mark a credential `revoked`. There is no `/.well-known/jwt-vc-issuer` endpoint and no built-in data-subject erasure workflow. A rotated-out signing key may remain published for verification, which is not revocation. These are documented pilot limitations, recorded in `SECURITY.md`. | ||
| - **CCCEV output is a profiled shape, not conformant.** [CCCEV](../../reference/standards/)-shaped output is a profiled subset and is not conformant to CCCEV 2.00. It is consumed by parsing the `@graph` for `cccev:Evidence` nodes. | ||
| - **Two signed artifacts, not one.** Notary issues SD-JWT VC credentials (`dc+sd-jwt`, signed with EdDSA/Ed25519 by default — ES256/P-256 is also supported per credential profile — with no W3C [Verifiable Credentials Data Model](../../reference/standards/) JSON-LD envelope). Relay's optional signed response credentials are VCDM 2.0 VC-JWT. These are different artifacts — don't describe one as the other. | ||
| - **Admin reload is not implemented.** The standalone Notary admin reload route returns HTTP 501 with code `registry.admin.capability.not_supported` and performs no reload; key and configuration changes require a service restart. |
There was a problem hiding this comment.
Qualify restart requirement for governed applies
For deployments using governed config apply or file-watch signing keys, not every key/config change requires a restart: accepted SigningRotation applies publish a new issuer runtime and record restart_required: false, client auth and OpenAPI auth-policy changes are also swapped into the auth state, and the file-watch signer reloads same-public-key replacements without restart. This absolute restart statement will make operators miss the supported hot-apply paths; narrow it to /admin/v1/reload and unsupported signed changes.
Useful? React with 👍 / 👎.
| - **Some claim-definition invariants surface only at runtime.** Duplicate claim ids, a | ||
| disclosure default outside the allowed set, a rule source naming an undeclared binding, and | ||
| the unimplemented plugin rule are not enforced at configuration load — they surface only at | ||
| evaluation or request time. A source binding with no matching policy resolves on identifiers |
There was a problem hiding this comment.
Flag duplicate claim IDs as silent shadowing
For configs with duplicate claim ids, this says the invariant surfaces as an evaluation/request error, but the runtime lookup path does not error: find_claim returns the first .iter().find(|claim| claim.id == claim_id), and find_claim_version similarly returns the first matching version. That means a later duplicate can be silently shadowed rather than rejected, so operators should not be told runtime will catch it.
Useful? React with 👍 / 👎.
…implementation Six code-verified refinements where an absolute "limit" is actually conditional: - data minimization: claim-level purpose constraints (the claim's permitted purposes and the deployment allowed-purposes list) are enforced before a source read even with no binding matching policy; only the binding-level gating is missing in that case. - known limitations (federation): replay protection defaults to in-memory, but a Redis-backed shared replay store is available for multi-instance deployments — "no shared replay storage" was too absolute. - known limitations (Relay): signed response credentials are config-gated via provenance, not Cargo-feature-gated; and with provenance enabled Relay can attach its own VC-JWT signed response credential (it still does not issue Notary's SD-JWT VC credentials). - known limitations (reload): standalone /admin/v1/reload returns 501 and non-swappable changes need a restart, but governed config apply and the file-watch signer hot-apply some changes (signing-key rotation, auth-policy swaps) without one. - threat model + known limitations (duplicate ids): duplicate claim ids are not rejected at load and are never reported — the runtime lookup returns the first match, so a later duplicate is silently shadowed (corrected the earlier "surface at runtime" wording on both pages). check:style, check:markdown, build, and check:links all pass. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011U1jTj594nGXhR2r9BzWqm Signed-off-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e51ab0310
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **Source data is read in place.** Relay reads sources as batch snapshots or table scans; | ||
| there is no write-back to the source registry, and runtime services expose no | ||
| data-mutation routes. The source keeps running as it always has. |
There was a problem hiding this comment.
Caveat Relay snapshot caches before promising read-in-place
In snapshot deployments, Relay does not only read the source in place: when materialization is unset, ResourceConfig::effective_materialization defaults to Snapshot, and prepare_snapshot_pipeline fully materializes batches and writes the projected data into the Parquet cache under cache_dir. For file or Postgres sources this still avoids source mutation, but it creates an internal cached copy that has retention and security implications, so this should distinguish "no write-back/no external handoff" from "no copy exists."
Useful? React with 👍 / 👎.
| - **Dynamic federation.** Dynamic trust-chain discovery, shared replay storage, audit | ||
| checkpoint exchange, and federated credential issuance are out of scope. Federation is |
There was a problem hiding this comment.
Do not list shared replay storage as out of scope
For multi-instance Notary deployments, shared replay storage is supported: config validation allows federation.replay.storage = redis when top-level replay.storage is Redis, ReplayStores::from_config builds a Redis-backed store, and federation inserts request jti values through require_replay_insert against that store. Fresh evidence: although the limitations page now documents the Redis option, this threat-model bullet still tells auditors that shared replay storage is out of scope, so it should be limited to the truly unsupported dynamic federation pieces.
Useful? React with 👍 / 👎.
| - **Feature-gated surfaces of other builds.** Several runtime surfaces — OGC API | ||
| Features/Records/EDR adapters, SP DCI sync, and signed response credentials — are | ||
| feature-gated and mount only when the build is configured with the matching Cargo feature. |
There was a problem hiding this comment.
Do not call Relay signed credentials Cargo-gated
Relay signed response credentials are config-gated, not Cargo-feature-gated: handlers use the provenance issuance helper when provenance.enabled is present and the caller negotiates Accept: application/vc+jwt, and the OpenAPI adds provenance paths based on provenance_enabled(config). Fresh evidence: the known-limitations page was corrected to say this, but this threat-model bullet still groups signed response credentials with Cargo-gated adapters, which can make operators miss a live VC-JWT issuance surface in a standard build.
Useful? React with 👍 / 👎.
| - **Admin reload is non-functional standalone.** Notary's admin reload route returns HTTP 501 | ||
| (`registry.admin.capability.not_supported`) in the standalone router and performs no | ||
| reload; key and configuration changes require a service restart. Do not treat hot reload as | ||
| a capability. |
There was a problem hiding this comment.
Preserve governed hot-apply support
In governed signed-config deployments, some key/config changes are applied without restart: accepted signing rotations and client/OpenAPI auth-policy changes publish a new runtime/auth state and record restart_required: false, and file-watch signers reload same-public-key replacements. Fresh evidence: the limitations page now qualifies standalone /admin/v1/reload, but this threat-model bullet still says key and configuration changes require restart and hot reload is not a capability, which overstates the operational limitation for supported hot-apply paths.
Useful? React with 👍 / 👎.
|
|
||
| ## Why predicate and redacted avoid sharing the underlying record | ||
|
|
||
| This is the core of the privacy story. Because Notary computes the answer from its own sources, a `predicate` result can satisfy a question while disclosing only a boolean. A question of whether someone has a registered record is modelled as an `exists` rule disclosed as `predicate`: the caller learns `true` or `false`, and the source row never leaves the service. A question about an eligibility threshold without exposing the figure behind it is modelled as a `cel` rule whose eligibility boolean is disclosed as `predicate`; the underlying value stays inside the service. |
There was a problem hiding this comment.
Do not promise false results for missing existence matches
For exists claims whose source binding does not resolve exactly one record, Notary fails the evaluation instead of returning a predicate false: evaluate_claim only returns Value::Bool(sources.contains_key(source)) after load_sources succeeds, and public tests for missing FHIR matches assert evidence.not_available. Saying the caller learns true or false will make integrators model absence as a negative claim result, when the current API returns an error/no evidence for those missing-match cases.
Useful? React with 👍 / 👎.
…el with limitations Two new code-verified corrections plus three threat-model alignments: - records-stay-home (read-in-place): in snapshot mode (the default) Relay materializes a projected copy into its local cache (cache_dir). There is still no write-back to the source and no external handoff, but a cached copy exists, with its own retention/access considerations — distinguished from "no copy". - disclosure modes (exists/predicate): an `exists` claim returns `true` when a record matches, but a subject with no matching record returns `evidence.not_available` (collapsed to a single public reason by default), not a bare `false` — absence is no-evidence, not a negative result. - threat model: align with the limitations page on three points the threat model still stated absolutely — shared replay storage is supported (Redis), Relay signed response credentials are config-gated via provenance (not Cargo-gated), and some config/key changes hot-apply without a restart. check:style, check:markdown, build, and check:links all pass. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011U1jTj594nGXhR2r9BzWqm Signed-off-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ecd22f2a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (`auth.scope_denied`). Relay read the workbook in place and never wrote back to it — the data | ||
| stayed where it started, and only an authorized, scoped request got an answer out. |
There was a problem hiding this comment.
Do not imply the sample leaves no cached copy
When the tutorial runs the generated benefits project, the Relay config does not set materialization, so ResourceConfig::effective_materialization defaults to Snapshot, and the snapshot ingest path writes projected batches into the Relay cache before serving them. In that default tutorial context the workbook is not written back to, but a projected local copy exists under cache_dir, so saying the data stayed only where it started gives users the wrong retention/security model.
Useful? React with 👍 / 👎.
| - **No data-subject erasure or right-to-be-forgotten workflow.** There is no built-in erasure or deletion flow anywhere in the design, so it does not satisfy erasure obligations on its own. As noted above, the read-only design cannot mutate source records, so erasure, where it is required, remains an operation on the source registry outside this system. | ||
| - **No rectification or data-subject-rights flow.** Beyond erasure, there is no rectification or general data-subject-rights mechanism. | ||
| - **No specified revocation flow; an optional status surface.** The specifications define no revocation flow, but the implementation includes an optional, off-by-default credential-status surface — a public `GET /v1/credentials/{id}/status` and an admin `POST /admin/v1/credentials/{id}/status` — that an operator can enable to mark a credential `revoked`. Key rotation exists — a rotated-out key may remain published so existing results stay verifiable — but that is not a way to revoke an already-issued credential. Treat status-based revocation as an operator-enabled capability, not an always-on one. | ||
| - **No broad cross-authority exchange beyond static peering.** Federation between authorities is static-peer only; dynamic trust-chain discovery, shared replay storage, and federated credential issuance are out of scope, so the design supports a narrower cross-authority data-exchange surface than the word "federation" might suggest. |
There was a problem hiding this comment.
Do not exclude Redis replay storage
For multi-instance Notary deployments, shared replay storage is supported: the config accepts replay.storage = redis and federation replay can use that same Redis-backed store for jti checks. Calling shared replay storage out of scope here will make privacy reviewers miss a supported hardening path; narrow this limitation to the default in-memory mode or to the truly unsupported dynamic federation pieces.
Useful? React with 👍 / 👎.
|
|
||
| This is the mechanism behind "prove a fact without sharing the record". To check whether a | ||
| person has a registered record, model the question as an *existence* rule and disclose it | ||
| as a `predicate`: the caller learns `true` or `false`, and the row never crosses the |
There was a problem hiding this comment.
Do not promise false existence results
This page still says an exists predicate tells the caller true or false, but when the source binding does not resolve exactly one record, Notary collapses the missing-match case to evidence.not_available by default rather than returning a predicate false. Integrators modelling absence as a negative result from this text will handle missing evidence incorrectly.
Useful? React with 👍 / 👎.
| freshness checks are backed by a real replay-store primitive that tracks one-time JWT `jti` | ||
| and nonce values (federation request `jti`, OID4VCI `c_nonce`, and holder-proof `jti`). |
There was a problem hiding this comment.
Qualify OID4VCI nonce replay tracking
For OID4VCI deployments that leave oid4vci.nonce.enabled at its default false, the credential path does not require or consume the proof nonce, and issue_c_nonce returns an unreserved nonce, so c_nonce values are not replay-store tracked in that configuration. This should be framed as applying only when the OID4VCI nonce endpoint is enabled; otherwise auditors will assume nonce single-use protection exists for wallet issuance when it is actually disabled.
Useful? React with 👍 / 👎.
| anonymous read was refused with `401` (`auth.missing_credential`), an authorized read returned | ||
| only the rows you asked for, and a key that lacked the row-read scope was refused with `403` |
There was a problem hiding this comment.
Do not say the sample row read returns rows
In the generated benefits sample, the person entity has a principal-bound required filter on id, so the row-reader principal (row_reader) adds an implicit id=row_reader filter in addition to the tutorial's household_id=hh-1001; there is no such person in the generated workbook, so this authorized request succeeds with an empty result rather than returning the household rows. Telling users the read returned rows makes the smoke test look broken when they follow the tutorial and see no data.
Useful? React with 👍 / 👎.
Handoff for Tier-C review — status + open itemsThis PR adds the Stage-1 Trust & Security explanation set and refreshes the Relay tutorial. Since opening, the automated review prompted a deep, code-grounded accuracy pass: the trust/privacy claims were re-verified against the notary/relay crates and the generated OpenAPI and corrected across several commits. Summary for the reviewer, with the open items that still need a maintainer who can run the lab.
|
Summary
Adds the first Stage-1 Trust & Security documentation set (owning area:
docs/site) and refreshes the Relay tutorial.Five new
explanation/pages, wired into the sidebar (a new "Trust & security" group, plus the disclosure page and therecords-stay-homepilot under Explanation):The Relay tutorial (
publish-spreadsheet-secured-registry-api) gains a clearer framing, the exactproblem+jsoncodes (auth.missing_credential/auth.scope_denied/auth.purpose_required), and a "what you built" recap, layered on top of its existing verified command output. Itssh-command count is unchanged, so the tutorial drift check still holds.House style throughout: second person, sentence-case headings, no inline spec citations (specs are linked at the foot of each page).
Checks
npm run check— all content stages pass: frontmatter, docset, content, markdown, Vale style (+ fixtures), openapi lint, config-vocabulary,tutorial:dry-run(the tutorial holds at its drift-checked command count), svg,build, andllms:built(89 pages).check:links:built(125,115 internal links/assets) andcheck:seo:built(208 latest + 621 archived HTML) directly — both pass.build:archivescould not run in the authoring environment: it clones a separate product repo for an OpenAPI spec, which was not reachable there (HTTP 403). It is independent of this change — please confirm it on CI.Notes
status: draft.RS-*specifications is required before they move beyond draft and merge. This PR is opened for that review.RS-*specs and the implementation. Two corrections of note are reflected in the pages — credential holder binding is per-profile and defaults to unbound (the pages no longer imply it is always on), and credential signing is EdDSA by default with ES256 also supported. The Notary audit record omits the spec'sscopesfield while Relay records it; the pages say so.none(unbound) — secure-by-default should bind via did:jwk #172.DCO
Signed-off-bytrailer.🤖 Generated with Claude Code
https://claude.ai/code/session_011U1jTj594nGXhR2r9BzWqm
Generated by Claude Code