From 03a965ca50a3db443be3ee8e20a7cccfd406163c Mon Sep 17 00:00:00 2001 From: Maximilian Kaske Date: Tue, 13 Oct 2026 23:06:24 +0200 Subject: [PATCH] chore: second review pass on the screenshot script - login guard: exact /login path, dashboard only, and not when /login itself is requested, so status-page password pages and the login screen can be shot - explicit --user or --token never fall back to a saved state; the chosen auth source is logged; --login is skipped once auth is resolved - deterministic session and workspace pick (user_id, created_at tiebreakers) - the non-dashboard probe detects a dashboard on --base up front - --width is validated; the header points at the skill instead of repeating it - turbo passes PORT through, so pnpm dev:dashboard honours it too; the skill says to use the --filter form when DATABASE_URL must differ - seed comment back to two lines --- .agents/skills/screenshot/SKILL.md | 5 ++- packages/db/src/seed.mts | 5 +-- scripts/screenshot.mjs | 65 +++++++++++++++++------------- turbo.json | 4 +- 4 files changed, 46 insertions(+), 33 deletions(-) diff --git a/.agents/skills/screenshot/SKILL.md b/.agents/skills/screenshot/SKILL.md index 90e57a416..823af0400 100644 --- a/.agents/skills/screenshot/SKILL.md +++ b/.agents/skills/screenshot/SKILL.md @@ -28,7 +28,10 @@ It prints the PNG paths. Read them with the Read tool and judge against `pnpm -w dev:dashboard`, `dev:web` or `dev:status-page` (each also starts sqld on :8080), logged to a file, stopped when done. If the default port is taken, start the app alone on another port and pass `--base`: - `PORT=3002 pnpm --filter @openstatus/dashboard dev`. + `PORT=3002 pnpm --filter @openstatus/dashboard dev`. Use that direct form + whenever you also need a different `DATABASE_URL`: the root `pnpm -w + dev:dashboard` runs through turbo, which passes `PORT` through but drops + other variables, so the app reads its `.env` database. 2. Database, dashboard only: the script reads live sessions from sqld over HTTP, default `--db http://127.0.0.1:8080`. A second sqld on :8099 is the throwaway seeded one; pass `--db http://127.0.0.1:8099` and point the diff --git a/packages/db/src/seed.mts b/packages/db/src/seed.mts index 82e5a25cc..c8a1644aa 100644 --- a/packages/db/src/seed.mts +++ b/packages/db/src/seed.mts @@ -210,9 +210,8 @@ async function main() { .onConflictDoNothing() .run(); - // The two maintainers, with sessions that outlive any local database, so - // the dashboard and scripts/screenshot.mjs log in without a magic link. Tokens are fixed on purpose: this seed only runs against - // localhost. + // Fixed tokens so the dashboard and scripts/screenshot.mjs log in without a + // magic link; main() refuses non-local databases. await db .insert(user) .values([ diff --git a/scripts/screenshot.mjs b/scripts/screenshot.mjs index 451a97a0d..eb97fa41b 100644 --- a/scripts/screenshot.mjs +++ b/scripts/screenshot.mjs @@ -5,18 +5,7 @@ // node scripts/screenshot.mjs --app web /pricing --full-page // node scripts/screenshot.mjs --app status-page /acme // -// Flags: --app dashboard|web|status-page (default dashboard), --base , -// --selector , --theme both|light|dark, --out , --width , -// --full-page, --user , --workspace , --token , -// --db , --login --log . -// -// The dashboard needs a login. In order: --token, a live row in the `session` -// table (the seed creates seed-session-max and seed-session-thibault; narrow -// with --user), a saved storage state, or --login, which submits the email -// form and follows the `>>> Magic Link:` line the dev server prints to --log. -// Playwright is borrowed from apps/screenshot-service so the version follows -// that package; the browser is the system Chrome, or Playwright's bundled -// Chromium where there is none. +// Flags, auth order and fixtures: .agents/skills/screenshot/SKILL.md. import { existsSync, mkdirSync, readFileSync } from "node:fs"; import { createRequire } from "node:module"; @@ -65,6 +54,10 @@ const themes = ? [flags.theme] : ["light", "dark"]; const width = Number(flags.width ?? 1400); +if (!Number.isFinite(width) || width <= 0) { + console.error(`--width must be a positive number, got ${flags.width}`); + process.exit(2); +} const stateFile = resolve(flags.state ?? join(out, "state.json")); const wantsAuth = app.auth || Boolean(flags.token || flags.login); mkdirSync(out, { recursive: true }); @@ -96,7 +89,7 @@ async function sessionCookies() { q: `select s.session_token as token, u.id as user_id, u.email as email from session s join user u on u.id = s.user_id where s.expires > unixepoch() * 1000 ${where} - order by s.expires desc limit 1`, + order by s.expires desc, s.user_id limit 1`, params, }, ]); @@ -107,7 +100,7 @@ async function sessionCookies() { const [ws] = await sql([ { q: `select w.slug as slug from users_to_workspaces m join workspace w on w.id = m.workspace_id - where m.user_id = ? order by m.workspace_id limit 1`, + where m.user_id = ? order by m.created_at, m.workspace_id limit 1`, params: [user_id], }, ]); @@ -154,8 +147,8 @@ async function magicLinkLogin(context) { ); } -// apps/web and the dashboard both default to :3001 on this machine; the -// dashboard's /login is a form, everything else answers 404 there. +// Three apps, two default ports, and on this machine any of them may sit on +// :3001. Only the dashboard serves a form at /login; use that to tell them apart. const probe = await fetch(base + (app.probe ?? paths[0])).catch(() => null); if (!probe || probe.status === 404 || probe.status >= 500) { console.error( @@ -163,6 +156,17 @@ if (!probe || probe.status === 404 || probe.status >= 500) { ); process.exit(1); } +if (!app.auth) { + const login = await fetch(`${base}/login`, { redirect: "manual" }).catch( + () => null, + ); + if (login?.status === 200) { + console.error( + `${base} serves the dashboard, not ${flags.app}; pass --base`, + ); + process.exit(1); + } +} // System Chrome locally; a remote VM has none, so fall back to Playwright's // bundled Chromium (`cd apps/screenshot-service && npx playwright install chromium`). @@ -180,18 +184,25 @@ const browser = await chromium }); }); let auth = null; -if (flags.token) auth = cookies(flags.token, flags.workspace); -else if (wantsAuth && !flags.login) +if (flags.token) { + auth = cookies(flags.token, flags.workspace); + console.error("auth: --token"); +} else if (wantsAuth && !flags.login) { auth = await sessionCookies().catch( (e) => (console.error(`auth: ${e.message}`), null), ); +} // One login for all themes: the dashboard rate-limits magic-link emails. -if (flags.login) { +if (flags.login && !auth) { const context = await browser.newContext(); await magicLinkLogin(context); await context.close(); } -const useState = wantsAuth && !auth && existsSync(stateFile); +// A saved state is only a fallback; an explicit identity must not silently +// become whoever logged in last. +const useState = + wantsAuth && !auth && !flags.user && !flags.token && existsSync(stateFile); +if (useState) console.error(`auth: saved state ${stateFile}`); if (wantsAuth && !auth && !useState) { console.error( "auth: no session found; seed the database, pass --token, or --login --log ", @@ -218,14 +229,12 @@ for (const theme of themes) { const page = await context.newPage(); for (const path of paths) { await page.goto(base + path, { waitUntil: "networkidle" }); - // Only the dashboard has a login page, so landing there means auth - // failed or --base points at the dashboard for another app. - if (page.url().includes("/login")) - throw new Error( - app.auth - ? `redirected to login for ${path}; auth failed` - : `${base} serves the dashboard, not ${flags.app}; pass --base`, - ); + if ( + app.auth && + new URL(page.url()).pathname === "/login" && + !path.startsWith("/login") + ) + throw new Error(`redirected to login for ${path}; auth failed`); const name = path.replace(/^\/+/, "").replace(/[^a-z0-9]+/gi, "-") || "root"; if (flags.selector) { diff --git a/turbo.json b/turbo.json index 72795002b..b3cd23ec2 100644 --- a/turbo.json +++ b/turbo.json @@ -4,7 +4,9 @@ // Vercel disables corepack unless it can confirm turbo >= 2.1.3 or sees this // passthrough; without corepack it falls back to pnpm 9, which can't read // `overrides` from pnpm-workspace.yaml and fails the frozen install. - "globalPassThroughEnv": ["COREPACK_HOME"], + // PORT: strict env mode would strip it, and `pnpm dev:dashboard` could never + // move off :3001. + "globalPassThroughEnv": ["COREPACK_HOME", "PORT"], "tasks": { "registry:build": { "outputs": ["public/r/**"] -- 2.51.2