Bug/mcp connection issue (#541)

* feat(api): implement caching and logging in health check endpoint

- Added in-memory caching for health check responses to reduce load on Postgres.
- Introduced logging for error handling in health check.
- Updated response structure to exclude error details from public responses.

feat(api): enhance OAuth consent UI and scope handling

- Improved consent UI to reflect exact requested scopes and added better user guidance.
- Updated scope handling logic to ensure least-privilege access.
- Enhanced styling for better user experience and accessibility.

chore(docker): improve security and resource management in Docker setup

- Updated Docker Compose configuration to enforce read-only file systems and resource limits.
- Added health checks and logging options for better observability.
- Introduced optional Caddy reverse proxy for TLS termination.

fix(migrations): resolve ambiguity in create_company_with_owner function

- Dropped orphaned 3-arg overload of create_company_with_owner function.
- Recreated canonical 4-arg version with cash account seeding logic.
- Ensured proper permissions for function execution in Postgres.

* feat: enhance security checks for team membership in company creation

* test: add CSP tests for OAuth authorization endpoint

* feat: enhance error handling and reporting in bank file import process

app/api/import/bank-file/execute/route.ts

@@ -99,6 +99,21 @@ export const POST = withRouteContext(
       if (role === 'viewer') ingestOptions.rawInsertOnly = true
       const ingestResult = await ingestTransactions(supabase, companyId, user.id, rawTransactions, ingestOptions)
 
+      if (ingestResult.errors > 0 && ingestResult.first_error) {
+        opLog.error('bank file ingest reported insert errors', new Error(ingestResult.first_error.message), {
+          errorCount: ingestResult.errors,
+          code: ingestResult.first_error.code,
+          details: ingestResult.first_error.details,
+          hint: ingestResult.first_error.hint,
+        })
+      }
+
+      const errorMessage = ingestResult.errors > 0
+        ? ingestResult.first_error
+          ? `${ingestResult.errors} fel: ${ingestResult.first_error.message}${ingestResult.first_error.details ? ` (${ingestResult.first_error.details})` : ''}`
+          : `${ingestResult.errors} transactions failed to import`
+        : null
+
       await supabase
         .from('bank_file_imports')
         .update({
@@ -106,9 +121,7 @@ export const POST = withRouteContext(
           duplicate_count: ingestResult.duplicates,
           matched_count: ingestResult.auto_matched_invoices,
           status: ingestResult.errors > 0 && ingestResult.imported === 0 ? 'failed' : 'completed',
-          error_message: ingestResult.errors > 0
-            ? `${ingestResult.errors} transactions failed to import`
-            : null,
+          error_message: errorMessage,
         })
         .eq('id', importRecord.id)
 

app/api/mcp-oauth/authorize/__tests__/route.test.ts

@@ -0,0 +1,185 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+
+const mocks = vi.hoisted(() => ({
+  createClient: vi.fn(),
+  isAllowedRedirectUri: vi.fn(),
+  requireCompanyId: vi.fn(),
+  getBranding: vi.fn(),
+}))
+
+vi.mock('@/lib/supabase/server', () => ({
+  createClient: () => mocks.createClient(),
+}))
+
+vi.mock('@/lib/auth/oauth-allowlist', () => ({
+  isAllowedRedirectUri: (...args: unknown[]) => mocks.isAllowedRedirectUri(...args),
+}))
+
+vi.mock('@/lib/company/context', () => ({
+  requireCompanyId: (...args: unknown[]) => mocks.requireCompanyId(...args),
+}))
+
+vi.mock('@/lib/branding/service', () => ({
+  getBranding: () => mocks.getBranding(),
+}))
+
+import { GET } from '../route'
+
+function buildAuthorizeUrl(params: Record<string, string>): string {
+  const url = new URL('http://localhost/api/mcp-oauth/authorize')
+  Object.entries(params).forEach(([k, v]) => url.searchParams.set(k, v))
+  return url.toString()
+}
+
+function buildSupabase(user: { id: string } | null, companyName = 'Test AB') {
+  return {
+    auth: { getUser: vi.fn().mockResolvedValue({ data: { user }, error: null }) },
+    from: vi.fn().mockReturnValue({
+      select: vi.fn().mockReturnValue({
+        eq: vi.fn().mockReturnValue({
+          single: vi.fn().mockResolvedValue({
+            data: { company_name: companyName },
+            error: null,
+          }),
+        }),
+      }),
+    }),
+  }
+}
+
+describe('GET /api/mcp-oauth/authorize — CSP', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    process.env.SUPABASE_SERVICE_ROLE_KEY = 'test-service-key'
+    mocks.createClient.mockResolvedValue(buildSupabase({ id: 'user-1' }))
+    mocks.isAllowedRedirectUri.mockResolvedValue(true)
+    mocks.requireCompanyId.mockResolvedValue('company-1')
+    mocks.getBranding.mockReturnValue({ appName: 'gnubok' })
+  })
+
+  it("form-action includes the redirect_uri origin so the post-consent redirect isn't blocked", async () => {
+    // Regression: the consent form POSTs same-origin, but the server's 303
+    // response redirects to the client callback. CSP form-action re-checks
+    // every hop in the chain, so 'self' alone blocks the post-consent step.
+    const request = new Request(
+      buildAuthorizeUrl({
+        response_type: 'code',
+        redirect_uri: 'https://claude.ai/api/mcp/auth_callback',
+        code_challenge: 'abc',
+        code_challenge_method: 'S256',
+        scope: 'mcp',
+        state: 'xyz',
+      })
+    )
+    const response = await GET(request)
+    expect(response.status).toBe(200)
+
+    const csp = response.headers.get('Content-Security-Policy')
+    expect(csp).toBeTruthy()
+    expect(csp).toMatch(/form-action 'self' https:\/\/claude\.ai(;|$)/)
+    // 'self' is preserved so the same-origin POST still works.
+    expect(csp).toContain("form-action 'self'")
+  })
+
+  it('form-action uses the redirect origin only (no path/query leakage)', async () => {
+    const request = new Request(
+      buildAuthorizeUrl({
+        response_type: 'code',
+        redirect_uri: 'https://claude.com/api/oauth/callback?env=prod',
+        code_challenge: 'abc',
+        code_challenge_method: 'S256',
+        scope: 'mcp',
+      })
+    )
+    const response = await GET(request)
+    expect(response.status).toBe(200)
+
+    const csp = response.headers.get('Content-Security-Policy') ?? ''
+    expect(csp).toContain('https://claude.com')
+    // Origin only — no path, no query string in the source expression.
+    expect(csp).not.toContain('/api/oauth/callback')
+    expect(csp).not.toContain('env=prod')
+  })
+
+  it('renders both read and write rows when client passes only the legacy `mcp` scope marker', async () => {
+    // Claude's connector sends scope=mcp today. The consent UI must render
+    // every scope group so the user can opt into :write grants — only the
+    // :read rows are pre-checked (Art. 25(2) data-protection-by-default).
+    // Regression: commit 04c097c2 hid all write rows by clamping the
+    // ceiling to DEFAULT_OAUTH_SCOPES.
+    const request = new Request(
+      buildAuthorizeUrl({
+        response_type: 'code',
+        redirect_uri: 'https://claude.ai/api/mcp/auth_callback',
+        code_challenge: 'abc',
+        code_challenge_method: 'S256',
+        scope: 'mcp',
+      })
+    )
+    const response = await GET(request)
+    expect(response.status).toBe(200)
+    const html = await response.text()
+
+    // Write scopes must render as checkboxes (un-pre-checked).
+    expect(html).toMatch(/value="transactions:write"/)
+    expect(html).toMatch(/value="bookkeeping:write"/)
+    expect(html).toMatch(/value="invoices:write"/)
+    expect(html).toMatch(/value="pending_operations:approve"/)
+
+    // The :write checkbox must NOT be pre-checked when the client passed
+    // no explicit scope request — the user has to opt in deliberately.
+    const writeRow = html.match(
+      /<input[^>]*value="transactions:write"[^>]*>/
+    )?.[0]
+    expect(writeRow).toBeDefined()
+    expect(writeRow!).not.toContain('checked')
+
+    // The :read counterpart must still be pre-checked (safe default).
+    const readRow = html.match(
+      /<input[^>]*value="transactions:read"[^>]*>/
+    )?.[0]
+    expect(readRow).toBeDefined()
+    expect(readRow!).toContain('checked')
+  })
+
+  it('renders only the requested scopes when the client passes them explicitly', async () => {
+    // RFC 6749 §3.3 strict least-privilege: an explicit `scope=` shrinks the
+    // ceiling, so a client that asked for read-only cannot have a write box
+    // surface at consent time.
+    const request = new Request(
+      buildAuthorizeUrl({
+        response_type: 'code',
+        redirect_uri: 'https://claude.ai/api/mcp/auth_callback',
+        code_challenge: 'abc',
+        code_challenge_method: 'S256',
+        scope: 'transactions:read invoices:read',
+      })
+    )
+    const response = await GET(request)
+    expect(response.status).toBe(200)
+    const html = await response.text()
+
+    expect(html).toContain('value="transactions:read"')
+    expect(html).toContain('value="invoices:read"')
+    expect(html).not.toContain('value="transactions:write"')
+    expect(html).not.toContain('value="bookkeeping:write"')
+  })
+
+  it('rejects disallowed redirect_uri before any CSP would be emitted', async () => {
+    mocks.isAllowedRedirectUri.mockResolvedValue(false)
+    const request = new Request(
+      buildAuthorizeUrl({
+        response_type: 'code',
+        redirect_uri: 'https://evil.example/cb',
+        code_challenge: 'abc',
+        code_challenge_method: 'S256',
+        scope: 'mcp',
+      })
+    )
+    const response = await GET(request)
+    expect(response.status).toBe(400)
+    // Important: the form-action whitelist must never be populated from an
+    // untrusted origin. A 400 here keeps the allowlist as the single source
+    // of truth for which origins can land at this endpoint.
+  })
+})

app/api/mcp-oauth/authorize/route.ts

@@ -200,19 +200,19 @@ export async function GET(request: Request) {
   const scopeBindingValue = scopeParam ?? ''
   const scopeBindingSignature = signScopeBinding(scopeBindingValue)
 
-  // The consent UI is bounded to a server-enforced ceiling, regardless of
-  // what the user ticks:
+  // Two-level model for the consent UI:
   //
-  //   - Client requested specific scopes → ceiling = that set (RFC 6749 §3.3
-  //     strict least-privilege).
-  //   - Client passed no scope (or only the legacy `mcp` marker — the case
-  //     Claude's connector hits today) → ceiling = DEFAULT_OAUTH_SCOPES
-  //     (read-only). This preserves GDPR Art. 25(2) data-protection-by-default
-  //     and keeps a server-enforced read-only guarantee for clients that
-  //     never declared any intent. Widening the ceiling beyond
-  //     DEFAULT_OAUTH_SCOPES requires the client to ask for it via the
-  //     `scope` parameter.
-  const grantCeiling = new Set<ApiKeyScope>(parsed.scopes ?? DEFAULT_OAUTH_SCOPES)
+  //   - Client requested specific scopes → ceiling = that set, pre-checked =
+  //     that set (RFC 6749 §3.3 strict least-privilege).
+  //   - Client passed no scope (or only the legacy `mcp` marker — Claude's
+  //     connector today) → ceiling = ALL_SCOPES so every read/write row
+  //     renders; pre-checked = DEFAULT_OAUTH_SCOPES so only the read rows
+  //     start ticked. The user has to actively tick :write to widen the
+  //     grant. This preserves GDPR Art. 25(2) (defaults are minimal /
+  //     read-only) while still letting the resource owner authorise write
+  //     scopes per RFC 6749 §3.3 ("based on … the resource owner's
+  //     instructions") — which is the whole point of the consent step.
+  const grantCeiling = new Set<ApiKeyScope>(parsed.scopes ?? ALL_SCOPES)
   const preChecked = new Set<ApiKeyScope>(parsed.scopes ?? DEFAULT_OAUTH_SCOPES)
   const scopeCheckboxesHtml = renderScopeCheckboxes(preChecked, grantCeiling)
 
@@ -574,11 +574,18 @@ export async function GET(request: Request) {
   // script-src bound to the per-request nonce ensures the consent page's
   // inline JS can only be the block we actually emitted. Anything injected
   // by a forged response or persisted XSS would be blocked.
+  //
+  // form-action must include the redirect_uri origin: the POST handler
+  // returns a 303 to the OAuth client's callback (e.g. claude.ai), and CSP
+  // form-action re-checks every hop in the redirect chain. With only 'self'
+  // the browser would block the post-consent redirect. The origin is safe
+  // to whitelist here because isAllowedRedirectUri() already gated it above.
+  const redirectOrigin = new URL(redirectUri).origin
   const csp = [
     "default-src 'none'",
     `script-src 'nonce-${cspNonce}'`,
     "style-src 'unsafe-inline'",
-    "form-action 'self'",
+    `form-action 'self' ${redirectOrigin}`,
     "base-uri 'none'",
     "frame-ancestors 'none'",
   ].join('; ')
@@ -677,13 +684,14 @@ export async function POST(request: Request) {
   //          can never end up with write grants, even if the user tampered
   //          with the form (least-privilege, SOC 2 CC6.3, NIST AC-6).
   //        • If the client passed no scope (or only the `mcp` marker), the
-  //          ceiling = DEFAULT_OAUTH_SCOPES (read-only). A client that never
-  //          declared write intent cannot receive write grants, even if the
-  //          user tampered with the form — preserving GDPR Art. 25(2)
-  //          data-protection-by-default.
+  //          ceiling = ALL_SCOPES. The resource owner has full discretion at
+  //          consent time, which RFC 6749 §3.3 permits ("based on … the
+  //          resource owner's instructions"). The silent fallback when the
+  //          user selects nothing remains DEFAULT_OAUTH_SCOPES (read-only),
+  //          preserving GDPR Art. 25(2) data-protection-by-default.
   const submittedScopes = formData.getAll('scopes').filter((s): s is string => typeof s === 'string')
   const validated = validateScopes(submittedScopes)
-  const clientCeiling: ApiKeyScope[] = parsed.scopes ?? [...DEFAULT_OAUTH_SCOPES]
+  const clientCeiling: ApiKeyScope[] = parsed.scopes ?? [...ALL_SCOPES]
   const ceilingSet = new Set<ApiKeyScope>(clientCeiling)
   const boundedToClient = (validated ?? []).filter(s => ceilingSet.has(s))
   const grantedScopes: ApiKeyScope[] = boundedToClient.length > 0

app/api/v1/companies/[companyId]/imports/bank/route.ts

@@ -60,7 +60,7 @@ registerEndpoint({
     'SIE bookkeeping import (use /imports/sie). Auto-bank sync (use the enable-banking extension). Single-transaction creation (use POST /transactions/ingest with a 1-element array).',
   pitfalls: [
     'File size cap: 10 MB. Larger files require splitting client-side.',
-    '`format` query parameter is optional; auto-detection works for all supported banks. Pass `format` only to force a specific format. Accepted values: seb, swedbank, handelsbanken, nordea, nordea_business, lansforsakringar, ica_banken, skandia, lunar, generic_csv, camt053.',
+    '`format` query parameter is optional; auto-detection works for all supported banks. Pass `format` only to force a specific format. Accepted values: seb, swedbank, handelsbanken, nordea, nordea_business, lansforsakringar, ica_banken, skandia, lunar, northmill, generic_csv, camt053.',
     'Duplicate detection is by external_id (composed from date + amount + counterparty); a re-import of the same file with the same flag set typically deduplicates rather than creating doubles.',
     'BFL 5 kap 6-7 §§ note: this endpoint creates `transactions` rows (the underlag for a verifikation), NOT verifikationer themselves. The verifikation content requirements are in BFL 5 kap 6-7 §§; until each transaction is matched to an invoice/supplier-invoice (POST /transactions/{id}/match-*) or categorised (POST /transactions/{id}/categorize), the bookkeeping obligation isn\'t discharged. A successful import here means the data is ingested — not booked.',
     'A successful import returns operation_id; poll /operations/{id} for the final ingested/duplicates/errors counts.',
@@ -134,6 +134,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
       'ica_banken',
       'skandia',
       'lunar',
+      'northmill',
       'generic_csv',
       'camt053',
     ])