feat(db): migrations create, list, and apply - #136
Conversation
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_<slug>.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 <file>.sql` benefits from too. Credential resolution moves to `db/credentials.ts`, shared by shell, studio, and migrations apply instead of a third copy.
|
@codex review |
🦋 Changeset detectedLatest commit: 50d741e The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR adds file-based database migrations and strengthens shared database credential and SQL-trigger parsing behavior.
Confidence Score: 4/5This PR is not safe to merge until database URLs loaded from A plaintext environment URL can bypass both credential-resolution checks, causing an existing or newly generated database token to be passed to shell, studio, and migration libSQL clients over an unencrypted connection. Files Needing Attention: packages/cli/src/commands/db/credentials.ts
|
| Filename | Overview |
|---|---|
| packages/cli/src/commands/db/credentials.ts | Centralizes URL/token resolution and protects explicit URL overrides, but fails to enforce encrypted transport for URLs sourced from .env. |
| packages/cli/src/commands/db/migrations/engine.ts | Implements migration discovery, checksums, tracking state, and atomic migration execution without an accepted finding. |
| packages/database-shell/src/parser.ts | Correctly fixes the previously reported trigger splitting failures involving CASE ... END and block-comment END text. |
| packages/cli/src/commands/db/migrations/apply.ts | Applies pending migrations through the shared credential resolver and therefore inherits its plaintext environment-URL exposure. |
| packages/cli/src/commands/db/migrations/list.ts | Lists migration state through the shared credential resolver and therefore inherits its plaintext environment-URL exposure. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
E[".env URL and optional token"] --> R["resolveCredentials"]
R --> C{"Token already available?"}
C -->|Yes| X["Early return"]
C -->|No| G{"opts.url is plaintext?"}
G -->|"No: opts.url absent"| T["Generate full-access token"]
X --> L["libSQL client"]
T --> L
L --> P["Connect using resolved .env URL"]
Reviews (4): Last reviewed commit: "fix(db): bind the .env token to the .env..." | Re-trigger Greptile
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e496926392
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
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.
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.
| } | ||
|
|
||
| // Refuse a plaintext target up front, before any lookup, prompt, or token creation. | ||
| if (opts.url && !token && !isEncrypted(opts.url)) { |
There was a problem hiding this comment.
When BUNNY_DATABASE_URL contains an http://, ws://, or libsql://...?tls=0 URL and no explicit --url is passed, the environment token returns before encryption validation or this opts.url-only guard is skipped before generating a full-access token. Shell, studio, migration apply, and migration list then pass the plaintext URL and token to a libSQL client, exposing the credential to network observers.
How this was verified: The environment-sourced URL bypasses both encryption checks before the resolved URL and token are passed directly to each libSQL client.
Knowledge Base Used: CLI db Command Group
2 participants
Adds
bunny db migrationsfor running plain SQL migration files against a Bunny Database.