🔐 Security & DevSecOps

📋 Summary

Most of the application's own server code is written with real care for security. Nearly every API route checks who is signing in and what they may touch before it uses the all-powerful database key. Share and upload links use long random tokens that expire and can be withdrawn, and emails, logs and upload paths show deliberate hardening. The weakness sits underneath that code, in the database's own permission rules. As written in the repository, those rules let any signed-in user change their own record, including the field that makes them an administrator. If the live database matches the repository, every other access control can be bypassed. Within a job, a project manager can make themselves the approver, which breaks separation of duties on sign-off. There is also a confirmed route by which a campaign editor can plant script that runs in an administrator's browser, an open redirect in the sign-in callback, and test-only API endpoints that are reachable in production builds. The web framework is pinned to a version with published critical advisories (covered in module 08). The CI pipeline has no security scanning, and the privacy and data-retention controls the team has itself written down as required are not yet built.

🎯 Rating

🔴 ●●○○○ 2 / 5 — Red. The application code is better than average, but one Critical, trivially reachable privilege escalation in the database policies, two High findings (stored XSS, self-granted approval rights) and an unpatched framework with critical advisories outweigh it. Most fixes are small (S effort), so this rating can move quickly once they are made.

🔍 Findings

F1. Authorisation model, as built

[Observed] There are two layers of authorisation:

1. Route layer. src/lib/api-auth.ts provides getUser, canAccessCampaign, canEditCampaign, canEditOpenCampaign, canEditArea, canApprove, canReviewCampaign and disciplineOf. They query users.role and campaign_members through the user's own client, so RLS applies, and they fail closed: a null row gives false (api-auth.ts:46-61, 120-141). Writes then go through the service-role client, which bypasses RLS (src/lib/supabase/service.ts). 67 of 83 route files use the service client.
2. Database layer. Row-level security (RLS) in supabase/baseline.sql: 44 enable row level security statements, 95 create policy statements. This layer covers direct PostgREST and Storage access from the browser, which works with the public anon key plus the user's JWT.

Every global "is admin" decision in the app reads users.role. In the route layer that happens in isStaff() and in 25 inline role === 'admin' checks. In RLS it happens through (select role from users where id = auth.uid()). The integrity of users.role therefore holds up the whole model. F2 shows it is not protected.

F2. CRITICAL: any signed-in user can make themselves an administrator (database policy)

[Observed] supabase/baseline.sql:1044-1046:

• Policy "Users can update own profile" on users is for update using (auth.uid() = id).
• It has no with check clause and no column restriction. The repository contains no revoke/grant statements (grep -rn "revoke\|grant " supabase/ finds only a comment in 092), and no trigger protects role (the only triggers on/near users are handle_new_user and timestamp triggers, baseline.sql:935-985).
• Migration 028 removed the only admin policy on users and noted that "Admin user-management writes happen via the service role client" (supabase/migrations/028_fix_users_rls_recursion.sql). It left the self-update policy unrestricted.

[Inferred] Supabase's default table privileges give the authenticated role UPDATE on public tables. Any account holder could therefore send a direct REST update to their own users row, setting role to admin (or discipline to traffic). They would need only the public anon key, which is shipped to every browser, and their own session. From then on every route treats them as staff. They could invite users (/api/admin/invite), delete users, read every campaign through the service client, and generate a password-recovery link for the protected owner account (F5). That would make the takeover permanent.

[Unavailable] Whether production matches the baseline. A dashboard change could have restricted this, and it should be checked against the live database (pg_policies, information_schema.column_privileges). The project's own test supabase/tests/040_rls.sql does not cover updates to users.

Related: handle_new_user() (baseline.sql:947) takes the new user's role from raw_user_meta_data->>'role'. That metadata is client-controlled whenever someone signs up through Supabase's public sign-up endpoint. [Unavailable] Whether public sign-ups are disabled in the Supabase project is a dashboard setting, and nothing in the repo records it (grep -rn signUp finds no hits). If they are enabled, anyone on the internet can create an admin account. If disabled, this is latent.

F3. Route authorisation audit (83 route files)

Method: for every src/app/api/**/route.ts, I recorded whether it calls getUser and which authorisation helper or role/token check runs before the first service-client use. Then I read each route that the pattern scan did not classify. Child-resource routes were checked for IDOR, meaning whether they confirm that the child id belongs to the campaign in the path or derive the campaign from the child row.

[Observed] Good IDOR discipline is the norm. Child ids are pinned to the path's campaign, for example .eq('id', unitId).eq('campaign_id', campaignId) in frames/card-writes.ts:35, and .eq('id', g.body.id).eq('campaign_id', g.id) in media-strategy, products, questions and wrapup. Comment and file routes derive the campaign from the object row before checking (review/files/[fileId]/comments/route.ts:21-23, unit-comments/[commentId]/route.ts:13-29). The supplier register route only accepts an adUnitId found among the token's own campaign units, and storage paths are validated with isReviewObject against the campaign id.

Routes with inadequate checks or IDOR exposure:

F4. Injection and output encoding

F5. Public, token and secret-gated surfaces

F6. Middleware (src/proxy.ts) and the Next.js 16.2.0 advisories

[Observed] proxy.ts:5-8 only refreshes the Supabase session cookie (lib/supabase/middleware.ts) and, in development, auto-signs-in a dev user. It enforces no authorisation. Pages check getUser() in (app)/layout.tsx:9-14 and routes check per handler. The several "Middleware / Proxy bypass" advisories against next@16.2.0 (/tmp/claude-0/dd/audit.txt) therefore have low direct relevance: getting past the proxy gains nothing. The advisories that do matter are the critical next/og ImageResponse RCE (used in src/app/icon.tsx and apple-icon.tsx, where the input is the admin-set logo URL, so F2 makes it attacker-reachable), the DoS and cache-poisoning advisories, and the Image Optimisation advisories (/_next/image is on by default even though next/image is not imported). See module 08 for upgrade guidance.

QA harness reachable in production. [Observed] When NEXT_PUBLIC_QA === '1' the proxy hands every request to harness(), which trusts an x-qa-user header for identity (proxy.ts:20-27). That flag is build-time, and production must never set it, but nothing in CI or config asserts that. Separately, the harness route handlers have no env gate: src/app/qa/api/[...path]/route.ts (exports at :451-456), qa/reset, qa/state, qa/demo/as and qa/demo/reset. The notFound() in src/app/qa/layout.tsx:11 covers pages only, because App Router layouts do not wrap route.ts handlers. In production the proxy calls updateSession (src/lib/supabase/middleware.ts), which refreshes cookies but does not require sign-in, so nothing in front of these handlers stops an anonymous request. [Inferred] These handlers are compiled into every production build and answer unauthenticated requests against the in-memory fixture store (src/app/qa/store.ts). They do not touch the Supabase database, so no real data is exposed, but this contradicts the documented claim that "The harness routes exist only when NEXT_PUBLIC_QA=1" (CLAUDE.md; store.ts:158). It also adds unnecessary attack surface, including unbounded in-memory writes per serverless instance (OWASP A05). Severity Low–Medium. [Unavailable] Confirming that they respond on the live deployment needs a request to it, which was deliberately not sent.

F7. Security headers and transport

[Observed] next.config.ts:36-55 sets X-Frame-Options: SAMEORIGIN, X-Content-Type-Options: nosniff, Referrer-Policy: strict-origin-when-cross-origin and a restrictive Permissions-Policy. There is deliberately no Content-Security-Policy (comment at :33-35). HSTS is said to come from Vercel [Unavailable — verify on live headers]. proxyClientMaxBodySize: '200mb' (:9) raises the per-request body ceiling, which increases DoS cost on serverless functions.

F8. Row-level security (sample review)

[Observed] The 040/061 hardening is folded in at the end of the baseline (baseline.sql:1885ff). It scopes approvals and utm_log, which earlier policies at :1155-1166 left open, and pins search_path on security-definer functions. supabase/tests/040_rls.sql with scripts/rls-check.sh is a genuine RLS regression test, which is uncommon and good.

These SELECT policies still read auth.uid() is not null, so any signed-in account, including global reviewers and external agency or supplier staff given accounts, can read every row:

This confirms CLAUDE.md's own admission ("any signed-in account can read them") and shows it reaches further than the brief and review workflow tables. Storage policies (baseline.sql:1640-1718) scope objects by first path folder = campaign id. Any accepted editor member can therefore read, write or delete every object in that campaign's folder directly, bypassing the discipline checks the routes apply. Objects under review/, concept/ and branding/ are admin-only via RLS and served through service-client routes.

F9. Secrets and configuration

• [Observed] gitleaks found no secrets in history or tracked files (brief). .gitignore ignores .env* except two examples, and the example values are placeholders.
• [Observed] SUPABASE_SERVICE_ROLE_KEY is read only via supabaseServiceRoleKey() (lib/supabase/env.ts). No 'use client' file imports createServiceClient, checked by scanning every .tsx importer. service.ts does not import 'server-only', so an accidental client import would fail at runtime rather than at build time.
• [Observed] NEXT_PUBLIC_* variables in use: SUPABASE_URL, SUPABASE_ANON_KEY, APP_URL, PRODUCT_NAME, UNIT_COMMENTS, QA, QA_TODAY. None is sensitive, since the anon key is public by design. The anon key's power, however, is exactly what F2 exploits.
• [Observed] src/lib/protected-accounts.ts falls back to owner@example.com / ghost@example.com when env is unset, so a deployment without the vars has no effective owner protection (documented).
• [Observed] Raw Supabase error messages go back to the browser in about 121 places in src/app/api (error: x.message). This leaks schema and constraint names (OWASP A05, Low).

F10. Security tooling in the pipeline

[Observed] .github/workflows/ci.yml runs typecheck, unit tests, a confidentiality scrub (scripts/scrub.js, which looks for client-identifying strings, not secrets), the harness contract, lint and build. There is no npm audit, Dependabot or Renovate, CodeQL or other SAST, secret scanning, or DAST. walk.yml is manual. The two gates that most help security are custom: write-results.test.ts (writes must read their result) and schema-columns.test.ts.

F11. Data protection (GDPR / POPIA)

🧾 Evidence

• Facts reused from the shared brief: npm audit (24 vulns; production 11; next@16.2.0 about 26 advisories including critical next/og RCE; xlsx unfixed), gitleaks clean, CI contents.
• Route classification script: for every find src/app/api -name route.ts, grep for exported handlers, getUser(, createServiceClient and the auth-helper names, followed by manual reads of the 30 routes the scan did not classify.
• Files read in full or in part: src/lib/api-auth.ts, src/lib/review-auth.ts, src/lib/supabase/{service,middleware,env,client}.ts, src/proxy.ts, next.config.ts, src/lib/share-links.ts, src/lib/brief.ts:147-195, src/lib/protected-accounts.ts, src/lib/activity.ts, src/lib/track.ts, src/app/auth/callback/route.ts, src/app/login/login-form.tsx, src/app/preview/[token]/page.tsx, src/app/qa/layout.tsx, src/app/qa/api/[...path]/route.ts, and the route files cited above.
• supabase/baseline.sql: tables at 116-125, 368-414, 625-645, 684-693; functions 905-950; policies 1036-1720; 040 section 1885ff. supabase/migrations/028_fix_users_rls_recursion.sql.
• Commands: grep -n "auth.uid() is not null" supabase/baseline.sql (60+ hits); grep -rn "revoke\|grant " supabase/ (none functional); grep -rn "\.rpc(\|dangerouslySetInnerHTML\|fetch(" src; grep -rln createServiceClient --include=*.tsx src with a 'use client' check (none); grep -rn "error: .*\.message" src/app/api | wc -l → 121.
• Not done, deliberately: no request was sent to any live deployment, and no exploit was attempted.

⚠️ Risks

✅ Recommendations