From b4c1dae417c6140f93b9e94d89c4ad126b5c9a23 Mon Sep 17 00:00:00 2001 From: mahmutKaya <33642821+mahmutkaya@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:53:33 +0200 Subject: [PATCH 1/8] feat(ci): a staging bake, and robots that refuses to index a non-canonical copy (#105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Groundwork for a deployed sofra staging at staging.sofrapiwas.com. A separate build job rather than an extra tag, because NEXT_PUBLIC_SITE_URL is a BUILD arg — one image cannot serve two hosts. `:staging` is develop-only, so rolling staging can never deploy main's build output to it, and `:migrate-staging` gives the staging database develop's migrations rather than main's. The robots change is the part that is not bookkeeping. `robots.ts` was allow-all and explicitly welcomed GPTBot, ClaudeBot, PerplexityBot and friends — correct for the canonical site, actively harmful on a public twin of it. A crawlable staging copy competes with the real marketing site for exactly the citations the AEO work exists to win, on content that is by definition ahead of what we decided to publish; and while the control plane is auth-gated, its login and signup pages are not. Keyed on the deployment's own base URL, not a separate flag, so it is self-correcting: anything that is not the canonical host is noindex without anyone remembering to set something. Co-authored-by: Claude Opus 5 --- .github/workflows/build-image.yml | 48 +++++++++++++++++++++++++++++++ app/robots.ts | 24 ++++++++++++++-- lib/seo.ts | 14 ++++++++- 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-image.yml b/.github/workflows/build-image.yml index b784eb5..4524c39 100644 --- a/.github/workflows/build-image.yml +++ b/.github/workflows/build-image.yml @@ -72,6 +72,7 @@ jobs: # Same main-branch gate as the app image above (not {{is_default_branch}}). tags: | type=raw,value=migrate,enable=${{ github.ref == 'refs/heads/main' }} + type=raw,value=migrate-staging,enable=${{ github.ref == 'refs/heads/develop' }} type=sha,format=long,prefix=migrate- - uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 @@ -84,3 +85,50 @@ jobs: labels: ${{ steps.meta-migrate.outputs.labels }} cache-from: type=gha cache-to: type=gha,mode=max + + # ── staging bake ────────────────────────────────────────────────────────────── + # A SEPARATE build, not just an extra tag on the one above: NEXT_PUBLIC_SITE_URL is a + # BUILD arg, so the same image cannot serve two hosts. Baked with the staging URL it + # emits staging canonicals — and, via lib/seo.ts IS_CANONICAL_SITE, a robots.txt that + # refuses every crawler. A staging twin of the marketing site left crawlable would + # compete with the real one for the citations the AEO work exists to win. + # + # develop only. `:staging` must never carry main's code, or rolling staging would + # silently deploy production build output to it. + build-staging: + name: build & push staging (GHCR) + if: github.ref == 'refs/heads/develop' + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # v3.11.1 + + - uses: docker/login-action@74a5d142397b4f367a81961eba4e8cd7edddf772 # v3.4.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - id: meta-staging + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 + with: + images: ghcr.io/${{ github.repository }} + # `:staging` moves; `:staging-` is immutable, for rollback. + tags: | + type=raw,value=staging + type=sha,format=long,prefix=staging- + + - uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 + with: + context: . + file: ./Dockerfile + target: runner + push: true + build-args: | + NEXT_PUBLIC_SITE_URL=${{ vars.STAGING_SITE_URL || 'https://staging.sofrapiwas.com' }} + tags: ${{ steps.meta-staging.outputs.tags }} + labels: ${{ steps.meta-staging.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=max diff --git a/app/robots.ts b/app/robots.ts index a15c4b7..dba9f80 100644 --- a/app/robots.ts +++ b/app/robots.ts @@ -1,6 +1,5 @@ import type { MetadataRoute } from "next"; - -const BASE_URL = process.env.NEXT_PUBLIC_SITE_URL ?? "https://sofrapiwas.com"; +import { IS_CANONICAL_SITE, SITE_URL } from "@/lib/seo"; // AI answer engines cite what they can crawl — explicitly welcome their bots // (AEO: workspace docs/plans/SOFRA-AEO-PLAN.md §1). Keep the default allow-all @@ -16,12 +15,31 @@ const AI_CRAWLERS = [ "CCBot", ]; +/** + * `robots.txt` — allow-all on the canonical site, DISALLOW-all anywhere else. + * + * The second half exists because this image is now also deployed to + * `staging.sofrapiwas.com`, and a publicly reachable copy of the marketing site is not a + * neutral thing to have. Left allow-all it would invite the crawlers listed above onto a + * TWIN of the pages the AEO work exists to get cited — competing with the real site for + * the same citations, on content that is by definition ahead of what we have decided to + * publish. The control plane behind it is auth-gated, but its login and signup pages are + * not, and a staging signup form in a search result is its own kind of bad. + * + * Keyed on the deployment's own base URL rather than a separate flag, so it is + * self-correcting: any deployment that is not the canonical host is noindex without + * anyone remembering to set anything. Caddy adds `X-Robots-Tag: noindex` on the staging + * host as well, for the crawlers that ignore this file. + */ export default function robots(): MetadataRoute.Robots { + if (!IS_CANONICAL_SITE) { + return { rules: [{ userAgent: "*", disallow: "/" }] }; + } return { rules: [ { userAgent: "*", allow: "/" }, ...AI_CRAWLERS.map((userAgent) => ({ userAgent, allow: "/" })), ], - sitemap: `${BASE_URL}/sitemap.xml`, + sitemap: `${SITE_URL}/sitemap.xml`, }; } diff --git a/lib/seo.ts b/lib/seo.ts index d763e20..3f959b3 100644 --- a/lib/seo.ts +++ b/lib/seo.ts @@ -1,7 +1,19 @@ import type { Metadata } from "next"; import { routing } from "@/i18n/routing"; -export const SITE_URL = process.env.NEXT_PUBLIC_SITE_URL ?? "https://sofrapiwas.com"; +/** The one host these pages are the canonical copy of. */ +export const CANONICAL_SITE_URL = "https://sofrapiwas.com"; + +export const SITE_URL = process.env.NEXT_PUBLIC_SITE_URL ?? CANONICAL_SITE_URL; + +/** + * Is THIS deployment the canonical site? + * + * False on `staging.sofrapiwas.com` and on any local run, which is what makes `robots.ts` + * refuse crawlers without a separate flag anyone has to remember to set. Baked, because + * `NEXT_PUBLIC_SITE_URL` is a build arg — the staging image is its own bake. + */ +export const IS_CANONICAL_SITE = SITE_URL === CANONICAL_SITE_URL; /** The SofraPiwas open-graph / social share image (resolved against metadataBase). */ export const OG_IMAGE = { From 94261e69642051d33a0d124559bd184fd531211d Mon Sep 17 00:00:00 2001 From: mahmutKaya <33642821+mahmutkaya@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:09:42 +0200 Subject: [PATCH 2/8] fix(ci): the staging bake could never fire (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `build-image.yml` triggers on `push: branches: [main]` only, so the `build-staging` job I added — gated on `refs/heads/develop` — was unreachable. The gate looked right and was dead code: merging #105 published no `:staging` image at all, which is how I found it. `develop` added to the trigger, matching the frontend and backend workflows. `build-push` now skips on develop so a develop merge does not also publish a `:sha` and `migrate-` nothing pulls; the ref gates inside each meta step still keep `:latest` and `:migrate` main-only. Co-authored-by: Claude Opus 5 --- .github/workflows/build-image.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-image.yml b/.github/workflows/build-image.yml index 4524c39..b61b421 100644 --- a/.github/workflows/build-image.yml +++ b/.github/workflows/build-image.yml @@ -8,7 +8,10 @@ name: build-image on: push: - branches: [main] + # `develop` was added with the staging bake: the build-staging job below is gated on + # that ref, and a job gate is useless if the WORKFLOW never runs on the branch. The + # tag gating inside each meta step is what keeps :latest and :migrate main-only. + branches: [main, develop] tags: ['v*'] concurrency: @@ -22,6 +25,9 @@ permissions: jobs: build-push: name: build & push (GHCR) + # main + tags only. On develop the staging bake below is the one that runs; letting + # this job run too would publish a :sha and a migrate- image nothing pulls. + if: github.ref != 'refs/heads/develop' runs-on: ubuntu-latest timeout-minutes: 30 steps: From 3b04163888e3b73caee24a92ec60254df5b22a1c Mon Sep 17 00:00:00 2001 From: mahmutKaya <33642821+mahmutkaya@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:23:20 +0200 Subject: [PATCH 3/8] fix(ci): build the staging migrate image in the job that runs on develop (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `:migrate-staging` was tagged on `build-push`'s meta-migrate step. That job skips on develop, so the tag was dead code and the image never existed — `docker pull ghcr.io/piwas-21/sofra:migrate-staging` on the box answered "not found", which is how I found it. Moved into `build-staging`, which is the job that actually runs on develop. Staging needs its own migrate image rather than reusing main's: the image carries prisma/migrations, so main's copy would apply main's schema to a database that tracks develop. Second-order fallout from gating build-push off develop in the previous commit. Both were only visible by trying to use the artifact. Co-authored-by: Claude Opus 5 --- .github/workflows/build-image.yml | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-image.yml b/.github/workflows/build-image.yml index b61b421..5edb5cd 100644 --- a/.github/workflows/build-image.yml +++ b/.github/workflows/build-image.yml @@ -78,7 +78,6 @@ jobs: # Same main-branch gate as the app image above (not {{is_default_branch}}). tags: | type=raw,value=migrate,enable=${{ github.ref == 'refs/heads/main' }} - type=raw,value=migrate-staging,enable=${{ github.ref == 'refs/heads/develop' }} type=sha,format=long,prefix=migrate- - uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 @@ -138,3 +137,29 @@ jobs: labels: ${{ steps.meta-staging.outputs.labels }} cache-from: type=gha cache-to: type=gha,mode=max + + # The DB-tooling image for staging. It has to be built HERE, in the job that runs + # on develop — putting the tag on `build-push`'s meta-migrate step made it dead + # code the moment that job started skipping on develop, and the tag simply never + # existed. Found by pulling it on the box. + # + # Staging needs its own: the migrate image carries prisma/migrations, so main's + # copy would apply main's schema to a database that tracks develop. + - id: meta-migrate-staging + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 + with: + images: ghcr.io/${{ github.repository }} + tags: | + type=raw,value=migrate-staging + type=sha,format=long,prefix=migrate-staging- + + - uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 + with: + context: . + file: ./Dockerfile + target: migrate + push: true + tags: ${{ steps.meta-migrate-staging.outputs.tags }} + labels: ${{ steps.meta-migrate-staging.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=max From 99fb4dba3fa3c4d87b94e8ba866ce0956f17eeb6 Mon Sep 17 00:00:00 2001 From: mahmutKaya <33642821+mahmutkaya@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:02:06 +0200 Subject: [PATCH 4/8] test(e2e): a live suite against the deployed staging control plane (#108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(e2e): a live suite against the deployed staging control plane The existing suite is already unmocked — real build, real Postgres, a real Mollie first payment on the test key. What it cannot see is anything that only exists once something is deployed: the box .env reaching the container with the RIGHT values, the migrate one-off having run, Caddy routing the host, and the image that was PUBLISHED rather than the one `next build` produces locally. Every one of those has bitten in the last two days. Two bit while standing this environment up — a `:staging` bake whose job could never fire (#106) and a `:migrate-staging` tag that was dead code (#107) — both invisible until something tried to pull the artifact. Six assertions, read-only by construction (no account, no payment, no row written): the entry points serve; staging is noindex in both robots.txt and a header while production is still crawlable; signing in proves the staging DB and AuthSecret are real; the Mollie TEST key is wired in; and provisioning is deliberately DISARMED, asserted positively so nobody "fixes" the empty PROVISION_GITHUB_TOKEN and hands a test environment write access to production infra. Gated on E2E_REMOTE so it never runs against the local `next start`, where every assertion would be vacuous or wrong. One login per run — lib/auth.ts allows 10 per email per 15 minutes, and a 429 there renders as "invalid credentials", which reads like a broken password rather than a suite that ate its own budget. Co-Authored-By: Claude Opus 5 * test(e2e): harden the live staging suite after review Ten review findings; the two that mattered most were bugs I had shipped into the working tree rather than in the new spec. 1. `STAGING_ADMIN={email: …}` in .env killed `npm run test:e2e:full`. scripts/e2e-suite.sh sources .env with `set -a && . ./.env`, and bash reads the unquoted brace value as an assignment plus the command `a@b.com,` — fatal under `set -euo pipefail`, reported as an email address. The repo's primary quality gate was dead on this clone. Value quoted, parser now unquotes, and .env.example documents why the quotes are load-bearing. 2. E2E_REMOTE skipped the webServer but gated nothing else, so a whole-directory remote run would have driven self-serve-signup and billing-mollie — which write rows and create real payments — against the shared staging environment. Now enforced in playwright.config.ts BOTH ways: remote runs this spec only, local runs everything but this spec. The local half is not symmetry for its own sake; without it the spec joined test:e2e:full and asserted deployment facts against localhost. Assertions that could not fail, or failed in the wrong direction: - the /admin heading check passed on the failure it claimed to catch — global-error.tsx is the app's only error boundary and renders an

, and waitForURL is status-blind, so an unmigrated table would have gone green. Now anchored on the page's own heading plus an explicit refusal of the error boundary. - the Mollie check was absence-only, and `goto` returns 200 after a redirect, so an expired session bouncing to /login reported the key as configured. Positive anchor added. - the test claimed to prove a `test_` key; mollieConfigured() only reports that SOME key is set and nothing surfaces the prefix. Renamed to what it establishes, and the gap — along with "this cannot tell you the image is current" — is now stated in the docstring. - both matched strings exist in all six locales with OPPOSITE failure directions, so NEXT_LOCALE is pinned to en. Also: the production-host compare is now host-based and runs before the first navigation, so a mistyped E2E_BASE_URL cannot burn the owner's production login budget; credential parsing rejects every wrong-but-truthy shape instead of timing out as "staging is down"; a missing credential is an error rather than a skip that exits 0; and the beforeAll gets its own timeout, since it previously shared the 30s test budget and could not reach the waitForURL value it passed. Verified: test:e2e:full 23 passed, test:e2e:staging 6 passed, remote --list confirms 1 file not 5, and pointing the suite at production fails in beforeAll before any login. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- .env.example | 10 ++ package.json | 3 +- playwright.config.ts | 42 +++++- tests/e2e/staging-live.spec.ts | 239 +++++++++++++++++++++++++++++++++ 4 files changed, 287 insertions(+), 7 deletions(-) create mode 100644 tests/e2e/staging-live.spec.ts diff --git a/.env.example b/.env.example index a4d11c0..78e9d5b 100644 --- a/.env.example +++ b/.env.example @@ -73,3 +73,13 @@ MOLLIE_WEBHOOK_URL= # (never by the app, which reads MOLLIE_API_KEY). The suite hard-refuses anything # without a test_ prefix. MOLLIE_API_KEY_TEST= + +# QA admin for the DEPLOYED staging control plane, read by tests/e2e/staging-live.spec.ts +# (`npm run test:e2e:staging`). Not used by the app or the local suite. +# +# The single quotes are REQUIRED. scripts/e2e-suite.sh sources this file's real counterpart +# with `set -a && . ./.env`, and bash reads an unquoted {email: a@b.com, password: x} as an +# assignment followed by the command `a@b.com,` — under the script's `set -euo pipefail` +# that kills the entire unmocked E2E suite before it starts Postgres, and reports the +# failure as an email address, which names nothing you would think to look at. +STAGING_ADMIN='{email: , password: }' diff --git a/package.json b/package.json index f1ef361..b2d2349 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,8 @@ "test:e2e": "playwright test", "db:migrate": "prisma migrate dev", "db:generate": "prisma generate", - "test:e2e:full": "bash scripts/e2e-suite.sh" + "test:e2e:full": "bash scripts/e2e-suite.sh", + "test:e2e:staging": "E2E_REMOTE=1 E2E_BASE_URL=https://staging.sofrapiwas.com playwright test tests/e2e/staging-live.spec.ts" }, "dependencies": { "@prisma/adapter-pg": "^7.8.0", diff --git a/playwright.config.ts b/playwright.config.ts index eafc2a8..c172b92 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -10,8 +10,32 @@ import { defineConfig, devices } from "@playwright/test"; const baseURL = process.env.E2E_BASE_URL ?? "http://localhost:3000"; +// E2E_REMOTE points the run at a DEPLOYED environment, and the split is enforced BOTH +// ways: remote runs the staging spec and nothing else, local runs everything else and never +// the staging spec. +// +// The remote direction is a safety interlock. Every other spec here MUTATES — +// self-serve-signup writes User/Plan/SignupRequest rows, billing-mollie creates real +// payments, owner-dashboard repoints billing records — and they reach their target through +// relative `page.goto`, so `E2E_REMOTE=1 E2E_BASE_URL=https://staging… npx playwright test` +// (the obvious generalisation of the npm script) would run all of them against the +// long-lived shared environment, which has no throwaway database to drop. Their own +// assertions would then fail against the wrong DATABASE_URL — but the writes would already +// have landed. +// +// The local direction is not symmetry for its own sake: without it the staging spec joins +// `test:e2e:full`, where it asserts deployment facts against `localhost` — robots is +// allow-all there by design, and the staging admin account does not exist — and fails for +// reasons that say nothing about the change under test. Enforcing it here rather than with a +// `test.skip` inside the spec is what lets a missing credential be a hard ERROR when the +// spec does run: a skipped authed half still exits 0, and reports success having verified +// nothing behind the login. +const REMOTE = Boolean(process.env.E2E_REMOTE); +const STAGING_SPEC = /staging-live\.spec\.ts$/; + export default defineConfig({ testDir: "tests/e2e", + ...(REMOTE ? { testMatch: STAGING_SPEC } : { testIgnore: STAGING_SPEC }), timeout: 30_000, forbidOnly: !!process.env.CI, retries: process.env.CI ? 1 : 0, @@ -36,10 +60,16 @@ export default defineConfig({ }, expect: { timeout: 15_000 }, projects: [{ name: "chromium", use: { ...devices["Desktop Chrome"] } }], - webServer: { - command: "npm run start", - url: baseURL, - reuseExistingServer: !process.env.CI, - timeout: 120_000, - }, + // Skip the local server when the run targets a DEPLOYED environment + // (E2E_REMOTE=1 with E2E_BASE_URL=https://staging.sofrapiwas.com). Without this, + // pointing baseURL at staging still runs `npm run start` and waits two minutes on a + // port nothing will answer. Mirrors the frontend repo's config. + webServer: REMOTE + ? undefined + : { + command: "npm run start", + url: baseURL, + reuseExistingServer: !process.env.CI, + timeout: 120_000, + }, }); diff --git a/tests/e2e/staging-live.spec.ts b/tests/e2e/staging-live.spec.ts new file mode 100644 index 0000000..a186591 --- /dev/null +++ b/tests/e2e/staging-live.spec.ts @@ -0,0 +1,239 @@ +import { expect, test } from "./helpers/fixtures"; +import { CANONICAL_SITE_URL } from "@/lib/seo"; +import { readFileSync } from "node:fs"; + +/** + * The DEPLOYED control plane at staging.sofrapiwas.com. + * + * The rest of this suite is already unmocked — real build, real Postgres, a real Mollie + * first payment on the `test_` key. It is not a weaker test than this one. What it cannot + * see is everything that only exists once something is deployed: + * + * - the box `.env` actually reaching the container, with the RIGHT values — the staging + * database rather than production's, its own AUTH_SECRET, a Mollie key at all + * - the founder-run `:migrate-staging` one-off having actually run, so the schema the + * app queries exists + * - Caddy routing the host at all, and TLS for a name that had never been served + * - that what is deployed is a STAGING bake rather than the production image + * + * Two of those bit while standing this environment up: a `:staging` image tag whose job + * could never fire, and a `:migrate-staging` tag that was dead code — both invisible until + * something tried to pull the artifact. + * + * What this suite does NOT prove, so nobody reads more into a green run than is there: + * - that the deployed image is CURRENT. A months-old `:staging` bake passes everything + * here. There is no version or health endpoint to assert against; adding one is the + * fix, not a cleverer assertion. + * - that the Mollie key is a `test_` key rather than a `live_` one. `mollieConfigured()` + * reports only that SOME key is set and nothing surfaces the prefix — see the billing + * test, which is named for what it can actually establish. + * + * READ-ONLY BY CONSTRUCTION. It signs in and looks: no account, no payment, no row. There + * is nothing to restore and no way for a failed run to leave the environment dirty. The + * mutating flows belong to `e2e-suite.sh`, which owns a throwaway database it can drop — + * and `playwright.config.ts` now enforces that split, so an `E2E_REMOTE` run cannot reach + * them however it is invoked. + */ + +const BASE = process.env.E2E_BASE_URL ?? ""; + +/** Hosts, not strings: `https://sofrapiwas.com/` must not slip past a `!==` compare. */ +function hostOf(url: string): string { + try { + return new URL(url).host.replace(/^www\./, ""); + } catch { + return ""; + } +} + +/** + * `STAGING_ADMIN='{email: …, password: …}'` from the gitignored `.env`. + * + * The quotes are REQUIRED and this is the reason: `scripts/e2e-suite.sh` sources this same + * file with `set -a && . ./.env`, and bash reads an unquoted `{email: a@b.com, password: x}` + * as an assignment followed by the command `a@b.com,`. Under the script's `set -euo pipefail` + * that is a fatal "command not found" and the entire primary E2E suite dies before it starts + * Postgres — reported as an email address, which names nothing you would think to look at. + * + * Parsed with two anchored patterns rather than `split(":")`, because the value carries three + * colons and a naive split yields an email of `"…, password"`; the API then answers a + * perfectly honest 401 that reads like a stale credential. Both captures are shape-checked + * and the whole line must match, so a malformed entry fails LOUDLY here instead of arriving + * at the login form as a wrong-but-truthy credential and timing out as "staging is down". + */ +function stagingAdmin(): { email: string; password: string } { + let raw: string; + try { + raw = readFileSync(".env", "utf8"); + } catch { + throw new Error("no .env in the sofra repo — STAGING_ADMIN is required for the deployed run"); + } + + const line = raw.split("\n").find((l) => l.startsWith("STAGING_ADMIN=")); + if (!line) throw new Error("no STAGING_ADMIN in .env — required for the deployed staging run"); + + const value = line + .slice(line.indexOf("=") + 1) + .trim() + .replace(/^["']|["']$/g, ""); + const body = value.replace(/^\{/, "").replace(/\}$/, ""); + // Per-value unquoting as well as per-line: `{email: "a@b.com", password: "p"}` is a + // perfectly reasonable thing to write, and without this the quotes ride along INTO the + // login form — truthy, shape-valid, and rejected by the API as a wrong password. + const unquote = (s: string | undefined) => s?.trim().replace(/^["']|["']$/g, ""); + const email = unquote(/email\s*:([^,]*)/.exec(body)?.[1]); + const password = unquote(/password\s*:([^}]*)$/.exec(body)?.[1]); + + // Shape checks, because every malformed variant above still yields something truthy: + // a swapped key order gives a password of "p, email: a@b.com", and a stray trailing + // comment gives "p} # staging box". Both would log in wrong and look like an outage. + if (!email || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) { + throw new Error("STAGING_ADMIN in .env has no valid email — expected {email: …, password: …}"); + } + if (!password || /[:}]/.test(password)) { + throw new Error("STAGING_ADMIN in .env has a malformed password — check the {email: …, password: …} shape"); + } + return { email, password }; +} + +test.describe("the deployed staging control plane", () => { + // No skip guard here: `playwright.config.ts` only admits this file when E2E_REMOTE is set, + // so reaching it at all means a deployed run was asked for. A missing credential is then an + // ERROR, not a skip — a skipped authed half exits 0 and reports success having verified + // nothing behind the login, which is where every claim in this file's docstring lives. + + test("serves the marketing site and the control-plane entry points", async ({ page }) => { + for (const path of ["/en", "/login"]) { + const res = await page.goto(path, { waitUntil: "domcontentloaded" }); + expect(res?.status(), `${path} should serve`).toBe(200); + } + // The signup redirects to the locale-prefixed route; following it must land on a real + // configurator, not a 404 — this is the funnel's front door. + await page.goto("/signup", { waitUntil: "domcontentloaded" }); + expect(page.url()).toMatch(/\/[a-z]{2}\/signup/); + await expect(page.getByRole("button", { name: /sign up|get started|continue/i }).first()).toBeVisible(); + }); + + test("staging is not indexable, and says so twice", async ({ page, request }) => { + // A public twin of the marketing site left crawlable would compete with the real one + // for exactly the citations the AEO work exists to win — on content that is by + // definition ahead of what we decided to publish. + const robots = await request.get(`${BASE}/robots.txt`); + expect(robots.status()).toBe(200); + const body = await robots.text(); + expect(body, "staging robots.txt must disallow everything").toMatch(/Disallow:\s*\/\s*$/m); + expect(body, "staging must not invite AI crawlers the canonical site invites").not.toMatch(/GPTBot/i); + + // And again in a header, for the crawlers that never fetch robots.txt. + const res = await page.goto("/en", { waitUntil: "domcontentloaded" }); + expect(res?.headers()["x-robots-tag"] ?? "", "X-Robots-Tag").toMatch(/noindex/i); + }); + + test("PRODUCTION is still crawlable (fails here, but the fault is on sofrapiwas.com)", async ({ request }) => { + // Deliberately asserted from the staging suite: the same commit that flips staging to + // noindex could flip production, and `robots.ts` decides between them at RUNTIME from the + // deployment's own base URL. A staging suite that never looks at production reports + // all-clear either way. The title carries the attribution because a prod incident or a + // Caddy blip turns this red inside a describe named for staging, and the reflex is to go + // look at the wrong environment. This wants a scheduled monitor eventually; until one + // exists, an assertion that runs is worth more than a plan for one that would. + const res = await request.get(`${CANONICAL_SITE_URL}/robots.txt`); + expect(res.status()).toBe(200); + const prod = await res.text(); + expect(prod, "production robots.txt must still allow crawling").toMatch(/^Allow:\s*\//m); + expect(prod, "production must still invite AI crawlers (AEO)").toMatch(/GPTBot/i); + }); + + // ONE login for all the authed assertions. `lib/auth.ts` allows 10 per email per 15 + // minutes, so a test-per-login suite would throttle itself after five runs — and a 429 + // there returns a null user, which renders as an ordinary "invalid credentials" and reads + // like a broken password rather than a suite that ate its own budget. + test.describe("signed in as the staging admin", () => { + test.describe.configure({ mode: "serial" }); + + let page: import("@playwright/test").Page; + + test.beforeAll(async ({ browser }) => { + // The config's testMatch keeps the MUTATING specs off a deployed host; this keeps + // THIS spec off production. Without it a mistyped E2E_BASE_URL would drive real logins + // against the live control plane using the owner's own address, and `lib/auth.ts` + // counts failures toward 10-per-email-per-15-minutes — a few runs lock them out of + // production. Checked before the first navigation, not after. + expect(hostOf(BASE), "refusing to run against the canonical production host").not.toBe( + hostOf(CANONICAL_SITE_URL), + ); + + const creds = stagingAdmin(); + // The 30s default is the whole-test budget and the hook shares it, so the explicit + // waitForURL timeout below was unreachable: a cold container after a roll would fail + // in navigation and read as an outage. Raising patience weakens nothing. + test.setTimeout(90_000); + + page = await browser.newPage({ baseURL: BASE }); + // Pin the control plane's locale. `lib/control-locale.ts` reads the NEXT_LOCALE cookie, + // and the two strings asserted below exist in all six message files — under a non-`en` + // locale the provisioning check fails loudly while the billing check would go QUIET, + // which is exactly the wrong pair of directions. + await page.context().addCookies([{ name: "NEXT_LOCALE", value: "en", url: BASE }]); + + await page.goto("/login", { waitUntil: "domcontentloaded" }); + await page.getByLabel(/email/i).fill(creds.email); + await page.getByLabel(/password/i).fill(creds.password); + await page.getByRole("button", { name: /sign in|log in/i }).click(); + await page.waitForURL(/\/admin/, { timeout: 60_000 }); + }); + + test.afterAll(async () => { + await page?.close(); + }); + + test("the admin dashboard renders — the schema and the box env are real", async () => { + // Asserted on the page's OWN heading, not `getByRole("heading").first()`. That earlier + // shape passed on the failure it claimed to catch: `app/global-error.tsx` is the only + // error boundary in the app and renders `

Something went wrong

`, so an + // unmigrated table would throw during render, leave the URL on /admin (waitForURL is + // status-blind), and satisfy a first-heading assertion. + // + // Reaching a WORKING /admin also stands in for the DB-separation check this suite + // otherwise lacks: the account it signs in with was seeded only on staging, so a + // container wired to production's DATABASE_URL cannot authenticate it at all. + await expect(page.getByRole("heading", { name: /partner applications|admin|dashboard/i }).first()).toBeVisible(); + await expect(page.getByText(/something went wrong/i), "the admin page must not be an error boundary").toHaveCount( + 0, + ); + }); + + test("a Mollie key is wired into the container", async () => { + // Named for what it can establish. `lib/mollie.ts#mollieConfigured` returns + // `Boolean(process.env.MOLLIE_API_KEY)` — absence of this banner means SOME key is set, + // and nothing on any surface exposes the prefix, so a `live_` key here would pass. That + // gap is real and belongs in the docstring rather than in an overclaiming test name. + const res = await page.goto("/admin/billing", { waitUntil: "domcontentloaded" }); + expect(res?.status()).toBe(200); + // A positive anchor FIRST. `goto` returns the final response after redirects, so a + // `requireAdmin()` bounce to /login is also a 200 — and a page that never mentions + // MOLLIE_API_KEY satisfies any absence assertion. Concretely: the container restarts + // with a regenerated AUTH_SECRET mid-run, the JWT is rejected, and an absence-only + // check reports the key as correctly configured. + await expect(page.getByRole("heading", { name: /billing/i }).first()).toBeVisible(); + await expect( + page.getByText(/MOLLIE_API_KEY is not set/i), + "a Mollie key should be reaching the staging container", + ).toHaveCount(0); + }); + + test("provisioning is deliberately DISARMED here", async () => { + // The most valuable thing this environment gets wrong on purpose. `PROVISION_GITHUB_TOKEN` + // is left empty in the staging service, so nothing here can open a registry PR or + // dispatch the provisioning chain against the real deploy repo. Asserted positively, + // because the failure to catch is someone helpfully "fixing" the missing env var and + // handing a test environment write access to production infrastructure. + const res = await page.goto("/admin/provision", { waitUntil: "domcontentloaded" }); + expect(res?.status()).toBe(200); + await expect( + page.getByText(/set PROVISION_GITHUB_TOKEN/i), + "staging must NOT be able to dispatch real provisioning", + ).toBeVisible(); + }); + }); +}); From 91dad52ac8b9da629b0bd0e5d386ccaf5d943db0 Mon Sep 17 00:00:00 2001 From: mahmutKaya <33642821+mahmutkaya@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:08:51 +0200 Subject: [PATCH 5/8] docs: the deployed-staging suite and the E2E_REMOTE interlock (#109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records both halves in §7 so the next session in this repo does not have to rediscover them: what test:e2e:staging covers (only deploy-time facts), what it deliberately does NOT prove (image currency, test_ vs live_ key), the .env quoting requirement, and why the remote/local split is enforced in the config rather than as a test.skip in each spec. Co-authored-by: Claude Opus 5 --- CLAUDE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 794f55c..ed8dbcb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,6 +59,8 @@ Output before writing code: (1) which `require*()` guard covers each new surface - **CI** (`.github/workflows/ci.yml`): typecheck · eslint · next build · prisma migrations-apply + drift check · **vitest unit + coverage floor** · **i18n parity (6 locales)** · **file-length** · **playwright login smoke** · gitleaks · TruffleHog · Trivy fs · npm audit · OSV · semgrep; weekly `security-audit.yml`. SonarCloud autoscan (no CI job — don't add one). - **Review gate ACTIVE in this repo** (since 2026-07-07): Stop + PreToolUse + PostToolUse (file-length checker) hooks (`.claude/settings.json`) + git pre-push → workspace `scripts/review-gate/` with the `sofra.md` overlay. No `--no-verify`, no bypasses. - **E2E, unmocked** — `npm run test:e2e:full` (`scripts/e2e-suite.sh`) stands up a throwaway Postgres, migrates, seeds, builds and runs the whole Playwright suite: the O2 self-serve funnel (`tests/e2e/self-serve-signup.spec.ts`, 12 tests) and **a real Mollie first payment on the `test_` key** (`tests/e2e/billing-mollie.spec.ts`). Three things to know before touching it: (1) the suite refuses to start on a `live_` key and the billing spec **skips with a stated reason** when no test key is present — it never silently passes; (2) it uses its own registry fixture, because since O2 an unreadable registry makes the signup fail **closed**, so an absent one is not a neutral condition; (3) each test gets a unique `x-forwarded-for` (`tests/e2e/helpers/fixtures.ts`) because the intake allows only 5 POSTs/IP/15min — the header, not a loosened limit, is how the suite avoids rate-limiting itself, and the worker index must be part of it or parallel workers collide. Mollie cannot call localhost (it validates reachability and 422s), so `MOLLIE_WEBHOOK_URL` points at an inert sink and the spec POSTs the real `tr_` id to the local handler — fetch-and-verify still runs against the real API; only the delivery hop is stood in for. +- **E2E against DEPLOYED staging** — `npm run test:e2e:staging` (`tests/e2e/staging-live.spec.ts`) runs against `https://staging.sofrapiwas.com`. Disjoint from the local suite by design: it covers only what cannot exist until something is deployed — the box `.env` reaching the container with the *right* values, the founder-run `:migrate-staging` one-off, Caddy + TLS, and that the deployed bake is the staging one. **Read-only by construction** (no account, no payment, no row), so there is nothing to restore. Needs `STAGING_ADMIN='{email: …, password: …}'` in the gitignored `.env` — **the quotes are load-bearing**, because `scripts/e2e-suite.sh` sources that same file with `set -a && . ./.env` and bash reads an unquoted brace value as an assignment plus a command, killing the whole local suite under `set -euo pipefail`. Two things it deliberately does NOT prove, so don't read them into a green run: that the deployed image is *current* (no health/version endpoint — a months-old `:staging` bake passes), and that the Mollie key is `test_` rather than `live_` (`mollieConfigured()` reports only that some key is set). +- **The `E2E_REMOTE` interlock** (`playwright.config.ts`): remote runs the staging spec **and nothing else**; local runs everything **but** the staging spec. Not a convenience — every other spec here mutates (signup rows, real test-key payments, repointed billing records) and reaches its target through relative `page.goto`, so an unrestricted remote run would write into the long-lived shared environment, which has no throwaway DB to drop. Keep it enforced in the config rather than as a `test.skip` in each spec: that is what lets a missing credential be a hard error instead of a skip that exits 0 having verified nothing behind the login. - **Tests** (DEV-PHASES-PLAN W1/W2): `npm run test` = Vitest unit suite over the pure `lib/` modules (format, mollie amount, validation schemas, rate-limit, tenant-registry, email-templates, email helpers — no DB/network; Mollie is never called). `npm run test:coverage` adds the **coverage floor** (W2 D9): v8 coverage scoped to those fully-unit-coverable pure modules (`vitest.config.ts` `coverage.include`), enforced in CI at ≥95% lines/statements/functions, ≥90% branches — modules with network/DB branches stay out of scope (they need mocks §7 forbids). Raise the floor as coverage grows. `npm run test:e2e` = Playwright login smoke (admin→/admin, partner→/dashboard, partner-blocked-from-/admin) against a seeded throwaway DB (`scripts/seed-e2e.mjs`). `scripts/e2e-local.mjs` (the no-browser progressive-enhancement walk of the partner program; needs a **clean** local DB — leftover LIVE client collides on `tenantSlug`) + the QA test accounts remain for manual/full-flow checks. ## §8 — Git workflow From 58bc7b1effc96a6f626aac5a4ab370057677c405 Mon Sep 17 00:00:00 2001 From: mahmutKaya <33642821+mahmutkaya@users.noreply.github.com> Date: Fri, 31 Jul 2026 13:02:58 +0200 Subject: [PATCH 6/8] feat(health): /api/health with build identity, and an indexing monitor (#110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(health): /api/health with build identity, and an indexing monitor Two gaps the deployed-staging suite had to admit to in its own docstring. /api/health reports the commit its image was built from (Dockerfile ARG BUILD_SHA, supplied by build-image.yml for both the prod and staging bakes). Without it nothing could tell a current deployment from a months-old one: pages render, login works and env vars are wired just as happily on a stale image, so a green suite could certify an environment nobody had rolled since the change under test was written. The staging suite now judges currency against this clone rather than a wall-clock threshold — an unknown commit fails, a diverged line fails, and a legitimately-behind-but-linear deployment is PRINTED rather than silently assumed current. The endpoint is unauthenticated because it is the Docker HEALTHCHECK target and a monitoring endpoint, and that is safe only because it touches nothing: no database, no session, no env beyond the two public build stamps. A local spec pins the payload to exactly four keys so this stays true; it fails the moment anything else is added. Liveness stays separate from readiness on purpose — pinging Postgres from a public unauthenticated route is a DoS lever and would down a container on a blip the app itself rides out. The Docker HEALTHCHECK now probes this instead of rendering /en. The production-robots assertion moves out of the staging suite into a daily indexing-monitor workflow. It was the wrong home twice: it only ran when someone happened to touch staging, while the failure it guards is a slow burn — a staging twin quietly accruing index coverage for weeks — and it turned a PRODUCTION fault red inside a suite named for staging. Daily, not uptime.yml's 5 minutes: robots changes only on deploy, so that cadence would re-alert 288 times on one regression. Verified: test:e2e:full 26 passed; the leak guard fails when DATABASE_URL is added to the payload; the monitor's checks run clean against the live properties. Co-Authored-By: Claude Opus 5 * fix(health): address review — currency check, prod de-index gap, CI wiring Two HIGH findings, both real. The currency check compared against local HEAD, which fails on the NORMAL workflow: pr-merge-gate.sh squashes every feature PR, so right after your change ships, local HEAD holds the unsquashed commits while the image was built from the squash commit — neither contains the other, so it cried "DIVERGED" exactly when the deployment was correct. Any unrelated PR landing on develop did the same, and a check that is red by default gets ignored, costing the very signal this was built to add. Now judged by reachability from origin/develop (with a best-effort fetch, since the deployment can legitimately be newer than this clone), which is stable whatever branch you are on. Also swapped `cat-file -e` for a real ancestry test: it succeeds for any object still in the local store, so a pre-squash or force-pushed commit passed while being unreachable from any branch — the exact case the failure message claimed to catch. The monitor never checked PRODUCTION's X-Robots-Tag. The Caddyfile's prod and staging blocks are near-identical and ~15 lines apart, so copying the staging noindex header up is a plausible edit — it would deindex the live site while robots.txt still said Allow, and every existing check would pass. Mutation-verified: pointed at a host that really does serve noindex, the new check fires. Also: an empty git result folded to zero through Number(""), so any rev-list failure read as "0 behind, 0 ahead" — a green check that compared nothing. Nothing in CI proved BUILD_SHA was wired at all (the Playwright job never touches the Dockerfile, so version is always the "unknown" fallback there), so both image builds now assert the baked value matches github.sha. Dockerfile stamps moved below the COPY layers, where a per-build-changing ENV no longer busts the runner-stage cache. Monitor fixes: the 000000 HTTP code on connection failure, a bounded and ::-stripped echo of a remote body, and the scope note that staging.fooderist.com is the RUMI frontend despite uptime.yml's label. Reverted the Docker HEALTHCHECK to /en. Nothing declares depends_on service_healthy, so the probe's only consumer is a human reading docker ps, and /api/health is dependency-free by design — it answers 200 while every page 500s on an i18n regression. Cheaper but strictly less informative was a bad trade. Corrected two claims of my own that were wrong: robots.txt is BAKED (NEXT_PUBLIC_SITE_URL is a build arg), not decided at runtime, so a wrong posture cannot be fixed by editing the box .env; and this repo and its GHCR images are PUBLIC, so the route's docblock now reasons from that rather than a private-repo assumption, and names the patch-gap oracle it leaves. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- .github/workflows/build-image.yml | 56 ++++++++ .github/workflows/indexing-monitor.yml | 171 +++++++++++++++++++++++++ Dockerfile | 13 ++ app/api/health/route.ts | 60 +++++++++ healthcheck.js | 7 + tests/e2e/health.spec.ts | 50 ++++++++ tests/e2e/staging-live.spec.ts | 100 ++++++++++++--- 7 files changed, 437 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/indexing-monitor.yml create mode 100644 app/api/health/route.ts create mode 100644 tests/e2e/health.spec.ts diff --git a/.github/workflows/build-image.yml b/.github/workflows/build-image.yml index 5edb5cd..42f9b06 100644 --- a/.github/workflows/build-image.yml +++ b/.github/workflows/build-image.yml @@ -41,6 +41,13 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} + - id: stamp + # Wall-clock build time, stamped here rather than taken from a github.event + # field: the event payloads differ per trigger (push vs tag vs dispatch), and + # repository.updated_at answers "when did the repo change", which is not the + # question /api/health is asked. + run: echo "built_at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$GITHUB_OUTPUT" + - id: meta uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: @@ -61,11 +68,32 @@ jobs: push: true build-args: | NEXT_PUBLIC_SITE_URL=${{ vars.SITE_URL || 'https://sofrapiwas.com' }} + BUILD_SHA=${{ github.sha }} + BUILD_TIME=${{ steps.stamp.outputs.built_at }} tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} cache-from: type=gha cache-to: type=gha,mode=max + - name: Verify the build identity actually reached the image + # Without this, nothing in CI ever proves BUILD_SHA is wired: the Playwright smoke + # runs `next build` directly and never touches the Dockerfile, so `version` is always + # the "unknown" fallback there and the local spec's toHaveProperty passes regardless. + # Drop the build-arg during an unrelated edit and every gate stays green — the break + # would surface only when a human remembered to run the staging suite by hand. + # Reads the env straight out of the pushed image; no server boot needed. + env: + IMAGE: ghcr.io/${{ github.repository }}:sha-${{ github.sha }} + run: | + set -euo pipefail + got=$(docker run --rm --entrypoint node "$IMAGE" \ + -e 'process.stdout.write(process.env.BUILD_SHA || "MISSING")') + echo "image reports BUILD_SHA=$got" + [ "$got" = "${GITHUB_SHA}" ] || { + echo "::error::image BUILD_SHA is '$got', expected '${GITHUB_SHA}' — /api/health would misreport what is deployed" + exit 1 + } + # One-off DB tooling image (prisma migrate deploy + admin seed) — pulled # on the box only when a release ships migrations. See DEPLOYMENT.md. # Published as a TAG of the main (public) package — a separate @@ -116,6 +144,13 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} + - id: stamp + # Wall-clock build time, stamped here rather than taken from a github.event + # field: the event payloads differ per trigger (push vs tag vs dispatch), and + # repository.updated_at answers "when did the repo change", which is not the + # question /api/health is asked. + run: echo "built_at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$GITHUB_OUTPUT" + - id: meta-staging uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: @@ -133,11 +168,32 @@ jobs: push: true build-args: | NEXT_PUBLIC_SITE_URL=${{ vars.STAGING_SITE_URL || 'https://staging.sofrapiwas.com' }} + BUILD_SHA=${{ github.sha }} + BUILD_TIME=${{ steps.stamp.outputs.built_at }} tags: ${{ steps.meta-staging.outputs.tags }} labels: ${{ steps.meta-staging.outputs.labels }} cache-from: type=gha cache-to: type=gha,mode=max + - name: Verify the build identity actually reached the image + # Without this, nothing in CI ever proves BUILD_SHA is wired: the Playwright smoke + # runs `next build` directly and never touches the Dockerfile, so `version` is always + # the "unknown" fallback there and the local spec's toHaveProperty passes regardless. + # Drop the build-arg during an unrelated edit and every gate stays green — the break + # would surface only when a human remembered to run the staging suite by hand. + # Reads the env straight out of the pushed image; no server boot needed. + env: + IMAGE: ghcr.io/${{ github.repository }}:staging-${{ github.sha }} + run: | + set -euo pipefail + got=$(docker run --rm --entrypoint node "$IMAGE" \ + -e 'process.stdout.write(process.env.BUILD_SHA || "MISSING")') + echo "image reports BUILD_SHA=$got" + [ "$got" = "${GITHUB_SHA}" ] || { + echo "::error::image BUILD_SHA is '$got', expected '${GITHUB_SHA}' — /api/health would misreport what is deployed" + exit 1 + } + # The DB-tooling image for staging. It has to be built HERE, in the job that runs # on develop — putting the tag on `build-push`'s meta-migrate step made it dead # code the moment that job started skipping on develop, and the tag simply never diff --git a/.github/workflows/indexing-monitor.yml b/.github/workflows/indexing-monitor.yml new file mode 100644 index 0000000..cadfb81 --- /dev/null +++ b/.github/workflows/indexing-monitor.yml @@ -0,0 +1,171 @@ +name: indexing monitor + +# Guards the INDEXING POSTURE of every public Sofra property: production must stay +# crawlable, and every non-canonical copy must stay invisible. +# +# Why a monitor and not a test. This assertion used to live in the deployed-staging e2e +# suite, which was the wrong home twice over. It only ran when someone happened to touch +# staging — and the failure it guards against is a slow burn: a staging twin quietly +# accumulating index coverage for weeks, competing with the real site for exactly the +# citations the AEO work exists to win. It also turned a *production* fault red inside a +# suite named for staging, which sends you to look at the wrong environment. +# +# `app/robots.ts` decides between allow-all and disallow-all from `NEXT_PUBLIC_SITE_URL`, +# which is a BUILD arg (lib/seo.ts) — so `/robots.txt` is prerendered into each image and +# both directions are one bake away from flipping. Nothing in CI can see it, because the +# served value depends on which image landed on which host. Note the mechanism: it is not +# a runtime env read, so this can NOT be corrected by editing the box `.env` — it takes a +# rebuild. Someone debugging at 3am will try the env first. +# +# Daily, deliberately. This changes only when something is deployed, so the ~5-minute +# cadence of uptime.yml would buy nothing and re-alert 288 times a day on one regression. +# +# Alerting reuses uptime.yml's secrets; it stays silent (with a warning) until both exist: +# gh secret set TELEGRAM_BOT_TOKEN -R piwas-21/sofra +# gh secret set TELEGRAM_CHAT_ID -R piwas-21/sofra + +on: + schedule: + - cron: "17 6 * * *" # daily ~06:17 UTC (off the hour — cron load is spiky on the hour) + workflow_dispatch: {} + +permissions: + contents: read + +concurrency: + group: indexing-monitor + cancel-in-progress: false + +jobs: + probe: + name: robots posture + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Check robots posture and alert on regression + env: + TELEGRAM_BOT_TOKEN: ${{ secrets.TELEGRAM_BOT_TOKEN }} + TELEGRAM_CHAT_ID: ${{ secrets.TELEGRAM_CHAT_ID }} + run: | + set -uo pipefail + + problems=() + # Scratch files go in a temp dir, not the working directory: the runner has a + # clean workspace, but anyone running this body locally to debug it would + # otherwise drop prod.txt/hidden.txt into the repo root — where a `git add -A` + # sweeps them into a commit. That happened on this very PR. + work=$(mktemp -d) + trap 'rm -rf "$work"' EXIT + + # Fetch to a file rather than piping into a matcher: on failure the body can be + # echoed into the log, which is the difference between "robots.txt is wrong" and + # "robots.txt is wrong AND here is what it actually said". + fetch() { # fetch -> prints http code + # `code=...; || code=000` rather than `|| echo 000`: curl PRINTS 000 on a + # connection failure AND exits non-zero, so appending would emit "000000" and the + # alert would read `HTTP 000000`. uptime.yml hit this and documents the same fix. + local code + code=$(curl -sS -o "$2" -w '%{http_code}' --max-time 20 \ + --retry 2 --retry-delay 5 --retry-all-errors "$1" 2>/dev/null) || code=000 + [ -z "$code" ] && code=000 + echo "$code" + } + + # `x-robots-tag: …` for a URL, or empty. Lowercased so the caller matches once. + robots_header() { + curl -sSI --max-time 20 "$1" 2>/dev/null | tr -d '\r' | tr 'A-Z' 'a-z' \ + | grep '^x-robots-tag:' || true + } + + # --- production MUST stay crawlable ------------------------------------------- + code=$(fetch "https://sofrapiwas.com/robots.txt" "$work/prod.txt") + if [ "$code" != "200" ]; then + problems+=("production robots.txt unreachable — HTTP $code") + else + # `Allow: /` with a capital A: Next writes `Disallow: /` for the deny case, so a + # case-insensitive match would find the substring "allow: /" inside it and pass + # on precisely the regression this exists to catch. + grep -qE '^Allow:[[:space:]]*/' "$work/prod.txt" \ + || problems+=("production robots.txt no longer allows crawling") + grep -q 'GPTBot' "$work/prod.txt" \ + || problems+=("production robots.txt no longer invites AI crawlers (AEO)") + fi + + # Production must ALSO carry no noindex header. Checked because robots.txt and the + # header are set in different places and can disagree: the Caddyfile's production + # and staging blocks are near-identical and sit ~15 lines apart, so copying the + # staging `header X-Robots-Tag "noindex, …"` line into the production block is a + # plausible edit. That deindexes the live site while robots.txt still says Allow — + # every content check above passes, and the monitor would report "crawlable ✅" + # while the site silently leaves every index. This is the revenue-costing direction. + prod_hdr=$(robots_header "https://sofrapiwas.com/en") + case "$prod_hdr" in + *noindex*|*none*) problems+=("production carries a de-indexing header: ${prod_hdr}") ;; + esac + + # --- every non-canonical copy of THIS app MUST stay hidden --------------------- + # url|label. Scope is deliberate: copies of the sofra marketing site, which is what + # competes with sofrapiwas.com for citations. `staging.fooderist.com` is NOT here + # despite uptime.yml labelling it "Sofra staging" — that label is stale, the host + # serves the RUMI tenant frontend, and it has no robots.txt at all (404). Its + # indexing posture is a question for the frontend repo, not this one. + hidden=( + "https://staging.sofrapiwas.com|sofra staging" + ) + for entry in "${hidden[@]}"; do + url="${entry%%|*}"; label="${entry##*|}" + code=$(fetch "$url/robots.txt" "$work/hidden.txt") + if [ "$code" != "200" ]; then + problems+=("$label robots.txt unreachable — HTTP $code") + continue + fi + if ! grep -qE '^Disallow:[[:space:]]*/[[:space:]]*$' "$work/hidden.txt"; then + problems+=("$label is INDEXABLE — robots.txt no longer disallows") + # Bounded, and `::` stripped: GitHub parses ::workflow commands from step + # stdout, and this is a remote response body we do not control. + echo "--- $label robots.txt ---"; head -c 2000 "$work/hidden.txt" | sed 's/^:://' + fi + grep -q 'GPTBot' "$work/hidden.txt" \ + && problems+=("$label invites AI crawlers — it must not") + + # The header too, for crawlers that never fetch robots.txt. Caddy adds it, so + # this catches a reverse-proxy regression the app itself cannot see. + hdr=$(robots_header "$url/en") + case "$hdr" in + *noindex*) ;; + *) problems+=("$label missing X-Robots-Tag: noindex (got: ${hdr:-none})") ;; + esac + done + + { + echo "### Indexing posture" + if [ ${#problems[@]} -eq 0 ]; then + echo "Production crawlable, non-canonical copies hidden ✅" + else + echo "Problems:"; for p in "${problems[@]}"; do echo "- $p"; done + fi + } >> "$GITHUB_STEP_SUMMARY" + + if [ ${#problems[@]} -eq 0 ]; then echo "Indexing posture correct."; exit 0; fi + + for p in "${problems[@]}"; do echo "PROBLEM: $p"; done + + run_url="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" + msg=$'\xF0\x9F\x94\x8D Indexing posture regression:\n' + for p in "${problems[@]}"; do msg+="• ${p}"$'\n'; done + msg+=$'\n'"${run_url}" + + if [ -z "${TELEGRAM_BOT_TOKEN}" ] || [ -z "${TELEGRAM_CHAT_ID}" ]; then + echo "::warning::Regression found but TELEGRAM_BOT_TOKEN/TELEGRAM_CHAT_ID not set — no alert sent." + else + curl -sS --max-time 20 \ + -X POST "https://api.telegram.org/bot${TELEGRAM_BOT_TOKEN}/sendMessage" \ + --data-urlencode "chat_id=${TELEGRAM_CHAT_ID}" \ + --data-urlencode "text=${msg}" \ + -o /dev/null -w 'telegram sendMessage: %{http_code}\n' \ + || echo "::warning::Telegram send failed" + fi + + # Fail the run as well as alerting: the Actions UI is the durable record, and a + # green history next to a Telegram message nobody kept is worse than neither. + exit 1 diff --git a/Dockerfile b/Dockerfile index 8744dfd..9e9b4d3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -60,6 +60,19 @@ COPY --from=builder --chown=nextjs:nodejs /app/healthcheck.js ./healthcheck.js # sibling `migrate` target (published as ghcr.io/piwas-21/sofra:migrate), # keeping this runtime image slim. See DEPLOYMENT.md for the one-off commands. +# Build identity, surfaced by /api/health. Read at request time, not compiled into the +# bundle, so they can be overridden on the box if an image is ever re-tagged by hand. +# Without them a deployed environment cannot be told apart from a months-old one — every +# other health signal passes either way. +# +# Placed HERE, below every COPY, because BUILD_TIME changes on every single build: any +# layer after it is rebuilt every time. Above the COPYs it would defeat `cache-from: gha` +# for the whole runner stage — the npm removal, the user creation and all three COPYs. +ARG BUILD_SHA=unknown +ARG BUILD_TIME=unknown +ENV BUILD_SHA=${BUILD_SHA} +ENV BUILD_TIME=${BUILD_TIME} + USER nextjs EXPOSE 3000 ENV PORT=3000 diff --git a/app/api/health/route.ts b/app/api/health/route.ts new file mode 100644 index 0000000..e6b7763 --- /dev/null +++ b/app/api/health/route.ts @@ -0,0 +1,60 @@ +import { NextResponse } from "next/server"; + +/** + * Liveness + **build identity**. + * + * The build identity is the reason this exists. Without it there is no way to tell a + * current deployment from a months-old one: every other check — pages render, login works, + * env vars are wired — passes just as happily against a stale image, so a deployed-staging + * test suite could report a clean bill of health for an environment nobody had rolled since + * the change under test was written. `version` closes that hole; it is baked at image build + * time from the commit SHA (Dockerfile `ARG BUILD_SHA`, supplied by build-image.yml). + * + * **Deliberately unauthenticated**, like every other route under `app/api/` (§5.1's guard + * rule covers `(control)` surfaces, which this is not). That is safe only because it touches + * nothing: no database, no session, no request body, no env beyond the two build stamps. Do + * not add anything here that reads user data or config — `tests/e2e/health.spec.ts` pins the + * payload to exactly these four keys so that stays true. + * + * **On publishing the commit SHA.** This repo is PUBLIC and its images are anonymously + * pullable from GHCR (`ghcr.io/piwas-21/sofra:latest` answers without credentials), so the + * source and the exact lockfile of any published build are already readable by anyone. What + * this adds is only *which* published build is live right now. The residual is real and + * worth naming: with a public repo and manual rollouts (CLAUDE.md §8), the pair + * `version` + `builtAt` is a patch-gap oracle — a caller can tell whether the live billing + * app has been rolled to a merged security fix yet, and poll until it has. Dropping + * `builtAt` would not close that; the SHA alone dates itself against the public history. + * Closing it means either a private repo or not publishing identity at all, which is a + * posture decision rather than a code one. + * + * **No rate limit**, unlike the sibling intake routes. Those meter because they write, send + * mail and cost money per call; this allocates one small object and touches no I/O. A + * limiter here would also be actively harmful: it would let request volume from anywhere + * turn a liveness probe into a 429 and report a perfectly healthy container as down. + * + * **Deliberately dependency-free.** `status: "ok"` means *this process is serving HTTP* and + * nothing more — it does NOT mean the database is reachable. Liveness and readiness are + * separate on purpose: pinging Postgres from an unauthenticated public endpoint makes a + * cheap request expensive, which is a DoS lever, and it would take the container down on a + * blip that the app itself recovers from. Whether the DB is healthy is already proven by + * anything that signs in. + */ + +// Never prerendered or cached: the point is to report what THIS running container is, and a +// static answer captured at build time would survive an env override on the box. +export const dynamic = "force-dynamic"; + +export function GET() { + return NextResponse.json( + { + status: "ok", + // Distinguishes this app from the tenant backend, which answers its own health probe + // with `service: "restaurant-system-api"` — a monitor pointed at the wrong host by a + // DNS or Caddy mistake would otherwise see a healthy 200 and report all clear. + service: "sofra-control-plane", + version: process.env.BUILD_SHA || "unknown", + builtAt: process.env.BUILD_TIME || "unknown", + }, + { headers: { "cache-control": "no-store" } }, + ); +} diff --git a/healthcheck.js b/healthcheck.js index 13f87de..e8f0009 100644 --- a/healthcheck.js +++ b/healthcheck.js @@ -1,4 +1,11 @@ // Docker HEALTHCHECK probe — not imported by the app (see Dockerfile note). +// +// Deliberately probes a RENDERED page, not /api/health. /api/health is dependency-free +// by design, so it answers 200 while every marketing page 500s on an i18n or +// message-catalog regression — and since nothing declares `depends_on: service_healthy` +// for this service, the only consumer of this probe is the STATUS column a human reads. +// A cheaper probe that proves less is a bad trade there. /en proves the App Router +// served HTML with messages resolved; the 5s timeout was never what a render failed. const http = require("http"); const req = http.get( diff --git a/tests/e2e/health.spec.ts b/tests/e2e/health.spec.ts new file mode 100644 index 0000000..fa7eac7 --- /dev/null +++ b/tests/e2e/health.spec.ts @@ -0,0 +1,50 @@ +import { expect, test } from "./helpers/fixtures"; + +/** + * `/api/health` against the LOCAL production build. + * + * Deliberately separate from the deployed-staging suite. That one asks "is the thing on the + * box current?" and needs a real deployment to answer; this one asks "does the endpoint work + * at all?", which is a property of the code and belongs in CI — where it runs on every PR, + * including the ones that would break it. Without this, the route's only coverage would be a + * suite that runs against an environment somebody has to remember to roll first. + * + * It also guards the Docker HEALTHCHECK, which now probes this path: break the route and + * every container goes unhealthy on its next check, which is a much worse way to find out. + * + * No credentials, no database, no seeded state — it must pass on a bare build. + */ +test.describe("/api/health", () => { + test("reports liveness and identifies the service", async ({ request }) => { + const res = await request.get("/api/health"); + expect(res.status()).toBe(200); + + const body = (await res.json()) as Record; + expect(body.status).toBe("ok"); + // The name is load-bearing, not decoration: it is how a probe pointed at the wrong host + // tells "healthy" from "healthy, but this is the tenant backend". + expect(body.service).toBe("sofra-control-plane"); + + // Present even in a build where the Dockerfile ARGs never ran — the endpoint must report + // `unknown` rather than omit the field, because a consumer checking currency needs to + // tell "no identity baked" from "no such key". + expect(body).toHaveProperty("version"); + expect(body).toHaveProperty("builtAt"); + }); + + test("is never cached", async ({ request }) => { + // A cached health response is a lie with a timestamp on it: the whole point is to report + // what THIS container is right now, and an intermediary serving a stored copy would keep + // answering for a container that had already gone. + const res = await request.get("/api/health"); + expect(res.headers()["cache-control"] ?? "").toContain("no-store"); + }); + + test("leaks no configuration", async ({ request }) => { + // Unauthenticated by necessity (Docker HEALTHCHECK, external monitors), so the payload is + // pinned to exactly four public keys. This fails the moment someone "helpfully" adds a + // database status, an env dump or a Mollie mode to it. + const body = (await (await request.get("/api/health")).json()) as Record; + expect(Object.keys(body).sort()).toEqual(["builtAt", "service", "status", "version"]); + }); +}); diff --git a/tests/e2e/staging-live.spec.ts b/tests/e2e/staging-live.spec.ts index a186591..fe4eab7 100644 --- a/tests/e2e/staging-live.spec.ts +++ b/tests/e2e/staging-live.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from "./helpers/fixtures"; import { CANONICAL_SITE_URL } from "@/lib/seo"; +import { execFileSync } from "node:child_process"; import { readFileSync } from "node:fs"; /** @@ -20,13 +21,16 @@ import { readFileSync } from "node:fs"; * could never fire, and a `:migrate-staging` tag that was dead code — both invisible until * something tried to pull the artifact. * - * What this suite does NOT prove, so nobody reads more into a green run than is there: - * - that the deployed image is CURRENT. A months-old `:staging` bake passes everything - * here. There is no version or health endpoint to assert against; adding one is the - * fix, not a cleverer assertion. - * - that the Mollie key is a `test_` key rather than a `live_` one. `mollieConfigured()` - * reports only that SOME key is set and nothing surfaces the prefix — see the billing - * test, which is named for what it can actually establish. + * Image currency IS now checked — `/api/health` reports the commit the image was built + * from, and the check judges it against this clone rather than a wall-clock threshold. + * Production's own indexing posture is NOT checked here any more: it moved to the + * `indexing-monitor` workflow, because it only ran when someone happened to touch staging, + * and it turned a production fault red inside a suite named for staging. + * + * What this suite still does NOT prove, so nobody reads more into a green run than is + * there: that the Mollie key is a `test_` key rather than a `live_` one. + * `mollieConfigured()` reports only that SOME key is set and nothing surfaces the prefix — + * see the billing test, which is named for what it can actually establish. * * READ-ONLY BY CONSTRUCTION. It signs in and looks: no account, no payment, no row. There * is nothing to restore and no way for a failed run to leave the environment dirty. The @@ -129,19 +133,75 @@ test.describe("the deployed staging control plane", () => { expect(res?.headers()["x-robots-tag"] ?? "", "X-Robots-Tag").toMatch(/noindex/i); }); - test("PRODUCTION is still crawlable (fails here, but the fault is on sofrapiwas.com)", async ({ request }) => { - // Deliberately asserted from the staging suite: the same commit that flips staging to - // noindex could flip production, and `robots.ts` decides between them at RUNTIME from the - // deployment's own base URL. A staging suite that never looks at production reports - // all-clear either way. The title carries the attribution because a prod incident or a - // Caddy blip turns this red inside a describe named for staging, and the reflex is to go - // look at the wrong environment. This wants a scheduled monitor eventually; until one - // exists, an assertion that runs is worth more than a plan for one that would. - const res = await request.get(`${CANONICAL_SITE_URL}/robots.txt`); - expect(res.status()).toBe(200); - const prod = await res.text(); - expect(prod, "production robots.txt must still allow crawling").toMatch(/^Allow:\s*\//m); - expect(prod, "production must still invite AI crawlers (AEO)").toMatch(/GPTBot/i); + test("the deployed image identifies itself, and is not from a diverged line", async ({ request }) => { + // Closes the hole this suite used to have to admit to: everything else here passes just + // as happily against a months-old bake, so a green run could certify an environment + // nobody had rolled since the change under test was written. + const res = await request.get(`${BASE}/api/health`); + expect(res.status(), "/api/health should serve").toBe(200); + const body = (await res.json()) as { status?: string; service?: string; version?: string; builtAt?: string }; + + expect(body.status).toBe("ok"); + // Names the app, not just "healthy": a monitor or a suite pointed at the wrong host by a + // DNS or Caddy mistake would otherwise see a 200 and report all clear. The tenant backend + // answers its own probe with `service: "restaurant-system-api"`. + expect(body.service, "wrong service answered — check where this host actually points").toBe( + "sofra-control-plane", + ); + expect(body.version, "image was built without BUILD_SHA — build-image.yml must pass it").toMatch( + /^[0-9a-f]{40}$/, + ); + + // Currency is judged against `origin/develop` — the branch this environment tracks — and + // NOT against local HEAD. HEAD is the wrong reference in the normal case: the merge gate + // SQUASHES every feature PR, so right after your own change ships, local HEAD holds the + // unsquashed commits while the deployed image was built from the squash commit. Neither + // contains the other, so a HEAD-relative divergence test fails at the exact moment the + // deployment is correct and current — and a check that is red by default gets ignored, + // which costs the very signal this test exists to add. Any unrelated PR landing on + // develop while you sit on a feature branch does the same thing. + const sha = body.version!; + const git = (args: string[]): string | null => { + try { + const out = execFileSync("git", args, { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }).trim(); + // Empty output is NOT success. `Number("")` is 0, so folding it into a count would + // make a failed comparison read as "0 behind, 0 ahead" — a confidently green currency + // check that compared nothing at all. + return out === "" ? null : out; + } catch { + return null; + } + }; + + // Best-effort refresh: the deployed image can legitimately be NEWER than anything this + // clone has seen (someone else merged), and without this that reads as "unknown commit". + git(["fetch", "--quiet", "origin", "develop"]); + + // Reachability from origin/develop, not mere object existence. `cat-file -e` succeeds for + // any object still in the local store — a pre-squash commit or one from a force-pushed + // branch lingers there for weeks and would pass while being unreachable from any branch, + // which is precisely the "deployed from outside this repo" case the failure claims to catch. + const onDevelop = (() => { + try { + execFileSync("git", ["merge-base", "--is-ancestor", sha, "origin/develop"], { stdio: "ignore" }); + return true; + } catch { + return false; + } + })(); + + const behind = git(["rev-list", "--count", `${sha}..origin/develop`]); + // Printed unconditionally, because a stale-but-linear deployment is legitimate (rollouts + // are manual, CLAUDE.md §8) and the only real fix is making it VISIBLE rather than letting + // a green run imply it is current. + console.log( + `deployed ${sha.slice(0, 8)} (built ${body.builtAt}) — ${behind ?? "?"} commit(s) behind origin/develop`, + ); + expect( + onDevelop, + `deployed commit ${sha.slice(0, 8)} is not on origin/develop — it was built from a line this repo does not track, or the clone could not be refreshed`, + ).toBe(true); + expect(behind, "could not measure drift against origin/develop — is this a shallow clone?").not.toBeNull(); }); // ONE login for all the authed assertions. `lib/auth.ts` allows 10 per email per 15 From ba98d62c31e6b33edf1a4bd0bbdc32f448d751cc Mon Sep 17 00:00:00 2001 From: mahmutKaya <33642821+mahmutkaya@users.noreply.github.com> Date: Fri, 31 Jul 2026 13:13:03 +0200 Subject: [PATCH 7/8] docs: /api/health and the indexing monitor (#111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records both in §7 so neither is rediscovered: what /api/health does and pointedly does not mean (ok != database up), why the payload is pinned to four keys, why the Docker HEALTHCHECK still probes /en, and that robots.txt is BAKED so a wrong indexing posture takes a rebuild rather than a box .env edit. Co-authored-by: Claude Opus 5 --- CLAUDE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index ed8dbcb..3e34716 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,6 +59,8 @@ Output before writing code: (1) which `require*()` guard covers each new surface - **CI** (`.github/workflows/ci.yml`): typecheck · eslint · next build · prisma migrations-apply + drift check · **vitest unit + coverage floor** · **i18n parity (6 locales)** · **file-length** · **playwright login smoke** · gitleaks · TruffleHog · Trivy fs · npm audit · OSV · semgrep; weekly `security-audit.yml`. SonarCloud autoscan (no CI job — don't add one). - **Review gate ACTIVE in this repo** (since 2026-07-07): Stop + PreToolUse + PostToolUse (file-length checker) hooks (`.claude/settings.json`) + git pre-push → workspace `scripts/review-gate/` with the `sofra.md` overlay. No `--no-verify`, no bypasses. - **E2E, unmocked** — `npm run test:e2e:full` (`scripts/e2e-suite.sh`) stands up a throwaway Postgres, migrates, seeds, builds and runs the whole Playwright suite: the O2 self-serve funnel (`tests/e2e/self-serve-signup.spec.ts`, 12 tests) and **a real Mollie first payment on the `test_` key** (`tests/e2e/billing-mollie.spec.ts`). Three things to know before touching it: (1) the suite refuses to start on a `live_` key and the billing spec **skips with a stated reason** when no test key is present — it never silently passes; (2) it uses its own registry fixture, because since O2 an unreadable registry makes the signup fail **closed**, so an absent one is not a neutral condition; (3) each test gets a unique `x-forwarded-for` (`tests/e2e/helpers/fixtures.ts`) because the intake allows only 5 POSTs/IP/15min — the header, not a loosened limit, is how the suite avoids rate-limiting itself, and the worker index must be part of it or parallel workers collide. Mollie cannot call localhost (it validates reachability and 422s), so `MOLLIE_WEBHOOK_URL` points at an inert sink and the spec POSTs the real `tr_` id to the local handler — fetch-and-verify still runs against the real API; only the delivery hop is stood in for. +- **`/api/health`** (`app/api/health/route.ts`) — public, unauthenticated, **dependency-free**: `{status, service, version, builtAt}`, where `version` is the commit the image was baked from (Dockerfile `ARG BUILD_SHA` ← `build-image.yml`, which then asserts the baked value matches `github.sha`). It exists so a deployed environment can be told apart from a months-old one; everything else about a stale image looks healthy. `status: "ok"` means *this process serves HTTP* and **not** that the DB is up — liveness and readiness are separate on purpose, because pinging Postgres from a public unauthenticated route is a DoS lever. Do not add fields: `tests/e2e/health.spec.ts` pins the payload to those four keys precisely so a DB status or a Mollie mode cannot be added quietly. The Docker HEALTHCHECK deliberately still probes `/en` — nothing declares `depends_on: service_healthy`, so the probe's only reader is a human, and a rendered page proves strictly more. +- **`indexing-monitor.yml`** (daily) — guards that production stays crawlable (robots.txt **and** no de-indexing header; the two live in different places and can disagree) and that every non-canonical copy stays hidden. `robots.txt` is **baked**, not runtime — `app/robots.ts` keys off `NEXT_PUBLIC_SITE_URL`, a *build* arg — so a wrong posture takes a rebuild, not a box `.env` edit. - **E2E against DEPLOYED staging** — `npm run test:e2e:staging` (`tests/e2e/staging-live.spec.ts`) runs against `https://staging.sofrapiwas.com`. Disjoint from the local suite by design: it covers only what cannot exist until something is deployed — the box `.env` reaching the container with the *right* values, the founder-run `:migrate-staging` one-off, Caddy + TLS, and that the deployed bake is the staging one. **Read-only by construction** (no account, no payment, no row), so there is nothing to restore. Needs `STAGING_ADMIN='{email: …, password: …}'` in the gitignored `.env` — **the quotes are load-bearing**, because `scripts/e2e-suite.sh` sources that same file with `set -a && . ./.env` and bash reads an unquoted brace value as an assignment plus a command, killing the whole local suite under `set -euo pipefail`. Two things it deliberately does NOT prove, so don't read them into a green run: that the deployed image is *current* (no health/version endpoint — a months-old `:staging` bake passes), and that the Mollie key is `test_` rather than `live_` (`mollieConfigured()` reports only that some key is set). - **The `E2E_REMOTE` interlock** (`playwright.config.ts`): remote runs the staging spec **and nothing else**; local runs everything **but** the staging spec. Not a convenience — every other spec here mutates (signup rows, real test-key payments, repointed billing records) and reaches its target through relative `page.goto`, so an unrestricted remote run would write into the long-lived shared environment, which has no throwaway DB to drop. Keep it enforced in the config rather than as a `test.skip` in each spec: that is what lets a missing credential be a hard error instead of a skip that exits 0 having verified nothing behind the login. - **Tests** (DEV-PHASES-PLAN W1/W2): `npm run test` = Vitest unit suite over the pure `lib/` modules (format, mollie amount, validation schemas, rate-limit, tenant-registry, email-templates, email helpers — no DB/network; Mollie is never called). `npm run test:coverage` adds the **coverage floor** (W2 D9): v8 coverage scoped to those fully-unit-coverable pure modules (`vitest.config.ts` `coverage.include`), enforced in CI at ≥95% lines/statements/functions, ≥90% branches — modules with network/DB branches stay out of scope (they need mocks §7 forbids). Raise the floor as coverage grows. `npm run test:e2e` = Playwright login smoke (admin→/admin, partner→/dashboard, partner-blocked-from-/admin) against a seeded throwaway DB (`scripts/seed-e2e.mjs`). `scripts/e2e-local.mjs` (the no-browser progressive-enhancement walk of the partner program; needs a **clean** local DB — leftover LIVE client collides on `tenantSlug`) + the QA test accounts remain for manual/full-flow checks. From 8c9f02a1de7cccad8d18c262af0bcc9fbe40d378 Mon Sep 17 00:00:00 2001 From: mahmutKaya <33642821+mahmutkaya@users.noreply.github.com> Date: Fri, 31 Jul 2026 15:23:28 +0200 Subject: [PATCH 8/8] fix(provisioning): pin generated tenants to released code, not develop (#112) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `backend_tag` was derived from `box`, and every self-serve tenant lands on `box: staging` because that is where the control plane runs. So every paying customer was given `:staging` — the develop build — by a default nobody reads. The cost is larger than "unreleased code", which is why this becomes a constant rather than staying a judgement call: `:staging` is republished on every merge to backend `develop`, the staging box's deploy then re-pulls and recreates every tenant pinned to it, and the backend runs a bare `MigrateAsync()` on boot. So a customer's database had develop's EF migrations applied on every merge — and that is not undone by re-pointing the image, so the rollback is not symmetric. Generated entries now always say `latest`. A develop-tracking showcase (`demo`) is a hand-edit at the merge checkpoint, which is the one place a human already reads the entry — the PR-body checklist names `backend_tag`, the field actually visible in Files changed, instead of branching on the box. Decision + blast radius: workspace docs/plans/SOFRA-ONBOARDING-PLAN.md §2b. Co-authored-by: Claude Opus 5 --- docs/adr/ADR-012-auto-provisioning-trigger.md | 7 ++- lib/provisioning-registry.ts | 30 +++++++----- tests/unit/provisioning-registry.test.ts | 48 +++++++++++-------- 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/docs/adr/ADR-012-auto-provisioning-trigger.md b/docs/adr/ADR-012-auto-provisioning-trigger.md index 6496659..1c1338a 100644 --- a/docs/adr/ADR-012-auto-provisioning-trigger.md +++ b/docs/adr/ADR-012-auto-provisioning-trigger.md @@ -104,7 +104,12 @@ it keeps the same privilege split without the git round-trip. - **The PR carries the full computed entry** (`lib/provisioning-registry.ts`): slug-derived `db`/`db_role`/`compose_project`/`domain`/`frontend_tag`, plus languages/modules/currency/template from the signup, `status: provisioning`, - `managed: scripts`, and a box-aware `backend_tag`. + `managed: scripts`, and `backend_tag: latest` — released code, always. It was + derived from the *box* until 2026-07-31, which silently handed every self-serve + tenant the develop build, since every one of them lands on `box: staging` because + that is where the control plane runs. A develop-tracking showcase (`demo`) is a + hand-edit at the merge checkpoint, which is the one place a human already reads + the entry. Workspace `docs/plans/SOFRA-ONBOARDING-PLAN.md` §2b. - **Deprovision stays founder-only** over SSH. Unchanged. - **Status reflection back to `/admin` is still open** — the registry `status` flip to `active` remains a manual follow-up commit, and nothing automatic reads it. diff --git a/lib/provisioning-registry.ts b/lib/provisioning-registry.ts index 33764a7..432561b 100644 --- a/lib/provisioning-registry.ts +++ b/lib/provisioning-registry.ts @@ -44,11 +44,18 @@ export function buildTenantRegistryEntry(input: TenantProvisionInput): string { db: `tenant_${slug}`, db_role: `tenant_${slug}`, compose_project: `tenant-${slug}`, - // `:latest` means main/prod since the 2026-07-16 tag fix, so a staging-box - // tenant pinned to it would silently run production code instead of the - // develop build it exists to showcase. Found by hand-correcting the first - // generated entry (deploy #61). - backend_tag: box === "staging" ? "staging" : "latest", + // Always `:latest` — released code, published only from `main`. NOT derived from + // `box` (as it was until 2026-07-31): every self-serve tenant lands on + // `box: staging`, because that is where the control plane runs, so deriving from + // the box silently gave every paying customer the develop build — and, because the + // staging box's deploy re-pulls `:staging` tenants on every backend develop merge, + // applied develop's EF migrations to their database each time. + // + // A develop-tracking SHOWCASE (`demo`) still wants `:staging`, but that is a + // founder judgement about one tenant, not something the generator can infer — so it + // is a hand-edit to this field in the proposed PR, which is exactly the review + // checkpoint ADR-012 puts at the merge. The PR body says so. + backend_tag: "latest", frontend_tag: `tenant-${slug}`, currency: input.currency, languages: input.languages, @@ -107,14 +114,11 @@ export function buildProvisioningPrBody(input: TenantProvisionInput): string { const box = input.box ?? "staging"; const chained = box === "staging"; - // `backend_tag` is pinned by box above, so the risk is NOT "a staging tenant might be - // on :latest" — that pairing cannot be generated. It is the reverse, and it is the one - // judgement no generator can make: every self-serve tenant lands on the staging box, so - // every self-serve tenant rides the DEVELOP build. Right for a showcase; a decision for - // someone paying. - const tagCheck = chained - ? `- [ ] **\`backend_tag: staging\`** is deliberate — a staging-box tenant rides the *develop* build, i.e. unreleased backend code. Correct for a showcase; for a paying customer, pin \`backend_tag: latest\` before merging` - : `- [ ] **\`backend_tag: latest\`** (prod box) — released code, which is what a prod tenant should ride`; + // One line, naming the one field in the diff the founder may need to change. It used to + // branch on the box and warn that a staging-box tenant rides develop; the generator no + // longer produces that entry, so warning about it would be an unfalsifiable checkbox. + const tagCheck = + "- [ ] **`backend_tag: latest`** — released code, published only from `main`. If this is a develop-tracking **showcase** rather than a customer, change it to `staging` in Files changed before merging; a customer should stay on `latest`, so their database is never migrated by unreleased code"; const header = chained ? [ diff --git a/tests/unit/provisioning-registry.test.ts b/tests/unit/provisioning-registry.test.ts index 39c1491..7491748 100644 --- a/tests/unit/provisioning-registry.test.ts +++ b/tests/unit/provisioning-registry.test.ts @@ -28,7 +28,7 @@ describe("buildTenantRegistryEntry", () => { db: "tenant_bistro-nova", db_role: "tenant_bistro-nova", compose_project: "tenant-bistro-nova", - backend_tag: "staging", + backend_tag: "latest", frontend_tag: "tenant-bistro-nova", currency: "EUR", languages: ["en", "nl"], @@ -56,13 +56,16 @@ describe("buildTenantRegistryEntry", () => { expect(t.city).toBeUndefined(); expect(t.box).toBe("prod"); expect(t.template).toBe("classic"); - // A prod-box tenant rides :latest... expect(t.backend_tag).toBe("latest"); }); - it("pins a staging-box tenant to :staging, not :latest", () => { - // ...while :latest means main/prod, so a staging tenant pinned to it would - // silently run production code instead of the develop build it showcases. + it("pins :latest even on the staging box — the box no longer decides the tag", () => { + // The regression this exists for: `backend_tag` was derived from `box`, and every + // self-serve tenant lands on `box: staging` because that is where the control plane + // runs. So a paying customer was handed the develop build — and develop's EF + // migrations, on every backend develop merge — by a default nobody read. This is the + // ONLY test that distinguishes the two derivations, since the box is the input the + // old rule keyed on and `staging` is its default value. const t = asTenant( buildTenantRegistryEntry({ slug: "onstaging", @@ -75,8 +78,8 @@ describe("buildTenantRegistryEntry", () => { }), "onstaging", ) as Record; - expect(t.box).toBe("staging"); // the default - expect(t.backend_tag).toBe("staging"); + expect(t.box).toBe("staging"); // the default box is unchanged... + expect(t.backend_tag).toBe("latest"); // ...but it no longer implies the develop build }); it("escapes YAML-special characters in free-text (no injection)", () => { @@ -132,19 +135,26 @@ describe("buildProvisioningPrBody", () => { expect(body).toContain("### Run these after merging"); }); - it("flags the backend_tag risk that actually exists for each box", () => { - // buildTenantRegistryEntry pins backend_tag FROM the box, so "a staging tenant might - // be on :latest" is impossible by construction — warning about it would be an - // unfalsifiable checkbox on every real PR. The live risk is the reverse: a staging-box - // tenant rides the develop build, which is wrong for someone paying. - const staging = buildProvisioningPrBody(input); - expect(staging).toContain("rides the *develop* build"); - expect(staging).toContain("unreleased backend code"); - expect(staging).not.toContain("staging-box tenant on `:latest`"); + it("names backend_tag as an editable field, not a box-dependent warning", () => { + // The checkbox has to name the risk THIS entry carries and a field the reader can + // actually see in Files changed, or it is an unfalsifiable box they learn to tick + // blind. Since the tag is now a constant, the line is the same on both boxes. + for (const body of [buildProvisioningPrBody(input), buildProvisioningPrBody({ ...input, box: "prod" })]) { + expect(body).toContain("**`backend_tag: latest`**"); + expect(body).toContain("change it to `staging` in Files changed"); + // The old, now-impossible warning must not survive anywhere in the body. + expect(body).not.toContain("rides the *develop* build"); + } + }); - const prod = buildProvisioningPrBody({ ...input, box: "prod" }); - expect(prod).toContain("released code"); - expect(prod).not.toContain("unreleased backend code"); + it("agrees with the entry the same PR proposes", () => { + // The body and the entry are two independent literals; asserting each separately + // would let one drift. Read the tag out of the generated YAML and require the body + // to quote that exact value. + const tag = ( + asTenant(buildTenantRegistryEntry(input), input.slug) as Record + ).backend_tag; + expect(buildProvisioningPrBody(input)).toContain(`**\`backend_tag: ${tag}\`**`); }); it("keeps a newline in the tenant name from breaking the fence or the command", () => {