From fa70ca9d4c99037ca8877b308c37473b00a03229 Mon Sep 17 00:00:00 2001 From: Alexgodoroja Date: Tue, 15 Sep 2026 22:12:02 -0700 Subject: [PATCH 1/2] Check the schema is the one this build expects Migrating reports what it applied. This reports what is there, which is the question the code about to serve traffic has: is anything still pending, has an applied file been edited since, and does every table the store reads exist. Read-only, so it is safe against production and says the same thing twice. --- app/package.json | 1 + app/server/verify-schema.ts | 113 ++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 app/server/verify-schema.ts diff --git a/app/package.json b/app/package.json index d8740f3..d8b838c 100644 --- a/app/package.json +++ b/app/package.json @@ -9,6 +9,7 @@ "check:protocol": "node scripts/check-protocol.mjs", "db:import": "tsx server/import-json.ts", "db:migrate": "tsx server/migrate.ts", + "db:verify": "tsx server/verify-schema.ts", "db:up": "docker run -d --name shell-online-pg -e POSTGRES_PASSWORD=dev -e POSTGRES_DB=shell_online -p 5433:5432 postgres:16-alpine", "dev": "vite", "dev:accounts": "tsx watch --env-file-if-exists=.env.local server/index.ts", diff --git a/app/server/verify-schema.ts b/app/server/verify-schema.ts new file mode 100644 index 0000000..caf88be --- /dev/null +++ b/app/server/verify-schema.ts @@ -0,0 +1,113 @@ +/** + * Checks that the database is the one this build expects, and stops. + * + * DATABASE_URL=postgres://... npm run db:verify + * + * Run after migrating and before the Worker goes out. Migrating reports what + * it applied; this reports what is *there*, which is a different question and + * the one that matters to the code about to serve traffic. It answers three: + * is any migration still pending, has an applied file been edited since, and + * does every table the store reads exist. + * + * It only reads, so it is safe to run against production at any time, and + * running it twice says the same thing as running it once. + */ +import { createHash } from "node:crypto"; +import { readdirSync, readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { Pool } from "pg"; + +/* + * The tables the Worker reads or writes. Kept here as a list rather than + * derived from the migrations, so that dropping one in a later migration + * without noticing what still selects from it fails here rather than in a + * request. + */ +const REQUIRED_TABLES = [ + "account_activity", + "account_keys", + "agent_commands", + "app_events", + "audit_events", + "auth_codes", + "cli_tokens", + "comments", + "deleted_accounts", + "feedback", + "invites", + "memberships", + "notifications", + "organizations", + "schema_migrations", + "session_key_shares", + "sessions", + "team_key_shares", + "team_keys", +]; + +function migrationsDir(): string { + const here = dirname(fileURLToPath(import.meta.url)); + return join(here, "lib", "migrations"); +} + +const url = process.env.DATABASE_URL; +if (!url) { + console.error("verify: set DATABASE_URL to the database to check"); + process.exit(1); +} + +const pool = new Pool({ connectionString: url, max: 1 }); +const problems: string[] = []; + +try { + const directory = migrationsDir(); + const files = readdirSync(directory).filter((name) => name.endsWith(".sql")).sort(); + + const { rows } = await pool.query<{ name: string; checksum: string | null }>( + "SELECT name, checksum FROM schema_migrations", + ); + const applied = new Map(rows.map((row) => [row.name, row.checksum])); + + for (const name of files) { + const checksum = createHash("sha256").update(readFileSync(join(directory, name), "utf8")).digest("hex"); + if (!applied.has(name)) { + problems.push(`${name} has not been applied`); + continue; + } + const recorded = applied.get(name); + /* Null is a row written before checksums; migrating fills it in. */ + if (recorded !== null && recorded !== checksum) { + problems.push(`${name} differs from the file that was applied`); + } + } + + /* A database ahead of this build: an older deploy must not migrate it back. */ + for (const name of applied.keys()) { + if (!files.includes(name)) problems.push(`${name} is applied but not in this build`); + } + + const present = new Set( + ( + await pool.query<{ table_name: string }>( + "SELECT table_name FROM information_schema.tables WHERE table_schema = 'public'", + ) + ).rows.map((row) => row.table_name), + ); + for (const table of REQUIRED_TABLES) { + if (!present.has(table)) problems.push(`table ${table} is missing`); + } + + if (problems.length > 0) { + console.error("verify: this database is not what this build expects"); + for (const problem of problems) console.error(` ${problem}`); + process.exitCode = 1; + } else { + console.log(`verify: schema is what this build expects (${files.length} migrations, ${REQUIRED_TABLES.length} tables).`); + } +} catch (error) { + console.error(`verify: ${(error as Error).message}`); + process.exitCode = 1; +} finally { + await pool.end(); +} From 549a74351ab22afddf23109b26c4b0fbb45a2691 Mon Sep 17 00:00:00 2001 From: Alexgodoroja Date: Tue, 15 Sep 2026 22:14:59 -0700 Subject: [PATCH 2/2] Deploy the app on a push to main, and put it back if it fails The workflow only ran when somebody remembered to dispatch it, and the last three runs failed on missing configuration, so app/ has been deployed by hand. It now runs on a push to main that touches app/, and nothing else: a change to the CLI, the relay or the site reaches people another way. Failing leaves production alone. The build, the tests and the migration all happen before anything is deployed, and the schema only ever gains things, so a run that stops partway leaves the Worker that is serving untouched. If the new Worker deploys but does not answer, or answers with the previous build, the version that was serving is put back. The database password is no longer a GitHub secret: it is read from Secret Manager with the short-lived identity the deploy already uses, so the one copy is the one the service reads, and this repository -- public -- holds none of it. Configured values reach scripts through the environment rather than being interpolated into them. CI migrates an empty database twice and verifies it, so a migration that only works on a schema that already exists, or that cannot be applied twice, fails there rather than against production. --- .github/workflows/ci.yml | 15 +++ .github/workflows/deploy-app.yml | 177 ++++++++++++++++++++++++++----- 2 files changed, 168 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7c88176..ff88576 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -161,6 +161,21 @@ jobs: working-directory: app env: TEST_DATABASE_URL: postgres://postgres:ci@localhost:5432/shell_online + # Against the empty `postgres` database on the same service, so it is a + # migration from nothing rather than whatever the suite left behind. + # Twice, because applying twice has to be the same as applying once: that + # is what lets a deploy re-run safely after a failure further along. + - name: Migrate a database from nothing, twice, and verify it + run: | + set -eu + npm run db:migrate + npm run db:verify + npm run db:migrate + npm run db:verify + working-directory: app + env: + DATABASE_URL: postgres://postgres:ci@localhost:5432/postgres + - name: Check vendored modules have not drifted run: node scripts/check-protocol.mjs working-directory: app diff --git a/.github/workflows/deploy-app.yml b/.github/workflows/deploy-app.yml index d2704e3..19b1da2 100644 --- a/.github/workflows/deploy-app.yml +++ b/.github/workflows/deploy-app.yml @@ -8,8 +8,20 @@ name: Deploy app # Migrations only ever add, so a Worker from before a migration still runs # against the newer schema. That is what makes this order safe and a rollback # safe with it. +# +# Nothing here changes production until the build, the tests and the migration +# have all succeeded, and the last word belongs to production itself: if the +# deployed Worker does not answer, or answers with the previous build, the +# version that was serving before is put back. on: + push: + branches: [main] + # Only what this Worker is built from. A change to the CLI, the relay or + # the site reaches people another way and must not redeploy this. + paths: + - "app/**" + - ".github/workflows/deploy-app.yml" workflow_dispatch: inputs: migrate: @@ -21,12 +33,17 @@ permissions: contents: read id-token: write # Workload Identity Federation, so there is no key to leak. -# One deploy at a time. Two overlapping runs could migrate and deploy in an -# order neither of them intended. +# One deploy at a time, and never cancelled halfway: two overlapping runs could +# migrate and deploy in an order neither of them intended. concurrency: group: deploy-app cancel-in-progress: false +env: + # Pinned rather than floating: a deploy should not change because a new + # wrangler was published this morning. + WRANGLER_VERSION: "4.131.0" + jobs: deploy: runs-on: ubuntu-latest @@ -37,6 +54,19 @@ jobs: with: persist-credentials: false + # A dispatch runs against whatever ref was chosen. Production is built + # from main, so anything else is a mistake worth catching before it is + # deployed rather than after. + - name: Refuse to deploy anything but main + env: + REF: ${{ github.ref }} + run: | + set -eu + if [ "$REF" != "refs/heads/main" ]; then + echo "this workflow deploys main; asked for $REF" >&2 + exit 1 + fi + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: 22 @@ -50,9 +80,13 @@ jobs: # where the symptom no longer resembles the cause. An unset repository # variable is the empty string, and every one of these was empty on the # first run of this workflow. + # + # Values arrive through the environment rather than ${{ }} inside the + # script, so nothing configured elsewhere can be read as shell. - name: Check the deploy has what it needs env: CLOUD_SQL_INSTANCE: ${{ vars.CLOUD_SQL_INSTANCE }} + DATABASE_URL_SECRET: ${{ vars.DATABASE_URL_SECRET }} HYPERDRIVE_ID: ${{ vars.HYPERDRIVE_ID }} MAIL_FROM: ${{ vars.MAIL_FROM }} WEB_ORIGIN: ${{ vars.WEB_ORIGIN }} @@ -64,34 +98,43 @@ jobs: VITE_FIREBASE_APP_ID: ${{ vars.VITE_FIREBASE_APP_ID }} CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} - DATABASE_URL: ${{ secrets.DATABASE_URL }} + GCP_WORKLOAD_IDENTITY_PROVIDER: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }} + GCP_SERVICE_ACCOUNT: ${{ secrets.GCP_SERVICE_ACCOUNT }} run: | + set -eu missing= - for name in CLOUD_SQL_INSTANCE HYPERDRIVE_ID MAIL_FROM WEB_ORIGIN \ - CLOUDFLARE_API_TOKEN CLOUDFLARE_ACCOUNT_ID DATABASE_URL; do - eval "value=\${$name}" + for name in CLOUD_SQL_INSTANCE DATABASE_URL_SECRET HYPERDRIVE_ID MAIL_FROM WEB_ORIGIN \ + CLOUDFLARE_API_TOKEN CLOUDFLARE_ACCOUNT_ID \ + GCP_WORKLOAD_IDENTITY_PROVIDER GCP_SERVICE_ACCOUNT; do + eval "value=\${$name-}" if [ -z "$value" ]; then missing="$missing $name"; fi done - if { [ -n "$VITE_OIDC_ISSUER" ] || [ -n "$VITE_OIDC_CLIENT_ID" ]; } && \ - { [ -z "$VITE_OIDC_ISSUER" ] || [ -z "$VITE_OIDC_CLIENT_ID" ]; }; then + if { [ -n "${VITE_OIDC_ISSUER-}" ] || [ -n "${VITE_OIDC_CLIENT_ID-}" ]; } && \ + { [ -z "${VITE_OIDC_ISSUER-}" ] || [ -z "${VITE_OIDC_CLIENT_ID-}" ]; }; then missing="$missing complete VITE_OIDC_ISSUER/VITE_OIDC_CLIENT_ID pair" fi firebase_missing= for name in VITE_FIREBASE_API_KEY VITE_FIREBASE_AUTH_DOMAIN \ VITE_FIREBASE_PROJECT_ID VITE_FIREBASE_APP_ID; do - eval "value=\${$name}" + eval "value=\${$name-}" if [ -z "$value" ]; then firebase_missing="$firebase_missing $name"; fi done - if [ -z "$VITE_OIDC_ISSUER" ] && [ -n "$firebase_missing" ]; then + if [ -z "${VITE_OIDC_ISSUER-}" ] && [ -n "$firebase_missing" ]; then missing="$missing Firebase auth:$firebase_missing" fi if [ -n "$missing" ]; then echo "Not set:$missing" >&2 - echo "Repository variables: infrastructure plus either complete VITE_OIDC_* or VITE_FIREBASE_* configuration" >&2 - echo "Repository secrets: CLOUDFLARE_API_TOKEN CLOUDFLARE_ACCOUNT_ID DATABASE_URL" >&2 + echo "Repository variables: CLOUD_SQL_INSTANCE DATABASE_URL_SECRET HYPERDRIVE_ID MAIL_FROM WEB_ORIGIN, plus either complete VITE_OIDC_* or VITE_FIREBASE_*" >&2 + echo "Repository secrets: CLOUDFLARE_API_TOKEN CLOUDFLARE_ACCOUNT_ID GCP_WORKLOAD_IDENTITY_PROVIDER GCP_SERVICE_ACCOUNT" >&2 exit 1 fi + # The same suite CI runs, run again against the commit being deployed, so + # a deploy is never the first thing to find out. + - name: Test + run: npm test + working-directory: app + # The provider's address and client id are compiled into the client, so # they are build inputs rather than runtime configuration. They are # per-environment and not kept in the repository, so they come from here. @@ -122,19 +165,51 @@ jobs: with: workload_identity_provider: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }} service_account: ${{ secrets.GCP_SERVICE_ACCOUNT }} + token_format: access_token + + # The database password is not a GitHub secret. It lives in Secret + # Manager, which is where the service reads it from as well, so there is + # one copy of it and this repository -- public -- never holds it. + # Fetched with the short-lived token from the step above. + # + # The stored URL names the Cloud SQL socket the Worker uses. Here the + # proxy is a TCP port on this runner, so the host is rewritten and the + # socket parameter dropped. Parsed rather than edited with sed: a + # password may contain the characters a pattern would split on. + - name: Read the database URL + if: ${{ github.event_name != 'workflow_dispatch' || inputs.migrate }} + env: + ACCESS_TOKEN: ${{ steps.gcp.outputs.access_token }} + DATABASE_URL_SECRET: ${{ vars.DATABASE_URL_SECRET }} + run: | + set -euo pipefail + encoded=$(curl -fsS \ + -H "Authorization: Bearer ${ACCESS_TOKEN}" \ + "https://secretmanager.googleapis.com/v1/${DATABASE_URL_SECRET}/versions/latest:access" \ + | jq -r '.payload.data') + raw=$(printf '%s' "$encoded" | base64 --decode) + url=$(RAW="$raw" node -e 'const u = new URL(process.env.RAW); u.hostname = "127.0.0.1"; u.port = "5432"; u.search = ""; process.stdout.write(u.toString());') + # Masked before it is used, so no later command can print it by accident. + echo "::add-mask::${url}" + password=$(RAW="$raw" node -e 'process.stdout.write(decodeURIComponent(new URL(process.env.RAW).password));') + if [ -n "$password" ]; then echo "::add-mask::${password}"; fi + echo "DATABASE_URL=${url}" >> "$GITHUB_ENV" # The instance has no authorized networks, so it is not reachable from # the internet. The proxy connects through Google's own network with the # identity above rather than opening a port to let this runner in. - name: Open a Cloud SQL connection if: ${{ github.event_name != 'workflow_dispatch' || inputs.migrate }} + env: + CLOUD_SQL_INSTANCE: ${{ vars.CLOUD_SQL_INSTANCE }} run: | + set -eu curl -fsSL -o cloud-sql-proxy \ "https://storage.googleapis.com/cloud-sql-connectors/cloud-sql-proxy/v2.14.1/cloud-sql-proxy.linux.amd64" echo "e48115261929445f468ae7ae86be199781bafda56837c9f33644892df8d46997 cloud-sql-proxy" \ | sha256sum --check --strict chmod +x cloud-sql-proxy - ./cloud-sql-proxy --port 5432 "${{ vars.CLOUD_SQL_INSTANCE }}" & + ./cloud-sql-proxy --port 5432 "$CLOUD_SQL_INSTANCE" & for _ in $(seq 1 30); do if nc -z 127.0.0.1 5432; then exit 0; fi sleep 1 @@ -142,12 +217,22 @@ jobs: echo "the Cloud SQL proxy never started listening" >&2 exit 1 + # Additive, one transaction per file, recorded with the checksum of the + # file that was applied. Applying twice does nothing the second time, and + # a file edited after the fact is refused rather than half-applied. - name: Apply migrations if: ${{ github.event_name != 'workflow_dispatch' || inputs.migrate }} run: npm run db:migrate working-directory: app - env: - DATABASE_URL: ${{ secrets.DATABASE_URL }} + + # What the database actually is, checked before the Worker that depends + # on it goes out: nothing pending, nothing edited since it was applied, + # every table the store reads present. A failure here stops the deploy + # with production still serving the build it already had. + - name: Verify the schema + if: ${{ github.event_name != 'workflow_dispatch' || inputs.migrate }} + run: npm run db:verify + working-directory: app - name: Render deployment configuration run: npm run render:deploy-config @@ -161,23 +246,67 @@ jobs: FIREBASE_PROJECT_ID: ${{ vars.VITE_FIREBASE_PROJECT_ID }} MAIL_FROM: ${{ vars.MAIL_FROM }} + # Which version is serving right now, so there is something exact to put + # back if the new one turns out not to work. The list is oldest first. + - id: serving + name: Note the version now serving + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + working-directory: app + run: | + set -euo pipefail + version=$(npx --yes "wrangler@${WRANGLER_VERSION}" deployments list \ + --config wrangler.deploy.jsonc --json | jq -r '.[-1].versions[0].version_id // empty') + echo "version=${version}" >> "$GITHUB_OUTPUT" + echo "currently serving ${version:-nothing}" + # Deployed after the schema it expects is in place. - - name: Deploy the Worker - uses: cloudflare/wrangler-action@ebbaa1584979971c8614a24965b4405ff95890e0 # v4.0.0 - with: - apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} - accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} - workingDirectory: app - command: deploy --config wrangler.deploy.jsonc + - id: deploy + name: Deploy the Worker + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + working-directory: app + run: npx --yes "wrangler@${WRANGLER_VERSION}" deploy --config wrangler.deploy.jsonc + # Production has the last word. A Worker can deploy successfully and + # still answer with the previous index.html, which leaves a fix live at + # its own URL and invisible to everyone; verify-deploy compares what the + # origin serves against what was just built. - name: Check the deployment answers + env: + WEB_ORIGIN: ${{ vars.WEB_ORIGIN }} + working-directory: app run: | + set -eu for _ in $(seq 1 20); do - if curl -fsS --max-time 10 "${{ vars.WEB_ORIGIN }}/api/health" >/dev/null; then - curl -fsS --max-time 20 "${{ vars.WEB_ORIGIN }}/api/ready" + if curl -fsS --max-time 10 "${WEB_ORIGIN}/api/health" >/dev/null; then + curl -fsS --max-time 20 "${WEB_ORIGIN}/api/ready" + node scripts/verify-deploy.mjs "${WEB_ORIGIN}" exit 0 fi sleep 3 done echo "the deployment never answered a health check" >&2 exit 1 + + # Only when this run is what changed production: a build or a migration + # that failed never reached it, and rolling back then would undo somebody + # else's deploy. The schema stays where it is, which is safe because a + # migration only ever added to it. + - name: Put the previous version back + if: ${{ failure() && steps.deploy.outcome == 'success' && steps.serving.outputs.version != '' }} + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + PREVIOUS: ${{ steps.serving.outputs.version }} + SHA: ${{ github.sha }} + working-directory: app + run: | + set -eu + npx --yes "wrangler@${WRANGLER_VERSION}" rollback "$PREVIOUS" \ + --config wrangler.deploy.jsonc \ + --message "deploy of ${SHA} did not answer; put ${PREVIOUS} back" \ + --yes + echo "rolled back to ${PREVIOUS}" >&2