fix(auth): route document integrity, transaction delete/list and agent categorize through withRouteContext (#1926)
Two handlers hand-rolled supabase.auth.getUser() and therefore skipped the MFA (AAL2) gate on hosted: DELETE /api/transactions/[id] and GET /api/transactions. Both sat next to a sibling handler that was already wrapped, and the raw-route-auth ratchet exempted a file as soon as any withRouteContext call appeared in it, so they were never flagged. GET /api/documents/[id]/integrity and POST /api/agent/categorize called requireAuth() directly (MFA enforced, but no request id, no completion log, no canonical error envelope). All four are now withRouteContext handlers with identical company scoping and responses; the transaction delete keeps its viewer rejection via requireWrite. The guard now judges each top-level export segment of a route file on its own, so a wrapped handler no longer exempts a hand-rolled sibling. Baseline is unchanged (mcp-oauth/authorize remains the one grandfathered file). Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Jakob Wennberg
Claude Fable 5
Jakob Wennberg
Claude Fable 5
parent
d3869e6694
commit
1a27b5bd4a
@@ -7,10 +7,17 @@ import {
|
||||
|
||||
const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase()
|
||||
|
||||
// The route goes through withRouteContext: requireAuth (MFA on hosted) plus
|
||||
// active-company resolution. The document is still authorized against its
|
||||
// own company's membership inside the handler.
|
||||
vi.mock('@/lib/auth/require-auth', () => ({
|
||||
requireAuth: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/company/context', () => ({
|
||||
getActiveCompanyId: vi.fn().mockResolvedValue('33333333-3333-4333-a333-333333333333'),
|
||||
}))
|
||||
|
||||
// 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.
|
||||
@@ -61,6 +68,14 @@ describe('GET /api/documents/[id]/integrity', () => {
|
||||
const res = await GET(req(), createMockRouteParams({ id: validDocId }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(401)
|
||||
expect(serviceStorageFromMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('resolves the session through requireAuth, never a hand-rolled getUser()', async () => {
|
||||
enqueue({ data: null, error: null })
|
||||
await GET(req(), createMockRouteParams({ id: validDocId }))
|
||||
expect(requireAuth).toHaveBeenCalledTimes(1)
|
||||
expect(mockSupabase.auth.getUser).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns 400 when id is not a UUID', async () => {
|
||||
|
||||
@@ -1,11 +1,8 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { z } from 'zod'
|
||||
import { requireAuth } from '@/lib/auth/require-auth'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { createServiceClient } from '@/lib/supabase/server'
|
||||
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() })
|
||||
|
||||
@@ -26,88 +23,91 @@ const ParamsSchema = z.object({ id: z.string().uuid() })
|
||||
* 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.
|
||||
*
|
||||
* Wrapped in withRouteContext so auth (MFA on hosted), request ids and the
|
||||
* completion log follow the house pattern. Authorization is still keyed on
|
||||
* the document's own company (a member may probe a document in any company
|
||||
* they belong to, same as document_attachments RLS), not on the wrapper's
|
||||
* active company.
|
||||
*/
|
||||
export async function GET(
|
||||
_request: Request,
|
||||
{ params }: { params: Promise<{ id: string }> }
|
||||
) {
|
||||
const { user, supabase, error } = await requireAuth()
|
||||
if (error) return error
|
||||
export const GET = withRouteContext<{ params: Promise<{ id: string }> }>(
|
||||
'document.integrity',
|
||||
async (_request, { supabase, user, log }, { params }) => {
|
||||
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
|
||||
|
||||
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()
|
||||
|
||||
// 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 })
|
||||
}
|
||||
|
||||
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()
|
||||
|
||||
// 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 (!membership) {
|
||||
return NextResponse.json({ error: 'Document not found' }, { status: 404 })
|
||||
}
|
||||
if (!doc.mime_type) {
|
||||
return NextResponse.json({ data: { valid: true } })
|
||||
}
|
||||
|
||||
if (!doc.mime_type) {
|
||||
return NextResponse.json({ data: { valid: true } })
|
||||
}
|
||||
// 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)
|
||||
|
||||
// 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)
|
||||
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 })
|
||||
}
|
||||
|
||||
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 at most 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)
|
||||
|
||||
// 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,
|
||||
})
|
||||
}
|
||||
|
||||
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 } })
|
||||
}
|
||||
return NextResponse.json({ data: { valid: magicError === null } })
|
||||
},
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user