Skip to content

test(server): guard schema.migrate on migration/execute; typecheck the CLI in CI - #489

Merged
huyplb merged 1 commit into
mainfrom
test/review-features-dialects-xhwu6u
Oct 7, 2026
Merged

huyplb merged 1 commit into
mainfrom
test/review-features-dialects-xhwu6u

Conversation

@huyplb

@huyplb huyplb commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

A code review of the previous branch turned up two gaps that were already on main. This PR closes both.

1. The schema.migrate guard on /api/migration/execute was untested

POST /api/migration/execute applies DDL to a real database. Its requirePermissions('schema.migrate') guard (migration.routes.ts:54) is the only check on that path. Compare has a second check in CompareService, with its own test; migrate has none. Removing the guard left the suite green.

The new packages/server/src/features/migration/migration.routes.test.ts follows the pattern of access.routes.test.ts, with a Fastify instance and an authenticated user whose permissions the test chooses. It checks two things:

  • Without schema.migrate: a user who can browse, compare and run SQL, but not migrate, gets 403 forbidden. The connection is never looked up and nothing executes.
  • With schema.migrate: the user gets past the guard.

2. CI never type-checked apps/cli

npm run typecheck, which the build gate runs, covered apps/web (including packages/*) and apps/e2e. apps/cli has its own strict tsconfig, and its esbuild build does not type-check. The script now also runs tsc --noEmit -p apps/cli/tsconfig.json. That passes today and adds about 11 seconds.

Verification

  • Guard test: with the requirePermissions('schema.migrate') guard removed, the first new test fails. With it in place, both pass.
  • Typecheck: a type error planted in apps/cli/src passes the old typecheck (exit 0) and fails the new one (TS2322).
  • Full checks:
    • npm run typecheck: clean.
    • ESLint and the security ESLint config on the new test: clean.
    • npx vitest run on Node 24: 480 test files passed, 8 skipped; 5860 tests passed.

🤖 Generated with Claude Code

https://claude.ai/code/session_016PyNv48vpYUw2HhpjYFNUi


Generated by Claude Code


Note

Low Risk
Test-only and CI script changes; no production behavior or auth logic is modified.

Overview
Adds regression coverage for POST /migration/execute: users without schema.migrate get 403 before connection lookup or migration execution; users with the permission pass the route guard.

Extends root npm run typecheck to include apps/cli/tsconfig.json, so CI type-checks the CLI alongside web and e2e.

Reviewed by Cursor Bugbot for commit a463b01. Bugbot is set up for automated code reviews on this repo. Configure here.

…e CLI in CI

Two gaps a review of this branch turned up, both already on main:

- POST /api/migration/execute applies DDL to a real database, and its
  requirePermissions('schema.migrate') guard is the only check on that
  path: unlike Compare, no service re-checks the permission. Nothing
  tested it, so dropping the guard left the suite green.
  migration.routes.test.ts now asserts that an actor with everything
  short of schema.migrate gets 403 before the connection is looked up,
  and that one with schema.migrate gets past the guard. Deleting the
  guard fails the first test.

- `npm run typecheck`, which CI's build gate runs, covered apps/web
  (with the packages) and apps/e2e but never apps/cli, whose esbuild
  build does not type-check. A planted type error in apps/cli/src passed
  it. The script now also runs `tsc -p apps/cli/tsconfig.json`, which
  passes today and adds about 11 s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016PyNv48vpYUw2HhpjYFNUi
@cursor

cursor Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: d4959be0-c803-4ccb-b723-30210f501b0e)

@huyplb
huyplb merged commit a267f7a into main Oct 7, 2026
12 checks passed
@huyplb
huyplb deleted the test/review-features-dialects-xhwu6u branch October 7, 2026 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants