From a463b0113a9c155bfd014f23ffbb6e61be6f13b4 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Wed, 7 Oct 2026 01:00:08 +0000
Subject: [PATCH] test(server): guard schema.migrate on migration/execute;
 typecheck the 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
---
 package.json                                  |  2 +-
 .../migration/migration.routes.test.ts        | 80 +++++++++++++++++++
 2 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 packages/server/src/features/migration/migration.routes.test.ts

diff --git a/package.json b/package.json
index 802cad842..5c11167e6 100644
--- a/package.json
+++ b/package.json
@@ -12,7 +12,7 @@
   ],
   "bugs": "https://github.com/tedious-code/foxschema/issues",
   "scripts": {
-    "typecheck": "tsc --noEmit -p apps/web/tsconfig.json && tsc --noEmit -p apps/e2e/tsconfig.json",
+    "typecheck": "tsc --noEmit -p apps/web/tsconfig.json && tsc --noEmit -p apps/e2e/tsconfig.json && tsc --noEmit -p apps/cli/tsconfig.json",
     "lint": "eslint .",
     "lint:security": "eslint . --config eslint.security.config.js --max-warnings 0",
     "test-ids": "node scripts/test-ids/extract-test-ids.mjs --write",
diff --git a/packages/server/src/features/migration/migration.routes.test.ts b/packages/server/src/features/migration/migration.routes.test.ts
new file mode 100644
index 000000000..ebf660419
--- /dev/null
+++ b/packages/server/src/features/migration/migration.routes.test.ts
@@ -0,0 +1,80 @@
+/**
+ * Fox Schema (foxschema)
+ * Copyright 2024-2026 Huy Phan <huyplb@gmail.com>
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Running a migration needs `schema.migrate`, checked at the route.
+ *
+ * `/migration/execute` applies DDL to a real database, and unlike Compare it
+ * has no service of its own that checks the permission again: the route guard
+ * is the only thing between a viewer and the target. So the refusal has to
+ * come before anything else, the connection lookup included.
+ */
+import Fastify, { type FastifyInstance } from 'fastify';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import type { Permission } from '@foxschema/shared';
+import type { AuthedRequest } from '../auth/auth.routes';
+import { bindRoutes } from '../../platform/http/fastify-bind';
+import { createMigrationRoutes } from './migration.routes';
+
+let app: FastifyInstance | undefined;
+
+afterEach(async () => {
+  await app?.close();
+  app = undefined;
+});
+
+async function serve(permissions: Permission[]) {
+  // Rejecting stops a permitted request at the connection lookup, so nothing
+  // past the guard needs a real database.
+  const resolveRef = vi.fn().mockRejectedValue(new Error('no such connection'));
+  const executeMigration = vi.fn();
+  const router = createMigrationRoutes({
+    resolveRef,
+    migrationModule: { executeMigration },
+    migrationHistory: {},
+    connectionModule: {},
+    sqlGenerator: {},
+    captureLiveSchema: vi.fn(),
+    normalizeTableSchemas: vi.fn(),
+    gitMigrations: { read: vi.fn(), recordApplied: vi.fn(), commitRequired: vi.fn(), canSee: vi.fn() },
+  });
+  app = Fastify();
+  app.addHook('onRequest', async (req) => {
+    const authed = req as unknown as AuthedRequest;
+    authed.userId = 'some-user';
+    authed.appRole = 'viewer';
+    authed.permissions = new Set(permissions);
+  });
+  bindRoutes(app, router.flatten());
+  await app.ready();
+  return { server: app, resolveRef, executeMigration };
+}
+
+const execute = (server: FastifyInstance) =>
+  server.inject({
+    method: 'POST',
+    url: '/migration/execute',
+    payload: { connectionId: 'c1', steps: [{ sql: 'DROP TABLE orders' }] },
+  });
+
+describe('POST /migration/execute', () => {
+  it('refuses an actor without schema.migrate before looking up the connection', async () => {
+    // Everything short of migrating: browsing, comparing, running SQL.
+    const { server, resolveRef, executeMigration } = await serve(['schema.browse', 'schema.compare', 'editor.run']);
+    const res = await execute(server);
+
+    expect(res.statusCode).toBe(403);
+    expect(res.json().code).toBe('forbidden');
+    expect(resolveRef).not.toHaveBeenCalled();
+    expect(executeMigration).not.toHaveBeenCalled();
+  });
+
+  it('lets an actor with schema.migrate past the guard', async () => {
+    const { server, resolveRef } = await serve(['schema.migrate']);
+    const res = await execute(server);
+
+    expect(res.json().code).not.toBe('forbidden');
+    expect(resolveRef).toHaveBeenCalledOnce();
+  });
+});