Harden for the Intuit security review: validated redirects, sealed realm id, security headers, same-origin guard (#331)

next.config.ts

@@ -14,6 +14,24 @@ const nextConfig: NextConfig = {
   },
   async headers() {
     return [
+      // Every response. Later, more specific entries win on the same key, so
+      // the status-report share below still gets its stricter values.
+      {
+        source: '/(.*)',
+        headers: [
+          { key: 'X-Content-Type-Options', value: 'nosniff' },
+          // Nothing off this origin may frame the app; the app frames its own
+          // previews and print views, so same-origin stays allowed.
+          { key: 'X-Frame-Options', value: 'SAMEORIGIN' },
+          { key: 'Referrer-Policy', value: 'strict-origin-when-cross-origin' },
+          // Camera and microphone are used by our own pages (scanning, photos,
+          // recordings); an embedded third-party frame gets neither.
+          {
+            key: 'Permissions-Policy',
+            value: 'camera=(self), microphone=(self), geolocation=(self)',
+          },
+        ],
+      },
       {
         source: '/share/status-report/:path*',
         headers: [

src/__tests__/features/integrations/quickbooks-server.test.ts

@@ -1,3 +1,4 @@
+import { realmRef } from '@/integrations/quickbooks/mapping'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type {
   AccountingCustomer,
@@ -93,7 +94,7 @@ function makeCtx(input: {
       state,
       externalAccountId: '9130357',
     },
-    credentials: { accessToken: 'tok', refreshToken: 'ref' },
+    credentials: { accessToken: 'tok', refreshToken: 'ref', realmId: '9130357' },
     http: {
       fetch: async () => {
         throw new Error('not used')
@@ -1025,7 +1026,8 @@ describe('QuickBooks: connecting', () => {
       },
     })
     const who = await connector.identify?.(t.ctx)
-    expect(who).toEqual({ id: '9130357', name: 'Sandbox Garage Ltd (sandbox)' })
+    expect(who).toEqual({ id: realmRef('9130357'), name: 'Sandbox Garage Ltd (sandbox)' })
+    expect(who?.id).not.toContain('9130357')
     expect(t.state).toMatchObject({
       environment: 'sandbox',
       country: 'GB',
@@ -1040,6 +1042,17 @@ describe('QuickBooks: connecting', () => {
     expect(t.calls.at(-1)?.host).toBe('sandbox-quickbooks.api.intuit.com')
   })
 
+  it('reads the company from the sealed credentials, falling back to older state', async () => {
+    const fresh = makeCtx({ state: { realmId: undefined }, answer: emptyCompany() })
+    await connector.jobs['accounting.invoice'](fresh.ctx, { entityId: 'svc1' })
+    expect(fresh.calls[0].path).toBe('/v3/company/9130357/query')
+
+    const legacy = makeCtx({ answer: emptyCompany() })
+    legacy.ctx.credentials = { accessToken: 'tok', refreshToken: 'ref' }
+    await connector.jobs['accounting.invoice'](legacy.ctx, { entityId: 'svc1' })
+    expect(legacy.calls[0].path).toBe('/v3/company/9130357/query')
+  })
+
   it('recognises automated sales tax and says when sales tax is off', async () => {
     const t = makeCtx({
       answer: (call) => {

src/__tests__/lib/safe-redirect.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it } from 'vitest'
+import { safeRedirectPath } from '@/lib/safe-redirect'
+
+describe('safeRedirectPath', () => {
+  it('keeps a path on this site, query string and all', () => {
+    expect(safeRedirectPath('/settings/integrations/quickbooks')).toBe(
+      '/settings/integrations/quickbooks'
+    )
+    expect(safeRedirectPath('/vehicles?tab=open#top')).toBe('/vehicles?tab=open#top')
+    expect(safeRedirectPath('  /settings  ')).toBe('/settings')
+  })
+
+  it('falls back for anything that would leave the site', () => {
+    for (const bad of [
+      'https://evil.example/',
+      'http://evil.example',
+      '//evil.example/path',
+      '/\\evil.example',
+      'javascript:alert(1)',
+      'settings',
+      '',
+      '/line\nbreak',
+    ]) {
+      expect(safeRedirectPath(bad)).toBe('/')
+    }
+  })
+
+  it('falls back to the given default when there is nothing usable', () => {
+    expect(safeRedirectPath(null, '/dashboard')).toBe('/dashboard')
+    expect(safeRedirectPath(undefined)).toBe('/')
+  })
+})

src/__tests__/lib/same-origin.test.ts

@@ -0,0 +1,48 @@
+import { describe, expect, it } from 'vitest'
+import { isCrossSiteWrite } from '@/lib/same-origin'
+
+function req(method: string, headers: Record<string, string>) {
+  const lower = Object.fromEntries(Object.entries(headers).map(([k, v]) => [k.toLowerCase(), v]))
+  return { method, header: (name: string) => lower[name.toLowerCase()] ?? null }
+}
+
+describe('isCrossSiteWrite', () => {
+  it('never blocks reads', () => {
+    expect(isCrossSiteWrite(req('GET', { origin: 'https://evil.example', host: 'app.x' }))).toBe(
+      false
+    )
+    expect(isCrossSiteWrite(req('HEAD', { 'sec-fetch-site': 'cross-site' }))).toBe(false)
+  })
+
+  it('trusts the browser when it says where the request came from', () => {
+    expect(isCrossSiteWrite(req('POST', { 'sec-fetch-site': 'cross-site' }))).toBe(true)
+    expect(isCrossSiteWrite(req('POST', { 'sec-fetch-site': 'same-origin' }))).toBe(false)
+    expect(isCrossSiteWrite(req('POST', { 'sec-fetch-site': 'same-site' }))).toBe(false)
+  })
+
+  it('compares Origin with the host the request was made to', () => {
+    const same = { origin: 'https://app.torqvoice.com', host: 'app.torqvoice.com' }
+    expect(isCrossSiteWrite(req('POST', same))).toBe(false)
+    expect(
+      isCrossSiteWrite(req('DELETE', { origin: 'https://evil.example', host: 'app.torqvoice.com' }))
+    ).toBe(true)
+    // Behind the proxy the public host arrives forwarded.
+    expect(
+      isCrossSiteWrite(
+        req('PATCH', {
+          origin: 'https://app.torqvoice.com',
+          host: 'torqvoice-app:3000',
+          'x-forwarded-host': 'app.torqvoice.com',
+        })
+      )
+    ).toBe(false)
+    expect(isCrossSiteWrite(req('POST', { origin: 'null', host: 'app.torqvoice.com' }))).toBe(true)
+    expect(isCrossSiteWrite(req('POST', { origin: 'not a url', host: 'app.torqvoice.com' }))).toBe(
+      true
+    )
+  })
+
+  it('lets requests without an Origin through, as native clients send none', () => {
+    expect(isCrossSiteWrite(req('POST', { host: 'app.torqvoice.com' }))).toBe(false)
+  })
+})

src/app/(public)/auth/sign-in/sign-in-form.tsx

@@ -1,6 +1,7 @@
 'use client'
 
 import { Suspense, useEffect, useRef, useState } from 'react'
+import { safeRedirectPath } from '@/lib/safe-redirect'
 import { useRouter, useSearchParams } from 'next/navigation'
 import Link from 'next/link'
 import { useTranslations } from 'next-intl'
@@ -50,8 +51,7 @@ function SignInFormInner({
         setPassword('')
         passwordRef.current?.focus()
       } else {
-        const redirect = searchParams.get('redirect') || '/'
-        router.push(redirect)
+        router.push(safeRedirectPath(searchParams.get('redirect')))
         router.refresh()
       }
     } catch {
@@ -100,8 +100,7 @@ function SignInFormInner({
         const msg = typeof result.error.message === 'string' ? result.error.message : ''
         setError(msg || t('errors.passkeyFailed'))
       } else {
-        const redirect = searchParams.get('redirect') || '/'
-        router.push(redirect)
+        router.push(safeRedirectPath(searchParams.get('redirect')))
         router.refresh()
       }
     } catch {
@@ -234,7 +233,7 @@ function SignInFormInner({
         <p className="mt-6 text-center text-sm text-muted-foreground">
           {t('noAccount')}{' '}
           <Link
-            href={`/auth/sign-up${searchParams.get('redirect') ? `?redirect=${encodeURIComponent(searchParams.get('redirect')!)}` : ''}`}
+            href={`/auth/sign-up${searchParams.get('redirect') ? `?redirect=${encodeURIComponent(safeRedirectPath(searchParams.get('redirect')))}` : ''}`}
             className="font-medium text-primary hover:underline"
           >
             {t('createOne')}

src/app/(public)/auth/sign-up/page.tsx

@@ -1,4 +1,5 @@
 import { redirect } from 'next/navigation'
+import { safeRedirectPath } from '@/lib/safe-redirect'
 import { headers } from 'next/headers'
 import { db } from '@/lib/db'
 import { auth } from '@/lib/auth'
@@ -14,7 +15,7 @@ export default async function SignUpPage({
 }) {
   const params = await searchParams
   const inviteToken = params.invite
-  const redirectTo = params.redirect
+  const redirectTo = params.redirect ? safeRedirectPath(params.redirect) : undefined
 
   // If already authenticated, redirect to the target or home
   const session = await auth.api.getSession({ headers: await headers() })

src/app/(public)/auth/sign-up/sign-up-form.tsx

@@ -1,5 +1,6 @@
 'use client'
 
+import { safeRedirectPath } from '@/lib/safe-redirect'
 import { useRef, useState } from 'react'
 import { useRouter } from 'next/navigation'
 import Link from 'next/link'
@@ -73,7 +74,7 @@ export function SignUpForm({
         const acceptResult = await acceptInvitation({ token: inviteToken })
         if (acceptResult.success) {
           // Invited users have a known email — skip verification, go straight to dashboard
-          router.push(redirectTo || '/')
+          router.push(safeRedirectPath(redirectTo))
           router.refresh()
         } else {
           setError(acceptResult.error || t('errors.invitationFailed'))

src/app/(public)/onboarding/page.tsx

@@ -1,5 +1,6 @@
 import { headers } from 'next/headers'
 import { redirect } from 'next/navigation'
+import { safeRedirectPath } from '@/lib/safe-redirect'
 import { auth } from '@/lib/auth'
 import { db } from '@/lib/db'
 import { OnboardingForm } from '@/features/onboarding/Components/OnboardingForm'
@@ -10,7 +11,7 @@ export default async function OnboardingPage({
   searchParams: Promise<{ redirect?: string }>
 }) {
   const params = await searchParams
-  const redirectTo = params.redirect
+  const redirectTo = params.redirect ? safeRedirectPath(params.redirect) : undefined
   const session = await auth.api.getSession({ headers: await headers() })
 
   if (!session?.user?.id) {

src/app/api/integrations/[connector]/oauth/callback/route.ts

@@ -80,24 +80,29 @@ export async function GET(
   }
 
   // What the vendor tells us only here, such as which company was chosen,
-  // goes on the state so identify and every job after it can read it.
+  // goes into the sealed credentials, next to the tokens, so identify and
+  // every job after it can read it and it never sits in the database in the
+  // clear: Intuit's rules put the realm id under the same encryption as the
+  // refresh token. A token refresh spreads the previous credentials, so it
+  // survives those.
   const extra: Record<string, string> = {}
   for (const name of spec.callbackParams ?? []) {
     const value = params.get(name)
     if (value) extra[name] = value
   }
-  const state0 = (connection.state as Record<string, unknown>) ?? {}
 
   await db.integrationConnection.update({
     where: { id: connection.id },
     data: {
       oauthState: null,
-      credentials: sealCredentials(credentials as unknown as Record<string, unknown>),
+      credentials: sealCredentials({
+        ...(credentials as unknown as Record<string, unknown>),
+        ...extra,
+      }),
       scopes: credentials.scope ?? spec.scopes.join(' '),
       status: 'active',
       lastError: null,
       lastHealthAt: new Date(),
-      ...(Object.keys(extra).length > 0 && { state: { ...state0, ...extra } as object }),
     },
   })
 

src/features/integrations/Lib/types.ts

@@ -59,9 +59,11 @@ export type AuthSpec =
        */
       tokenAuth?: 'body' | 'basic'
       /**
-       * Query parameters the vendor adds to the callback beside code and
-       * state, kept on the connection's state under the same names. Intuit
-       * sends the company id (realmId) this way and nowhere else.
+       * Query parameters the vendor adds to the callback besides code and
+       * state, kept inside the sealed credentials under the same names, next
+       * to the tokens, so they are encrypted at rest and survive a token
+       * refresh. Intuit sends the company id (realmId) this way and nowhere
+       * else.
        */
       callbackParams?: string[]
       /** Environment variable names holding the platform-owned app's client id and secret. */

src/features/onboarding/Components/OnboardingForm.tsx

@@ -1,5 +1,6 @@
 'use client'
 
+import { safeRedirectPath } from '@/lib/safe-redirect'
 import { useState } from 'react'
 import { useRouter } from 'next/navigation'
 import { useTranslations } from 'next-intl'
@@ -34,7 +35,7 @@ export function OnboardingForm({ redirectTo }: { redirectTo?: string }) {
       if (!result.success) {
         modal.open('error', t('setupFailed'), result.error || t('couldNotCreate'))
       } else {
-        router.push(redirectTo || '/')
+        router.push(safeRedirectPath(redirectTo))
         router.refresh()
       }
     } catch {

src/integrations/quickbooks/mapping.ts

@@ -7,6 +7,7 @@
  * that the ledger arrived at the same total.
  */
 
+import { createHash } from 'node:crypto'
 import type {
   AccountingCustomer,
   AccountingInvoice,
@@ -33,6 +34,11 @@ export const NOTES_MAX = 2000
 
 export type Environment = 'production' | 'sandbox'
 
+/** A stable, non-reversible reference for a company id, for rows the browser sees. */
+export function realmRef(realm: string): string {
+  return `qbo-${createHash('sha256').update(realm).digest('hex').slice(0, 16)}`
+}
+
 export interface QboRef {
   value: string
   name?: string

src/integrations/quickbooks/server.ts

@@ -37,6 +37,7 @@ import {
   type QboPayment,
   type QboTaxCode,
   type QboTaxRate,
+  realmRef,
   REVOKE_URL,
   WALK_IN_NAME,
   apiHost,
@@ -83,6 +84,7 @@ class QboError extends Error {
 }
 
 interface State {
+  /** Legacy: newer connections keep the realm in the sealed credentials. */
   realmId: string
   environment: Environment
   country: string | null
@@ -144,8 +146,14 @@ function settingsOf(ctx: ConnectorContext) {
   }
 }
 
+/**
+ * The company id. It arrives on the OAuth callback and is kept in the
+ * sealed credentials next to the tokens. Connections made before that was
+ * so still carry it on the state; they lose it the next time they connect.
+ */
 function realmOf(ctx: ConnectorContext): string {
-  const realm = stateOf(ctx).realmId
+  const sealed = ctx.credentials.realmId
+  const realm = typeof sealed === 'string' && sealed ? sealed : stateOf(ctx).realmId
   if (!realm) throw new Error('No QuickBooks company on this connection; reconnect')
   return realm
 }
@@ -934,7 +942,12 @@ export const connector: ConnectorServer = {
         'Custom transaction numbers are off in this QuickBooks company, so invoices there get QuickBooks numbers instead of the Torqvoice invoice numbers. Turn them on under Account and settings, Sales, Sales form content.'
       )
     }
-    return { id: realm, name: env === 'sandbox' ? `${company.name} (sandbox)` : company.name }
+    // The realm is customer-identifying, so the row and the browser get a
+    // reference derived from it, enough to tell two companies apart.
+    return {
+      id: realmRef(realm),
+      name: env === 'sandbox' ? `${company.name} (sandbox)` : company.name,
+    }
   },
   async test(ctx) {
     try {

src/lib/safe-redirect.ts

@@ -0,0 +1,18 @@
+/**
+ * A redirect target taken from a query string, kept on this site.
+ *
+ * Sign-in, sign-up and onboarding carry the page a visitor was heading for in
+ * a `redirect` parameter and push it once they are through. Anything that is
+ * not a plain path on this origin is thrown away: an absolute URL would send
+ * the freshly signed-in person to someone else's site, and browsers read a
+ * leading `//` or `/\\` as protocol-relative, so those count as absolute too.
+ */
+export function safeRedirectPath(value: string | null | undefined, fallback = '/'): string {
+  if (typeof value !== 'string') return fallback
+  const path = value.trim()
+  if (!/^\/(?![/\\])/.test(path)) return fallback
+  // Control characters have no place in a path and are how header splitting starts.
+  // biome-ignore lint/suspicious/noControlCharactersInRegex: that is the point of the check
+  if (/[\u0000-\u001f\u007f]/.test(path)) return fallback
+  return path
+}

src/lib/same-origin.ts

@@ -0,0 +1,31 @@
+/**
+ * Whether a state-changing request came from another site.
+ *
+ * The cookie-authenticated API under /api/protected is called by our own
+ * pages, so a write arriving from someone else's origin is never legitimate.
+ * SameSite=Lax cookies already keep a cross-site POST from carrying the
+ * session, so this is a second wall, and one a security scanner can see.
+ *
+ * Reads are left alone, as are requests that carry neither header: native
+ * clients and server-to-server callers send no Origin, and they are not the
+ * problem a cross-site request forgery poses.
+ */
+const SAFE_METHODS = new Set(['GET', 'HEAD', 'OPTIONS'])
+
+export function isCrossSiteWrite(input: {
+  method: string
+  header: (name: string) => string | null
+}): boolean {
+  if (SAFE_METHODS.has(input.method.toUpperCase())) return false
+  const site = input.header('sec-fetch-site')
+  if (site === 'cross-site') return true
+  const origin = input.header('origin')
+  if (!origin || origin === 'null') return origin === 'null'
+  const host = input.header('x-forwarded-host') ?? input.header('host')
+  if (!host) return false
+  try {
+    return new URL(origin).host.toLowerCase() !== host.split(',')[0].trim().toLowerCase()
+  } catch {
+    return true
+  }
+}

src/proxy.ts

@@ -1,5 +1,6 @@
 import { NextResponse } from 'next/server'
 import type { NextRequest } from 'next/server'
+import { isCrossSiteWrite } from '@/lib/same-origin'
 
 /**
  * Cross-origin access to the technician API, in development only.
@@ -42,7 +43,18 @@ function applyCors(response: NextResponse, origin: string | null) {
   return response
 }
 
+const PROTECTED_API = '/api/protected/'
+
 export default function proxy(request: NextRequest) {
+  // A write to the cookie-authenticated API from another site is refused
+  // before any handler runs. See lib/same-origin.ts.
+  if (request.nextUrl.pathname.startsWith(PROTECTED_API)) {
+    if (isCrossSiteWrite({ method: request.method, header: (n) => request.headers.get(n) })) {
+      return NextResponse.json({ error: 'Cross-site request refused' }, { status: 403 })
+    }
+    return undefined
+  }
+
   if (!IS_DEV) return undefined
 
   const origin = request.headers.get('origin')
@@ -65,5 +77,7 @@ export const config = {
     // Sign-in lives here, so the app hits it before it has a session and
     // would otherwise fail its very first request from a browser.
     '/api/public/auth/:path*',
+    // The cookie-authenticated API: cross-site writes are refused here.
+    '/api/protected/:path*',
   ],
 }