feat: shareable score links and og card images#60
Conversation
|
@Gmin2 is attempting to deploy a commit to the Dodo Payments Team on Vercel. A member of the Team first needs to authorize it. |
thepushkaraj
left a comment
There was a problem hiding this comment.
Hey @Gmin2, Thanks for working on this, it's working correctly.
I have one suggestion: We can show the time it was scanned here since we already carry t.
There was a problem hiding this comment.
Review: feat: shareable score links and og card images
Verdict: Approve with one security follow-up (SSRF hardening). The crypto/share design is genuinely well done; the one thing I'd like addressed is the server-side URL fetch allowlist.
What I exercised
- Verified the HMAC sign/verify roundtrip and forgery resistance directly (see below)
dualmark-docs: typecheck ✓,next build✓ — all new routes compile (/api/play/score,/api/og/score[edge],/play/r/[token],/play/opengraph-image)- Confirmed graceful degradation when
DUALMARK_SHARE_SECRETis unset
Token security — verified, solid
lib/share-token.ts is the right design:
- HMAC-SHA256 over base64url(JSON), via Web Crypto so the Node score route and Edge OG route share one impl 👍
- Constant-time signature comparison (
equalBytes, XOR-accumulate) — no timing oracle - Verify-before-parse: signature checked before
JSON.parse, fail-closed on any throw - Payload shape validated after decode
I drove it end-to-end:
roundtrip valid → PASS
tampered payload (s: 100→125, same sig) → rejected ✓ (can't forge a score)
tampered signature → rejected
malformed ("", ".", "abc.", ".abc", "no-dot") → all rejected
token signed with secret A, verified with secret B → rejected
levelFromScore thresholds (0.95/0.8/0.6) + maxScore=0 guard → correct
The OG route and /play/r/[token] page both verifySharePayload before rendering and notFound()/404 on failure — no unsigned input reaches rendering.
Security follow-up (please address) — SSRF allowlist is incomplete
/api/play/score fetches the user-supplied URL server-side (both verifyUrl and detectFramework, which reads 8KB of body). safeParseUrl only blocks localhost, 127.0.0.1, and *.local. I confirmed these are still allowed:
169.254.169.254 ← cloud metadata endpoint ⚠️
10.0.0.5 / 192.168.1.1 ← private ranges
[::1] ← IPv6 loopback
0.0.0.0
metadata.google.internal
On a hosted playground this is a classic SSRF surface (cloud metadata/IMDS, internal services). Impact is bounded (the response only surfaces a conformance score + a few headers, not the raw body — though err.message is echoed and framework detection does read body bytes), but I'd harden before this ships publicly: block RFC1918 / loopback / link-local / ULA ranges and the 169.254.169.254 + metadata.* hosts, ideally resolving the host to reject private IPs (and mind DNS-rebinding). A small allowlist helper covering IPv4+IPv6 would close it.
Other notes (non-blocking)
- Good hygiene already present:
MAX_URL_LENGTHcap, protocol allowlist,cache-control: no-storeon scores, immutable cache on OG, bounded head-byte reader,AbortControllertimeouts. - The PR description already flags migrating snapshots from URL tokens to a KV store (short/opaque links, expiry, revocation). Agree that's the right longer-term direction — current HMAC-in-URL is fine for v1 but tokens are long and can't be revoked/expired individually (there's no
t-based TTL check on verify, so shared links live forever). - No changeset — fine, since this is docs-app only (no published package changes).
- CI shows
UNSTABLEmergeState — worth confirming the pending check is green before merge.
Really nice feature. Approving on the strength of the token design and clean build; please fold in the SSRF host-filtering before it's live.
Summary
Adds a share flow to the playground. when you score a site you now get a signed link (/play/r/) and an og image at
/api/og/score, so dropping the link on x or slack unfurls a card with the score, level and domain. the result rides inside the url as an hmac signed token, no datastore, and the signature stops anyone faking a score. one env var,DUALMARK_SHARE_SECRET, signs the tokens. if its unset the playground still works, the share buttons just dont render.Note: we need to move the snapshots into a kv cache store (vercel kv / upstash) instead of url tokens. that gives short opaque links, real expiry and revocation, and room to store the full check breakdown. currently we pack the whole result into the url as an hmac signed token, so theres no datastore to run, but the links are long and you cant expire or revoke a single one.
Changes
Fixes #42
Type
AEO_SPEC_VERSION)Verification
bun run buildpassesbun run testpassesbun run typecheckpassesdualmark verifyagainst the example end-to-end/spec/: ranbun run --filter dualmark-docs sync-specand committed the mirrorChangeset
bun run changeset) for any package with a user-visible change