Feat/persona webhook - #197
Conversation
KYC providers like Persona return verification results asynchronously via webhook, not inline in the API response. The old /api/issue flow resolved the inquiry synchronously right after the Persona redirect, which breaks whenever decisioning isn't instant. - app/api/persona/webhook: verifies the Persona-Signature HMAC (reject invalid/missing), and on an approved inquiry.completed/approved event derives the credential commitment and signs it via IssuerClient, using the holder/issuerId/attributes carried through Persona's reference-id. - app/api/persona/result: lets the holder's browser poll for the signed credential by inquiry id. - lib/persona-cache: short-lived in-memory TTL cache, keyed by inquiry id, storing only the signed Credential (commitment/sig/value/salt) — never the raw identity fields (name, government ID number, etc.) Persona returns. - lib/persona: Persona API + webhook helpers shared between the demo/mock sync path and the new async path. - app/verify: polls /api/persona/result instead of re-POSTing /api/issue after the Persona redirect. - Mock mode (no PERSONA_API_KEY) is untouched — still issues synchronously inline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Soroban persistent storage entries expire (archive) unless their TTL is extended, independent of the credential's own `expiry`. Long-lived credentials (e.g. a 1-year expiry) could have their proof entries archived well before the credential itself expires, making is_verified/check_claim silently behave as if the holder never proved anything. - submit_proof / submit_proofs_batch now extend the entry TTL to cover at least the credential's expiry (converted from a ledger timestamp to a ledger count via the new ttl_for_expiry helper), floored at the existing 90-day default and capped at the network's max allowed entry TTL (env.storage().max_ttl()). - Add bump_claim(holder, credential_type): a permissionless entry point (no require_auth) that anyone can call to top up the TTL of a still-valid claim without resubmitting a proof. Panics ProofNotFound for an unknown claim, ClaimNotValid for one that's revoked or past expiry. - Document the rent/archival model in a module-level doc comment in contracts/proof_registry/src/lib.rs and in the root README's Security model section. - Add tests asserting TTL is extended to cover a long expiry, floored for a short one, capped at the network max, and correctly topped up (or left alone) by bump_claim, plus its failure paths. cargo test passes (53 tests across the workspace, 38 in proof_registry). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Someone is attempting to deploy a commit to the psalmuel01's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Wilfred007 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export function verifyPersonaWebhookSignature( | ||
| rawBody: string, | ||
| signatureHeader: string | null, | ||
| secret: string, | ||
| ): boolean { | ||
| if (!signatureHeader) return false; | ||
|
|
||
| let timestamp: string | undefined; | ||
| const signatures: string[] = []; | ||
| for (const part of signatureHeader.split(",")) { | ||
| const eq = part.indexOf("="); | ||
| if (eq === -1) continue; | ||
| const key = part.slice(0, eq).trim(); | ||
| const value = part.slice(eq + 1).trim(); | ||
| if (key === "t" && !timestamp) timestamp = value; | ||
| else if (key === "v1") signatures.push(value); | ||
| } | ||
| if (!timestamp || signatures.length === 0) return false; | ||
|
|
||
| const expectedHex = createHmac("sha256", secret) | ||
| .update(`${timestamp}.${rawBody}`) | ||
| .digest("hex"); | ||
| const expected = Buffer.from(expectedHex, "hex"); | ||
|
|
||
| return signatures.some((sig) => { | ||
| if (!/^[0-9a-f]+$/i.test(sig)) return false; | ||
| const actual = Buffer.from(sig, "hex"); | ||
| return actual.length === expected.length && timingSafeEqual(actual, expected); | ||
| }); | ||
| } |
There was a problem hiding this comment.
verifyPersonaWebhookSignature validates the HMAC over {t}.{rawBody} but never checks that t is close to Date.now(). Any captured valid webhook payload (including the Persona-Signature header) can be replayed indefinitely. Persona and every major webhook provider (Stripe, GitHub, etc.) recommend rejecting webhooks where |now − t| > 5 minutes specifically to prevent this. While the practical blast radius is low here (replay just re-stores under the same inquiryId), a replay landing after the 15-minute cache TTL has expired would silently re-issue and re-cache credentials for an already-completed inquiry.
The fix is to parse timestamp as a number and return false when Math.abs(Date.now() / 1000 - timestampSeconds) > 300.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/lib/persona.ts
Line: 102-131
Comment:
**Missing timestamp staleness check enables webhook replay**
`verifyPersonaWebhookSignature` validates the HMAC over `{t}.{rawBody}` but never checks that `t` is close to `Date.now()`. Any captured valid webhook payload (including the `Persona-Signature` header) can be replayed indefinitely. Persona and every major webhook provider (Stripe, GitHub, etc.) recommend rejecting webhooks where `|now − t| > 5 minutes` specifically to prevent this. While the practical blast radius is low here (replay just re-stores under the same `inquiryId`), a replay landing after the 15-minute cache TTL has expired would silently re-issue and re-cache credentials for an already-completed inquiry.
The fix is to parse `timestamp` as a number and return `false` when `Math.abs(Date.now() / 1000 - timestampSeconds) > 300`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| if (!timestamp || signatures.length === 0) return false; | ||
|
|
||
| const expectedHex = createHmac("sha256", secret) |
There was a problem hiding this comment.
| if (!timestamp || signatures.length === 0) return false; | |
| const expectedHex = createHmac("sha256", secret) | |
| if (!timestamp || signatures.length === 0) return false; | |
| const timestampSeconds = Number(timestamp); | |
| if ( | |
| !Number.isFinite(timestampSeconds) || | |
| Math.abs(Date.now() / 1000 - timestampSeconds) > 300 | |
| ) { | |
| return false; | |
| } | |
| const expectedHex = createHmac("sha256", secret) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/lib/persona.ts
Line: 119-121
Comment:
Add a timestamp staleness check so replayed webhooks are rejected after 5 minutes, matching Persona's documented best practice.
```suggestion
if (!timestamp || signatures.length === 0) return false;
const timestampSeconds = Number(timestamp);
if (
!Number.isFinite(timestampSeconds) ||
Math.abs(Date.now() / 1000 - timestampSeconds) > 300
) {
return false;
}
const expectedHex = createHmac("sha256", secret)
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| function isPersonaReference(value: unknown): value is PersonaReference { | ||
| if (!value || typeof value !== "object") return false; | ||
| const v = value as Record<string, unknown>; | ||
| return ( | ||
| typeof v.holder === "string" && | ||
| typeof v.issuerId === "string" && | ||
| typeof v.issuerName === "string" && | ||
| Array.isArray(v.credentialTypes) | ||
| ); | ||
| } |
There was a problem hiding this comment.
isPersonaReference guard omits expiry field validation
PersonaReference declares expiry: string, but the type guard never checks typeof v.expiry === "string". A crafted webhook payload with expiry: null, expiry: 0, or expiry missing entirely would still pass isPersonaReference and be cast to PersonaReference. reference.expiry is then forwarded directly to issuer.issue() without further sanitization, meaning the issued credential could have an invalid or unexpected expiry.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/lib/persona.ts
Line: 141-150
Comment:
**`isPersonaReference` guard omits `expiry` field validation**
`PersonaReference` declares `expiry: string`, but the type guard never checks `typeof v.expiry === "string"`. A crafted webhook payload with `expiry: null`, `expiry: 0`, or `expiry` missing entirely would still pass `isPersonaReference` and be cast to `PersonaReference`. `reference.expiry` is then forwarded directly to `issuer.issue()` without further sanitization, meaning the issued credential could have an invalid or unexpected expiry.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| return ( | ||
| typeof v.holder === "string" && | ||
| typeof v.issuerId === "string" && | ||
| typeof v.issuerName === "string" && | ||
| Array.isArray(v.credentialTypes) | ||
| ); |
There was a problem hiding this comment.
expiry field check to prevent a webhook with a non-string expiry from passing the type guard and reaching issuer.issue() unchecked.
| return ( | |
| typeof v.holder === "string" && | |
| typeof v.issuerId === "string" && | |
| typeof v.issuerName === "string" && | |
| Array.isArray(v.credentialTypes) | |
| ); | |
| return ( | |
| typeof v.holder === "string" && | |
| typeof v.issuerId === "string" && | |
| typeof v.issuerName === "string" && | |
| typeof v.expiry === "string" && | |
| Array.isArray(v.credentialTypes) | |
| ); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/lib/persona.ts
Line: 144-149
Comment:
Add the missing `expiry` field check to prevent a webhook with a non-string expiry from passing the type guard and reaching `issuer.issue()` unchecked.
```suggestion
return (
typeof v.holder === "string" &&
typeof v.issuerId === "string" &&
typeof v.issuerName === "string" &&
typeof v.expiry === "string" &&
Array.isArray(v.credentialTypes)
);
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export function getPersonaResult(inquiryId: string): Credential[] | null { | ||
| sweep(); | ||
| return cache.get(inquiryId)?.credentials ?? null; | ||
| } |
There was a problem hiding this comment.
getPersonaResult returns the cached Credential[] but never removes the entry. After the browser's poll loop picks up the credentials on the first successful response, subsequent calls to /api/persona/result?inquiry_id=... within the 15-minute TTL window will return the same credentials again. Removing the entry on read eliminates this window.
| export function getPersonaResult(inquiryId: string): Credential[] | null { | |
| sweep(); | |
| return cache.get(inquiryId)?.credentials ?? null; | |
| } | |
| export function getPersonaResult(inquiryId: string): Credential[] | null { | |
| sweep(); | |
| const entry = cache.get(inquiryId); | |
| if (!entry) return null; | |
| cache.delete(inquiryId); | |
| return entry.credentials; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/lib/persona-cache.ts
Line: 35-38
Comment:
**Credentials remain in cache after successful retrieval**
`getPersonaResult` returns the cached `Credential[]` but never removes the entry. After the browser's poll loop picks up the credentials on the first successful response, subsequent calls to `/api/persona/result?inquiry_id=...` within the 15-minute TTL window will return the same credentials again. Removing the entry on read eliminates this window.
```suggestion
export function getPersonaResult(inquiryId: string): Credential[] | null {
sweep();
const entry = cache.get(inquiryId);
if (!entry) return null;
cache.delete(inquiryId);
return entry.credentials;
}
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
@Wilfred007 resolve conflicts |
2 participants
What does this PR do?
What changed in contracts/proof_registry/src/lib.rs:
Added a ttl_for_expiry helper that converts a credential's expiry (a ledger timestamp, seconds) into a ledger-count TTL, floored at the existing 90-day default and capped at env.storage().max_ttl()
(the network's max allowed entry TTL, ~1 year on mainnet).
submit_proof and submit_proofs_batch now extend the proof entry's TTL using ttl_for_expiry(expiry) instead of the old fixed 90-day constant, so long-lived credentials (e.g. 1-year) get a TTL that
actually covers their lifetime.
New bump_claim(holder, credential_type) entry point — no require_auth, callable by anyone — tops up the TTL of a still-valid claim. Panics ProofNotFound if unknown, ClaimNotValid if
revoked/expired.
New ClaimNotValid error variant.
A module-level "Rent and archival" doc comment explains the TTL-vs-expiry distinction and why bump_claim exists (network max TTL can be shorter than a credential's remaining life;the
seconds→ledgers conversion is an approximation).
Root README.md Security model section gained a "Rent and archival" bullet summarizing the same, linking to the code comment.
submit_proof and submit_proofs_batch now extend the proof entry's TTL using ttl_for_expiry(expiry) instead of the old fixed 90-day constant, so long-lived credentials (e.g. 1-year)
get a TTL that actually covers their lifetime.
New bump_claim(holder, credential_type) entry point — no require_auth, callable by anyone — tops up the TTL of a still-valid claim. Panics ProofNotFound if unknown, ClaimNotValid
if revoked/expired.
New ClaimNotValid error variant.
A module-level "Rent and archival" doc comment explains the TTL-vs-expiry distinction and why bump_claim exists (network max TTL can be shorter than a credential's remaining life;
the seconds→ledgers conversion is an approximation).
Root README.md Security model section gained a "Rent and archival" bullet summarizing the same, linking to the code comment.
Tests (8 new, in contracts/proof_registry/src/test.rs): TTL exactly covers a long (200-day) expiry, floors at 90 days for a short expiry, caps at the network max for an absurdly long
expiry, bump_claim extends TTL for a valid claim (verified via get_ttl testutils) and is a no-op when TTL is already high, and panics for unknown/expired/revoked claims.
cargo test --workspace (1.91.0 toolchain — the default stable toolchain on this machine is missing its cargo component, unrelated to this change) passes all 53 tests, 38 of them in
proof_registry. I also rebuilt the wasm32v1-none release artifact, which is why the pre-existing upgrade_by_admin_succeeds/upgrade_by_non_admin_panics/admin_transfer_works test
snapshots changed (they embed the current wasm's hash/cost inputs) — those, plus new snapshots for the 8 new tests, are included in the commit per the repo's existing convention of
tracking test_snapshots/.
One gap worth flagging given the PR's Greptile/CI requirements: I couldn't run cargo fmt/clippy against CI's actual settings since this repo's CI only runs cargo build + cargo test
(no fmt/clippy gate), so I hand-checked my added code against cargo fmt --check locally but didn't touch the pre-existing formatting drift elsewhere in the file. Also, since neither
Greptile nor CI can be invoked from here, I can't pre-empt what review comments will come back — you'll need to push and check those once the PR is open.
Closes #71
Closes #75
Closes #70
Closes #69
Type of change
Checklist
cargo testpasses (contracts)pnpm tsc --noEmitpasses (frontend)pnpm buildpasses (frontend)fixtures/<type>/artifacts updatedNEXT_PUBLIC_prefix on server-only env varsNotes for reviewers
Greptile Summary
This PR adds two largely independent features: a Persona KYC webhook integration for the frontend (async credential issuance after identity verification) and dynamic TTL management for the
ProofRegistrysmart contract (expiry-aligned storage TTL plus a new permissionlessbump_claimentry point).lib/persona.ts(inquiry creation, webhook signature verification, attribute extraction),app/api/persona/webhook/route.ts(receives Persona callbacks and issues/caches credentials),app/api/persona/result/route.ts(browser polling endpoint), and an in-memory credential cache inlib/persona-cache.ts. The verify page gains a polling loop that activates when Persona redirects back with aninquiry-idquery param.ProofRegistry): Introducesttl_for_expiryto translate a credential'sexpirytimestamp into a ledger-count TTL (floored at 90 days, capped at network max), applies it on everysubmit_proof/submit_proofs_batchwrite, and adds the newbump_claimpermissionless entry point with theClaimNotValiderror variant. Eight new tests and updated snapshots accompany the changes.Confidence Score: 3/5
The Rust contract changes are solid and safe to merge on their own; the frontend Persona webhook integration has a security gap that should be addressed before going to production.
The smart contract TTL logic,
bump_claimentry point, and accompanying tests are correct and well-documented. The frontend webhook handler correctly verifies HMAC signatures and strips PII before logging. However,verifyPersonaWebhookSignaturenever checks that the timestamp in thePersona-Signatureheader is recent, so any captured valid webhook payload can be re-delivered indefinitely. Additionally,isPersonaReferencedoes not validate theexpiryfield type, and the credential cache does not evict entries after the first successful read.Files Needing Attention: frontend/lib/persona.ts (timestamp staleness check and type guard completeness) and frontend/lib/persona-cache.ts (cache eviction on read)
Security Review
frontend/lib/persona.tsverifyPersonaWebhookSignature: The HMAC over{t}.{rawBody}is verified correctly, but the timestamptis never compared against the current time. A captured validPersona-Signatureheader can be replayed to/api/persona/webhookindefinitely. Persona's own documentation recommends rejecting webhooks where|now − t| > 5 minutes.frontend/lib/persona-cache.ts: After the holder's browser successfully retrieves issued credentials from/api/persona/result, the cache entry remains live for the full 15-minute TTL, allowing a second retrieval via replay or a duplicated session.Important Files Changed
isPersonaReferencetype guard omitsexpiryfield validation.ttl_for_expiryhelper andbump_claimentry point to align proof storage TTL with credential expiry — logic, edge cases (floor/cap), and error variants look correct; permissionless design ofbump_claimis intentional and well-documented.submit_proof, plusbump_claimhappy path, no-op, and panic cases — good coverage of the new behaviour.inquiry-idquery param and handles redirect after successful credential pickup — return-URL validation and HTTPS protocol checks look correct.SAFE_FIELDS) to strip sensitive fields before logging — design is sound and correctly excludes PII.Sequence Diagram
sequenceDiagram participant Browser participant IssueAPI as /api/issue participant Persona participant WebhookAPI as /api/persona/webhook participant Cache as persona-cache participant ResultAPI as /api/persona/result Browser->>IssueAPI: POST (credential_types incl. kyc) IssueAPI->>Persona: "createPersonaInquiry(reference-id=JSON)" Persona-->>IssueAPI: "{url, inquiryId}" IssueAPI-->>Browser: "202 {needsPersona, personaUrl, inquiryId}" Browser->>Persona: redirect to hosted flow Persona-->>Browser: "redirect back to /verify?inquiry-id=XXX" Note over Browser: polls /api/persona/result Persona->>WebhookAPI: POST webhook (Persona-Signature header) WebhookAPI->>WebhookAPI: verifyPersonaWebhookSignature() WebhookAPI->>WebhookAPI: parsePersonaWebhookEvent() WebhookAPI->>WebhookAPI: extractKycAttributes(fields) WebhookAPI->>WebhookAPI: issuer.issue() per credentialType WebhookAPI->>Cache: storePersonaResult(inquiryId, credentials) Browser->>ResultAPI: "GET /api/persona/result?inquiry_id=XXX" ResultAPI->>Cache: getPersonaResult(inquiryId) Cache-->>ResultAPI: credentials[] ResultAPI-->>Browser: "{ready: true, credentials}" Browser->>Browser: saveCredential() + redirectPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: explicit TTL management for ProofR..." | Re-trigger Greptile