fix(invoices): scope-check article ids in buildInvoiceWriteData so every invoice write refuses a foreign company's article (part of #2059) (#2339)
Part of #2059 (Part 2, the hardening bug). The FK on invoice_items.article_id proves the article exists, not that it belongs to the writing company: FK validation ignores RLS, and the v1 routes run on the service-role client with no RLS at all. Only the two MCP commit executors checked tenancy; the cookie POST/PATCH, v1 POST/PATCH, webshop and sales-order writers passed items[].article_id straight through the builder. Move the check to the one point every writer converges on: buildInvoiceWriteData collects the distinct article ids from product lines, runs one select scoped on company_id, and refuses with the new INVOICE_CREATE_ARTICLE_INVALID (400, Swedish message via the structured-error registry) on any miss. The MCP executor checks stay as the tamper gate for staged rows. Tests: builder unit cases (miss refused with details, dedupe + happy path, no article ids means no query, DB error surfaces as dbError), cookie PATCH and v1 POST refusal cases, and the existing v1 persist + MCP update tests now answer the builder's scoped select. Claude-Session: https://claude.ai/code/session_019SaJfqNi4VmsG8FMKq99G6 Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Fable 5.1
parent
8313f527c9
commit
ea4da0eb07
@@ -4,10 +4,11 @@ import {
|
||||
createMockRouteParams,
|
||||
parseJsonResponse,
|
||||
createQueuedMockSupabase,
|
||||
makeCustomer,
|
||||
} from '@/tests/helpers'
|
||||
import { eventBus } from '@/lib/events'
|
||||
|
||||
const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase()
|
||||
const { supabase: mockSupabase, enqueue, reset, findCall } = createQueuedMockSupabase()
|
||||
vi.mock('@/lib/supabase/server', () => ({
|
||||
createClient: () => Promise.resolve(mockSupabase),
|
||||
}))
|
||||
@@ -225,4 +226,59 @@ describe('PATCH /api/invoices/[id]', () => {
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('INVOICE_UPDATE_NOT_DRAFT')
|
||||
})
|
||||
|
||||
it('refuses an article id that belongs to another company before writing anything (issue #2059)', async () => {
|
||||
// The FK on invoice_items.article_id proves the article exists, not that
|
||||
// it is this company's; the cookie route relies on the builder's scoped
|
||||
// select for that (RLS never sees FK validation).
|
||||
const FOREIGN_ARTICLE = 'ffffffff-ffff-4fff-8fff-ffffffffffff'
|
||||
enqueue({
|
||||
data: {
|
||||
id: 'inv-1',
|
||||
status: 'draft',
|
||||
invoice_number: null,
|
||||
journal_entry_id: null,
|
||||
is_self_billed: false,
|
||||
credited_invoice_id: null,
|
||||
document_type: 'invoice',
|
||||
quote_status: null,
|
||||
deduction_personnummer_encrypted: null,
|
||||
deduction_personnummer_last4: null,
|
||||
},
|
||||
error: null,
|
||||
}) // invoices: the draft being edited
|
||||
enqueue({ data: makeCustomer({ id: '22222222-2222-4222-8222-222222222222', customer_type: 'swedish_business' }), error: null }) // customers
|
||||
enqueue({ data: { vat_registered: true }, error: null }) // company_settings (builder VAT gate)
|
||||
enqueue({ data: [], error: null }) // articles: no company-scoped hit
|
||||
|
||||
const response = await PATCH(
|
||||
createMockRequest('/api/invoices/inv-1', {
|
||||
method: 'PATCH',
|
||||
body: {
|
||||
customer_id: '22222222-2222-4222-8222-222222222222',
|
||||
invoice_date: '2026-07-14',
|
||||
due_date: '2026-08-13',
|
||||
currency: 'SEK',
|
||||
items: [
|
||||
{
|
||||
description: 'Konsult',
|
||||
quantity: 1,
|
||||
unit: 'tim',
|
||||
unit_price: 1000,
|
||||
vat_rate: 25,
|
||||
article_id: FOREIGN_ARTICLE,
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
createMockRouteParams({ id: 'inv-1' }),
|
||||
)
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string; details?: { invalidArticleIds?: string[] } } }>(response)
|
||||
|
||||
expect(status).toBe(400)
|
||||
expect(body.error.code).toBe('INVOICE_CREATE_ARTICLE_INVALID')
|
||||
expect(findCall('articles', 'eq')).toEqual(['company_id', 'company-1'])
|
||||
expect(findCall('invoices', 'update')).toBeUndefined()
|
||||
expect(findCall('invoice_items', 'insert')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -957,9 +957,11 @@ describe('POST /api/v1/companies/:companyId/invoices', () => {
|
||||
? SWEDISH_BUSINESS_CUSTOMER
|
||||
: table === 'chart_of_accounts'
|
||||
? [{ account_number: '3041' }]
|
||||
: table === 'invoices'
|
||||
? createdInvoice
|
||||
: null
|
||||
: table === 'articles'
|
||||
? [{ id: ARTICLE_ID }]
|
||||
: table === 'invoices'
|
||||
? createdInvoice
|
||||
: null
|
||||
return (r: (v: unknown) => void) => r({ data, error: null })
|
||||
}
|
||||
return () => new Proxy({}, handler)
|
||||
@@ -1100,6 +1102,39 @@ describe('POST /api/v1/companies/:companyId/invoices', () => {
|
||||
const body = await res.json()
|
||||
expect(body.error.code).toBe('INVOICE_CREATE_REVENUE_ACCOUNT_INVALID')
|
||||
})
|
||||
|
||||
it('rejects an article_id that belongs to another company (issue #2059)', async () => {
|
||||
// v1 runs on the service-role client with no RLS: the FK on
|
||||
// invoice_items.article_id proves existence only, so the scoped select in
|
||||
// the builder is the only tenancy guard for article linkage.
|
||||
withInvoiceWriteScope()
|
||||
const FOREIGN_ARTICLE = 'ffffffff-ffff-4fff-8fff-ffffffffffff'
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
customers: { data: SWEDISH_BUSINESS_CUSTOMER, error: null },
|
||||
articles: { data: [], error: null },
|
||||
}),
|
||||
)
|
||||
|
||||
const res = await createInvoice(
|
||||
makePostInvoice(`https://x.test/api/v1/companies/${COMPANY_ID}/invoices`, {
|
||||
customer_id: CUSTOMER_ID,
|
||||
invoice_date: '2026-05-12',
|
||||
due_date: '2026-06-11',
|
||||
currency: 'SEK',
|
||||
items: [
|
||||
{ description: 'x', quantity: 1, unit: 'st', unit_price: 100, article_id: FOREIGN_ARTICLE },
|
||||
],
|
||||
}),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error.code).toBe('INVOICE_CREATE_ARTICLE_INVALID')
|
||||
expect(body.error.details).toEqual({ invalidArticleIds: [FOREIGN_ARTICLE] })
|
||||
})
|
||||
})
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user