Home Explore Help
Register Sign In
claude
/
moviedice
1
0
Fork 0
Files Issues 0 Pull Requests 0 Wiki
Tree: 7ac26b11c7
Branches Tags
moviedice
/
research
/
PM_ASSESSMENT_R2.md

PM_ASSESSMENT_R2.md 45 KB
History Raw

PM Assessment — Round 2 Audit Findings

Prepared: 2026-04-05 Scope version reviewed: Post-Round-1 PROJECT_SCOPE.md Sources: TECHFILE.md (Second Review), SECFILE.md (Second Review), COMPLIANCE.md (Second Review) Status: Research/analysis only — PROJECT_SCOPE.md NOT modified

Summary

Priority Level Count
MUST DO (before code) 7
MUST DO (during Phase 1) 6
SHOULD DO 8
NICE TO HAVE 3
SKIP / DEFER 2
NEEDS DISCUSSION 4
Total actionable findings 30

Deduplicated findings: 30 unique items derived from 30 raw findings across the three reports. Three near-duplicates were merged (noted below). No conflicts between reports — the three audits are complementary and non-contradictory on all overlapping topics.

Conflicts between reports: None. SECFILE finding 24 and COMPLIANCE finding 36 cover the same issue (Supabase Studio exposure) and have been merged. SECFILE finding 27 and TECHFILE finding R7 both address Kong/Supabase network architecture and have been cross-referenced but are distinct enough to keep separate.

PRIORITY 1 — MUST DO (before writing any code)

These are items that will break the implementation if not resolved first, or that are architectural decisions that cannot be retrofitted cheaply.

Finding A1 — GoTrue Anonymous Sign-In Is Disabled by Default in Self-Hosted Supabase

Source: TECHFILE C1 Original Finding: Self-hosted GoTrue requires GOTRUE_EXTERNAL_ANONYMOUS_USERS_ENABLED=true in the docker-compose auth service environment. Without it, signInAnonymously() returns a 400 error. The Studio UI setting does not survive container recreation — the env var is the only durable configuration.

PM Assessment: This is a silent, total blocker for the entire onboarding flow. It takes five seconds to add the env var and zero seconds to forget it. The failure mode ("failed auth call") gives no indication the problem is configuration, not code — it will look like a bug and waste significant debugging time. This must be treated as a pre-flight checklist item for Phase 1.2 when the Supabase stack is first stood up.

Recommendation: MUST DO (before code)

Scope Impact: Add GOTRUE_EXTERNAL_ANONYMOUS_USERS_ENABLED=true to the env var list in §7 Deployment. Add a note to task 1.2 to verify this is set before Phase 1.4 work begins. Optionally extend the /api/health endpoint (task 1.7) to verify GoTrue reachability.

Finding A2 — Edge Functions Do Not Exist in the Default Self-Hosted Docker Stack

Source: TECHFILE C2 Original Finding: The standard Supabase self-hosted docker-compose does NOT include the supabase/edge-runtime (Deno) container. The scope's plan of pg_cron → net.http_post() → Edge Function will silently fail with 404s — the background jobs for landing reel refresh (task 5.2) and trailer URL refresh (task 6.2) will never run. Additionally, pg_cron calling net.http_post() also requires the pg_net extension, which is not mentioned in the scope.

PM Assessment: This is a critical architecture decision that affects two MVP deliverables (5.2 is in the MVP cutoff phase). The finding offers a clear simpler alternative: replace the Edge Function approach with a small Node.js cron container (node:22-alpine + node-cron) that calls TMDB and writes directly to Postgres via the service role key. That alternative eliminates the Deno runtime entirely, reduces operational complexity, and is easier to debug.

Given the April 26 MVP deadline, the Node.js cron container alternative is strongly preferable. Adding and correctly wiring the supabase/edge-runtime container is non-trivial and introduces a new runtime to operate. The Node.js cron container uses the same language and toolchain already in the project.

Recommendation: MUST DO (before code) — but this is a scope decision, not just an additive fix. The architecture of the background jobs must be decided before Phase 1.7.

Scope Impact: Either (a) replace "pg_cron + Edge Functions" throughout the scope with "Node.js cron container (node:22-alpine + node-cron)" and add pg_net to the extensions list — OR — (b) add the supabase/edge-runtime container to the docker-compose spec and add pg_net extension. Task 5.2, 6.2, and the §7 Deployment description all reference this. Recommend option (a).

Finding A3 — All Supabase Default Secrets Must Be Replaced Before First Deployment

Source: SECFILE 23 Original Finding: The official Supabase self-hosted .env ships with well-known placeholder values for JWT_SECRET, POSTGRES_PASSWORD, ANON_KEY, SERVICE_ROLE_KEY, DASHBOARD_USERNAME, and DASHBOARD_PASSWORD. A public JWT_SECRET means anyone can forge valid JWTs and bypass RLS entirely. The anon key and service role key are both derived from the JWT secret and must all be regenerated together as a lockstep set.

PM Assessment: Unambiguously critical. This is not theoretical — the defaults are published on GitHub. An instance deployed with default secrets is compromised before launch. The fix is a one-time 15-minute checklist during setup. The lockstep regeneration requirement (JWT_SECRET → regenerate both ANON_KEY and SERVICE_ROLE_KEY together) is the only complexity here, and forgetting it results in silent auth failures that look like application bugs.

Recommendation: MUST DO (before code) — specifically, before the first docker compose up in Phase 1.2.

Scope Impact: Add an explicit "Replace all default Supabase secrets" step to task 1.2 and/or task 1.7 with a checklist of the six values that must be replaced. Document the ANON_KEY + SERVICE_ROLE_KEY regeneration dependency on JWT_SECRET. Consider adding a startup check in the app that refuses to start if detected-default values are present.

Finding A4 — NEXT_PUBLIC_SUPABASE_URL Naming and Server vs. Client URL Split

Source: TECHFILE R5 (partially), TECHFILE R7 Original Finding (combined): Two related issues. First (R5): the scope lists SUPABASE_URL and SUPABASE_ANON_KEY as server-side env vars, but the browser-side Supabase client needs them with the NEXT_PUBLIC_ prefix. Only SUPABASE_SERVICE_ROLE_KEY must be server-side only. Second (R7): server-side Supabase calls should use an internal Docker network URL (http://supabase_kong:8000) rather than routing through Caddy, requiring a separate SUPABASE_INTERNAL_URL env var. These issues compound: if the env var naming is wrong, the browser client cannot initialize; if the URL strategy is wrong, server-side calls either fail or incur unnecessary latency.

PM Assessment: The naming fix (R5) is a pre-code correctness issue — getting it wrong means the entire auth flow breaks on first run with a confusing error. The internal URL split (R7) is an important architectural pattern for self-hosted deployments that is genuinely non-obvious. Both are zero-effort to fix in the scope now and expensive to discover during Phase 3 or 4 when server-rendered pages fail auth in non-obvious ways.

Recommendation: MUST DO (before code)

Scope Impact: In §7 Deployment env var list: rename SUPABASE_URL → NEXT_PUBLIC_SUPABASE_URL and SUPABASE_ANON_KEY → NEXT_PUBLIC_SUPABASE_ANON_KEY. Add SUPABASE_INTERNAL_URL as a new server-only env var (the Docker internal Kong URL). Note in §7 Tech Stack or Deployment that the Supabase server client uses SUPABASE_INTERNAL_URL + SUPABASE_SERVICE_ROLE_KEY, while the browser client uses NEXT_PUBLIC_SUPABASE_URL + NEXT_PUBLIC_SUPABASE_ANON_KEY. Also note using @supabase/ssr (createBrowserClient / createServerClient) rather than base @supabase/supabase-js for correct Next.js App Router session handling.

Finding A5 — t3-env Over Raw Zod for Env Validation (NEXTPUBLIC Structural Enforcement)

Source: TECHFILE R5 Original Finding: t3-env (@t3-oss/env-nextjs) structurally enforces the server/client split for env vars — server secrets in a server block, public vars in a client block — producing a build-time error if, for example, TMDB_API_KEY is placed in the wrong block. Raw zod requires enforcing this split by convention. The boilerplate is nearly identical.

PM Assessment: This is a build-time safety net for the project's most important security constraint: TMDB_API_KEY must never be NEXT_PUBLIC_. The structural enforcement costs nothing. It is directly relevant here because the scope already acknowledges the risk ("TMDB*API_KEY environment variable must NEVER use the NEXT_PUBLIC* prefix"). The scope already says "zod (or t3-env)" — this is just a recommendation to resolve that ambiguity in favor of t3-env.

Recommendation: MUST DO (before code) — resolve the "zod (or t3-env)" ambiguity in favor of t3-env in task 1.3.

Scope Impact: In §7 Tech Stack table and task 1.3, change "zod (or t3-env)" to "t3-env (@t3-oss/env-nextjs) with zod". Minor wording change, no structural scope impact.

Finding A6 — Supabase Studio Must Not Be Publicly Accessible

Source: SECFILE 24, COMPLIANCE 36 (merged — both flag the same issue) Original Finding: The self-hosted Supabase stack exposes Studio (full database admin UI) on port 3000 by default. Studio provides unrestricted read/write access to all data, bypassing RLS, using the service role key. If port 3000 is mapped to the host network or proxied through Caddy, it is internet-accessible. Default Studio credentials are well-known.

PM Assessment: This is a pre-deployment configuration requirement, not a code issue. The fix is removing or restricting a single port mapping in docker-compose. The operational cost (SSH tunnel to access Studio during development) is minimal and standard practice for self-hosted deployments. This should be a line item in the Phase 1.7 deployment checklist, not something left to developer judgment.

Recommendation: MUST DO (before code) — must be part of the docker-compose specification.

Scope Impact: Add to §7 Deployment: Studio port must not be mapped to the host network in the production docker-compose (remove or restrict to 127.0.0.1:3000). Access via SSH tunnel only. Optionally note this is acceptable during local development. Task 1.7 should include this as an explicit checklist item.

Finding A7 — Kong and Postgres Ports Must Not Be Internet-Accessible

Source: SECFILE 27, SECFILE 33 Original Finding (combined): Kong (Supabase API gateway, ports 8000/8443) and Postgres (port 5432) must not be exposed to the host network in production docker-compose. Kong exposed directly allows bypassing the Next.js app and probing Supabase APIs directly. Postgres exposed allows direct database connections that bypass all application security and RLS. Both are development conveniences that must be removed for production.

PM Assessment: Same category as A6 — docker-compose configuration, not code. Both are standard Docker hardening steps. The Kong exposure is particularly relevant because the Supabase JS browser client needs to reach Supabase services — the correct path is through Caddy, not direct Kong port exposure. This is already implied by the Caddy architecture but must be made explicit in the docker-compose specification.

Recommendation: MUST DO (before code) — part of the docker-compose specification.

Scope Impact: Add to §7 Deployment: Kong ports (8000, 8443) must be internal to the Docker network only — no host port mapping. Postgres port 5432 must be internal to Docker network only — no host port mapping. Access via SSH tunnel for administration. Task 1.7 should include these as checklist items.

PRIORITY 2 — MUST DO (during Phase 1 or specified phase)

These findings are not blocking to start coding, but must be addressed in the phase indicated or they become expensive to fix later.

Finding B1 — argon2 npm Package Requires Native Build Tools in Docker Multi-Stage Build

Source: TECHFILE R1 Original Finding: The argon2 npm package compiles a C extension at install time via node-gyp. The node:22-slim base image does not include python3, make, or gcc. These must be in the builder stage of the multi-stage Dockerfile or npm install will fail. The compiled .node binary must be verified to survive the standalone output trace; if not, it must be explicitly copied.

PM Assessment: Valid and practical. This will cause a hard Docker build failure if not addressed — not a subtle bug, a complete build failure. The fix is a two-line apt-get addition to the Dockerfile builder stage. It is a Phase 1.7 item by nature. The alternative (@node-rs/argon2, which ships pre-compiled NAPI binaries eliminating node-gyp entirely) is worth noting as a simpler option if build complexity becomes a concern.

Recommendation: MUST DO (during Phase 1.7)

Scope Impact: Add to task 1.7: "Dockerfile builder stage must install python3 make g++ for argon2 native build; verify argon2 compiled binary is present in standalone output." A note in §7 Tech Stack about @node-rs/argon2 as a drop-in alternative if node-gyp causes issues is optional.

Finding B2 — GoTrue Auth Methods Must Be Restricted to Anonymous-Only

Source: SECFILE 28 Original Finding: GoTrue enables multiple auth providers by default (email/password, magic link, OAuth). Since the app uses only anonymous sign-in, all other methods should be explicitly disabled via environment variables: GOTRUE_EXTERNAL_EMAIL_ENABLED=false, GOTRUE_EXTERNAL_PHONE_ENABLED=false, all OAuth providers disabled. This prevents attackers from creating non-anonymous accounts via unexpected endpoints, which could have different RLS treatment.

PM Assessment: Valid and low-effort. Disabling unused auth methods reduces attack surface with zero cost. This is a GoTrue environment variable configuration task that belongs in Phase 1.2 alongside the other Supabase setup work. The finding correctly notes this also addresses the anonymous account linking issue (finding 37 in SECFILE, an informational finding not separately listed here).

Recommendation: MUST DO (during Phase 1.2)

Scope Impact: Add to task 1.2: "Configure GoTrue to restrict auth methods to anonymous sign-in only: disable email, phone, and all OAuth providers via environment variables." Add the relevant env vars to the §7 Deployment env var list.

Finding B3 — RLS INSERT Policies Must Include WITH CHECK to Prevent Column Spoofing

Source: SECFILE 26 Original Finding: INSERT policies without explicit WITH CHECK clauses allow group members to insert movies attributed to other users (added_by spoofing) or to insert group_members records for other users (user_id spoofing). UPDATE policies must also prevent users from changing added_by or escalating their own role from member to admin.

PM Assessment: Valid. This is an RLS design gap that must be addressed when writing the migration in Phase 1.2. The scope defines the policies at a high level ("Full CRUD for members of the owning group only") but does not specify WITH CHECK constraints. Missing these is easy to overlook. Worth noting: the group_members INSERT (joining a group) is probably best handled as a server-side operation via the service role key rather than a direct client INSERT — this eliminates the WITH CHECK complexity for that specific case and is more consistent with the rate-limited join endpoint pattern already in the scope.

Recommendation: MUST DO (during Phase 1.2)

Scope Impact: Extend the RLS section in §7 to add WITH CHECK requirements: movies INSERT must check added_by = auth.uid(). movies UPDATE must prevent changing added_by. group_members: consider whether client-side INSERT is appropriate or if joining should be server-side via service role key. group_members UPDATE must prevent role escalation.

Finding B4 — Database Backup Strategy Missing

Source: COMPLIANCE 47 Original Finding: First-round finding flagged missing backups; the updated scope has self-hosted Supabase but still no backup mechanism. Self-hosted Postgres has no automatic backups. Volume failure = total data loss. Anonymous auth means users cannot be contacted to rebuild their accounts.

PM Assessment: Critical operational gap. The scope is otherwise well-specified for a self-hosted deployment, but a docker volume rm or disk failure without backups is a project-ending event. The fix is a simple pg_dump cron container added to docker-compose. This is a Phase 1.7 item — add it when the Docker infrastructure is being built, not as an afterthought post-launch.

Recommendation: MUST DO (during Phase 1.7)

Scope Impact: Add to §7 Deployment: a backup service in docker-compose running daily pg_dump with 7-day retention. Add to task 1.7: "Add pg_dump backup container to docker-compose; document restore procedure; test restore before launch."

Finding B5 — Caddy TLS Certificate Volume Must Be Persistent

Source: SECFILE 32 Original Finding: Caddy obtains TLS certificates via Let's Encrypt. If /data is not mounted as a persistent Docker volume, certificates are lost on every container restart. Let's Encrypt rate-limits duplicate certificate requests to 5 per domain per week — hitting this limit makes the site inaccessible for up to a week.

PM Assessment: Straightforward and important. This is a one-line volume mount in docker-compose. Missing it produces a production outage that takes a week to recover from. No reason not to specify it explicitly.

Recommendation: MUST DO (during Phase 1.7)

Scope Impact: Add to §7 Deployment or task 1.7: Caddy container must mount persistent named volumes for /data and /config. Use Let's Encrypt staging endpoint for initial testing.

Finding B6 — Argon2id Parameters Must Be Explicitly Specified

Source: SECFILE 31 Original Finding: The scope specifies Argon2id for recovery code hashing but does not define the memory, iterations, or parallelism parameters. The argon2 npm library default is 64 MiB memory per hash, which could cause OOM kills in constrained Docker containers if concurrent operations occur. OWASP 2024 recommended parameters: memory=19,456 KiB (~19 MiB), iterations=2, parallelism=1, output=32 bytes.

PM Assessment: Valid. The library default (64 MiB) is risky in a Docker container. Specifying OWASP-recommended parameters is a one-line change to the hash call and prevents unexpected OOM behavior. This belongs in Phase 1.5 when the recovery code hashing is implemented.

Recommendation: MUST DO (during Phase 1.5)

Scope Impact: Add to §7 Data Model or the Privacy section: Argon2id parameters — memory=19,456 KiB, iterations=2, parallelism=1, output=32 bytes. Add to task 1.5: "Use explicit Argon2id parameters (OWASP 2024 recommended) rather than library defaults."

PRIORITY 3 — SHOULD DO

Valid findings that improve correctness, security, or maintainability but are not project-ending if deferred briefly. These should all land before the MVP deadline.

Finding C1 — GDPR: auth.users Table Not Deleted in Account Deletion Flow

Source: COMPLIANCE 37 Original Finding: Deleting a row from the app's public.users table does NOT delete the corresponding auth.users record in GoTrue. Under GDPR Art. 17, erasure must be complete. Account deletion (both self-service in Extra Features and Master Admin deletion) must also call supabase.auth.admin.deleteUser(userId) server-side.

PM Assessment: Valid compliance requirement. This is a small but meaningful gap — a soft delete of the app table alone is incomplete. The fix is adding a server-side admin.deleteUser call to both deletion flows. Relevant to Phase 7.6 (Master Admin user deletion) and the Extra Features self-service deletion. Worth specifying now before those features are built.

Recommendation: SHOULD DO (during Phase 7.6 and Extra Features self-service deletion)

Scope Impact: Add to §7 or the privacy section: account deletion must call supabase.auth.admin.deleteUser(userId) via the service role key to remove the auth.users record. Add to task 7.6: "Deletion must remove both public.users row and auth.users record." Add the same note to the Extra Features self-service deletion entry.

Finding C2 — GDPR: Docker Container Log Rotation Not Configured

Source: COMPLIANCE 38 Original Finding: Docker's default json-file logging driver has no size limit or rotation. All Supabase containers (Kong, GoTrue, PostgREST, Realtime) produce logs containing IP addresses, user agents, and JWTs — personal data under GDPR. Logs grow unbounded. Privacy policy mentions server logs but does not address container logs.

PM Assessment: Valid and low-effort. logging: { driver: "json-file", options: { max-size: "10m", max-file: "5" } } added to each container in docker-compose is a five-minute fix that prevents both disk exhaustion and unaddressed GDPR retention. This belongs in Phase 1.7.

Recommendation: SHOULD DO (during Phase 1.7)

Scope Impact: Add to §7 Deployment: all containers must configure Docker log rotation (max-size: 10m, max-file: 5). Update the Privacy section to note Supabase container logs contain IP data and are retained for ~30 days via rotation.

Finding C3 — CSP Must Reflect Self-Hosted URLs, Not Managed Supabase Wildcard

Source: SECFILE 35 Original Finding: The scope's CSP example uses *.supabase.co for connect-src, which is the managed hosting pattern. Self-hosted Supabase routes through Caddy on the project's own domain. The CSP must use the actual deployment domain. If the Supabase API is proxied through Caddy, connect-src 'self' may be sufficient for API calls; only the Realtime WebSocket needs an explicit wss://yourdomain.com entry.

PM Assessment: Valid. The current CSP template is wrong for the deployment model. An incorrect CSP blocks Realtime WebSocket connections and breaks the real-time sync feature, which is an MVP requirement. This needs to be correct before task 5.7 (security headers configuration). Low effort to fix in the scope — remove *.supabase.co, replace with wss://[deployment domain].

Recommendation: SHOULD DO (during Phase 5.7)

Scope Impact: Update the §7 Security Headers section: replace *.supabase.co in connect-src with wss://[deployment domain]. Note that if Supabase API calls go through Caddy at the same origin, connect-src 'self' covers API; only the WebSocket URL needs an explicit entry.

Finding C4 — HSTS Should Be Configured in Caddy, Not Left Ambiguous

Source: COMPLIANCE 45 Original Finding: The scope says "configure HSTS in next.config.js or Caddy" — the "or" is ambiguous. HSTS must be configured at the TLS termination point (Caddy). The scope should be explicit. Caddy does not add HSTS by default. Recommendation: start with a short max-age during testing, increase to production values before launch.

PM Assessment: Valid. The ambiguity creates a risk that HSTS ends up only in Next.js headers, which works correctly for most requests but is less robust than having it at the Caddy level. More importantly, the "or" leaves room for neither being done. This is a small wording fix in the scope.

Recommendation: SHOULD DO (during Phase 5.7)

Scope Impact: Update §7 Security Headers: specify that HSTS is configured in the Caddyfile (not in Next.js). Add a note to use a short max-age during testing (e.g., max-age=86400) before promoting to the two-year production value. Do not submit to the HSTS preload list until confident.

Finding C5 — TMDB Adult Content Must Be Explicitly Filtered

Source: COMPLIANCE 35 Original Finding: TMDB search results and popular/top-rated endpoints can include adult content if include_adult is not explicitly set to false. The scope does not specify this parameter. The landing page reel poster fetch and search proxy both need explicit filtering. Defense in depth: also check the adult field on each result object server-side.

PM Assessment: Valid. This is a one-line parameter addition to all TMDB API calls in the server proxy. The consequences of not doing it (adult posters appearing in a social app used by families) are worse than the effort to fix it. Straightforward to add to task 3.1 / 5.2 proxy implementation.

Recommendation: SHOULD DO (during Phase 3.1 and 5.2)

Scope Impact: Add to §7 TMDB API Proxy: "All TMDB API calls must explicitly set include_adult=false and server-side filter results by the adult field." Add to task 3.1 and 5.2.

Finding C6 — Auto-Deletion Must Handle Orphan Groups and Admin Transfer

Source: SECFILE 29 Original Finding: The 12-month inactivity auto-deletion does not address what happens to groups when the sole admin is deleted (orphan admin-less group) or when the last member is deleted (orphan group with data). The added_by foreign key needs ON DELETE SET NULL to prevent FK violations. The deletion job should run in a transaction per user.

PM Assessment: Valid. The auto-deletion job as currently described could leave the database in inconsistent states. The logic is not complex — check group admin status before delete, auto-transfer or cascade-delete as appropriate — but it must be specified before implementation. This is relevant to the data retention job, which is a background task rather than a specific implementation phase.

Recommendation: SHOULD DO (specify before implementing the auto-deletion job, likely Phase 6 or later)

Scope Impact: Add to §7 Data Model or Privacy section: auto-deletion job must handle orphaned groups (auto-transfer admin to longest-tenured member; cascade-delete group if last member). added_by FK should use ON DELETE SET NULL. Deletion must be wrapped in a transaction per user.

Finding C7 — TMDB Metadata Staleness (TMDB ToS Compliance)

Source: COMPLIANCE 34 Original Finding: The movies table stores TMDB metadata at add-time and never refreshes it. The trailer URL refresh job only targets null entries. TMDB ToS requires keeping cached data reasonably current. Poster paths can become invalid. A monthly metadata refresh job for movies added more than 30 days ago is needed. Requires a metadata_refreshed_at column.

PM Assessment: This is a legitimate TMDB ToS compliance issue. However, it is a post-MVP background job, not an MVP feature. The scope already has a pattern for this (the trailer URL refresh job). Adding a metadata refresh job is a natural extension of that same pattern. It requires a schema column addition (metadata_refreshed_at) — that column is easier to add now (Phase 1.2 migration) than to retrofit later. The job itself can be implemented post-MVP.

Recommendation: SHOULD DO — add metadata_refreshed_at column to movies table in Phase 1.2 migration; defer the refresh job implementation to post-MVP (alongside or after Phase 6 trailer refresh).

Scope Impact: Add metadata_refreshed_at (timestamp, nullable) to the movies table in §7 Data Model. Add a post-MVP task (likely Phase 6) for a monthly TMDB metadata refresh job covering title, poster_path, genres, year.

Finding C8 — Invite Code Word List Requirements Not Specified

Source: SECFILE 30, COMPLIANCE 49 (both flag the same gap, slightly different focus — merged) Original Finding (combined): The scope specifies WORD-WORD format but does not specify word list size, case sensitivity, filtering criteria, or collision handling. SECFILE notes a minimum of 2,000 words for adequate brute-force resistance with the current rate limiting. COMPLIANCE notes that with a small word list (1,000 words = 1M combinations), brute force is feasible even with rate limiting. Both recommend 4,000+ words. Additional specifications needed: uppercase display, case-insensitive comparison, word length 3-8 characters, filtering for offensive/confusing words, collision check on generation.

PM Assessment: Valid. The word list decision should be explicit because it directly affects the user experience (longer words are harder to share verbally) and the security margin. With the rate limit at 5 attempts per 15-minute window, even 2,000 words gives adequate protection (brute force would take years). The filtering and collision check requirements are good hygiene to specify up front. This is a task 2.1 implementation concern, not a pre-code blocker, but it should be in the scope before Phase 2.

Recommendation: SHOULD DO (specify before Phase 2.1)

Scope Impact: Add to §7 Data Model or the groups section: word list must contain at least 2,000 words (4,000+ preferred); uppercase display, case-insensitive comparison; filter words shorter than 3 characters, longer than 8 characters, and offensive/confusing terms; collision check on generation. Rate limit window is 15 minutes (add to the existing rate-limit spec in §4).

PRIORITY 4 — NICE TO HAVE

Valid but not urgent. Acceptable to defer past MVP.

Finding D1 — iron-session v8 API Breakage Warning (Awareness Item)

Source: TECHFILE R2 Original Finding: iron-session v8 has a breaking API change from v7 — withIronSessionApiRoute and withIronSessionSsr exports no longer exist. Many tutorials still reference the v7 API. The v8 pattern uses getIronSession() directly in Route Handlers. Also: sameSite: 'strict' means the admin cookie is not sent on the first request following cross-site navigation (not a bug, expected behavior to document).

PM Assessment: This is an implementation awareness note, not a scope gap. The scope already correctly specifies iron-session with the right cookie settings. The PM value here is adding a note to Phase 7.3 to explicitly use the v8 README as the reference rather than tutorials. Not worth a scope change, but worth flagging before someone codes it.

Recommendation: NICE TO HAVE — add a note to task 7.3 to use the v8 README directly.

Scope Impact: Optional: add a parenthetical to task 7.3: "(use v8 README directly — v7 patterns are incompatible)."

Finding D2 — TanStack Query persistQueryClient Requires Three Additional Packages

Source: TECHFILE R3 Original Finding: persistQueryClient with IndexedDB requires @tanstack/react-query-persist-client, @tanstack/query-async-storage-persister, and idb-keyval — not a one-line config. Phase 8.2 should budget 2-4 hours. API changed between TanStack Query v4 and v5 (uses PersistQueryClientProvider wrapper in v5).

PM Assessment: Useful implementation detail for Phase 8.2, which is a post-MVP phase. The main value is preventing schedule underestimation. Not a scope change — the feature is already specified correctly. A time budget note on task 8.2 would be the only relevant addition.

Recommendation: NICE TO HAVE — add a time budget note to task 8.2.

Scope Impact: Optional: add to task 8.2: "Requires three packages (@tanstack/react-query-persist-client, @tanstack/query-async-storage-persister, idb-keyval); budget 2-4 hours including serialization testing."

Finding D3 — serwist Requires Custom sw.ts File and Separate tsconfig.worker.json

Source: TECHFILE R4 Original Finding: @serwist/next does not auto-generate a service worker — app/sw.ts must be authored manually. The TypeScript config requires a separate tsconfig.worker.json because the service worker needs lib: ['WebWorker'], which conflicts with the main app's lib: ['DOM']. Phase 8.1 should budget a half-day, not an hour.

PM Assessment: Useful implementation detail for Phase 8.1, which is post-MVP. The main value is preventing the schedule underestimation that @serwist/next is a "quick config step." The scope already correctly specifies the package — this is just a time-budget and implementation complexity note.

Recommendation: NICE TO HAVE — add a time budget note to task 8.1.

Scope Impact: Optional: add to task 8.1: "Requires authoring app/sw.ts, a tsconfig.worker.json for service worker TypeScript compilation, and public/manifest.json. Budget a half-day."

PRIORITY 5 — SKIP / DEFER

These findings are valid in theory but are overkill for this project's scale, threat model, or MVP timeline.

Finding E1 — Docker Container Security Hardening (CIS Benchmark: cap_drop, no-new-privileges, memory limits, read-only root filesystem)

Source: COMPLIANCE 44 Original Finding: CIS Docker Benchmark controls not addressed: no cap_drop: [ALL], no no-new-privileges, no memory/CPU limits, no read_only: true root filesystem.

PM Assessment: These are legitimate hardening measures for high-value production deployments. For a small self-hosted friend group app, they add meaningful operational complexity (particularly read_only: true, which requires identifying all writable paths and mounting tmpfs). The scope already has non-root user, tini, and health checks — these represent a reasonable baseline. The additional CIS controls would be appropriate for a security-sensitive business application but are overkill for this project's threat model and scale. The risk of getting read_only wrong and causing mysterious production failures outweighs the security benefit for this use case.

Recommendation: SKIP / DEFER — the existing Docker hardening (non-root user, tini, health check) is adequate. Revisit if the project scales significantly.

Scope Impact: None.

Finding E2 — Encryption at Rest for Postgres Volume

Source: COMPLIANCE 46 Original Finding: Docker volumes are not encrypted by default. GDPR Art. 32 lists encryption at rest as an example of "appropriate technical measures." Recommendation is host-level full-disk encryption (LUKS on Linux).

PM Assessment: Host-level full-disk encryption is a valid recommendation. However, this is an infrastructure/hosting decision that is entirely outside the scope of the application and docker-compose configuration. If the host is a cloud VM (DigitalOcean, Hetzner, etc.), the cloud provider likely offers disk encryption as a one-click option. A brief note in §7 Deployment as a deployment prerequisite is worth adding, but it is not a scope item and cannot be enforced via docker-compose. The data model already hashes recovery codes — the most sensitive field. Display names and movie preferences are low-sensitivity data. Full column-level encryption would be overkill.

Recommendation: SKIP / DEFER for scope purposes — optionally add a single line to §7 Deployment recommending host-level disk encryption as a deployment prerequisite.

Scope Impact: Optional single-line note in §7 Deployment: "Recommendation: enable full-disk encryption on the Docker host (LUKS or cloud provider equivalent) to protect Postgres volume data at rest."

PRIORITY 6 — NEEDS DISCUSSION

These findings require a product or architecture decision before a recommendation can be finalized.

Finding F1 — Supabase Auth Tokens Stored in localStorage (XSS Risk)

Source: SECFILE 25 Original Finding: The Supabase JS client stores JWTs (access token and refresh token) in localStorage by default. This is vulnerable to XSS exfiltration. Mitigation requires switching to @supabase/ssr with cookie-based token storage, reducing GoTrue refresh token lifetime, and prioritizing CSP as primary defense. Note: @supabase/ssr tokens in cookies are still not httpOnly (the JS client must read them) but provide SameSite/Secure benefits.

PM Assessment: This is a known Supabase design tradeoff. The @supabase/ssr recommendation aligns with Supabase's own guidance for Next.js App Router projects — this was actually already mentioned in Finding A4 (R7), where @supabase/ssr's createBrowserClient/createServerClient was recommended over base @supabase/supabase-js. If Finding A4 is accepted, @supabase/ssr is already in scope and this finding is partially addressed.

What remains as a decision: reducing GoTrue's refresh token lifetime (default ~1 week in self-hosted GoTrue). This is a GOTRUE_JWT_EXP and refresh token configuration change. A 24-hour refresh token lifetime is more conservative but means users need to re-authenticate more often — for an anonymous auth app where re-auth means starting from scratch or using a recovery code, this could frustrate users. The decision involves a UX/security tradeoff.

Recommendation: NEEDS DISCUSSION — if @supabase/ssr is adopted via Finding A4, the main residual question is GoTrue refresh token lifetime configuration. Recommend discussing the acceptable session duration before Phase 1.4.

Scope Impact (if accepted): Add GoTrue JWT and refresh token lifetime configuration to §7 Deployment env vars. Note that @supabase/ssr is the required Supabase client library (not base @supabase/supabase-js) — this may already be addressed by Finding A4.

Finding F2 — GDPR: Sentry May Capture Personal Data (International Transfer)

Source: COMPLIANCE 39 Original Finding: Sentry's default SDK captures request URLs (which may contain user UUIDs), breadcrumbs, and potentially user context. This constitutes a potential international data transfer (US servers) requiring disclosure and a beforeSend sanitization hook. Alternative: self-host Sentry.

PM Assessment: This is a real compliance concern but the practical fix is a beforeSend callback that strips UUID path segments from URLs — a 15-minute implementation task. The privacy policy update is also straightforward (Sentry is a common third-party processor with existing DPA/SCC documentation). Self-hosting Sentry adds significant operational overhead for a small project and is not worth considering. The open question is whether the user wants to configure Sentry with sanitization from the start (recommended) or defer the detailed privacy policy wording until Phase 5.1.

Recommendation: NEEDS DISCUSSION — the beforeSend sanitization should be added to task 1.8 (Sentry setup). The privacy policy wording is a Phase 5.1 item.

Scope Impact (if accepted): Add to task 1.8: "Configure Sentry beforeSend to strip UUID path segments from error events; do not call Sentry.setUser() with user identifiers." Add to Phase 5.1 privacy policy task: "Disclose Sentry as a third-party processor."

Finding F3 — GDPR: Privacy Policy Missing Required Sections

Source: COMPLIANCE 40 Original Finding: GDPR Arts. 13-14 and CCPA require specific sections not currently mentioned in the scope's privacy policy description: controller identity, lawful basis per processing activity, international transfers, right to complain to supervisory authority, CCPA categories/do-not-sell disclosure, children's data disclaimer (COPPA/GDPR under-16).

PM Assessment: The scope's current description ("factual description of what data is stored, how it is used, and user rights") is underspecified for compliance. The full list of required sections is well-defined by the finding. However, the practical question is: what jurisdiction is the operator in, and how strictly must GDPR/CCPA be followed for a small personal project? For a commercial product with EU users, full GDPR compliance is mandatory. For a personal project, a good-faith effort covers most practical risk.

This is also a Phase 5.1 item that will be written by the developer — specifying the required sections in the scope gives the developer a checklist rather than having them guess. Low risk to add this to the scope.

Recommendation: NEEDS DISCUSSION — likely SHOULD DO for any app with public access. Recommend expanding the §7 Privacy section to enumerate required policy sections as a checklist for the Phase 5.1 author.

Scope Impact (if accepted): Expand §7 Privacy to list required privacy policy sections: controller identity, lawful basis, data inventory with retention, third-party recipients (TMDB, Sentry), international transfer basis, full user rights with exercise instructions, children's disclaimer (under-13/under-16), cookie/localStorage disclosure, change notification procedure.

Finding F4 — WCAG Accessibility Gaps: Alt Text, Status Messages, Reel Pause Control

Source: COMPLIANCE 41, 42, 43 (three separate findings, combined here as they form a coherent accessibility cluster) Original Findings: (41) Slot machine reel must be strictly user-triggered and capped under 5 seconds per WCAG 2.2.2. (42) All poster <img> tags need meaningful alt text; spinning reel posters should be aria-hidden; result posters need alt text. (43) Dynamic status messages (roll result, filter state, watched toggle, action confirmations) need aria-live="polite" regions per WCAG 4.1.3.

PM Assessment: These are legitimate WCAG 2.1 AA requirements, and the scope already commits to accessibility (§6 Usability: "Accessibility: sufficient color contrast, tap targets, screen reader labels"). The inline panel keyboard navigation is already specified in detail — these three findings extend that pattern to the rest of the app. However, the scope currently defers the accessibility pass to Phase 8.3. The question is whether these specific items belong in Phase 8.3 or whether they should be embedded into the Phase 3 (poster grid) and Phase 4 (animation) implementation tasks as inline requirements, which would reduce the Phase 8.3 rework burden.

The reel pause control finding (41) is largely a design confirmation — since the reel is user-triggered and time-bounded, WCAG 2.2.2 compliance is easy. Alt text (42) and aria-live (43) are best addressed at component build time, not in a post-hoc accessibility pass.

Recommendation: NEEDS DISCUSSION — decide whether to embed these requirements into Phase 3/4 tasks or consolidate into Phase 8.3. Embedding is more efficient.

Scope Impact (if accepted): Option A: add alt text requirement to task 3.4 (poster grid), 4.3 (roll animation), 5.3 (landing reel); add aria-live requirement to tasks 4.2, 3.7, 3.8; confirm user-triggered and time-bounded constraint in task 5.3. Option B: expand task 8.3 to include these as explicit line items. Either option is a clarifying addition, not a new feature.

Cross-Report Notes

Confirmed Confirmations (No Action Required):

  • TECHFILE R6 (tini): Confirms tini is correct for Node.js 22 Docker containers and should not be removed. Scope is already right. No change needed.
  • TECHFILE R8 (Vitest/Playwright scoping): Vitest cannot test React Server Components — Vitest tests should be scoped to pure utilities and Client Components. Playwright should run against the full Docker stack, not next dev. This is an implementation guidance note. If accepted, add a clarifying note to tasks 1.1 and 9.16, but it does not change any feature or deliverable.
  • SECFILE 36 (pg_cron superuser): Keep pg_cron job SQL static and simple. Use it only to trigger HTTP calls, not complex data operations. This is a best practice note consistent with the current scope design. No scope change needed; guidance for implementation.
  • SECFILE 34 (iron-session secret rotation): Documents a rotation procedure for if the secret is compromised. Worth adding to operational documentation. Low priority for scope — rotation is documented behavior of iron-session's array API. Optionally add a one-line note to §7 that secret rotation procedure uses the array API without redeployment-invalidating sessions.
  • SECFILE 37 (anonymous user linking): Addressed if Finding B2 (disable all GoTrue auth methods except anonymous) is accepted. No additional action needed.
  • SECFILE 38 (last_active_at update strategy): Specifies which events should update last_active_at (write operations, not reads; throttled to once per 24 hours per user). This is a useful implementation note but is a development detail rather than a scope gap. Could be added as a parenthetical to the last_active_at field in the data model.
  • COMPLIANCE 50 (PWA manifest and icon requirements): Detailed specification of manifest fields, icon sizes (192x192, 512x512, maskable variants), iOS meta tags. This is useful Phase 8.1 implementation detail. Could be added to task 8.1 as a checklist.
  • COMPLIANCE 51 (error response format standardization): A standard error response shape ({ error: { code: string, message: string } }) and error code enum should be defined before Phase 2. This was flagged in Round 1 and partially addressed in Phase 5.6. Worth consolidating into a Phase 1 or 2 task to ensure it precedes API route implementation.
  • COMPLIANCE 48 (TMDB 429 rate limit handling): Server-side rate limiting on the TMDB proxy (token bucket, 30 req/10s) and Retry-After header handling on 429 responses. Useful addition to task 3.1, though the debounce and TanStack Query caching already reduce risk significantly.

End of PM Assessment Round 2