Skip to content

(gpt-5.6-sol medium) Pre-Launch System Review #571

Description

@kilemensi

Executive summary

Recommendation: no-go for public launch until the critical findings below
are resolved.

The application compiles and type-checks under the production Node version,
and its overall Next.js/Payload structure is reasonable. The main launch risks
are authorization and tenant isolation, credential exposure, publication
controls that can be bypassed through the CMS API, unsafe active-content and
remote-file boundaries, and the absence of a reliable release gate.

Critical findings

1. Authenticated users effectively have platform-wide administrator access

There is no role or tenant-membership model on users, while the multi-tenant
plugin explicitly grants every authenticated user access to all tenants.

Evidence:

  • src/collections/Users/index.ts:22 enables authentication but defines no
    roles, tenant membership, or explicit access controls.
  • src/plugins/index.ts:64 sets
    userHasAccessToAllTenants: () => true.
  • Payload's unspecified collection access defaults to authenticated users.

Impact:

  • An editor can manage other tenants' pages, political entities, and settings.
  • An authenticated user can manage other user accounts.
  • Compromise of any editor account becomes a platform-wide compromise.

Required remediation:

  • Add explicit superAdmin and tenant-scoped editor roles.
  • Store and enforce tenant membership on users.
  • Replace the unconditional userHasAccessToAllTenants function with a
    super-admin check.
  • Add explicit access controls for users and every tenant-owned collection.
  • Add negative authorization tests for cross-tenant reads and writes.

2. Authenticated users can retrieve platform credentials in plaintext

Airtable, Meedan, and AI-provider credentials are normal Payload text fields.
The custom password component masks their presentation in the admin UI but
does not protect the stored values at the API boundary.

Evidence:

  • src/globals/Settings/index.ts:12-15 permits every authenticated user to read
    and update the settings global.
  • src/globals/Settings/tabs/AirtableTab.ts:21 stores the Airtable key as text.
  • src/globals/Settings/tabs/MeedanTab.ts:21 stores the Meedan key as text.
  • src/globals/Settings/tabs/AITab.ts:487 stores AI-provider keys as text.
  • src/globals/Settings/tabs/MaskedApiKeyField.tsx only changes UI rendering.

Impact:

  • Any authenticated user can query REST or GraphQL for saved credentials.
  • Credentials can be exfiltrated, overwritten, or used for billing abuse and
    access to external data.

Required remediation:

  • Prefer a secret manager or server-only environment configuration.
  • Otherwise restrict fields and the settings global to super administrators.
  • Use write-only replacement semantics so existing keys are never returned.
  • Encrypt secrets at rest if they must remain in the database.
  • Rotate all saved credentials after access controls are corrected.

3. Public pages can resolve another tenant's entity and promises

Political-entity slugs are unique per tenant, but multiple public blocks query
published entities by slug without including the current tenant.

Evidence:

  • src/components/Hero/index.tsx:284
  • src/components/Promises/index.tsx:17
  • src/components/LatestPromises/index.tsx:30
  • src/components/KeyPromises/index.tsx:62

If two tenants use the same entity slug, Payload can return the first matching
record, causing the page to show another tenant's statistics, promises,
branding, or locale.

Required remediation:

  • Pass the already-resolved political-entity ID or object into each block.
  • Alternatively, include the current tenant ID in every entity query.
  • Add regression tests containing identical entity slugs in two tenants.

4. Publication controls are bypassable through REST and GraphQL

The frontend filters promises to publishStatus === "published", but the
collection's public read policy does not. Documents and AI-extraction records
are also publicly readable in their entirety.

Evidence:

  • src/collections/Promises.ts:24-25
  • src/collections/Documents/index.ts:20-21
  • src/collections/AIExtractions/index.ts:26-27

Impact:

  • Unpublished promises can be enumerated through the Payload API.
  • Documents can expose extracted text and processing/source fields.
  • AI extractions can expose internal extraction data, CheckMedia references,
    and upload errors.

Required remediation:

  • Make unauthenticated promise access return only published records.
  • Keep documents and AI extractions authenticated unless product ownership has
    explicitly approved every exposed field as public.
  • Use field-level access controls for internal processing metadata.
  • Test REST and GraphQL access without a session.

5. CMS-controlled HTML creates a stored active-content/XSS boundary

Newsletter embed code is rendered as raw HTML. Promise-update embeds accept an
arbitrary iframe source and fall back to raw HTML when no iframe is detected.

Evidence:

  • src/components/Newsletter/Newsletter.tsx:190-195
  • src/components/ActNowCard/UpdateDialog.tsx:11-15
  • src/components/ActNowCard/UpdateDialog.tsx:71-93

Combined with the current broad CMS authorization, an editor can inject event
handlers, hostile iframes, phishing forms, or other active content into public
pages. No Content Security Policy limits the impact.

Required remediation:

  • Store structured provider/origin configuration rather than arbitrary HTML.
  • Allowlist trusted iframe and form origins.
  • Sanitize any HTML that remains necessary.
  • Add restrictive iframe sandbox and allow attributes.
  • Deploy a nonce-based CSP compatible with MUI/Emotion and approved embeds.

6. Remote-file ingestion permits SSRF and weak file-type validation

The Meedan webhook checks only that an image URL begins with https://, then
passes it to a downloader that follows the URL without hostname or network
validation. The documented MEEDAN_ALLOWED_IMAGE_HOSTS variable is unused.

Evidence:

  • src/app/api/meedan-sync/route.ts:510-537
  • src/utils/files.ts:76-116

The Airtable upload route accepts a file when either the MIME type or extension
matches. It does not inspect magic bytes, allows SVG, and permits document
uploads up to 1 GiB by default.

Evidence:

  • src/app/api/airtable-upload/utils.ts:197-215
  • src/app/api/airtable-upload/utils.ts:178-191

Required remediation:

  • Enforce an HTTPS hostname allowlist.
  • Resolve hostnames and reject private, loopback, link-local, and metadata IPs.
  • Revalidate every redirect target or disable redirects.
  • Validate file signatures and require consistent type/extension metadata.
  • Remove SVG support or sanitize SVG content and serve it from an isolated
    origin with safe response headers.
  • Reduce upload limits and add rate, concurrency, storage, and cost quotas.

Release and operational blockers

7. Deployment has no quality or security gate

.github/workflows/deploy-dev.yml builds and deploys on every push to main,
but does not run lint, type-checking, integration tests, E2E tests, or security
checks. There is no production deployment workflow or approval environment.

Required remediation:

  • Add an isolated verification job for install, generated-artifact checks,
    lint, type-checking, integration tests, and E2E smoke tests.
  • Make deployment depend on that job.
  • Add an approval-gated production workflow with immutable image promotion.
  • Document rollback and verify it before launch.

8. Tests are not isolated from developer/shared databases

vitest.setup.ts:3-7 loads .env.local and .env, so integration tests can
silently connect to a shared or production-like database. The existing API
test reads data today, but future tests could mutate or delete it.

Required remediation:

  • Load test.env explicitly and refuse non-local/non-ephemeral database URIs.
  • Provision a unique database per CI run.
  • Reset it before and after the suite.
  • Add a hard safety guard against production hostnames and database names.

9. Producing a release artifact requires a live database

The Node 22 production build compiled and passed TypeScript validation, then
failed during page-data collection when MongoDB was unreachable. The Docker
build therefore requires a valid, reachable database secret at image-build
time (Dockerfile:30-35).

This couples artifact creation to mutable external state and means a database
outage can prevent building or rolling back an otherwise valid release.

Required remediation:

  • Avoid initializing Payload/database clients at module scope.
  • Ensure public routes are explicitly dynamic where appropriate.
  • Separate artifact compilation from database-dependent validation.
  • Run database smoke tests as a release check rather than during artifact
    construction.

10. Migration execution is absent from the release path

The repository contains a backfill migration, and package.json defines a
migrate command, but neither the deployment workflow nor a release phase runs
it.

Required remediation:

  • Add a single-instance, observable migration release step before traffic is
    switched.
  • Do not run migrations concurrently in every application replica.
  • Define rollback/forward-fix behavior for data migrations.

11. No external health/readiness contract exists

There is no application health endpoint, readiness endpoint, Docker
HEALTHCHECK, or deployment probe. The entrypoint checks Tika internally but
does not expose database and application readiness to the platform.

Required remediation:

  • Add liveness and readiness endpoints with bounded timeouts.
  • Include database readiness and, if essential to serving, Tika readiness.
  • Wire probes into Dokku/container orchestration.

High-priority correctness and reliability findings

12. Unknown URLs can return valid content instead of a 404

In src/app/(frontend)/[...slugs]/page.tsx:314-370, when the first segment is
not an entity, the second segment may be treated as a page slug. If neither
page exists, the tenant entity-selection page is rendered rather than a 404.

Examples:

  • /unknown/about can render the about page.
  • /unknown can return the entity-selection page with HTTP 200.

Impact: duplicate content, misleading links, poor analytics, and SEO indexing
of invalid routes.

Required remediation: define the accepted route grammar and call notFound()
for every non-matching path. Add canonical URLs and route-matrix tests.

13. The main promise list is silently paginated

src/components/Promises/index.tsx:46-65 omits limit: 0 or
pagination: false, so Payload's default page size applies. A "promise list"
can therefore display only the first page without telling the user.

Required remediation: either request all intended records or implement visible
pagination/infinite loading with accurate counts.

14. AI calls have no explicit deadline or document cost ceiling

src/tasks/extractPromises.ts chunks every extracted document and invokes the
configured AI provider sequentially for each chunk. Calls configure retries but
no explicit abort deadline, and there is no hard maximum document size or chunk
count before API work begins.

Impact: hung jobs, unexpectedly long workflows, and unbounded provider cost on
very large or malicious documents.

Required remediation:

  • Add per-call and per-document deadlines.
  • Cap source characters, chunks, tokens, and total provider spend per document.
  • Record truncated/rejected documents explicitly for operator review.
  • Add metrics and alerts for calls, tokens, duration, retries, and failures.

15. Webhook and upload endpoints lack abuse controls

Authentication uses bearer/shared secrets, but there is no application rate
limiting, replay window, idempotency key policy, or per-source quota. The
Meedan webhook also compares its secret using normal string equality rather
than constant-time comparison.

Required remediation:

  • Use constant-time secret comparison.
  • Prefer signed webhook payloads with timestamps and replay rejection.
  • Add idempotency records for state-changing events.
  • Apply IP/source and token-level rate limits and concurrency limits.

Performance and scalability findings

16. Public requests perform repeated uncached database work

Hot routes and blocks repeatedly fetch the tenant, tenant settings, entity,
statuses, promises, update settings, and media. Pages are host-dependent and
dynamic, but shared request-level and bounded cross-request caching are still
possible.

Required remediation:

  • Deduplicate reads with React cache() or a request-scoped data layer.
  • Add tag-based caching/revalidation for published CMS data.
  • Pass resolved entities/settings into blocks rather than refetching them.
  • Parallelize independent promise-detail lookups.

17. Hot promise queries need an explicit indexing strategy

Public queries repeatedly filter by political entity and publication status and
sort by creation/update time. The collection configuration does not declare an
obvious compound index for these access patterns.

Required remediation:

  • Validate query plans against production-like data.
  • Add compound indexes such as political entity + publish status + sort field.
  • Establish latency and throughput targets before load testing.

18. Several media and partner paths use N+1 lookups

Media records are resolved individually in multiple blocks. This increases
database round trips and tail latency as lists grow.

Required remediation: request required depth/fields in the parent query or
batch media IDs into one lookup.

Accessibility and frontend findings

19. The update dialog is not a real accessible modal

src/components/ActNowCard/UpdateDialog.tsx:19-52 hides the actual MUI dialog
and renders a separate fixed-position box. Autofocus and focus enforcement are
disabled, and the visible content lacks complete dialog semantics.

Impact: keyboard and screen-reader users can interact with background content,
lose focus, or fail to understand the modal state.

Required remediation: render the iframe/content inside the MUI Dialog, keep
focus trapping enabled, provide a labelled title, restore focus on close, and
test Escape/Tab behavior.

20. Frontend error and not-found experiences are incomplete

There is a global error boundary, but no route-group-specific frontend error
and not-found experience. Combined with the catch-all fallback behavior, users
may receive generic or misleading responses.

Required remediation: add branded error.tsx and not-found.tsx, preserve
correct HTTP semantics, and include retry/support paths.

21. Automated accessibility coverage is absent

The E2E suite contains only a shallow homepage assertion. There are no automated
checks for landmarks, keyboard navigation, dialogs, contrast, accessible names,
or responsive behavior.

Required remediation: add axe-based checks and targeted Playwright scenarios
for navigation, filtering, share actions, dialogs, and tenant selection.

Security hardening and observability

22. Baseline browser security headers are not configured

next.config.mjs has no Content Security Policy, HSTS, Referrer Policy,
Permissions Policy, or explicit anti-clickjacking policy.

Required remediation: add a tested header policy, accounting for approved
third-party forms, Sentry, media hosts, MUI/Emotion, and Payload admin needs.

23. Sentry samples all traces and enables logs

Client, server, and edge Sentry configurations set tracesSampleRate: 1 and
enableLogs: true.

Evidence:

  • src/instrumentation-client.ts:9-15
  • sentry.server.config.ts:7-14
  • sentry.edge.config.ts

Impact: unnecessary client/server overhead, high telemetry cost, and increased
privacy exposure.

Required remediation:

  • Configure environment-specific sampling.
  • Scrub IP addresses, authorization data, uploaded filenames, URLs, and PII.
  • Define alert thresholds and an on-call response path.

24. Operational runbooks and recovery evidence are missing

No repository documentation covers backup frequency, restore testing, RPO/RTO,
incident ownership, credential rotation, capacity, rollback, or degraded-mode
behavior when Airtable, Meedan, AI providers, Tika, MongoDB, S3, or SMTP fail.

Required remediation: document and rehearse the launch-day and incident
runbooks, including a verified database restore and image rollback.

Engineering hygiene

25. Runtime support is broader than the verified runtime

package.json permits all Node versions greater than or equal to 20.9, but the
Node 24 build failed inside Webpack. The production container uses Node 22.

Required remediation: pin Node 22 across local development and CI, add a
version file, and narrow engines.node until Node 24 is proven supported.

26. Package-manager versions are inconsistent

package.json declares pnpm 10.33.0 while Dockerfile installs pnpm 10.16.1.

Required remediation: use Corepack with the package-manager version declared in
package.json, consistently across Docker, CI, and developer setup.

27. Lint debt is non-zero

Lint completed with no errors and 39 warnings, primarily explicit any usage
and unused imports/variables. Several warnings occur in workflow and document
processing code where strong typing is particularly valuable.

Required remediation: make warnings fail CI after clearing the current set.

Verification performed

Static analysis

  • pnpm lint: passed with 39 warnings.
  • Production-equivalent Node 22 build: Webpack compilation and TypeScript
    validation passed; page-data collection failed because MongoDB was
    unreachable.
  • Node 24 build: failed inside Webpack, despite the declared engine range.

Tests

  • Integration suites: 7 passed.
  • Integration assertions: 32 passed.
  • Database-backed API suite: could not initialize MongoDB.
  • E2E was not meaningfully runnable without a bootable database.
  • Existing E2E coverage contains one shallow homepage test.

Dependency audit

pnpm audit --prod was inconclusive because the registry returned HTTP 410 for
pnpm's audit endpoints. A current software-composition-analysis scan remains a
launch requirement.

Positive observations

  • The production image uses a multi-stage build and a non-root runtime user.
  • BuildKit secrets are used for database, Payload, and Sentry build secrets.
  • Payload's default unspecified access control was verified rather than assumed.
  • Public page queries usually filter published entities and promises in the UI.
  • File downloads are streamed and enforce a byte limit while streaming.
  • Airtable upload bearer-token comparison uses timingSafeEqual.
  • Webhook processing has useful structured logging and Sentry integration.
  • Background workflows generally process tasks sequentially and report task
    failures to Sentry.

Required launch sequence

  1. Implement roles, tenant membership, and credential protection; rotate keys.
  2. Correct tenant-unscoped entity resolution and add isolation tests.
  3. Enforce publication-aware REST and GraphQL access.
  4. Remove unsafe raw HTML and harden all remote-file ingestion.
  5. Establish isolated test databases and mandatory CI quality gates.
  6. Add release migrations, health probes, backups, and rollback procedures.
  7. Add route correctness, accessibility, integration, and critical-flow E2E
    coverage.
  8. Run dependency, DAST, accessibility, and load testing in staging.
  9. Launch only after every critical item is closed and the complete production
    pipeline is green.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions