fix(persona): read the disclosure context from its typed shape, and check the type - #190
Conversation
…heck the type Pairs with OpenVTC/verifiable-trust-infrastructure#1306, which gives the disclosure's authorization context the `{type, summary, risk, action}` shape an approver's card can render. The check that shape makes possible is the point: authorization contexts share one `ext` key, so a Cierge share ask arrives down the same path as a disclosure. Without discriminating on `type`, a correctly-signed share ask from an enrolled agent passes every other guard in this module — proof, issuer, tamper — and would be shown to the holder in a disclosure's words, its `action` mined for claim types it never had. Now it is refused, and a test drives it. `summary` is surfaced on the context too: the agent guarantees it equals the request's `reason`, so a surface may show either. No compatibility fold for the old flat shape — nothing is deployed, and the two repos cut over together. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
🛡️ AI Agentic Security Code Review2 AI-confirmed issues. Mandatory to check: 🔒 Security Code Review Report Details🛡️ Security Code Review Report — PR #190
🗺️ Scan CoverageModules scanned: 1 · with findings: 1 · files: 2 · findings: 3
Executive Summary
🔒 Security IssuesConfirmed Vulnerabilities (2)🟡 action.kind is never validated to equal 'disclose' after type gate — potential semantic type confusion
🧠 AI Triage:
Summary: The disclosure step-up verifier checks a top-level 📝 Description: A holder relying on this library's output to render a consent prompt could be shown disclosure-flavoured language and claimTypes for an action that is not actually a disclosure, undermining accurate informed consent in the approval UX. 🧪 Proof of Concept: Only ctx.type is checked before proceeding to trust action.previewId/claimTypes; action.kind, the field the comment says is the per-action discriminator, is read nowhere in this function. Vulnerable lines: 182, 206 🔎 Evidence: 💥 Impact: A holder relying on this library's output to render a consent prompt could be shown disclosure-flavoured language and claimTypes for an action that is not actually a disclosure, undermining accurate informed consent in the approval UX. Confidentiality: low — no additional data is disclosed beyond what the actual signed action already grants at the field level · Integrity: medium — the semantic meaning presented to the holder/relying UI may not match the actual operation approved, undermining informed consent · Availability: none 🧭 Reachability:
⚖️ Triage Factors:
Attack scenario: A compromised or buggy agent signs an authorization-context with the correct disclosure 🔧 Remediation:
Adds an explicit check that action.kind === 'disclose', enforcing the same discriminator discipline at the nested level that was already applied at the top level, closing the semantic type-confusion gap. Vulnerable code: Secure code: Additional recommendations:
🟡 ctx.summary read from unsigned-adjacent path independent of action-level type gate
🧠 AI Triage:
Summary: context.summary is populated from the top-level ctx.summary field rather than from the type-gated action object, which is safe today only because of the early-return type check earlier in the function — a structurally fragile pattern flagged by the threat model as a latent spoofing risk. 📝 Description: No current exploitable impact given existing control flow; if reintroduced via refactor, could let a non-disclosure context's summary text be shown to a holder as disclosure-approval language, undermining informed consent. 🧪 Proof of Concept: summary is the only field in the returned context sourced from the outer ctx object rather than the type-gated action object, breaking the 'one shape every context uses' discipline applied to the rest of the fields. Vulnerable lines: 205, 217 🔎 Evidence: 💥 Impact: No current exploitable impact given existing control flow; if reintroduced via refactor, could let a non-disclosure context's summary text be shown to a holder as disclosure-approval language, undermining informed consent. Confidentiality: none · Integrity: low — potential for holder-facing summary text to diverge from the actual approved action fields if code evolves · Availability: none 🧭 Reachability:
⚖️ Triage Factors:
Attack scenario: If the type-gate ordering or early-return logic in verifyDisclosureStepUp is ever refactored, ctx.summary could become readable for a non-disclosure context type, letting a foreign action's summary be shown as if it were a disclosure summary. 🔧 Remediation:
Prefers action.summary (inside the type-gated, kind-specific object) when present, falling back to the top-level ctx.summary only as a legacy/compat path, keeping all rendered fields consistently sourced from the validated action shape. Vulnerable code: Secure code: Additional recommendations:
Generated by Agentic Sec — AI Security Validation Agent Details🛡️ Threat Model & Affect Analysis — PR #190
📋 Affect AnalysisChange SummaryFixes a type-confusion vulnerability in the persona disclosure step-up verification by adding a 'type' discriminator check and reading operation-specific fields (previewId, verifierDid, claimTypes, purpose) from a nested 'action' object instead of the shared top-level 'ext' context. This prevents a Cierge share-ask authorization context (which shares the same reverse-DNS ext key) from being misread as a Persona disclosure approval and shown to the holder with disclosure-specific language. Diff: +62 / -15 lines Risk Assessment
This is a security-hardening fix (closes a real type-confusion vulnerability, CWE-843) with good test coverage including a targeted adversarial regression test. Risk is elevated to medium rather than low because: (1) the fix is incomplete — action.kind is never validated, leaving a narrower residual type-confusion gap within the same declared type; (2) the schema change is breaking for any payload producer not included in this diff, creating an operational/availability risk if not coordinated; (3) the new summary field is inconsistently scoped relative to the rest of the fix. No new external attack surface or entry point is introduced, and the change strictly narrows/adds validation rather than removing controls, which limits the ceiling of the risk. Review Focus Areas:
|
| Component | Impact | Change | What Changed |
|---|---|---|---|
| persona/step-up disclosure verification | high | modified | Added a type discriminator check on the shared authorization-context ext payload and restructured field extraction to read operation-specifi |
| Shared 'org.openvtc.authorization-context' ext-key namespace (cross-feature, includes Cierge) | medium | modified | This module now enforces a type discriminator on its consumption of the shared ext key, but other consumers of the same key (e.g. Cierge sha |
| persona.step-up test suite | low | modified | Test fixtures updated to the new nested schema (type, action.*), and a new adversarial regression test added asserting that a Cierge-shaped |
📁 File Classifications
packages/core/src/persona/step-up.ts
- Type: security
💡 Recommendations
- MUST — Add explicit validation that
action.kind === "disclose"before extracting any action fields (effort: small)- Closes the residual type-confusion gap where a payload with the correct top-level type but wrong/missing action.kind would still be processed as a disclosure.
- MUST — Confirm and update all upstream producers of the signed authorization-context payload to the new nested action schema before merging/deploying (effort: medium)
- This is a breaking wire-format change; unmigrated producers will have all approvals rejected, causing a functional outage of the disclosure step-up flow.
- SHOULD — Add structured audit logging on all ok:false rejection paths (type mismatch, previewId mismatch) (effort: small)
- Currently no logging exists for rejected step-up attempts, hindering detection of probing/confusion attempts against the shared ext-key channel.
- SHOULD — Move
summaryextraction to be scoped consistently with the other action fields (post type-check, ideally under action) (effort: small)- Keeps the new field's trust boundary consistent with the design intent of the rest of this fix.
- SHOULD — Audit all other consumers of the shared 'org.openvtc.authorization-context' ext key (e.g. Cierge) to confirm each enforces its own type discriminator (effort: medium)
- The vulnerability class fixed here is inherent to the shared-key design and could recur in any other consumer that hasn't been similarly hardened.
🛡️ STRIDE Threat Model
Identified Threats (11)
🟡 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Spoofing |
| Severity | Medium |
| Likelihood | Possible |
Mitigation: Move summary extraction under the type-gated action object, or re-derive it strictly from the already-verified refusal.reason/request.reason rather than from attacker/agent-supplied ctx.summary.
🟡 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Spoofing |
| Severity | Medium |
| Likelihood | Possible |
Mitigation: Explicitly validate action.kind === "disclose" in addition to the top-level type check, rejecting any other kind even when type matches.
🔵 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Tampering |
| Severity | Low |
| Likelihood | Unlikely |
Mitigation: Use an explicit allow-list constructor for context instead of spread-merging attacker-influenced keys, and reject reserved keys like __proto__.
🔵 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Tampering |
| Severity | Low |
| Likelihood | Possible |
Mitigation: Validate claimTypes against an enumerated allow-list of known claim type URIs before returning them for display.
🔵 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Information Disclosure |
| Severity | Low |
| Likelihood | Likely |
Mitigation: Sanitize or whitelist the value before interpolation into user-facing/logged error reasons; avoid reflecting arbitrary attacker-controlled types verbatim.
🔵 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Repudiation |
| Severity | Low |
| Likelihood | Likely |
Mitigation: Add structured audit logging on every ok:false rejection path, including the offending type/previewId values and verified issuer DID.
🟡 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Elevation of Privilege |
| Severity | Medium |
| Likelihood | Possible |
Mitigation: Require and validate action.kind === "disclose" as a hard precondition before any field extraction from action.
🟡 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Tampering |
| Severity | Medium |
| Likelihood | Possible |
Mitigation: Add operation-scoped authorization checks (e.g., per-operation capability/permission on the executor DID) in addition to the enrollment list.
🔵 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Denial of Service |
| Severity | Low |
| Likelihood | Unlikely |
Mitigation: Cap the maximum number of claimTypes processed/rendered (e.g., truncate or reject arrays beyond a sane limit).
🔵 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Spoofing |
| Severity | Low |
| Likelihood | Unlikely |
Mitigation: Consider per-feature ext keys instead of a shared key with a type discriminator, or add a version/schema registry with strict validation at ingestion.
🟡 : Untitled Threat
| Field | Detail |
|---|---|
| Category | Tampering |
| Severity | Medium |
| Likelihood | Possible |
Mitigation: Document and enforce that action/ctx must only ever be read from the cryptographically verified payload object, never from cached/derived copies, and add integration tests covering UI-layer consumption paths.
🍝 PASTA Threat Model
Objectives
Technical Scope
Entry Points (1): EP-001 FUNCTION_CALL verifyDisclosureStepUp(seen, enrolled)
Trust Boundaries (3): TB-1 Client → HTTP Service (untrusted user input) · TB-2 Service → Database (query construction boundary) · TB-3 Service → External APIs (SSRF boundary)
Technologies And Dependencies (2): TD-1 TypeScript · TD-2 mjs
Use Cases (5)
- Data Flow 1:
- Data Flow 2:
- Data Flow 3:
- Data Flow 4:
- Data Flow 5:
⚔️ Attack Scenarios (3)
Exfiltrate or corrupt sensitive data
flowchart LR
S0["Step 1: undefined"]
S1["Step 2: undefined"]
S2["Step 3: undefined"]
S3["Step 4: undefined"]
S4["Step 5: undefined"]
S0 --> S1
S1 --> S2
S2 --> S3
S3 --> S4
Escalate privileges and assume unauthorized identity
flowchart LR
S0["Step 1: undefined"]
S1["Step 2: undefined"]
S2["Step 3: undefined"]
S3["Step 4: undefined"]
S0 --> S1
S1 --> S2
S2 --> S3
Disrupt service availability and hide evidence
flowchart LR
S0["Step 1: undefined"]
S1["Step 2: undefined"]
S2["Step 3: Chain denial of service + repudiation weaknesses to achieve disrupt service availability and hide evidence"]
S0 --> S1
S1 --> S2
🛡️ Recommended Countermeasures
- CM-001 (Immediate): Implement robust authentication controls including multi-factor authentication, secure session handling with proper token rotation, and anti-replay mechanisms. Validate all authentication tokens server-side and enforce session timeouts.
- Effort: days
- Mitigates: Spoofing, A07, undefined, undefined, undefined
- CM-002 (Immediate): Apply strict input validation at all trust boundaries using allowlists. Parameterize all database queries, validate request payloads against schemas, and implement integrity checks for sensitive data modifications.
- Effort: days
- Mitigates: Tampering, Injection, A03, undefined, undefined, undefined
- CM-003 (Short-term): Implement tamper-evident structured logging for all security-relevant events including authentication attempts, authorization decisions, and data access. Forward logs to a centralized SIEM with integrity protection.
- Effort: days
- Mitigates: Repudiation, A09, undefined
- CM-004 (Immediate): Enforce least-privilege access controls at every endpoint. Remove sensitive data from error responses, implement proper object-level authorization, and encrypt sensitive data at rest and in transit.
- Effort: days
- Mitigates: Information Disclosure, A01, IDOR, undefined
- CM-005 (Short-term): Apply rate limiting at route and user level. Set request size limits, implement timeouts for all external calls, use circuit breakers for downstream services, and validate algorithmic complexity of user inputs.
- Effort: days
- Mitigates: Denial of Service, DoS, A04, undefined
- CM-006 (Immediate): Implement centralized authorization middleware with RBAC/ABAC. Enforce object-level and function-level access checks on every endpoint. Apply principle of least privilege to service accounts and API keys.
- Effort: days
- Mitigates: Elevation of Privilege, A01, undefined
📊 Risk Summary
Total Threats: 11
By Severity: Low: 6 · Medium: 5
By Category: Spoofing: 3 · Tampering: 4 · Information Disclosure: 1 · Repudiation: 1 · Elevation of Privilege: 1 · Denial of Service: 1
🛡️ Risk Mitigation Strategy
- Countermeasures: {"id":"CM-001","name":"Strengthen authentication and session management","effort":"days","priority":"Immediate","mitigates":["Spoofing","A07","undefined","undefined","undefined"],"description":"Implement robust authentication controls including multi-factor authentication, secure session handling with proper token rotation, and anti-replay mechanisms. Validate all authentication tokens server-side and enforce session timeouts.","breaks_story":"Mitigates 3 Spoofing threats by removing the preconditions required for exploitation.","effectiveness":"medium"}; {"id":"CM-002","name":"Input validation and data integrity controls","effort":"days","priority":"Immediate","mitigates":["Tampering","Injection","A03","undefined","undefined","undefined"],"description":"Apply strict input validation at all trust boundaries using allowlists. Parameterize all database queries, validate request payloads against schemas, and implement integrity checks for sensitive data modifications.","breaks_story":"Mitigates 4 Tampering threats by removing the preconditions required for exploitation.","effectiveness":"medium"}; {"id":"CM-003","name":"Comprehensive audit logging and monitoring","effort":"days","priority":"Short-term","mitigates":["Repudiation","A09","undefined"],"description":"Implement tamper-evident structured logging for all security-relevant events including authentication attempts, authorization decisions, and data access. Forward logs to a centralized SIEM with integrity protection.","breaks_story":"Mitigates 1 Repudiation threat by removing the preconditions required for exploitation.","effectiveness":"medium"}; {"id":"CM-004","name":"Data exposure prevention and access controls","effort":"days","priority":"Immediate","mitigates":["Information Disclosure","A01","IDOR","undefined"],"description":"Enforce least-privilege access controls at every endpoint. Remove sensitive data from error responses, implement proper object-level authorization, and encrypt sensitive data at rest and in transit.","breaks_story":"Mitigates 1 Information Disclosure threat by removing the preconditions required for exploitation.","effectiveness":"medium"}; {"id":"CM-005","name":"Rate limiting and resource exhaustion guards","effort":"days","priority":"Short-term","mitigates":["Denial of Service","DoS","A04","undefined"],"description":"Apply rate limiting at route and user level. Set request size limits, implement timeouts for all external calls, use circuit breakers for downstream services, and validate algorithmic complexity of user inputs.","breaks_story":"Mitigates 1 Denial of Service threat by removing the preconditions required for exploitation.","effectiveness":"medium"}; {"id":"CM-006","name":"Authorization enforcement and privilege separation","effort":"days","priority":"Immediate","mitigates":["Elevation of Privilege","A01","undefined"],"description":"Implement centralized authorization middleware with RBAC/ABAC. Enforce object-level and function-level access checks on every endpoint. Apply principle of least privilege to service accounts and API keys.","breaks_story":"Mitigates 1 Elevation of Privilege threat by removing the preconditions required for exploitation.","effectiveness":"medium"}
- Priority Actions: Strengthen authentication and session management; Input validation and data integrity controls; Data exposure prevention and access controls; Authorization enforcement and privilege separation
Generated by Agentic Sec — Threat Model & Affect Analysis Agent
📊 Summary & findings
| ✅ Confirmed | |
|---|---|
| 2 | 0 |
Confirmed (2)
- 🟡 action.kind is never validated to equal 'disclose' after type gate — potential semantic type confusion
- 🟡 ctx.summary read from unsigned-adjacent path independent of action-level type gate (triaged LOW→MEDIUM)
Security review on #190 found the gap, correctly: the module's own comment names `kind` as the per-action discriminator and nothing checked it. `type` and `kind` answer different questions — which producer's vocabulary the context speaks, and which action within it. Checking one and not the other leaves the narrower confusion open. It rejects nothing today, because the persona context type carries only `disclose`; it is here for the second `kind` added under this type, which would otherwise be read as a disclosure, have its fields mined for claimTypes it never had, and be shown to the holder in a disclosure's words. Absence is not permission either — an action with no `kind` is refused, which is the shape a producer that forgot the discriminator emits. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
|
Thanks — one of the two is a real gap and is now fixed; the other I'm declining, with reasons. Fixed:
|
2 participants
Pairs with OpenVTC/verifiable-trust-infrastructure#1306, which fixes a defect in
the agent-side landing (VTI #1304): the disclosure's authorization context was a
flat bag with no
type, and an approver's card discriminates ontype.The agent now sends the shape every authorization context uses:
{ "type": "https://openvtc.org/persona/authorization-context/0.1", "summary": "Approve disclosing 1 fact to did:key:z…", "risk": "high", "action": { "kind": "disclose", "previewId": "…", "verifierDid": "…", "claimTypes": ["payment.card"], "purpose": "checkout" } }verifyDisclosureStepUpreads it from there, and gains the check thetypemakes possible.
The new refusal is the point of the change
A context whose
typeis not the disclosure one is refused. Authorizationcontexts are a shared channel — a Cierge share ask travels under the same
org.openvtc.authorization-contextkey. Without this check, one of those wouldbe read as a disclosure: shown to the holder in a disclosure's words, its
actionmined for claim types it never had, and approved as though it were arelease of their identity.
The new test drives exactly that — a genuine, correctly-signed share ask, from
an enrolled agent — and requires it to be refused. Note that every other guard
in this module passes it: the proof verifies, the issuer is enrolled, nothing
is tampered. Only the
typetells them apart.summaryis now surfaced onDisclosureApprovalContexttoo, since the agentguarantees it equals the request's
reason— a surface may show either withoutthe two differing.
Cut over, not folded
Per this repo's rule: nothing is deployed, so there is no dual-accept arm for
the old flat shape and no deprecation window. #1306 and this land together.
Tests
10 in
persona.step-up.mjs(one new: the share-ask refusal); the previewIdcross-check and the tampered-context test move to
actionand still pass.Full suite green — 915 across the four workspaces, 0 failures;
tsc -bclean.Guide checklist (§9)
wire change cuts over with #1306 in the same cycle
typerefuses rather than falling backto a permissive read; an absent
actionyields emptyclaimTypes