Skip to content

Weekly Security Review: May 07 – May 15, 2026 #1337

Description

@timothyfroehlich

PRs reviewed: #1270, #1287, #1291, #1296, #1297, #1298, #1299, #1300, #1302, #1303, #1304, #1305, #1306, #1307, #1308, #1309, #1310, #1311, #1312, #1313, #1314, #1315, #1317, #1320, #1322, #1323, #1327, #1328 (28 total)

Substantive diffs reviewed: #1270, #1306, #1307, #1308, #1309, #1322, #1323, #1328

Verdict: All clear — no violations. Three proactive security hardening PRs and two CVE patches make this an above-average security week.


Non-Negotiable Checklist

Status Rule Note
CORE-SEC-001 Auth on all actions #1308 retroactively added checkPermission() to toggleWatcherAction, markAsReadAction, markAllAsReadAction — all three were previously missing permission enforcement. All other new code (page.tsx in #1270) gates on auth.getUser().
CORE-SEC-002 Input validation No new unvalidated inputs. issueNumber URL param in #1270's page.tsx is validated via parseInt + isNaN check (pre-existing pattern, unchanged).
CORE-SEC-003/004 CSP / nonces No middleware or CSP changes this week.
CORE-SEC-005 No hardcoded hostnames No hardcoded hosts introduced.
CORE-SEC-006 Minimal data at server→client boundary #1270's three new "use client" components receive only narrow scalars (issueId: string, value: Date|string, fallback?: string). Bonus improvement: PR #1270 also fixed a pre-existing over-serialization bug — non-triage users previously received the full member roster (allUsers) as RSC payload even though they couldn't use the assignee picker. Now gated on canTriage; non-triage viewers receive only the assigned user's name + ID.
CORE-SEC-007 Email addresses never displayed #1270 confirmed: reporter display resolves name → invitedReporter.name → reporterName → "Anonymous" — never reporterEmail. The "email-only reporter" E2E spec asserts "Anonymous" is displayed.
CORE-SSR-001/002 Supabase SSR patterns #1270's page.tsx: createClient() → immediate auth.getUser() destructure, no intervening logic. #1309 corrected the URL fallback order in admin.ts to prefer the server-side SUPABASE_URL over the client-exposed NEXT_PUBLIC_SUPABASE_URL.
CORE-SSR-003/004 Middleware + auth callback No changes.
CORE-SSR-007 No direct auth.users queries None introduced.
CORE-ARCH-008 Permissions matrix matches enforcement #1308 updated matrix.ts alongside the server action changes, adding the notifications category with notifications.manage_own. Matrix and enforcement are in sync.

Broader Analysis

Positive security improvements worth calling out

#1306 — Session revocation on account delete (src/app/(app)/settings/actions.ts)
Before this PR, deleteUser() did not invalidate existing JWTs — a deleted account remained accessible until token expiry (Supabase's configured TTL, typically 1 hour). Now adminClient.auth.admin.signOut(userId, "global") is called before deleteUser(), revoking all sessions first. Tests verify ordering (signOut call order < deleteUser call order). Best-effort error handling correctly logs failures but still proceeds with deletion since data is already anonymized. The normal logout path stays 'local' scope intentionally — multi-device sessions are unaffected by a logout on one device.

#1307 — HMAC signing key decoupled from service-role key (src/lib/notifications/channels/email-channel.ts)
Unsubscribe tokens were previously HMAC-signed with SUPABASE_SERVICE_ROLE_KEY. This created a hidden coupling: Supabase key rotation would silently invalidate all outstanding unsubscribe links (a CAN-SPAM compliance risk). Now uses a dedicated UNSUBSCRIBE_SIGNING_SECRET env var. Build fails fast on Vercel if the secret is missing in production (next.config.ts assertion). CI uses a test-only placeholder that is clearly not a real secret.

#1309handle_new_user() EXECUTE revoked from anon/authenticated (drizzle/0035_lock_handle_new_user.sql)
The trigger function was SECURITY DEFINER but not explicitly restricted. Any authenticated or anon role could RPC-invoke it directly, which serves no legitimate purpose. Migration 0035 applies REVOKE ALL ON FUNCTION public.handle_new_user() FROM PUBLIC and FROM anon, authenticated. The trigger itself continues to fire normally (triggers run as the definer regardless of caller privileges). Follows the same pattern as migration 0029 for get_discord_config().

CVE status — #1323

  • postcss CVE-2026-41305 (XSS via unescaped < in CSS stringify output): Patched. Override postcss >=8.5.10 added to package.json. Transitive via next@16.2.6.
  • sanitize-html GHSA-rpr9-rxv7-x643 (CVSS 9.3, <xmp> raw-text passthrough): Mitigated, not yet patched — no upstream fix exists. Risk is low: all three callsites (src/lib/markdown.ts, src/lib/tiptap/render.ts, src/lib/notifications/channels/email-channel.ts) use explicit positive allowedTags lists that do not admit <xmp>. Defensive hardening applied: all three configs now extend nonTextTags with xmp, noscript, noembed, noframes so content of those elements is dropped (not re-surfaced as text) when the tag is stripped. Tracked for upstream patch removal in PP-3ds.

Dependency bump — #1322

17 packages bumped. Security-relevant:

  • @supabase/supabase-js 2.104.1 → 2.105.1: passkey/WebAuthn support (additive). Two correctness fixes (PostgREST query regression, realtime error surfacing). No CVEs.
  • zod 4.3.6 → 4.4.1: stricter tuple parsing in 4.4.0. Verified: zero z.tuple() calls in src/ — no impact.
  • nodemailer 8.0.6 → 8.0.7: Unicode domain handling fix. Low practical risk for this user base.

Skipped (non-substantive)


Recommendations

  1. (P2, open) sanitize-html GHSA-rpr9-rxv7-x643: Remove the ignoreGhsas entry in package.json once an upstream patch ships. Already tracked as PP-3ds.

  2. (P3, observation) The warnedMissingSecret module-level boolean in email-channel.ts (chore(notifications): decouple unsubscribe HMAC from service-role key (PP-7xt) #1307) deduplicates the missing-secret log within a single Node process lifetime. In serverless/edge environments the module reloads on each cold start, so the warning can fire more than once under load. This is purely observability — not a security issue — but worth knowing when reading production logs.

  3. (Positive pattern to reinforce) fix(perms): add checkPermission to notifications + watcher actions (PP-907) #1308 correctly updated matrix.ts alongside the server action changes (CORE-ARCH-008). This is the right workflow — enumerate it as a checklist item in PR template or Copilot review instructions to keep the matrix from drifting on future permission PRs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions