Skip to content

SEC-002: one SSRF address policy, an unset inbound secret refuses, stored feed links open only as web links - #983

Merged
NewWorldVenture merged 7 commits into
mainfrom
claude/sec-002-sweep-etzqg7
Oct 10, 2026
Merged

NewWorldVenture merged 7 commits into
mainfrom
claude/sec-002-sweep-etzqg7

Conversation

@NewWorldVenture

Copy link
Copy Markdown
Owner

SEC-002 (High, not started) covers redirects, validation, secrets, cross-site and uploads. Three parallel read-only sweeps traced every sink on main 36a516d. None found a High or Medium issue. This PR fixes the three Low findings that need only code, each with a test that fails on the old source. The full write-up is in docs/audit/sec-002-sweep-20261009.md.

Fixes

Finding Fix Test before → after
F1 The calendar/feed SSRF guard had drifted from the document guard. It let these through: 6to4 (2002:a9fe:a9fe::1, i.e. 169.254.169.254), Teredo, SIIT, fec0::/10, 100::/64, 3fff::/20, 192.88.99.0/24 and 168.63.129.16. The guard covers calendar sync, feed sync, weekend discovery and link previews. One shared policy, lib/server/public-address.ts. The document fetcher re-exports it. The calendar fetcher still unwraps mapped/NAT64 IPv4, so NAT64 to a public host still works. 16 failed → 20/20
F2 /api/contact-center/email, a public callback, accepted every request when CONTACT_CENTER_INBOUND_SECRET was unset and NODE_ENV was not production. An unset secret now refuses in every build, matching the Twilio ingress. 6 failed → 9/9
F3 Social Feed "Open post" passed a family-writable permalink to window.open unchecked, and addFeedItemAction stored links unchecked. The link is opened through safeSocialLink, the action refuses non-web links, and a new ratchet holds every window.open to a literal or a safe link. 6 failed → 8/8

Recorded, not fixed here

  • R1 (Low): the private documents and chore-proof buckets have no allowed_mime_types. An SVG with script would run on the Supabase origin when a parent opens its signed URL. Fixing it needs a bucket migration.
  • R2 (Low, suspected): a child can overwrite or delete tax-vault and insurance-card objects. Fixing it needs a storage-policy migration.
  • R3 (Low): public /api/health names unset secrets. Changing that is an operator decision, since Health reports RESEND_WEBHOOK_SECRET, whose absence refuses every bounce and complaint (ENV-C472B8258F2A) #877 added the list on purpose.
  • R4 and R5 (informational): two URLs are built from X-Forwarded-Host, and a message cursor is not UUID-checked.

Out of scope, and the next task: kids' proof photos and videos go through a server action whose body is capped at 1 MB by default. That refuses most phone photos and every video with a 413.

The register row stays IN PROGRESS. The browser workflow cases were not run, and R1–R3 are still open. Nothing here touches the auth/session-cache files, migrations, configuration or production.

Validation

  • New tests: the three new test files fail on the old source and pass after the fix.
  • Related suites: all 16 SSRF suites, the 21 push/Alexa/media suites, the 68 contact-center suites (1,575 tests) and the 13 social/link suites pass.
  • Full unit run: 30,404 passed, 1 failed. The failure was whole-table-reads-are-not-capped hitting its 20 s timeout under full parallel load. It passes 40/40 when run alone, and this branch does not touch it.
  • Static checks: tsc --noEmit exit 0, eslint clean on changed files, git diff --check clean. The docs-migration-name guard passes 616/616.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4


Generated by Claude Code

claude added 4 commits October 9, 2026 21:41
The app had two SSRF address policies. The calendar/feed fetcher (calendar
sync, feed sync, weekend discovery, social link previews) kept its own range
list, which had drifted from the document fetcher's: a hostname resolving to
6to4 wrapping 169.254.169.254, Teredo, SIIT, site-local, the discard prefix,
3fff::/20, the 6to4 relay anycast or the Azure platform endpoint passed it.

Both now use one policy, lib/server/public-address.ts. The calendar fetcher
still unwraps an IPv4 address carried in IPv6 (mapped, compatible, NAT64) and
judges it as that IPv4 address, so NAT64 to a public host keeps working.

New test: 16 failures on the old source, 20/20 after. The source-shape guard
now asserts the shared import instead of a private range literal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4
/api/contact-center/email is a public callback in middleware.ts, so the
route's own check is all that stands in front of it. With
CONTACT_CENTER_INBOUND_SECRET unset it let every request through unless
NODE_ENV was 'production': a dev or self-hosted server run without that
filed anyone's mail into a family's inbox, ran the concierge on it and sent
an auto-reply to an address the sender chose. Deployed `next build` output
inlines NODE_ENV=production, so hosted builds already refused.

Now an unset secret refuses whatever NODE_ENV says, as the Twilio ingress
already does. New test: 6 failures on the old source (development, test,
empty NODE_ENV), 9/9 after; all 68 contact-center suites pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4
The Social Feed's "Open post" passed social_reader_items.permalink straight
to window.open. Any active family member (child and guest included) can write
that column under the 0101 policy, and addFeedItemAction stored permalink,
thumbnail and media URLs unchecked. React's javascript: filtering covers href,
not window.open; whether a javascript: URL then ran depended on the browser's
noopener handling.

The module opens the permalink through safeSocialLink (the SEC-004 rule every
stored href already uses) and shows "Open post" only for a web link; the action
refuses a non-web permalink, thumbnail or media URL before writing. A new
ratchet holds every window.open call to a literal, a fixed-host template or a
safe*Link value. New test: 6 failures on the old source, 8/8 after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4
Redirects, validation, secrets, cross-site and uploads traced end to end on
main 36a516d. Three Low findings fixed in this branch with before/after
tests; five recorded with a proposed owner and scope (two need a storage
migration, one an operator decision on /api/health). The register row stays
IN PROGRESS: browser workflow cases were not run and R1-R3 remain open.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4
@vercel

vercel Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
bubaly Ignored Ignored Oct 10, 2026 2:40am UTC

Request Review

Copy link
Copy Markdown
Owner Author

SEC-002 scoped review at5e70bb2288c1a15c4e72807353e2e48795e94ad6 found the changed runtime boundaries correctly wired; exact-head CI remains separate. Bounded follow-on for this lane: tighten tests/a-stored-link-is-opened-only-as-a-web-link.test.ts so the global ratchet does not trust an arbitrary variable named href or accept a string-literal prefix followed by unsafe concatenation. Inspect the full argument/binding or narrow the stated guarantee; add mutation controls for unvalidated href, concatenation and an additional unsafe window.open call. The actual Social Feed sink is correctly guarded; this is a future-regression detection gap. Also update the inbound-email route header that still claims an unset secret is allowed in development. Keep SEC-002, release/storage/operator holds and browser evidence gaps open. Public address-policy sharing does not make calendar/document transports identical. No production configuration changes or live external probes are requested.

…pelling

Owner review on #983: the ratchet's regex trusted any variable named href
and any string literal followed by concatenation. It now parses each file
and accepts only a literal, a fixed-scheme or fixed-host template, a
safeWebLink/safeSocialLink call, or a const bound to one of those in an
enclosing block. Synthetic controls refuse a stored field bound to href, a
parameter, a let, a concatenation after a literal prefix, a stored-field
template, a conditional and an unknown call, and report an unsafe call added
to the real file set. Against the original Social Feed module it reports
social-feed-module.tsx:220.

Also: the inbound-email route header no longer says an unset secret is
allowed in development, and the evidence doc says only the address ranges
are shared, not the transports.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4

Copy link
Copy Markdown
Owner Author

Re 6092751883: both items are in 4fe0c9b.

Ratchet (tests/a-stored-link-is-opened-only-as-a-web-link.test.ts). The regex is gone; each file is now parsed with the TypeScript compiler API. A window.open URL argument passes only if it is:

  • a string literal;
  • a template whose head fixes the scheme (tel:, mailto:) or an https://host plus a path or query separator;
  • a safeWebLink or safeSocialLink call;
  • a const bound to one of those in an enclosing block, declared before the call.

Everything else fails, including any concatenation.

Controls:

  • Refused: a stored field bound to href, a parameter, a let (even one first set safely), a const from another function, '' + field, 'https://example.com/' + field, `${field}`, a stored scheme in a template, a conditional, and an unknown call.
  • Accepted: a literal, no URL, a checked link, a const bound to a checked link, a tel: template, and the maps template.
  • Comments: window.open inside comments is ignored.
  • Added call: an unsafe call added to the real file set is reported, and only that one.
  • Original module: with main's Social Feed module put back, the repo-wide check reports components/modules/social-feed-module.tsx:220.

26/26 pass on the branch.

Route header. app/api/contact-center/email/route.ts now reads: "Without the secret set it rejects every request, in every build, so the endpoint is never an open relay (SEC-002)."

Transports. The evidence doc now says only the address ranges are shared. The calendar fetcher still allows plain http, unwraps mapped/NAT64 IPv4 before applying the policy, and keeps its own redirect and size handling.

Checks: eslint, tsc --noEmit (exit 0), git diff --check, and the inbound-auth, stored-link and docs-migration-name suites (646 tests) all pass.

SEC-002 stays IN PROGRESS. The release, storage and operator holds (R1–R3) and the browser evidence gaps stay open. No production configuration or live external probes.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Follow-on to6092751883 at4fe0c9b: the original unsafe href/concatenation cases and documentation are source-corrected. One bounded Low test-assurance residual remains in constInitializer: a nearer parameter, let/var or destructured binding can be skipped in favor of an outer safe const with the same name. Add controls combining that outer safe const with each unsafe inner shadow, and resolve the actual lexical binding or stop conservatively at any nearer binding. This concerns the ratchet's detection guarantee; no unsafe changed Social Feed runtime path was found. Keep guard-import identity outside any claim of complete semantic provenance unless verified. No additional source/security scope or live probes requested.

…s to

Owner follow-on on #983: constInitializer looked only for const
declarations, so a nearer parameter, let, var or destructured binding was
skipped in favour of an outer safe const of the same name. The ratchet now
resolves the nearest scope that declares the name (parameters, let/const,
hoisted var, destructuring, catch and loop bindings, imports) and accepts an
identifier only when that binding is a plain const declared before the use
with a safe initializer. A guard call counts only when its name is bound by
an import of that name from @/lib/utils/safe-link or @/lib/social/links.

New controls: an outer safe const shadowed by a parameter, arrow parameter,
destructured parameter, inner let, hoisted var, destructured const, catch or
for-of binding is refused; a const used before declaration is refused; a
local look-alike, unimported, other-module, renamed or parameter-shadowed
guard is refused. 8 of the shadowing controls and the guard control fail on
the previous resolver; 38/38 pass now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4
…t rule

@next/next/no-assign-module-variable refuses a local named `module`; the
guard's source module is now `source`. No behaviour change: 38/38 still pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4

Copy link
Copy Markdown
Owner Author

Re 6092846875: the binding fix is in d969d25. b7ff7fd renames a local the Next lint rule refuses (module → source), with no behaviour change.

Resolution. An identifier now resolves to the nearest enclosing scope that declares its name. Those scopes are parameters, let/const, hoisted var (outside nested functions), destructuring, catch and for bindings, and imports. The identifier is accepted only if that binding is a plain const name = <safe> declared before the use. Any other nearer binding stops the search, and so does more than one binding in the scope.

Guard identity. A safeWebLink/safeSocialLink call now counts only if the name is bound by an import of that same name from @/lib/utils/safe-link or @/lib/social/links, respectively. That is the extent of the claim. The ratchet checks import identity, not what those modules do; that is covered by tests/a-stored-link-is-a-web-link.test.ts.

New controls, outer safe const href = safeSocialLink(p) plus an inner shadow. Each of these is refused:

  • a parameter
  • an arrow parameter
  • a destructured parameter
  • an inner let
  • a hoisted var declared after the call
  • an inner destructured const
  • an inner const bound to a stored field
  • a catch binding
  • a for-of binding

These are refused too:

  • a const used before its declaration
  • a guard that is not imported
  • a local look-alike guard
  • a guard imported from another module
  • a guard renamed on import
  • a guard shadowed by a parameter

Accepted: the unshadowed outer const, and an inner safe const over an outer unsafe one.

Against the previous resolver. I ran the same controls against the resolver from 4fe0c9b. Eight of the shadow cases and the guard-identity case fail there; the inner-stored-field const was already handled. On this head all 38 tests pass, including the repo-wide check.

Checks:

  • eslint passes on every changed .ts/.tsx file in the branch.
  • tsc --noEmit exits 0.
  • git diff --check is clean.

The Social Feed runtime path is unchanged. SEC-002 stays IN PROGRESS with R1–R3 and the browser gaps open.


Generated by Claude Code

@NewWorldVenture
NewWorldVenture marked this pull request as ready for review October 10, 2026 12:39
@NewWorldVenture
NewWorldVenture merged commit 66f7def into main Oct 10, 2026
9 checks passed
NewWorldVenture pushed a commit that referenced this pull request Oct 10, 2026
…integration

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KP9rt5rVQ9jDMpZp2xBy3K
NewWorldVenture pushed a commit that referenced this pull request Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants