Bug/year end failure (#575)

* feat: implement findNextPeriod function and integrate into year-end closing logic * feat: add integrity check for PDF documents and enhance user feedback for corrupt files * Refactor year-end service and period creation logic for improved UTC handling and error messaging - Update `validateYearEndReadiness` to assert on stable warning messages without interpolating period names. - Modify `createNextPeriod` to ensure date calculations are performed in UTC, preventing DST-related issues. - Enhance error handling in `validateYearEndReadiness` and `executeYearEndClosing` to avoid exposing database details. - Introduce structured error messages for year-end processes in `structured-errors.ts`. - Add tests for document integrity checks, ensuring proper authentication and error handling. - Implement GUC checks in document versioning to prevent unauthorized modifications and ensure company membership. - Update migration scripts to reflect changes in document immutability enforcement. * fix: add comment to clarify GUC behavior in document supersession logic
2026-05-27 14:19:30 +02:00
parent a2a556d837
commit 8979f6eda3
14 changed files with 1214 additions and 70 deletions
@@ -0,0 +1,220 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import {
+  parseJsonResponse,
+  createMockRouteParams,
+  createQueuedMockSupabase,
+} from '@/tests/helpers'
+
+const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase()
+
+vi.mock('@/lib/auth/require-auth', () => ({
+  requireAuth: vi.fn(),
+}))
+
+vi.mock('@/lib/supabase/server', () => ({
+  createClient: () => Promise.resolve(mockSupabase),
+}))
+
+vi.mock('@/lib/core/documents/document-service', () => ({
+  validateDocumentMagicBytes: vi.fn(),
+}))
+
+import { GET } from '../route'
+import { requireAuth } from '@/lib/auth/require-auth'
+import { validateDocumentMagicBytes } from '@/lib/core/documents/document-service'
+import { NextResponse } from 'next/server'
+
+// v4 UUIDs (variant 'a' / 'b' in the 4th group) so Zod's stricter validators
+// accept them — the looser `2222...` style fails on the variant check.
+const mockUser = { id: '11111111-1111-4111-a111-111111111111' }
+const validDocId = '22222222-2222-4222-a222-222222222222'
+const companyId = '33333333-3333-4333-a333-333333333333'
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  reset()
+  vi.mocked(requireAuth).mockResolvedValue({
+    user: mockUser as never,
+    supabase: mockSupabase as never,
+    error: null,
+  })
+})
+
+function req() {
+  return new Request(`http://localhost/api/documents/${validDocId}/integrity`)
+}
+
+describe('GET /api/documents/[id]/integrity', () => {
+  it('returns 401 when the user is not authenticated', async () => {
+    vi.mocked(requireAuth).mockResolvedValue({
+      user: null as never,
+      supabase: mockSupabase as never,
+      error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
+    })
+
+    const res = await GET(req(), createMockRouteParams({ id: validDocId }))
+    const { status } = await parseJsonResponse(res)
+    expect(status).toBe(401)
+  })
+
+  it('returns 400 when id is not a UUID', async () => {
+    const res = await GET(req(), createMockRouteParams({ id: 'not-a-uuid' }))
+    const { status, body } = await parseJsonResponse<{ error: string }>(res)
+    expect(status).toBe(400)
+    expect(body.error).toMatch(/invalid/i)
+  })
+
+  it('returns 404 when document is not found or is superseded', async () => {
+    enqueue({ data: null, error: null })
+    const res = await GET(req(), createMockRouteParams({ id: validDocId }))
+    const { status } = await parseJsonResponse(res)
+    expect(status).toBe(404)
+  })
+
+  it('returns 404 when caller is not a member of the document company', async () => {
+    enqueue({
+      data: {
+        id: validDocId,
+        company_id: companyId,
+        mime_type: 'application/pdf',
+        storage_path: 'documents/foo/bar.pdf',
+      },
+      error: null,
+    })
+    enqueue({ data: null, error: null }) // membership check returns null
+    const res = await GET(req(), createMockRouteParams({ id: validDocId }))
+    const { status, body } = await parseJsonResponse<{ error: string }>(res)
+    expect(status).toBe(404)
+    // The endpoint deliberately returns the same 404 as missing-document so
+    // the response cannot be used to enumerate documents across companies.
+    expect(body.error).toBe('Document not found')
+  })
+
+  it('returns { valid: true } and skips download when mime_type is null', async () => {
+    enqueue({
+      data: {
+        id: validDocId,
+        company_id: companyId,
+        mime_type: null,
+        storage_path: 'documents/foo/bar.pdf',
+      },
+      error: null,
+    })
+    enqueue({ data: { company_id: companyId }, error: null }) // membership
+
+    const res = await GET(req(), createMockRouteParams({ id: validDocId }))
+    const { status, body } = await parseJsonResponse<{ data: { valid: boolean } }>(res)
+    expect(status).toBe(200)
+    expect(body.data.valid).toBe(true)
+    expect(mockSupabase.storage.from).not.toHaveBeenCalled()
+  })
+
+  it('returns { valid: true } when the bytes match the declared mime type', async () => {
+    enqueue({
+      data: {
+        id: validDocId,
+        company_id: companyId,
+        mime_type: 'application/pdf',
+        storage_path: 'documents/foo/bar.pdf',
+      },
+      error: null,
+    })
+    enqueue({ data: { company_id: companyId }, error: null })
+
+    const pdfBytes = new Uint8Array([0x25, 0x50, 0x44, 0x46, 0x2d, 0x31, 0x2e, 0x37])
+    const blob = new Blob([pdfBytes], { type: 'application/pdf' })
+    mockSupabase.storage.from.mockReturnValue({
+      download: vi.fn().mockResolvedValue({ data: blob, error: null }),
+    } as never)
+
+    vi.mocked(validateDocumentMagicBytes).mockReturnValue(null)
+
+    const res = await GET(req(), createMockRouteParams({ id: validDocId }))
+    const { status, body } = await parseJsonResponse<{ data: { valid: boolean } }>(res)
+    expect(status).toBe(200)
+    expect(body.data.valid).toBe(true)
+    expect(Object.keys(body.data)).toEqual(['valid'])
+  })
+
+  it('returns { valid: false } without leaking the reason text', async () => {
+    enqueue({
+      data: {
+        id: validDocId,
+        company_id: companyId,
+        mime_type: 'application/pdf',
+        storage_path: 'documents/foo/bar.pdf',
+      },
+      error: null,
+    })
+    enqueue({ data: { company_id: companyId }, error: null })
+
+    const blob = new Blob([new Uint8Array([0x00, 0x00, 0x00, 0x00])])
+    mockSupabase.storage.from.mockReturnValue({
+      download: vi.fn().mockResolvedValue({ data: blob, error: null }),
+    } as never)
+
+    vi.mocked(validateDocumentMagicBytes).mockReturnValue('Internal /storage/v1/object error 42')
+
+    const res = await GET(req(), createMockRouteParams({ id: validDocId }))
+    const { status, body } = await parseJsonResponse<{ data: Record<string, unknown> }>(res)
+    expect(status).toBe(200)
+    expect(body.data.valid).toBe(false)
+    // The whole point of the V1.2.5 / Art 25(2) hardening — internal text
+    // never appears in the response.
+    expect(body.data.reason).toBeUndefined()
+    expect(JSON.stringify(body)).not.toContain('storage/v1/object')
+  })
+
+  it('returns a generic 500 without leaking the underlying storage error', async () => {
+    enqueue({
+      data: {
+        id: validDocId,
+        company_id: companyId,
+        mime_type: 'application/pdf',
+        storage_path: 'documents/foo/bar.pdf',
+      },
+      error: null,
+    })
+    enqueue({ data: { company_id: companyId }, error: null })
+
+    mockSupabase.storage.from.mockReturnValue({
+      download: vi.fn().mockResolvedValue({
+        data: null,
+        error: { message: 'storage internal: bucket=documents object=secret/path.pdf' },
+      }),
+    } as never)
+
+    const res = await GET(req(), createMockRouteParams({ id: validDocId }))
+    const { status, body } = await parseJsonResponse<{ error: string }>(res)
+    expect(status).toBe(500)
+    expect(body.error).toBe('Integrity check unavailable')
+    expect(JSON.stringify(body)).not.toContain('secret/path.pdf')
+  })
+
+  it('does not import the service-role supabase client', async () => {
+    // The download must go through the per-request user-scoped client so
+    // RLS on storage.objects can act as defense-in-depth. Statically
+    // verifying the source is the cleanest check — runtime mocking of the
+    // service-client export would not catch a future regression where
+    // someone added an import but conditionally used it.
+    const fs = await import('node:fs/promises')
+    const path = await import('node:path')
+    const routePath = path.resolve(__dirname, '../route.ts')
+    const source = await fs.readFile(routePath, 'utf8')
+    expect(source).not.toMatch(/createServiceClient(\b|NoCookies)/)
+  })
+
+  it('filters the document lookup to the current version', async () => {
+    // The route's first `from('document_attachments')` chain must include
+    // `.eq('is_current_version', true)` so superseded versions cannot have
+    // their bytes probed via this surface. Source-level check rather than
+    // runtime spying — the proxy-based queued mock collapses every chained
+    // method into the same handler, so introspecting individual .eq calls
+    // is not feasible without rebuilding the mock.
+    const fs = await import('node:fs/promises')
+    const path = await import('node:path')
+    const routePath = path.resolve(__dirname, '../route.ts')
+    const source = await fs.readFile(routePath, 'utf8')
+    expect(source).toMatch(/\.eq\(['"]is_current_version['"],\s*true\)/)
+  })
+})
@@ -0,0 +1,109 @@
+import { NextResponse } from 'next/server'
+import { z } from 'zod'
+import { requireAuth } from '@/lib/auth/require-auth'
+import { validateDocumentMagicBytes } from '@/lib/core/documents/document-service'
+import { createLogger } from '@/lib/logger'
+
+const log = createLogger('documents.integrity')
+
+const ParamsSchema = z.object({ id: z.string().uuid() })
+
+/**
+ * GET /api/documents/:id/integrity
+ *
+ * Probes the actual stored bytes against the declared MIME type. Used by the
+ * Bilagor modal to surface a clear "this file is corrupt — please re-upload"
+ * warning instead of relying on the browser's PDF viewer error UI, which
+ * only fires after the user has already tried to view the file.
+ *
+ * Some legacy MCP uploads landed with non-PDF bytes under
+ * `mime_type = 'application/pdf'` because magic-byte validation was added
+ * after those rows were written. This endpoint lets the UI detect and steer
+ * the user toward replacing them.
+ *
+ * Response shape is intentionally minimal — { valid: boolean } only. The
+ * reason for an invalid result is logged server-side rather than returned
+ * to the client to avoid information disclosure (V1.2.5 / GDPR Art 25(2))
+ * and to keep this from being a probe surface for storage internals.
+ */
+export async function GET(
+  _request: Request,
+  { params }: { params: Promise<{ id: string }> }
+) {
+  const { user, supabase, error } = await requireAuth()
+  if (error) return error
+
+  const rawParams = await params
+  const parsed = ParamsSchema.safeParse(rawParams)
+  if (!parsed.success) {
+    return NextResponse.json({ error: 'Invalid document id' }, { status: 400 })
+  }
+  const { id } = parsed.data
+
+  // Filter to the current version. The integrity check is meaningful only
+  // on the live file; superseded versions are archived bytes and should
+  // not be re-probed (they're already preserved in the version chain
+  // exactly as uploaded).
+  const { data: doc, error: docError } = await supabase
+    .from('document_attachments')
+    .select('id, company_id, mime_type, storage_path')
+    .eq('id', id)
+    .eq('is_current_version', true)
+    .single()
+
+  if (docError || !doc) {
+    return NextResponse.json({ error: 'Document not found' }, { status: 404 })
+  }
+
+  // Tenant membership: even with the user-scoped supabase client below,
+  // we want a clear 404 rather than relying on a storage-layer RLS deny
+  // (which can present as a generic error). RLS on document_attachments
+  // is the primary control; this is defense in depth.
+  const { data: membership } = await supabase
+    .from('company_members')
+    .select('company_id')
+    .eq('company_id', doc.company_id)
+    .eq('user_id', user.id)
+    .maybeSingle()
+
+  if (!membership) {
+    return NextResponse.json({ error: 'Document not found' }, { status: 404 })
+  }
+
+  if (!doc.mime_type) {
+    return NextResponse.json({ data: { valid: true } })
+  }
+
+  // Use the user-scoped supabase client so the storage download is subject
+  // to RLS on storage.objects, not just the application-layer membership
+  // check above. A logic bug in the membership check would still be
+  // arrested at the storage layer.
+  const { data: blob, error: downloadError } = await supabase.storage
+    .from('documents')
+    .download(doc.storage_path)
+
+  if (downloadError || !blob) {
+    log.error('storage download failed for integrity check', downloadError as Error, {
+      documentId: id,
+      companyId: doc.company_id,
+    })
+    return NextResponse.json({ error: 'Integrity check unavailable' }, { status: 500 })
+  }
+
+  // Only the first 16 bytes are needed for magic-byte detection (PDF/PNG
+  // use ≤8, WebP needs 12). Trimming here doesn't change bandwidth — the
+  // full blob is already downloaded — but it makes the intent explicit and
+  // keeps memory churn off the hot path for large PDFs.
+  const headerBuffer = await blob.slice(0, 16).arrayBuffer()
+  const magicError = validateDocumentMagicBytes(headerBuffer, doc.mime_type)
+
+  if (magicError) {
+    log.warn('document failed magic-byte integrity check', {
+      documentId: id,
+      companyId: doc.company_id,
+      reason: magicError,
+    })
+  }
+
+  return NextResponse.json({ data: { valid: magicError === null } })
+}