diff --git a/DECISIONS.md b/DECISIONS.md index d374f065..d47f4603 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -36,3 +36,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-06] Paywall leak sweep gating choices: SKV unlock (DELETE /declaration/lock) left ungated so a lapsed company can recover a draft it locked while entitled; agi/kontrollera HU/IU gated (direct SKV API interaction = paid, file download stays free); recurring auto-send blocks only the email, invoice creation stays free (freeze-and-retain). [2026-07-07] Sjalvfaktura via the public invoice API (support request: "kan inte hitta det i docs"): exposed the RECEIVED self-billing invoice (mottagen sjalvfaktura, ML 17 kap 15: a SALE, Debit 1510 / Credit 30xx+26xx) on the public API as an OPTIONAL is_self_billed flag on the existing POST /api/v1/.../invoices endpoint (+ external_invoice_number, self_billing_agreement_ref, received_date), on Emil's explicit call ("configure sjalvfaktura when creating an invoice ... optional field") over a dedicated /invoices/self-billed v1 endpoint. First built the WRONG interpretation (issue a self-invoice on a SUPPLIER's behalf = a purchase, on the supplier-invoice stack, new SJ- series + PDF + MCP tool + migration 20260706130000); reverted all of it after Emil clarified the user meant the existing seller/received feature, which already existed internally (/api/invoices/self-billed, cookie-session) but was absent from the public API. No migration for B (is_self_billed/external_invoice_number/self_billing_agreement_ref columns already exist from 20260613100000). Extracted lib/invoices/self-billed-sale.ts (resolveSelfBilledSaleDraft + createSelfBilledSaleInvoice) as the single implementation and refactored the internal route to a thin wrapper over it, so the dashboard "Sjalvfaktura" tab and the API can't drift (internal route test still green, 8/8). Fields added as PLAIN optionals (no z.superRefine) so UpdateInvoiceSchema = CreateInvoiceSchema.omit() keeps working (superRefine turns it into ZodEffects, which has no .omit); "external_invoice_number + received_date required when is_self_billed" enforced in the route instead. Documented the flag in the invoices.create registerEndpoint (description + pitfall) since the whole ask was "can't find it in docs". No git touched; nothing deployed. [2026-07-07] Compliance-review sweep on add/api-and-invoice. FIXED: (1) recurring cron double-send window: replaced the read-only "already ran today" check with an atomic compare-and-set claim on last_run_at (release-on-failure) so two overlapping hourly invocations can't both spawn from the same stale batch row; (2) recurring schedule edit dialog could PATCH auto_send=true for a customer with no email (disabled-but-checked box, async customer load after defaultValues): added a useEffect that forces auto_send=false whenever the effective customer lacks an email, mirroring the manual-select guard; (3) momsdeklaration manual-filing rows: switched Math.round -> Math.trunc (öretal faller bort per SFL 22 kap 1 §; in-repo swedish-sru-filing skill confirms öre are DROPPED not rounded, and this now matches the SRU income-tax path). This narrows the earlier [2026-07-06] "whole-krona rounding is deliberate" decision: whole-krona stays deliberate, but the öre handling is truncation, not round-to-nearest. DECLINED (with rationale): self-billed "Självfakturering" notation + own-voucher-number findings are misdirected: createSelfBilledSaleInvoice books a RECEIVED självfaktura (the counterparty issues the document, we render no PDF), and numberOverride only sets the human-readable verifikat description/tag, not the sequential verifikationsnummer (still assigned atomically by commit_journal_entry). Bank-account mod11 padStart padding is correct: leading zeros contribute 0 and preserve the right-aligned check-digit weighting (already tested vs a real Forex account). AGI/KU10 employer name kept as [2026-07-06] (särskilt-företagsnamn caveat already accepted). Livsmedel 12%->6% (April 2026) reactivation guard declined as a description-keyword food-detector: violates the determinism/never-guess principle, and the one-time pause-all-on-deploy already forces conscious reactivation as the natural checkpoint. +[2026-07-07] bank_file_imports dedup key widened (user_id,file_hash) -> (company_id,file_hash), migration 20260707130000 applied to prod: the old key made a same-user re-import of the same file into a second company resolve the upsert onto the first company's row, which RLS rightly blocked (42501). Mirrors what 20260330130000 did for sie_imports; v1 route's BANK_IMPORT_DUPLICATE_OTHER_COMPANY pre-check removed as obsolete (structured-errors entry kept for API compat). diff --git a/app/api/import/bank-file/execute/route.ts b/app/api/import/bank-file/execute/route.ts index 39f8a8bf..061b4f60 100644 --- a/app/api/import/bank-file/execute/route.ts +++ b/app/api/import/bank-file/execute/route.ts @@ -79,7 +79,7 @@ export const POST = withRouteContext( status: 'processing', date_from: transactions.map((t) => t.date).sort()[0] || null, date_to: transactions.map((t) => t.date).sort().reverse()[0] || null, - }, { onConflict: 'user_id,file_hash' }) + }, { onConflict: 'company_id,file_hash' }) .select() .single() diff --git a/app/api/v1/companies/[companyId]/imports/bank/route.ts b/app/api/v1/companies/[companyId]/imports/bank/route.ts index 7c0a8b60..1f71a869 100644 --- a/app/api/v1/companies/[companyId]/imports/bank/route.ts +++ b/app/api/v1/companies/[companyId]/imports/bank/route.ts @@ -199,57 +199,13 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>( ) try { - // Cross-company collision pre-check. The `bank_file_imports` unique - // constraint is `(user_id, file_hash)`: set when the table was - // designed for the single-tenant single-company-per-user world. If - // the same user is a member of two companies and uploads the same - // file to both, a naive upsert with onConflict='user_id,file_hash' - // would silently overwrite the first company's row with the second - // company_id. Pre-check for that case and surface a structured - // error so an agent sees the explicit conflict instead of a - // silently-stolen row. - // - // A migration to widen the unique constraint to (user_id, file_hash, - // company_id) is the proper fix; that's an engine-PR concern. - const { data: existingImport } = await ctx.supabase - .from('bank_file_imports') - .select('id, company_id, filename, imported_at, status') - .eq('user_id', ctx.userId) - .eq('file_hash', fileHash) - .maybeSingle() - if (existingImport && (existingImport as { company_id: string }).company_id !== ctx.companyId) { - // Log the cross-tenant collision details server-side for operator - // investigation (CC7.2: audit trail), but do NOT echo the other - // company's id or the other import's id back to the caller. Doing - // so would be a cross-tenant enumeration vector (V8.2.1 / CC6.1). - // The caller sees a fixed error code + a generic message; the - // server log carries enough context to debug. - ctx.log.warn('bank import: cross-company file-hash collision', { - fileHash, - attemptedCompanyId: ctx.companyId, - existingCompanyId: (existingImport as { company_id: string }).company_id, - existingImportId: (existingImport as { id: string }).id, - }) - await failOperation( - ctx.supabase, - { - id: op.id, - error: { - code: 'BANK_IMPORT_DUPLICATE_OTHER_COMPANY', - message: 'This file has already been imported into another company by this user.', - }, - }, - ctx.log, - ) - return v1ErrorResponseFromCode('BANK_IMPORT_DUPLICATE_OTHER_COMPANY', ctx.log, { - requestId: ctx.requestId, - // Deliberately empty details: see comment above. - }) - } - // Record the import row so the dashboard's "bank file imports" tab - // shows v1 imports too. `upsert` on (user_id, file_hash) gives - // duplicate-rerun protection for the same-company case. + // shows v1 imports too. The unique constraint is (company_id, + // file_hash) since 20260707130000, so the same user importing the + // same statement into two companies is two independent rows, and the + // upsert gives duplicate-rerun protection within one company. The + // old (user_id, file_hash) key and its cross-company pre-check + // (BANK_IMPORT_DUPLICATE_OTHER_COMPANY) are gone. await ctx.supabase .from('bank_file_imports') .upsert( @@ -264,7 +220,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>( date_from: parseResult.date_from, date_to: parseResult.date_to, }, - { onConflict: 'user_id,file_hash' }, + { onConflict: 'company_id,file_hash' }, ) // Convert parsed transactions to the RawTransaction shape that @@ -288,12 +244,10 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>( raw, ) - // Mark the bank_file_imports row complete. Scope by all three - // identifying fields: `(user_id, file_hash)` is the unique - // constraint today but adding `company_id` is defense in depth: - // even if a concurrent same-user same-hash import in a different - // company slipped past the pre-check, this update can never - // overwrite the wrong company's status row. + // Mark the bank_file_imports row complete. The unique constraint is + // `(company_id, file_hash)` since 20260707130000; scoping the update by + // user_id as well is defense in depth so a concurrent same-hash import + // can never overwrite the wrong company's status row. await ctx.supabase .from('bank_file_imports') .update({ diff --git a/supabase/migrations/20260707130000_bank_file_imports_company_scoped_hash.sql b/supabase/migrations/20260707130000_bank_file_imports_company_scoped_hash.sql new file mode 100644 index 00000000..ea22e2bd --- /dev/null +++ b/supabase/migrations/20260707130000_bank_file_imports_company_scoped_hash.sql @@ -0,0 +1,14 @@ +-- bank_file_imports dedup key: (user_id, file_hash) -> (company_id, file_hash). +-- The old key predates multi-tenancy: it blocked the same user from importing +-- the same statement file into two different companies (upsert resolved onto +-- the other company's row, and RLS correctly rejected the cross-company +-- UPDATE with 42501). The 20260330130000 refactor made this exact swap for +-- sie_imports but missed bank_file_imports. Verified in prod before this +-- migration: no duplicate (company_id, file_hash) pairs exist. +ALTER TABLE public.bank_file_imports + DROP CONSTRAINT IF EXISTS bank_file_imports_user_id_file_hash_key; +ALTER TABLE public.bank_file_imports + ADD CONSTRAINT bank_file_imports_company_id_file_hash_key + UNIQUE (company_id, file_hash); + +NOTIFY pgrst, 'reload schema';