diff --git a/DECISIONS.md b/DECISIONS.md index 5d0546c5..38db8ae9 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1583,3 +1583,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-04] Supplier-invoice list overflow (#2262) fixed per page, not in a shared list component: none exists (every list hand-writes the overflow-x-auto wrapper) and the three overflow reports had three different causes, so the column budget went into .claude/rules/design.md instead. Fakturadatum was dropped rather than Kvar or the action column (förfaller is the payer's date and the default order, the customer list has no invoice-date column either); the always-visible sort icon from #2091 was kept although it was the proximate regression. [2026-09-04] Parties: the model reads a counterpart only on demand (picker, review list), never when the queue builds: a five-hundred-row queue would cost five hundred calls nobody asked for and a rebuild would repeat them; the reading is a 'model' fact and a search query, never a hard key. The review list ticks rows with exactly one active SCB match but writes nothing until a person approves: an exact legal name plus one active hit is high precision, auto-attaching would still be the system choosing. Exact legal-form names ("Visma Spcs AB", not "Visma") group keys and attach to existing parties: registered company names are unique in Sweden, so this is a key in all but form; the "name never merges" rule keeps applying to fuzzy and form-less names. [2026-09-04] reset_fiscal_year's next_year_dependency no longer counts an opening-balance verifikat in the following year as reliance (migration 20260904163000 redefines fiscal_year_reset_snapshot; the block now fires only when the following year is locked, closed or has its own closing entry). Why: the old check (opening_balance_entry_id / opening_balances_set on the next period) fired for the dominant migration shape, import the first year with its own #IB and later backfill the year before it, so a backfilled year could never be reset (Aisen & Adison AB, 2026-09-03), while a next year WITHOUT an IB, whose balansrapport really rolls from this year's books, was allowed: the check was inverted relative to actual reliance. An IB in the next year is its own verifikat with its own underlag and survives the reset untouched; the one IB that IS derived from this year's books, the bokslut-generated one, is still refused via this year's closing_entry_id (year_end_state). The preview now returns next_period {name, has_opening_balances} and the dialog says the following year's IB stays as it is, instead of a blocker. Rejected alternative: stornoing the next year's IB inside the reset (it would destroy a correct migration boundary and re-create the #1022 dead end). Sibling fix in the same change: CreatePeriodDialog now derives the name from the dates the user types until the name is hand-edited, which is how a 2022-07-01..2023-12-31 year got saved as "Räkenskapsår 2027" (the seed suggestion is always the next forward year). +[2026-09-04] Underlag attach on a folder-picked Fortnox export (Loftux, 50 of 50 files refused with UNDERLAG_REF_MISMATCH): the multipart filename is reduced to its basename at the route boundary (lib/documents/upload-file-name.ts), rather than teaching the voucher-ref parser to strip directories or adding a client-supplied file_name field. Chrome writes webkitRelativePath as the multipart filename for folder selections, so the attach check saw "2026/06/Leverantörsfakturor/A166_x.pdf" while the preview had resolved File.name "A166_x.pdf"; the two endpoints received the same file under two names and the guard compared them. Stripping inside the parser would turn a typed manual ref "2024/01/31" into voucher 31 (the manual box shares the parser), and a second client-supplied name is no more trustworthy than the first, so the boundary is the only level that fixes the class. diff --git a/app/api/import/documents/attach/__tests__/route.test.ts b/app/api/import/documents/attach/__tests__/route.test.ts index be2fdcb5..4bad0dca 100644 --- a/app/api/import/documents/attach/__tests__/route.test.ts +++ b/app/api/import/documents/attach/__tests__/route.test.ts @@ -198,6 +198,59 @@ describe('POST /api/import/documents/attach', () => { expect(uploadDocumentMock).not.toHaveBeenCalled() }) + it('resolves the basename when the browser sends a folder-relative multipart filename', async () => { + // Chrome fills the multipart `filename` with webkitRelativePath for files + // picked through a folder selection, so the server sees the Fortnox export + // tree (///) while the preview, built from + // File.name, saw only the basename. The check must run on the name the + // user reviewed, and the archived document must carry that name, not the + // path. Long exporter suffixes after the ref are part of the same shape. + const res = await POST( + makeRequest({ + fileName: + '2026/06/Leverantörsfakturor/A31_90493_62864442_Hetzner_2026-05-13_089000921156.pdf', + }), + emptyParams, + ) + + expect(res.status).toBe(200) + const [, , , file] = uploadDocumentMock.mock.calls[0] + expect(file).toMatchObject({ + name: 'A31_90493_62864442_Hetzner_2026-05-13_089000921156.pdf', + }) + }) + + it('still refuses a folder-relative filename whose basename points elsewhere', async () => { + // Normalizing the path must not loosen the guard: the basename is checked + // exactly as a bare filename would be. + vouchers = [{ ...VOUCHER, id: OTHER_ID }] + + const res = await POST( + makeRequest({ fileName: '2026/06/Leverantörsfakturor/A31_kvitto.pdf' }), + emptyParams, + ) + const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res) + + expect(status).toBe(409) + expect(body.error.code).toBe('UNDERLAG_REF_MISMATCH') + expect(uploadDocumentMock).not.toHaveBeenCalled() + }) + + it('attaches several underlag to the same verifikat, one request each', async () => { + // Fortnox exports one file per attachment, so a verifikat with six + // receipts is six files sharing a prefix. Each lands on the same target + // under the same entry-scoped idempotency key; the content hash keeps + // them apart as separate documents. + for (const fileName of ['A31_90470_1_faktura.pdf', 'A31_90470_2_kvitto.pdf']) { + expect((await POST(makeRequest({ fileName }), emptyParams)).status).toBe(200) + } + + expect(uploadDocumentMock).toHaveBeenCalledTimes(2) + for (const call of uploadDocumentMock.mock.calls) { + expect(call[4]).toMatchObject({ journal_entry_id: TARGET_ID, idempotency_key: TARGET_ID }) + } + }) + it('returns 400 when the declared fiscal year is missing or not a uuid', async () => { expect((await POST(makeRequest({ declaredPeriodId: null }), emptyParams)).status).toBe(400) expect((await POST(makeRequest({ declaredPeriodId: '2024' }), emptyParams)).status).toBe(400) diff --git a/app/api/import/documents/attach/route.ts b/app/api/import/documents/attach/route.ts index f7903315..c64f14de 100644 --- a/app/api/import/documents/attach/route.ts +++ b/app/api/import/documents/attach/route.ts @@ -4,6 +4,7 @@ import { ensureInitialized } from '@/lib/init' import { withRouteContext } from '@/lib/api/with-route-context' import { uploadDocument, validateDocumentFile } from '@/lib/core/documents/document-service' import { planPermitsAttach } from '@/lib/documents/underlag-import' +import { uploadedFileBaseName } from '@/lib/documents/upload-file-name' import { getErrorMessage } from '@/lib/errors/get-error-message' import { errorResponseFromCode } from '@/lib/errors/get-structured-error' @@ -65,6 +66,14 @@ export const POST = withRouteContext( return errorResponseFromCode('DOC_UPLOAD_NO_FILE', log, { requestId }) } + // The multipart `filename` is not `File.name`. For a folder selection + // Chrome writes the relative path (`2026/06/Leverantörsfakturor/A166_x.pdf`) + // while the preview the user approved was built from `File.name` + // (`A166_x.pdf`). Every use below, the resolver check and the archived + // name alike, must see the name the user reviewed, so it is normalized + // once, here, at the boundary. + const fileName = uploadedFileBaseName(file.name) + const fields = AttachFieldsSchema.safeParse({ journal_entry_id: formData.get('journal_entry_id'), fiscal_period_id: formData.get('fiscal_period_id'), @@ -98,7 +107,13 @@ export const POST = withRouteContext( }) } - const opLog = log.child({ filename: file.name, journalEntryId }) + const opLog = log.child({ + filename: fileName, + journalEntryId, + // Kept only when the browser sent something else, so a refusal can be + // read against exactly what arrived on the wire. + ...(file.name !== fileName ? { uploadedAs: file.name } : {}), + }) // Tenant check first and explicitly: RLS covers the cookie session, but the // link is irreversible, so the route never takes the client's word for which @@ -156,7 +171,7 @@ export const POST = withRouteContext( const permitted = await planPermitsAttach( supabase, companyId!, - file.name, + fileName, journalEntryId, fiscalPeriodId, override, @@ -173,7 +188,7 @@ export const POST = withRouteContext( supabase, user.id, companyId!, - { name: file.name, buffer, type: file.type }, + { name: fileName, buffer, type: file.type }, { upload_source: 'file_upload', journal_entry_id: journalEntryId, diff --git a/lib/documents/__tests__/upload-file-name.test.ts b/lib/documents/__tests__/upload-file-name.test.ts new file mode 100644 index 00000000..c9269859 --- /dev/null +++ b/lib/documents/__tests__/upload-file-name.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest' +import { uploadedFileBaseName } from '@/lib/documents/upload-file-name' + +describe('uploadedFileBaseName', () => { + it('returns a bare filename unchanged', () => { + expect(uploadedFileBaseName('A31_8c2db060.pdf')).toBe('A31_8c2db060.pdf') + }) + + it('strips the folder-relative path Chrome writes for a folder selection', () => { + // Real shape from a Fortnox export: ///. + expect( + uploadedFileBaseName( + '2026/06/Leverantörsfakturor/A166_90493_62864442_Hetzner_2026-05-13_089000921156.pdf', + ), + ).toBe('A166_90493_62864442_Hetzner_2026-05-13_089000921156.pdf') + }) + + it('strips Windows-style separators too', () => { + expect(uploadedFileBaseName('2026\\01\\Verifikationer\\A17_kvitto.pdf')).toBe('A17_kvitto.pdf') + }) + + it('keeps dots, spaces and Swedish characters inside the name', () => { + expect(uploadedFileBaseName('2026/01/A17_Förhandsavi 2026 Årsavgift 734 314 922.pdf')).toBe( + 'A17_Förhandsavi 2026 Årsavgift 734 314 922.pdf', + ) + expect(uploadedFileBaseName('A31.kvitto.v2.pdf')).toBe('A31.kvitto.v2.pdf') + }) +}) diff --git a/lib/documents/filename-voucher-ref.ts b/lib/documents/filename-voucher-ref.ts index 70462e9c..f2c908ad 100644 --- a/lib/documents/filename-voucher-ref.ts +++ b/lib/documents/filename-voucher-ref.ts @@ -129,7 +129,11 @@ const YEAR_LIKE_RE = /^(?:19|20)\d{2}$/ * Trim only. Directory components are NOT stripped: `file.name` from an * `` never carries a path, while the manual-reference box * feeds arbitrary user text through this same parser, where splitting on `/` - * would quietly turn the typed date `2024/01/31` into voucher 31. + * would quietly turn the typed date `2024/01/31` into voucher 31. The one + * place a path does show up is the multipart `filename` of an upload (Chrome + * writes the folder-relative path for folder selections); the attach route + * reduces that to a basename before it reaches here, see + * `lib/documents/upload-file-name.ts`. */ function baseName(fileName: string): string { return fileName.trim() diff --git a/lib/documents/upload-file-name.ts b/lib/documents/upload-file-name.ts new file mode 100644 index 00000000..d9d4515f --- /dev/null +++ b/lib/documents/upload-file-name.ts @@ -0,0 +1,23 @@ +/** + * The name of an uploaded file as the user saw it: the last path segment of + * whatever the browser wrote into the multipart `filename` parameter. + * + * `File.name` in the browser is a bare filename by spec. The `filename` the + * same browser writes into multipart/form-data is not guaranteed to be that + * string: Chrome fills it with `webkitRelativePath` for files that came from + * a folder selection. A receipt picked out of a Fortnox export as + * `2026/06/Leverantörsfakturor/A166_Hetzner.pdf` therefore arrives server-side + * under that whole path, while every client-side read of `file.name`, and so + * every preview the user approved, said `A166_Hetzner.pdf`. + * + * Any server logic that compares an uploaded name to something the client + * computed from `File.name`, or stores the name for the user to read back, + * must go through this first. Both separators are stripped: Chrome writes `/` + * on every platform, legacy Windows clients sent `\`. Nothing else is + * normalized, on purpose: the voucher-ref parser must see the name exactly as + * the exporting system wrote it. + */ +export function uploadedFileBaseName(name: string): string { + const cut = Math.max(name.lastIndexOf('/'), name.lastIndexOf('\\')) + return cut === -1 ? name : name.slice(cut + 1) +}