Fix/compliance and invoices (#433)

* feat(invoices): implement inline membership checks for invoice-number RPCs and enhance error handling

* fix(invoices): enhance unpaid amount calculation to support currency-specific rounding

* fix(api-client): handle response.text() error for 403 status in skvRequest

app/(dashboard)/invoices/page.tsx

@@ -124,7 +124,12 @@ export default function InvoicesPage() {
     unpaid: invoices.filter(isOutstandingReceivable).length,
     unpaidAmount: invoices
       .filter(isOutstandingReceivable)
-      .reduce((sum, i) => sum + Number(i.total_sek || i.total), 0),
+      .reduce((sum, i) => {
+        if (i.currency === 'SEK') {
+          return sum + getDisplayTotal({ total: Number(i.total), currency: 'SEK' }, { ore_rounding: oreRounding }).displayed
+        }
+        return sum + Number(i.total_sek || i.total)
+      }, 0),
     overdue: invoices.filter((i) => i.status === 'overdue' && !i.credited_invoice_id).length,
   }
 

app/api/sandbox/seed/route.ts

@@ -1,25 +1,45 @@
+import crypto from 'crypto'
 import { createClient } from '@/lib/supabase/server'
 import { NextResponse } from 'next/server'
 import { getActiveCompanyId } from '@/lib/company/context'
 import { createLogger } from '@/lib/logger'
 
-const log = createLogger('sandbox:seed')
-
 /**
  * POST /api/sandbox/seed
  * Seeds demo data for an anonymous sandbox user.
  * Only callable by anonymous users (is_anonymous === true).
+ *
+ * Defense-in-depth: also requires SANDBOX_ENABLED=true. The anonymous-user
+ * check is the primary control; the env guard exists so that if anonymous
+ * sign-in is ever turned on accidentally in a production environment, this
+ * destructive seed endpoint stays inert until an operator explicitly opts in.
  */
 export async function POST() {
+  // Per-request logger so seed-failure entries are correlatable in the SIEM.
+  // Cannot reuse withRouteContext here — it requires an active company, but
+  // the sandbox seed runs *before* a company exists for the user.
+  const requestId = `req_${crypto.randomUUID()}`
+  const log = createLogger('sandbox:seed', { requestId })
+
+  if (process.env.SANDBOX_ENABLED !== 'true') {
+    return NextResponse.json(
+      { error: 'Sandbox is not enabled in this environment', requestId },
+      { status: 403 },
+    )
+  }
+
   const supabase = await createClient()
   const { data: { user } } = await supabase.auth.getUser()
 
   if (!user) {
-    return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
+    return NextResponse.json({ error: 'Unauthorized', requestId }, { status: 401 })
   }
 
   if (!user.is_anonymous) {
-    return NextResponse.json({ error: 'Sandbox is only available for anonymous users' }, { status: 403 })
+    return NextResponse.json(
+      { error: 'Sandbox is only available for anonymous users', requestId },
+      { status: 403 },
+    )
   }
 
   // Anonymous users start with no company. Create one before seeding.
@@ -39,7 +59,7 @@ export async function POST() {
     if (companyError || !newCompanyId) {
       log.error('failed to create sandbox company', { error: companyError, userId: user.id })
       return NextResponse.json(
-        { error: 'Failed to create sandbox company' },
+        { error: 'Failed to create sandbox company', requestId },
         { status: 500 }
       )
     }
@@ -552,7 +572,7 @@ export async function POST() {
   } catch (err) {
     log.error('failed to seed sandbox data', { error: err, userId: user.id, companyId })
     return NextResponse.json(
-      { error: 'Failed to seed sandbox data' },
+      { error: 'Failed to seed sandbox data', requestId },
       { status: 500 }
     )
   }

extensions/general/skatteverket/__tests__/api-client.test.ts

@@ -55,7 +55,7 @@ describe('skvRequest — error mapping', () => {
     }
   })
 
-  it('maps 401 with body text → SESSION_EXPIRED and includes body', async () => {
+  it('maps 401 with body text → SESSION_EXPIRED with a clean Swedish message (no body leak)', async () => {
     mockFetchStatus(401, 'token expired')
     try {
       await skvRequest(fakeSupabase, 'user-1', 'GET', '/x')
@@ -63,7 +63,10 @@ describe('skvRequest — error mapping', () => {
     } catch (e) {
       expect(e).toBeInstanceOf(SkatteverketAuthError)
       expect((e as SkatteverketAuthError).code).toBe('SESSION_EXPIRED')
-      expect((e as SkatteverketAuthError).message).toContain('token expired')
+      expect((e as SkatteverketAuthError).message).toMatch(/Sessionen har gått ut/)
+      // Audit V16.1: the raw response body must NOT be concatenated into the
+      // user-facing message — that information stays in server-side logs.
+      expect((e as SkatteverketAuthError).message).not.toContain('token expired')
     }
   })
 

extensions/general/skatteverket/lib/api-client.ts

@@ -1,9 +1,27 @@
 import crypto from 'crypto'
 import type { SupabaseClient } from '@supabase/supabase-js'
+import { createLogger } from '@/lib/logger'
 import { refreshAccessToken } from './oauth'
 import { getTokens, storeTokens, deleteTokens } from './token-store'
 import type { SkatteverketTokens } from '../types'
 
+const log = createLogger('skatteverket-api-client')
+
+// Cap diagnostic-body logging at 200 chars and redact any Bearer token
+// patterns. The audit (V16.1 / A.8.15) flagged that raw 401/403 bodies were
+// being concatenated into user-facing error messages and written to logs
+// without redaction. Diagnostic data still belongs in server-side logs, but
+// not in unbounded form and not in anything that reaches the user.
+const MAX_LOG_BODY_LEN = 200
+const BEARER_PATTERN = /\bBearer\s+[A-Za-z0-9\-._~+/=]+/gi
+
+function safeBodyForLog(body: string): string {
+  const redacted = body.replace(BEARER_PATTERN, 'Bearer [REDACTED]')
+  return redacted.length > MAX_LOG_BODY_LEN
+    ? redacted.slice(0, MAX_LOG_BODY_LEN) + '…'
+    : redacted
+}
+
 /**
  * Skatteverket API client.
  *
@@ -235,15 +253,17 @@ export async function skvRequest(
         skvHeaders[k] = v
       }
     })
-    console.error('[skatteverket] 401 from API', { url, body: text, headers: skvHeaders })
+    // Diagnostic detail (headers, body) belongs in server-side logs only —
+    // not in user-facing error messages. The structured logger redacts
+    // sensitive keys and we further cap body length and strip Bearer tokens
+    // so the diagnostic is bounded.
+    log.error('401 from Skatteverket API', {
+      url,
+      statusCode: 401,
+      body: safeBodyForLog(text),
+      headers: skvHeaders,
+    })
 
-    // (A) Surface SKV's WWW-Authenticate verbatim — when the body is empty
-    // this header is usually the only diagnostic SKV gives us. Carry both
-    // header and body into every thrown message below.
-    const headerSuffix = Object.keys(skvHeaders).length > 0
-      ? ` Headers: ${JSON.stringify(skvHeaders)}`
-      : ''
-    const bodySuffix = text ? ` Svar: ${text}` : ''
     const lower = text.toLowerCase()
 
     // OAuth's standard insufficient_scope marker. SKV sometimes emits this
@@ -258,8 +278,7 @@ export async function skvRequest(
       throw new SkatteverketAuthError(
         'Anslutningen mot Skatteverket saknar nödvändig behörighet för denna ' +
         'tjänst. Koppla bort och anslut igen via Inställningar → Skatteverket ' +
-        'för att förnya tokenen med rätt scope.' +
-        headerSuffix + bodySuffix,
+        'för att förnya tokenen med rätt scope.',
         'MISSING_SCOPE'
       )
     }
@@ -277,13 +296,12 @@ export async function skvRequest(
       try {
         await deleteTokens(supabase, userId)
       } catch (cleanupErr) {
-        console.error('[skatteverket] failed to clear revoked token row', cleanupErr)
+        log.error('failed to clear revoked token row', cleanupErr as Error, { userId })
       }
       throw new SkatteverketAuthError(
         'Skatteverket har återkallat anslutningen. Detta händer t.ex. om ' +
         'BankID-sessionen avslutats eller om en ny anslutning gjorts från ' +
-        'en annan enhet. Anslut igen med BankID för att fortsätta.' +
-        headerSuffix + bodySuffix,
+        'en annan enhet. Anslut igen med BankID för att fortsätta.',
         'TOKEN_REVOKED'
       )
     }
@@ -303,9 +321,7 @@ export async function skvRequest(
       throw new SkatteverketAuthError(
         'Skatteverkets API-gateway nekade anropet. Kontrollera att din ' +
         'APIGW-klient (SKATTEVERKET_APIGW_CLIENT_ID) har prenumeration på ' +
-        'denna tjänst i Utvecklarportalen.' +
-        headerSuffix +
-        ` Svar från Skatteverket: ${text || '(tomt svar)'}`,
+        'denna tjänst i Utvecklarportalen.',
         'ACCESS_DENIED'
       )
     }
@@ -337,19 +353,26 @@ export async function skvRequest(
         `inte prenumeration på tjänsten "${apiHint}" i Utvecklarportalen, ` +
         'eller den lagrade tokenen saknar rätt scope. Kontrollera ' +
         'prenumerationen, koppla annars bort och anslut igen via ' +
-        'Inställningar → Skatteverket.' + headerSuffix,
+        'Inställningar → Skatteverket.',
         'ACCESS_DENIED'
       )
     }
 
     throw new SkatteverketAuthError(
-      `Sessionen har gått ut. Logga in med BankID igen.${headerSuffix}${bodySuffix}`,
+      'Sessionen har gått ut. Logga in med BankID igen.',
       'SESSION_EXPIRED'
     )
   }
 
   if (response.status === 403) {
-    const text = await response.text()
+    const text = await response.text().catch(() => '')
+    // Same diagnostic-vs-user-message split as the 401 path: log the body
+    // server-side, surface only the actionable Swedish guidance.
+    log.error('403 from Skatteverket API', {
+      url,
+      statusCode: 403,
+      body: safeBodyForLog(text),
+    })
     // Missing scope on the access token — fires when an existing connection
     // pre-dates an extension that needed a new scope (the AGI/`agd` rollout
     // is the canonical example). The user has to disconnect + reconnect to
@@ -375,7 +398,7 @@ export async function skvRequest(
       )
     }
     throw new SkatteverketAuthError(
-      `Åtkomst nekad av Skatteverket (403): ${text}`,
+      'Åtkomst nekad av Skatteverket (403). Kontakta support om problemet kvarstår.',
       'ACCESS_DENIED'
     )
   }

lib/invoices/__tests__/invoice-number-authorization.pg.test.ts

@@ -0,0 +1,134 @@
+import { randomUUID } from 'node:crypto'
+import { describe, expect, it } from 'vitest'
+import { getPool, withUserContext } from '@/tests/pg/setup'
+import { seedCompany } from '@/tests/pg/fixtures'
+
+// Authorization tests for the invoice-number RPCs added in
+// 20260510140000_harden_invoice_number_rpcs.sql. The functions are
+// SECURITY DEFINER, so they bypass caller RLS. The migration added an inline
+// auth.uid() membership check as defense-in-depth; these tests prove it.
+
+async function ensureCompanySettings(params: {
+  userId: string
+  companyId: string
+  invoicePrefix?: string
+  nextInvoiceNumber?: number
+}): Promise<void> {
+  await getPool().query(
+    `INSERT INTO public.company_settings
+       (user_id, company_id, invoice_prefix, next_invoice_number)
+     VALUES ($1, $2, $3, $4)
+     ON CONFLICT (company_id) DO UPDATE
+       SET invoice_prefix = EXCLUDED.invoice_prefix,
+           next_invoice_number = EXCLUDED.next_invoice_number`,
+    [params.userId, params.companyId, params.invoicePrefix ?? 'F', params.nextInvoiceNumber ?? 1],
+  )
+}
+
+async function insertDraftInvoice(params: {
+  userId: string
+  companyId: string
+}): Promise<string> {
+  const customerId = randomUUID()
+  await getPool().query(
+    `INSERT INTO public.customers (id, user_id, company_id, name)
+     VALUES ($1, $2, $3, 'Test Customer')`,
+    [customerId, params.userId, params.companyId],
+  )
+
+  const invoiceId = randomUUID()
+  await getPool().query(
+    `INSERT INTO public.invoices
+       (id, user_id, company_id, customer_id, invoice_number, document_type,
+        invoice_date, due_date, currency, subtotal, vat_amount, total,
+        vat_treatment, vat_rate, moms_ruta, status)
+     VALUES ($1, $2, $3, $4, NULL, 'invoice',
+             '2026-04-27', '2026-05-27', 'SEK', 1000, 250, 1250,
+             'standard_25', 25, '10', 'draft')`,
+    [invoiceId, params.userId, params.companyId, customerId],
+  )
+  return invoiceId
+}
+
+describe('invoice-number RPCs — authorization (defense in depth)', () => {
+  it('peek_next_invoice_number raises when caller is not a member of the target company', async () => {
+    const intruder = await seedCompany()
+    const target = await seedCompany()
+    await ensureCompanySettings({ userId: target.userId, companyId: target.companyId })
+
+    await expect(
+      withUserContext(intruder.userId, async (client) => {
+        await client.query('SELECT public.peek_next_invoice_number($1, $2)', [
+          target.companyId,
+          'invoice',
+        ])
+      }),
+    ).rejects.toThrow(/unauthorized/)
+  })
+
+  it('peek_next_invoice_number succeeds for a member of the target company', async () => {
+    const { userId, companyId } = await seedCompany()
+    await ensureCompanySettings({ userId, companyId, invoicePrefix: 'F', nextInvoiceNumber: 7 })
+
+    const preview = await withUserContext(userId, async (client) => {
+      const { rows } = await client.query<{ peek_next_invoice_number: string }>(
+        'SELECT public.peek_next_invoice_number($1, $2)',
+        [companyId, 'invoice'],
+      )
+      return rows[0]!.peek_next_invoice_number
+    })
+
+    expect(preview).toBe('F007')
+  })
+
+  it('generate_invoice_number raises when caller is not a member of the target company', async () => {
+    const intruder = await seedCompany()
+    const target = await seedCompany()
+    await ensureCompanySettings({ userId: target.userId, companyId: target.companyId })
+    const invoiceId = await insertDraftInvoice({
+      userId: target.userId,
+      companyId: target.companyId,
+    })
+
+    await expect(
+      withUserContext(intruder.userId, async (client) => {
+        await client.query('SELECT public.generate_invoice_number($1, $2, $3)', [
+          target.companyId,
+          invoiceId,
+          'invoice',
+        ])
+      }),
+    ).rejects.toThrow(/unauthorized/)
+  })
+
+  it('generate_invoice_number succeeds for a member of the target company', async () => {
+    const { userId, companyId } = await seedCompany()
+    await ensureCompanySettings({ userId, companyId, invoicePrefix: 'G', nextInvoiceNumber: 42 })
+    const invoiceId = await insertDraftInvoice({ userId, companyId })
+
+    const assigned = await withUserContext(userId, async (client) => {
+      const { rows } = await client.query<{ generate_invoice_number: string }>(
+        'SELECT public.generate_invoice_number($1, $2, $3)',
+        [companyId, invoiceId, 'invoice'],
+      )
+      return rows[0]!.generate_invoice_number
+    })
+
+    expect(assigned).toBe('G042')
+  })
+
+  it('superuser (no JWT context) bypasses the membership check', async () => {
+    // Service role / cron / pg-real seed paths run without a JWT. The
+    // membership check is intentionally skipped when auth.uid() IS NULL —
+    // calling the RPC directly on the pool (no withUserContext) must still
+    // succeed.
+    const { userId, companyId } = await seedCompany()
+    await ensureCompanySettings({ userId, companyId, invoicePrefix: 'F', nextInvoiceNumber: 1 })
+
+    const { rows } = await getPool().query<{ peek_next_invoice_number: string }>(
+      'SELECT public.peek_next_invoice_number($1, $2)',
+      [companyId, 'invoice'],
+    )
+    expect(rows[0]!.peek_next_invoice_number).toBe('F001')
+  })
+})

supabase/migrations/20260510140000_harden_invoice_number_rpcs.sql

@@ -0,0 +1,128 @@
+-- Harden invoice-number RPCs with inline membership check + empty search_path.
+--
+-- Compliance hardening triggered by external audit (OWASP V8.2.1, SOC 2 CC6.1,
+-- ISO 27001 A.8.28).
+--
+-- generate_invoice_number and peek_next_invoice_number are SECURITY DEFINER,
+-- which means they execute with elevated privileges and bypass the caller's
+-- RLS. They are reached from the /api/invoices/next-number route, which
+-- already validates company membership at the application layer
+-- (lib/company/context.ts → withRouteContext). This migration adds two layers
+-- of defense-in-depth at the DB layer:
+--
+--   1. Inline membership check using auth.uid() against public.company_members.
+--      Skipped when auth.uid() is NULL (service role / cron / pg-real
+--      superuser tests that bypass JWT context). This mirrors how Supabase's
+--      service role bypasses RLS by design.
+--
+--   2. SET search_path = '' (was 'public'). All table/function references
+--      below are now schema-qualified so they cannot be hijacked by an
+--      object created in another schema later on the search path.
+--      pg_catalog is searched implicitly so built-in functions (LPAD,
+--      GREATEST, COALESCE, length, now) still resolve without qualification.
+--
+-- Behaviour for the LPAD / proforma / GREATEST(3, length) logic is preserved
+-- verbatim from 20260510130000_invoice_number_no_truncate.sql.
+
+CREATE OR REPLACE FUNCTION public.generate_invoice_number(
+  p_company_id uuid,
+  p_invoice_id uuid,
+  p_document_type text DEFAULT 'invoice'
+)
+RETURNS text
+LANGUAGE plpgsql
+SECURITY DEFINER
+SET search_path = ''
+AS $function$
+DECLARE
+  v_existing text;
+  v_prefix text;
+  v_number integer;
+  v_final text;
+BEGIN
+  -- Defense-in-depth: refuse to operate on companies the caller is not a
+  -- member of. NULL auth.uid() (service role / cron / superuser) is allowed
+  -- through; those code paths are trusted and need to operate across tenants.
+  IF auth.uid() IS NOT NULL AND NOT EXISTS (
+    SELECT 1 FROM public.company_members
+    WHERE user_id = auth.uid() AND company_id = p_company_id
+  ) THEN
+    RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id
+      USING ERRCODE = '42501';
+  END IF;
+
+  SELECT invoice_number INTO v_existing
+  FROM public.invoices
+  WHERE id = p_invoice_id AND company_id = p_company_id
+  FOR UPDATE;
+
+  IF NOT FOUND THEN
+    RAISE EXCEPTION 'Invoice % not found in company %', p_invoice_id, p_company_id;
+  END IF;
+
+  IF v_existing IS NOT NULL THEN
+    RETURN v_existing;
+  END IF;
+
+  UPDATE public.company_settings
+  SET next_invoice_number = next_invoice_number + 1,
+      updated_at = now()
+  WHERE company_id = p_company_id
+  RETURNING invoice_prefix, next_invoice_number - 1
+  INTO v_prefix, v_number;
+
+  IF v_number IS NULL THEN
+    RAISE EXCEPTION 'Company settings not found for company %', p_company_id;
+  END IF;
+
+  v_final := CASE
+    WHEN p_document_type = 'proforma' THEN 'PF-'
+    ELSE COALESCE(v_prefix, '')
+  END || LPAD(v_number::text, GREATEST(3, length(v_number::text)), '0');
+
+  UPDATE public.invoices
+  SET invoice_number = v_final
+  WHERE id = p_invoice_id AND company_id = p_company_id;
+
+  RETURN v_final;
+END;
+$function$;
+
+CREATE OR REPLACE FUNCTION public.peek_next_invoice_number(
+  p_company_id uuid,
+  p_document_type text DEFAULT 'invoice'
+)
+RETURNS text
+LANGUAGE plpgsql
+STABLE
+SECURITY DEFINER
+SET search_path = ''
+AS $function$
+DECLARE
+  v_prefix text;
+  v_number integer;
+BEGIN
+  IF auth.uid() IS NOT NULL AND NOT EXISTS (
+    SELECT 1 FROM public.company_members
+    WHERE user_id = auth.uid() AND company_id = p_company_id
+  ) THEN
+    RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id
+      USING ERRCODE = '42501';
+  END IF;
+
+  SELECT invoice_prefix, next_invoice_number INTO v_prefix, v_number
+  FROM public.company_settings
+  WHERE company_id = p_company_id;
+
+  IF v_number IS NULL THEN
+    RETURN NULL;
+  END IF;
+
+  RETURN CASE
+           WHEN p_document_type = 'proforma' THEN 'PF-'
+           ELSE COALESCE(v_prefix, '')
+         END || LPAD(v_number::text, GREATEST(3, length(v_number::text)), '0');
+END;
+$function$;
+
+NOTIFY pgrst, 'reload schema';