fix: use correct supabase auth in stripe-setup edge-function (#385)

* fix: use secret key auth for stripe-setup

* move to a separate function

* refactor: rename a function to better reflect its purpose

---------

Co-authored-by: Yostra <straya.mark@gmail.com>

Author: imor

packages/sync-engine/src/supabase/edge-functions/stripe-setup.ts

@@ -5,6 +5,10 @@ import { embeddedMigrations } from '../../database/migrations-embedded.ts'
 import { parseSchemaComment } from '../schemaComment.ts'
 import postgres from 'postgres'
 
+// Name of the vault secret used to authenticate callers of this function.
+// Generated per-install and stored in vault, mirroring the stripe-worker secret.
+const SETUP_SECRET_NAME = 'stripe_setup_secret'
+
 // Get management API base URL from environment variable (for testing against localhost/staging)
 // Caller should provide full URL with protocol (e.g., http://localhost:54323 or https://api.supabase.com)
 const MGMT_API_BASE_RAW = Deno.env.get('MANAGEMENT_API_URL') || 'https://api.supabase.com'
@@ -55,6 +59,45 @@ async function deleteSecret(
   }
 }
 
+// Thrown when setup-secret validation fails; carries the HTTP status for the response.
+class SetupAuthError extends Error {
+  constructor(
+    readonly status: number,
+    message: string
+  ) {
+    super(message)
+  }
+}
+
+// Validates the caller's bearer secret against the per-install setup secret in vault.
+// Returns only on a strict match with a non-empty stored secret; throws on every other case.
+async function authenticateCaller(callerSecret: string): Promise<void> {
+  const dbUrl = Deno.env.get('SUPABASE_DB_URL')
+  if (!dbUrl) {
+    throw new SetupAuthError(500, 'SUPABASE_DB_URL not set')
+  }
+
+  let authSql: ReturnType<typeof postgres> | undefined
+  try {
+    authSql = postgres(dbUrl, { max: 1, prepare: false })
+    const secretResult = await authSql`
+      SELECT decrypted_secret
+      FROM vault.decrypted_secrets
+      WHERE name = ${SETUP_SECRET_NAME}
+    `
+    const storedSecret: unknown = secretResult[0]?.decrypted_secret
+    if (typeof storedSecret !== 'string' || storedSecret.length === 0) {
+      throw new SetupAuthError(500, 'Setup secret not configured in vault')
+    }
+    if (callerSecret === storedSecret) {
+      return
+    }
+    throw new SetupAuthError(403, 'Forbidden: Invalid setup secret')
+  } finally {
+    if (authSql) await authSql.end()
+  }
+}
+
 Deno.serve(async (req) => {
   // Extract project ref from SUPABASE_URL (format: https://{projectRef}.{base})
   const supabaseUrl = Deno.env.get('SUPABASE_URL')
@@ -66,13 +109,36 @@ Deno.serve(async (req) => {
   }
   const projectRef = new URL(supabaseUrl).hostname.split('.')[0]
 
-  // Validate access token for all requests
+  // Authenticate the caller against the per-install secret stored in vault.
+  // The Authorization bearer token must match `stripe_setup_secret`. An attacker
+  // cannot read the vault to learn this value, and because the destructive
+  // branches below only run after this check passes, a forged token is rejected
+  // before any cleanup occurs. This mirrors how the stripe-worker function
+  // authenticates against its own vault secret.
   const authHeader = req.headers.get('Authorization')
   if (!authHeader?.startsWith('Bearer ')) {
     return new Response('Unauthorized', { status: 401 })
   }
+  const callerSecret = authHeader.substring(7) // Remove 'Bearer '
+
+  // The Management API access token used for the function's own Management API
+  // operations (deleting secrets and edge functions during uninstall). It is
+  // supplied out-of-band from the authenticating secret so that this function
+  // never has to trust the bearer token for privileged Management operations.
+  const accessToken = req.headers.get('x-management-api-token') ?? ''
 
-  const accessToken = authHeader.substring(7) // Remove 'Bearer '
+  try {
+    await authenticateCaller(callerSecret)
+  } catch (error: unknown) {
+    if (error instanceof SetupAuthError) {
+      return new Response(error.message, { status: error.status })
+    }
+    console.error('Setup secret validation error:', error)
+    return new Response(JSON.stringify({ error: (error as Error).message }), {
+      status: 500,
+      headers: { 'Content-Type': 'application/json' },
+    })
+  }
 
   // Handle GET requests for status
   if (req.method === 'GET') {
@@ -216,7 +282,7 @@ Deno.serve(async (req) => {
       try {
         await stripeSync.postgresClient.query(`
           DELETE FROM vault.secrets
-          WHERE name IN ('stripe_sync_worker_secret', 'stripe_sigma_worker_secret')
+          WHERE name IN ('stripe_sync_worker_secret', 'stripe_sigma_worker_secret', 'stripe_setup_secret')
         `)
       } catch (err) {
         console.warn('Could not delete vault secret:', err)

packages/sync-engine/src/supabase/supabase.ts

@@ -28,6 +28,7 @@ export class SupabaseSetupClient {
   private supabaseManagementUrl?: string
   private accessToken: string
   private workerSecret: string
+  private setupSecret: string
 
   constructor(options: DeployClientOptions) {
     this.api = new SupabaseManagementAPI({
@@ -39,6 +40,35 @@ export class SupabaseSetupClient {
     this.supabaseManagementUrl = options.supabaseManagementUrl
     this.accessToken = options.accessToken
     this.workerSecret = crypto.randomUUID()
+    this.setupSecret = crypto.randomUUID()
+  }
+
+  /**
+   * Store the per-install setup secret in vault. The stripe-setup edge function
+   * authenticates callers against this value, so it must exist before the
+   * function is invoked.
+   */
+  private async storeSetupSecret(): Promise<void> {
+    const escapedSetupSecret = this.setupSecret.replace(/'/g, "''")
+    await this.runSQL(`
+      DELETE FROM vault.secrets WHERE name = 'stripe_setup_secret';
+      SELECT vault.create_secret('${escapedSetupSecret}', 'stripe_setup_secret');
+    `)
+  }
+
+  /**
+   * Read the per-install setup secret from vault. Used during uninstall to
+   * authenticate the call to the stripe-setup edge function.
+   */
+  private async getSetupSecret(): Promise<string | null> {
+    try {
+      const result = (await this.runSQL(
+        `SELECT decrypted_secret FROM vault.decrypted_secrets WHERE name = 'stripe_setup_secret'`
+      )) as { decrypted_secret: string }[]
+      return result[0]?.decrypted_secret ?? null
+    } catch {
+      return null
+    }
   }
 
   /**
@@ -271,15 +301,23 @@ export class SupabaseSetupClient {
   async invokeFunction(
     slug: string,
     method: string,
-    bearerToken: string
+    bearerToken: string,
+    managementApiToken?: string
   ): Promise<{ success: boolean; error?: string }> {
     const url = `https://${this.projectRef}.${this.projectBaseUrl}/functions/v1/${slug}`
+    const headers: Record<string, string> = {
+      Authorization: `Bearer ${bearerToken}`,
+      'Content-Type': 'application/json',
+    }
+    // The stripe-setup function authenticates the caller via the bearer secret,
+    // but performs its own Management API operations using a separate token
+    // passed out-of-band in this header.
+    if (managementApiToken) {
+      headers['x-management-api-token'] = managementApiToken
+    }
     const response = await fetch(url, {
       method,
-      headers: {
-        Authorization: `Bearer ${bearerToken}`,
-        'Content-Type': 'application/json',
-      },
+      headers,
     })
 
     if (!response.ok) {
@@ -450,9 +488,22 @@ export class SupabaseSetupClient {
         await this.updateComment({ status: 'uninstalling', startTime })
       }
 
+      // Read the setup secret stored in vault at install time to authenticate
+      // the uninstall call. The Management API token (this.accessToken) authorizes
+      // reading the secret here, and is also forwarded for the function's own
+      // Management API cleanup operations.
+      const setupSecret = await this.getSetupSecret()
+      if (!setupSecret) {
+        throw new Error('Setup secret not found in vault; cannot authenticate uninstall')
+      }
+
       // Invoke the DELETE endpoint on stripe-setup function
-      // Use accessToken in Authorization header for Management API validation
-      const setupResult = await this.invokeFunction('stripe-setup', 'DELETE', this.accessToken)
+      const setupResult = await this.invokeFunction(
+        'stripe-setup',
+        'DELETE',
+        setupSecret,
+        this.accessToken
+      )
 
       if (!setupResult.success) {
         throw new Error(`Uninstall failed: ${setupResult.error}`)
@@ -523,9 +574,19 @@ export class SupabaseSetupClient {
       const versionedSetup = this.injectPackageVersion(setupFunctionCode, version)
       await this.deployFunction('stripe-setup', versionedSetup, false)
 
+      // Store the setup secret in vault before invoking the function, since the
+      // function authenticates the caller against this value.
+      await this.storeSetupSecret()
+
       // Run setup (migrations + webhook creation)
-      // Use accessToken for Management API validation
-      const setupResult = await this.invokeFunction('stripe-setup', 'POST', this.accessToken)
+      // Authenticate with the setup secret; pass the Management token separately
+      // for the function's own Management API operations.
+      const setupResult = await this.invokeFunction(
+        'stripe-setup',
+        'POST',
+        this.setupSecret,
+        this.accessToken
+      )
 
       if (!setupResult.success) {
         throw new Error(`Setup failed: ${setupResult.error}`)

packages/sync-engine/src/tests/unit/supabase.test.ts

@@ -1,4 +1,5 @@
-import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
 import { SupabaseSetupClient } from '../../supabase/supabase'
 
 describe('SupabaseDeployClient', () => {
@@ -655,9 +656,6 @@ describe('SupabaseDeployClient', () => {
       })
 
       // Mock only what we need to test
-      client.validateProject = vi
-        .fn()
-        .mockResolvedValue({ id: mockProjectRef, name: 'test', region: 'us-east-1' })
       client.runSQL = vi.fn().mockResolvedValue(null)
       client.deployFunction = vi.fn().mockResolvedValue(null)
       client.setSecrets = mockSetSecrets
@@ -683,9 +681,6 @@ describe('SupabaseDeployClient', () => {
       })
 
       // Mock only what we need to test
-      client.validateProject = vi
-        .fn()
-        .mockResolvedValue({ id: mockProjectRef, name: 'test', region: 'us-east-1' })
       client.runSQL = vi.fn().mockResolvedValue(null)
       client.deployFunction = vi.fn().mockResolvedValue(null)
       client.setSecrets = mockSetSecrets
@@ -701,4 +696,134 @@ describe('SupabaseDeployClient', () => {
       ])
     })
   })
+
+  describe('Setup secret authentication', () => {
+    it('passes the setup secret as bearer and the management token as x-management-api-token', async () => {
+      const client = new SupabaseSetupClient({
+        accessToken: mockAccessToken,
+        projectRef: mockProjectRef,
+        projectBaseUrl: 'test-domain.com',
+      })
+
+      const mockFetch = vi.fn().mockResolvedValue({
+        ok: true,
+        json: async () => ({ success: true }),
+      })
+      global.fetch = mockFetch
+
+      await client.invokeFunction('stripe-setup', 'DELETE', 'the-setup-secret', mockAccessToken)
+
+      expect(mockFetch).toHaveBeenCalledWith(
+        `https://${mockProjectRef}.test-domain.com/functions/v1/stripe-setup`,
+        expect.objectContaining({
+          method: 'DELETE',
+          headers: expect.objectContaining({
+            Authorization: 'Bearer the-setup-secret',
+            'x-management-api-token': mockAccessToken,
+          }),
+        })
+      )
+
+      vi.restoreAllMocks()
+    })
+
+    it('does not set x-management-api-token when no management token is provided', async () => {
+      const client = new SupabaseSetupClient({
+        accessToken: mockAccessToken,
+        projectRef: mockProjectRef,
+      })
+
+      const mockFetch = vi.fn().mockResolvedValue({
+        ok: true,
+        json: async () => ({ success: true }),
+      })
+      global.fetch = mockFetch
+
+      await client.invokeFunction('stripe-worker', 'POST', 'worker-secret')
+
+      const headers = mockFetch.mock.calls[0][1].headers
+      expect(headers).not.toHaveProperty('x-management-api-token')
+
+      vi.restoreAllMocks()
+    })
+
+    it('stores the setup secret in vault before invoking stripe-setup during install', async () => {
+      const runSqlCalls: string[] = []
+      const client = new SupabaseSetupClient({
+        accessToken: mockAccessToken,
+        projectRef: mockProjectRef,
+      })
+
+      client.runSQL = vi.fn().mockImplementation(async (sql: string) => {
+        runSqlCalls.push(sql)
+        return null
+      })
+      client.deployFunction = vi.fn().mockResolvedValue(null)
+      client.setSecrets = vi.fn().mockResolvedValue(null)
+      client.setupPgCronJob = vi.fn().mockResolvedValue(null)
+      client.setupSigmaPgCronJob = vi.fn().mockResolvedValue(null)
+
+      let setupSecretStoredBeforeInvoke = false
+      client.invokeFunction = vi.fn().mockImplementation(async (slug: string) => {
+        if (slug === 'stripe-setup') {
+          setupSecretStoredBeforeInvoke = runSqlCalls.some((q) =>
+            q.includes("'stripe_setup_secret'")
+          )
+        }
+        return { success: true }
+      })
+
+      await client.install('sk_test_key')
+
+      expect(setupSecretStoredBeforeInvoke).toBe(true)
+    })
+
+    it('authenticates stripe-setup with the secret read from vault, not the access token', async () => {
+      const storedSecret = 'stored-setup-secret'
+      const client = new SupabaseSetupClient({
+        accessToken: mockAccessToken,
+        projectRef: mockProjectRef,
+      })
+
+      client.runSQL = vi.fn().mockImplementation(async (sql: string) => {
+        if (sql.includes('stripe_setup_secret')) {
+          return [{ decrypted_secret: storedSecret }]
+        }
+        if (sql.includes('schema_exists')) {
+          return [{ schema_exists: true }]
+        }
+        return [{ comment: null }]
+      })
+
+      const invokeSpy = vi.fn().mockResolvedValue({ success: true })
+      client.invokeFunction = invokeSpy
+      client.updateComment = vi.fn().mockResolvedValue(undefined)
+
+      await client.uninstall()
+
+      expect(invokeSpy).toHaveBeenCalledWith(
+        'stripe-setup',
+        'DELETE',
+        storedSecret,
+        mockAccessToken
+      )
+    })
+
+    it('fails uninstall without invoking stripe-setup when the setup secret cannot be read', async () => {
+      const client = new SupabaseSetupClient({
+        accessToken: 'attacker-controlled-token',
+        projectRef: mockProjectRef,
+      })
+
+      // Simulate the Management API rejecting an unauthorized token: every
+      // runSQL (including the vault read) throws.
+      client.runSQL = vi.fn().mockRejectedValue(new Error('401 Unauthorized'))
+
+      const invokeSpy = vi.fn().mockResolvedValue({ success: true })
+      client.invokeFunction = invokeSpy
+
+      await expect(client.uninstall()).rejects.toThrow(/Setup secret not found in vault/)
+      expect(invokeSpy).not.toHaveBeenCalled()
+    })
+  })
 })