From 968161b42be950e832a05c61bf5299bfab314637 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:34:11 +0200 Subject: [PATCH] fix(documents): read attachments with service client so colleague uploads open (#1207) The documents bucket SELECT policy only covers the uploader's own folder (documents/{uid}/...), but document_attachments rows are company-scoped. Every surface that touched storage with the user-bound client therefore failed for attachments uploaded by another member of the same company (colleague uploads, email-inbox ingest attributed to the company creator): - GET /api/documents/:id 500ed with "Failed to create download URL", so viewing a bilaga on a verifikat or supplier invoice was broken for every member except the uploader (support case: Odin Aero, where all 40 documents live in the owner's folder and the second member could open none of them). - GET /api/documents/:id/integrity 500ed the same way. - POST /api/documents/:id/verify failed the storage download. - invoice-inbox retry-extraction could not download the attachment. - cloud-backup user-triggered syncs silently dropped colleague-uploaded documents from the Drive archive (manifest rows flipped to 'error'). Fix: authorize on the user client (RLS + explicit company filter, plus the membership check where present), then do the storage read with the service-role client. This is the pattern the inline proxy route and the v1 download route already use; these five call sites were left behind. Co-authored-by: Claude Fable 5 --- DECISIONS.md | 1 + .../documents/[id]/__tests__/route.test.ts | 51 ++++++++++++++++--- .../[id]/integrity/__tests__/route.test.ts | 45 ++++++++-------- app/api/documents/[id]/integrity/route.ts | 14 +++-- app/api/documents/[id]/route.ts | 12 ++++- .../cloud-backup/lib/__tests__/sync.test.ts | 8 +++ extensions/general/cloud-backup/lib/sync.ts | 14 ++++- .../__tests__/retry-extraction.test.ts | 26 ++++++---- extensions/general/invoice-inbox/index.ts | 8 ++- .../__tests__/document-service.test.ts | 18 ++++--- lib/core/documents/document-service.ts | 8 ++- 11 files changed, 152 insertions(+), 53 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 98027727..534835e6 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -384,3 +384,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-25] Declined the review suggestion to 200-ack the Resend delivery webhook when RESEND_DELIVERY_WEBHOOK_SECRET is unset; kept 503. The endpoint is only ever called because an operator pointed Resend at it, so a missing secret at that moment is a live misconfiguration: Svix retry then endpoint-disable is a visible signal, whereas a silent 200 loses every delivery outcome with only a log line. The "optional" wording in docs/WHITELABEL.md describes not wiring the webhook at all, not wiring it half way. [2026-07-25] Reverted the settings panel-sheet redesign on bug/resend-and-invoices back to main: Emil prefers the settings UI as it stands on main. The routed sheet, the sheet/ primitives (SettingsMasterDetail, SettingsAccordion, SettingsFieldRow), the *Subsections.tsx decompositions, the cold-load sheet and the settings_sheet i18n namespace were removed; every app/(dashboard)/settings/* page, components/settings/** file and MainContainer scroll exception now matches origin/main byte for byte. Unrelated branch work (invoice delivery outcomes, Stripe feed-only, article currency/deactivation, PDF logo) is untouched. [2026-07-25] Settings UI on bug/resend-and-invoices now comes from feat/settings-fonster-redesign (dbae8792, Jakob) instead of the panel-sheet work reverted earlier the same day: Emil chose the Fonster concept (flat hairline rows, help behind "?", sticky dirty-only save bar, 920x680 modal, switches instead of checkboxes). Applied as a patch rather than a merge because the redesign branch forks from b5e3c476 and merging would have dragged that older main in; every file applied cleanly since no settings file changed on main since that fork point. The 10 settings_payments keys the redesign still carries (needs_review_*, reason_*, sync_done_description/transactions) were deliberately NOT restored: the Stripe feed-only commit on this branch deleted both them and their call sites. +[2026-07-26] Fixed cross-user attachment access (Odin Aero support case) at the call sites with service-role clients after company-scoped authorization, instead of rewriting the documents bucket storage policy to be company-scoped like sie-files got in 20260416120000: the documents path layout (documents/{userId}/...) carries no company_id, so a company-scoped policy needs a per-object join against document_attachments on every storage op, and the authorize-then-service-client pattern was already the established model (inline proxy route, v1 download route, MCP tools). Sweep found and fixed the same defect in the metadata/sign route, the integrity probe, verifyIntegrity, invoice-inbox retry-extraction, and cloud-backup archive generation. Known leftover, deliberately unfixed: deleteDocument and the upload-failure cleanups call storage remove() with a user-bound client, which silently no-ops (no DELETE policy, WORM), orphaning storage objects; harmless for compliance, needs a separate decision on whether files should ever be hard-deleted. diff --git a/app/api/documents/[id]/__tests__/route.test.ts b/app/api/documents/[id]/__tests__/route.test.ts index e83a3c37..b095bc3c 100644 --- a/app/api/documents/[id]/__tests__/route.test.ts +++ b/app/api/documents/[id]/__tests__/route.test.ts @@ -27,6 +27,17 @@ vi.mock('@/lib/init', () => ({ ensureInitialized: vi.fn(), })) +// The GET route signs with the service-role client: the storage SELECT +// policy only covers the uploader's own folder, so a company member viewing +// a colleague's upload cannot sign with their own client. +const createSignedUrlMock = vi.fn() +const serviceStorageFromMock = vi.fn(() => ({ createSignedUrl: createSignedUrlMock })) +vi.mock('@/lib/supabase/server', () => ({ + createServiceClient: () => ({ + storage: { from: serviceStorageFromMock }, + }), +})) + import { GET, DELETE } from '../route' import { requireWritePermission } from '@/lib/auth/require-write' import { NextResponse } from 'next/server' @@ -40,6 +51,10 @@ beforeEach(() => { requireAuthMock.mockResolvedValue({ user: mockUser, supabase: mockSupabase, error: null }) // Reset write-permission mock to default ok vi.mocked(requireWritePermission).mockResolvedValue({ ok: true }) + createSignedUrlMock.mockResolvedValue({ + data: { signedUrl: 'https://example.com/signed' }, + error: null, + }) }) function makeReq(method: 'GET' | 'DELETE' = 'DELETE') { @@ -69,9 +84,7 @@ describe('GET /api/documents/[id]', () => { it('returns 500 when the signed URL cannot be created', async () => { enqueue({ data: makeDocumentAttachment({ id: 'doc-1' }), error: null }) - mockSupabase.storage.from.mockReturnValueOnce({ - createSignedUrl: vi.fn().mockResolvedValue({ data: null, error: { message: 'boom' } }), - } as never) + createSignedUrlMock.mockResolvedValue({ data: null, error: { message: 'boom' } }) const res = await GET(makeReq('GET'), createMockRouteParams({ id: 'doc-1' })) const { status, body } = await parseJsonResponse<{ error: string }>(res) @@ -100,9 +113,8 @@ describe('GET /api/documents/[id]', () => { expect(body.data.id).toBe('doc-1') expect(body.data.download_url).toBe('https://example.com/signed') - expect(mockSupabase.storage.from).toHaveBeenCalledWith('documents') - const storageBucket = mockSupabase.storage.from.mock.results[0]?.value - expect(storageBucket.createSignedUrl).toHaveBeenCalledWith('documents/user-1/kvitto.pdf', 3600) + expect(serviceStorageFromMock).toHaveBeenCalledWith('documents') + expect(createSignedUrlMock).toHaveBeenCalledWith('documents/user-1/kvitto.pdf', 3600) expect(handler).toHaveBeenCalledOnce() expect(handler).toHaveBeenCalledWith( @@ -113,6 +125,33 @@ describe('GET /api/documents/[id]', () => { }), ) }) + + it('signs attachments stored under another company member folder', async () => { + // Regression: the storage SELECT policy is per-uploader-folder, so signing + // with the user-bound client failed for every colleague-uploaded document + // ("Failed to create download URL"). The service client must sign after + // the company-scoped row fetch has authorized access. + const row = makeDocumentAttachment({ + id: 'doc-2', + file_name: 'leverantorsfaktura.pdf', + storage_path: 'documents/other-member/leverantorsfaktura.pdf', + }) + enqueue({ data: row, error: null }) + + const res = await GET(makeReq('GET'), createMockRouteParams({ id: 'doc-2' })) + const { status, body } = await parseJsonResponse<{ + data: { download_url: string } + }>(res) + + expect(status).toBe(200) + expect(body.data.download_url).toBe('https://example.com/signed') + expect(createSignedUrlMock).toHaveBeenCalledWith( + 'documents/other-member/leverantorsfaktura.pdf', + 3600, + ) + // The user-bound client must not be used for signing at all. + expect(mockSupabase.storage.from).not.toHaveBeenCalled() + }) }) describe('DELETE /api/documents/[id]', () => { diff --git a/app/api/documents/[id]/integrity/__tests__/route.test.ts b/app/api/documents/[id]/integrity/__tests__/route.test.ts index 1464cc5f..99c06172 100644 --- a/app/api/documents/[id]/integrity/__tests__/route.test.ts +++ b/app/api/documents/[id]/integrity/__tests__/route.test.ts @@ -11,8 +11,14 @@ vi.mock('@/lib/auth/require-auth', () => ({ requireAuth: vi.fn(), })) +// The storage download goes through the service-role client: the storage +// SELECT policy only covers the uploader's own folder, so the user-scoped +// client cannot read colleague-uploaded files within the same company. +const serviceDownloadMock = vi.fn() +const serviceStorageFromMock = vi.fn(() => ({ download: serviceDownloadMock })) vi.mock('@/lib/supabase/server', () => ({ createClient: () => Promise.resolve(mockSupabase), + createServiceClient: () => ({ storage: { from: serviceStorageFromMock } }), })) vi.mock('@/lib/core/documents/document-service', () => ({ @@ -106,7 +112,7 @@ describe('GET /api/documents/[id]/integrity', () => { 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() + expect(serviceStorageFromMock).not.toHaveBeenCalled() }) it('returns { valid: true } when the bytes match the declared mime type', async () => { @@ -123,9 +129,7 @@ describe('GET /api/documents/[id]/integrity', () => { 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) + serviceDownloadMock.mockResolvedValue({ data: blob, error: null }) vi.mocked(validateDocumentMagicBytes).mockReturnValue(null) @@ -149,9 +153,7 @@ describe('GET /api/documents/[id]/integrity', () => { 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) + serviceDownloadMock.mockResolvedValue({ data: blob, error: null }) vi.mocked(validateDocumentMagicBytes).mockReturnValue('Internal /storage/v1/object error 42') @@ -177,12 +179,10 @@ describe('GET /api/documents/[id]/integrity', () => { }) 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) + serviceDownloadMock.mockResolvedValue({ + data: null, + error: { message: 'storage internal: bucket=documents object=secret/path.pdf' }, + }) const res = await GET(req(), createMockRouteParams({ id: validDocId })) const { status, body } = await parseJsonResponse<{ error: string }>(res) @@ -191,17 +191,22 @@ describe('GET /api/documents/[id]/integrity', () => { 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. + it('authorizes on the user-scoped client before the service-role download', async () => { + // The storage SELECT policy on the documents bucket only covers the + // uploader's own folder, so the download must use the service-role + // client or colleague-uploaded files 500 for every other company + // member. The compensating control is that both the row fetch and the + // explicit membership check run on the user-scoped client first. + // Source-level check: the queued mock collapses chained calls, so + // runtime introspection cannot distinguish which client ran the fetch. 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)/) + const membershipIdx = source.indexOf("from('company_members')") + const downloadIdx = source.indexOf('createServiceClient()') + expect(membershipIdx).toBeGreaterThan(-1) + expect(downloadIdx).toBeGreaterThan(membershipIdx) }) it('filters the document lookup to the current version', async () => { diff --git a/app/api/documents/[id]/integrity/route.ts b/app/api/documents/[id]/integrity/route.ts index 7d960555..0d0418e3 100644 --- a/app/api/documents/[id]/integrity/route.ts +++ b/app/api/documents/[id]/integrity/route.ts @@ -1,6 +1,7 @@ import { NextResponse } from 'next/server' import { z } from 'zod' import { requireAuth } from '@/lib/auth/require-auth' +import { createServiceClient } from '@/lib/supabase/server' import { validateDocumentMagicBytes } from '@/lib/core/documents/document-service' import { createLogger } from '@/lib/logger' @@ -74,11 +75,14 @@ export async function GET( 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 + // Download via the service-role client: the storage SELECT policy only + // covers the uploader's own folder (documents/{uid}/...), so the + // user-scoped client cannot read colleague-uploaded files even within + // the same company. The document_attachments RLS fetch plus the explicit + // membership check above are the authorization (same model as the + // inline proxy route). + const serviceClient = createServiceClient() + const { data: blob, error: downloadError } = await serviceClient.storage .from('documents') .download(doc.storage_path) diff --git a/app/api/documents/[id]/route.ts b/app/api/documents/[id]/route.ts index 5f247b81..922801f7 100644 --- a/app/api/documents/[id]/route.ts +++ b/app/api/documents/[id]/route.ts @@ -1,6 +1,7 @@ import { NextResponse } from 'next/server' import { ensureInitialized } from '@/lib/init' import { withRouteContext } from '@/lib/api/with-route-context' +import { createServiceClient } from '@/lib/supabase/server' import { deleteDocument } from '@/lib/core/documents/document-service' import { eventBus } from '@/lib/events' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' @@ -34,8 +35,17 @@ export const GET = withRouteContext<{ params: Promise<{ id: string }> }>( // not race Vercel function suspension) and never rejects (the bus // settles handlers via Promise.allSettled), so it cannot fail this // Promise.all. + // + // Sign with the service-role client: the storage SELECT policy only + // covers the uploader's own folder (documents/{uid}/...), while + // document_attachments rows are company-scoped. Signing with the + // user-bound client fails for every attachment uploaded by another + // member of the same company. The row fetch above (RLS + explicit + // company filter) is the authorization, mirroring the inline proxy + // route. + const serviceClient = createServiceClient() const [signResult] = await Promise.all([ - supabase.storage.from('documents').createSignedUrl(doc.storage_path, 3600), + serviceClient.storage.from('documents').createSignedUrl(doc.storage_path, 3600), eventBus.emit({ type: 'document.accessed', payload: { diff --git a/extensions/general/cloud-backup/lib/__tests__/sync.test.ts b/extensions/general/cloud-backup/lib/__tests__/sync.test.ts index 87048bff..f17ab033 100644 --- a/extensions/general/cloud-backup/lib/__tests__/sync.test.ts +++ b/extensions/general/cloud-backup/lib/__tests__/sync.test.ts @@ -12,6 +12,14 @@ vi.mock('@/lib/reports/archive-readme', () => ({ buildDriveFolderReadme: vi.fn().mockReturnValue('README TEXT'), })) +// performSync builds archives with a service-role client (the documents +// bucket SELECT policy is per-uploader-folder, so a user-bound client would +// drop colleague-uploaded files from the backup). The archive generators are +// mocked above, so a bare stub is enough here. +vi.mock('@/lib/supabase/server', () => ({ + createServiceClient: vi.fn(() => ({ storage: { from: vi.fn() } })), +})) + vi.mock('@/lib/branding/service', () => ({ getBranding: () => ({ appName: 'Accounted', appUrl: 'https://app.test' }), })) diff --git a/extensions/general/cloud-backup/lib/sync.ts b/extensions/general/cloud-backup/lib/sync.ts index 3c70d755..3a374c0b 100644 --- a/extensions/general/cloud-backup/lib/sync.ts +++ b/extensions/general/cloud-backup/lib/sync.ts @@ -7,6 +7,7 @@ import { } from '@/lib/reports/full-archive-export' import { buildDriveFolderReadme } from '@/lib/reports/archive-readme' import { getBranding } from '@/lib/branding/service' +import { createServiceClient } from '@/lib/supabase/server' import { fetchAllRows } from '@/lib/supabase/fetch-all' import { getOAuthEnv, @@ -126,6 +127,15 @@ interface PeriodRow { export async function performSync(params: PerformSyncParams): Promise { const { supabase, companyId, userId, origin } = params + // Archive generation reads the private `documents` bucket, whose SELECT + // policy only covers the uploader's own folder. User-triggered syncs pass + // a user-bound client, which would silently drop every colleague-uploaded + // document from the backup (manifest rows flip to 'error'). The service + // client is used for archive generation only; authorization happened at + // the extension dispatcher (or the cron), and every archive query filters + // by the explicit companyId. + const archiveClient = createServiceClient() + const connection = await loadExtensionData( supabase, companyId, @@ -322,7 +332,7 @@ export async function performSync(params: PerformSyncParams): Promise - generateFullArchive(supabase, companyId, { + generateFullArchive(archiveClient, companyId, { scope: 'period', period_id: period.id, include_documents: includeDocuments, @@ -355,7 +365,7 @@ export async function performSync(params: PerformSyncParams): Promise - generateBaseDataArchive(supabase, companyId, { + generateBaseDataArchive(archiveClient, companyId, { include_documents: baseDecision.includeDocuments, }), }) diff --git a/extensions/general/invoice-inbox/__tests__/retry-extraction.test.ts b/extensions/general/invoice-inbox/__tests__/retry-extraction.test.ts index 0bc819d3..2f6f8fcf 100644 --- a/extensions/general/invoice-inbox/__tests__/retry-extraction.test.ts +++ b/extensions/general/invoice-inbox/__tests__/retry-extraction.test.ts @@ -23,6 +23,16 @@ vi.mock('@/lib/entitlements/has-capability', async (importOriginal) => { return { ...actual, hasCapability: vi.fn().mockResolvedValue(true) } }) +// The attachment download runs on the service-role client (the storage +// SELECT policy is per-uploader-folder, and inbox documents are attributed +// to the company creator, not the caller). +const serviceDownloadMock = vi.fn() +vi.mock('@/lib/supabase/server', () => ({ + createServiceClient: () => ({ + storage: { from: vi.fn(() => ({ download: serviceDownloadMock })) }, + }), +})) + import { extractInvoiceFields } from '@/extensions/general/invoice-inbox/lib/extract-invoice-fields' import { checkInboxUploadRateLimit } from '@/lib/rate-limits/inbox' import { hasCapability } from '@/lib/entitlements/has-capability' @@ -155,11 +165,9 @@ describe('POST /items/:id/retry-extraction', () => { }) enqueue({ data: null, error: null }) // inbox update on success - supabase.storage.from = vi.fn().mockReturnValue({ - download: vi.fn().mockResolvedValue({ - data: new Blob([new Uint8Array([1, 2, 3])], { type: 'application/pdf' }), - error: null, - }), + serviceDownloadMock.mockResolvedValue({ + data: new Blob([new Uint8Array([1, 2, 3])], { type: 'application/pdf' }), + error: null, }) vi.mocked(extractInvoiceFields).mockResolvedValueOnce(EXTRACTION_SUCCESS as never) @@ -183,11 +191,9 @@ describe('POST /items/:id/retry-extraction', () => { }) enqueue({ data: null, error: null }) // error-state update - supabase.storage.from = vi.fn().mockReturnValue({ - download: vi.fn().mockResolvedValue({ - data: new Blob([new Uint8Array([1])], { type: 'application/pdf' }), - error: null, - }), + serviceDownloadMock.mockResolvedValue({ + data: new Blob([new Uint8Array([1])], { type: 'application/pdf' }), + error: null, }) vi.mocked(extractInvoiceFields).mockRejectedValueOnce(new Error('pdfjs blew up')) diff --git a/extensions/general/invoice-inbox/index.ts b/extensions/general/invoice-inbox/index.ts index 638f29d6..fdcf9c89 100644 --- a/extensions/general/invoice-inbox/index.ts +++ b/extensions/general/invoice-inbox/index.ts @@ -3,6 +3,7 @@ import { NextResponse } from 'next/server' import { createClient } from '@supabase/supabase-js' import { z } from 'zod' import { uploadDocument } from '@/lib/core/documents/document-service' +import { createServiceClient } from '@/lib/supabase/server' import { extractInvoiceFields, ExtractionSchema, emptyResult } from './lib/extract-invoice-fields' import { verifyInboundWebhook, @@ -1179,7 +1180,12 @@ export const invoiceInboxExtension: Extension = { return NextResponse.json({ error: 'Bilagan kunde inte hittas.' }, { status: 404 }) } - const { data: blob, error: dlError } = await ctx.supabase.storage + // Download via the service-role client: the storage SELECT policy + // only covers the uploader's own folder, and inbox documents are + // attributed to the company creator, so ctx.supabase (user-bound) + // cannot read them for other members. The company-scoped row fetch + // above is the authorization. + const { data: blob, error: dlError } = await createServiceClient().storage .from('documents') .download(doc.storage_path) diff --git a/lib/core/documents/__tests__/document-service.test.ts b/lib/core/documents/__tests__/document-service.test.ts index ef7be049..a0f3b79e 100644 --- a/lib/core/documents/__tests__/document-service.test.ts +++ b/lib/core/documents/__tests__/document-service.test.ts @@ -43,8 +43,13 @@ function makeClient(storageOverrides: Record = {}) { } } +// verifyIntegrity downloads via the service-role client (the storage SELECT +// policy is per-uploader-folder); tests set this override to control the +// downloaded bytes. +let serviceClientOverride: ReturnType | null = null + vi.mock('@/lib/auth/api-keys', () => ({ - createServiceClientNoCookies: vi.fn(() => makeClient()), + createServiceClientNoCookies: vi.fn(() => serviceClientOverride ?? makeClient()), })) import { @@ -66,6 +71,7 @@ beforeEach(() => { _resetBucketVerified() resultIdx = 0 results = [] + serviceClientOverride = null }) describe('validateDocumentMagicBytes: application/xhtml+xml', () => { @@ -298,15 +304,15 @@ describe('verifyIntegrity', () => { { data: { storage_path: 'docs/test.pdf', sha256_hash: expectedHash }, error: null }, ] - // Create a client with matching download content - const supabase = makeClient({ + // The download runs on the service-role client; give it matching bytes. + serviceClientOverride = makeClient({ download: vi.fn().mockResolvedValue({ data: new Blob([content]), error: null, }), }) - const result = await verifyIntegrity(supabase as never, 'user-1', 'doc-1') + const result = await verifyIntegrity(makeClient() as never, 'user-1', 'doc-1') expect(result.valid).toBe(true) expect(result.storedHash).toBe(expectedHash) expect(result.computedHash).toBe(expectedHash) @@ -317,14 +323,14 @@ describe('verifyIntegrity', () => { { data: { storage_path: 'docs/test.pdf', sha256_hash: 'stored-hash-abc' }, error: null }, ] - const supabase = makeClient({ + serviceClientOverride = makeClient({ download: vi.fn().mockResolvedValue({ data: new Blob(['different content']), error: null, }), }) - const result = await verifyIntegrity(supabase as never, 'user-1', 'doc-1') + const result = await verifyIntegrity(makeClient() as never, 'user-1', 'doc-1') expect(result.valid).toBe(false) expect(result.storedHash).toBe('stored-hash-abc') expect(result.computedHash).not.toBe('stored-hash-abc') diff --git a/lib/core/documents/document-service.ts b/lib/core/documents/document-service.ts index 5deffe21..1fc02bb9 100644 --- a/lib/core/documents/document-service.ts +++ b/lib/core/documents/document-service.ts @@ -488,8 +488,12 @@ export async function verifyIntegrity( throw new Error('Document not found') } - // Download file from storage - const { data: fileData, error: downloadError } = await supabase.storage + // Download via the service-role client: the storage SELECT policy only + // covers the uploader's own folder (documents/{uid}/...), so a caller-bound + // client cannot read colleague-uploaded files. The company-filtered row + // fetch above (RLS on document_attachments) is the authorization. + const serviceClient = createServiceClientNoCookies() + const { data: fileData, error: downloadError } = await serviceClient.storage .from('documents') .download(doc.storage_path)