fix: mark completed till validation for migration reset

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>

Author: ferhatelmas

src/http/routes/admin/migrations.ts

@@ -1,6 +1,6 @@
 import { multitenantKnex } from '@internal/database'
 import {
-  DBMigration,
+  isDBMigrationName,
   resetMigrationsOnTenants,
   runMigrationsOnAllTenants,
 } from '@internal/database/migrations'
@@ -30,27 +30,25 @@ export default async function routes(fastify: FastifyInstance) {
       return reply.status(400).send({ message: 'Queue is not enabled' })
     }
 
-    const { untilMigration, markCompletedTillMigration } = req.body as any
+    const { untilMigration, markCompletedTillMigration } = req.body as Record<string, unknown>
 
-    if (
-      typeof untilMigration !== 'string' ||
-      !DBMigration[untilMigration as keyof typeof DBMigration]
-    ) {
+    if (!isDBMigrationName(untilMigration)) {
       return reply.status(400).send({ message: 'Invalid migration' })
     }
 
     if (
       typeof markCompletedTillMigration === 'string' &&
-      !DBMigration[untilMigration as keyof typeof DBMigration]
+      !isDBMigrationName(markCompletedTillMigration)
     ) {
       return reply.status(400).send({ message: 'Invalid migration' })
     }
 
     await resetMigrationsOnTenants({
-      till: untilMigration as keyof typeof DBMigration,
-      markCompletedTillMigration: markCompletedTillMigration
-        ? markCompletedTillMigration
-        : undefined,
+      till: untilMigration,
+      markCompletedTillMigration:
+        isDBMigrationName(markCompletedTillMigration)
+          ? markCompletedTillMigration
+          : undefined,
       signal: req.signals.disconnect.signal,
     })
 

src/http/routes/admin/tenants.ts

@@ -8,7 +8,7 @@ import {
   TenantMigrationStatus,
 } from '@internal/database'
 import {
-  DBMigration,
+  isDBMigrationName,
   lastLocalMigrationName,
   progressiveMigrations,
   resetMigration,
@@ -596,20 +596,17 @@ export default async function routes(fastify: FastifyInstance) {
   })
 
   fastify.post<tenantRequestInterface>('/:tenantId/migrations/reset', async (req, reply) => {
-    const { untilMigration, markCompletedTillMigration } = req.body
+    const { untilMigration, markCompletedTillMigration } = req.body as Record<string, unknown>
 
     const { databaseUrl } = await getTenantConfig(req.params.tenantId)
 
-    if (
-      typeof untilMigration !== 'string' ||
-      !DBMigration[untilMigration as keyof typeof DBMigration]
-    ) {
+    if (!isDBMigrationName(untilMigration)) {
       return reply.status(400).send({ message: 'Invalid migration' })
     }
 
     if (
       typeof markCompletedTillMigration === 'string' &&
-      !DBMigration[untilMigration as keyof typeof DBMigration]
+      !isDBMigrationName(markCompletedTillMigration)
     ) {
       return reply.status(400).send({ message: 'Invalid migration' })
     }
@@ -618,10 +615,11 @@ export default async function routes(fastify: FastifyInstance) {
       await resetMigration({
         tenantId: req.params.tenantId,
         databaseUrl,
-        untilMigration: untilMigration as keyof typeof DBMigration,
-        markCompletedTillMigration: markCompletedTillMigration
-          ? (markCompletedTillMigration as keyof typeof DBMigration)
-          : undefined,
+        untilMigration,
+        markCompletedTillMigration:
+          isDBMigrationName(markCompletedTillMigration)
+            ? markCompletedTillMigration
+            : undefined,
       })
 
       return reply.send({ message: 'Migrations reset' })

src/internal/database/migrations/guards.ts

@@ -0,0 +1,7 @@
+import { DBMigration } from './types'
+
+export function isDBMigrationName(value: unknown): value is keyof typeof DBMigration {
+  return (
+    typeof value === 'string' && Object.prototype.hasOwnProperty.call(DBMigration, value)
+  )
+}

src/internal/database/migrations/index.ts

@@ -1,3 +1,4 @@
 export * from './files'
+export * from './guards'
 export * from './migrate'
 export * from './types'

src/internal/database/migrations/types.ts

@@ -56,4 +56,4 @@ export const DBMigration = {
   'drop-index-object-level': 54,
   'prevent-direct-deletes': 55,
   'fix-optimized-search-function': 56,
-}
+} as const

src/scripts/migrations-types.ts

@@ -39,7 +39,7 @@ function main() {
 
   const template = `export const DBMigration = {
 ${migrationsEnum.join('\n')}
-}
+} as const
 `
 
   const destinationPath = path.resolve(

src/test/admin-migrations.test.ts

@@ -0,0 +1,124 @@
+jest.mock('@internal/database/migrations', () => {
+  const actual = jest.requireActual('@internal/database/migrations')
+  return {
+    ...actual,
+    resetMigrationsOnTenants: jest.fn(),
+    resetMigration: jest.fn(),
+    runMigrationsOnAllTenants: jest.fn(),
+    runMigrationsOnTenant: jest.fn(),
+  }
+})
+
+import * as migrations from '@internal/database/migrations'
+import { DBMigration } from '@internal/database/migrations'
+import { mergeConfig } from '../config'
+import { multitenantKnex } from '../internal/database/multitenant-db'
+
+mergeConfig({
+  pgQueueEnable: true,
+})
+
+import { adminApp } from './common'
+
+const tenantId = 'admin-migrations-test-tenant'
+
+const tenantPayload = {
+  anonKey: 'anon-key',
+  databaseUrl: 'postgres://tenant-db',
+  jwtSecret: 'jwt-secret',
+  serviceKey: 'service-key',
+}
+
+describe('Admin migrations routes', () => {
+  beforeAll(async () => {
+    await migrations.runMultitenantMigrations()
+  })
+
+  afterEach(async () => {
+    jest.clearAllMocks()
+
+    await adminApp.inject({
+      method: 'DELETE',
+      url: `/tenants/${tenantId}`,
+      headers: {
+        apikey: process.env.ADMIN_API_KEYS,
+      },
+    })
+  })
+
+  afterAll(async () => {
+    await multitenantKnex.destroy()
+  })
+
+  test('rejects invalid markCompletedTillMigration for fleet reset', async () => {
+    const resetSpy = jest.mocked(migrations.resetMigrationsOnTenants).mockResolvedValue(undefined)
+
+    const response = await adminApp.inject({
+      method: 'POST',
+      url: '/migrations/reset/fleet',
+      payload: {
+        untilMigration: 'storage-schema' satisfies keyof typeof DBMigration,
+        markCompletedTillMigration: 'not-a-real-migration',
+      },
+      headers: {
+        apikey: process.env.ADMIN_API_KEYS,
+      },
+    })
+
+    expect(response.statusCode).toBe(400)
+    expect(JSON.parse(response.body)).toEqual({ message: 'Invalid migration' })
+    expect(resetSpy).not.toHaveBeenCalled()
+  })
+
+  test('rejects invalid markCompletedTillMigration for tenant reset', async () => {
+    await adminApp.inject({
+      method: 'POST',
+      url: `/tenants/${tenantId}`,
+      payload: tenantPayload,
+      headers: {
+        apikey: process.env.ADMIN_API_KEYS,
+      },
+    })
+
+    const resetSpy = jest.mocked(migrations.resetMigration).mockResolvedValue(false)
+
+    const response = await adminApp.inject({
+      method: 'POST',
+      url: `/tenants/${tenantId}/migrations/reset`,
+      payload: {
+        untilMigration: 'storage-schema' satisfies keyof typeof DBMigration,
+        markCompletedTillMigration: 'not-a-real-migration',
+      },
+      headers: {
+        apikey: process.env.ADMIN_API_KEYS,
+      },
+    })
+
+    expect(response.statusCode).toBe(400)
+    expect(JSON.parse(response.body)).toEqual({ message: 'Invalid migration' })
+    expect(resetSpy).not.toHaveBeenCalled()
+  })
+
+  test('accepts untilMigration that maps to numeric id 0 for fleet reset', async () => {
+    const resetSpy = jest.mocked(migrations.resetMigrationsOnTenants).mockResolvedValue(undefined)
+
+    const response = await adminApp.inject({
+      method: 'POST',
+      url: '/migrations/reset/fleet',
+      payload: {
+        untilMigration: 'create-migrations-table' satisfies keyof typeof DBMigration,
+      },
+      headers: {
+        apikey: process.env.ADMIN_API_KEYS,
+      },
+    })
+
+    expect(response.statusCode).toBe(200)
+    expect(JSON.parse(response.body)).toEqual({ message: 'Migrations scheduled' })
+    expect(resetSpy).toHaveBeenCalledWith({
+      till: 'create-migrations-table',
+      markCompletedTillMigration: undefined,
+      signal: expect.any(AbortSignal),
+    })
+  })
+})