From 242aaf00028d3d54f5ded3948ebc8a5c4fc2a456 Mon Sep 17 00:00:00 2001 From: Kilian Mateo <13885240+kilianmc@users.noreply.github.com> Date: Thu, 13 Aug 2026 20:31:02 +0200 Subject: [PATCH] chore: stop the migrate job printing the Neon endpoint, and write down why MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Migrate workflow's first real run (read-only `current` against dev) worked, and leaked. `alembic current --verbose` emits a header: Current revision(s) for ***ep-flat-...eu-central-1.aws.neon.tech/neondb?***: Alembic obscures the password and GitHub masked the secret, but the Neon endpoint hostname, region and database name reached a PUBLIC Actions log — breaking the rule stated at the top of that very file, via a flag rather than an `echo`. Credentials were not exposed and the run has been deleted; the residual risk was reconnaissance only, since Neon still requires credentials and the free tier has no IP allowlist to bypass. Both `current` steps now run bare. `alembic current` prints `0002 (head)`, which is all the audit trail needs. `alembic history --verbose` keeps its flag: it reads the migration files and never opens a connection, so it has no URL to print. Also verified nothing else on that path logs one — `sqlalchemy.engine` is pinned to WARNING in alembic.ini and migrations/env.py never prints it. CLAUDE.md gains the two traps that made this workflow inert for a day: `workflow_dispatch` only registers from the DEFAULT branch, and `environment` selects the database while the REF selects the migrations. Plus the rule that main's copy must stay byte-identical to dev's, because the merge base predates the file and any difference is an add/add conflict at the first promotion. It also gains the end-to-end security verification pass Kilian asked for: a dated, tiered checklist to run once the product is feature-complete and before it is shown to anyone. Tier 1 is what CI already proves and must NOT be re-tested by hand; tier 2 is what only the real deployment can show (routing, docs-off, CORS, cookie attributes, IDOR with two real accounts, demo writes, rate limits, headers, bundle secrets); tier 3 is the infra that lives outside this repo, where no test in CI can ever notice a control going missing — the WAF rule above all. --- .github/workflows/migrate.yml | 32 +++++++++- CLAUDE.md | 107 ++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 3 deletions(-) diff --git a/.github/workflows/migrate.yml b/.github/workflows/migrate.yml index cc232f1..989f00a 100644 --- a/.github/workflows/migrate.yml +++ b/.github/workflows/migrate.yml @@ -11,6 +11,19 @@ # # Nothing in this file may print a connection string. The repository is public and so # are these logs: no `set -x`, no `echo "$DATABASE_URL"`, not even a masked prefix. +# That is easier to violate than it looks, and it was violated on the first real run — +# by a `--verbose` flag, not by an `echo`. See the note on the `current` steps below. +# +# DISPATCHING THIS: the workflow is only *registered* because the file exists on the +# default branch (`main`) — `workflow_dispatch` is ignored on any other branch, and +# before it was registered `gh workflow run` answered `HTTP 404: not found on the +# default branch`. But GitHub takes the job definition AND the checkout from the ref you +# select, so pick it deliberately: run the ref whose `migrations/` directory you mean to +# apply. Until the first `dev`->`main` promotion, `main` has no migrations and no alembic +# dependency at all, so a run must select `dev` (`gh workflow run migrate.yml --ref dev +# -f environment=dev -f action=current`). `environment` chooses the DATABASE; the ref +# chooses the MIGRATIONS. They are independent, and mixing them up is how production +# gets a revision that was never reviewed. name: Migrate on: @@ -77,9 +90,21 @@ jobs: - run: uv sync --frozen # The "before" half of the audit trail. For `action: current` this is the whole job. + # + # NO `--verbose` HERE, and that is not a style preference. `alembic current + # --verbose` prints a `Current revision(s) for :` header before the revision. + # Alembic obscures the password and GitHub masks the secret, but the Neon endpoint + # HOSTNAME, region and database name survive into a **public** Actions log — which + # is precisely what the header comment at the top of this file forbids. It happened + # (2026-08-13, first run; the run was deleted). Bare `current` prints `0002 (head)` + # and emits no such header, which is all the audit trail needs. Nothing else on this + # path logs the URL: `sqlalchemy.engine` is pinned to WARNING in `alembic.ini` and + # `migrations/env.py` never prints it. - name: Applied revision (before) - run: uv run alembic current --verbose + run: uv run alembic current + # `--verbose` IS safe here: `history` reads the migration files and never opens a + # connection, so there is no URL for it to put in the header. - name: Revision history if: ${{ inputs.action == 'history' }} run: uv run alembic history --verbose @@ -89,10 +114,11 @@ jobs: run: uv run alembic upgrade head # The "after" half. The two together are the record of what this run actually - # moved, which matters because nothing else logs it. + # moved, which matters because nothing else logs it. Bare `current` again — same + # reason as above. - name: Applied revision (after) if: ${{ inputs.action == 'upgrade' }} - run: uv run alembic current --verbose + run: uv run alembic current # The same seed module CI, local development and production all use. Upserts and # never deletes, so re-running it is safe and cheap. diff --git a/CLAUDE.md b/CLAUDE.md index 53ac6ba..a8ee9ca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -408,6 +408,44 @@ secrets. Without them the job starts and fails on the first Alembic step. The workflow is `workflow_dispatch`-only and never prints a connection string; keep both properties if you edit it. +##### ⚠️ Two traps that cost a debugging session on 2026-08-13, the day it first ran + +**1. A `workflow_dispatch` workflow only registers if the file exists on the DEFAULT +branch.** `migrate.yml` shipped on `dev` in PR #2 and was therefore *completely inert*: +`gh workflow run` returned `HTTP 404: workflow migrate.yml not found on the default +branch`, and it did not appear in the Actions UI at all — so there was nothing to click +either. The GitHub environments and their secrets were correct the whole time and it made +no difference. PR #3 fixed it by putting the file on `main` on its own, as a deliberate +exception to the "`main` receives only promotion PRs" rule. + +> **Keep `main`'s copy BYTE-IDENTICAL to `dev`'s.** The branches' merge base predates the +> file, so any difference is an **add/add conflict** at the first promotion. Identical +> content merges silently. A change to this workflow therefore takes **two** PRs — one to +> `dev`, one to `main` — and the same rule applies to every future +> `workflow_dispatch` workflow. Notes about the workflow go in *this* file, on `dev`, not +> in a comment that would have to be duplicated. + +**2. `environment` chooses the DATABASE; the REF chooses the MIGRATIONS.** Registration +comes from the default branch, but GitHub takes both the job definition and the checkout +from the ref you select in the dialog. They are independent inputs and confusing them is +how production gets a revision nobody reviewed. Until the first `dev`→`main` promotion, +`main` has no `migrations/` directory and no alembic dependency, so a run must select +`dev`: + +```bash +gh workflow run migrate.yml --ref dev -f environment=dev -f action=current +``` + +**And a third, smaller one: `alembic current --verbose` prints the connection URL.** It +emits a `Current revision(s) for :` header. Alembic hides the password and GitHub +masks the secret, but the **Neon endpoint hostname, region and database name reach the +public log** — breaking the workflow's own no-connection-string rule via a flag rather +than an `echo`. The steps use bare `alembic current` for that reason; `alembic history +--verbose` is fine because it never opens a connection. Verified 2026-08-13 that nothing +else on the path logs a URL: `sqlalchemy.engine` is pinned to `WARNING` in `alembic.ini` +and `migrations/env.py` never prints one. **The lesson generalises — in a public repo, +audit what a tool prints at its chosen verbosity, not just what the workflow echoes.** + ### SQLite is disqualified for tests The schema uses native Postgres enums, `text[]`, `GENERATED … STORED`, GIN indexes and @@ -516,6 +554,75 @@ addresses that do not exist, so it is not an account-existence oracle. It is an control only; see the correction in the compute-budget section for why it does not protect awake time. +### 🔒 TODO — the end-to-end security verification pass (Kilian's call, 2026-08-13) + +**Not yet done. Do not tick any of it off from memory.** Every rule in this file was +written because of a real risk, and a rule that was implemented once and never verified +against the running system is indistinguishable from a rule that quietly stopped working. +Two of the controls listed below **do not live in this repository at all**, so no test in +CI can ever notice their absence. + +**When:** once the product is feature-complete on `climb.kilianmc.com` — realistically +after PR #7 — and **before** the project is shown to anyone as a portfolio piece. Run it +against the **production deploy**, not a preview: previews are cross-site +(`*.vercel.app` is on the Public Suffix List) and behave differently on purpose. + +**Scope it honestly — three tiers, and only two of them need a human:** + +- **Already proven by CI on every push. Do NOT re-test by hand:** the route-enumeration + auth/demo tests, the routing contract, the version wiring, refresh rotation and reuse + detection under a real row lock, the rate-limiter's single-statement upsert, `alembic + upgrade head` + `alembic check` on a fresh Postgres, gitleaks over full history. + Re-checking these manually is how a verification pass becomes theatre. +- **Only verifiable against the real deployment**, because the Vercel rewrite, the CDN + and the browser are the parts CI does not have: + 1. **Routing** — `/api/health` → JSON, `/api/nope` → FastAPI's **JSON** 404 (not the + SPA shell), `/` and `/deep/link` → `text/html`. The dangerous failure is a + `200 text/html` from an `/api/*` path. + 2. **`/api/docs` and `/api/openapi.json` → 404 in production.** + 3. **CORS** — an allowed origin is echoed with `Vary: Origin`; an **unknown origin gets + no `Access-Control-Allow-Origin` header at all**; there is no `*` anywhere on + `/api/*`. Test with a real preflight, not just a GET. + 4. **Cookies** (browser devtools — ask Kilian) — the refresh cookie is `HttpOnly`, + `Secure`, `SameSite=Lax`, `Path=/api/auth`, and has **no `Domain` attribute**. + And **nothing token-shaped in `localStorage` or `sessionStorage` in either mount** — + check the federated mount too, where the storage is kilianmc.com's. + 5. **Deny-by-default through the rewrite** — hit a protected endpoint anonymously on + the deploy. The enumerated test proves this in-process; this proves the rewrite + doesn't route around it. + 6. **IDOR, with two real accounts** — as user A, request user B's plan / session / + ascent / diary entry by id. Expect 404 or 403 and **never** a row. This is the + single highest-value check in the list: it is the actual extraction risk. + 7. **Demo mode against a live mutating endpoint** — a demo token must 403, and the + `SET LOCAL transaction_read_only` layer must reject a write if the first layer is + ever bypassed. + 8. **Login rate limit** — it trips; the 429 is byte-identical for a real and a + non-existent address; it self-heals within the window and no account is ever + disabled. + 9. **Security response headers** — once that PR has landed: HSTS, CSP, + `Referrer-Policy`, `X-Content-Type-Options`, and **verify `frame-ancestors` does not + break the federated mount inside `portfolio-shell`** (this is the one that will). + 10. **No secret in the client bundle** — grep the built `web/dist` for any value of a + non-`VITE_` env var, and for anything resembling `AUTH_SECRET`. +- **Infra, outside the repo — these are the ones nothing else can catch:** + 11. **The Vercel WAF rule on `/api/auth/*` still exists and actually fires** (20 req / + 10 min / IP). Confirm the denials appear in the Firewall traffic view. **This is + the only rate limit on demo-token minting** — see the ⚠️ in the compute-budget + section. + 12. **Vercel project settings** — `framework` is still `null` (re-check after any + `vercel link`), and `ssoProtection` is still **ON** for this project's previews. + 13. **Zero open Dependabot alerts**, or each remaining one triaged with a written + reason it does not apply. On 2026-08-13 there were 6 (starlette + pytest) and none + were exploitable here, but "not exploitable" needs re-deciding per alert, not once. + 14. **2FA still enabled on GitHub, Vercel, Neon and Cloudflare.** + 15. **Neon CU-hours for the month match the model** in the compute-budget section. A + figure well above it means something is waking the database — that is the signal. + +**Script what is scriptable** (1, 2, 3, 5, 6, 7, 8, 10 are all `curl`/CLI), and **ask +Kilian for the browser-only ones** (4, 9, 11) rather than burning turns on them. +**Write the outcome down** — in the PR that does the pass, with the date — so the next +person verifies rather than re-verifies. + --- ## Injection defence and input minimisation (OWASP)