From e4969263921c2bed2d2456ed614da5ebbd8334ab Mon Sep 17 00:00:00 2001 From: jamie-at-bunny Date: Tue, 28 Jul 2026 12:28:53 +0100 Subject: [PATCH 1/5] feat(db): migrations create, list, and apply Adds `bunny db migrations` for running plain SQL migration files against a Bunny Database. Files live in `migrations/` (falling back to `drizzle/`) and are named `NNNN_.sql`; the filename is the migration's identity and its numeric prefix is the apply order. Applied migrations are recorded in `__bunny_migrations`, which existing introspection excludes already, so it stays out of `db studio` and the REST layer. Each file runs through `client.migrate()` together with its tracking row, so a migration either lands and is recorded or neither happens, and foreign keys stay deferred for table rebuilds. `list` reports applied, pending, modified, and missing state without creating the tracking table. `apply` stops at the first failure and confirms only when a TTY is attached. `splitStatements` now keeps `CREATE TRIGGER ... BEGIN ... END;` bodies intact, which `db shell .sql` benefits from too. Credential resolution moves to `db/credentials.ts`, shared by shell, studio, and migrations apply instead of a third copy. --- .changeset/db-migrations.md | 6 + AGENTS.md | 72 +++- README.md | 3 + packages/cli/src/commands/db/credentials.ts | 90 +++++ packages/cli/src/commands/db/index.ts | 2 + .../cli/src/commands/db/migrations/apply.ts | 233 +++++++++++ .../src/commands/db/migrations/constants.ts | 11 + .../cli/src/commands/db/migrations/create.ts | 92 +++++ .../cli/src/commands/db/migrations/drift.ts | 33 ++ .../src/commands/db/migrations/engine.test.ts | 379 ++++++++++++++++++ .../cli/src/commands/db/migrations/engine.ts | 257 ++++++++++++ .../cli/src/commands/db/migrations/index.ts | 14 + .../cli/src/commands/db/migrations/list.ts | 161 ++++++++ packages/cli/src/commands/db/shell.ts | 96 +---- packages/cli/src/commands/db/studio.ts | 87 +--- packages/database-shell/src/parser.ts | 17 + packages/database-shell/src/shell.test.ts | 26 ++ skills/bunny-cli/SKILL.md | 3 +- skills/bunny-cli/references/database.md | 55 +++ 19 files changed, 1465 insertions(+), 172 deletions(-) create mode 100644 .changeset/db-migrations.md create mode 100644 packages/cli/src/commands/db/credentials.ts create mode 100644 packages/cli/src/commands/db/migrations/apply.ts create mode 100644 packages/cli/src/commands/db/migrations/constants.ts create mode 100644 packages/cli/src/commands/db/migrations/create.ts create mode 100644 packages/cli/src/commands/db/migrations/drift.ts create mode 100644 packages/cli/src/commands/db/migrations/engine.test.ts create mode 100644 packages/cli/src/commands/db/migrations/engine.ts create mode 100644 packages/cli/src/commands/db/migrations/index.ts create mode 100644 packages/cli/src/commands/db/migrations/list.ts diff --git a/.changeset/db-migrations.md b/.changeset/db-migrations.md new file mode 100644 index 00000000..0d8f6f67 --- /dev/null +++ b/.changeset/db-migrations.md @@ -0,0 +1,6 @@ +--- +"@bunny.net/cli": minor +"@bunny.net/database-shell": patch +--- + +feat(db): `bunny db migrations create/list/apply` runs numbered `.sql` files in `migrations/` (or `drizzle/`) once each, tracked in `__bunny_migrations`; `splitStatements` now keeps `CREATE TRIGGER` bodies intact diff --git a/AGENTS.md b/AGENTS.md index 7d088b91..2fb35484 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -279,6 +279,7 @@ bunny-cli/ │ │ │ ├── create.ts # Create a new database (interactive region selection or flags) │ │ │ ├── delete.ts # Delete a database (double confirmation or --force) │ │ │ ├── docs.ts # Open database documentation in browser +│ │ │ ├── credentials.ts # Shared: resolve libSQL url + token (flags → .env → API) for shell, studio, migrations apply │ │ │ ├── link.ts # Link directory to a database (.bunny/database.json) │ │ │ ├── list.ts # List all databases │ │ │ ├── quickstart.ts # Generate quickstart guide for connecting to a database @@ -289,6 +290,14 @@ bunny-cli/ │ │ │ ├── show.ts # Show database details (regions, size, status) │ │ │ ├── studio.ts # Open a visual database explorer in the browser (local web UI) │ │ │ ├── usage.ts # Show database usage statistics +│ │ │ ├── migrations/ +│ │ │ │ ├── index.ts # defineNamespace("migrations", ...) — registers migration commands +│ │ │ │ ├── constants.ts # Default dir, drizzle fallback dir, tracking table name +│ │ │ │ ├── engine.ts # Shared: discover files, checksums, applied/pending state, apply one migration +│ │ │ │ ├── drift.ts # Shared: warn when applied migrations were edited or deleted +│ │ │ │ ├── apply.ts # Apply pending migrations in filename order +│ │ │ │ ├── create.ts # Write an empty numbered migration file +│ │ │ │ └── list.ts # Show applied/pending/modified/missing state │ │ │ ├── regions/ │ │ │ │ ├── index.ts # defineNamespace("regions", ...) — registers region commands │ │ │ │ ├── add.ts # Add primary/replica regions (interactive multiselect or flags) @@ -1050,6 +1059,13 @@ bunny │ ├── docs Open database documentation in browser │ ├── list (alias: ls) [--group-id] │ │ List all databases +│ ├── migrations Create and apply SQL migrations (files are the source of truth) +│ │ ├── apply [database-id] [--dir] [--url] [--token] [--dry-run] [--force] +│ │ │ Apply pending migrations in filename order (each file + its tracking row is one atomic batch) +│ │ ├── create (alias: new) [--dir] +│ │ │ Write an empty migrations/NNNN_.sql +│ │ └── list [database-id] (aliases: ls, status) [--dir] [--url] [--token] +│ │ Show applied / pending / modified / missing migrations │ ├── quickstart [database-id] [--lang] [--url] [--token] │ │ Generate quickstart guide for a database │ ├── regions @@ -1440,7 +1456,7 @@ The shell is split across two packages: - **Formatting** (`format.ts`) — `printResultSet()` with 5 output modes: `default`, `table`, `json`, `csv`, `markdown`. Sensitive column masking (full mask for passwords/secrets, email mask for email columns). - **Views** (`views.ts`) — Saved queries scoped per database. Stored at `~/.config/bunny/views//` (respects `XDG_CONFIG_HOME`). Callers can override via `ShellOptions.viewsDir`. - **History** (`history.ts`) — Stored at `~/.config/bunny/shell_history` (respects `XDG_CONFIG_HOME`). Max 1000 entries. -- **SQL parsing** (`parser.ts`) — `splitStatements()` for `.sql` file execution. +- **SQL parsing** (`parser.ts`) — `splitStatements()` for `.sql` file execution. Splits on `;` outside string literals, strips `--` comments (so drizzle's `--> statement-breakpoint` markers are ignored), and keeps `CREATE TRIGGER ... BEGIN ... END;` bodies intact. **Dependency injection** — The shell engine accepts a `ShellLogger` interface instead of importing the CLI logger directly: @@ -1456,7 +1472,7 @@ interface ShellLogger { **CLI wrapper** (`packages/cli/src/commands/db/shell.ts`) provides: -- Credential resolution (--url/--token flags → .env → API lookup) +- Credential resolution via `resolveCredentials()` in `packages/cli/src/commands/db/credentials.ts` (--url/--token flags → .env → API lookup), shared with `db studio` and `db migrations apply` - `shellLogger()` adapter that wraps the CLI `logger` - `createClient()` call and delegation to `startShell()`/`executeQuery()`/`executeFile()` - Passes resolved `databaseId` and optional `--views-dir` to `startShell()` for saved views @@ -1481,6 +1497,58 @@ bunny db shell seed.sql --- +## Database Migrations (`bunny db migrations`) + +### Overview + +Schema changes live in plain `.sql` files that the developer writes (or generates with an ORM). The CLI's job is only to run them in order, once each, and record what it ran. There is no rollback: SQLite can't reverse most DDL, so the fix for a bad migration is another migration. + +### Convention + +- Files live in `migrations/` by default, one statement group per file, named `NNNN_.sql`. +- The **filename is the migration's identity**, and its numeric prefix is the order. Nothing else (no journal, no manifest) tracks migrations locally. +- Files are applied in lexicographic filename order, which is why prefixes are zero-padded to four digits. +- Applied migrations are recorded in `__bunny_migrations` (`id`, `name`, `checksum`, `applied_at`). The `__` prefix means `DEFAULT_EXCLUDE_PATTERNS` in `packages/database-adapter-libsql/src/introspect.ts` already hides it from `db studio` and the REST layer. + +### Engine (`packages/cli/src/commands/db/migrations/engine.ts`) + +All file and state logic is here so the commands stay thin and the logic is testable against an in-memory libSQL database (`engine.test.ts`, no network): + +- `resolveMigrationsDir(dirArg?)` — `--dir` wins; otherwise `migrations/`, falling back to `drizzle/` when `migrations/` doesn't exist (`detected: true` so the caller can say which directory it used). +- `discoverMigrations(dir)` — every `.sql` file, sorted by name. Skips dotfiles and subdirectories, so `drizzle/meta/` is ignored. +- `checksum(sql)` — sha256 of the body with CRLF normalized and edges trimmed, so reformatting line endings isn't reported as a change. +- `migrationStatuses(files, applied)` / `pendingMigrations(files, applied)` — join disk against the table. A recorded migration whose file changed is `modified`; one whose file is gone is `missing`. Both are warnings (`drift.ts`), never fatal: pending migrations still apply cleanly, and the remedy is the developer's call. +- `applyMigration(client, file)` — splits the file with `splitStatements()` and runs the statements plus the tracking-row insert through `client.migrate()`, so a migration either lands and is recorded or neither happens. + +`client.migrate()` is used rather than `client.batch()` because it defers foreign key enforcement for the batch, which table rebuilds and `ALTER TABLE` need. `db shell .sql` still uses `batch()` and is not migration-aware. + +### ORM-generated migrations + +`drizzle-kit generate` (sqlite/turso dialect) writes flat `0000_.sql` files, matching this convention, so no glob or pattern config is needed. Generate with the ORM, apply with the CLI: + +```bash +drizzle-kit generate # writes drizzle/0000_curly_bat.sql +bunny db migrations apply # finds drizzle/ automatically +bunny db migrations apply --dir drizzle # or be explicit +``` + +`db migrations create` only writes top-level files; use the ORM's own generate command when an ORM owns the schema. + +### Applying + +`apply` runs pending migrations sequentially and stops at the first failure, reporting how many applied and how many are still pending. It confirms before writing when a TTY is attached, and skips the prompt under `--force` or any non-interactive run (`--output json`, no TTY) so CI and agents aren't blocked. `--dry-run` lists what would run without writing. + +```bash +bunny db migrations create add_users_table # migrations/0001_add_users_table.sql +bunny db migrations list # applied / pending / modified / missing +bunny db migrations apply --dry-run +bunny db migrations apply +``` + +`list` never creates the tracking table (it checks `sqlite_master` first), so it's safe to run against a database that has never had a migration applied. + +--- + ## Conventions for Adding New Commands 1. Create a new directory under `packages/cli/src/commands/` for the domain (e.g., `packages/cli/src/commands/deploy/`). diff --git a/README.md b/README.md index 79f9be2f..53a2b1aa 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,9 @@ bun ny # Examples bun ny login bun ny db list +bun ny db migrations create add_users # write migrations/0001_add_users.sql (numeric prefix = apply order) +bun ny db migrations list # show applied / pending migrations +bun ny db migrations apply # apply pending migrations in order (--dry-run to preview, --dir drizzle for drizzle-kit output) bun ny apps deploy ghcr.io/me/api:v1.2 # deploy a pre-built image bun ny apps deploy --dockerfile # build ./Dockerfile and deploy bun ny apps deploy # first run? Imports docker-compose.yml if present; otherwise auto-detects Dockerfile(s) (including monorepo subdirs) so you can pick one or many, or falls back to a pre-built image. diff --git a/packages/cli/src/commands/db/credentials.ts b/packages/cli/src/commands/db/credentials.ts new file mode 100644 index 00000000..05a9f3a2 --- /dev/null +++ b/packages/cli/src/commands/db/credentials.ts @@ -0,0 +1,90 @@ +import { createDbClient } from "@bunny.net/openapi-client"; +import { resolveConfig } from "../../config/index.ts"; +import { clientOptions } from "../../core/client-options.ts"; +import { UserError } from "../../core/errors.ts"; +import { spinner } from "../../core/ui.ts"; +import { readEnvValue } from "../../utils/env-file.ts"; +import { generateToken, tokenExpiryFromNow } from "./api.ts"; +import { ENV_DATABASE_AUTH_TOKEN, ENV_DATABASE_URL } from "./constants.ts"; +import { resolveDbId } from "./resolve-db.ts"; + +export interface ResolvedCredentials { + url: string; + token: string; + databaseId: string | undefined; + /** True when a short-lived token was created for this run rather than read from flags or `.env`. */ + tokenGenerated: boolean; +} + +export interface ResolveCredentialsOptions { + url?: string; + token?: string; + databaseId?: string; + profile: string; + apiKey?: string; + verbose?: boolean; +} + +/** + * Resolve the database URL and auth token needed to connect over libSQL. + * + * Resolution order: + * 1. Explicit `url` / `token` (the `--url` / `--token` flags) + * 2. `BUNNY_DATABASE_URL` / `BUNNY_DATABASE_AUTH_TOKEN` from `.env` + * 3. API lookup (fetches the URL and/or creates a short-lived token on the fly) + * + * Shared by `db shell`, `db studio`, and `db migrations apply`. + */ +export async function resolveCredentials( + opts: ResolveCredentialsOptions, +): Promise { + let url = opts.url ?? readEnvValue(ENV_DATABASE_URL)?.value; + let token = opts.token ?? readEnvValue(ENV_DATABASE_AUTH_TOKEN)?.value; + + if (url && token) { + return { + url, + token, + databaseId: opts.databaseId, + tokenGenerated: false, + }; + } + + const config = resolveConfig(opts.profile, opts.apiKey, opts.verbose); + const apiClient = createDbClient(clientOptions(config, opts.verbose)); + + const { id: databaseId } = await resolveDbId(apiClient, opts.databaseId); + + const spin = spinner("Connecting..."); + spin.start(); + + const willGenerateToken = !token; + + const dbFetch = url + ? Promise.resolve(null) + : apiClient.GET("/v2/databases/{db_id}", { + params: { path: { db_id: databaseId } }, + }); + + if (willGenerateToken) spin.text = "Generating token..."; + + const tokenFetch = willGenerateToken + ? generateToken(apiClient, databaseId, { + authorization: "full-access", + expiresAt: tokenExpiryFromNow(), + }) + : Promise.resolve(null); + + const [dbResult, tokenResult] = await Promise.all([dbFetch, tokenFetch]); + + spin.stop(); + + if (!url && dbResult) url = dbResult.data?.db?.url; + if (willGenerateToken && tokenResult) token = tokenResult.token; + + if (!url || !token) { + throw new UserError("Could not resolve database URL or generate token."); + } + + return { url, token, databaseId, tokenGenerated: willGenerateToken }; +} diff --git a/packages/cli/src/commands/db/index.ts b/packages/cli/src/commands/db/index.ts index c7410308..57f04932 100644 --- a/packages/cli/src/commands/db/index.ts +++ b/packages/cli/src/commands/db/index.ts @@ -4,6 +4,7 @@ import { dbDeleteCommand } from "./delete.ts"; import { dbDocsCommand } from "./docs.ts"; import { dbLinkCommand } from "./link.ts"; import { dbListCommand } from "./list.ts"; +import { dbMigrationsNamespace } from "./migrations/index.ts"; import { dbQuickstartCommand } from "./quickstart.ts"; import { dbRegionsNamespace } from "./regions/index.ts"; import { dbShellCommand } from "./shell.ts"; @@ -18,6 +19,7 @@ export const dbNamespace = defineNamespace("db", "Manage databases.", [ dbDocsCommand, dbLinkCommand, dbListCommand, + dbMigrationsNamespace, dbQuickstartCommand, dbRegionsNamespace, dbShellCommand, diff --git a/packages/cli/src/commands/db/migrations/apply.ts b/packages/cli/src/commands/db/migrations/apply.ts new file mode 100644 index 00000000..569fdd2a --- /dev/null +++ b/packages/cli/src/commands/db/migrations/apply.ts @@ -0,0 +1,233 @@ +import { relative } from "node:path"; +import { defineCommand } from "../../../core/define-command.ts"; +import { errorMessage, UserError } from "../../../core/errors.ts"; +import { logger } from "../../../core/logger.ts"; +import { confirm, isInteractive, spinner } from "../../../core/ui.ts"; +import { ARG_DATABASE_ID, TOKEN_TTL_MINUTES } from "../constants.ts"; +import { resolveCredentials } from "../credentials.ts"; +import { ARG_DIR, MIGRATIONS_TABLE } from "./constants.ts"; +import { warnOnDrift } from "./drift.ts"; +import { + applyMigration, + discoverMigrations, + ensureMigrationsTable, + fetchApplied, + migrationStatuses, + pendingMigrations, + resolveMigrationsDir, +} from "./engine.ts"; + +const COMMAND = `apply [${ARG_DATABASE_ID}]`; +const DESCRIPTION = "Apply pending migrations to a database."; + +const ARG_URL = "url"; +const ARG_TOKEN = "token"; +const ARG_DRY_RUN = "dry-run"; +const ARG_FORCE = "force"; +const ARG_FORCE_ALIAS = "f"; + +interface ApplyArgs { + [ARG_DATABASE_ID]?: string; + [ARG_DIR]?: string; + [ARG_URL]?: string; + [ARG_TOKEN]?: string; + [ARG_DRY_RUN]?: boolean; + [ARG_FORCE]?: boolean; +} + +/** + * Apply every pending migration, in filename order. + * + * Each file runs as one atomic batch together with its tracking row, so a + * migration either lands and is recorded or neither happens. The run stops at + * the first failure and leaves the remaining migrations pending. + * + * @example + * ```bash + * bunny db migrations apply + * bunny db migrations apply --dry-run + * bunny db migrations apply --dir drizzle --force + * ``` + */ +export const dbMigrationsApplyCommand = defineCommand({ + command: COMMAND, + describe: DESCRIPTION, + examples: [ + ["$0 db migrations apply", "Apply all pending migrations"], + ["$0 db migrations apply --dry-run", "Show what would run without writing"], + ["$0 db migrations apply --dir drizzle", "Apply drizzle-kit output"], + ], + + builder: (yargs) => + yargs + .positional(ARG_DATABASE_ID, { + type: "string", + describe: + "Database ID (db_). Auto-detected from BUNNY_DATABASE_URL in .env if omitted.", + }) + .option(ARG_DIR, { + type: "string", + describe: "Migrations directory (default: migrations)", + }) + .option(ARG_URL, { + type: "string", + describe: "Database URL (skips API lookup)", + }) + .option(ARG_TOKEN, { + type: "string", + describe: "Auth token (skips token generation)", + }) + .option(ARG_DRY_RUN, { + type: "boolean", + default: false, + describe: "List the migrations that would run, without applying them", + }) + .option(ARG_FORCE, { + alias: ARG_FORCE_ALIAS, + type: "boolean", + default: false, + describe: "Skip confirmation prompts", + }), + + handler: async ({ + [ARG_DATABASE_ID]: databaseIdArg, + [ARG_DIR]: dirArg, + [ARG_URL]: urlArg, + [ARG_TOKEN]: tokenArg, + [ARG_DRY_RUN]: dryRun, + [ARG_FORCE]: force, + profile, + output, + verbose, + apiKey, + }) => { + const json = output === "json"; + + const { dir, detected } = resolveMigrationsDir(dirArg); + const files = discoverMigrations(dir); + const displayDir = relative(process.cwd(), dir) || "."; + + if (files.length === 0) { + throw new UserError( + `No migrations found in ${displayDir}.`, + "Run `bunny db migrations create ` to add one.", + ); + } + + if (detected && !json) logger.dim(`Using ${displayDir}`); + + const { url, token, tokenGenerated } = await resolveCredentials({ + url: urlArg, + token: tokenArg, + databaseId: databaseIdArg, + profile, + apiKey, + verbose, + }); + + if (tokenGenerated && !json) { + logger.dim( + `Session active for ${TOKEN_TTL_MINUTES} minutes. Re-run after that to reconnect.`, + ); + } + + const { createClient } = await import("@libsql/client/web"); + const client = createClient({ url, authToken: token }); + + await ensureMigrationsTable(client); + const applied = await fetchApplied(client); + const statuses = migrationStatuses(files, applied); + const pending = pendingMigrations(files, applied); + + /** `pending` is what was outstanding at the start; `done` is what actually ran. */ + const report = (done: string[]) => + logger.log( + JSON.stringify( + { + dir: displayDir, + table: MIGRATIONS_TABLE, + pending: pending.map((f) => f.name), + applied: done, + dry_run: Boolean(dryRun), + }, + null, + 2, + ), + ); + + if (pending.length === 0) { + if (json) { + report([]); + return; + } + logger.success("Already up to date."); + warnOnDrift(statuses); + return; + } + + if (!json) { + logger.log( + `${pending.length} pending migration${pending.length === 1 ? "" : "s"}:`, + ); + for (const file of pending) logger.log(` ${file.name}`); + logger.log(""); + warnOnDrift(statuses); + } + + if (dryRun) { + if (json) { + report([]); + return; + } + logger.dim("Dry run: nothing was applied."); + return; + } + + // Prompt only when a human is watching, so CI and agent runs aren't blocked. + const confirmed = await confirm("Apply now?", { + force: force || !isInteractive(output), + initial: true, + }); + if (!confirmed) { + logger.log("Cancelled."); + return; + } + + const done: string[] = []; + + for (const file of pending) { + const spin = spinner(`Applying ${file.name}...`); + if (!json) spin.start(); + + try { + const { statements } = await applyMigration(client, file); + spin.stop(); + done.push(file.name); + if (!json) { + logger.success( + `${file.name} (${statements} statement${statements === 1 ? "" : "s"})`, + ); + } + } catch (err: unknown) { + spin.stop(); + const remaining = pending.length - done.length - 1; + throw new UserError( + `${file.name} failed: ${errorMessage(err)}`, + done.length > 0 || remaining > 0 + ? `${done.length} applied, ${remaining} still pending. Fix ${file.name} and re-run.` + : undefined, + ); + } + } + + if (json) { + report(done); + return; + } + + logger.log(""); + logger.success( + `Applied ${done.length} migration${done.length === 1 ? "" : "s"}.`, + ); + }, +}); diff --git a/packages/cli/src/commands/db/migrations/constants.ts b/packages/cli/src/commands/db/migrations/constants.ts new file mode 100644 index 00000000..d5fffa74 --- /dev/null +++ b/packages/cli/src/commands/db/migrations/constants.ts @@ -0,0 +1,11 @@ +/** Default directory holding migration files, relative to the working directory. */ +export const DEFAULT_MIGRATIONS_DIR = "migrations"; + +/** Directories checked when `--dir` is omitted and the default doesn't exist. */ +export const FALLBACK_MIGRATIONS_DIRS = ["drizzle"] as const; + +/** Table recording applied migrations. The `__` prefix keeps it out of studio and REST introspection. */ +export const MIGRATIONS_TABLE = "__bunny_migrations"; + +/** Flag name for overriding the migrations directory. */ +export const ARG_DIR = "dir"; diff --git a/packages/cli/src/commands/db/migrations/create.ts b/packages/cli/src/commands/db/migrations/create.ts new file mode 100644 index 00000000..047b6150 --- /dev/null +++ b/packages/cli/src/commands/db/migrations/create.ts @@ -0,0 +1,92 @@ +import { existsSync, mkdirSync, writeFileSync } from "node:fs"; +import { join, relative } from "node:path"; +import { defineCommand } from "../../../core/define-command.ts"; +import { UserError } from "../../../core/errors.ts"; +import { logger } from "../../../core/logger.ts"; +import { ARG_DIR } from "./constants.ts"; +import { + discoverMigrations, + nextSequence, + resolveMigrationsDir, + slugify, +} from "./engine.ts"; + +const COMMAND = "create "; +const ALIASES = ["new"] as const; +const DESCRIPTION = "Create an empty migration file."; + +interface CreateArgs { + name: string; + [ARG_DIR]?: string; +} + +/** + * Create an empty, numbered migration file. + * + * The filename (`0001_add_users_table.sql`) is the migration's identity, so the + * numeric prefix determines the order `db migrations apply` runs them in. + * + * @example + * ```bash + * bunny db migrations create add_users_table + * bunny db migrations create "add users table" --dir db/migrations + * ``` + */ +export const dbMigrationsCreateCommand = defineCommand({ + command: COMMAND, + aliases: ALIASES, + describe: DESCRIPTION, + examples: [ + [ + "$0 db migrations create add_users_table", + "Create migrations/0001_add_users_table.sql", + ], + [ + "$0 db migrations create add_index --dir db/migrations", + "Use a custom directory", + ], + ], + + builder: (yargs) => + yargs + .positional("name", { + type: "string", + describe: "Migration name, used as the filename suffix", + demandOption: true, + }) + .option(ARG_DIR, { + type: "string", + describe: "Migrations directory (default: migrations)", + }), + + handler: async ({ name, [ARG_DIR]: dirArg, output }) => { + const { dir } = resolveMigrationsDir(dirArg); + + if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); + + const slug = slugify(name); + const sequence = nextSequence(discoverMigrations(dir)); + const filename = `${sequence}_${slug}.sql`; + const path = join(dir, filename); + + if (existsSync(path)) { + throw new UserError( + `Migration already exists: ${relative(process.cwd(), path)}`, + ); + } + + writeFileSync(path, `-- ${filename}\n`); + + const displayPath = relative(process.cwd(), path); + + if (output === "json") { + logger.log( + JSON.stringify({ name: filename, path: displayPath }, null, 2), + ); + return; + } + + logger.success(`Created ${displayPath}`); + logger.dim("Add your SQL, then run `bunny db migrations apply`."); + }, +}); diff --git a/packages/cli/src/commands/db/migrations/drift.ts b/packages/cli/src/commands/db/migrations/drift.ts new file mode 100644 index 00000000..630062e7 --- /dev/null +++ b/packages/cli/src/commands/db/migrations/drift.ts @@ -0,0 +1,33 @@ +import { logger } from "../../../core/logger.ts"; +import type { MigrationStatus } from "./engine.ts"; + +/** + * Warn when the files on disk no longer describe what the database has applied. + * + * Both cases are reported rather than fatal: pending migrations can still be + * applied safely, and the fix (restore the file, or re-create the change as a + * new migration) is the developer's call. + */ +export function warnOnDrift(statuses: MigrationStatus[]): void { + const modified = statuses.filter((s) => s.state === "modified"); + const missing = statuses.filter((s) => s.state === "missing"); + + if (modified.length > 0) { + logger.log(""); + logger.warn( + `${modified.length} applied migration${modified.length === 1 ? " has" : "s have"} changed since being applied:`, + ); + for (const s of modified) logger.dim(` ${s.name}`); + logger.dim( + " The database was not updated. Add a new migration instead of editing an applied one.", + ); + } + + if (missing.length > 0) { + logger.log(""); + logger.warn( + `${missing.length} applied migration${missing.length === 1 ? "" : "s"} no longer exist${missing.length === 1 ? "s" : ""} on disk:`, + ); + for (const s of missing) logger.dim(` ${s.name}`); + } +} diff --git a/packages/cli/src/commands/db/migrations/engine.test.ts b/packages/cli/src/commands/db/migrations/engine.test.ts new file mode 100644 index 00000000..b2c9e24a --- /dev/null +++ b/packages/cli/src/commands/db/migrations/engine.test.ts @@ -0,0 +1,379 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createClient } from "@libsql/client"; +import { + applyMigration, + checksum, + discoverMigrations, + ensureMigrationsTable, + fetchApplied, + type MigrationClient, + migrationStatuses, + migrationsTableExists, + nextSequence, + pendingMigrations, + resolveMigrationsDir, + slugify, +} from "./engine.ts"; + +let dir: string; + +beforeEach(() => { + // realpath so chdir-based assertions match on macOS, where /var is a symlink to /private/var. + dir = realpathSync(mkdtempSync(join(tmpdir(), "bunny-migrations-"))); +}); + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +function write(name: string, sql: string) { + writeFileSync(join(dir, name), sql); +} + +function memoryClient(): MigrationClient { + return createClient({ url: ":memory:" }); +} + +describe("discoverMigrations", () => { + test("returns .sql files in filename order", () => { + write("0002_second.sql", "SELECT 2;"); + write("0001_first.sql", "SELECT 1;"); + write("0010_tenth.sql", "SELECT 10;"); + + expect(discoverMigrations(dir).map((f) => f.name)).toEqual([ + "0001_first.sql", + "0002_second.sql", + "0010_tenth.sql", + ]); + }); + + test("ignores non-sql files, dotfiles, and subdirectories", () => { + write("0001_first.sql", "SELECT 1;"); + write("README.md", "not sql"); + write(".hidden.sql", "SELECT 0;"); + mkdirSync(join(dir, "meta")); + writeFileSync(join(dir, "meta", "_journal.json"), "{}"); + + expect(discoverMigrations(dir).map((f) => f.name)).toEqual([ + "0001_first.sql", + ]); + }); + + test("throws a hinted error when the directory is missing", () => { + expect(() => discoverMigrations(join(dir, "nope"))).toThrow( + /Migrations directory not found/, + ); + }); + + test("ten or more migrations stay ordered because prefixes are zero-padded", () => { + for (let i = 1; i <= 12; i++) { + write(`${String(i).padStart(4, "0")}_m.sql`, `SELECT ${i};`); + } + + const names = discoverMigrations(dir).map((f) => f.name); + expect(names[8]).toBe("0009_m.sql"); + expect(names[9]).toBe("0010_m.sql"); + }); +}); + +describe("checksum", () => { + test("ignores line endings and trailing whitespace", () => { + expect(checksum("SELECT 1;\nSELECT 2;")).toBe( + checksum("SELECT 1;\r\nSELECT 2;\n\n"), + ); + }); + + test("changes when the SQL changes", () => { + expect(checksum("SELECT 1;")).not.toBe(checksum("SELECT 2;")); + }); +}); + +describe("nextSequence", () => { + test("starts at 0001 with no migrations", () => { + expect(nextSequence([])).toBe("0001"); + }); + + test("increments past the highest prefix, not the count", () => { + write("0001_a.sql", "SELECT 1;"); + write("0007_b.sql", "SELECT 1;"); + expect(nextSequence(discoverMigrations(dir))).toBe("0008"); + }); + + test("follows on from drizzle's zero-based numbering", () => { + write("0000_curly_bat.sql", "SELECT 1;"); + expect(nextSequence(discoverMigrations(dir))).toBe("0001"); + }); + + test("ignores files with no numeric prefix", () => { + write("init.sql", "SELECT 1;"); + expect(nextSequence(discoverMigrations(dir))).toBe("0001"); + }); +}); + +describe("slugify", () => { + test("normalizes separators and casing", () => { + expect(slugify("Add Users Table")).toBe("add_users_table"); + expect(slugify("add-users--table")).toBe("add_users_table"); + expect(slugify(" trim me ")).toBe("trim_me"); + }); + + test("rejects names with nothing usable", () => { + expect(() => slugify("---")).toThrow(/at least one letter or number/); + }); +}); + +describe("resolveMigrationsDir", () => { + const cwd = process.cwd(); + + afterEach(() => { + process.chdir(cwd); + }); + + test("an explicit dir wins", () => { + process.chdir(dir); + mkdirSync(join(dir, "migrations")); + const resolved = resolveMigrationsDir("custom"); + expect(resolved.dir).toBe(join(dir, "custom")); + expect(resolved.detected).toBe(false); + }); + + test("prefers migrations/ when it exists", () => { + process.chdir(dir); + mkdirSync(join(dir, "migrations")); + mkdirSync(join(dir, "drizzle")); + expect(resolveMigrationsDir()).toEqual({ + dir: join(dir, "migrations"), + detected: false, + }); + }); + + test("falls back to drizzle/ when migrations/ is absent", () => { + process.chdir(dir); + mkdirSync(join(dir, "drizzle")); + expect(resolveMigrationsDir()).toEqual({ + dir: join(dir, "drizzle"), + detected: true, + }); + }); + + test("returns the default when nothing exists", () => { + process.chdir(dir); + expect(resolveMigrationsDir()).toEqual({ + dir: join(dir, "migrations"), + detected: false, + }); + }); +}); + +describe("migrationStatuses", () => { + test("classifies applied, pending, modified, and missing", () => { + write("0001_applied.sql", "SELECT 1;"); + write("0002_modified.sql", "SELECT 2;"); + write("0003_pending.sql", "SELECT 3;"); + const files = discoverMigrations(dir); + + const statuses = migrationStatuses(files, [ + { + name: "0001_applied.sql", + checksum: checksum("SELECT 1;"), + applied_at: "2026-07-01 10:00:00", + }, + { + name: "0002_modified.sql", + checksum: checksum("SELECT 999;"), + applied_at: "2026-07-01 10:00:01", + }, + { + name: "0000_deleted.sql", + checksum: "abc", + applied_at: "2026-06-01 09:00:00", + }, + ]); + + expect(statuses).toEqual([ + { + name: "0001_applied.sql", + state: "applied", + appliedAt: "2026-07-01 10:00:00", + }, + { + name: "0002_modified.sql", + state: "modified", + appliedAt: "2026-07-01 10:00:01", + }, + { name: "0003_pending.sql", state: "pending" }, + { + name: "0000_deleted.sql", + state: "missing", + appliedAt: "2026-06-01 09:00:00", + }, + ]); + }); +}); + +describe("pendingMigrations", () => { + test("excludes applied files and keeps order", () => { + write("0001_a.sql", "SELECT 1;"); + write("0002_b.sql", "SELECT 2;"); + write("0003_c.sql", "SELECT 3;"); + const files = discoverMigrations(dir); + + const pending = pendingMigrations(files, [ + { name: "0002_b.sql", checksum: "x", applied_at: "now" }, + ]); + + expect(pending.map((f) => f.name)).toEqual(["0001_a.sql", "0003_c.sql"]); + }); + + test("a modified file counts as applied, not pending", () => { + write("0001_a.sql", "SELECT 1;"); + const files = discoverMigrations(dir); + + expect( + pendingMigrations(files, [ + { name: "0001_a.sql", checksum: "stale", applied_at: "now" }, + ]), + ).toEqual([]); + }); +}); + +describe("applyMigration", () => { + test("runs the statements and records the migration", async () => { + const client = memoryClient(); + await ensureMigrationsTable(client); + + write( + "0001_users.sql", + "CREATE TABLE users (id INTEGER PRIMARY KEY, name TEXT);\nINSERT INTO users VALUES (1, 'Ada');", + ); + const [file] = discoverMigrations(dir); + if (!file) throw new Error("no migration discovered"); + + const result = await applyMigration(client, file); + expect(result.statements).toBe(2); + + const rows = await client.execute("SELECT name FROM users"); + expect(rows.rows).toHaveLength(1); + + const applied = await fetchApplied(client); + expect(applied).toHaveLength(1); + expect(applied[0]?.name).toBe("0001_users.sql"); + expect(applied[0]?.checksum).toBe(file.checksum); + expect(applied[0]?.applied_at).toBeTruthy(); + }); + + test("records nothing when a statement fails", async () => { + const client = memoryClient(); + await ensureMigrationsTable(client); + + write( + "0001_broken.sql", + "CREATE TABLE ok (id INTEGER);\nCREATE TABLE ok (id INTEGER);", + ); + const [file] = discoverMigrations(dir); + if (!file) throw new Error("no migration discovered"); + + await expect(applyMigration(client, file)).rejects.toThrow(); + expect(await fetchApplied(client)).toEqual([]); + + const tables = await client.execute( + "SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'ok'", + ); + expect(tables.rows).toHaveLength(0); + }); + + test("applying the same migration twice is rejected by the unique name", async () => { + const client = memoryClient(); + await ensureMigrationsTable(client); + + write("0001_a.sql", "CREATE TABLE a (id INTEGER);"); + const [file] = discoverMigrations(dir); + if (!file) throw new Error("no migration discovered"); + + await applyMigration(client, file); + await expect(applyMigration(client, file)).rejects.toThrow(); + expect(await fetchApplied(client)).toHaveLength(1); + }); + + test("defers foreign keys so table rebuilds work", async () => { + const client = memoryClient(); + await ensureMigrationsTable(client); + await client.execute("PRAGMA foreign_keys = ON"); + await client.execute("CREATE TABLE parent (id INTEGER PRIMARY KEY)"); + await client.execute( + "CREATE TABLE child (id INTEGER PRIMARY KEY, parent_id INTEGER REFERENCES parent(id))", + ); + await client.execute("INSERT INTO parent VALUES (1)"); + await client.execute("INSERT INTO child VALUES (1, 1)"); + + write( + "0001_rebuild.sql", + [ + "CREATE TABLE parent_new (id INTEGER PRIMARY KEY, label TEXT);", + "INSERT INTO parent_new (id) SELECT id FROM parent;", + "DROP TABLE parent;", + "ALTER TABLE parent_new RENAME TO parent;", + ].join("\n"), + ); + const [file] = discoverMigrations(dir); + if (!file) throw new Error("no migration discovered"); + + await applyMigration(client, file); + + const cols = await client.execute("SELECT label FROM parent WHERE id = 1"); + expect(cols.rows).toHaveLength(1); + }); + + test("rejects a file with no statements", async () => { + const client = memoryClient(); + await ensureMigrationsTable(client); + + write("0001_empty.sql", "-- nothing to do\n"); + const [file] = discoverMigrations(dir); + if (!file) throw new Error("no migration discovered"); + + await expect(applyMigration(client, file)).rejects.toThrow( + /No SQL statements found/, + ); + }); +}); + +describe("migrationsTableExists", () => { + test("false before the table is created, true after", async () => { + const client = memoryClient(); + expect(await migrationsTableExists(client)).toBe(false); + await ensureMigrationsTable(client); + expect(await migrationsTableExists(client)).toBe(true); + }); +}); + +describe("ensureMigrationsTable", () => { + test("is idempotent and preserves rows", async () => { + const client = memoryClient(); + await ensureMigrationsTable(client); + + write("0001_a.sql", "CREATE TABLE a (id INTEGER);"); + const [file] = discoverMigrations(dir); + if (!file) throw new Error("no migration discovered"); + await applyMigration(client, file); + + await ensureMigrationsTable(client); + expect(await fetchApplied(client)).toHaveLength(1); + }); + + test("refuses a table name that isn't a bare identifier", async () => { + const client = memoryClient(); + await expect( + ensureMigrationsTable(client, 'x"; DROP TABLE users; --'), + ).rejects.toThrow(/Invalid table name/); + }); +}); diff --git a/packages/cli/src/commands/db/migrations/engine.ts b/packages/cli/src/commands/db/migrations/engine.ts new file mode 100644 index 00000000..0c9e4b14 --- /dev/null +++ b/packages/cli/src/commands/db/migrations/engine.ts @@ -0,0 +1,257 @@ +import { createHash } from "node:crypto"; +import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { splitStatements } from "@bunny.net/database-shell"; +import type { Client } from "@libsql/client"; +import { UserError } from "../../../core/errors.ts"; +import { + DEFAULT_MIGRATIONS_DIR, + FALLBACK_MIGRATIONS_DIRS, + MIGRATIONS_TABLE, +} from "./constants.ts"; + +/** The libSQL client surface the engine needs, so tests can pass an in-memory client. */ +export type MigrationClient = Pick; + +export interface MigrationFile { + /** Filename including the `.sql` extension, e.g. `0001_add_users.sql`. */ + name: string; + path: string; + sql: string; + checksum: string; +} + +export interface AppliedMigration { + name: string; + checksum: string; + applied_at: string; +} + +export type MigrationState = "applied" | "pending" | "modified" | "missing"; + +export interface MigrationStatus { + name: string; + state: MigrationState; + /** Set for every state except `pending`. */ + appliedAt?: string; +} + +/** Only bare identifiers are safe to interpolate into SQL, so refuse anything else. */ +function quoteIdentifier(name: string): string { + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(name)) { + throw new UserError(`Invalid table name: ${name}`); + } + return `"${name}"`; +} + +/** Hash of the migration body, normalized so line endings and trailing whitespace don't count as a change. */ +export function checksum(sql: string): string { + const normalized = sql.replace(/\r\n/g, "\n").trim(); + return createHash("sha256").update(normalized).digest("hex"); +} + +/** + * Pick the migrations directory. + * + * An explicit `--dir` always wins. Otherwise `migrations/` is used, falling back + * to a known ORM output directory (`drizzle/`) when `migrations/` doesn't exist, + * so `drizzle-kit generate` output works without configuration. + */ +export function resolveMigrationsDir(dirArg?: string): { + dir: string; + detected: boolean; +} { + if (dirArg) return { dir: resolve(dirArg), detected: false }; + + if (isDirectory(DEFAULT_MIGRATIONS_DIR)) { + return { dir: resolve(DEFAULT_MIGRATIONS_DIR), detected: false }; + } + + for (const candidate of FALLBACK_MIGRATIONS_DIRS) { + if (isDirectory(candidate)) { + return { dir: resolve(candidate), detected: true }; + } + } + + return { dir: resolve(DEFAULT_MIGRATIONS_DIR), detected: false }; +} + +function isDirectory(path: string): boolean { + return existsSync(path) && statSync(path).isDirectory(); +} + +/** + * Read every `.sql` file in `dir`, sorted by filename. + * + * Filenames are the migration identity, so the numeric prefix written by + * `db migrations create` (and by `drizzle-kit generate`) determines order. + * Subdirectories are ignored, which skips `drizzle/meta/`. + */ +export function discoverMigrations(dir: string): MigrationFile[] { + if (!isDirectory(dir)) { + throw new UserError( + `Migrations directory not found: ${dir}`, + "Run `bunny db migrations create ` to create your first migration.", + ); + } + + const files: MigrationFile[] = []; + + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (!entry.isFile()) continue; + if (entry.name.startsWith(".")) continue; + if (!entry.name.endsWith(".sql")) continue; + + const path = join(dir, entry.name); + const sql = readFileSync(path, "utf-8"); + files.push({ name: entry.name, path, sql, checksum: checksum(sql) }); + } + + return files.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); +} + +/** Next zero-padded sequence number, one above the highest numeric prefix present. */ +export function nextSequence(files: MigrationFile[]): string { + let highest = 0; + for (const file of files) { + const match = /^(\d+)/.exec(file.name); + if (!match?.[1]) continue; + highest = Math.max(highest, Number.parseInt(match[1], 10)); + } + return String(highest + 1).padStart(4, "0"); +} + +/** Normalize a user-supplied migration name into a filename-safe slug. */ +export function slugify(name: string): string { + const slug = name + .trim() + .toLowerCase() + .replace(/[^a-z0-9]+/g, "_") + .replace(/^_+|_+$/g, ""); + + if (!slug) { + throw new UserError( + `Migration name must contain at least one letter or number: ${name}`, + ); + } + + return slug; +} + +/** Create the tracking table if it isn't there yet. */ +export async function ensureMigrationsTable( + client: MigrationClient, + table = MIGRATIONS_TABLE, +): Promise { + await client.execute( + `CREATE TABLE IF NOT EXISTS ${quoteIdentifier(table)} ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL UNIQUE, + checksum TEXT NOT NULL, + applied_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP + )`, + ); +} + +/** True when the tracking table is present, so read-only commands don't have to create it. */ +export async function migrationsTableExists( + client: MigrationClient, + table = MIGRATIONS_TABLE, +): Promise { + const result = await client.execute({ + sql: "SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?", + args: [table], + }); + return result.rows.length > 0; +} + +/** Read the applied migrations, oldest first. Assumes the table exists. */ +export async function fetchApplied( + client: MigrationClient, + table = MIGRATIONS_TABLE, +): Promise { + const result = await client.execute( + `SELECT name, checksum, applied_at FROM ${quoteIdentifier(table)} ORDER BY id`, + ); + + return (result.rows as unknown as AppliedMigration[]).map((row) => ({ + name: String(row.name), + checksum: String(row.checksum), + applied_at: String(row.applied_at), + })); +} + +/** + * Join the files on disk with what the database has recorded. + * + * A file whose checksum no longer matches the recorded one is `modified`; a + * recorded migration with no matching file is `missing`. Both mean the local + * migrations no longer describe the database, so callers surface them. + */ +export function migrationStatuses( + files: MigrationFile[], + applied: AppliedMigration[], +): MigrationStatus[] { + const byName = new Map(applied.map((row) => [row.name, row])); + + const statuses: MigrationStatus[] = files.map((file) => { + const record = byName.get(file.name); + if (!record) return { name: file.name, state: "pending" }; + return { + name: file.name, + state: record.checksum === file.checksum ? "applied" : "modified", + appliedAt: record.applied_at, + }; + }); + + const onDisk = new Set(files.map((file) => file.name)); + for (const record of applied) { + if (onDisk.has(record.name)) continue; + statuses.push({ + name: record.name, + state: "missing", + appliedAt: record.applied_at, + }); + } + + return statuses; +} + +/** Files that haven't been applied yet, in filename order. */ +export function pendingMigrations( + files: MigrationFile[], + applied: AppliedMigration[], +): MigrationFile[] { + const byName = new Set(applied.map((row) => row.name)); + return files.filter((file) => !byName.has(file.name)); +} + +/** + * Apply one migration. + * + * Uses `migrate()` rather than `batch()` so foreign keys are deferred for the + * duration, which table rebuilds and `ALTER TABLE` need. The tracking row is + * part of the same batch, so a migration either lands and is recorded or + * neither happens. + */ +export async function applyMigration( + client: MigrationClient, + file: MigrationFile, + table = MIGRATIONS_TABLE, +): Promise<{ statements: number }> { + const statements = splitStatements(file.sql); + + if (statements.length === 0) { + throw new UserError(`No SQL statements found in ${file.name}.`); + } + + await client.migrate([ + ...statements.map((sql) => ({ sql })), + { + sql: `INSERT INTO ${quoteIdentifier(table)} (name, checksum) VALUES (?, ?)`, + args: [file.name, file.checksum], + }, + ]); + + return { statements: statements.length }; +} diff --git a/packages/cli/src/commands/db/migrations/index.ts b/packages/cli/src/commands/db/migrations/index.ts new file mode 100644 index 00000000..621fb8df --- /dev/null +++ b/packages/cli/src/commands/db/migrations/index.ts @@ -0,0 +1,14 @@ +import { defineNamespace } from "../../../core/define-namespace.ts"; +import { dbMigrationsApplyCommand } from "./apply.ts"; +import { dbMigrationsCreateCommand } from "./create.ts"; +import { dbMigrationsListCommand } from "./list.ts"; + +export const dbMigrationsNamespace = defineNamespace( + "migrations", + "Create and apply SQL migrations.", + [ + dbMigrationsApplyCommand, + dbMigrationsCreateCommand, + dbMigrationsListCommand, + ], +); diff --git a/packages/cli/src/commands/db/migrations/list.ts b/packages/cli/src/commands/db/migrations/list.ts new file mode 100644 index 00000000..15bbad48 --- /dev/null +++ b/packages/cli/src/commands/db/migrations/list.ts @@ -0,0 +1,161 @@ +import { relative } from "node:path"; +import { defineCommand } from "../../../core/define-command.ts"; +import { formatTable } from "../../../core/format.ts"; +import { logger } from "../../../core/logger.ts"; +import { ARG_DATABASE_ID } from "../constants.ts"; +import { resolveCredentials } from "../credentials.ts"; +import { ARG_DIR, MIGRATIONS_TABLE } from "./constants.ts"; +import { warnOnDrift } from "./drift.ts"; +import { + type AppliedMigration, + discoverMigrations, + fetchApplied, + migrationStatuses, + migrationsTableExists, + resolveMigrationsDir, +} from "./engine.ts"; + +const COMMAND = `list [${ARG_DATABASE_ID}]`; +const ALIASES = ["ls", "status"] as const; +const DESCRIPTION = "Show which migrations have been applied."; + +const ARG_URL = "url"; +const ARG_TOKEN = "token"; + +const STATE_LABELS = { + applied: "Applied", + pending: "Pending", + modified: "Modified", + missing: "Missing", +} as const; + +interface ListArgs { + [ARG_DATABASE_ID]?: string; + [ARG_DIR]?: string; + [ARG_URL]?: string; + [ARG_TOKEN]?: string; +} + +/** + * Compare the migration files on disk against what the database has recorded. + * + * @example + * ```bash + * bunny db migrations list + * bunny db migrations list --output json + * ``` + */ +export const dbMigrationsListCommand = defineCommand({ + command: COMMAND, + aliases: ALIASES, + describe: DESCRIPTION, + examples: [ + ["$0 db migrations list", "Show applied and pending migrations"], + ["$0 db migrations list --output json", "JSON output for scripting"], + ], + + builder: (yargs) => + yargs + .positional(ARG_DATABASE_ID, { + type: "string", + describe: + "Database ID (db_). Auto-detected from BUNNY_DATABASE_URL in .env if omitted.", + }) + .option(ARG_DIR, { + type: "string", + describe: "Migrations directory (default: migrations)", + }) + .option(ARG_URL, { + type: "string", + describe: "Database URL (skips API lookup)", + }) + .option(ARG_TOKEN, { + type: "string", + describe: "Auth token (skips token generation)", + }), + + handler: async ({ + [ARG_DATABASE_ID]: databaseIdArg, + [ARG_DIR]: dirArg, + [ARG_URL]: urlArg, + [ARG_TOKEN]: tokenArg, + profile, + output, + verbose, + apiKey, + }) => { + const { dir, detected } = resolveMigrationsDir(dirArg); + const files = discoverMigrations(dir); + const displayDir = relative(process.cwd(), dir) || "."; + + if (detected && output !== "json") { + logger.dim(`Using ${displayDir}`); + } + + const { url, token } = await resolveCredentials({ + url: urlArg, + token: tokenArg, + databaseId: databaseIdArg, + profile, + apiKey, + verbose, + }); + + const { createClient } = await import("@libsql/client/web"); + const client = createClient({ url, authToken: token }); + + // Don't create the tracking table from a read-only command. + const applied: AppliedMigration[] = (await migrationsTableExists(client)) + ? await fetchApplied(client) + : []; + + const statuses = migrationStatuses(files, applied); + + if (output === "json") { + logger.log( + JSON.stringify( + { + dir: displayDir, + table: MIGRATIONS_TABLE, + migrations: statuses.map((s) => ({ + name: s.name, + state: s.state, + applied_at: s.appliedAt ?? null, + })), + }, + null, + 2, + ), + ); + return; + } + + if (statuses.length === 0) { + logger.info(`No migrations found in ${displayDir}.`); + logger.dim("Run `bunny db migrations create ` to add one."); + return; + } + + logger.log( + formatTable( + ["Migration", "State", "Applied"], + statuses.map((s) => [ + s.name, + STATE_LABELS[s.state], + s.appliedAt ?? "-", + ]), + output, + ), + ); + + const pending = statuses.filter((s) => s.state === "pending").length; + logger.log(""); + logger.dim( + pending === 0 + ? "Up to date." + : `${pending} pending. Run \`bunny db migrations apply\` to apply ${pending === 1 ? "it" : "them"}.`, + ); + + warnOnDrift(statuses); + }, +}); diff --git a/packages/cli/src/commands/db/shell.ts b/packages/cli/src/commands/db/shell.ts index 9bcbf659..06bbd24d 100644 --- a/packages/cli/src/commands/db/shell.ts +++ b/packages/cli/src/commands/db/shell.ts @@ -1,22 +1,11 @@ import { existsSync } from "node:fs"; import { resolve } from "node:path"; import type { PrintMode, ShellLogger } from "@bunny.net/database-shell"; -import { createDbClient } from "@bunny.net/openapi-client"; -import { resolveConfig } from "../../config/index.ts"; -import { clientOptions } from "../../core/client-options.ts"; import { defineCommand } from "../../core/define-command.ts"; import { UserError } from "../../core/errors.ts"; import { logger } from "../../core/logger.ts"; -import { spinner } from "../../core/ui.ts"; -import { readEnvValue } from "../../utils/env-file.ts"; -import { generateToken, tokenExpiryFromNow } from "./api.ts"; -import { - ARG_DATABASE_ID, - ENV_DATABASE_AUTH_TOKEN, - ENV_DATABASE_URL, - TOKEN_TTL_MINUTES, -} from "./constants.ts"; -import { resolveDbId } from "./resolve-db.ts"; +import { ARG_DATABASE_ID, TOKEN_TTL_MINUTES } from "./constants.ts"; +import { resolveCredentials } from "./credentials.ts"; const COMMAND = `shell [${ARG_DATABASE_ID}] [query]`; const DESCRIPTION = "Open an interactive SQL shell for a database."; @@ -43,79 +32,6 @@ function shellLogger(): ShellLogger { }; } -/** - * Resolve the database URL and auth token needed to connect. - * - * Resolution order: - * 1. Explicit `--url` / `--token` flags - * 2. `BUNNY_DATABASE_URL` / `BUNNY_DATABASE_AUTH_TOKEN` from `.env` - * 3. API lookup (fetches the URL and/or generates a token on the fly) - */ -async function resolveCredentials( - urlArg: string | undefined, - tokenArg: string | undefined, - databaseIdArg: string | undefined, - profile: string, - apiKeyOverride?: string, - verbose = false, -): Promise<{ - url: string; - token: string; - databaseId: string | undefined; - tokenGenerated: boolean; -}> { - let url = urlArg ?? readEnvValue(ENV_DATABASE_URL)?.value; - let token = tokenArg ?? readEnvValue(ENV_DATABASE_AUTH_TOKEN)?.value; - - if (url && token) { - return { url, token, databaseId: databaseIdArg, tokenGenerated: false }; - } - - const config = resolveConfig(profile, apiKeyOverride, verbose); - const apiClient = createDbClient(clientOptions(config, verbose)); - - const { id: databaseId } = await resolveDbId(apiClient, databaseIdArg); - - const spin = spinner("Connecting..."); - spin.start(); - - const fetches: Promise[] = []; - const willGenerateToken = !token; - - if (!url) { - fetches.push( - apiClient.GET("/v2/databases/{db_id}", { - params: { path: { db_id: databaseId } }, - }), - ); - } else { - fetches.push(Promise.resolve(null)); - } - - if (willGenerateToken) { - spin.text = "Generating token..."; - fetches.push( - generateToken(apiClient, databaseId, { - authorization: "full-access", - expiresAt: tokenExpiryFromNow(), - }), - ); - } - - const [dbResult, tokenResult] = await Promise.all(fetches); - - spin.stop(); - - if (!url && dbResult) url = dbResult.data?.db?.url; - if (willGenerateToken && tokenResult) token = tokenResult.token; - - if (!url || !token) { - throw new UserError("Could not resolve database URL or generate token."); - } - - return { url, token, databaseId, tokenGenerated: willGenerateToken }; -} - export const dbShellCommand = defineCommand<{ [ARG_DATABASE_ID]?: string; query?: string; @@ -215,14 +131,14 @@ export const dbShellCommand = defineCommand<{ token, databaseId: resolvedDbId, tokenGenerated, - } = await resolveCredentials( - urlArg, - tokenArg, + } = await resolveCredentials({ + url: urlArg, + token: tokenArg, databaseId, profile, apiKey, verbose, - ); + }); if (tokenGenerated && output !== "json" && modeArg !== "json") { logger.dim( diff --git a/packages/cli/src/commands/db/studio.ts b/packages/cli/src/commands/db/studio.ts index ea571fb9..ecd55410 100644 --- a/packages/cli/src/commands/db/studio.ts +++ b/packages/cli/src/commands/db/studio.ts @@ -1,19 +1,8 @@ -import { createDbClient } from "@bunny.net/openapi-client"; -import { resolveConfig } from "../../config/index.ts"; -import { clientOptions } from "../../core/client-options.ts"; import { defineCommand } from "../../core/define-command.ts"; -import { UserError } from "../../core/errors.ts"; import { logger } from "../../core/logger.ts"; -import { confirm, spinner } from "../../core/ui.ts"; -import { readEnvValue } from "../../utils/env-file.ts"; -import { generateToken, tokenExpiryFromNow } from "./api.ts"; -import { - ARG_DATABASE_ID, - ENV_DATABASE_AUTH_TOKEN, - ENV_DATABASE_URL, - TOKEN_TTL_MINUTES, -} from "./constants.ts"; -import { resolveDbId } from "./resolve-db.ts"; +import { confirm } from "../../core/ui.ts"; +import { ARG_DATABASE_ID, TOKEN_TTL_MINUTES } from "./constants.ts"; +import { resolveCredentials } from "./credentials.ts"; const COMMAND = `studio [${ARG_DATABASE_ID}]`; const DESCRIPTION = "Open a visual database explorer in your browser."; @@ -26,66 +15,6 @@ const ARG_DEV = "dev"; const ARG_FORCE = "force"; const ARG_FORCE_ALIAS = "f"; -/** - * Resolve database credentials — same pattern as shell.ts. - */ -async function resolveCredentials( - urlArg: string | undefined, - tokenArg: string | undefined, - databaseIdArg: string | undefined, - profile: string, - apiKeyOverride?: string, - verbose = false, -): Promise<{ url: string; token: string; databaseId: string | undefined }> { - let url = urlArg ?? readEnvValue(ENV_DATABASE_URL)?.value; - let token = tokenArg ?? readEnvValue(ENV_DATABASE_AUTH_TOKEN)?.value; - - if (url && token) return { url, token, databaseId: databaseIdArg }; - - const config = resolveConfig(profile, apiKeyOverride, verbose); - const apiClient = createDbClient(clientOptions(config, verbose)); - - const { id: databaseId } = await resolveDbId(apiClient, databaseIdArg); - - const spin = spinner("Connecting..."); - spin.start(); - - const fetches: Promise[] = []; - - if (!url) { - fetches.push( - apiClient.GET("/v2/databases/{db_id}", { - params: { path: { db_id: databaseId } }, - }), - ); - } else { - fetches.push(Promise.resolve(null)); - } - - if (!token) { - spin.text = "Generating token..."; - fetches.push( - generateToken(apiClient, databaseId, { - authorization: "full-access", - expiresAt: tokenExpiryFromNow(), - }), - ); - } - - const [dbResult, tokenResult] = await Promise.all(fetches); - - spin.stop(); - - if (!url && dbResult) url = dbResult.data?.db?.url; - if (!token && tokenResult) token = tokenResult.token; - - if (!url || !token) { - throw new UserError("Could not resolve database URL or generate token."); - } - - return { url, token, databaseId }; -} - export const dbStudioCommand = defineCommand<{ [ARG_DATABASE_ID]?: string; [ARG_PORT]?: number; @@ -174,14 +103,14 @@ export const dbStudioCommand = defineCommand<{ const { createClient } = await import("@libsql/client/web"); const { startStudio } = await import("@bunny.net/database-studio"); - const { url, token } = await resolveCredentials( - urlArg, - tokenArg, - databaseIdArg, + const { url, token } = await resolveCredentials({ + url: urlArg, + token: tokenArg, + databaseId: databaseIdArg, profile, apiKey, verbose, - ); + }); const client = createClient({ url, authToken: token }); diff --git a/packages/database-shell/src/parser.ts b/packages/database-shell/src/parser.ts index 2d4644fd..426db058 100644 --- a/packages/database-shell/src/parser.ts +++ b/packages/database-shell/src/parser.ts @@ -1,6 +1,19 @@ +/** Statements whose body is a `BEGIN ... END` block, so inner semicolons don't terminate them. */ +const BLOCK_BODY_START = /^CREATE\s+(?:TEMP\s+|TEMPORARY\s+)?TRIGGER\b/i; + +/** True when `current` opens a `BEGIN ... END` block that hasn't been closed yet. */ +function inBlockBody(current: string): boolean { + const trimmed = current.trim(); + if (!BLOCK_BODY_START.test(trimmed)) return false; + return !/\bEND$/i.test(trimmed); +} + /** * Split a SQL string into individual statements, handling single-quoted strings * and `--` line comments. Trims whitespace and filters empty results. + * + * `CREATE TRIGGER` bodies are kept intact: semicolons inside `BEGIN ... END` + * don't split the statement. */ export function splitStatements(sql: string): string[] { const statements: string[] = []; @@ -37,6 +50,10 @@ export function splitStatements(sql: string): string[] { } if (ch === ";" && !inString) { + if (inBlockBody(current)) { + current += ch; + continue; + } const trimmed = current.trim(); if (trimmed.length > 0) statements.push(trimmed); current = ""; diff --git a/packages/database-shell/src/shell.test.ts b/packages/database-shell/src/shell.test.ts index 8d812772..d675a4da 100644 --- a/packages/database-shell/src/shell.test.ts +++ b/packages/database-shell/src/shell.test.ts @@ -571,6 +571,32 @@ describe("splitStatements", () => { const sql = "-- this; is a comment\nSELECT 1;"; expect(splitStatements(sql)).toEqual(["SELECT 1"]); }); + + test("keeps a CREATE TRIGGER body intact", () => { + const sql = + "CREATE TRIGGER touch AFTER UPDATE ON users BEGIN\n UPDATE users SET updated_at = CURRENT_TIMESTAMP WHERE id = NEW.id;\nEND;"; + expect(splitStatements(sql)).toEqual([ + "CREATE TRIGGER touch AFTER UPDATE ON users BEGIN\n UPDATE users SET updated_at = CURRENT_TIMESTAMP WHERE id = NEW.id;\nEND", + ]); + }); + + test("keeps a multi-statement trigger body intact and splits what follows", () => { + const sql = + "CREATE TEMPORARY TRIGGER log AFTER INSERT ON t BEGIN\n INSERT INTO audit VALUES (1);\n INSERT INTO audit VALUES (2);\nEND;\nSELECT 1;"; + expect(splitStatements(sql)).toEqual([ + "CREATE TEMPORARY TRIGGER log AFTER INSERT ON t BEGIN\n INSERT INTO audit VALUES (1);\n INSERT INTO audit VALUES (2);\nEND", + "SELECT 1", + ]); + }); + + test("splits drizzle statement-breakpoint files", () => { + const sql = + "CREATE TABLE `users` (\n\t`id` integer PRIMARY KEY NOT NULL\n);\n--> statement-breakpoint\nCREATE UNIQUE INDEX `users_id` ON `users` (`id`);"; + expect(splitStatements(sql)).toEqual([ + "CREATE TABLE `users` (\n\t`id` integer PRIMARY KEY NOT NULL\n)", + "CREATE UNIQUE INDEX `users_id` ON `users` (`id`)", + ]); + }); }); describe("views", () => { diff --git a/skills/bunny-cli/SKILL.md b/skills/bunny-cli/SKILL.md index a5483b06..ab7d0b49 100644 --- a/skills/bunny-cli/SKILL.md +++ b/skills/bunny-cli/SKILL.md @@ -34,6 +34,7 @@ bunny api GET /user bunny db create bunny db list bunny db shell +bunny db migrations apply # run pending migrations/*.sql files # manage Edge Scripts bunny scripts init @@ -63,7 +64,7 @@ bunny sites deployments publish --previous --force # instant rollback Use this to route to the correct reference file: - **Authenticate or switch profiles** -> `references/auth.md` -- **Database management (create, list, show, link, delete, shell, studio, regions, tokens)** -> `references/database.md` +- **Database management (create, list, show, link, delete, shell, studio, migrations, regions, tokens)** -> `references/database.md` - **DNS (zones, delegation checks, records, presets, BIND import/export, DNSSEC, logging, Scriptable DNS scripts)** -> `references/dns.md` - **Edge Scripts (init, create, deploy, link, stats, deployments/rollback, env vars, custom domains)** -> `references/scripts.md` - **Static sites (create, deploy, rollback, previews, custom domains)** -> `references/sites.md` diff --git a/skills/bunny-cli/references/database.md b/skills/bunny-cli/references/database.md index be9bc6e9..9ed23b5d 100644 --- a/skills/bunny-cli/references/database.md +++ b/skills/bunny-cli/references/database.md @@ -204,6 +204,61 @@ Spins up a local server, generates a short-lived auth token if needed, and opens --- +## `bunny db migrations` — Create and apply SQL migrations + +Migrations are plain `.sql` files in `migrations/`, named `NNNN_.sql`. The filename is the migration's identity and its numeric prefix is the apply order. Applied migrations are recorded in a `__bunny_migrations` table in the database. There is no rollback: fix a bad migration with another migration. + +```bash +bunny db migrations create add_users_table # writes migrations/0001_add_users_table.sql +bunny db migrations list # applied / pending / modified / missing +bunny db migrations apply --dry-run # show what would run +bunny db migrations apply # apply pending migrations in order +bunny db migrations apply --dir drizzle # apply drizzle-kit generate output +``` + +### `bunny db migrations create ` (alias: `new`) + +| Flag | Default | Description | +| ------- | ------------ | -------------------- | +| `--dir` | `migrations` | Migrations directory | + +Numbers the file one above the highest existing prefix and slugifies the name. Creates the directory if needed. + +### `bunny db migrations list` (aliases: `ls`, `status`) + +| Flag | Default | Description | +| --------- | ------------ | ----------------------------------- | +| `--dir` | `migrations` | Migrations directory | +| `--url` | | Database URL (skips API lookup) | +| `--token` | | Auth token (skips token generation) | + +Never creates the tracking table, so it is safe against a database that has never had a migration applied. States are `Applied`, `Pending`, `Modified` (the file changed after being applied), and `Missing` (the file was deleted). Modified and missing are warnings, not errors. + +### `bunny db migrations apply` + +| Flag | Short | Default | Description | +| ----------- | ----- | ------------ | ------------------------------------ | +| `--dir` | | `migrations` | Migrations directory | +| `--dry-run` | | `false` | List what would run without applying | +| `--force` | `-f` | `false` | Skip the confirmation prompt | +| `--url` | | | Database URL (skips API lookup) | +| `--token` | | | Auth token (skips token generation) | + +Each file runs as one atomic batch together with its tracking row, so a migration either lands and is recorded or neither happens. Foreign keys are deferred for the duration, so table rebuilds and `ALTER TABLE` work. The run stops at the first failure and leaves the rest pending. + +Confirms before writing when a TTY is attached; the prompt is skipped under `--force`, `--output json`, or any non-interactive run, so CI and agent flows aren't blocked. Credential resolution mirrors `db shell`. + +### ORM-generated migrations + +`drizzle-kit generate` writes flat `0000_.sql` files, which match this convention. When `migrations/` doesn't exist, `drizzle/` is used automatically. Generate with the ORM, apply with the CLI: + +```bash +drizzle-kit generate +bunny db migrations apply +``` + +--- + ## `bunny db quickstart` — Language-specific getting-started guide ```bash From e114a6d4b9207a07ee149ab1ff09988ce2f9d751 Mon Sep 17 00:00:00 2001 From: jamie-at-bunny Date: Tue, 28 Jul 2026 12:55:49 +0100 Subject: [PATCH 2/5] fix(db): address review feedback on migrations Parser: a trigger body statement ending in `CASE ... END;` was mistaken for the trigger's own terminator, shredding a valid trigger into three fragments. Nesting is now counted across `BEGIN` and `CASE` openers, with quoted strings and identifiers scrubbed first so a column named `end` doesn't skew the count. Credentials: an explicit database ID no longer falls through to `.env`, which could target a different database than the one named on the command line. A generated token is now bound to the resolved database's host, so `--url` without `--token` is refused on mismatch instead of sending a full-access token to an unverified endpoint, and the check runs before the token is created. Apply: `ensureMigrationsTable()` moved after the dry-run exit and the confirmation, so a preview or a declined run writes nothing. The failed migration is now counted as still pending, since its tracking row rolls back with it. Also wraps the pre-confirmation read in `readApplied()`, turning a bad URL or token into a hinted error instead of an unexpected-error exit. --- .changeset/db-migrations.md | 2 +- AGENTS.md | 7 +- .../cli/src/commands/db/credentials.test.ts | 46 ++++++++++ packages/cli/src/commands/db/credentials.ts | 90 ++++++++++++++----- .../cli/src/commands/db/migrations/apply.ts | 15 ++-- .../src/commands/db/migrations/engine.test.ts | 22 +++++ .../cli/src/commands/db/migrations/engine.ts | 25 +++++- .../cli/src/commands/db/migrations/list.ts | 8 +- packages/database-shell/src/parser.ts | 19 +++- packages/database-shell/src/shell.test.ts | 21 +++++ skills/bunny-cli/references/database.md | 5 ++ 11 files changed, 218 insertions(+), 42 deletions(-) create mode 100644 packages/cli/src/commands/db/credentials.test.ts diff --git a/.changeset/db-migrations.md b/.changeset/db-migrations.md index 0d8f6f67..a8d3648d 100644 --- a/.changeset/db-migrations.md +++ b/.changeset/db-migrations.md @@ -3,4 +3,4 @@ "@bunny.net/database-shell": patch --- -feat(db): `bunny db migrations create/list/apply` runs numbered `.sql` files in `migrations/` (or `drizzle/`) once each, tracked in `__bunny_migrations`; `splitStatements` now keeps `CREATE TRIGGER` bodies intact +feat(db): `bunny db migrations create/list/apply` runs numbered `.sql` files in `migrations/` (or `drizzle/`) once each, tracked in `__bunny_migrations`; `splitStatements` keeps `CREATE TRIGGER` bodies intact; `db shell`, `db studio`, and `db migrations apply` now honour an explicit database ID over `.env` credentials and refuse to send a generated token to a `--url` on a different host diff --git a/AGENTS.md b/AGENTS.md index 2fb35484..bc33de35 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1472,7 +1472,7 @@ interface ShellLogger { **CLI wrapper** (`packages/cli/src/commands/db/shell.ts`) provides: -- Credential resolution via `resolveCredentials()` in `packages/cli/src/commands/db/credentials.ts` (--url/--token flags → .env → API lookup), shared with `db studio` and `db migrations apply` +- Credential resolution via `resolveCredentials()` in `packages/cli/src/commands/db/credentials.ts` (--url/--token flags → .env → API lookup), shared with `db studio` and `db migrations apply`. Two safety rules live there: an explicit database ID skips `.env` entirely (it may describe a different database, and silently connecting there would target the wrong one), and a generated token is only sent to a URL whose host matches that database's canonical URL, so `--url` without `--token` is rejected on mismatch rather than handing a full-access token to an unverified host. - `shellLogger()` adapter that wraps the CLI `logger` - `createClient()` call and delegation to `startShell()`/`executeQuery()`/`executeFile()` - Passes resolved `databaseId` and optional `--views-dir` to `startShell()` for saved views @@ -1519,6 +1519,7 @@ All file and state logic is here so the commands stay thin and the logic is test - `checksum(sql)` — sha256 of the body with CRLF normalized and edges trimmed, so reformatting line endings isn't reported as a change. - `migrationStatuses(files, applied)` / `pendingMigrations(files, applied)` — join disk against the table. A recorded migration whose file changed is `modified`; one whose file is gone is `missing`. Both are warnings (`drift.ts`), never fatal: pending migrations still apply cleanly, and the remedy is the developer's call. - `applyMigration(client, file)` — splits the file with `splitStatements()` and runs the statements plus the tracking-row insert through `client.migrate()`, so a migration either lands and is recorded or neither happens. +- `readApplied(client)` — the read path for `list` and for `apply` before confirmation. Checks `sqlite_master` rather than creating the tracking table, so a preview never writes, and converts connection or query failures into a hinted `UserError` instead of an unexpected-error exit. `client.migrate()` is used rather than `client.batch()` because it defers foreign key enforcement for the batch, which table rebuilds and `ALTER TABLE` need. `db shell .sql` still uses `batch()` and is not migration-aware. @@ -1536,7 +1537,9 @@ bunny db migrations apply --dir drizzle # or be explicit ### Applying -`apply` runs pending migrations sequentially and stops at the first failure, reporting how many applied and how many are still pending. It confirms before writing when a TTY is attached, and skips the prompt under `--force` or any non-interactive run (`--output json`, no TTY) so CI and agents aren't blocked. `--dry-run` lists what would run without writing. +`apply` runs pending migrations sequentially and stops at the first failure, reporting how many applied and how many are still pending (the failed file counts as pending, since its tracking row rolled back with it). It confirms before writing when a TTY is attached, and skips the prompt under `--force` or any non-interactive run (`--output json`, no TTY) so CI and agents aren't blocked. `--dry-run` lists what would run without writing. + +Nothing is written before confirmation, including the tracking table: `ensureMigrationsTable()` runs only after the confirm and after the `--dry-run` exit, so a preview against read-only credentials lists pending files instead of failing on a schema write. ```bash bunny db migrations create add_users_table # migrations/0001_add_users_table.sql diff --git a/packages/cli/src/commands/db/credentials.test.ts b/packages/cli/src/commands/db/credentials.test.ts new file mode 100644 index 00000000..c862be61 --- /dev/null +++ b/packages/cli/src/commands/db/credentials.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, test } from "bun:test"; +import { sameHost } from "./credentials.ts"; + +const CANONICAL = "libsql://my-db-abc.lite.bunnydb.net/"; + +describe("sameHost", () => { + test("accepts the canonical URL with or without a trailing slash", () => { + expect(sameHost("libsql://my-db-abc.lite.bunnydb.net", CANONICAL)).toBe( + true, + ); + expect(sameHost(CANONICAL, CANONICAL)).toBe(true); + }); + + test("accepts https for the same host, since libsql maps onto it", () => { + expect(sameHost("https://my-db-abc.lite.bunnydb.net", CANONICAL)).toBe( + true, + ); + }); + + test("ignores host casing and path", () => { + expect( + sameHost("libsql://MY-DB-ABC.lite.bunnydb.net/anything", CANONICAL), + ).toBe(true); + }); + + test("rejects a different database on the same domain", () => { + expect(sameHost("libsql://other-db-xyz.lite.bunnydb.net", CANONICAL)).toBe( + false, + ); + }); + + test("rejects a foreign host", () => { + expect(sameHost("libsql://evil.example.com", CANONICAL)).toBe(false); + }); + + test("rejects a host that only prefixes the canonical one", () => { + expect( + sameHost("libsql://my-db-abc.lite.bunnydb.net.example.com", CANONICAL), + ).toBe(false); + }); + + test("rejects unparseable input rather than treating it as a match", () => { + expect(sameHost("my-db-abc.lite.bunnydb.net", CANONICAL)).toBe(false); + expect(sameHost("", CANONICAL)).toBe(false); + }); +}); diff --git a/packages/cli/src/commands/db/credentials.ts b/packages/cli/src/commands/db/credentials.ts index 05a9f3a2..de9c93ff 100644 --- a/packages/cli/src/commands/db/credentials.ts +++ b/packages/cli/src/commands/db/credentials.ts @@ -25,6 +25,17 @@ export interface ResolveCredentialsOptions { verbose?: boolean; } +/** Same host, ignoring scheme, port, and path, since `libsql://` and `https://` address the same endpoint. */ +export function sameHost(a: string, b: string): boolean { + try { + return ( + new URL(a).hostname.toLowerCase() === new URL(b).hostname.toLowerCase() + ); + } catch { + return false; + } +} + /** * Resolve the database URL and auth token needed to connect over libSQL. * @@ -33,13 +44,24 @@ export interface ResolveCredentialsOptions { * 2. `BUNNY_DATABASE_URL` / `BUNNY_DATABASE_AUTH_TOKEN` from `.env` * 3. API lookup (fetches the URL and/or creates a short-lived token on the fly) * + * An explicit database ID skips step 2 entirely: `.env` may describe a different + * database, and silently connecting there would target the wrong database. + * + * A generated token is only ever sent to a URL that belongs to the database it + * was created for, so overriding `--url` without `--token` is rejected rather + * than handing a full-access token to an unverified host. + * * Shared by `db shell`, `db studio`, and `db migrations apply`. */ export async function resolveCredentials( opts: ResolveCredentialsOptions, ): Promise { - let url = opts.url ?? readEnvValue(ENV_DATABASE_URL)?.value; - let token = opts.token ?? readEnvValue(ENV_DATABASE_AUTH_TOKEN)?.value; + const useEnv = !opts.databaseId; + let url = + opts.url ?? (useEnv ? readEnvValue(ENV_DATABASE_URL)?.value : undefined); + let token = + opts.token ?? + (useEnv ? readEnvValue(ENV_DATABASE_AUTH_TOKEN)?.value : undefined); if (url && token) { return { @@ -60,27 +82,49 @@ export async function resolveCredentials( const willGenerateToken = !token; - const dbFetch = url - ? Promise.resolve(null) - : apiClient.GET("/v2/databases/{db_id}", { - params: { path: { db_id: databaseId } }, - }); - - if (willGenerateToken) spin.text = "Generating token..."; - - const tokenFetch = willGenerateToken - ? generateToken(apiClient, databaseId, { - authorization: "full-access", - expiresAt: tokenExpiryFromNow(), - }) - : Promise.resolve(null); - - const [dbResult, tokenResult] = await Promise.all([dbFetch, tokenFetch]); - - spin.stop(); - - if (!url && dbResult) url = dbResult.data?.db?.url; - if (willGenerateToken && tokenResult) token = tokenResult.token; + const fetchDatabase = () => + apiClient.GET("/v2/databases/{db_id}", { + params: { path: { db_id: databaseId } }, + }); + + const mintToken = () => { + spin.text = "Generating token..."; + return generateToken(apiClient, databaseId, { + authorization: "full-access", + expiresAt: tokenExpiryFromNow(), + }); + }; + + try { + if (url && willGenerateToken) { + // Verify the override before creating a token, so a token is never created for a host we'd refuse. + const { data } = await fetchDatabase(); + const canonical = data?.db?.url; + + if (!canonical) { + throw new UserError(`Could not fetch database ${databaseId}.`); + } + + if (!sameHost(url, canonical)) { + throw new UserError( + `--url does not point at ${databaseId}.`, + `Pass --token for that URL, or drop --url to connect to ${canonical}.`, + ); + } + + token = (await mintToken())?.token; + } else { + const [dbResult, tokenResult] = await Promise.all([ + url ? Promise.resolve(null) : fetchDatabase(), + willGenerateToken ? mintToken() : Promise.resolve(null), + ]); + + if (!url) url = dbResult?.data?.db?.url; + if (willGenerateToken) token = tokenResult?.token; + } + } finally { + spin.stop(); + } if (!url || !token) { throw new UserError("Could not resolve database URL or generate token."); diff --git a/packages/cli/src/commands/db/migrations/apply.ts b/packages/cli/src/commands/db/migrations/apply.ts index 569fdd2a..1fd86b17 100644 --- a/packages/cli/src/commands/db/migrations/apply.ts +++ b/packages/cli/src/commands/db/migrations/apply.ts @@ -11,9 +11,9 @@ import { applyMigration, discoverMigrations, ensureMigrationsTable, - fetchApplied, migrationStatuses, pendingMigrations, + readApplied, resolveMigrationsDir, } from "./engine.ts"; @@ -134,8 +134,8 @@ export const dbMigrationsApplyCommand = defineCommand({ const { createClient } = await import("@libsql/client/web"); const client = createClient({ url, authToken: token }); - await ensureMigrationsTable(client); - const applied = await fetchApplied(client); + // Read without creating the table, so --dry-run and a declined confirm leave the database untouched. + const applied = await readApplied(client); const statuses = migrationStatuses(files, applied); const pending = pendingMigrations(files, applied); @@ -193,6 +193,8 @@ export const dbMigrationsApplyCommand = defineCommand({ return; } + await ensureMigrationsTable(client); + const done: string[] = []; for (const file of pending) { @@ -210,12 +212,11 @@ export const dbMigrationsApplyCommand = defineCommand({ } } catch (err: unknown) { spin.stop(); - const remaining = pending.length - done.length - 1; + // The failed file rolled back, so it is still pending along with everything unattempted. + const remaining = pending.length - done.length; throw new UserError( `${file.name} failed: ${errorMessage(err)}`, - done.length > 0 || remaining > 0 - ? `${done.length} applied, ${remaining} still pending. Fix ${file.name} and re-run.` - : undefined, + `${done.length} applied, ${remaining} still pending. Fix ${file.name} and re-run.`, ); } } diff --git a/packages/cli/src/commands/db/migrations/engine.test.ts b/packages/cli/src/commands/db/migrations/engine.test.ts index b2c9e24a..ad76a42e 100644 --- a/packages/cli/src/commands/db/migrations/engine.test.ts +++ b/packages/cli/src/commands/db/migrations/engine.test.ts @@ -20,6 +20,7 @@ import { migrationsTableExists, nextSequence, pendingMigrations, + readApplied, resolveMigrationsDir, slugify, } from "./engine.ts"; @@ -356,6 +357,27 @@ describe("migrationsTableExists", () => { }); }); +describe("readApplied", () => { + test("returns empty without creating the table", async () => { + const client = memoryClient(); + expect(await readApplied(client)).toEqual([]); + expect(await migrationsTableExists(client)).toBe(false); + }); + + test("turns a connection failure into a hinted UserError", async () => { + const broken = { + execute: async () => { + throw new Error("SERVER_ERROR: Server returned HTTP status 404"); + }, + migrate: async () => [], + } as unknown as MigrationClient; + + await expect(readApplied(broken)).rejects.toThrow( + /Could not read migration state: SERVER_ERROR/, + ); + }); +}); + describe("ensureMigrationsTable", () => { test("is idempotent and preserves rows", async () => { const client = memoryClient(); diff --git a/packages/cli/src/commands/db/migrations/engine.ts b/packages/cli/src/commands/db/migrations/engine.ts index 0c9e4b14..a1b32189 100644 --- a/packages/cli/src/commands/db/migrations/engine.ts +++ b/packages/cli/src/commands/db/migrations/engine.ts @@ -3,7 +3,7 @@ import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; import { join, resolve } from "node:path"; import { splitStatements } from "@bunny.net/database-shell"; import type { Client } from "@libsql/client"; -import { UserError } from "../../../core/errors.ts"; +import { errorMessage, UserError } from "../../../core/errors.ts"; import { DEFAULT_MIGRATIONS_DIR, FALLBACK_MIGRATIONS_DIRS, @@ -181,6 +181,29 @@ export async function fetchApplied( })); } +/** + * Read the applied migrations without creating the tracking table. + * + * Used by the read paths (`list`, and `apply` before it has confirmation) so a + * preview never writes. Connection and query failures become `UserError`, since + * a bad URL or token is a user problem, not a crash. + */ +export async function readApplied( + client: MigrationClient, + table = MIGRATIONS_TABLE, +): Promise { + try { + return (await migrationsTableExists(client, table)) + ? await fetchApplied(client, table) + : []; + } catch (err: unknown) { + throw new UserError( + `Could not read migration state: ${errorMessage(err)}`, + "Check that the database URL and token are correct.", + ); + } +} + /** * Join the files on disk with what the database has recorded. * diff --git a/packages/cli/src/commands/db/migrations/list.ts b/packages/cli/src/commands/db/migrations/list.ts index 15bbad48..81bae4e3 100644 --- a/packages/cli/src/commands/db/migrations/list.ts +++ b/packages/cli/src/commands/db/migrations/list.ts @@ -7,11 +7,9 @@ import { resolveCredentials } from "../credentials.ts"; import { ARG_DIR, MIGRATIONS_TABLE } from "./constants.ts"; import { warnOnDrift } from "./drift.ts"; import { - type AppliedMigration, discoverMigrations, - fetchApplied, migrationStatuses, - migrationsTableExists, + readApplied, resolveMigrationsDir, } from "./engine.ts"; @@ -105,9 +103,7 @@ export const dbMigrationsListCommand = defineCommand({ const client = createClient({ url, authToken: token }); // Don't create the tracking table from a read-only command. - const applied: AppliedMigration[] = (await migrationsTableExists(client)) - ? await fetchApplied(client) - : []; + const applied = await readApplied(client); const statuses = migrationStatuses(files, applied); diff --git a/packages/database-shell/src/parser.ts b/packages/database-shell/src/parser.ts index 426db058..66c70506 100644 --- a/packages/database-shell/src/parser.ts +++ b/packages/database-shell/src/parser.ts @@ -1,11 +1,26 @@ /** Statements whose body is a `BEGIN ... END` block, so inner semicolons don't terminate them. */ const BLOCK_BODY_START = /^CREATE\s+(?:TEMP\s+|TEMPORARY\s+)?TRIGGER\b/i; -/** True when `current` opens a `BEGIN ... END` block that hasn't been closed yet. */ +/** Quoted strings and identifiers, so keywords inside them don't affect nesting. */ +const QUOTED = /'(?:[^']|'')*'|"(?:[^"]|"")*"|`[^`]*`|\[[^\]]*\]/g; + +/** + * True when `current` opens a `BEGIN ... END` block that hasn't been closed yet. + * + * `BEGIN` opens the trigger body and `CASE` opens an expression; both are closed + * by `END`, so the body ends only once every opener has been matched. Counting + * rather than checking for a trailing `END` is what keeps a body statement like + * `SET x = CASE ... END;` from being mistaken for the end of the trigger. + */ function inBlockBody(current: string): boolean { const trimmed = current.trim(); if (!BLOCK_BODY_START.test(trimmed)) return false; - return !/\bEND$/i.test(trimmed); + + const bare = trimmed.replace(QUOTED, ""); + const openers = (bare.match(/\b(?:BEGIN|CASE)\b/gi) ?? []).length; + const closers = (bare.match(/\bEND\b/gi) ?? []).length; + + return closers < openers; } /** diff --git a/packages/database-shell/src/shell.test.ts b/packages/database-shell/src/shell.test.ts index d675a4da..0ecb4978 100644 --- a/packages/database-shell/src/shell.test.ts +++ b/packages/database-shell/src/shell.test.ts @@ -589,6 +589,27 @@ describe("splitStatements", () => { ]); }); + test("keeps a trigger body whose statement ends in CASE ... END intact", () => { + const sql = + "CREATE TRIGGER grade AFTER UPDATE ON scores BEGIN\n UPDATE scores SET band = CASE WHEN NEW.v > 90 THEN 'a' ELSE 'b' END;\n UPDATE scores SET seen = 1;\nEND;"; + expect(splitStatements(sql)).toEqual([sql.slice(0, -1)]); + }); + + test("handles nested CASE expressions in a trigger body", () => { + const sql = + "CREATE TRIGGER t AFTER INSERT ON x BEGIN\n UPDATE x SET a = CASE WHEN b THEN CASE WHEN c THEN 1 ELSE 2 END ELSE 3 END;\nEND;\nSELECT 1;"; + expect(splitStatements(sql)).toEqual([ + "CREATE TRIGGER t AFTER INSERT ON x BEGIN\n UPDATE x SET a = CASE WHEN b THEN CASE WHEN c THEN 1 ELSE 2 END ELSE 3 END;\nEND", + "SELECT 1", + ]); + }); + + test("ignores block keywords inside strings and quoted identifiers", () => { + const sql = + "CREATE TRIGGER t AFTER INSERT ON x BEGIN\n INSERT INTO log (\"end\") VALUES ('CASE END');\nEND;"; + expect(splitStatements(sql)).toEqual([sql.slice(0, -1)]); + }); + test("splits drizzle statement-breakpoint files", () => { const sql = "CREATE TABLE `users` (\n\t`id` integer PRIMARY KEY NOT NULL\n);\n--> statement-breakpoint\nCREATE UNIQUE INDEX `users_id` ON `users` (`id`);"; diff --git a/skills/bunny-cli/references/database.md b/skills/bunny-cli/references/database.md index 9ed23b5d..e5a84f06 100644 --- a/skills/bunny-cli/references/database.md +++ b/skills/bunny-cli/references/database.md @@ -177,6 +177,11 @@ bunny db shell --url libsql://... --token ey... # explicit credentials 2. `BUNNY_DATABASE_URL` / `BUNNY_DATABASE_AUTH_TOKEN` from `.env` 3. API lookup (fetches URL and generates a temporary token) +Shared by `db shell`, `db studio`, and `db migrations apply`. Two rules apply: + +- **Passing a database ID skips `.env`.** `bunny db shell db_01ABC` targets that database even when `.env` describes another one, so an explicit target is never silently redirected. +- **`--url` without `--token` must match the resolved database.** A generated token is only sent to that database's own host; a mismatch errors and asks for `--token`. + ### REPL dot-commands In interactive mode, the shell supports dot-commands like `.tables`, `.schema`, `.fk`, etc. From 68ed0f1c13049c40019fde3d76500c2dde78d853 Mon Sep 17 00:00:00 2001 From: jamie-at-bunny Date: Tue, 28 Jul 2026 13:49:00 +0100 Subject: [PATCH 3/5] fix(db): handle block comments and plaintext token targets Parser: block comments were not tokenized at all, so `/* END */` inside a trigger body counted as a structural closer, and more broadly a `;` or a quote inside any block comment split or corrupted the statement around it. Block comments are now skipped like `--` comments, which fixes both. Credentials: a hostname match let a plaintext URL receive a token. A token the user did not pass on the command line now requires an encrypted target, rejecting `http:`, `ws:`, and `libsql://host:port?tls=0`, which the libSQL client downgrades to plaintext. This covers the token read from `.env` as well as a generated one, since neither was paired with the URL by the user. The scheme check runs before any lookup or prompt, so an unusable URL fails immediately rather than after picking a database. An explicit `--token` alongside a plaintext `--url` is still allowed: that pairing is deliberate, and it covers a local sqld over http. --- .changeset/db-migrations.md | 2 +- AGENTS.md | 6 ++- .../cli/src/commands/db/credentials.test.ts | 28 +++++++++- packages/cli/src/commands/db/credentials.ts | 53 ++++++++++++++++--- packages/database-shell/src/parser.ts | 12 ++++- packages/database-shell/src/shell.test.ts | 27 ++++++++++ skills/bunny-cli/references/database.md | 1 + 7 files changed, 119 insertions(+), 10 deletions(-) diff --git a/.changeset/db-migrations.md b/.changeset/db-migrations.md index a8d3648d..2b61dbc5 100644 --- a/.changeset/db-migrations.md +++ b/.changeset/db-migrations.md @@ -3,4 +3,4 @@ "@bunny.net/database-shell": patch --- -feat(db): `bunny db migrations create/list/apply` runs numbered `.sql` files in `migrations/` (or `drizzle/`) once each, tracked in `__bunny_migrations`; `splitStatements` keeps `CREATE TRIGGER` bodies intact; `db shell`, `db studio`, and `db migrations apply` now honour an explicit database ID over `.env` credentials and refuse to send a generated token to a `--url` on a different host +feat(db): `bunny db migrations create/list/apply` runs numbered `.sql` files in `migrations/` (or `drizzle/`) once each, tracked in `__bunny_migrations`; `splitStatements` keeps `CREATE TRIGGER` bodies intact and drops block comments; `db shell`, `db studio`, and `db migrations apply` now honour an explicit database ID over `.env` credentials and refuse to send a token to a `--url` on a different host or over an unencrypted connection diff --git a/AGENTS.md b/AGENTS.md index bc33de35..380ebca9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1472,7 +1472,11 @@ interface ShellLogger { **CLI wrapper** (`packages/cli/src/commands/db/shell.ts`) provides: -- Credential resolution via `resolveCredentials()` in `packages/cli/src/commands/db/credentials.ts` (--url/--token flags → .env → API lookup), shared with `db studio` and `db migrations apply`. Two safety rules live there: an explicit database ID skips `.env` entirely (it may describe a different database, and silently connecting there would target the wrong one), and a generated token is only sent to a URL whose host matches that database's canonical URL, so `--url` without `--token` is rejected on mismatch rather than handing a full-access token to an unverified host. +- Credential resolution via `resolveCredentials()` in `packages/cli/src/commands/db/credentials.ts` (--url/--token flags → .env → API lookup), shared with `db studio` and `db migrations apply`. Its job is to never pair a credential with a target the user didn't pair it with: + +- An explicit database ID skips `.env` entirely. `.env` may describe a different database, and silently connecting there would target the wrong one. +- A generated token is only sent to a URL whose host matches that database's canonical URL, so `--url` without `--token` is rejected on mismatch. The host check runs before the token is created, so nothing is minted for an endpoint we'd refuse. +- A token bound for an explicit `--url` must travel encrypted unless the user passed it as `--token` on the same command line. `isEncrypted()` allows `libsql:`, `https:`, and `wss:`, and rejects `libsql://host:port?tls=0`, which the libSQL client downgrades to plaintext. The scheme check runs first of all, before any lookup or prompt. An explicit `--token` with a plaintext `--url` is left alone: that pairing is deliberate, and it covers a local `sqld` over http. - `shellLogger()` adapter that wraps the CLI `logger` - `createClient()` call and delegation to `startShell()`/`executeQuery()`/`executeFile()` - Passes resolved `databaseId` and optional `--views-dir` to `startShell()` for saved views diff --git a/packages/cli/src/commands/db/credentials.test.ts b/packages/cli/src/commands/db/credentials.test.ts index c862be61..69cf39a0 100644 --- a/packages/cli/src/commands/db/credentials.test.ts +++ b/packages/cli/src/commands/db/credentials.test.ts @@ -1,8 +1,34 @@ import { describe, expect, test } from "bun:test"; -import { sameHost } from "./credentials.ts"; +import { isEncrypted, sameHost } from "./credentials.ts"; const CANONICAL = "libsql://my-db-abc.lite.bunnydb.net/"; +describe("isEncrypted", () => { + test("accepts libsql, https, and wss", () => { + expect(isEncrypted("libsql://h.lite.bunnydb.net")).toBe(true); + expect(isEncrypted("https://h.lite.bunnydb.net")).toBe(true); + expect(isEncrypted("wss://h.lite.bunnydb.net")).toBe(true); + }); + + test("rejects plaintext schemes", () => { + expect(isEncrypted("http://h.lite.bunnydb.net")).toBe(false); + expect(isEncrypted("ws://h.lite.bunnydb.net")).toBe(false); + }); + + test("rejects libsql that opts out of TLS, which downgrades to http", () => { + expect(isEncrypted("libsql://h.lite.bunnydb.net:8080?tls=0")).toBe(false); + }); + + test("still accepts libsql with tls left on", () => { + expect(isEncrypted("libsql://h.lite.bunnydb.net:8080?tls=1")).toBe(true); + }); + + test("rejects unparseable input", () => { + expect(isEncrypted("h.lite.bunnydb.net")).toBe(false); + expect(isEncrypted("")).toBe(false); + }); +}); + describe("sameHost", () => { test("accepts the canonical URL with or without a trailing slash", () => { expect(sameHost("libsql://my-db-abc.lite.bunnydb.net", CANONICAL)).toBe( diff --git a/packages/cli/src/commands/db/credentials.ts b/packages/cli/src/commands/db/credentials.ts index de9c93ff..f1efc96e 100644 --- a/packages/cli/src/commands/db/credentials.ts +++ b/packages/cli/src/commands/db/credentials.ts @@ -25,6 +25,25 @@ export interface ResolveCredentialsOptions { verbose?: boolean; } +/** Schemes that encrypt in transit. `libsql:` resolves to `https:`/`wss:` unless it opts out with `?tls=0`. */ +const ENCRYPTED_SCHEMES = new Set(["libsql:", "https:", "wss:"]); + +/** + * True when traffic to this URL is encrypted, so a token we create can be sent to it. + * + * The scheme alone isn't enough: `libsql://host:port?tls=0` downgrades to + * plaintext `http:`/`ws:` inside the libSQL client. + */ +export function isEncrypted(url: string): boolean { + try { + const parsed = new URL(url); + if (!ENCRYPTED_SCHEMES.has(parsed.protocol)) return false; + return parsed.searchParams.get("tls") !== "0"; + } catch { + return false; + } +} + /** Same host, ignoring scheme, port, and path, since `libsql://` and `https://` address the same endpoint. */ export function sameHost(a: string, b: string): boolean { try { @@ -47,9 +66,13 @@ export function sameHost(a: string, b: string): boolean { * An explicit database ID skips step 2 entirely: `.env` may describe a different * database, and silently connecting there would target the wrong database. * - * A generated token is only ever sent to a URL that belongs to the database it - * was created for, so overriding `--url` without `--token` is rejected rather - * than handing a full-access token to an unverified host. + * A generated token is only ever sent to an encrypted URL that belongs to the + * database it was created for, so overriding `--url` without `--token` is + * rejected rather than handing a full-access token to an unverified or + * plaintext endpoint. A token read from `.env` is likewise refused for a + * plaintext `--url`, since the user never paired the two. A token passed as + * `--token` alongside `--url` is left alone: that pairing is explicit, and it + * covers connecting to a local `sqld` over plain http. * * Shared by `db shell`, `db studio`, and `db migrations apply`. */ @@ -57,13 +80,23 @@ export async function resolveCredentials( opts: ResolveCredentialsOptions, ): Promise { const useEnv = !opts.databaseId; + const envToken = useEnv + ? readEnvValue(ENV_DATABASE_AUTH_TOKEN)?.value + : undefined; + let url = opts.url ?? (useEnv ? readEnvValue(ENV_DATABASE_URL)?.value : undefined); - let token = - opts.token ?? - (useEnv ? readEnvValue(ENV_DATABASE_AUTH_TOKEN)?.value : undefined); + let token = opts.token ?? envToken; if (url && token) { + // A stored token wasn't paired with this URL by the user, so don't leak it in the clear. + if (opts.url && !opts.token && envToken && !isEncrypted(opts.url)) { + throw new UserError( + "--url must be encrypted to receive the token from .env.", + "Use libsql:// or https://, or pass --token to send a credential of your choosing.", + ); + } + return { url, token, @@ -72,6 +105,14 @@ export async function resolveCredentials( }; } + // Refuse a plaintext target up front, before any lookup, prompt, or token creation. + if (opts.url && !token && !isEncrypted(opts.url)) { + throw new UserError( + "--url must be encrypted to receive a generated token.", + "Use libsql:// or https://, or pass --token to send your own credential.", + ); + } + const config = resolveConfig(opts.profile, opts.apiKey, opts.verbose); const apiClient = createDbClient(clientOptions(config, opts.verbose)); diff --git a/packages/database-shell/src/parser.ts b/packages/database-shell/src/parser.ts index 66c70506..2bd44586 100644 --- a/packages/database-shell/src/parser.ts +++ b/packages/database-shell/src/parser.ts @@ -25,7 +25,8 @@ function inBlockBody(current: string): boolean { /** * Split a SQL string into individual statements, handling single-quoted strings - * and `--` line comments. Trims whitespace and filters empty results. + * and both `--` line and block comments. Trims whitespace and filters empty + * results. Comments are dropped, so a `;` or a quote inside one is inert. * * `CREATE TRIGGER` bodies are kept intact: semicolons inside `BEGIN ... END` * don't split the statement. @@ -48,6 +49,15 @@ export function splitStatements(sql: string): string[] { continue; } + // Handle /* */ block comments (only outside strings) + if (!inString && ch === "/" && sql[i + 1] === "*") { + const close = sql.indexOf("*/", i + 2); + if (close === -1) break; + i = close + 1; + current += " "; + continue; + } + if (ch === "'") { if (inString) { // '' is an escaped quote inside a string, not end of string diff --git a/packages/database-shell/src/shell.test.ts b/packages/database-shell/src/shell.test.ts index 0ecb4978..906bc6b1 100644 --- a/packages/database-shell/src/shell.test.ts +++ b/packages/database-shell/src/shell.test.ts @@ -589,6 +589,33 @@ describe("splitStatements", () => { ]); }); + test("drops block comments and the semicolons inside them", () => { + expect(splitStatements("SELECT 1 /* a ; b */;")).toEqual(["SELECT 1"]); + expect(splitStatements("SELECT 1; /* between */ SELECT 2;")).toEqual([ + "SELECT 1", + "SELECT 2", + ]); + }); + + test("ignores quotes inside block comments", () => { + expect(splitStatements("SELECT 1 /* it's fine */; SELECT 2;")).toEqual([ + "SELECT 1", + "SELECT 2", + ]); + }); + + test("does not treat a block comment as a trigger block closer", () => { + const sql = + "CREATE TRIGGER t AFTER INSERT ON x BEGIN\n /* END of story */\n UPDATE x SET a = 1;\nEND;"; + expect(splitStatements(sql)).toHaveLength(1); + expect(splitStatements(sql)[0]).toContain("UPDATE x SET a = 1;"); + expect(splitStatements(sql)[0]?.endsWith("END")).toBe(true); + }); + + test("stops at an unterminated block comment without losing the statement", () => { + expect(splitStatements("SELECT 1; /* never closed")).toEqual(["SELECT 1"]); + }); + test("keeps a trigger body whose statement ends in CASE ... END intact", () => { const sql = "CREATE TRIGGER grade AFTER UPDATE ON scores BEGIN\n UPDATE scores SET band = CASE WHEN NEW.v > 90 THEN 'a' ELSE 'b' END;\n UPDATE scores SET seen = 1;\nEND;"; diff --git a/skills/bunny-cli/references/database.md b/skills/bunny-cli/references/database.md index e5a84f06..8cc42357 100644 --- a/skills/bunny-cli/references/database.md +++ b/skills/bunny-cli/references/database.md @@ -181,6 +181,7 @@ Shared by `db shell`, `db studio`, and `db migrations apply`. Two rules apply: - **Passing a database ID skips `.env`.** `bunny db shell db_01ABC` targets that database even when `.env` describes another one, so an explicit target is never silently redirected. - **`--url` without `--token` must match the resolved database.** A generated token is only sent to that database's own host; a mismatch errors and asks for `--token`. +- **`--url` must be encrypted to receive a token you didn't pass yourself.** `libsql://`, `https://`, and `wss://` are accepted; `http://`, `ws://`, and `libsql://host:port?tls=0` are refused, whether the token would be generated or read from `.env`. Passing `--token` alongside a plaintext `--url` is allowed, for cases like a local `sqld`. ### REPL dot-commands From 3f839e9dd0c9f9c830b9e4a455253779211fc53f Mon Sep 17 00:00:00 2001 From: jamie-at-bunny Date: Tue, 28 Jul 2026 19:49:45 +0100 Subject: [PATCH 4/5] fix(db): bind the .env token to the .env URL An encrypted `--url` on a foreign host still received the token from `.env`, because the plaintext guard added in the previous commit was the only check on that path. Last round I assumed host ownership couldn't be verified there without an API call, which was wrong: the `.env` URL is the pairing the user established, so comparing against it is a local check. The `.env` token is now reused only for a `--url` on the same host as the `.env` URL. Anything else falls through to the API path, where a fresh token is created and checked against the database's canonical URL, so the stored credential is never the one that travels. Comparing against `.env` rather than the API keeps the offline case working: both values in `.env` with `--url` naming the same host still needs no network call. Folding the encryption check into the same predicate removes the separate `.env`-specific error path; a plaintext override now falls through to the existing "must be encrypted" refusal. --- AGENTS.md | 7 ++- .../cli/src/commands/db/credentials.test.ts | 45 ++++++++++++++++- packages/cli/src/commands/db/credentials.ts | 48 ++++++++++++------- skills/bunny-cli/references/database.md | 6 ++- 4 files changed, 84 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 380ebca9..db39aab5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1476,7 +1476,12 @@ interface ShellLogger { - An explicit database ID skips `.env` entirely. `.env` may describe a different database, and silently connecting there would target the wrong one. - A generated token is only sent to a URL whose host matches that database's canonical URL, so `--url` without `--token` is rejected on mismatch. The host check runs before the token is created, so nothing is minted for an endpoint we'd refuse. -- A token bound for an explicit `--url` must travel encrypted unless the user passed it as `--token` on the same command line. `isEncrypted()` allows `libsql:`, `https:`, and `wss:`, and rejects `libsql://host:port?tls=0`, which the libSQL client downgrades to plaintext. The scheme check runs first of all, before any lookup or prompt. An explicit `--token` with a plaintext `--url` is left alone: that pairing is deliberate, and it covers a local `sqld` over http. +- The `.env` token is only reused for an explicit `--url` on the same host as the `.env` URL (`envTokenAllowedFor()`). That pairing is the user's own and holds for nothing else, so an override addressing anywhere else falls through to the API path, where a fresh token is created and checked against the canonical URL. The comparison is against `.env` rather than the API so the offline case (both values in `.env`, `--url` naming the same host) still needs no network call. +- A token bound for an explicit `--url` must travel encrypted. `isEncrypted()` allows `libsql:`, `https:`, and `wss:`, and rejects `libsql://host:port?tls=0`, which the libSQL client downgrades to plaintext. The scheme check runs first of all, before any lookup or prompt, so an unusable URL fails immediately instead of after a database prompt. +- An explicit `--token` is exempt from all of the above: pairing it with `--url` is deliberate, and it covers a local `sqld` over plain http. + +The invariant behind all of it: a credential the user didn't pass on this command line is never sent to a target they did. + - `shellLogger()` adapter that wraps the CLI `logger` - `createClient()` call and delegation to `startShell()`/`executeQuery()`/`executeFile()` - Passes resolved `databaseId` and optional `--views-dir` to `startShell()` for saved views diff --git a/packages/cli/src/commands/db/credentials.test.ts b/packages/cli/src/commands/db/credentials.test.ts index 69cf39a0..079ee30e 100644 --- a/packages/cli/src/commands/db/credentials.test.ts +++ b/packages/cli/src/commands/db/credentials.test.ts @@ -1,8 +1,51 @@ import { describe, expect, test } from "bun:test"; -import { isEncrypted, sameHost } from "./credentials.ts"; +import { envTokenAllowedFor, isEncrypted, sameHost } from "./credentials.ts"; const CANONICAL = "libsql://my-db-abc.lite.bunnydb.net/"; +describe("envTokenAllowedFor", () => { + test("allows the .env token when no --url overrides it", () => { + expect(envTokenAllowedFor(undefined, CANONICAL)).toBe(true); + expect(envTokenAllowedFor(undefined, undefined)).toBe(true); + }); + + test("allows a --url naming the same host as the .env URL", () => { + expect( + envTokenAllowedFor("libsql://my-db-abc.lite.bunnydb.net", CANONICAL), + ).toBe(true); + expect( + envTokenAllowedFor("https://my-db-abc.lite.bunnydb.net", CANONICAL), + ).toBe(true); + }); + + test("refuses an encrypted --url on a different host", () => { + expect(envTokenAllowedFor("https://evil.example.com", CANONICAL)).toBe( + false, + ); + expect( + envTokenAllowedFor("libsql://other-db.lite.bunnydb.net", CANONICAL), + ).toBe(false); + }); + + test("refuses a plaintext --url even on the matching host", () => { + expect( + envTokenAllowedFor("http://my-db-abc.lite.bunnydb.net", CANONICAL), + ).toBe(false); + expect( + envTokenAllowedFor( + "libsql://my-db-abc.lite.bunnydb.net:8080?tls=0", + CANONICAL, + ), + ).toBe(false); + }); + + test("refuses when .env has a token but no URL to pair it with", () => { + expect( + envTokenAllowedFor("https://my-db-abc.lite.bunnydb.net", undefined), + ).toBe(false); + }); +}); + describe("isEncrypted", () => { test("accepts libsql, https, and wss", () => { expect(isEncrypted("libsql://h.lite.bunnydb.net")).toBe(true); diff --git a/packages/cli/src/commands/db/credentials.ts b/packages/cli/src/commands/db/credentials.ts index f1efc96e..8078d5f9 100644 --- a/packages/cli/src/commands/db/credentials.ts +++ b/packages/cli/src/commands/db/credentials.ts @@ -55,6 +55,26 @@ export function sameHost(a: string, b: string): boolean { } } +/** + * True when the token stored in `.env` may be sent to an explicit `--url`. + * + * The `.env` token belongs to the `.env` URL: that pairing is the user's own, so + * it holds for the same host and nothing else. An override addressing anywhere + * else falls through to the API path, where a fresh token is created and checked + * against the database's canonical URL instead of reusing the stored one. + * + * Checked against `.env` rather than the API so the offline case (both values in + * `.env`, `--url` naming the same host) still needs no network call. + */ +export function envTokenAllowedFor( + explicitUrl: string | undefined, + envUrl: string | undefined, +): boolean { + if (!explicitUrl) return true; + if (!envUrl) return false; + return sameHost(explicitUrl, envUrl) && isEncrypted(explicitUrl); +} + /** * Resolve the database URL and auth token needed to connect over libSQL. * @@ -66,13 +86,12 @@ export function sameHost(a: string, b: string): boolean { * An explicit database ID skips step 2 entirely: `.env` may describe a different * database, and silently connecting there would target the wrong database. * - * A generated token is only ever sent to an encrypted URL that belongs to the - * database it was created for, so overriding `--url` without `--token` is - * rejected rather than handing a full-access token to an unverified or - * plaintext endpoint. A token read from `.env` is likewise refused for a - * plaintext `--url`, since the user never paired the two. A token passed as - * `--token` alongside `--url` is left alone: that pairing is explicit, and it - * covers connecting to a local `sqld` over plain http. + * The rule for tokens is that a credential the user didn't pass on this command + * line is never sent to a target they did. So a generated token only goes to an + * encrypted URL belonging to the database it was created for, and the `.env` + * token only goes to an encrypted `--url` on the same host as the `.env` URL. + * A token passed as `--token` is left alone: pairing it with `--url` is explicit, + * and it covers connecting to a local `sqld` over plain http. * * Shared by `db shell`, `db studio`, and `db migrations apply`. */ @@ -80,23 +99,16 @@ export async function resolveCredentials( opts: ResolveCredentialsOptions, ): Promise { const useEnv = !opts.databaseId; + const envUrl = useEnv ? readEnvValue(ENV_DATABASE_URL)?.value : undefined; const envToken = useEnv ? readEnvValue(ENV_DATABASE_AUTH_TOKEN)?.value : undefined; - let url = - opts.url ?? (useEnv ? readEnvValue(ENV_DATABASE_URL)?.value : undefined); - let token = opts.token ?? envToken; + let url = opts.url ?? envUrl; + let token = + opts.token ?? (envTokenAllowedFor(opts.url, envUrl) ? envToken : undefined); if (url && token) { - // A stored token wasn't paired with this URL by the user, so don't leak it in the clear. - if (opts.url && !opts.token && envToken && !isEncrypted(opts.url)) { - throw new UserError( - "--url must be encrypted to receive the token from .env.", - "Use libsql:// or https://, or pass --token to send a credential of your choosing.", - ); - } - return { url, token, diff --git a/skills/bunny-cli/references/database.md b/skills/bunny-cli/references/database.md index 8cc42357..25ed8d14 100644 --- a/skills/bunny-cli/references/database.md +++ b/skills/bunny-cli/references/database.md @@ -180,8 +180,10 @@ bunny db shell --url libsql://... --token ey... # explicit credentials Shared by `db shell`, `db studio`, and `db migrations apply`. Two rules apply: - **Passing a database ID skips `.env`.** `bunny db shell db_01ABC` targets that database even when `.env` describes another one, so an explicit target is never silently redirected. -- **`--url` without `--token` must match the resolved database.** A generated token is only sent to that database's own host; a mismatch errors and asks for `--token`. -- **`--url` must be encrypted to receive a token you didn't pass yourself.** `libsql://`, `https://`, and `wss://` are accepted; `http://`, `ws://`, and `libsql://host:port?tls=0` are refused, whether the token would be generated or read from `.env`. Passing `--token` alongside a plaintext `--url` is allowed, for cases like a local `sqld`. +- **`--url` without `--token` must match the resolved database.** A generated token is only sent to that database's own host; a mismatch errors and asks for `--token`. The token from `.env` is likewise only reused for a `--url` on the same host as `BUNNY_DATABASE_URL`. +- **`--url` must be encrypted to receive a token you didn't pass yourself.** `libsql://`, `https://`, and `wss://` are accepted; `http://`, `ws://`, and `libsql://host:port?tls=0` are refused. + +In short, a credential you didn't type on the command line never goes to a URL you did. Passing `--token` alongside a plaintext or foreign `--url` is always allowed, for cases like a local `sqld`. ### REPL dot-commands From 50d741ef30d67580e571f6d8714750dba5f629ce Mon Sep 17 00:00:00 2001 From: jamie-at-bunny Date: Fri, 31 Jul 2026 16:44:43 +0100 Subject: [PATCH 5/5] interactive create --- .gitignore | 3 +++ AGENTS.md | 4 ++-- .../cli/src/commands/db/migrations/create.ts | 23 +++++++++++++++---- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index dde602cc..3aff7416 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,6 @@ report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json .bunny bunny bsql + +# Throwaway local testing +.test diff --git a/AGENTS.md b/AGENTS.md index db39aab5..b436717e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1062,8 +1062,8 @@ bunny │ ├── migrations Create and apply SQL migrations (files are the source of truth) │ │ ├── apply [database-id] [--dir] [--url] [--token] [--dry-run] [--force] │ │ │ Apply pending migrations in filename order (each file + its tracking row is one atomic batch) -│ │ ├── create (alias: new) [--dir] -│ │ │ Write an empty migrations/NNNN_.sql +│ │ ├── create [name] (alias: new) [--dir] +│ │ │ Write an empty migrations/NNNN_.sql (prompts for name when omitted) │ │ └── list [database-id] (aliases: ls, status) [--dir] [--url] [--token] │ │ Show applied / pending / modified / missing migrations │ ├── quickstart [database-id] [--lang] [--url] [--token] diff --git a/packages/cli/src/commands/db/migrations/create.ts b/packages/cli/src/commands/db/migrations/create.ts index 047b6150..14d25e76 100644 --- a/packages/cli/src/commands/db/migrations/create.ts +++ b/packages/cli/src/commands/db/migrations/create.ts @@ -1,8 +1,10 @@ import { existsSync, mkdirSync, writeFileSync } from "node:fs"; import { join, relative } from "node:path"; +import prompts from "prompts"; import { defineCommand } from "../../../core/define-command.ts"; import { UserError } from "../../../core/errors.ts"; import { logger } from "../../../core/logger.ts"; +import { isInteractive } from "../../../core/ui.ts"; import { ARG_DIR } from "./constants.ts"; import { discoverMigrations, @@ -11,12 +13,12 @@ import { slugify, } from "./engine.ts"; -const COMMAND = "create "; +const COMMAND = "create [name]"; const ALIASES = ["new"] as const; const DESCRIPTION = "Create an empty migration file."; interface CreateArgs { - name: string; + name?: string; [ARG_DIR]?: string; } @@ -41,6 +43,7 @@ export const dbMigrationsCreateCommand = defineCommand({ "$0 db migrations create add_users_table", "Create migrations/0001_add_users_table.sql", ], + ["$0 db migrations create", "Prompt for a name"], [ "$0 db migrations create add_index --dir db/migrations", "Use a custom directory", @@ -52,14 +55,26 @@ export const dbMigrationsCreateCommand = defineCommand({ .positional("name", { type: "string", describe: "Migration name, used as the filename suffix", - demandOption: true, }) .option(ARG_DIR, { type: "string", describe: "Migrations directory (default: migrations)", }), - handler: async ({ name, [ARG_DIR]: dirArg, output }) => { + handler: async ({ name: nameArg, [ARG_DIR]: dirArg, output }) => { + let name = nameArg; + if (!name && isInteractive(output)) { + const { value } = await prompts({ + type: "text", + name: "value", + message: "Migration name:", + validate: (v: string) => + /[a-z0-9]/i.test(v) || "Must contain at least one letter or number", + }); + name = value; + } + if (!name) throw new UserError("Migration name is required."); + const { dir } = resolveMigrationsDir(dirArg); if (!existsSync(dir)) mkdirSync(dir, { recursive: true });