Appearance
Security review — pre-stakeholder-exposure round (2026-07)
Requirement: #570 (p1-critical). A security-review round before the POC is reachable on the public internet (behind Cloudflare Access) and handling session and lead data that flows to Eloqua.
Scope of the build reviewed: the local main build at review time. The review covers authentication/authorization, data handling & privacy, and the standard web checks (injection, XSS, CSRF, SSRF, secret handling, rate-limiting). The perimeter design is reviewed here; verifying the live Cloudflare Access perimeter is the one part gated on the hosting requirement (#560) being provisioned.
How to read this doc: each section states what was reviewed, what is sound, and the findings. Findings are tracked as GitHub issues (labelled security); the triage table at the end lists them by severity. Criticals must be resolved before public exposure.
1. Authentication & authorization
What was reviewed
- The public vs. admin surface split.
- The Cloudflare Access perimeter design (
src/lib/server/auth/access.ts,jwt.ts,roles.ts). - App-level admin authorization: the request hook, the
/adminlayout guard, the section capability guard, and every admin mutation surface (form actions and+server.tsendpoints).
What is sound
Authorization is always decided server-side, in one place. The request hook (src/hooks.server.ts) resolves the admin decision once per /admin request into event.locals.admin — a discriminated union (ok / unauthenticated / forbidden). The UI never gates on its own; it reads the server decision.
The Access JWT is verified, not trusted. verifyAccessJwt (auth/jwt.ts) checks the RS256 signature against the team's JWKS via Web Crypto, then validates aud, iss, exp, and nbf with a bounded clock skew. A forged or tampered token fails signature verification; a token minted for another Access application fails the aud check. JWKS are fetched from https://<team>/cdn-cgi/access/certs and cached with a TTL. No JWT library is trusted — the verification is first-party and unit-tested.
The dev bypass cannot leak into production. isDevBypassEnabled requires BOTH an explicit ADMIN_DEV_BYPASS==='true' AND the absence of a configured CF_ACCESS_TEAM_DOMAIN. In the built Worker dev is statically false, so the hook never auto-enables the bypass in production, and in production the team domain is set — so even a leaked flag can't silently disable auth. With no team domain and no bypass, resolveAdminAuth returns unauthenticated for everyone: the admin fails closed.
Capability enforcement is complete on writes, not just reads. Roles map to capabilities (edit_data, edit_rules, view_audit, manage_users); every check asks "does this role hold this capability?", never "is this a specific role". Verified across the whole admin surface:
- Every
+page.server.tsundersrc/routes/admin/that exportsactionscallsrequireCapability(...)inside each action (0 files missing the guard). - Every admin
+server.tsendpoint (analytics/export,drupal-sync/chunk, and all fivemedia/*endpoints) callsrequireCapability(...)itself. This matters because SvelteKit+server.tsendpoints do not run the+layout.server.tsguard — they are guarded individually, and they are. manage_usersisadmin-only, so an editor role can never grant access to itself or others.- The
/admin/[section]catch-all resolves the section's required capability and 403s a role that lacks it.
Public routes that surface an admin affordance re-resolve the decision. Because locals.admin is only populated for /admin paths, a public route that shows an "edit in admin" link uses isRequestAdmin (auth/admin-request.ts), which is cheap for buyers — with no Access token present, resolveAdminAuth returns unauthenticated before any DB read, so only a signed-in admin pays the full verification cost.
Findings
F-A1 (medium) — workers_dev and preview_urls are enabled, exposing the app outside the Access-bound hostname. wrangler.jsonc sets "workers_dev": true and "preview_urls": true. A Cloudflare Access policy is bound to a hostname; the *.workers.dev origin and per-deploy preview_urls are separate hostnames that an Access policy on the custom .nanawall.com subdomain does not cover. The admin still fails closed there (no CF_ACCESS_TEAM_DOMAIN → unauthenticated), so this is not an auth bypass, but it does mean the whole app — including a future state where prod vars are present — is reachable at a guessable workers.dev URL outside the intended perimeter. Recommendation: disable workers_dev (and ideally preview_urls) for the production deployment, or ensure the Access application covers those hostnames too. Aligns with the "prod must be a .nanawall.com subdomain" constraint. Verification of the live perimeter is gated on #560.
F-A2 (low, informational) — read-only loads on a few admin pages rely on the layout guard and may execute their query before the 401. admin/+page.server.ts, admin/[section]/+page.server.ts, admin/pages/+page.server.ts, and admin/categories/+page.server.ts have read-only loads with no per-route requireCapability. SvelteKit runs layout and leaf loads concurrently, so these read queries can run for an unauthenticated request — but the layout's 401/403 prevents the data from reaching the response, and nothing mutates. The capability-sensitive sections (audit, leads, analytics, media) already add their own load guards. Recommendation: optionally add a per-load requireCapability to these for defence-in-depth; not required before exposure.
F-A3 → re-graded as F-D1 (medium). The dev-bypass config risk was initially noted here as low/informational; on closer analysis (the committed config ships an empty CF_ACCESS_TEAM_DOMAIN) it is re-graded to medium and tracked as F-D1 in §2, resolved by the deploy gate described there.
Note (by design, not a finding): privacy erasure actions (GDPR delete of buyer sessions/leads) are gated as edit_data, so a data_editor can erase buyer data. This is the declared design (privacy lives in the data half), recorded for awareness.
2. Data handling & privacy
What was reviewed
- Anonymous session identity (
ns_sessioncookie) and the session store (src/lib/server/sessions.ts). - Consent model and gating (
src/lib/server/consent.ts), retention/purge (SESSION_TTL_DAYS,workers/session-purge/). - Lead PII capture and the Eloqua push (
eloqua-lead-sync.ts,eloqua-retry.ts,eloqua-visitor-recognition.ts,integrations/eloqua.ts). - D1 access patterns across
src/lib/server/**(injection surface, PII exposure).
What is sound
The session store is PII-free by construction, not by convention.sanitizeSessionPayload (sessions.ts) rejects non-objects, drops unknown keys, and validates every id against ID_PATTERN (^[a-zA-Z0-9][a-zA-Z0-9_-]{0,99}$) before anything reaches D1. Free text and emails simply cannot enter the store. Payloads are bounded (MAX_ANSWERS 100, MAX_SYSTEMS 50), and keys are copied entry-by-entry into fresh object literals — no untrusted spread/merge, so prototype-pollution has no path. The purge logic (isSessionExpired, purgeExpiredSessions) is idempotent and audit-logged.
Consent genuinely fails closed. An unconfigured gate, a malformed cookie, an absent purpose, or any parse error all resolve to NO_CONSENT. Measurement and marketing capture happen only when the corresponding tier is granted; the dev-only grantAll override is gated on the same compile-time dev constant used elsewhere, so it cannot enable capture in the built Worker.
Eloqua credentials are secret-sourced and never logged. Credentials come exclusively from Worker env (EloquaEnv), are never persisted to D1, and are never logged. Client methods return typed results rather than throwing, and search terms are quote-stripped before interpolation into Eloqua's REST search grammar.
Lead PII in the retry queue is scrubbed on success and purged on a schedule.scrubQueuePii clears PII the instant a queued sync succeeds; terminal rows are purged on a bounded retention window (SYNC_QUEUE_RETENTION_DAYS); the admin drill-down (listRetryQueueItems / maskEmail) never surfaces a raw email. The admin lead view (lead-intelligence-read.ts) is keyed entirely by opaque Eloqua contactId — no email/name ever touches selection_sessions or the read path.
D1 access is uniformly parameterized. Every query uses Drizzle's query builder or sql tagged templates; all interpolated values become bind parameters, including inside the larger raw aggregate queries in analytics/rollup.ts / event-rollup.ts. No string-concatenated SQL exists anywhere in src/lib/server. No PII column is exposed through any public (non-admin) endpoint.
Findings
F-D1 (medium, config-dependent) — the admin fail-closed guarantee currently rests on CF_ACCESS_TEAM_DOMAIN being unset, which the committed config ships as empty.isDevBypassEnabled is ADMIN_DEV_BYPASS==='true' && !CF_ACCESS_TEAM_DOMAIN. The code comment asserts "in production the team domain is always set", but the committed wrangler.jsonc ships CF_ACCESS_TEAM_DOMAIN: "" (pending Access setup). As shipped today, if ADMIN_DEV_BYPASS=true were ever present in the production env (copied from .dev.vars, left in a deploy script) while the team domain is still empty, the entire /admin area would open to anyone as admin@test.local with no Access check at all. The code logic is correct given its assumption; the risk is that the assumption isn't true until #560/#898 provision the Access app.
Resolution (deploy gate, not a code change). The structural invariant already exists: isDevBypassEnabled returns false whenever CF_ACCESS_TEAM_DOMAIN is set — proven by the existing test (access.spec.ts:81, "refused in production even if the flag leaks"). So the fix is to guarantee the team domain is set in production, not to change the code. A tempting code hardening — additionally requiring the build-time dev flag — was considered and rejected: the dev-bypass is deliberately usable under wrangler dev/preview (where the Vite dev flag is false but Cloudflare Access is not yet in front), and that guard would break the legitimate local-preview path. The correct control is therefore the deploy checklist for #560/#898: set CF_ACCESS_TEAM_DOMAIN (and CF_ACCESS_AUD) as real production values before go-live, and never set ADMIN_DEV_BYPASS in the production environment. Tracked as a security issue. This supersedes the earlier F-A3 framing.
F-D2 (low) — CSV export has no formula-injection guard. csvField() (src/lib/server/csv.ts) implements RFC 4180 quoting but does not neutralize leading =, +, -, @ (Excel/Sheets formula triggers). Not currently exploitable — the only consumer (analytics/session-export.ts) emits system-controlled ids/timestamps/booleans, never free text. Flagged so a future export reusing toCsv() with ingested/free-text content (e.g. media titles) does not silently inherit the gap.
F-D3 (informational) — the Iubenda consent gate ships unconfigured.IUBENDA_SITE_ID / IUBENDA_PURPOSE_* are empty in the committed config; consentConfigFromEnv correctly resolves to null and the gate fails closed (capture nothing). Not a vulnerability — but confirm these are populated before go-live, or analytics/lead-sync silently capture nothing in production.
(Rate-limiting is a cross-cutting standard web check — see §3, F-W*.)
3. Standard web checks
What was reviewed
Injection, XSS, CSRF, SSRF (via the Drupal/JSON/corpus fetches), secret handling in the Worker, and rate-limiting/abuse protection — across src/**, wrangler.jsonc, and (to confirm framework defaults) the SvelteKit runtime.
What is sound
Injection — none found. Every D1 query uses Drizzle's query builder or sql tagged templates, so all interpolated values become bind parameters, including the raw aggregate queries in analytics/rollup.ts / event-rollup.ts. LIKE searches escape %/_ via escapeLikePattern (admin/media.ts). The Eloqua client strips quote characters from search terms before building that vendor's search grammar. No eval, no new Function, no command execution anywhere in src/.
XSS — the two dynamic-injection sites are both constrained. There is no innerHTML/{@html} with untrusted content in src/. The two places that interpolate into markup are (1) the theme/layout %ns.theme% / %ns.layout% chunk replacement in hooks.server.ts, where both values are resolved through resolveTheme/resolveLayout allowlists (a bad settings row falls back to the default, never reaches the document raw), and (2) the Eloqua tracking snippet (eloqua-tracking.ts), which is regex-constrained. Video embeds (media/VideoEmbed.svelte) are rebuilt server-side from regex-validated provider IDs, never passed through raw.
CSRF — protected, contrary to the common SvelteKit assumption. This was traced into the SvelteKit runtime (@sveltejs/kit/src/runtime/server/respond.js) by two independent reviewers. csrf.checkOrigin defaults to on and is not disabled in this project (no svelte.config.js; the sveltekit() Vite-plugin config carries no csrf override). It rejects any cross-origin POST/PUT/PATCH/DELETE whose Content-Type is a CORS-"simple" type (x-www-form-urlencoded, multipart/form-data, text/plain) — the exact vector that would otherwise bypass preflight — and this applies to raw +server.ts endpoints, not just form actions. Cross-origin application/json requests are separately blocked by the browser's CORS preflight (the app returns no permissive Access-Control-Allow-Origin). Cookies are SameSite=Lax. So both the simple-request and JSON-preflight CSRF vectors are covered. No action needed — recorded so it is not re-litigated as a false positive. (Residual: if the origin check were ever disabled, the JSON handlers would be exposed; the worst a successful CSRF could do is write the victim's own session, no cross-account effect.)
Secret handling — clean. Secrets (Eloqua creds, CF_ACCESS_AUD) are read only from Worker env / .dev.vars (gitignored and untracked; only *.example files are committed, containing no real credentials). No secret is logged or serialized into a load return value / page data. No PUBLIC_-prefixed leak of a private value.
Findings
F-W1 (medium) — no rate-limiting or abuse protection on public write endpoints. POST /api/lead, PUT /api/session, and POST /api/session/events have no application-level throttle, and wrangler.jsonc declares no Cloudflare Rate Limiting rule. client-classification.ts only labels traffic — it never blocks. Failure scenario: a scripted client rotating a fresh ns_session cookie per request can hammer POST /api/lead to mass-create/update Eloqua contacts (API quota exhaustion, polluted CRM data), or flood session/events to fill D1 with junk rows — at zero attacker cost, with the endpoint always returning {ok:true} (by design), giving no negative signal to throttle on. This is the single most material pre-exposure gap. Recommendation: add a Cloudflare edge Rate Limiting rule (or the Workers Rate Limiting binding) in front of /api/lead at minimum, and/or a Turnstile challenge on the lead form. Best delivered alongside the hosting/WAF work (#560). Filed as a security requirement.
F-W2 (low–medium, amplifier of F-W1) — the consent gate is entirely client-asserted. readRequestConsent (consent.ts) trusts the _iub_cs-<siteId> cookie verbatim; it is not cryptographically bound, so a scripted client can set a "marketing granted" cookie and unlock the marketing tier. Failure scenario: combined with F-W1, an attacker self-grants marketing consent and drives POST /api/lead to create real Eloqua contacts for arbitrary emails at volume. This is an inherent property of client-readable CMPs (the gate defends against accidental capture by legitimate browsers, not a scripted abuser); the real control is the F-W1 rate-limit. Recorded on the F-W1 issue.
F-W3 (low) — blind SSRF via the corpus source-URL liveness prober, gated behind admin auth. checkUrlLiveness (corpus-liveness.ts) is invoked with an admin-supplied sourceUrl (corpus-ingest.ts, corpus.ts) with no host/protocol allowlist beyond new URL() parsing. Failure scenario: an authenticated edit_data admin (or an attacker who has compromised such an account) can point sourceUrl at an internal/metadata host and get a HEAD/GET issued from the Worker, with the ok/status outcome reflected into the corpus item — a blind SSRF oracle. Severity is low (requires admin capability; the response body is never echoed, only ok/dead + HTTP status). Recommendation: constrain the target to http/https with a denylist for loopback/link-local/metadata ranges as defence-in-depth. Filed as a security issue.
4. Triage & conclusion
No critical or high findings. AC-04 ("criticals resolved before exposure") is satisfied by absence — there are no criticals. The codebase is well-defended: the load-bearing controls (server-side-only authZ, verified Access JWT, PII-free-by- construction session store, fail-closed consent, uniformly parameterized D1, and SvelteKit's origin CSRF check) all hold.
| ID | Severity | Area | Summary | Resolution / owner |
|---|---|---|---|---|
| F-W1 | medium | rate-limiting | No throttle on public write endpoints → Eloqua/D1 abuse | Edge Rate Limiting rule / Turnstile — with #560 |
| F-D1 | medium | auth / deploy | Admin fail-closed rests on empty CF_ACCESS_TEAM_DOMAIN | Deploy gate: set team domain in prod — #560/#898 |
| F-A1 | medium | perimeter | workers_dev/preview_urls sit outside the Access hostname | Disable for prod or cover with Access — #560 |
| F-W2 | low–medium | consent | Consent cookie client-asserted; amplifies F-W1 | Mitigated by F-W1 rate-limit |
| F-W3 | low | SSRF | Admin-gated blind SSRF in corpus liveness prober | Host/protocol allowlist (defence-in-depth) |
| F-D2 | low | data export | CSV export lacks formula-injection guard (not exploitable now) | Add guard before any free-text export reuses it |
| F-A2 | low / info | authZ | A few read-only admin loads rely on the layout guard | Optional per-load guard (defence-in-depth) |
| F-D3 | informational | consent config | Iubenda gate ships unconfigured (fails closed) | Populate Iubenda IDs before go-live |
Pre-exposure gate. Nothing here blocks the local build. The three items that must be closed as part of public exposure are all deploy-time and align with the hosting requirement (#560) and the Access provisioning (#898): F-D1 (team domain set), F-A1 (workers_dev off / Access coverage), and F-W1 (edge rate-limit).
Tracked issues
| ID | Issue | Priority |
|---|---|---|
| F-W1 / F-W2 | #1057 | p2-high |
| F-D1 | #1058 | p2-high |
| F-A1 | #1059 | p3-medium |
| F-W3 | #1060 | p4-low |
| F-D2 | #1061 | p4-low |
F-A2 and F-D3 are informational (defence-in-depth / config confirmation) and are recorded in this document rather than filed as separate issues.