38 Commits

Author	SHA1	Message	Date
Jakob Wennberg	c06395f633	feat(mcp): agent-native API sprint — quick wins (items 8/10/38/39/50) (#505) ... * feat(mcp): agent-native API sprint — quick wins (items 8/10/38/39/50) Five Tier-S items from dev_docs/api_ai_architecture/PLAN.md, picked for highest impact-per-day on a solo budget. ~7.5 engineer-days of work. Item 38 — gnubok_reverse_journal_entry MCP tool. Wraps the existing reverseEntry() engine function (lib/bookkeeping/engine.ts) as a staged high-risk operation. Description distinguishes pure makulering (use this) from rättelse (use gnubok_correct_entry) per BFL 5 kap 5§ guidance — leaving a real affärshändelse unbooked is itself a BFL violation, so agents must understand which storno pattern to apply. New operation_type 'reverse_entry' wired through PendingOperationType, risk-tiers (high), commit.ts executor, and TOOL_SCOPE_MAP (bookkeeping:write). Six executor cases + three staging-gate cases cover the new tool. Item 39 — period_status threading. New helper resolvePeriodStatusForDate() in lib/core/bookkeeping/period-service.ts returns { period_id, status, lock_date } using the same two-layer logic as the v1 REST check (company-wide bookkeeping_locked_through + fiscal_period flags). Threaded through stagePendingOperation via a new dateForPeriodCheck option so agents and widgets can detect locked/closed periods without round-trips. Applied to seven bookkeeping-touching tools: categorize_transaction, create_transactions, create_voucher, approve_supplier_invoice, mark_invoice_as_paid, correct_entry, reverse_journal_entry. Resolution failure is non-fatal — DB triggers stay authoritative. Item 50 — gnubok://company/current expansion. Replaces the metadata-only resource with per-company working memory: active fiscal period status, lock dates, counts (customers, suppliers, open AR/AP, uncategorized transactions), voucher series state across open periods, recency signals (last categorization, last invoice sent, last bank sync), and the next five approaching deadlines. All queries parallelized via Promise.all; payload stays well under 8 KB. Mirrors the context.md pattern from Shipper+Claude's agent-native architecture guidance and prevents the context-starvation anti-pattern. Item 8 — schema strictness. additionalProperties: false on every one of the 67 inputSchemas in extensions/general/mcp-server/server.ts. New strict-schemas.test.ts guards against regression on newly authored tools. CLAUDE.md documents the tool-authoring contract (strict input schemas, description ≤280 chars, STAGED_OPERATION_SCHEMA + next as the completion-signal pattern — do NOT introduce a parallel S/H/C/O envelope). Payload-size ceiling raised from 20K → 25K tokens with a comment pointing at item 15 (Tool Search + defer_loading) as the long-term answer rather than relaxing the watchdog further. Item 10 — prompt cache groundwork. The only Anthropic SDK call site in the codebase is the invoice-inbox extension's Bedrock-backed extractor; tagged the ~3.5 KB SYSTEM_PROMPT with cache_control: { type: 'ephemeral' } and added usage logging (cache_read_input_tokens / cache_creation_input_tokens) so the hit ratio is measurable. The plan's 1h TTL is direct-Anthropic-only; documented the constraint and the MCP-side determinism contract (tool definitions must be byte-stable across requests) in the new mcp-server README.md. Carry-over: includes a small untracked migration (20260516060000_journal_entries_source_type_inbox_item) and its pg test guard that fix a production CHECK-constraint gap for source_type='inbox_item' — unrelated to the sprint but bundled per request. Tests: 3615/3615 pass across 252 files. TypeScript build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): address PR #505 review — cross-tenant leaks, company-wide lock, PII Five reviewer findings on PR #505 addressed: 1. Cross-tenant leak — voucher_sequences (OWASP V8.2.1, SOC 2 CC6.3). Resource query filtered by user_id only; switched to company_id since the table has both (added in the 2026-03 multi-tenant refactor migration). 2. Cross-tenant leak — deadlines (OWASP V8.2.1, GDPR Art.5(1)(f), ISO A.8.3). Same fix; the deadlines table also gained a company_id column in the multi-tenant refactor and the RLS policies enforce it. With the company_id filter active, the userId parameter is no longer needed in the resource — removed from the destructure. 3. Compliance gap — commitReverseEntry and commitCorrectEntry only checked fiscal_periods.is_closed, not company_settings.bookkeeping_locked_through. Agents could stage a reversal with period_status: locked warning (caught by resolvePeriodStatusForDate at staging time), have the user approve, and the commit would slip through. Both executors now run resolvePeriodStatusForDate at commit time so the gate matches the staging-time signal. Pre-existing gap on commitCorrectEntry also fixed. 4. Schema mismatch — period_status was spread into both `preview` and the top-level response, but STAGED_OPERATION_SCHEMA only declares it at the top level. Removed the preview-nested copy to match the schema and avoid ambiguous reads. 5. Tool description — swedish-compliance bot flagged that "pure makulering (storno)" conflates two distinct Swedish accounting terms: storno preserves the original; makulering voids it entirely. Code does storno; description now says so plainly and cites BFL 5 kap. 6. Input hardening — added ^\d{4}-\d{2}-\d{2}$ pattern to reversal_date in inputSchema plus a runtime regex check in execute(), so a malformed date never reaches the pending_operations payload. 7. GDPR — ai_extraction_usage and the two pre-existing fileName log emissions in extract-invoice-fields.ts replaced raw fileName with a 12-char SHA-256 prefix. Raw invoice file names (e.g. "faktura_Sven_Andersson.pdf") can constitute personal data; hashing preserves operator correlation without exposing PII to log destinations that may lack documented retention controls. Notes on findings NOT addressed: - Double-reversal guard (Greptile/swedish-compliance): false positive. reverseEntry() flips the original's status to 'reversed' (engine.ts:538) and the staging tool already rejects anything not 'posted'. Engine also has a CAS guard at lines 541-551. - Staging vs commit TOCTOU re-validation: pre-flight + DB triggers remain authoritative; the window is narrow enough that adding executor-side re-checks isn't load-bearing this sprint. - Runtime Zod validation of args inside execute(): codebase doesn't do this for any MCP tool today; cross-cutting refactor deferred. New test: voucher-executors.test.ts adds a case for the company-wide lock branch on reverse_entry (verifies the new resolvePeriodStatusForDate gate fires when bookkeeping_locked_through covers entry_date). Tests: 3616/3616 pass. TypeScript build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): address second-round PR #505 review — locked_at, reason cap, log Re-review by compliance-swarm and swedish-accounting-compliance bots after the first fixes raised three more legitimate findings: 1. Per-period `locked_at` not directly checked from the fetched row (swedish-accounting-compliance). Both commitCorrectEntry and commitReverseEntry already call resolvePeriodStatusForDate which covers locked_at, but a transient DB blip in the resolve helper would silently skip that gate. Now reading locked_at directly from the inner-join row and checking it alongside is_closed before the resolve helper runs — same pattern, two defense-in-depth layers instead of one. 2. `reason` field had no maxLength (OWASP V4.5). Added maxLength: 500 to the inputSchema and a runtime length check; an adversarial agent could otherwise push an arbitrarily large string into pending_operations. 3. periodStatus resolution failure was silently swallowed (ISO 27001 A.8.15). Now logging via console.warn with operationType, companyId, dateForPeriodCheck, and error so a systematic outage (missing company_settings row, dropped query) is observable in audit logs rather than degraded silently. Findings deliberately NOT addressed (pushed back to the bots): - gnubok_reverse_journal_entry needs per-operation role check (V8.2.1) and narrower 'bookkeeping:reverse' scope (CC6.3) — cross-cutting refactor; no MCP tool in gnubok enforces per-operation roles today. Introducing it just for one tool would be inconsistent. Will surface as a separate item. - Reduce line_description in reverse_entry preview (A.8.3, Art.5(1)(c)) — the preview is shown to the human approver who needs to see what they're approving under BFL 5 kap. Aggregate-only previews would harm the approval workflow. - Audit company-current fields for PII (A.8.12, Art.25(1)) — vat_number, org_number, etc. are intentionally part of working memory; agents need them to make compliant booking decisions. - Payload-size ADR reference (A.8.9) — the test comment already cites plan item 15 (Tool Search) as the long-term answer. - mime_type classification label (CC7.2) — theoretical concern; ai_extraction_usage events are already operator-only. - False positive: commitReverseEntry already has the closed-period check (V2.3); bot was hallucinating. Tests: 3616/3616 pass. TypeScript build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp,env): structured logger + description trim + env alias support Two further follow-ups on PR #505: 1. resolvePeriodStatusForDate catch now uses the structured logger (createLogger from @/lib/logger) instead of console.warn. Three reviewers (compliance-swarm V16.1.1, ISO 27001 A.8.15, SOC 2 CC7.2) independently flagged that console.warn bypasses the centralized log aggregation pipeline used elsewhere, so systemic outages of the period-status resolver were invisible to the SIEM. log.warn now routes through the same sink as other server events. 2. Tool description for gnubok_reverse_journal_entry now routes the refund case explicitly to gnubok_credit_invoice. The Swedish accounting compliance bot flagged that the previous "cancelled credit invoice" example was ambiguous — a real credit invoice flow goes through gnubok_credit_invoice, not this tool. Description stays under 280 chars. 3. lib/init.ts: REQUIRED_EXTENSION_VARS now models each entry as a list of acceptable aliases instead of a single required name. The fallback in extensions/general/enable-banking/lib/jwt.ts already accepts the _PRODUCTION-suffixed variants (used by Vercel prod) as equivalent to the base names, but the env validator at boot didn't, so every cold start in prod warned about missing ENABLE_BANKING_APP_ID even though ENABLE_BANKING_APP_ID_PRODUCTION was set and the runtime was healthy. Each entry now satisfies if ANY listed alias is present; missing entries print all acceptable names so operators can pick either form. Tests: 3616/3616 pass. TypeScript build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): staging tools reject locked_at periods too, not just is_closed Swedish accounting compliance bot flagged that gnubok_reverse_journal_entry and gnubok_correct_entry pre-flight checks only rejected closed periods — locked-but-not-closed periods passed staging and only got rejected at commit time. The commit-time gate was correct (both executors check is_closed AND locked_at AND resolvePeriodStatusForDate), but the staging-time signal was confusing: agent saw staged:true with period_status:"locked" in the same envelope. Now the staging pre-flight reads locked_at from the same inner-join and rejects on either flag, matching the commit-time pattern. The error message updated to "locked or closed" since both branches reach the same throw. BFL 5 kap 5§ alignment is unchanged — both paths still block mutations to locked/closed periods; only the layer at which the rejection fires changes. Findings pushed back (response in PR thread, not addressed here): - companyId/mimeType in log.warn flagged as PII (overreach; tenant IDs are operational identifiers, not personal data, and the codebase logs them consistently elsewhere). - HMAC-keyed file_name_hash instead of plain SHA-256 prefix (overreach; 48 bits already addresses the immediate GDPR Art. 5(1)(f) concern). - 'title' field in deadlines may contain PII (overreach; would require redacting every text field in every read resource). - RLS regression test for voucher_sequences/deadlines (legitimate but pg-test scope; tracked for a follow-up sprint). - Payload-size ADR record (comment already cites plan item 15). - company-current data minimisation (already pushed back; agents need the fields for compliant booking decisions). - Error message conflates "locked" and "closed" — minor UX nit not worth distinguishing here since the remediation step (unlock / omprövning) is the same for the user. - reversal_date period attribution & voucher series integrity flagged as unverifiable from diff — false positives, both already handled by the engine (period_id from original, atomic voucher number). Tests: 3616/3616 pass. TypeScript build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): address Swedish-accounting compliance round 4 — BFL invariant + VAT warning Three legitimate findings from the swedish-accounting-compliance bot acted on (out of five total; two pushed back as theoretical/false positive): 1. BFL 5 kap 5§ invariant assertion (finding 1). The engine guarantees that reverseEntry() posts the storno to original.fiscal_period_id (engine.ts:492 — verified by reading the code), but the executor previously took that on faith. commitReverseEntry now asserts reversal.fiscal_period_id === original.fiscal_period_id after the call and returns a 500 with an explicit "BFL invariant broken" error if the engine ever drifts. New executor test covers this. The reversal_date parameter is unchanged — it's used as the storno's entry_date (operational date), not for period attribution, per BFL practice (entry_date can differ from period_id's range for a rättelse made later). 2. resolvePeriodStatusForDate unhandled-rejection path (finding 2). Both commitCorrectEntry and commitReverseEntry now wrap the resolve call in try/catch, returning a clean Swedish 500 instead of letting the dispatcher surface a raw Postgres error message. Matches the log-and-degrade pattern already used at staging time in stagePendingOperation. 3. VAT-period warning in the reverse preview (finding 4 — swedish-vat). When the original entry contains 2610–2670 BAS accounts, the staged preview now includes a Swedish warnings[] field telling the approver that a storno is legally insufficient if the moms period has been filed with Skatteverket — they must use omprövning per ML 2023:200 instead. Soft warning (not a hard block) since gnubok doesn't track per-VAT-period filing status today; the human decides at approval. Pushed back: - Finding 3 (TOCTOU between staging and commit on fiscal_period_id): posted entries are immutable per the enforce_journal_entry_immutability trigger (migration 20240101000017). fiscal_period_id can't change between staging and commit. Status change is already caught by the status !== 'posted' check. - Finding 5 (migration 20260516060000 not wrapped in BEGIN/COMMIT): Supabase migration tooling runs each migration file in an implicit transaction. PostgreSQL DDL is transactional. The DROP/ADD pair is atomic in practice. The bot acknowledges this as low severity. Tests: 3617/3617 pass (one new — BFL invariant assertion). Build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-16 11:42:47 +02:00
Jakob Wennberg	b94ed3bec2	feat(api): cookbooks + webhook audit_log + secret rotation (PR-500 carry-overs) (#501) ... * docs(api): ship 4 cookbook recipes (close docs polish backlog) Promotes the four placeholder cookbook entries to full narrative recipes matching the Stripe-grade quality bar set by quickstart + webhooks. Closes the docs follow-up bucket from the PR-500 description's deferred list. Recipes: - ingest-bank-transactions: bank-file upload (CSV / CAMT.053 auto-detect) → async poll → list uncategorised → suggest-categories → categorize (single + batch) → match-invoice / match-supplier-invoice. Multicurrency notes covering Riksbanken FX lookup and the kontantmetoden partial- payment guard. - file-vat-declaration: GET /reports/vat-declaration → rutor 05–62 walkthrough → GL reconciliation block → 2026-04-01 livsmedel 12% → 6% transition explicitly covered (delivery_date supply-date rule) → voucher- gap pre-flight → period lock workflow → manual Skatteverket Mina Sidor submission with confirmation-reference capture → EU / reverse-charge / import handling. - run-payroll-and-agi: draft → calculate → approve → mark-paid → book → generate-agi state machine. Per-step idempotency, strict-mode book failure semantics, förmånsbeskattning + bilförmån + bruttolöne­avdrag vs nettolöneavdrag ordering. AGI XML download for manual Mina Sidor upload (direct API submission requires BankID via the Skatteverket extension, not the public REST surface). - year-end-closing: IB/UB continuity check per BFL 5 kap → voucher-gap pre-flight → missing-documents pre-flight → lock (reversible) → year- end async operation (resultatdisposition + periodiseringsfond + överavskrivningar + bolagsskatt + opening-balance batch) → close (irreversible per BFL 5 kap 8 §, typed-phrase confirmation) → årsredovisning + INK2/NE generation. Brutet räkenskapsår variant documented. Each cookbook follows the same shape as the existing quickstart and webhooks recipes — concrete curl commands, response samples, common pitfalls, next-steps cross-links. Lengths are deliberately uneven: the year-end recipe is longest because the consequences of getting it wrong are most severe (BFL violations, irreversible close). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): V16 audit_log entries for webhook lifecycle + secret rotation endpoint Two intertwined changes that together close the "real audit attribution gap in actively-used routes" item from the PR description. 1. POST /api/v1/companies/{companyId}/webhooks/{id}/rotate-secret New endpoint that issues a fresh HMAC signing secret and invalidates the previous one immediately. Returns the new secret EXACTLY ONCE in the response, mirroring the create-time contract. Required scope: webhooks:manage. Idempotency-Key mandatory. Rotation is instant — no grace period. Documented workflow: stage the new secret on the receiver side (separate config slot, not yet active) → POST /rotate-secret → activate the new secret on the receiver → POST /webhooks/{id}/test to verify. A "previous_secret" column with TTL-based grace window (Stripe-style) is the natural follow-up; the instant-rotation shape ships first because it closes the "secret leaked, need to rotate now" use case with minimum new surface. The route is wired into load-routes.ts and lib/auth/scopes.ts. Spec snapshot updated. 2. V16 audit_log entries on every webhook lifecycle mutation The audit_log column shape (user_id, company_id, action, table_name, record_id, actor_id, old_state, new_state, description) is exactly what V16 / Art.32(1)(b) / A.8.24 audit-trail requirements call for. Wired entries on: - POST /webhooks (create) — action INSERT, new_state captures the row WITHOUT the secret (signing material must not land in the audit trail; only secret-event metadata). - PATCH /webhooks/:id (update) — action UPDATE, before/after pair so reviewers can reconstruct exactly what changed. - DELETE /webhooks/:id (delete) — action DELETE, old_state snapshot so the row's prior state survives the delete. - POST /webhooks/:id/rotate-secret — action SECURITY_EVENT, new_state carries the event marker only (no secret value). - dispatcher.disableWebhook (auto-disable on HTTP 410 / redirect / url_unsafe) — action SECURITY_EVENT, before/after capturing the disable cause for SIEM correlation. actor_id is set to ctx.apiKeyId on caller-driven entries so the audit row points back to the specific API key that triggered the change (PR-500 round-1 CC6.3 finding: actor attribution via created_by_api_key_id alone leaves a gap if a key is deleted — keeping the actor_id in audit_log closes that). 4 new integration tests cover the rotate-secret happy path, 404, 401 unauthorized, and Idempotency-Key required. The existing webhook integration tests continue to pass because the audit_log inserts fall through to the default mock response (no-op) without disturbing the per-table queues. 39 integration tests pass on the webhook surface (+4 vs round-2). Total: 3588 unit tests passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-501 review round 1 — correctness + Swedish compliance Round 1 of review fixes. Two real correctness bugs Greptile caught, two audit-trail gaps, and four Swedish-compliance errors in the cookbook prose. Compliance Swarm has 17 findings (0 blocking); the 4 architectural items (secret-at-rest encryption, dedicated rotate scope, rate-limit on rotation, URL redaction) remain deferred with rationale. Greptile (3 / 3 — all addressed): 1. rotate-secret silent 0-row UPDATE — fixed by adding `.select('id').maybeSingle()` to the UPDATE and returning NOT_FOUND when no row was touched. Closes the TOCTOU window between the existence check and the secret update; a concurrent DELETE no longer hands the caller a freshly-generated secret that no webhook in the database matches. 2. DELETE handler audit_log silently skipped when prior snapshot is null — fixed by writing the audit row UNCONDITIONALLY with `old_state: prior ?? null` and a degraded description when the snapshot is unavailable. A successful DELETE now always produces exactly one audit row (CC6.3 attribution contract). 3. Typo "bookslut" → "bokslut" in year-end-closing.ts. Compliance Swarm code-quality items addressed: 4. PATCH new_state now derived from the DB-confirmed returned `data` with an explicit field allowlist, not from the request-body-derived `update` object (A.8.11 / V16.1.1). Closes the gap where a future trigger that rejects a field would leave the audit trail out of sync with the actual stored state. 5. All four route-side audit_log inserts (create, update, delete, rotate-secret) now capture the insert error and emit a structured warning via ctx.log; mirrors the dispatcher pattern (CC7.2). 6. Dispatcher null-user_id path now emits a structured warning instead of silently skipping the audit_log entry — SIEM can alert on the gap (CC7.2 / V16.1.1 / A.8.15). Swedish compliance (cookbook content fixes — all real errors): 7. VAT cookbook ruta 06 label corrected: "Övrig försäljning (ej skattepliktig)" → "Momspliktig försäljning som inte ingår i ruta 05" (Skatteverket's verbatim label). The old label conflated exempt vs zero-rated supplies and would cause integrators to omit export / EU zero-rated sales from box 06. 8. Livsmedel rate-change framing rewritten: leads with the supply-date rule (ML 1 kap 3 §) as the decisive date, not invoice_date. The old opening sentence ("invoices created with invoice_date >= 2026-04-01 book to 2631") was wrong on its face — a copy-paste reader would mis-book pre-cutover deliveries invoiced in April at the new 6% rate. 9. Reverse-charge EU 2645 note adds the blandad-verksamhet caveat: "Net zero impact on cash flow" only holds when full avdragsrätt applies; partial avdragsrätt requires proportional restriction per HFD 2023 ref. 45. 10. Payroll cookbook age bounds corrected: "under-25 / over-66" → "18-22 years old (born 2003-2007) / 67+ from 2026", per Prop. 2025/26:66. The old bounds would cause integrators to apply the reduced rate (20.81%) to 23-24-year-olds who must pay 31.42%, producing non-compliant AGI files. 11. Payroll cookbook BAS 2615 corrected to 2731 (Avräkning sociala avgifter). 2615 is "Utgående moms vid import" in BAS 2026 — using it for the payroll liability would misclassify a payroll payable as an import-VAT payable and break moms reconciliation. 12. Year-end cookbook periodiseringsfond cap base corrected: IL 30 kap 5 § cap is on taxable profit BEFORE the periodiseringsfond deduction itself (and after schablonintäkt is added back). Note on materiellt samband (BFNAR 2016:10 kap 13) added — the reservation is BOOKED on 2110-2139, not declaration-only. Deferred to follow-ups (architectural / out of scope for round 1): - Secret-at-rest encryption (CC6.1 / Art.5(1)(f)): PR-1 architectural carryover, applies to existing webhooks.secret column too. - Dedicated `webhooks:rotate` scope (CC6.3 informational): introduces friction without closing a real gap when the only caller-driven action gated by `webhooks:manage` is the rotation itself. - Per-route rate-limit on :rotate-secret (Art.32 abuse case): part of the wider per-route rate-limit pass already on the deferred list. - webhook_url redaction in audit_log (Art.5(1)(c)): URLs are admin- supplied configuration values with no expected sensitive params; truncation would degrade audit value for legitimate review. 23 webhook integration tests pass locally (no regressions). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-501 review round 2 — atomic mutations + audit completeness + cookbook compliance Round 2 of review fixes. Compliance Swarm flagged refinements to the round-1 fixes; Swedish-compliance had a fresh batch of cookbook items (including a self-contradiction in payroll pitfalls I missed last round). All addressed. Code changes — atomicity + audit completeness: 1. rotate-secret collapsed to a single UPDATE … RETURNING (V8.2.1). The preflight existence-check SELECT was redundant after round 1 added .select().maybeSingle() on the UPDATE — the same null-row signal indicates non-existence, but in one round trip with no TOCTOU window. RETURNING `name` so the audit_log description still carries a human identifier without a second read. 2. DELETE handler collapsed to atomic .delete().select().maybeSingle() (V8.2.1). Eliminates the pre-read TOCTOU window entirely. A 0-row delete (already-deleted webhook) still returns 204 — idempotent DELETE — and the audit entry captures the attempt with old_state: null. Description discriminates the two cases ("deleted: name" vs "delete attempted on missing id"). 3. Cache-Control: no-store, no-cache, must-revalidate, private on the rotate-secret response (Art.25). The HMAC secret is sensitive credential material returned exactly once; this header prevents any intermediary (CDN, proxy, gateway access log, browser cache) from persisting the response body in a store with a different retention policy than intended. 4. Dispatcher auto-disable now writes the audit_log entry UNCONDITIONALLY (A.8.15 / V16.1.1 / CC7.2). Previously a null prior snapshot or a legacy null user_id caused the audit row to be silently skipped — only a warn log was emitted. Now writes user_id=NULL when unavailable (post-multi-tenant-refactor schema allows it; row is invisible under user RLS but queryable under service-role review, which is correct for system-initiated SECURITY_EVENT records). Description discriminates the snapshot- available / snapshot-unavailable cases. Swedish compliance — cookbook content fixes (all real errors): 5. VAT cookbook rounding rule corrected: SFL 22 kap 1 § mandates TRUNCATION of öre (Math.floor for positive amounts), not half-up rounding. Last round mislabeled this as "Math.round (half-up)"; the SRU filing skill is canonical and uses truncation. Using Math.round would produce values that differ from Skatteverket's expectations and cause GL-reconciliation mismatches at the öre level. 6. VAT reconciliation block now includes 2614 (Utgående moms vid omvänd skattskyldighet, matches ruta 30). The previous list of 2611/2621/2631/2641/2645 omitted 2614; a reconciliation that skips it would show rutor_match_gl: true even when the 2614 balance is non-zero and un-reconciled. 7. Livsmedel rate-change adds a one-sentence caveat for continuous/ subscription supplies — the supply-date framing in round 1 was too tight for cases where multiple deliveries roll up into a subscription. Confirms against ML 1 kap 3 § rather than assuming a single delivery date is decisive. 8. Payroll pitfalls bullet contradicted step 2 — "Employees under 26 (2024 rule for 2026 birth year ≥ 2001)" rewritten to match step 2: "18–22 years old at the start of 2026 (born 2003–2007) AND 67+ from 2026". An integrator reading only the pitfalls section would have applied the reduced rate too broadly, producing underpaid arbetsgivaravgifter and a non-compliant AGI. 9. Year-end periodiseringsfond cap now states schablonintäkt explicitly: 1.94% × outstanding prior-year balance (SLR + 1% for 2026) is ADDED to taxable income before the 25% cap is computed. Last round mentioned the "BEFORE the periodiseringsfond deduction" ordering but elided the schablonintäkt step; omitting it produces a cap that's too low when prior-year reserves exist. 10. Year-end SRU format characterization corrected: SRU is plain text encoded in ISO 8859-1, NOT XML. iXBRL (XML-based) is the Bolagsverket digital annual-report format — a separate artefact for a separate authority. Round 1 conflated them. Deferred (architectural / out of scope, documented in commit): - Audit-log dead-letter queue / SIEM alert escalation (Art.32 / A.8.15): infra setup, not code-PR scope. The warn-on-failure path is the in-process surface; durable delivery is a SRE/SIEM concern. - Secret encryption at rest (CC6.1): PR-1 architectural carryover. - webhook_url + description redaction in audit_log (Art.5(1)(c)): URLs are admin-supplied configuration values; redaction would degrade audit reconstructibility without closing a real PII gap. - PATCH old_state TOCTOU via Postgres function (CC6.3): the read- then-write pattern produces an append-only audit row capturing the read state; the small race window is non-load-bearing for audit purposes and a stored-procedure refactor exceeds the cost/value. 23 webhook integration tests pass locally (no regressions). Type-check clean for all changed files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-501 review round 3 — real cookbook tax errors + cache-control on create Round 3 closes two tax-impact errors in the cookbooks plus the consistency gap on the create response. Compliance Swarm's remaining findings are recurring architectural carryovers or oscillation against prior rounds. Real cookbook errors (would mislead integrators): 1. Schablonintäkt rate corrected. Round 2 hardcoded 1.94% — that's the 2024 rate (SLR 0.94% + 1%). For 2026 SLR is 2.55%, so the rate is 3.55%. A wrong rate produces a too-low add-back, a too-high periodiseringsfond cap, and an IL 30 kap compliance error for any integrator copying the cookbook number. Rewrite to describe the formula (SLR + 1%, where SLR is the Riksbank statslåneränta on 30 Nov of the preceding year) with the 2026 figure as an example, and note the engine reads the canonical rate from `tax_rates`. 2. SRU format is a TWO-file pair, not one. Round 2 correctly said "plain text encoded in ISO 8859-1 (NOT XML)" but described it as a single file. Skatteverket requires both INFO.SRU (metadata header) AND BLANKETTER.SRU (declaration body) uploaded together — a single-file upload is rejected by their validation. Fix the prose to describe the two-file pair explicitly. Code consistency: 3. POST /webhooks (create) now returns the same `Cache-Control: no-store, no-cache, must-revalidate, private` + `Pragma: no-cache` headers as the rotate-secret endpoint (A.8.12). Both endpoints return the HMAC secret exactly once; both need the same intermediary-cache prevention. Smaller cookbook refinements (round 3 bot follow-ups): 4. VAT reconciliation block now includes 2615 (Utgående moms vid import, matches ruta 60) — the previous list covered 2611-2645 but omitted import VAT. A reconciliation that skips 2615 would show rutor_match_gl: true falsely for any importer. 5. Service supply-date fallback statement qualified to "one-off service supplies where delivery and invoice coincide" — long- running service contracts (subscriptions, maintenance) have per-delprestation skattskyldighet and need an explicit delivery_date per billing cycle. 6. Payroll elder-reduction boundary clarified: "67 years or older AT THE START OF the income year (1 January 2026)" — a 66-year- old whose 67th birthday falls in February does NOT qualify in 2026. Prevents misreading the pithy "67+ from 2026" as a birthday-during-year rule. Bot oscillation (skipping with rationale documented here for posterity): - Compliance Swarm Art.25 now asks to REMOVE webhook_url from DELETE old_state — direct contradiction with CC6.3's round-1 ask for complete attribution. webhook_url is admin-supplied configuration, not PII; keeping it preserves audit reconstructibility. - Swedish-compliance flags the unconditional re-delete audit row as "polluting" the behandlingshistorik — direct contradiction with Compliance Swarm V8.2.1 + CC6.3 round-1 / round-2 asks for unconditional writes. The audit_log is operational, not BFL räkenskapsinformation (which lives on journal_entries and related tables under explicit immutability triggers). Audit trail completeness wins over BFL purity for this table. Architectural carryovers (already documented in earlier commit bodies as deferred to follow-up PRs): - Secret encryption at rest (CC6.1, recurring) - Audit-log dead-letter / SIEM alerting (Art.32 / A.8.15, infra) - webhook_url userinfo stripping (A.8.11 low — URLs are admin- configured, no expected credentials; validating at registration would be a registration-time concern, not audit-time) 23 webhook integration tests pass. Type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-15 21:31:47 +02:00
Jakob Wennberg	e9e0fd726f	feat(api): Phase 6 PR-1 — webhooks substrate (delivery pipeline + routes) (#496) ... * feat(api): Phase 6 PR-1 — webhooks substrate (delivery pipeline + routes) First half of the final API plan phase. Ships the webhook delivery substrate end-to-end: schema, in-process fan-out from the event bus, per-minute Vercel cron dispatcher with HMAC signing + exponential backoff, and the seven v1 routes that let an integrator manage subscriptions and replay failed deliveries. Mirrors the architectural shape of Phase 4 PR #469 (new substrate + register routes + cron worker + audit table with immutability trigger). Migration (supabase/migrations/20260515170000_webhooks_v2.sql): - Repurpose automation_webhooks → webhooks. Drops the legacy UNIQUE (company_id, event_type) — multiple receivers per event are valid (Stripe pattern). Adds name, description, secret, created_by_api_key_id, api_version_pinned, disabled_at, disabled_reason. Backfills any pre-existing rows with a placeholder secret before the NOT NULL constraint is added. - New webhook_deliveries table — pending|in_flight|delivered|failed| dead state machine, attempts + next_attempt_at fields for the dispatcher, response_status/body/headers capture for receiver-side debugging, partial-index on (next_attempt_at) WHERE status IN ('pending','failed') for the worker pickup. - BFNAR 2013:2 kap 8 § immutability: BEFORE UPDATE trigger blocks writes when OLD.status IN ('delivered','dead'). The :retry route bypasses this by INSERTing a fresh row pointing at the same payload, never mutating the terminal one. - RLS: members SELECT own-company deliveries; writes restricted to service role. lib/webhooks/{handler,dispatcher,signing,diff}.ts: - handler.ts subscribes to 24 public CoreEventTypes and inserts one webhook_deliveries row per active subscription matching (company_id, event_type). Wired into ensureInitialized() via registerWebhookHandler() so every API route that emits events also enqueues webhook deliveries — same module-level pattern as the supplier-invoice and event-log handlers. - dispatcher.ts is the per-minute cron worker. Claims up to 50 due rows, POSTs each one with HMAC signature, updates row to delivered (2xx), failed (other → bumps next_attempt_at by exponential backoff), or dead (HTTP 410 OR attempts exhausted). HTTP 410 additionally auto-disables the webhook. 10s request timeout, 4 KB response-body cap. Backoff: 1m / 5m / 30m / 2h / 12h / 24h / 48h (7 retries, ~72h total) — matches Stripe. - signing.ts: Stripe-style X-Gnubok-Signature: t=<unix>,v1=<hex> with HMAC-SHA256 over `${t}.${rawBody}`. Constant-time verify with default 5-min tolerance window for the cookbook examples. generateWebhookSecret() returns 256 bits of crypto-random hex. - diff.ts: computePreviousAttributes() for Stripe-style update events. Stubbed in PR-1 (every emit passes null); each route's emit() call site captures the prior row in a follow-up so receivers don't need a second GET. v1 routes (app/api/v1/...): - /companies/{companyId}/webhooks GET (list) + POST (create) - /companies/{companyId}/webhooks/{id} GET / PATCH / DELETE - /companies/{companyId}/webhooks/{id}/test POST :test - /companies/{companyId}/webhooks/{id}/deliveries GET (cursor-paginated) - /webhook-deliveries/{id}/retry POST :retry POST /webhooks generates the HMAC secret server-side and returns it EXACTLY ONCE in the response — every subsequent endpoint omits it (same shape as the existing api_keys table). Idempotency-Key required on POST; dry-run supported. PATCH active=false manually pauses (sets disabled_at + disabled_reason = 'manually_disabled'); active=true clears the disable bookkeeping that the dispatcher's HTTP-410 auto-disable may have set. event_type is immutable — delete and recreate to change. POST /webhook-deliveries/{id}/retry lives outside /companies/{id}/ because callers reference deliveries by id; tenancy is enforced inside the handler via company_members lookup. Re-enqueues by INSERT (immutability trigger blocks in-place mutation), so the original row stays in the audit log. /api/webhooks/dispatch/cron: - withCronContext-wrapped, CRON_SECRET-guarded. - Returns dispatch summary { picked, delivered, failed, dead } in the body so an operator can grep Vercel logs to see per-tick throughput. - Per-minute schedule added to vercel.json (* * * * *). lib/auth/scopes.ts: webhooks:manage scope (already in API_KEY_SCOPES since the catalogue placeholder was added pre-Phase-6) extended with :test, :deliveries, and :retry route entries. Substrate-only by design. The PR's review-round commits will add: - claim_due_webhook_deliveries(p_now, p_limit) SQL function for proper FOR UPDATE SKIP LOCKED claim (current select-then-update has a tight CAS race window that the partial index narrows but a SQL function tightens further). - Integration tests under app/api/v1/companies/[companyId]/webhooks/__tests__/ covering list, create-returns-secret-once, list-never-returns-secret, PATCH active toggle, DELETE cascade, :test enqueue, :retry rejects non-terminal status, IDOR (cross-company), missing-Idempotency-Key, scope-deny. - *.pg.test.ts for the immutability trigger (CLAUDE.md mandate for any PR touching a trigger / RLS policy). - 30-day TTL cleanup cron for webhook_deliveries (same shape as the existing event_log cleanup at /api/events/cleanup/cron). Phase 6 PR-2 ships the docs polish (cookbook suite, error reference, signature-verify samples in Node + Python, versioning + deprecation policy, llms-full.txt rebuild, spec-snapshot test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 1 — 4 real bugs + retention FK Fixes the 4 real bugs Greptile flagged on the round-1 review of the Phase 6 PR-1 webhooks substrate, plus the swedish-compliance-bot finding about 7-year audit retention on accounting-event delivery rows. Compliance Swarm noise items are documented inline (see end of this commit body) rather than ping-ponged. FIXED — real bugs: 1. **dispatcher: SELECT-then-UPDATE double-delivery race** (lib/webhooks/dispatcher.ts:claimDueDeliveries) The previous implementation returned the full SELECT result set regardless of whether the CAS UPDATE actually claimed any rows. Per-minute Vercel cron has best-effort single-instance semantics — under load (50 deliveries × 10s timeout = up to 500s > 60s) the next tick can fire while this one is still running and pick up the same SELECT batch. Both ticks would then dispatch the same deliveries. Fix: have the UPDATE return the IDs it actually claimed via `.select('id')`, intersect with the candidate set, and only dispatch that intersection. The CAS guard `(status IN ('pending','failed'))` ensures at most one tick wins for any given row. 2. **dispatcher: `clearTimeout` called before response body read** (lib/webhooks/dispatcher.ts:attemptDelivery) The AbortController timeout was cleared before `readBoundedText`, so a slow body stream could stall the entire serial dispatch batch indefinitely. Fix: move the clearTimeout to a `finally` block AFTER the body read so the abort stays armed across the whole HTTP cycle. 3. **signing: `verifySignature` throws RangeError on invalid hex** (lib/webhooks/signing.ts) The guard compared hex-string lengths before calling timingSafeEqual, but `Buffer.from(v1, 'hex')` silently drops invalid hex bytes — a v1 that is the right hex length (64 chars for SHA-256) but contains non-hex characters decodes to a SHORTER buffer than `expected`. timingSafeEqual then throws RangeError instead of returning false. Receivers using this helper to verify inbound webhook signatures would crash on a forged or corrupted header instead of cleanly rejecting it. Fix: compare buffer lengths AFTER decoding. 4. **GET /webhooks response shape mismatch** (app/api/v1/companies/[companyId]/webhooks/route.ts) The handler passed a flat array to `paginated()`, producing `data: [...]`, but the registered WebhooksListResponse schema and the inline example both document `data: { webhooks: [...] }`. Any client built against the spec would not find the expected key. Fix: switched from `paginated()` (which is for top-level array payloads) to `ok()` and wrapped as `{ webhooks: data ?? [] }` to match the schema. The webhook-count ceiling per company is bounded, so dropping cursor pagination on this surface is fine for v1.0. FIXED — swedish-compliance: 5. **Webhook DELETE no longer destroys accounting-event audit trail** (supabase/migrations/20260515180000_webhook_deliveries_retention.sql, app/api/v1/companies/[companyId]/webhooks/[id]/route.ts, lib/webhooks/dispatcher.ts) swedish-compliance-bot flagged that ON DELETE CASCADE on webhook_deliveries.webhook_id let a webhook DELETE silently remove terminal delivery rows that constitute behandlingshistorik for accounting events (journal_entry.committed, period.locked, salary_run.booked, agi.generated, ...). BFNAR 2013:2 kap 8 § requires 7-year retention of these rows. Fix: new migration changes the FK to ON DELETE SET NULL and makes webhook_id nullable. Webhook DELETE now leaves the delivery audit trail in place — it just loses the back-reference to the no-longer- existing webhook row. The dispatcher SELECT was updated to filter `webhook_id IS NOT NULL` so dangling pending/failed rows go dormant in the audit trail rather than retrying against nothing. Documentation updated on the DELETE route header + endpoint description + pitfall list to reflect the new semantic. FIXED — defense in depth: 6. **Retry route: re-verify webhook still belongs to caller's company immediately before INSERT** (app/api/v1/webhook-deliveries/[id]/retry/route.ts) Compliance Swarm V8.2.1 (medium) flagged that the retry endpoint verified tenancy via the delivery's company → company_members lookup, then INSERTed a fresh delivery without re-checking that the parent webhook still existed in that company at INSERT time. A webhook deleted between the membership check and the INSERT would have left a dangling row; a webhook re-registered to a different company would let the caller redeliver to a webhook they never created. Fix: explicit re-fetch of the webhook scoped to (id, company_id) immediately before INSERT, with NOT_FOUND if the webhook is gone or VALIDATION_ERROR if it's been disabled. DEFERRED — documented inline: - **OWASP V14.2 plaintext webhooks.secret**: Inline rationale added to lib/webhooks/signing.ts:generateWebhookSecret(). Outbound HMAC signing requires the original byte sequence on every delivery, so one-way hashing is precluded by definition. Stripe / GitHub / Slack / Twilio all follow the same pattern. Defense in depth: service- role-only writes on webhooks, column-level select projection on every read endpoint (the row never includes secret outside the create response), Supabase encryption-at-rest. Re-evaluate when KMS-backed signing becomes available without per-call latency cost. - **Compliance Swarm V13.2 cron uses CRON_SECRET only**: false positive — matches the documented Vercel cron pattern used by every other cron in the project (deadlines, invoice reminders, document verify, sandbox cleanup, event log cleanup, ...). - **Compliance Swarm V1.2 cursor pagination injection**: false positive — `decodeDefaultCursor` in lib/api/v1/pagination.ts already validates `ts` against a strict ISO 8601 regex and `id` against a UUID regex, returns null otherwise. The bot couldn't see the helper's internals. - **Compliance Swarm V8.2.1 retry-route TOCTOU on tenancy** (high): the secondary company_members lookup is deliberate — the route lives outside /companies/{id}/ tree because callers reference deliveries by id (already noted in the file header). The defense- in-depth tightening at INSERT time (item 6 above) closes the practical TOCTOU window. Round-2 may add an atomic DB function if swarm escalates this. - **Compliance Swarm V2.4 no rate limits on :test / :retry**: defer to Phase 6 PR-2 alongside the per-route rate-limit pass we owe across the v1 surface (Phase 3 deferral list). - **Compliance Swarm V16 audit logging on webhook secret generation / deletion**: defer to Phase 6 PR-2 (audit-event durability is on the Phase 6 architectural-floor list per Phase 4 lessons-learned). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 2 — SSRF, tenancy, retention triggers Round 2 of the Phase 6 PR-1 review cycle. Compliance Swarm went 23 → 24 between rounds (oscillation pattern documented in Phase 4 lessons). This commit fixes 7 real items, four of them surfaced by the round-1 commit opening up new attack surfaces / new audit gaps. FIXED: 1. **SSRF: webhook_url HTTPS-only + private/loopback/link-local/CGNAT/ metadata IP rejection** (V12.1, V1.2, CC6.6) New helper lib/webhooks/url-guard.ts validates webhook_url at three layers: - Zod schema (Create + Patch) rejects non-https before the handler runs. - Route handler runs validateWebhookUrl() which performs DNS lookup and rejects IPs in 10/8, 172.16/12, 192.168/16, 127/8, 169.254/16 (link-local + AWS/GCP/Azure metadata 169.254.169.254 explicitly classified), 100.64/10 (CGNAT), 0/8, plus IPv6 ::1, fc00::/7, fe80::/10, and IPv4-mapped IPv6 ::ffff:<v4> via recursive reclassification. - Dispatcher re-runs the same check immediately before each outbound POST — DNS rebinding / record swap between webhook creation and dispatch is the common bypass and the create-time check alone is insufficient. A failure at dispatch time marks the delivery dead with reason='url_unsafe:<class>' AND auto- disables the webhook. The dispatch-time check adds one DNS lookup per delivery, which is acceptable on the per-minute cron with batches up to 50. 2. **Cross-tenant dispatch refusal** (A.8.3) loadWebhooksByIds now selects company_id alongside id/webhook_url/ secret. The dispatch loop asserts webhook.company_id === delivery.company_id BEFORE signing. A poisoned delivery row pointing at another tenant's webhook (compromised service-role write, future buggy code path) is refused with status='dead' and reason='cross_tenant_mismatch' rather than dispatched with the wrong tenant's secret. 3. **DB-level invariants for retention + tenancy** (supabase/migrations/20260515190000_webhook_deliveries_db_guards.sql) Two triggers the application can never bypass: - block_webhook_delivery_terminal_delete (BEFORE DELETE): raises check_violation when OLD.status IN ('delivered','dead'). Closes the BEFORE UPDATE-only loophole the round-1 immutability trigger left open. BFNAR 2013:2 kap 8 § retention is now enforced against DELETE as well as UPDATE. - assert_webhook_delivery_company_match (BEFORE INSERT): raises check_violation when NEW.company_id doesn't match the parent webhooks.company_id. Mirrors the application-layer dispatcher assertion at the database boundary so even a misbehaving service-role caller can't enqueue a cross-tenant delivery. webhook_id IS NULL bypasses the check (dangling rows from webhook DELETE under the round-1 ON DELETE SET NULL FK have no parent to compare against). 4. **Stuck in_flight row recovery** (operational, swedish-compliance note) Before claiming new rows, dispatcher sweeps in_flight rows whose updated_at is older than 2× REQUEST_TIMEOUT_MS back to 'failed' with next_attempt_at = now. A cron killed mid-flight (Vercel function timeout, hard crash, manual termination) would otherwise leave rows marked in_flight forever, violating the audit trail's "every row reaches a terminal state" invariant. 2× REQUEST_TIMEOUT_MS gives an unambiguous "this is stuck, not in-flight" boundary — a live attempt cannot exceed REQUEST_TIMEOUT_MS plus the body read. 5. **Response-body content-type filter + header allowlist** (CC7.2, A.8.12, Art.32(1)(b)) readBoundedText now drops response_body unless Content-Type starts with text/plain or application/json — receivers returning HTML error pages routinely echo PII, request bodies, or stack traces back from their error renderers, all of which would land in our delivery audit log otherwise. Bytes are still drained so the connection stays reusable. headersToObject now filters to a small allowlist (content-type, content-length, date, server, x-request-id, cf-ray). Set-Cookie, Authorization, WWW-Authenticate, and vendor x-* headers are dropped before persistence. 6. **Test payload data minimisation** (Art.25(2)) The :test event payload no longer includes api_key_id. The X-Gnubok-Delivery header on the outbound request already correlates to the audit trail on the gnubok side, so the receiver gains nothing from seeing an internal credential identifier. 7. **Silent-drop log promoted to error** (PI1.3) handler.ts:fanOutToWebhooks logs at error (not warn) when an event payload is missing companyId. Every CoreEvent payload variant types companyId as required, so a missing value indicates an emit-site bug that silently breaks webhook delivery — must be visible in monitoring, not buried in routine warn-noise. DEFERRED (remaining oscillation, documented in commit body): - **V14.2 / Art.5(1)(f) plaintext webhooks.secret**: documented inline in lib/webhooks/signing.ts as accepted-risk per Stripe / GitHub / Slack precedent. The bot will continue to flag it every round; the documented decision is the established pattern. KMS integration is a cross-cutting concern that touches the auth layer too — not a Phase 6 PR-1 scope. - **Art.5(1)(e) 90-day TTL cleanup cron for non-accounting deliveries**: on the deferred list, ships in Phase 6 PR-2 docs/cron suite. - **V2.4 rate limits on :test and :retry**: deferred to Phase 6 PR-2 alongside the v1-wide rate-limit pass (Phase 3 deferral list). - **V16 audit log on webhook secret/delete lifecycle**: deferred to Phase 6 PR-2. - **A.8.24 plaintext secret in migration backfill log**: false positive, the migration comment notes "no production rows" so no real backfill ever runs. Compliance Swarm count expected to drop from 24 → ~10–14 on round 3 as the SSRF + cross-tenant findings clear together. Architectural floor is the V14.2 plaintext-secret oscillation + V16 audit-event-durability (deferred to PR-2) — that's the merge-ready signal per Phase 4 lessons. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 3 — 5 fixes + migration consolidation Compliance Swarm went 24 → 16 (5 high / 8 medium / 3 low) after round 2, clearing the SSRF + cross-tenant cluster as predicted. Round 3 closes the remaining real items, leaving the architectural floor (V14.2 plaintext, V16 audit log, V2.4 rate limits, Art.5(1)(e) TTL — all deferred to Phase 6 PR-2). That's the documented merge-ready signal. FIXED: 1. **Deliveries list — webhook ownership pre-check** (V8.2.1 medium) GET /webhooks/{id}/deliveries already filters by (company_id, webhook_id) so a cross-tenant id returns nothing, but emitting an explicit 404 when the webhook doesn't belong to the caller's company matches the pattern used for :retry and :test (round 2 fix not propagated to deliveries) and gives a clean signal vs a confusing empty list. Defense in depth alongside RLS. 2. **url-guard: enumerate ALL DNS records** (V1.2 medium) Replaced single dns.lookup with parallel dns.resolve4 + dns.resolve6. A hostname with two A records [public, private] returns either non-deterministically per call — single-lookup validation could return the public IP at create time and the private IP at dispatch. Multi-record enumeration rejects if ANY resolved address is unsafe. Per-family ENODATA / ENOTFOUND is normal (v6-only or v4-only host) and treated as "no records of that family" rather than hard failure; other DNS errors propagate. New 'no_dns_records' reason for the case where neither family resolves anything. The DNS-rebinding window between dispatch-time validation and the actual fetch remains — closing it requires a custom HTTPS agent that pins the resolved IP, tracked for follow-up. Multi-record enumeration shrinks the practical bypass surface substantially. 3. **markDead no longer stamps delivered_at** (swedish-compliance) delivered_at means "the receiver acknowledged the event". For dead rows (HTTP 410, attempts exhausted, webhook deleted, cross-tenant mismatch, unsafe URL) the receiver did NOT acknowledge — leaving delivered_at NULL keeps audit semantics clean. An auditor querying `WHERE delivered_at IS NOT NULL` correctly sees only genuinely delivered rows. The terminal-state timestamp lives on `updated_at` (auto-stamped by the table's BEFORE UPDATE trigger). 4. **Elevated scope check for salary/agi event subscriptions** (swedish-compliance, GDPR Art.32) Subscribing to salary_run.* or agi.generated routes personnummer + lönesummor + skatteavdrag to an external receiver — payroll-grade exposure. POST /webhooks now requires BOTH webhooks:manage AND payroll:read for these event types. A key minted only for webhook management can no longer reach the payroll surface; integrators building payroll integrations must mint a key with the payroll scope alongside webhook management. The check uses a regex (^salary_run\.|^agi\.) so future payroll event types automatically inherit the gate. Same pattern will extend to other sensitive event families when they ship. 5. **Migration consolidation: fold retention into 170000** (swedish-compliance) The round-1 retention migration (20260515180000) was a follow-on that ALTERed the FK from ON DELETE CASCADE to ON DELETE SET NULL. swedish-compliance flagged that if 170000 ever applied in isolation (rollback of 180000, partial replay), CASCADE would silently delete accounting-event audit rows. Edited 170000 to declare the FK with ON DELETE SET NULL and nullable webhook_id directly. Deleted 180000. Migration 190000 (DB guards from round 2) updated to reference 170000 as the source of the SET NULL FK. All in-code references to "20260515180000" updated to "20260515170000" (DELETE route header, dispatcher comments). Net result: a single migration shipping a correct table from the start, no chained ALTER, no isolation risk. DEFERRED (architectural floor, all bound for Phase 6 PR-2): - **V8.2.1 retry ctx.userId may be null for API-key callers**: false positive — validateApiKey unconditionally returns a real userId; the wrapper sets ctx.userId = auth.userId for every authenticated call. - **V1.2 DNS rebinding TOCTOU between validate and fetch**: high-effort proper fix needs a custom HTTPS agent that pins the resolved IP. The multi-record check substantially shrinks the practical bypass window; full closure tracked for PR-2 hardening. - **V16.1 cross-tenant log not in security-event taxonomy**: this project doesn't have a separate security-event log substrate — log.error with structured fields is the established pattern. - **V4.3 dispatch summary in cron response body**: same shape every other cron uses (deadlines, invoice reminders, document verify, ...). CRON_SECRET-gated; project pattern. - **V5.3 / Art.5(1)(f) response_body returned to API callers**: already addressed by round-2 content-type filter — only text/plain or application/json gets persisted. Residual oscillation; the bot didn't see the new filter. - **Art.5(1)(e) 90-day TTL non-accounting deliveries**: Phase 6 PR-2 cron suite. - **Art.32(1)(b) / V14.2 plaintext webhooks.secret**: established defer, documented inline in signing.ts (Stripe / GitHub / Slack precedent). - **swedish-compliance company_id FK CASCADE**: system-wide pattern (every per-company table cascades on company delete). Cross-cutting compliance decision, not webhook-specific. - **swedish-compliance period.unlocked emitted before DB commit**: cross-cutting refactor of the entire event-bus emit pattern across every v1 route. Project-wide concern, not Phase 6 PR-1 scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 4 — 2 critical fixes + 5 hardening Two critical items + 5 supporting hardening fixes. The criticals were both surfaced after round 3 — one by the Supabase preview build, one by swedish-compliance — and would have caused real failures in production. CRITICAL: 1. **Supabase Preview reconciliation broken by round-3 migration deletion** Round 3 deleted supabase/migrations/20260515180000_webhook_deliveries_ retention.sql after folding its FK fix into 170000. The Supabase preview branch had already applied 180000 and tracks the set of applied remote migrations — when a previously-applied filename disappears locally the preview build fails with "Remote migration versions not found in local migrations directory". Fix: restored 180000 with the original idempotent ALTER content. On a fresh install 170000 creates the FK with SET NULL directly so 180000's ALTER is a no-op (DROP IF EXISTS + ADD with the same constraint shape). On the existing preview branch the second run is also a no-op — the FK already has the SET NULL shape from the original 180000 application. Idempotent retro-application is intentional; documented in the file header. 2. **`recoverStuckInFlight` queries a column that doesn't exist** swedish-compliance bot caught that lib/webhooks/dispatcher.ts: recoverStuckInFlight filters `.lt('updated_at', stuckBefore)` against webhook_deliveries.updated_at, but migration 170000 never declared the column. The query would return zero rows at runtime; stuck in_flight rows would stall forever, breaking the BFNAR 2013:2 kap 8 § audit-log completeness guarantee that every delivery row must reach a terminal state. Fix: new migration 20260515200000_webhook_deliveries_updated_at.sql adds the column with NOT NULL DEFAULT now() and wires it to the project-wide update_updated_at_column() trigger function. The new trigger runs BEFORE UPDATE — the immutability check_violation guards from migrations 170000 + 190000 fire FIRST on terminal rows, so no audit-row mutation can occur via the timestamp bump. HARDENING: 3. **Dispatcher: fetch redirect: 'error'** (V1.2 medium) A receiver returning 3xx could redirect the dispatcher to a private/internal address AFTER the SSRF guard validated the original webhook_url. Pass redirect: 'error' so any redirect throws and the delivery enters the failed/retry path with a clean diagnostic. Receivers that legitimately move endpoints should ask integrators to update the webhook URL via PATCH. 4. **Defensive ctx.companyId early-return** (V8.2.1 medium) The deliveries list route used `ctx.companyId!` non-null assertion. The wrapper guarantees companyId for routes inside /companies/{id}/, but a misconfiguration would silently produce `WHERE company_id = NULL` (always-empty result) rather than a hard auth failure. Added an explicit early INTERNAL_ERROR return when ctx.companyId is falsy. Drops the `!` everywhere in the file. 5. **Per-delivery structured logs** (V16 low) Added info/warn-level outcome logs at the dispatch loop boundary with deliveryId, webhookId, companyId, eventType, attempt fields. Per-tenant audit-trail reconstruction now works from log aggregation alone without grepping individual mark*-helper writes. Failure types (delivered / failed / dead) emit at correct levels; webhook auto-disable surfaces as a distinct warn line. 6. **Strip userId from outbound webhook payloads** (Art.5(1)(c)) New minimisePayload() in handler.ts drops the internal Supabase auth.users.id UUID before insert into webhook_deliveries. The companyId stays (it's the tenant scope, useful for multi-tenant receivers). Centralising the projection means future tightening (e.g. stripping personnummer fields from payroll payloads if those ever land in the payload shape) goes here, not per-emit-site. 7. **Migration legal citations** (swedish-compliance precision) swedish-compliance noted the citations conflated BFL 7 kap (the 7-year retention period) with BFNAR 2013:2 kap 8 § (audit-log integrity). Both apply but they're distinct grounds. Updated comments in 170000 and 190000 + the trigger error message in 190000 to cite both correctly. REMAINING DEFERS (architectural floor — Phase 6 PR-2 territory): - **V14 / Art.32 plaintext webhooks.secret**: established defer per Stripe / GitHub / Slack precedent; documented inline in signing.ts. - **V8.2.1 retry endpoint userId may be null for API-key callers**: false positive — validateApiKey unconditionally returns a real userId; ctx.userId is always set after auth. - **V1.2 DNS rebinding TOCTOU between validation and fetch()**: high- effort fix needs a custom HTTPS agent that pins the resolved IP. Multi-record check (round 3) + redirect: 'error' (this round) substantially shrink the practical bypass window. Full closure is Phase 6 PR-2 hardening. - **V2.3 dry-run rate limiting**: Phase 6 PR-2 with the v1-wide rate-limit pass. - **V16.1 cross-tenant log not in security-event taxonomy**: project doesn't have a separate security-event log substrate. - **Art.9 DPIA entry for outbound payroll webhooks**: out-of-repo documentation work, tracked separately. - **Art.5(1)(e) 90-day TTL non-accounting deliveries**: Phase 6 PR-2 cron suite. - **swedish-compliance company_id FK CASCADE**: system-wide pattern; cross-cutting decision, not webhook-specific. - **swedish-compliance period.unlocked emit-before-commit**: cross- cutting refactor of every v1 route's event-bus emit timing. Compliance Swarm count expected to drop materially as the V1.2 + V8.2.1 + V16 cluster clears. If the next round plateaus at the documented architectural floor (~5–9 findings, all in the deferred list above), that's the merge-ready signal per Phase 4 lessons. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 5 — 5 small fixes (audit gaps + hardening) Round 5 closes the actionable items round 4 surfaced. Compliance Swarm went 16 → 23 between rounds (severity dropped — 0 critical, 5 high, 10 medium, 8 low — the bot is now surfacing low-severity items it skipped before; classic plateau approach). Round 5 fixes 3 real gaps + 2 documentation-precision items, all small. FIXED: 1. **`request_id` populated at every webhook_deliveries INSERT site** (swedish-compliance — BFNAR 2013:2 kap 8 § behandlingshistorik) The webhook_deliveries.request_id column was declared in migration 170000 with the documented intent of correlating each delivery row back to the originating API request, but no INSERT call site ever set it — the column was always NULL, breaking audit-trail traceback. - test/route.ts and retry/route.ts now stamp ctx.requestId. - handler.ts:fanOutToWebhooks (the async fanout from the event bus) can't recover the originating request id — the event bus emit is decoupled from the route's request context. Synthesised a 'whfan_<uuid>' batch correlation id so the column is never NULL and rows from the same emission can be grouped. Threading the originating request_id through the event payload itself is a future-direction improvement (would require touching every emit site across the v1 surface). 2. **Retry route re-runs minimisePayload before INSERT** (A.8.12 medium) The retry endpoint was inserting o.payload verbatim — a delivery from before the round-4 minimisation tightening would have its unminimised payload re-delivered on retry. minimisePayload exported from handler.ts; retry now applies it. Idempotent on already- minimised payloads, so no semantic change for current data. 3. **Stuck-recovery sweep guarded against terminal-row race** (swedish-compliance — operational integrity) recoverStuckInFlight filtered status='in_flight' but Postgres applies the predicate to the CURRENT row state at UPDATE time. A row that raced from in_flight to delivered/dead between SELECT and UPDATE would be picked up by the bulk UPDATE; the BEFORE UPDATE immutability trigger would then raise check_violation, aborting the ENTIRE bulk UPDATE statement and leaving legitimately stuck rows unrecovered. Added `.not('status', 'in', '(delivered,dead)')` as defense in depth. The sweep is now safe across mixed batches even when one row terminalizes mid-flight. 4. **'server' header dropped from response_headers allowlist** (A.8.12 low) Receiver infrastructure version strings (nginx/1.21.6, Apache/2.4.41, ...) carry no diagnostic value but routinely leak into a multi- tenant audit table. Removed from SAFE_RESPONSE_HEADERS. 5. **Migration citations narrowed: don't over-claim BFL on non-accounting rows** (swedish-compliance — legal precision) The immutability triggers apply uniformly to all terminal delivery rows, but BFL 7 kap 1 § retention only applies to rows derived from räkenskapsinformation (journal_entry.*, period.*, salary_run.booked, agi.generated, invoice.paid, supplier_invoice.paid). For non- accounting events (customer.created, document.uploaded, transaction.categorized, webhook.test) the same lock applies as gnubok's operational audit-log integrity policy — NOT as a BFL obligation. Updated comments in 170000 and the trigger error message in 190000 to draw the distinction; BFNAR 2013:2 kap 8 § audit-log integrity continues to apply uniformly. REMAINING DEFERS (architectural floor — Phase 6 PR-2): - V14 / Art.32 / V9.1 / A.8.24 / CC6.1 plaintext webhooks.secret (5 separate findings of the same documented-defer item; established Stripe / GitHub / Slack precedent inline in signing.ts). - V8.2.1 retry endpoint userId may be null for API-key callers — false positive, validateApiKey unconditionally returns userId; bot has re-flagged 5 rounds in a row (entrenched oscillation). - V1.2 cursor pagination injection — false positive, decodeDefaultCursor validates ISO 8601 + UUID via regex. - V13 cron secret verification — false positive, withCronContext validates Authorization: Bearer. - V1.2 DNS rebinding TOCTOU — high-effort fix needs custom HTTPS agent pinning resolved IP. Multi-record check (round 3) + redirect: 'error' (round 4) substantially shrink the practical window. Phase 6 PR-2. - V2.4 rate limits on :test / :retry — Phase 6 PR-2 v1-wide pass. - V16.1 / A.8.15 / A.8.16 / CC7.2 SIEM / log drain / monitoring — out-of-repo infra, tracked separately. - Art.5(1)(e) 90-day TTL non-accounting deliveries — Phase 6 PR-2. - Art.9 DPIA entry for outbound payroll webhooks — out-of-repo doc. - Art.25(2) payload field-level redaction (response_body for payroll events) — defensive defer; current emit-site payloads don't carry personnummer or salary fields per the CoreEvent type definitions. - swedish-compliance company_id FK CASCADE — system-wide pattern, cross-cutting decision. - swedish-compliance period.unlocked emit-before-commit — cross- cutting refactor of every v1 route's event-bus emit timing. - PI1.3 SELECT-then-UPDATE claim race — already addressed in round 1 with the CAS-then-intersect pattern. Bot's recommended SQL function approach is the documented round-1 follow-up. Compliance Swarm count expected to plateau in the 12–18 range — all remaining items either deferred to PR-2, recurring oscillation false positives, or cross-cutting concerns outside the webhook surface. That's the documented merge-ready signal per Phase 4 lessons-learned. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 6 — 3 small fixes (last actionable items) Closes the 3 genuinely-new actionable items round 5 surfaced. Every remaining swarm finding now falls into one of: established Phase 6 PR-2 defer (V14 plaintext, V2.4 rate limits, V1.2 DNS rebinding, Art.5(1)(e) TTL, V16/A.8.15/A.8.16/CC7.2 SIEM), oscillation false positive (V8.2.1 retry userId, V1.2 cursor, V13 cron secret), already-addressed (Art.5(1)(c) response_body content-type filter, response_headers allowlist, BFL citation narrowing), or cross-cutting (FK CASCADE, period.unlocked emit timing, plaintext secret variants × 5). FIXED: 1. **`granted_scopes` removed from INSUFFICIENT_SCOPE response details** (Art.5(1)(f) medium) POST /webhooks elevated-scope error echoed the API key's full scope set back to the caller and into ctx.log structured fields. Required scope alone is sufficient for the caller to understand what they need; the granted set is sensitive and should not surface in error envelopes or logs. 2. **Redirect error → terminal `dead` + auto-disable** (CC6.7 medium) Round 4's redirect: 'error' on fetch causes the runtime to throw a TypeError when the receiver returns 3xx. The catch was mapping it to retryable 'failed', so a stubborn-redirect receiver burned all 8 retry attempts (~72h) before going dead. Detect the redirect-shaped error message and short-circuit to dead + auto-disable, mirroring the HTTP 410 treatment. Operator surfaces the misbehaving receiver immediately rather than after three days of log noise. Detection uses /redirect/i on the error message — Node's undici has used several wordings ('unexpected redirect', 'redirect mode is set to error', etc.) across versions; case-insensitive substring is the stable shape. 3. **Retry route re-runs `validateWebhookUrl` against current URL** (CC6.6 medium) The retry handler verifies the webhook's existence + active state + tenancy match, but never re-ran the SSRF guard against the webhook's CURRENT url. A URL changed via PATCH between the original delivery and this retry call would slip a fresh delivery row into the queue that the dispatch-time guard would only catch on the next cron tick. Validating in the retry handler refuses the request up-front with VALIDATION_ERROR — the audit trail gets a clean refusal rather than a deferred 'dead' row with reason='url_unsafe'. REMAINING (architectural floor — not blocking merge): - 5 plaintext webhooks.secret findings (V14 / V11.1 / Art.32 / A.8.24 / CC6.1) — established Stripe / GitHub / Slack precedent, documented inline in signing.ts. - V8.2.1 retry endpoint userId may be null for API-key callers — false positive, validateApiKey unconditionally returns userId. Bot has re-flagged 7 rounds in a row. - V1.2 cursor pagination injection — false positive, decodeDefaultCursor validates ISO 8601 + UUID via regex. - V13 cron secret verification — false positive, withCronContext validates Authorization: Bearer. - V8.2.1 deliveries cross-webhook leak — false positive, bot acknowledges the .eq('webhook_id') filter handles it. - V1.2 DNS rebinding TOCTOU — Phase 6 PR-2 (custom HTTPS agent that pins resolved IP). - V2.4 rate limits on :test / :create / :retry — Phase 6 PR-2 with v1-wide rate-limit pass. - V16.1 / A.8.15 / A.8.16 / CC7.2 SIEM / log drain / monitoring — out-of-repo infra. - Art.5(1)(c) response_body / response_headers — already addressed by round-2 content-type filter + round-2 allowlist + round-5 'server' drop. - Art.5(1)(e) 90-day TTL non-accounting deliveries — Phase 6 PR-2. - Art.25(2) per-event-type field projection (personnummer / lönesummor) — current CoreEvent type definitions don't carry these fields; defensive defer. - Art.9 DPIA / RoPA entries for outbound webhooks — out-of-repo doc. - A.8.28 computePreviousAttributes diff — previous_attributes is null in PR-1; populated in follow-up. - A.5.17 / V11.1 secret in response logged — depends on whether the logging middleware captures response bodies (it doesn't, per project pattern). Defensive defer. - CC9.2 TLS validation / CC3.2 credential-pattern scrub — out-of-scope hardening. - swedish-compliance company_id FK CASCADE — system-wide pattern, cross-cutting decision. - swedish-compliance period.unlocked emit-before-commit — cross- cutting refactor of every v1 route's event-bus emit timing. - swedish-compliance non-terminal accounting row delete — defensible: pending/failed transition to terminal within minutes; blocking deletes there would prevent legitimate cleanup. - swedish-compliance BFL citation in trigger error message — addressed in round 5 (narrowed to "audit-log integrity policy" with BFL only attaching to accounting-event rows). If round 7 plateaus or the count drops, that's the merge-ready signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-15 14:00:04 +02:00
Jakob Wennberg	523a8650cc	feat(api): Phase 5 PR-3 — reports + import async (final Phase 5 PR) (#490) ... * feat(api): Phase 5 PR-3 — reports + import async (final Phase 5 PR) Combines the originally-planned PR-3 (import) and PR-4 (reports) into one final Phase 5 PR per the user's "split into two PRs" scoping after PR-2. 16 new endpoints, 12 new tests, 1 shared helper. 502 v1+salary tests total (was 490 before this PR). Endpoints (16): **JSON reports (14):** - trial-balance, balance-sheet, income-statement, general-ledger, journal-register, vat-declaration, monthly-breakdown, ar-ledger, supplier-ledger, continuity-check, salary-journal, avgifter-basis, vacation-liability — all wrap existing `lib/reports/*` generators byte-equivalently with the dashboard. - New shared helpers (`lib/api/v1/report-period.ts`): - `loadPeriodFromQuery(request, ctx)` — parse + validate the `period_id` query param, fetch the fiscal_periods row scoped to the caller's company, return a discriminated result so the route either gets a typed period or a pre-built 400/404 response. - `safeGenerate(fn, ctx)` — wrap a lib generator call in a try/catch that surfaces a structured REPORT_GENERATION_FAILED instead of letting the raw error leak. - Net effect: each report route stays at ~50 lines of business logic while preserving complete OpenAPI documentation per endpoint. **Binary report (1):** - sie-export: returns text/plain UTF-8 SIE4 content with Content-Disposition: attachment. OWASP V3.2 sanitisation strips everything but [0-9a-fA-F-] from the period_id before splicing into the filename header. **Async imports (2):** - POST /imports/sie: multipart, 50 MB cap, 5-minute maxDuration. Auto- detects encoding (CP437/Windows-1252/UTF-8), parses, dedupes by SHA-256 hash, then calls executeSIEImport(). Records lifecycle on the `operations` table for `GET /operations/{id}` polling. Returns the 202 envelope from `accepted()`. - POST /imports/bank: multipart, 10 MB cap. Auto-detects format across 11 bank format modules (SEB, Swedbank, Handelsbanken, Nordea, Nordea Business, Lansforsakringar, Lunar, ICA Banken, Skandia, CAMT053, generic CSV) — or honors a `format` override. Calls `ingestTransactions()` with the parsed transactions; updates the `bank_file_imports` row to completed; emits `transaction.synced` per ingested row through the standard ingest path. Same operations table polling shape. Both imports execute INLINE today. A future cron worker can take over by flipping `initialStatus` from `'running'` to `'queued'` in startOperation — the response contract stays identical. Deferred to a follow-up (each has lib-module structure quirks that warrant their own focused PR): - `kpi` — composition of multiple lib generators rather than wrapping one - `audit-trail` — lives in lib/core/audit/ not lib/reports/ - `ne-bilaga` + `ink2` — each has its own subdir + engine layer - `periodisk-sammanstallning` — JSON + CSV variants with complex params - PDF variants of balance-sheet / income-statement / etc. — agents can render from JSON; binary PDF is nice-to-have not must-have for v1 Tests: - 12 new integration tests (route-layer contract: auth/scope, period_id validation, the shared loadPeriodFromQuery helper, the safeGenerate error path, sie-export Content-Type + Content-Disposition, vat- declaration query-param validation, generator pass-through). The lib functions have their own unit tests; route tests focus on the wrapper. - 502 total v1 + lib/salary tests pass. - Type-check clean. 3 new structured-error codes: SIE_IMPORT_DUPLICATE, BANK_IMPORT_FAILED, BANK_FILE_FORMAT_UNKNOWN. Plus the existing SIE_PARSE_FAILED / SIE_IMPORT_FAILED / BANK_FILE_NO_TRANSACTIONS reused. Plan doc updated to mark Phase 5 complete (3 PRs shipped: PR-1 registers, PR-2 lifecycle, PR-3 reports+imports). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-490 review round 1 — 3 Greptile P1 bugs + 5 defensive items Compliance Swarm landed at 17 findings (5 high + 10 medium + 2 low) on the first round; Swedish bot at 8; Greptile flagged 3 inline P1 bugs. CI all green from the first push. FIXED — Greptile P1 bugs (all 3 confirmed real): - **VAT declaration cross-field bounds** (app/api/v1/.../reports/vat-declaration/route.ts). The schema validated `period` as 1-12 for every period_type. A caller could pass period_type=quarterly + period=7 (or yearly + period=5) and the route would forward garbage to calculateVatDeclaration — the agent might submit a nonsensical declaration to Skatteverket. Added a .superRefine() that enforces: monthly → 1-12, quarterly → 1-4, yearly → must equal 1. Swedish-compliance bot flagged the same concern independently. - **SIE import options JSON.parse unguarded** (app/api/v1/.../imports/sie/route.ts). The route inlined `JSON.parse(optionsRaw)` inside the Zod safeParse call. A malformed options string threw SyntaxError before Zod ran, producing an unhandled 500 instead of the documented 400 VALIDATION_ERROR. Wrapped in an explicit try/catch that returns a structured 400 with the parse-error message. - **Bank import upsert conflict key cross-company collision** (app/api/v1/.../imports/bank/route.ts). The `bank_file_imports` unique constraint is (user_id, file_hash) from the single-tenant single-company-per-user era. If the same user uploads the same file to two companies they're a member of, the second upload's upsert (with onConflict='user_id,file_hash') would silently overwrite the first row's company_id. Added a pre-check that loads the existing row by (user_id, file_hash) and returns BANK_IMPORT_DUPLICATE_OTHER_COMPANY (409) if the company_id differs. The proper fix is a migration widening the unique index to (user_id, file_hash, company_id) — engine-PR-queue concern. FIXED — defensive items from Compliance Swarm V2.2, V5.2: - **General-ledger account_from/account_to validation** (V2.2). The query params were passed straight through to the generator without format checks. Added a `^\d{3,8}$` regex (covers 4-digit BAS today + sub-account schemes up to 8 digits). - **SIE import file header sanity check** (V5.2). Before invoking parseSIEFile we now check the first 4 KiB of the decoded content for at least one of #FLAGGA / #PROGRAM / #FORMAT / #SIETYP — the mandatory SIE4 header records. An HTML / executable / JSON payload that got past the multipart filter would lack all of them and gets a structured 400 SIE_PARSE_FAILED instead of being fed to parseSIEFile. FIXED — doc / metadata corrections (Swedish bot): - **VAT description**: Expanded the rutor list from "05/10/11/12/30/31/ 32/39/40/48/49" to include the import-VAT rutor 20-24, 35-36, 50, and 60-62. Matters because agents read the description to decide what fields to map; an incomplete list causes agents to omit import VAT. - **Continuity-check citation**: Replaced the wrong "BFL 5 kap 7 §" citation (which is rättelse, not IB/UB continuity) with the correct derivation — BFL 5 kap (löpande bokföring) + BFNAR 2013:2 + SIE4 spec's #IB(N) = #UB(N-1) invariant. - **Vacation-liability description**: Clarified that the "sums to BAS 2920" guarantee only holds when no employees use `semesterersattning` (which is expensed immediately, not accrued). The exclusion of vacation_rule='semesterersattning' and 'none' was already mentioned in pitfalls; now the legal-basis text is consistent. DOCUMENTED (architectural floor / dashboard parity / engine concerns — not changed): - **V8.2.1 path-based tenant check** (4th repeat across phases). The wrapper resolves companyId from the URL AND verifies company_members membership before any handler runs. - **V5.2 bank file magic-byte check**: defensible defense-in-depth, but the dashboard's /api/import/bank-file/parse uses the same content- + filename + format-module detection pattern. Diverging in v1 would break parity. Tracked for a cross-cutting "tighten upload validation" PR. - **V16 error log internals leak**: the error responses do surface err.message in the operation_id error envelope, but this is the intentional contract for an integrator polling operations/{id}. Stack traces are not included. - **Art.32 SIE raw fileContent persisted**: the executeSIEImport helper receives the raw content for hash + parse purposes; whether it persists it beyond the import transaction is an engine-layer concern. Tracked. - **Art.25(1) journal-register + general-ledger no pagination**: dashboard parity. The reports are designed to return the period's full content because period-bounded reports have natural size limits (a single fiscal year). Cursor pagination would diverge from dashboard behavior. - **Swedish: SIE export UTF-8 vs CP437**: legacy SIE consumers (BL Administration, older Hogia/Visma) want CP437. The dashboard serves UTF-8 today and modern SIE consumers accept it. Diverging in v1 would break parity. If real-world legacy-consumer demand surfaces, add a `?encoding=cp437` override; not building on speculation. - **Swedish: SIE #FLAGGA mutation, bank_file_imports mutability**: schema + engine concerns; v1 mirrors dashboard behavior. - **Swedish: avgifter-basis age-tier verification**: requires reading the lib generator's internals; tracked. 1 new structured-error code: BANK_IMPORT_DUPLICATE_OTHER_COMPANY (409). Test count: 261 v1 (unchanged — fixes are internal). 502 across v1 + lib/salary. Type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-490 review round 2 — IDOR fix + bank format enum + calendar date validation + 3 doc fixes Compliance Swarm went 17 → 12 between rounds (high count 5 → 2 — the three Greptile P1s from round 1 dropped out cleanly). 6 actionable items this round; the rest are recurring architectural-floor noise documented in the PR-1/PR-2 commit pattern. FIXED (security): - **V8.2.1 / CC6.1 — BANK_IMPORT_DUPLICATE_OTHER_COMPANY IDOR leak** (app/api/v1/.../imports/bank/route.ts). The round-1 fix added a pre- check that returned the cross-company collision details (existing_ company_id + existing_import_id) in the error response body — that's a cross-tenant enumeration vector. The fix now logs those details server-side for operator investigation (CC7.2 audit trail) but returns ONLY the fixed error code + the generic message to the caller. The agent learns "file already imported into another company" but never sees the other company's UUID. - **V2.2 / PI1.1 — bank format query param allowlist** (app/api/v1/.../imports/bank/route.ts). The route cast `url.searchParams.get('format')` directly to `BankFileFormatId` without validation. Now validated against an explicit Zod enum of all 11 accepted format ids before reaching parseBankFile / detectFileFormat. Unknown values fail fast with 400 VALIDATION_ERROR + a helpful list of accepted values. FIXED (correctness): - **A.8.28 — as_of_date calendar validity** (ar-ledger + supplier-ledger routes). The regex `^\d{4}-\d{2}-\d{2}$` matched '2026-13-45'. Now we also round-trip through Date(): construct with the date string, check the ISOString re-extraction equals the input. Catches month/day/leap-year invalidity without pulling in a date library. FIXED (docs — Swedish bot + Compliance Swarm): - **VAT example block** completed to include all rutor (60/61/62 import VAT + 20-24 + 35-36 + 50). The round-1 description was extended; this round extends the example so an agent reading the OpenAPI spec sees the complete contract. - **salary-journal description** — clarified that `paid`-but-unbooked runs are excluded. Matters for AGI-vs-ledger reconciliation: an operator checking the lönejournal against AGI will see a gap for any paid run that hasn't been booked yet. - **bank import description** — added an explicit BFL 5 kap 1 § note that `ingestTransactions` creates transaction rows (the underlag) NOT verifikationer (the bookings themselves). Operators relying on this endpoint as their "bookkeeping is complete" signal would be wrong; the transactions still need matching/categorization to become verifikationer. DOCUMENTED (architectural floor / recurring / engine concerns — not changed): - **V5.2 magic-byte upload validation** (5th repeat across phases). Dashboard pattern; magic-byte inspection would diverge from the internal /api/import/bank-file/parse behavior. Tracked for a cross-cutting upload-validation hardening PR. - **V16 err.message reflection** (2nd repeat). The integrator-facing contract for an operations.failed result deliberately includes the reason — agents need actionable info to retry vs abort. Removing err.message would be a regression for debuggability. - **A.8.28 / CC6.1 parser DoS on large SIE/bank files**. Bounded by the 50 MB / 10 MB file caps + 5-min maxDuration. A pathological 50 MB SIE file caps the line count at ~5M lines (10 bytes per line minimum); the parser is sync and hits the route timeout long before exhausting memory. - **CC7.2 log injection via err.message**. Best-effort logging by design; structured fields include fileHash + operationId (server-safe) and the message tag is fixed. - **Swedish: SIE export UTF-8 vs CP437** (2nd repeat — dashboard parity). A future `?encoding=cp437` override is the right evolution if real legacy-consumer demand materialises. - **Swedish: #FLAGGA reset to 1, period-occupancy check on SIE import**. Engine-layer concerns inside executeSIEImport. Tracked. Test count: 261 v1 (unchanged — fixes are internal). Type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-490 review round 3 — SIE IDOR symmetry, bank log fields, VAT doc corrections Compliance Swarm went 12 → 26 between rounds — the documented oscillation pattern at its most aggressive (the bot reactivates and finds more speculative items as the actionable ones resolve). 4 small real fixes this round; the rest are recurring noise documented across PR-1/PR-2/PR-3. FIXED (security parity): - **V8.2.1 — SIE duplicate IDOR leak** (app/api/v1/.../imports/sie/route.ts). The bank-import IDOR fix in round 2 removed existing_company_id + existing_import_id from the response details; SIE_IMPORT_DUPLICATE was still echoing existing_import_id + imported_at. Symmetric fix: log forensics server-side (CC7.2 audit trail), return only the error code + generic message to the caller. FIXED (audit log consistency): - **V16 — bank error log missing userId/companyId fields** (app/api/v1/.../imports/bank/route.ts). The SIE error log includes these fields per ASVS V16 audit-record content requirements; the bank error log didn't. Added for consistency. FIXED (Swedish bot doc corrections): - **VAT description: rutor 35/36 don't exist on SKV 4700**. The round-1 expansion incorrectly listed "ruta 35-36 (export + EU services)". SKV 4700 has ruta 39 (export) and ruta 40 (EU services) — there are no boxes 35 or 36. Removed from both the description and the example block. - **ML 13 kap → ML 15 kap**. The kontantmetod citation referenced the pre-2023 chapter. ML 2023:200 replaced ML 1994:200 on 1 July 2023 and moved kontantmetod to ML 15 kap 8–11 §§. Fixed the pitfall text to cite the current statute with a brief explanation of why the old reference appears in older documentation. DOCUMENTED (recurring noise / architectural floor / dashboard parity / feature work — same triage method as PR-1/PR-2/PR-3 prior rounds): - **V5.2 magic-byte upload validation** (6th repeat). Dashboard doesn't do this either. Tracked for a cross-cutting hardening PR if a real attack surface emerges. - **V2.3 / Art.5(1)(f) err.message reflection in API response** (3rd repeat). Intentional contract for operations.failed result — agents need actionable info to retry vs abort. Removing err.message would be a regression for debuggability. The bot framings ("PII leakage" / "implementation detail leak") differ round-to-round but the underlying ask is the same. - **Art.5(1)(c) — z.unknown() response schemas on salary-journal / avgifter-basis / ar-ledger / supplier-ledger** (new framing). Typing every report response would require importing the lib's domain types and would break under future lib changes; the dashboard doesn't enforce typed responses either. Recurring dashboard-parity concern. - **Art.5(1)(f) — cross-tenant log linkage from the round-2 IDOR fix**. The server log carrying who-attempted-what IS the audit trail; log retention + access control are infrastructure-layer obligations (RoPA + log-store ACL), not code-layer. The bot wants me to confirm/document; tracked outside this PR. - **Art.5(1)(f) — filename in SIE error log** (new framing). Marginal: SIE filenames sometimes encode company name + fiscal year, but the route logs them server-side, never reflects in responses. The audit trail is more valuable than the marginal identifying surface. - **Art.25(2) — report endpoint pagination** (2nd repeat). Dashboard returns full-period data; pagination would diverge from parity. A future date-range filter (date_from/date_to) could be added if real callers hit response-size pain. - **Swedish: SIE export UTF-8 vs CP437** (3rd repeat — dashboard parity). - **Swedish: #FLAGGA mutation / IB-UB chain on import / AGI-vs- ledger flag / transactions_pending_booking counter** — all feature work, not bug fixes. Tracked for engine PR queue or future Phase 5.x. Test count: 261 v1 (unchanged — fixes are internal). Type-check clean. Trajectory: 17 → 12 → 26. The count is oscillating widely — not the documented "plateau-then-stop" signal exactly, but the actual finding set is mostly recurring noise. Continuing to fix small real items while the noise stabilises. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-490 review round 4 — 7 small final fixes (defense in depth + doc corrections) Compliance Swarm: 17 → 12 → 26 → **10** between rounds. Round 4 is the plateau-then-stop signal per the documented merge-ready criterion — the count dropped back significantly after round 3's fixes resolved the real items the bot was finding alongside its speculative noise. FIXED (defense in depth): - **V8.2.1 — bank `bank_file_imports` UPDATE missing company_id filter** (app/api/v1/.../imports/bank/route.ts). The cross-company pre-check in round 1 catches the collision case, but the post-ingest UPDATE itself only scoped to `(file_hash, user_id)`. Added `.eq('company_id', ctx.companyId!)` so even a hypothetical race past the pre-check can't overwrite the wrong company's status row. - **V5.2 — SIE header check line-start regex** (app/api/v1/.../imports/sie/route.ts). The round-3 string-contains check would have accepted an HTML payload with `<!-- #FLAGGA -->`. Tightened to require line-start anchoring: `/(^|\n)\s*#(FLAGGA|PROGRAM|FORMAT|SIETYP)\b/`. A SIE header record always starts on its own line per the spec. - **V2.2 — as_of_date year range clamp** (ar-ledger + supplier-ledger routes). Calendar validity (round 2) alone accepts `as_of_date=9999-01-01`. Added a sanity range: year 2000 → currentYear + 1. The +1 tolerance allows year-end filing for the year that just turned over. FIXED (Zod hardening): - **V4.5 — SIE options `.strict()`** (sie/route.ts). The options schema accepts unknown keys; Zod's default strips them, but `.strict()` rejects them with VALIDATION_ERROR so a future schema edit doesn't silently mass-assign through an extension. FIXED (Swedish bot doc corrections): - **Bank pitfall: BFL 5 kap 1 § → BFL 5 kap 6-7 §§**. BFL 5 kap 1 § is the general bokföringsskyldighet; the verifikation content requirements are in 6-7 §§. Important because the pitfall is the legal-citation surface agents consume to understand the compliance boundary. - **Vacation-liability description**: replaced "the 2920 reconciliation only matches when no employees use that rule" — which incorrectly implied a reconciliation failure — with "the 2920 reconciliation is CORRECT whether or not the company has semesterersättning employees, since those employees contribute zero to both the report and the 2920 balance." Same fact, but no longer signals a phantom failure. - **Salary-journal warning**: strengthened the paid-but-unbooked exclusion note to flag that KU preparation from this report can understate wages if any paid runs are still unbooked at KU time (an SFL obligation breach). Now an explicit ⚠️ warning rather than a buried pitfall bullet. DOCUMENTED (architectural floor — same as prior rounds, 3rd-7th repeats): - **V8.2.1 widen `bank_file_imports` unique constraint** — schema migration concern (route-layer pre-check is the mitigation). - **V5.2 magic-byte upload validation** (7th repeat across phases) — dashboard pattern. - **V16.1.1 / CC6.1 err.message / operation_id reflection in API response** (3rd-4th repeat) — intentional contract for operations.failed. - **Swedish: SIE #FLAGGA writeback / SIE UTF-8 vs CP437 (4th repeat) / sequential verifikation numbering** — engine/lib concerns. - **Swedish: VAT formula omits rutor 20-24** — false positive. My formula matches Skatteverket's SKV 4700 spec: ruta 20-24 are EU acquisition BASES (amounts without VAT), not output-VAT rutor. The corresponding output VAT for EU acquisitions goes via reverse charge into rutor 30-32, which my formula already includes. Test count: 261 v1 (unchanged — fixes are internal). Type-check clean. Compliance Swarm trajectory: 17 → 12 → 26 → 10. The round-4 count is the lowest across the four rounds AND matches the architectural-floor pattern documented in the plan (5-9 findings across PR #467, #469, #471 once actionable items are fixed). This PR has reached the plateau-then-stop signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-14 22:50:35 +02:00
Jakob Wennberg	1f89a71962	feat(api): Phase 5 PR-1 — payroll registers (employees + salary-runs CRUD) (#479) ... * feat(api): Phase 5 PR-1 — payroll registers (employees + salary-runs CRUD) 10 endpoints under /api/v1, 35 integration tests. Mirrors Phase 4 PR-1 size and review profile. No engine interaction; no period-lock checks. The lifecycle verbs (calculate / approve / mark-paid / book / generate-agi) ship in Phase 5 PR-2 after the 557-line internal /calculate orchestration is extracted into a shared lib/salary/run-calculation.ts helper. Employees CRUD: - GET/POST /employees + GET/PATCH/DELETE /{id} - Soft-delete via is_active=false (BFL 7 kap retention — the employees table has no archived_at column, deliberately diverging from suppliers and customers) - PATCH drops personnummer changes — identity is immutable post-create - GDPR Art.5(1)(c) personnummer masking: list, create response, and dry-run preview mask to ÅÅÅÅMMDDXXXX. Detail endpoint (deliberate drill-in) returns the full value. EMPLOYEE_DUPLICATE_PERSONNUMMER error never echoes back the supplied value. - Mask helper extracted to lib/api/v1/mask-personnummer.ts Salary-runs CRUD: - GET/POST /salary-runs + GET/PATCH/DELETE /{id} - POST emits salary_run.created - PATCH + DELETE are draft-only with optimistic-lock guards (status filter on the UPDATE / DELETE so a concurrent verb that flips status yields a clean 409 rather than a silent no-op) - PATCH only writes keys explicitly present in the request body to avoid Zod-default overwrite (every PATCH would silently reset is_sidoinkomst=false otherwise) - DELETE is hard delete on the salary_runs row — CASCADE on salary_run_employees and salary_line_items. Only draft runs can be deleted; once :calculate runs the BFL 5 kap immutability applies and storno is the only correction path Scopes: - Reuses existing payroll:read / payroll:write from the MCP tool surface - 16 new endpoint patterns registered in V1_ENDPOINT_SCOPES (10 for PR-1 + 6 placeholders for PR-2's lifecycle verbs and AGI generation) Error codes (12 new structured-error entries): - PR-1 live: EMPLOYEE_NOT_FOUND, EMPLOYEE_DUPLICATE_PERSONNUMMER, SALARY_RUN_DUPLICATE_PERIOD, SALARY_RUN_PATCH_NOT_DRAFT, SALARY_RUN_DELETE_NOT_DRAFT - PR-2 pre-registered: SALARY_RUN_CALCULATE_NOT_DRAFT, SALARY_RUN_APPROVE_NOT_REVIEW, SALARY_RUN_APPROVE_VALIDATION_FAILED, SALARY_RUN_MARK_PAID_NOT_APPROVED, SALARY_RUN_BOOK_NOT_PAID, AGI_GENERATE_NOT_BOOKABLE Tests (35 cases): - Employees: 18 — list with masked pnr, detail with full pnr, create happy path, duplicate-pnr 409 with no echo, dry-run masking, missing Idempotency-Key, wrong-length pnr, A-skatt tax-table requirement, PATCH happy + 404, identity-change drop, soft-delete + idempotent re-delete + 404 - Salary-runs: 17 — list + filter validation + scope rejection, detail + 404, create happy + duplicate-period 409 + period_month range + missing Idempotency-Key + dry-run, PATCH happy + non-draft 400 + 404 + voucher_series regex, DELETE draft + non-draft 400 + 404 Plan doc updated to reflect the 4-PR split for Phase 5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-479 review — disambiguate 23505, mask PATCH responses, return 400 on personnummer-in-PATCH Triage of PR-479 review bots: - **Greptile P1 (`ensureInitialized()` missing on salary-runs/route.ts)** — FALSE POSITIVE. The v1 wrapper at `lib/api/v1/with-api-v1.ts:52` calls `ensureInitialized()` at module load; every v1 route inherits the initialization transitively via the `withApiV1` import. All 10+ existing v1 routes that emit events (suppliers, customers, invoices, supplier- invoices, etc.) follow the same pattern. The wrapper file's own comment documents the centralization. No fix needed; Greptile is applying the CLAUDE.md rule literally without checking the wrapper. - **Greptile P2 (23505 constraint disambiguation)** — FIXED. Both employees and salary-runs POST routes previously mapped every 23505 unique-violation to a single error code (EMPLOYEE_DUPLICATE_ PERSONNUMMER / SALARY_RUN_DUPLICATE_PERIOD). A future migration adding another unique index (e.g. employees(company_id, email)) would have produced misleading errors. Now check `error.constraint` and only map when the constraint name matches the known column. Substring match rather than exact equality so an explicit constraint rename doesn't silently fall through. Added a defensive test asserting that a hypothetical `employees_company_id_email_key` 23505 does NOT get mapped to EMPLOYEE_DUPLICATE_PERSONNUMMER. - **GDPR Art.5(1)(c) — PATCH response + dry-run preview masking** — FIXED. Previously the PATCH response and dry-run preview echoed the full personnummer back via the EmployeeDetail schema. Now both return `personnummer_masked` instead, symmetric with the POST response. Added `EmployeeWriteResponse` schema (EmployeeDetail.omit + extend) so the OpenAPI spec accurately distinguishes GET (full) from PATCH (masked). Added `maskExistingForResponse` helper to drop the raw field and substitute the masked form. The GET drill-in endpoint still returns the full value (deliberate design — caller already has the id). - **SOC 2 PI1.3 — silent personnummer drop on PATCH** — FIXED. PATCH previously dropped any personnummer field in the body via a runtime `delete` after parsing. Caller saw no signal that the intent was rejected. Now return explicit 400 VALIDATION_ERROR with `field: 'personnummer'` and a remediation message ("DELETE and recreate if the natural-person identity has changed"). The Zod schema can't enforce this because `UpdateEmployeeSchema` is shared with the internal dashboard route (which DOES support personnummer updates); the check is route-specific. - **ISO A.5.34 — real-format personnummer in docs/tests** — FIXED. Replaced `198504121234` / `199001019999` / `199012105678` with obviously-synthetic `190001010000` / `190001020000` / `190001029999` (year 1900, day 1, zero-suffix) across the registerEndpoint examples and SAMPLE_PERSONNUMMER test fixture. Still passes the `^\d{12}$` schema regex, but no longer looks like a real birthdate that could be mistaken for production-format PII in CI artefacts or doc renders. Findings explicitly NOT addressed in this commit (and rationale): - **Detail endpoint returns full personnummer + bank account** (multiple bots: GDPR Art.5(1)(c), ISO A.8.11, SOC 2 CC6.1). INTENTIONAL design. The detail endpoint is the deliberate drill-in for callers who already have the id and the `payroll:read` scope. Matches the dashboard's internal /api/salary/employees/[id] behavior. Splitting into a separate `payroll:admin` scope is a CC6.3 architectural decision deferred (same as the Phase 4 `payroll:read` vs `payroll:write` split — fine-grained tiers haven't been justified by integrator demand yet). - **calculation_params shape (Art.5(1)(b) / CC2.1)** — DEFERRED to Phase 5 PR-2. PR-1 only READS the column; the column is WRITTEN by the lifecycle verbs (PR-2's :calculate). PR-2 will define the typed shape and revisit whether the public response shape should expose it. - **F-skatt re-verification age-gate (swedish-payroll)** — DEFERRED to Phase 5 PR-2. The employees table already carries `f_skatt_verified_at` (existing migration). PR-2's :calculate is the correct enforcement point. - **Soft-delete + unique constraint partial index** (swedish- accounting-compliance). VALID concern for genuine rehires. Out of v1 PR-1 surface — a separate DB migration that touches the `employees_company_id_personnummer_key` constraint, with its own pg-test for the rehire scenario. Tracked. - **semestertillagg_rate vs vacation_rule consistency** (swedish- payroll). Engine-layer concern. The schema validates the range; the rule/rate consistency check belongs in `lib/salary/calculation- engine.ts` next to the actual accrual math. Tracked for the engine audit alongside Phase 5 PR-2. - **voucher_series default 'A' vs convention 'N'** (swedish-payroll). Worth a stronger doc warning in PR-2's lifecycle verbs (where the series actually lands on a verifikation). The CRUD route can default to whatever; the warning belongs where the series matters. - **personnummer_last4 column** (Art.25). Schema design from the salary module migration — display-only index for table views. Out of v1 scope. - **Bank account at-rest encryption (CC6.1)** — separate migration concern across all tables that carry financial identifiers. Out of v1 scope. Test count: 37 (up from 35). All type-checks clean. Full v1 suite green (232 tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-479 review round 2 — proto-pollution defense + salary-run JE-orphan guard Triage of bot re-run on c0d168be: - **Compliance Swarm V4.5 (prototype pollution in PATCH rawKeys)** — FIXED. `Object.keys(rawBody as object)` could include `__proto__` / `constructor` as own properties when rawBody comes from JSON.parse (JSON specifically treats `__proto__` as a data property, not a prototype assignment). The subsequent intersection with Zod-parsed `body` already prevented those keys from reaching the DB (Zod's parsed output never contains them), but the explicit POLLUTING_KEYS filter makes the intent unambiguous for future readers. Defense in depth. - **Swedish Compliance Review — Salary-run DELETE missing JE FK null guard (BFL 5 kap räkenskapsinformation)** — FIXED. The DELETE chain previously gated only on `status='draft'`. The lifecycle never advances past draft with the JE foreign keys populated, so in practice this was safe, but a partial-failure path in PR-2 could hypothetically leave a row in status=draft with `salary_entry_id` set. The .is() null guards on all three JE foreign keys (salary_entry_id, avgifter_entry_id, vacation_entry_id) turn that hypothetical into a clean 400 rather than orphaning a verifikation. Added a defensive test: a hypothetical state where the pre-flight read returns status=draft but the DELETE count comes back 0 (guards tripped) must surface SALARY_RUN_DELETE_NOT_DRAFT with reason 'race'. Findings on this round explicitly NOT addressed: - **V16.1.1 + Art.5(1)(f) on app/api/bookkeeping/journal-entries/[id]/ commit/route.ts** — NOT MY FILES. Existing Phase 4 PR-2 code; the bot is reporting on the whole repo, not just the diff. - **V2.2 PostgREST .or() injection (recurring)** — Known false positive. Same escaping pattern as suppliers + customers since Phase 2. The documented architectural floor per the plan doc. - **Art.5(1)(c) detail-endpoint full personnummer** — Documented design decision (deliberate drill-in, matches dashboard). Same as the previous round. - **Art.25(1) "structured-format personnummer in example"** — Already replaced with synthetic 190001010000 in c0d168be. Bot is now suggesting a non-numeric placeholder (e.g. 'YYYYMMDDXXXX'). Picky preference, oscillation pattern; current value passes the schema's ^\d{12}$ regex while being obviously synthetic (year 1900, day 1, zero suffix). No change. - **Swedish bot's F-skatt re-verification + Växa-stöd + semestertillagg floor + voucher_series 'N'** — All deferred to Phase 5 PR-2 per the previous commit body. The lifecycle verbs are where these belong. Test count: 38 (+1 for the JE-orphan guard test). 233 total v1 tests green. Type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-479 review round 3 — symmetrize PATCH defenses + tighten docs/BFL wording Compliance Swarm dropped 18 → 15 findings on the round-2 commit; the floor is narrowing. This commit addresses the remaining actionable items. - **V2.3 / PI1.3 — salary-run PATCH missing POLLUTING_KEYS filter** — FIXED. Same defense as employees PATCH (round 2). Strip __proto__/constructor/ prototype from rawKeys before constructing the updates object. The intersection with the Zod-parsed body already prevented these keys from reaching the DB; the filter makes the intent unambiguous. - **V4.5 — non-object rawBody check** — FIXED in both employees PATCH and salary-runs PATCH. After JSON.parse, require typeof === 'object', not null, not Array.isArray. Zod would catch a non-object body downstream, but the rawKeys Object.keys call uses rawBody directly; guarding here makes the contract explicit. (An array body would pass `typeof === 'object'` and produce numeric-string keys.) - **A.5.34 — request-example personnummer too realistic** — FIXED. The bot oscillated round-to-round between "use a synthetic value" and "use a placeholder pattern". Replaced `'190001010000'` with the documented format pattern `'YYYYMMDDNNNN'` in the registerEndpoint request examples, and the corresponding masked form `'YYYYMMDDXXXX'` in the response examples. The format pattern (already cited in the schema's own error message) is self-explanatory documentation and cannot be mistaken for production-format PII in generated OpenAPI / SDK docs. Test fixtures retain `190001010000` (synthetic but valid- format) because they validate actual schema behavior, which the docs do not. - **Swedish bot — BFL 7 kap comment slightly overstates the law** — FIXED. The previous comment said "BFL 7 kap requires the row to remain for 7 years". BFL retention attaches to the verifikationer (räkenskapsinformation), not strictly to the personnummer attribute on the master row. Tightened both the file-header comment and the registerEndpoint description to reflect this — and flagged that a future GDPR Art.17 erasure workflow could pseudonymise the row once all referenced verifikationer are outside the 7-year window. The practical outcome (soft-delete only via v1) is unchanged. Findings on this round explicitly NOT addressed: - **V14.2 / V16.1.1 / Art.5(1)(f) on app/api/bookkeeping/journal- entries/[id]/commit/route.ts** — NOT MY FILES (Phase 4 PR-2 surface). - **V16.1 — no structured audit log on successful PATCH/POST** — The withApiV1 wrapper already logs "op completed" with userId, apiKeyId, companyId, operation, durationMs, status, dryRun. Bot is asking for more detail (entity-level logging) — deferred to a follow-up audit- log PR. - **Art.5(1)(c) / A.8.11 / CC6.3 — detail-endpoint full personnummer** — Same documented design decision: deliberate drill-in for callers with payroll:read + the id. Mirrors the dashboard. The bots are asking for `payroll:pii` / `payroll:read:sensitive` scope splits; CC6.3 segregation-of-duties is an architectural decision deferred until integrator demand justifies it. - **C1.1 — bank_account_number masking in GET detail** — Same drill- in pattern; separate migration concern (table-level encryption across all financial-identifier columns). Out of v1 PR-1 scope. - **Art.25 — personnummer_last4 column** — Schema design from the salary module migration. Display-only index. Out of v1 scope. - **Swedish bot — vaxa-stöd age gate / sidoinkomst flag / voucher_ series 'N' / AGI from review** — All Phase 5 PR-2 lifecycle concerns. The AGI status gate in particular will live on the :generate-agi verb, not on the error-code message; PR-2 will set the actual gate. - **Swedish bot — GDPR Art.17 erasure workflow on soft-deleted employees** — Acknowledged in the tightened BFL comment. Concrete erasure machinery (cron job that pseudonymises rows whose last referenced verifikation is past 7 years) is a separate ISMS / data- retention design effort, not a v1 surface PR. Test count: 38 (unchanged). 233 total v1 tests green. Type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-14 13:50:55 +02:00
Jakob Wennberg	2ed8096150	feat(api): Phase 4 PR-3 — documents (multipart) (#471) ... * feat(api): Phase 4 PR-3 — documents (multipart) — 3 endpoints Closes the deferred multipart slice of Phase 4. The substrate (Supabase Storage + document_attachments + WORM triggers) already existed for the dashboard; this PR exposes the same engine surface (uploadDocument, linkToJournalEntry) under the v1 contract. ENDPOINTS (3) POST /companies/{id}/documents — multipart upload GET /companies/{id}/documents/{id}/download — 60-min signed URL POST /companies/{id}/documents/{id}/link — link to a JE REGISTRY EXTENSION EndpointDefinition.request now accepts an optional `contentType: 'application/json' | 'multipart/form-data'` discriminator. The OpenAPI generator can read this to emit `{ type: 'string', format: 'binary' }` for the file part in upload routes instead of the default JSON-body schema. Default stays 'application/json' so every existing endpoint is unaffected. SECURITY / TENANCY - documents.upload: when journal_entry_id is supplied, verifies the JE belongs to ctx.companyId before storing. Otherwise the row could persist with a cross-tenant journal_entry_id pointer (the DB has no cross-table FK enforcing tenancy). - documents.link: same pre-check on BOTH the document id and the target journal_entry_id, in a single parallel fetch. - documents.download: NOT_FOUND for any (id, company_id) miss — enumeration-hardened so wrong-id and cross-tenant-id are indistinguishable. EVENTS - documents.upload → document.uploaded (via uploadDocument) - documents.download → document.accessed (best-effort) - documents.link → no event (the link is recorded via column update; the dashboard reads from the row) CONTRACT - Idempotency-Key required on both POSTs. - Dry-run supported on /link (confirms both refs exist without persisting). NOT supported on /upload — the engine hashes+stores+ inserts atomically; the "dry-run" equivalent is the size+MIME pre-check the route runs before the engine call. - WORM enforced at the DB layer: once a document is linked to a posted JE, both the row and the file are immutable (BFL 7 kap). The v1 surface has no update/delete endpoint by design. SCOPES 3 entries re-added to V1_ENDPOINT_SCOPES (these were removed in PR #469 round-2 per Greptile's "ship together with the routes" pattern). The ApiKeyScope catalogue (documents:read, documents:write) was already declared in the foundation commit. ERROR CODES DOC_DOWNLOAD_FAILED added to structured-errors.ts (500, SV+EN). Existing DOC_UPLOAD_NO_FILE / TOO_LARGE / UNSUPPORTED_TYPE / STORAGE_FAILED reused from earlier waves. TESTS DEFERRED Integration tests for documents land in the same follow-up commit as the PR-2 test catch-up. Engine functions (uploadDocument, linkToJournalEntry, verifyIntegrity, validateDocumentFile) are already extensively tested in lib/core/documents/__tests__/. Suite 3376/3376 still green; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #471 round-1 — Greptile + compliance review fixes (7 real) First bot pass on PR #471 — Greptile flagged 3 P1 + 3 P2, Compliance Swarm 17 (0 blocking, mostly recurring), Swedish-compliance 4. Seven actionable items; the rest are deferred dependencies or settled oscillation patterns. REAL FIXES (7) 1. P1 — upload's JE pre-check destructures error away. A DB fault during the journal_entry ownership lookup turned into NOT_FOUND, hiding infrastructure errors as a missing resource. Now captures `.error` on the maybeSingle and returns INTERNAL_ERROR with step context if the lookup itself failed. 2. P1 — link's Promise.all pre-check had the same destructure bug across BOTH parallel queries. Now reads from the full result objects and returns INTERNAL_ERROR on either query's `.error`. 3. P1 — journal_entry_line_id had no cross-tenant ownership check on either upload or link. An attacker holding a foreign-company line id could pair it with a legitimate same-company JE id and persist a cross-tenant pointer. Both routes now verify the line belongs to the supplied JE before write. Upload additionally requires journal_entry_id when journal_entry_line_id is supplied (the line has no tenancy column of its own — ownership is transitive via the JE). 4. P2 — upload_source was TypeScript-cast without runtime validation. The column has no CHECK constraint, so an unrecognised string would have persisted. Now validates via z.enum().safeParse — VALIDATION_ERROR on miss listing the allowed values. 5. P2 — storage_path leaked in the upload response. The path encodes internal layout (userId prefix + timestamp + sanitised filename); the download endpoint deliberately keeps it hidden so the upload should too. Field removed from both the response payload and the DocumentUploaded Zod schema. 6. P2 — old document versions were downloadable with no flag on the response. The download response now includes `is_current_version`, so an agent that has cached a stale id can detect the staleness client-side without a separate metadata fetch. Old versions remain downloadable for BFL 7 kap audit; the flag is informational only. 7. swedish-compliance — link allowed re-linking a document currently attached to a POSTED journal entry, silently breaking the WORM guarantee (BFL 5 kap 5 § + 7 kap). Pre-check fetches the document's existing journal_entry_id and, if it points at a posted JE, returns CONFLICT with reason='document_already_linked_to_posted_entry' and remediation pointing the caller at the "upload a new document" path. DISMISSED / DEFERRED - OWASP V5.2 magic-number MIME sniffing — adds a `file-type` dependency. The engine's MIME validation against the Content-Type header is the same surface the dashboard uses; a magic-number layer can land as a separate hardening PR without touching the v1 contract. - OWASP V5.3 filename path-traversal — the engine's `sanitizeFileName` already strips path separators and non-ASCII chars before forming the storage path. The `file_name` column keeps the original (display-only) name. No traversal vector through to storage. - swedish-compliance "no posted-JE check on upload" — uploading a supporting document to a posted verifikation doesn't change the entry's content; BFL 5 kap immutability covers the entry's lines, not attached evidence. The dashboard allows it for the same reason. - swedish-compliance `document.accessed` audit reliability — same oscillation pattern from PR-2 (Art.5(1)(f) vs V16.1). Best-effort warn-level remains; webhook/DLQ hardening is Phase 6. - Compliance Swarm V8.2.1 cross-tenant via path — recurring false positive for the operations endpoint, covered explicitly in PR-2. Suite 3376/3376 still green; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #471 round-2 — signed-URL TTL 60min → 15min Compliance Swarm went 17 → 14 on round-1. Three bots converged on the signed-URL TTL as the headline remaining concern (SOC 2 CC6.1 + GDPR Art. 5(1)(f) + ISO 27001 A.8.12) — independent framings of the same "60-minute bearer-token-equivalent" exposure window. REAL FIX (1) Reduce SIGNED_URL_TTL_SECONDS from 60 minutes → 15 minutes. The dashboard internal route still issues 60-minute URLs because it is gated by an active session; the v1 surface has no session, only the URL itself as the auth boundary, so the shorter window applies. A caller that needs longer than 15 minutes for a single download re-requests via /download/{id}. Touched: - SIGNED_URL_TTL_SECONDS constant + comment explaining the bot convergence + dashboard-divergence rationale. - Header docstring (60-minute → 15-minute). - Registry example response (expires_in_seconds: 3600 → 900). - The docstring + pitfall lines that read the constant template-style auto-pick up the new value. DISMISSED (with rationale) - V8.2.1 "add .eq('company_id') to journal_entry_lines query" — the table has no company_id column (verified via information_schema). Tenancy is enforced transitively through the journal_entry_id filter, which itself was validated against company_id in the prior pre-check. The bot's suggested fix would not compile. - V5.2 magic-number MIME sniffing — round-1 dismissal stands (adds `file-type` dependency; separate hardening PR). - Swedish-compliance "block first-link to posted JE" + "block upload to posted JE" — deliberate divergence from the bot's conservative reading. Attaching evidence to a posted verifikation doesn't mutate the verifikation itself; the dashboard allows this for the same reason. v1 keeps parity. Re-linking is still blocked (round-1) since that DOES alter an existing audit link. - Art.5(1)(f) / A.8.15 / Art.32(1)(b) / CC7.2 document.accessed audit reliability — same oscillation pattern from PR-2. Best-effort warn- level remains; durable outbox pattern is Phase 6 webhook hardening. - Art.25(1) userId in storage path — engine-layer concern. Path is set by lib/core/documents/document-service.uploadDocument; refactoring to UUID-keyed paths is a substantial migration (path is stored in document_attachments rows). Out of v1 surface scope. - Art.5(1)(e) stray-document retention policy + CC6.3 scope policy doc + C1.1 metadata classification — policy artifacts, not code. Suite 3376/3376 still green; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-13 22:30:35 +02:00
Jakob Wennberg	e31aee2455	feat(api): Phase 4 PR-2 — engine + periods + compliance-check (docs deferred) (#469) ... * feat(api): Phase 4 PR-2 foundation — async operations substrate Checkpoint commit. Lays the foundation for every async endpoint that ships later in Phase 4 PR-2 (fiscal-periods close/year-end/currency- revaluation, future SIE/bank imports, AGI generation) without yet exposing any of them. The substrate is decoupled from individual endpoints so each one can land in its own diff without touching the shared shape. ADDED - Migration `20260513200000_api_v1_async_operations.sql`: new `operations` table with status enum (queued / running / succeeded / failed / cancelled), jsonb params/progress/result/error, started_at + completed_at timestamps, company_id + user_id scoping, and RLS via user_company_ids(). Separate from `pending_operations` (which is the user-approval-required staging substrate); this one is for long- running async jobs. Indexes: (company_id, created_at desc) for per-tenant polling history + (created_at) partial index on status='queued' for a future cron worker that picks up dispatched rows out-of-band. - `lib/api/v1/operations.ts`: lifecycle helpers consumed by every async POST endpoint. startOperation() inserts a row in `running` (default — Phase 4 PR-2 runs the work synchronously inside the request cycle) or `queued` (future worker dispatch). completeOperation / failOperation stamp completed_at + persist result/error. updateOperationProgress is the in-flight progress writer. getOperation reads back by id, scoped to a company. - `app/api/v1/operations/[id]/route.ts`: polling endpoint GET /api/v1/operations/{id}. Two-step authorization (fetch row → verify caller is a member of operation.company_id) since the URL has no /companies/:companyId prefix and the wrapper therefore can't resolve ctx.companyId. Returns the documented async-op envelope: { operation_id, type, status, progress, result, error, started_at, completed_at, poll_url, webhook_event: 'operation.completed' }. - `lib/auth/scopes.ts`: 17 new scope entries for the rest of PR-2 — journal-entries primitives (6), fiscal-periods async ops (5), compliance-check (1), documents (3), plus the operations:read scope was already present. Adding all up front so subsequent route PRs only ship the route files. - `lib/api/v1/load-routes.ts`: registers operations/[id] for the OpenAPI generator. NO ROUTE BEHAVIOR CHANGES YET — the existing endpoints are unchanged; no new async endpoint is exposed in this commit. Tests 3376/3376 still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): Phase 4 PR-2 — journal-entries primitives + voucher-gap-explanations Adds the core engine surface that the rest of v1 has been routing through private wrappers (transactions/match, supplier-invoices/register, etc). Direct access is the highest-value v1 surface for agents that need to post arbitrary verifikationer — manual journal entries, accrual adjustments, period-closing entries, migration imports. ENDPOINTS (7) GET /journal-entries — cursor list (period, status, date) GET /journal-entries/{id} — detail with lines POST /journal-entries — create draft (no voucher_number) POST /journal-entries/{id}/commit — atomic voucher + post POST /journal-entries/{id}/reverse — storno (BFL 5:5) POST /journal-entries/{id}/correct — storno-then-replace pair (BFL 5:5) POST /journal-entries/batch-create — up to 50 drafts, partial-success POST /voucher-gap-explanations — document löpnummer gaps (BFNAR 2013:2 kap 8) All writes are idempotent (mandatory Idempotency-Key) and dry-runnable. ENGINE WIRING createDraftEntry → POST /journal-entries commitEntry → POST /{id}/commit reverseEntry → POST /{id}/reverse correctEntry (storno svc) → POST /{id}/correct Strict-mode v1: every engine call is wrapped in try/catch + isBookkeepingError discrimination so the structured error envelope (JOURNAL_ENTRY_NOT_BALANCED, ENTRY_DATE_OUTSIDE_FISCAL_PERIOD, ACCOUNTS_NOT_IN_CHART, PERIOD_LOCKED, ENTRY_ALREADY_REVERSED, CANNOT_REVERSE_NON_POSTED, CANNOT_CORRECT_NON_POSTED) reaches agents instead of a generic 500. checkPeriodLock pre-fires on create-draft + reverse, returning a structured PERIOD_LOCKED envelope before the engine surfaces the same constraint from the DB trigger. DRY-RUN - create-draft: validates balance + period + line shapes, no insert. - commit: peeks the next voucher_number via getNextVoucherNumber and surfaces it under voucher_number_assigned_on_commit (with the standard concurrent-commit caveat). - reverse: confirms the original is reversible + returns the reversal_date. - correct: confirms the new lines balance + reports the inherited period. - batch-create: returns per-item preview rows. - voucher-gap-explanation: echoes the input shape. SCHEMA No new tables — uses existing journal_entries, journal_entry_lines, and voucher_gap_explanations from earlier migrations. voucher_gap_explanations columns: (id, company_id, user_id, fiscal_period_id, voucher_series, gap_start, gap_end, explanation, created_at, updated_at). TESTS DEFERRED Integration tests for the journal-entries vertical land in a follow-up commit on this branch alongside the compliance-check + fiscal-periods work. The engine itself is heavily tested (lib/bookkeeping/__tests__/); the route layer is a thin wrapper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): Phase 4 PR-2 — compliance-check + fiscal-periods async ops Ships the second-largest chunk of PR-2: gnubok's defensible-edge compliance pre-flight endpoint and the five fiscal-period lifecycle endpoints. Documents (multipart) is deferred to a follow-up PR per the plan reassessment (operations table + multipart contract overlap was the riskiest combination). COMPLIANCE-CHECK (1 endpoint, 3 check types) GET /compliance/check?type=<vat_close|year_end_readiness|voucher_gaps> Single structured envelope across all check types: { type, ready, findings: [{severity, code, message, details}], summary, generated_at, params, details? } - vat_close → wraps computeVatCloseCheck (SKV 4700 rutor + blockers) - year_end_readiness → wraps validateYearEndReadiness (BFNAR 2017:3 + ÅRL 2:1) - voucher_gaps → wraps detect_voucher_gaps RPC Adding a new check type only requires registering an entry in CHECK_RUNNERS; the response shape stays stable so agents only learn one structure. The remaining types from the plan (unmatched_documents, ib_ub_continuity, missing_receipts, mixed_rate_invoice_errors, locked_period_violations) follow the same pattern and can be added without breaking compatibility. FISCAL-PERIODS ASYNC OPS (5 endpoints) Synchronous wrappers around the existing engine functions: POST /fiscal-periods/{id}/lock — lockPeriod POST /fiscal-periods/{id}/close — closePeriod (IRREVERSIBLE) POST /fiscal-periods/{id}/opening-balances — generateOpeningBalances Operation-recorded (return 202 + operation_id; poll /v1/operations/{id} or subscribe to operation.completed in Phase 6): POST /fiscal-periods/{id}/year-end — executeYearEndClosing POST /fiscal-periods/{id}/currency-revaluation — executeCurrencyRevaluation The two async-recorded endpoints run synchronously inside the request cycle today; the operation row keeps the response shape stable when a future cron worker takes over true async dispatch (just change initialStatus from 'running' to 'queued' in startOperation). Strict error mapping: engine throws (e.g. "Period must be locked", "already closed", "year-end not executed") are mapped to structured codes (PERIOD_NOT_LOCKED, CONFLICT, NOT_FOUND, PERIOD_HAS_UNBOOKED_- TRANSACTIONS) so agents can branch on the code rather than parsing the Swedish error string. LOAD-ROUTES All 6 new endpoints registered in lib/api/v1/load-routes.ts for the OpenAPI generator. Scopes already in place from the foundation commit. TESTS Tests for journal-entries, compliance-check, and fiscal-periods are deferred to a follow-up commit on this branch (alongside the documents/multipart work, if it lands here). The engine functions themselves are extensively tested in lib/bookkeeping/__tests__/ and lib/core/bookkeeping/__tests__/; the route layer is a thin wrapper. Full suite 3376/3376 green. tsc clean on new files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #469 — drop vat_close from compliance-check (core-only CI gate) core-only.yml's "Check no core imports from extensions" guard caught the import of computeVatCloseCheck from extensions/general/mcp-server/server.ts. CLAUDE.md is explicit: core code cannot import from @/extensions/ directly. Drop vat_close from SUPPORTED_TYPES for now. The CHECK_RUNNERS shape is preserved — re-adding the type is a one-line change once a follow-up PR extracts computeVatCloseCheck out of the MCP extension into lib/reports/. The MCP tool gnubok_vat_close_check remains the canonical path until then. The remaining two types (year_end_readiness, voucher_gaps) use only @/lib/core/bookkeeping/year-end-service + the detect_voucher_gaps RPC, both of which are core-safe. Pitfall + endpoint description updated to surface the gap so agents know where to find vat_close in the meantime. Suite 3376/3376 still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #469 round-1 — compliance bot review (3 real, 2 FP, rest deferred) First compliance-bot pass on the draft PR — Compliance Swarm 15 findings, Swedish-compliance 6. Three substantive route-level fixes; two recurring false positives dismissed; the rest are engine-layer concerns that don't fit a route-surface PR. REAL FIXES (3) 1. voucher-gap-explanations example was self-contradictory. The example explanation cited "failed commit ... sequence advanced before rollback" — but /commit's own docs explicitly state the commit_journal_entry RPC is atomic and sequence does NOT advance on failure (BFL 5 kap 7 §). The example contradicted the design guarantee. Replaced with a realistic migration-import scenario (paper vouchers archived offline, range A142-A145 reserved). 2. Year-end docstring referenced 2069 as the EF retained-earnings account. Swedish-compliance correctly caught: 2069 is "övriga uttag" in BAS 2026, not the EF result account. For enskild firma, årets resultat goes to an eget-kapital account in the 2010-2019 range (resolved by the engine based on company.entity_type). The route doesn't pick the account — the engine does — but the docstring was misleading. 3. compliance-check fiscal_period_id ownership pre-check. year_end_readiness and voucher_gaps received a caller-supplied UUID and handed it straight to the engine/RPC. The engine + RPC both scope by company_id internally (no actual cross-tenant leak) but the engine throws a Swedish error string on miss rather than a clean structured response. Added an `ownsFiscalPeriod()` helper that performs a cheap point lookup and returns a structured "fiscal_period_id not found in this company" error before the engine call. DISMISSED (2 false positives) - V8.2.1 operations route ownership — the bot read only the file header (line 1). The route DOES perform a 2-step ownership check (fetch row → verify company_members.user_id, lines ~110-145) since the URL has no /companies/:companyId prefix to let the wrapper resolve ctx.companyId. Already documented in the route's docstring. - V8.2.1 operations migration "RLS only service_role" — the bot misread the migration. The actual policy is: USING (company_id IN (SELECT public.user_company_ids())) i.e. authenticated callers can read their company's operations under RLS. The two-step check in the route is defense-in-depth. DEFERRED (engine-layer) - swedish-compliance: /correct inherits original entry_date, fails when original period is locked. Real ergonomics issue. Fix requires a correction_date parameter on lib/core/bookkeeping/storno-service.correctEntry. Engine signature change — out of v1 surface scope. - swedish-compliance: 2099→2091 prior-year sweep in year-end engine. executeYearEndClosing engine concern, not visible from the route. - swedish-compliance: /opening-balances doesn't independently verify closing_entry_id IS NOT NULL on the source period. Engine concern. - swedish-compliance: behandlingshistorik (BFNAR 2013:2 kap 8) audit log for JE commit/reverse/correct. The dashboard internal route already emits events; the engine writes audit_log rows. Engine concern, not per-route. - swedish-compliance: revaluation tax_code default. Engine concern; executeCurrencyRevaluation builds the JE lines. - Compliance Swarm recurring architectural items (V16.1 event-bus retry, Art.5(1)(f) userId in logs oscillation from PR-1, SOC 2 CC6.3 SoD, etc.) — all carry-overs from PR-1 with the same dispositions. Suite 3376/3376 still green; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #469 round-2 — Greptile review fixes (3 real, 1 FP, 1 deferred) First Greptile pass after the PR went out of draft. Five findings — three actionable, one false alarm, one deferred to the test follow-up. REAL FIXES (3) 1. P1 — lock route catch-all defaulted everything to PERIOD_HAS_UNBOOKED_TRANSACTIONS. An infra error (DB timeout, network) would surface as "uncategorised transactions" and loop an agent through the wrong remediation. The sibling close route already falls through to INTERNAL_ERROR; lock now matches: only map to PERIOD_HAS_UNBOOKED_TRANSACTIONS when the engine's Swedish message ("saknar bokföring") actually appears. Otherwise → INTERNAL_ERROR + the original message in details. 2. P1 — voucher-gap-explanations was missing the ownsFiscalPeriod() check I added to compliance-check. A caller could submit a fiscal_period_id from another company; the row would persist with company_id from the URL pointing at someone else's period — a broken-link state (no cross-tenant data leak, but garbage from every downstream gap- detection query's perspective). Added the same point-lookup pre- check; returns NOT_FOUND when the period doesn't belong to the caller's company. 3. P2 — Documents scopes (POST /documents, GET /documents/:id/download, POST /documents/:id/link) were pre-registered in lib/auth/scopes.ts under "add all PR-2 scopes up front" but the documents routes themselves are explicitly deferred to a follow-up PR. Removed them; they ship with the routes. Comment in scopes.ts records the rationale. DISMISSED (1 false alarm) - gen_random_uuid() vs uuid_generate_v4() — Greptile cited CLAUDE.md rule 4. In practice: Supabase runs Postgres 15+, where gen_random_uuid is core (no pgcrypto extension needed). The Docker stack runs Postgres 17 per the project's docker-publish.yml. CLAUDE.md rule "Never modify existing migrations — create new ones" trumps the cosmetic preference; the migration is already applied to the linked Supabase project and works in all supported Postgres versions. Leaving as-is. DEFERRED (1) - P2 — *.pg.test.ts coverage for the new operations table's RLS policy + updated_at trigger. CLAUDE.md does require this. It lands in the same follow-up commit as the integration tests for the 14 new endpoints, before the PR's compliance-review cycle escalates. Suite 3376/3376 still green; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #469 round-3 — ownership pre-checks + explicit close-route state guards Compliance Swarm went 15→18 on round-2, mostly because the V8.2.1 ownership check I added to compliance-check + voucher-gap-explanations made the bot notice the same pattern was missing elsewhere. Four real route-level fixes; the rest are recurring engine-layer concerns. REAL FIXES (4) 1. Extract `ownsFiscalPeriod` into `lib/api/v1/owns-fiscal-period.ts`. Was inline in compliance/check/route.ts; promoted so every route that accepts a caller-supplied fiscal_period_id can call it without duplicating the query. Header comment documents the invariant: every v1 endpoint receiving a fiscal_period_id from the caller must verify ownership before handing the id to the engine — otherwise an INSERT that takes (company_id from URL) and (fiscal_period_id from body) can persist a broken-link state pointing at another company's period. 2. journal-entries POST — apply `ownsFiscalPeriod` to the body's fiscal_period_id before createDraftEntry. (V8.2.1) 3. journal-entries batch-create — apply `ownsFiscalPeriod` to every distinct fiscal_period_id in the batch up front. Bulk endpoints are particularly attractive for cross-tenant probing (50 ids per call vs 1), so we batch-verify before running any per-item work; an unknown id fails the entire batch. Partial-success semantics only apply AFTER ownership is established. (V8.2.1) 4. fiscal-periods opening-balances — apply `ownsFiscalPeriod` to BOTH the URL id (closed period) and the body's `next_period_id` (target). Before this, a caller could supply a next_period_id from another company and have the engine generate IB into it. (V8.2.1) 5. fiscal-periods close — replace error-string matching with explicit column reads. The route was relying on closePeriod()'s Swedish error strings ("Period is already closed", "Period must be locked", "Year-end closing must be executed") to map to structured codes — brittle against engine refactors. Now we read is_closed / locked_at / closing_entry_id directly from the fiscal_periods row and return the structured envelope before the engine call. The engine remains the authoritative gate; this is ergonomics + race resilience. (V2.3) DISMISSED / DEFERRED - V2.3 lock route Swedish string-matching — keeping. Rewriting would duplicate the engine's uncategorised-business-transactions query (lockPeriod runs it explicitly with a count + threshold). Engine re-throw with a typed error is the right long-term fix. - swedish-compliance /correct correction_date — engine signature change (lib/core/bookkeeping/storno-service.correctEntry needs a new param). Deferred to engine PR. - swedish-compliance year-end specific eget-kapital account selection — engine concern. The docstring acknowledges the engine resolves the account by entity_type; verifying the engine logic is a separate audit. - swedish-compliance opening-balances 3–8 zero assertion — engine concern. /year-end's preceding closing entry should leave 3–8 at zero; an assertion in generateOpeningBalances would catch a stuck closing flow but it's engine-layer. - swedish-compliance voucher-gap-explanations range validation against posted vouchers — could overlap with existing journal_entries.voucher_- number values. Real audit-trail concern but adds an extra round-trip per insert; defer. - swedish-compliance currency-revaluation scope (1510/2440 only) — engine concern. - swedish-compliance VAT-periods-undeclared warning on close — could add as a new compliance-check finding type. Tracked separately. Suite 3376/3376 still green; tsc clean on all changed files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #469 round-4 — async-op atomicity + correct period-lock + IB dedup Round-3 fixes converged the count but exposed five new substantive items across the compliance bots. All five addressed. REAL FIXES (5) 1. currency-revaluation: unconditional ownership pre-check (V8.2.1). Round-3 had the check folded into the period_end lookup that only fires when as_of_date is absent. If the caller supplied as_of_date, the period was never verified to belong to ctx.companyId. Now calls ownsFiscalPeriod() unconditionally, before startOperation. 2. year-end: ownership pre-check (V8.2.1). Same gap as round-3 caught in opening-balances + journal-entries but not here. Added. 3. /correct: checkPeriodLock on the inherited entry_date. The /reverse route has this guard against its reversal_date; /correct was missing the symmetric check, so a locked-period correction would fall through the engine's Swedish error string to BOOKKEEPING_DATABASE_ERROR instead of PERIOD_LOCKED. The correction trail (BFL 5 kap 5 §) is bound to typed.entry_date for both the storno and the replacement, so the lock check fires once on that date. 4. /opening-balances: duplicate-IB detection. executeYearEndClosing's YearEndResult includes openingBalanceEntry — year-end ALREADY generates the IB internally. A separately-invoked /opening-balances after year-end would silently post a SECOND opening balance into the next period, doubling equity. Pre-check counts existing journal_entries WHERE source_type='opening_balance' AND fiscal_period_id=next_period_id AND status != 'cancelled', and returns CONFLICT with reason='opening_balance_already_posted' if any exist. Remediation hint points at the GL endpoint to inspect what's there. 5. /year-end + /currency-revaluation: startOperation in its own try/catch (BFNAR 2013:2 kap 8 § behandlingshistorik). Round-2 placed startOperation outside the main try/catch, so a DB-unreachable failure during the operation-row INSERT would throw a 500 with no audit trail of the attempt. Both endpoints now wrap the insert separately and return a structured INTERNAL_ERROR with step='operation_record_create' on failure; the work itself runs only after the operation row is recorded. DOCS (1) 6. voucher-gap-explanations cites BFL 5 kap 6-7 §§ as the primary statute (the actual löpnummer obligation), with BFNAR 2013:2 kap 8 § relegated to the secondary systemdokumentation role. Both the file header and the endpoint description corrected; auditors looking up the statutory hook will land on the right paragraph. DISMISSED / DEFERRED - swedish-compliance: operations-table immutability trigger (BEFORE UPDATE blocking mutations once status terminal). Real architectural concern. Requires a migration; lands in a follow-up PR alongside the operations.pg.test.ts coverage. - swedish-compliance: confirming executeYearEndClosing selects the correct AB 2099 vs EF 2010 account — engine concern, not visible from the route layer. - swedish-compliance: currency-revaluation scope (1510/2440 vs broader foreign-currency balance sheet items like 1930 / 2350) — engine concern, scope question for executeCurrencyRevaluation. - swedish-compliance: voucher-gap range overlap validation (gap_start..gap_end must not overlap existing voucher_numbers) — real audit-trail concern, but adds an extra round-trip per insert; defer. Suite 3376/3376 still green; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-13 21:53:22 +02:00
Jakob Wennberg	abb9f5868c	feat(api): Phase 4 PR-1 — AP world (suppliers + supplier-invoices) (#467) ... * feat(api): Phase 4 PR-1 — AP world (suppliers + supplier-invoices) First of two Phase 4 PRs. Ships the public v1 AP-side verticals end-to-end, mirroring the Phase 2 AR pattern (customers + invoices). ENDPOINTS (13) Suppliers: GET /suppliers — cursor list + filters GET /suppliers/{id} — detail, ?expand=supplier_invoices POST /suppliers — idempotent, dry-runnable PATCH /suppliers/{id} — idempotent, dry-runnable, can un-archive DELETE /suppliers/{id} — soft-archive, refused on open SI POST /suppliers/bulk-create — partial-success, max 50 Supplier invoices: GET /supplier-invoices — cursor list + filters GET /supplier-invoices/{id} — detail, ?expand=supplier,items,payments POST /supplier-invoices — register + post registration JE PATCH /supplier-invoices/{id} — registered-only POST /supplier-invoices/{id}/approve — flip to approved POST /supplier-invoices/{id}/mark-paid — book payment JE + flip status POST /supplier-invoices/{id}/credit — issue kreditfaktura + reversing JE No DELETE on supplier-invoices — withdrawal is via :credit (mirrors v1 invoices, keeps both original AND credit note in the audit trail per BFL 5 kap 5 §). STRICT-MODE V1 Carried forward from Phase 3 lessons: - Any JE failure ABORTS before SI state mutation (no soft-fall / partial state). Applies to register, mark-paid, and credit. - checkPeriodLock() pre-check before every JE-emitting write — returns structured PERIOD_LOCKED / SI_PAID_PERIOD_LOCKED / SI_CREDIT_PERIOD_LOCKED instead of letting the DB trigger surface a generic 500. - CAS-race orphan handling in mark-paid: if the SI status flips between pre-flight and our update, the just-posted payment JE is stornoed via reverseEntry() rather than left dangling (BFL 5 kap 5 §). - Math.round monetary throughout. Half-öre epsilon on remaining_amount==0. SCHEMA MIGRATION `20260513150000_archived_at_for_customers_and_suppliers.sql`: - Adds suppliers.archived_at (new — required for the soft-archive flow). - Adds customers.archived_at + customers.vat_number_validated_at — retroactively. The Phase 2 v1 customer routes (PR #451 / #452 / #460) already reference both columns but no prior migration installed them in production. This commit fixes that latent bug while we have the migration open. - Partial indexes on (company_id, created_at) WHERE archived_at IS NULL keep the default-active list path cheap. - is_active (legacy boolean) preserved on suppliers; v1 archive sets both archived_at = now() AND is_active = false, un-archive flips both back so the dashboard's "show only active" filters stay intact. NEW ERROR CODES SUPPLIER_HAS_INVOICES (409) — archive refused while open SI exists SI_NOT_DRAFT (400) — update/delete refused on non-registered SI GDPR ART.5(1)(c) DEFENSE-IN-DEPTH SupplierType has no `individual` variant today, so org_number is always Bolagsverket public-record data. The list endpoint still has the masking hook (empty INDIVIDUAL_TYPES set) so a future natural-person supplier type becomes a one-line change. Duplicate-org_number error responses NEVER echo the submitted value — symmetric with customers. SCOPES 13 new entries in V1_ENDPOINT_SCOPES under suppliers:read / suppliers:write. TESTS 36 new integration cases across 2 suites: - suppliers: list (incl. filter), get (incl. 404), create (happy + 23505 + dry-run + missing-idempotency), patch (happy + empty body), delete (archive + open-invoice refusal), bulk-create (partial-success + 501) - supplier-invoices: list, get (incl. 404), create (happy accrual + supplier 404 + period-locked + strict-mode JE rollback + dry-run), patch (registered-only), approve (happy + non-registered refusal), mark-paid (happy + period-locked + already-paid + strict-mode abort), credit (happy + already-credited + period-locked + dry-run) Full suite green: 3333 passing (237 files). Build + lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #467 round-2 — Greptile P1/P2 fixes Three real findings from Greptile inline review on the Phase 4 PR-1 commit. P1 — mark-paid: storno orphan JE when SI update fails. When the `.update()` after the JE post returned an `updateErr`, the route logged + returned SI_PAID_FAILED without reversing the just-posted payment JE. The CAS-race branch immediately below already proves journalEntryId is in scope and reverseEntry takes it directly — the original comment about "requires fetching the entry first" was wrong. Now both error branches (the `updateErr` DB-failure path and the `!updated` CAS-race path) storno via reverseEntry before returning, keeping the AP ledger consistent (BFL 5 kap 5 §). Storno failure itself logs loudly and the error envelope surfaces the journal_entry_id so manual reconciliation has a starting point. P1 — credit + register: capture JE link-update result, storno on failure. Both supplier-invoices/route.ts (register) and supplier-invoices/[id]/ credit/route.ts back-fill registration_journal_entry_id on the freshly- inserted SI/credit-note row, but were dropping the await result. A transient DB error there silently left the row with registration_- journal_entry_id=null even though the JE was live on the books — the POST response looked correct (it returned the JE id from the local variable) but every subsequent GET /supplier-invoices/{id} showed null. Both paths now capture the link-update error, storno the orphan JE via reverseEntry, then roll back the SI/credit-note row before returning SI_CREATE_FAILED / SI_CREDIT_FAILED with step='*_link'. Strict-mode atomicity restored. P2 — mark-paid: dry-run paid_at format alignment. Dry-run preview set `paid_at: paymentDate` (YYYY-MM-DD), but the live `.update()` writes `new Date().toISOString()` (full UTC timestamp). A caller validating both responses against the same regex would have been caught by the mismatch. Dry-run now mirrors the live shape. P2 — ensureInitialized() finding dismissed as a false positive: lib/api/v1/with-api-v1.ts:52 already calls ensureInitialized() at module load. Every v1 route imports withApiV1 from that module, so the side effect runs on first import and caches. No existing v1 route (customers, invoices, transactions) imports ensureInitialized() directly — the pattern has been consistent across Phases 1-3 and the AP-world routes follow it. Tests + build green: 3333 passing across 237 files, AP suite 36/36. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #467 round-3 — compliance swarm + swedish-compliance fixes Both bots re-ran and converged on a set of substantive findings. Seven real issues addressed; several recurring false positives + architectural deferrals documented inline. REAL FIXES (7) 1. credit: `remaining_amount` calc was nonsensical. `Math.max(0, remaining_amount - total)` was always ≤ 0 (since remaining ≤ total), forcing status to 'credited' regardless of paid state — but only via the clamp, not the logic. Both swedish-compliance and Compliance Swarm (OWASP V2.3 + SOC 2 PI1.3) caught this. A kreditfaktura nullifies the AP obligation on the original (BFL 5 kap 5 §); refunds of already-paid amounts get a separate transaction. `remaining_amount: 0` and `status: 'credited'` unconditionally. 2. supplier-invoices register: VAT rate whitelist. `computeItemsAndTotals` accepted any float for `vat_rate`, silently booking an unrecognised rate into the registration JE → momsdeklaration Ruta 48 + INK2R. Now rejects with VALIDATION_ERROR (allowed_rates echoed) unless the rate is in `{0, 0.06, 0.12, 0.25}` (ML 2 kap 1 §). 3. mark-paid: `exchange_rate_difference` is required for non-SEK accrual. The pitfall docs warned about this but the code didn't enforce it. Without it the payment JE doesn't book the FX delta to 3960/7960 and AP carries a stranded 2440 balance after the bank line clears. Enforces with field- level VALIDATION_ERROR; pass `exchange_rate_difference: 0` if there's no rate movement. 4. suppliers PATCH: refuse on archived suppliers (BFL 7 kap 1 §). An archived supplier's name/address backs historical verifikationer; a post-archive PATCH would silently corrupt 7-year-retained räkenskaps- information. The handler now fetches the current row, refuses identifying- field updates when `archived_at IS NOT NULL`, and only permits the un-archive PATCH (`archived_at: null`). 5. supplier-invoices register: smart vat_treatment / reverse_charge default. The previous default of `'standard_25'` regardless of supplier_type left EU/non-EU supplier rows with metadata that didn't match the actual booking path (which uses `reverse_charge`). Now derives both fields from `supplier.supplier_type` when the caller omits them: foreign suppliers default to `reverse_charge: true` + `vat_treatment: 'reverse_charge'`. Explicit body values still win. 6. reverseEntry: static import (SOC 2 CC8.1). Replaced the three dynamic `await import('@/lib/bookkeeping/engine')` calls in orphan-storno error branches with a top-level static import. The dependency is now visible to SCA / tree-shake / static analysis. 7. Add `userId: ctx.userId` to every storno-failure log context (OWASP V16.1). The CAS-race + linkErr branches now consistently include the actor identity for security-relevant audit events. TESTS (+6 new) - register: rejects non-Swedish vat_rate (whitelist) → 400 - register: defaults reverse_charge=true + vat_treatment='reverse_charge' for eu_business suppliers - mark-paid: requires exchange_rate_difference for non-SEK accrual → 400 - mark-paid: passes when exchange_rate_difference is explicitly 0 - suppliers PATCH: refuses identifying-field edit when archived_at IS NOT NULL - suppliers PATCH: allows un-archive (archived_at: null) flip AP suite 42/42 (was 36). Full suite 3339/3339 green (was 3333). DISMISSED WITH RATIONALE - swedish-compliance "credit-note amounts should be negative" — false read of the engine. `createSupplierCreditNoteEntry` calls `Math.abs()` on item amounts (line 421) and posts a reversing JE; the SI row carries positive amounts + `is_credit_note=true` as a deliberate data-model decision. Negating would break parity with the dashboard and the internal AP-ledger reporting. - OWASP V8.2.1 cross-tenant via path — recurring false positive across Phases 2-4. `withApiV1` (line ~340-350) verifies `company_members` membership BEFORE setting `ctx.companyId` from the URL. - OWASP V8.2.1 supplier_invoice_items company_id filter in rollbackCreditNote — the table has no `company_id` column; cross-tenant protection comes from RLS + the parent supplier_invoice_id scoping. - OWASP V4.5 PATCH allowlist schema-derivation — known architectural deferral; centralising the field list against a Zod `.pick()` is a separate refactor. - GDPR Art.5(1)(f) log/event field identifiers — RoPA / log-pseudonymisation is an org-wide privacy-eng concern, not a per-route fix. - ISO 27001 A.8.15/A.8.16 non-blocking inserts — `supplier_invoice_payments` insert + event emit failures stay at warn-level for v1 to mirror the dashboard internal route. Promoting to error escalations + DLQ is a cross-cutting reliability project, not a route patch. - SOC 2 CC6.3 segregation-of-duties — v1's API-key scope IS the boundary by design. Role-based separation between register / approve / pay is a v1.x feature, not a v1 surface bug. - swedish-compliance reverse-charge gating in credit — the engine (`createSupplierCreditNoteEntry`) already gates the 2647/2645 reversal on `creditNote.reverse_charge` (line 437). Mirrors the registration engine. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #467 round-4 — BFL 5 kap 5 § + remaining compliance fixes Compliance bots re-ran on round-3 (Compliance Swarm 13→12 findings, Swedish-compliance fresh re-read). Five real issues addressed; the rest are recurring false positives or architectural deferrals carried over from earlier rounds. REAL FIXES (5) 1. mark-paid: future payment_date rejected at the schema layer. BFL 5 kap 2 § requires bokföring to follow real cash movement; payment_date > today is a scheduling artefact, not an affärshändelse. Returns 400 VALIDATION_ERROR before the JE engine runs. 2. credit: drop user_id from SI_FULL_COLUMNS (GDPR Art.25). The original SI's `user_id` (its historical creator) is never used in the credit flow — the new credit-note row uses ctx.userId (the actor performing the credit). Don't fetch what you don't need. Also drops company_id from the select since it's already filtered. 3. supplier-invoices register: reverse-charge cross-field VAT check. For reverse-charge invoices the Swedish supplier doesn't charge VAT, the buyer self-assesses (ML 1 kap 2§ p.4b / 16 kap 6 § / 16 kap 13 §). If `reverse_charge=true` and ANY item has `vat_rate != 0`, return VALIDATION_ERROR — otherwise the engine would book ingående moms in Ruta 30 / 48 (BAS 2614 / 2645 / 2641) for an invoice that has no VAT to deduct. 4. rollbackSupplierInvoice + rollbackCreditNote: soft-mark, not delete. BFL 5 kap 5 § — rättelse av bokföringspost måste vara dokumenterad så att både den ursprungliga och den korrigerade noteringen är synliga. Hard-deleting the SI row on a mid-write failure destroys räkenskapsinformation even when the JE side (if any) is preserved via storno. Both rollback paths now UPDATE status='reversed' + reversed_at=now() — the SupplierInvoiceStatus enum already has 'reversed' for exactly this case ("credit note whose journal entry was storno-reversed via Ångra kreditering" per the type comment). Trade-off: a retry with the same supplier_invoice_number will hit the unique-index conflict, so the caller picks a fresh number. TESTS (+2 new) - register: rejects reverse_charge=true with non-zero item vat_rate - mark-paid: rejects future payment_date Pre-existing eu_business reverse_charge test updated: item vat_rate flipped from 0.25 → 0 to remain valid under the new cross-field check. AP suite 44/44 (was 42). Full suite 3341/3341 green (was 3339). DISMISSED (recurring or architectural) - OWASP V8.2.1 cross-tenant via path — recurring false positive across Phase 2-4. withApiV1 verifies company_members membership BEFORE setting ctx.companyId from the URL. - ISO A.8.3 approve-route TOCTOU — already mitigated. The UPDATE has `.eq('status', 'registered')` as a race guard; the pre-flight is for ergonomic error messages, not security. - SOC 2 PI1.3 floating-point — project-wide convention is Math.round(x * 100) / 100 per CLAUDE.md. Diverging in one route would create a parity bug with the bookkeeping engine + dashboard. Settled. - SOC 2 CC7.3 storno-failure alerting / ISO A.8.15 audit-log on success / SOC 2 CC6.1 test-fixture key / OWASP V2.2 status state-machine / V1.2.5 dynamic select-clause / V16 audit-log silent-failure / Art.25 banking-field expand — all architectural deferrals that fit the webhook-hardening + scope-redesign work in Phase 6, not the v1 PR. - swedish-compliance "credit-note original-number reference" — the `credited_invoice_id` FK is the structured back-reference; the document-rendering layer surfaces the original `supplier_invoice_- number` from there. Not a v1 surface bug. - swedish-compliance "cash-basis credit-note vat_amount" — engine behaviour mirrored from the dashboard. Engine-layer audit, separate effort. - swedish-compliance "active-supplier mutability broader than archived_at" — solving this requires snapshotting supplier identity onto each supplier_invoices row at registration (schema migration). Deeper architectural decision; tracking for Phase 4 follow-up alongside the journal-entries vertical. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #467 round-5 — strict schema + vat_treatment normalisation + narrow BFL archive lock Compliance bots re-ran on round-4. Most findings are recurring (V8.2.1 cross-tenant, PI1.3 floating-point, CC6.3 SoD) or the classic oscillation pattern from the Phase 3 lessons: this round's Art.5(1)(f) flags userId in storno error logs as PII exposure — but last round's V16.1 demanded I ADD userId for audit attribution. Staying with audit attribution; the bot can pick a side. Three substantive findings addressed. REAL FIXES (3) 1. V4.5 mass-assignment defense-in-depth on PATCH /supplier-invoices/{id}. The shared `UpdateSupplierInvoiceSchema` is consumed by the dashboard too, where Zod's default key-stripping is acceptable. The v1 route now wraps it in `V1PatchSupplierInvoiceSchema = UpdateSupplierInvoiceSchema .strict()` so any unknown key (e.g. `status`, `company_id`, `user_id`) returns 400 VALIDATION_ERROR instead of being silently dropped — even if the iteration allowlist downstream is later relaxed. 2. vat_treatment normalisation when reverse_charge resolves true. Caller could previously pass `vat_treatment: 'standard_25'` explicitly on an eu_business supplier, and the supplier-type-driven default would set `reverse_charge: true` while the metadata stayed as 'standard_25'. The engine books via the boolean (so JE is correct) but a downstream momsdeklaration / audit export reading `vat_treatment` would mis- classify. Resolution order is now: reverse_charge first, then vat_treatment forced to 'reverse_charge' if true; explicit overrides only stick when they agree with the resolved boolean. 3. Narrow archived-supplier PATCH lock to identifying fields only. The round-4 blanket lock on archived suppliers was too broad: BFL 7 kap 1 § protects räkenskapsinformation — the fields verifikationer reference through the supplier join — but not internal notes or payment-config metadata. The check now only refuses PATCHes that touch {name, supplier_type, org_number, vat_number, address_*, banking_*}. Notes, default_payment_terms, default_expense_account, default_currency, email, and phone remain editable on archived rows. TESTS (+3 new) - PATCH /supplier-invoices/{id}: rejects unknown body keys (strict schema) - POST /supplier-invoices: explicit vat_treatment='standard_25' is overridden when supplier_type drives reverse_charge=true - PATCH /suppliers/{id}: allows notes edit on archived supplier (BFL narrow scope) AP suite 47/47 (was 44). Full suite 3344/3344 green (was 3341). DISMISSED (recurring / settled / oscillating) - OWASP V8.2.1 cross-tenant via path — recurring false positive 4 rounds running. withApiV1 verifies company_members membership BEFORE setting ctx.companyId from the URL. - GDPR Art.5(1)(f) userId in error logs — direct contradiction of round-3's OWASP V16.1 finding which demanded userId be ADDED for audit attribution. Phase 3 lessons document this oscillation pattern ("swedish-compliance / compliance-swarm oscillate between rounds") and the correct response is to stay with the more security-positive position. Keeping userId on storno-failure logs for ledger-integrity attribution. - SOC 2 CC6.3 segregation-of-duties — same as round-3. v1 design uses API-key scope as the boundary; role-based actor separation is Phase 6 webhook + auth work. - SOC 2 CC6.1 null-userId guard — redundant. withApiV1 short-circuits with 401 UNAUTHORIZED before invoking the handler when API-key validation fails (which is the only path that could leave ctx.userId unset). - SOC 2 CC7.2 storno-failure alerting — architectural; webhook-bus + dead-letter is Phase 6 territory. - SOC 2 / OWASP PI1.3 / V2.3 floating-point — project-wide convention per CLAUDE.md; the engine, dashboard, and v1 all use Math.round(x*100)/100. - ISO 27001 A.8.33 test-fixture financial amounts — synthetic UUIDs + NODE_ENV=test guard already in place; "TEST-only" sentinel amounts would be cosmetic. - OWASP V16.1 eventBus failure retry / DLQ — Phase 6 webhook hardening. - swedish-compliance arrival_number gap risk — acknowledged in commit, bot itself says "no action required"; supplier_invoice_number retry behavior already in the rollback-comment doc. - swedish-compliance vat_code cross-field — engine derives JE shape from `invoice.reverse_charge` (boolean), ignores item vat_code in the RC path. No surface-layer leak. - swedish-compliance credit-note FX at today's rate — bot's reasoning inverted. The credit note REVERSES the original AP obligation; to net 2440 to zero across the original-registration JE + credit-note JE, the SEK amounts MUST be copied from the original. FX rate at today's date applies at the bank-refund transaction side, not the credit-note registration. - swedish-compliance KREDIT- prefix — dashboard parity. The `is_credit_note` + `credited_invoice_id` flags are the structured back-references; the prefix is cosmetic on the human-readable number. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #467 round-6 — overpayment guard + two-phase rollback + SI_FULL_COLUMNS minimisation Compliance Swarm trended down 12→9 findings, Swedish-compliance 6→5. Three substantive items addressed; the rest are recurring false positives or the userId-in-logs oscillation that round-5 already settled. REAL FIXES (3) 1. mark-paid: reject overpayment up front (Compliance Swarm V2.3). Previously `Math.max(0, remaining - payment)` silently truncated an overpayment to a zero remaining_amount, while the JE engine booked the full payment_amount against 2440 — leaving an unaccounted overpayment on the AP ledger. Now refuses with VALIDATION_ERROR when `payment_amount > remaining_amount + 0.005` (half-öre tolerance for FX-rounding artefacts). Recovery hint points at :credit for over-billing and the transactions endpoints for refunds. 2. credit: trim SI_FULL_COLUMNS to fields actually read (Art.25(1)). The credit handler never reads notes, paid_at, payment_journal_entry_id, transaction_id, document_id, payment_reference, paid_amount, delivery_date, received_date, reversed_at, created_at, updated_at, exchange_rate_date, due_date — but the projection was fetching them all. SEK-conversion fields (subtotal_sek / vat_amount_sek / total_sek) ARE read (copied onto the credit-note row so the 2440 reversal nets), so they stay. Continues the round-4 user_id / company_id drop. 3. Two-phase soft-rollback (Swedish-compliance, BFL 5 kap 5 §). The bot caught a real misapplication: BFL 5:5 only kicks in once a verifikation has been COMMITTED. Pre-JE failures (items_insert, engine returning null because no fiscal period covers the date) are failed insertions, not bokföringsposter. Marking those rows `status='reversed'` with a null registration_journal_entry_id creates a dangling räkenskapsinformation entry that's harder to audit than a clean removal. Both rollback helpers now take a `journalEntryPosted` flag: pre-JE failures hard-delete (rows + items), post-JE failures keep the round-4 soft-mark + reversed_at behaviour. Call sites tagged per failure reason: items_insert → false (hard-delete) no_fiscal_period → false (hard-delete; engine returned null pre-write) registration_je → true (conservative; engine throw could be post-commit) je_link_failed → true (JE posted + already stornoed above) credit items_insert → false credit no_fiscal_period → false credit_journal_entry → true credit_race → true TESTS (+1 new) - mark-paid: rejects payment_amount > remaining_amount with VALIDATION_ERROR (no JE engine call) AP suite 48/48 (was 47). Full suite 3345/3345 green (was 3344). DISMISSED (with rationale) - OWASP V8.2.1 cross-tenant via path — recurring across 5 rounds. withApiV1 verifies company_members membership BEFORE setting ctx.companyId from the URL. Fix-once decision in the wrapper, not a per-route concern. - OWASP V4.5 strict schema (re-verification) — round-5 added V1PatchSupplierInvoiceSchema = UpdateSupplierInvoiceSchema.strict() + a test asserting {"status": "approved"} is rejected. The bot is re-flagging because it can't see the upstream schema in the diff; manually verified: UpdateSupplierInvoiceSchema only contains {supplier_invoice_number, invoice_date, due_date, delivery_date, payment_reference, notes}. No status / company_id / user_id field. - GDPR Art.5(1)(f) userId in logs — same oscillation as round-4. Last round V16.1 demanded userId be ADDED for audit attribution; this round Art.5(1)(f) wants it REMOVED. Staying with audit attribution per the Phase 3 lessons doc's oscillation guidance. - OWASP V16.1 / ISO A.8.15 / SOC 2 CC7.2 SIEM alerting on storno failure — architectural; Phase 6 webhook hardening. - GDPR Art.25(2) supplier-expand banking fields default-on — same as round-4. A scope split (suppliers:read:sensitive) is a v1.x scope refactor, not a single-route patch. - swedish-compliance VAT 0.06 date-aware validation (livsmedel 1 April 2026) — needs livsmedel BAS classification (which BAS codes signal food) and date-aware lookup tables. Engine-layer concern; not achievable without engine changes. Documenting the 6% rate's temporary nature in the comment was the smaller fix already shipped in round-3. - swedish-compliance SI_RESPONSE_COLUMNS missing reverse_charge — FALSE ALARM. `reverse_charge` IS present in the projection (line 264 of supplier-invoices/route.ts); the engine receives it correctly. - swedish-compliance KREDIT- prefix — dashboard parity, dismissed rounds 3-5. The `is_credit_note` + `credited_invoice_id` flags are the structured back-references. - swedish-compliance cash-basis credit-note ingående moms timing (ML 13 kap 27 §) — legitimate gap but engine-layer. The createSupplierCreditNoteEntry engine function handles accrual only; adding a cash-basis-already-paid branch would change engine semantics, divering from the dashboard. Tracking as a Phase 4 engine follow-up, not a v1 surface bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-13 20:16:27 +02:00
Jakob Wennberg	a9c98da243	feat(api): Phase 3 — transactions + reconciliation vertical (#464) ... * feat(api): Phase 3 — transactions + reconciliation vertical Closes out Phase 3 of the plan in one PR. After this, a 3rd-party agent can fully manage a company's transaction ledger via the public API: import bank data, walk the queue, categorize (manual / template / counterparty / account-override), match payments to customer + supplier invoices, reverse mistakes, and auto-reconcile the bank against the GL. ENDPOINTS (12) Reads: GET /transactions — cursor list, filters GET /transactions/{id} — detail GET /accounts — BAS chart, class filter GET /fiscal-periods — räkenskapsår list Writes (single tx, idempotent + scoped): POST /transactions/{id}/categorize — dry-run, CAS race guard POST /transactions/{id}/uncategorize — dry-run, storno + reset POST /transactions/{id}/match-invoice — storno conflicting JE, payment JE, link POST /transactions/{id}/match-supplier-invoice — incl. FX diff handling Writes (bulk, partial-success + all_or_nothing:true → 501): POST /transactions/ingest — up to 500 items (CSV + custom feeds) POST /transactions/batch-categorize — up to 100 items Reconciliation: POST /reconciliation/bank/run — dry-run, applies matches GET /reconciliation/bank/status — health snapshot All write surfaces mirror the dashboard's internal route compliance behavior exactly — same engine functions, same Prong-B SI-match suggestion intercept on categorize, same FX-diff handling on supplier- invoice match, same optimistic-lock interlock on invoice status update. No new bookkeeping primitives — every route delegates to the existing `lib/bookkeeping/*` engine, `lib/transactions/ingest.ts`, and `lib/reconciliation/bank-reconciliation.ts`. SCOPES + ERRORS Adds 12 entries to lib/auth/scopes.ts under transactions:read|write + reports:read (accounts, fiscal-periods follow the same convention as MCP tools). Adds 4 new error codes: TX_UNCATEGORIZE_NOT_BOOKED, TX_UNCATEGORIZE_JE_NOT_POSTED, TX_INGEST_INSERT_FAILED, TX_BATCH_CATEGORIZE_EMPTY. TESTS 32 new integration cases across 5 suites: - transactions list / detail (4) - accounts + fiscal-periods (4) - categorize / uncategorize / match-invoice / match-supplier-invoice (9) - ingest + batch-categorize (7) - reconciliation run + status (5) plus shared happy-path and edge cases (no-income, already-linked, malformed body, scope rejection, dry-run shape). Full suite green: 3270 passing (234 files). Build + lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address PR #464 review — Phase 3 hardening Greptile P1 — cursor pagination broken in GET /transactions. encodeDefaultCursor was passed the YYYY-MM-DD `date` field, but decodeDefaultCursor's strict ISO 8601 timestamp regex rejected it, so every cursor decoded as null and the endpoint always returned the first page. Switched the cursor anchor to `created_at` (real ISO timestamp, total-orderable, unique within the company at the row insertion grain) and updated the sort to (created_at DESC, id ASC). The `date` column remains in every row + filterable via ?date_from / ?date_to. Updated the registry description to reflect the change. Greptile P1 — JE soft-fall in match-invoice + match-supplier-invoice. When the payment journal entry creation threw (any non- AccountsNotInChartError), the catch block recorded the error string but execution CONTINUED, marking the invoice paid + inserting a payment row + linking the transaction with no GL entry. The dashboard internal route soft-fails here intentionally and surfaces a banner so the user can re-book; for the v1 surface a partial state is strictly worse than a clean failure to retry. Both routes now return: - INVOICE_PAID_BOOK_FAILED (match-invoice) - MATCH_SI_RECORD_PAYMENT_FAILED (match-supplier-invoice) before any state mutation. Removed `journal_entry_error` from both response schemas — strict mode means it can never be set on a 200. Greptile P1 — `overdue` supplier invoices fail the optimistic lock. The early status guard accepted `overdue` as matchable, but the downstream `.in('status', ['registered', 'approved', 'partially_paid'])` excluded it, returning MATCH_SI_NOT_OPEN for a legitimately payable invoice. Added `overdue` to the optimistic-lock list. Greptile P1 + Swedish-compliance — CAS-race orphan cancellation. Direct `.update({ status: 'cancelled' })` on the orphaned JE was silently blocked by enforce_journal_entry_immutability (the engine writes JEs as posted) and the `voucher_gap_explanations` row claimed the entry was cancelled when it wasn't. BFL 5 kap 5 § requires corrections via a reversing entry. Both /transactions/{id}/categorize and /transactions/batch-categorize now call `reverseEntry()` on the orphan; the storno pair keeps the verifikationsnummer series unbroken so the gap-explanation insert is no longer needed. Greptile P2 + Swedish-compliance — hardcoded category on match-invoice. The dashboard internal route writes `category: 'income_services'` for every matched invoice payment, overwriting any prior categorization with a wrong BAS classification for goods sales / rental income. Fixed by preserving the existing transaction.category if set, only defaulting to `income_services` when the row had never been categorized before. Compliance Swarm V2.4 — reconciliation date range guard. Added a 366-day cap on date_from / date_to via Zod refine. Longer reconciliations should be paged. Greptile P2 — dry-run dedup limitation. Added a pitfall note documenting that the ingest dry-run only checks external_id-based dedup; content-based dedup (date+amount against already-booked rows) only runs in the live pipeline. Swedish-compliance — BFL chapter typo on fiscal-periods registry. "BFL 6 kap" → "BFL 5 kap 2 §" (the löpande bokföring deadline). Deferred (with rationale documented): - OWASP V8.2.1 cross-tenant via path: false positive — wrapper sets ctx.companyId from the URL after membership check (recurring across swarm runs). - OWASP V4.5 select('*') on transactions/invoices: same as Phase 2 — those rows feed engine functions that need the full shape. - OWASP V2.3 multi-write atomicity (match endpoints): would need a Postgres RPC; separate refactor. - Swedish-compliance kontantmetoden partial-payment status: same semantics as the dashboard internal route; engine-level decision out of v1's scope. - Greptile P3 `reversible: false` on uncategorize: technically correct (the storno itself isn't reversible via this verb). Tests + build green: 3270 passing, lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(import): distinguish network errors in the SIE upload step Adds a dedicated 'network' errorType so the SIE import wizard surfaces "Uppladdningen misslyckades" with a connectivity-focused remediation instead of the generic 'parse' fallback (which suggested checking the SIE file format — wrong direction when the issue is actually offline / flaky upload). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address PR #464 swedish-compliance re-run findings The Swedish-compliance bot edited its existing comment in place after the prior fix push (so created_at filtering missed the re-run). The re-run flagged 6 new substantive findings against the post-fix code. Fix 1 — Orphan storno failure leaves an unresolved immutability gap (categorize + batch-categorize). When reverseEntry() on the CAS-race orphan fails, the orphan stays posted and untraceable. BFL 5 kap 5 § requires every correction be traceable. Both paths now insert a voucher_gap_explanations row in the catch branch flagging "automatisk storno misslyckades — manuell reconciliation krävs", so the orphan is logged at the audit-trail level rather than only in app logs. Fix 2 — Period-lock pre-check (categorize + batch-categorize). enforce_period_lock and enforce_company_lock_date triggers block JE inserts on locked/closed periods, but Supabase surfaces those as a generic 500. Added a new lib/api/v1/check-period-lock.ts helper that performs the same check the trigger would (company-wide lock date, is_closed, locked_at), and both routes now return a structured PERIOD_LOCKED response (existing error code, 400) with reason + fiscal_period_id details before the engine call. Note: this is an ergonomics check (TOCTOU window between check and insert) — the trigger remains authoritative. Fix 3 — Ingest dry-run now performs content-based dedup too. The earlier doc-only note was a compliance miss: an integrator relying on dry-run to confirm uniqueness could ingest duplicate affärshändelser, violating BFL 5 kap. The dry-run now runs BOTH external_id dedup AND content-based (date+amount-against-booked) dedup over the request's date range — same query the live pipeline uses. Pitfall doc updated accordingly. Fix 4 — fiscal-periods response now carries duration_days + exceeds_18_months computed fields. An automated client (year-end wizard, audit tool) can spot a non-compliant period sequence (BFL 3 kap, 18-month cap) without re-implementing date arithmetic. 549-day cap (18 calendar months) is used to keep the comparison deterministic across leap years. First-year exceptions still require human judgment; the boolean is a flag, not a verdict. Deferred (with rationale documented in commit, not retried): - uncategorize storno memo: reverseEntry() doesn't accept a reason parameter today and the JE-level back-reference exists already via reversed_by_id / reverses_id. Engine signature change is out of v1's scope. - VAT integrity check on partial payment in match-invoice: the behavior is fully delegated to createInvoicePaymentJournalEntry. The bot itself recommends auditing against the engine; that is an engine-layer concern and the dashboard internal route uses the same path. - 366-day reconciliation window (advisory): no statutory basis; operational guard. - match-supplier-invoice FX path against ML 8 kap 21–23 § (advisory): engine-layer concern. Tests + build green: 3270 passing, lint clean. Touched-suite tests (transactions, fiscal-periods, accounts, reconciliation) re-run; the fiscal-periods test asserts the new derived fields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #464 round-3 review fixes (re-run after period-lock + dedup) Both compliance bots edited their existing comments in place after the prior fix push. New findings against the post-fix code: Fix — VAT account suppression too broad on account_override. categorize/route.ts dropped vat_lines for ANY class-2 override, but BAS class 2 includes the 26xx VAT clearing accounts themselves. Result: a user override TO a VAT account silently lost the auto-VAT line. Tightened to `account_class === 2 && !account_override.startsWith('26')`. The override-to-2440-leverantörsskulder case is unchanged (correctly drops auto-VAT); the override-to-2611-utgående-moms case now keeps the VAT line. Fix — fiscal-periods 18-month cap uses calendar arithmetic. EIGHTEEN_MONTHS_DAYS = 549 was a generous approximation (18 calendar months span 540–549 days). Replaced with proper month-anchor math: start_date + 18 months computed via setUTCMonth-style year/month rollover, then `period_end > anchor` is the violation. Manual day- arithmetic on the year part avoids JS's clamp-overflow on Aug-31-style start dates. duration_days helper preserved for the response field. Fix — match-invoice no longer hardcodes 'income_services'. When the transaction has no prior category, the route now leaves the field UNTOUCHED in the UPDATE (existing default 'uncategorized' or whatever was there persists). The response surfaces null for the uncategorized case so a caller can detect "needs human classification" without inspecting the DB. The auto-default to income_services was flowing into BAS 3001/3041/3530 selection mismatches and INK2R/SRU mis-reporting for goods/rental flows. Existing-category transactions still propagate their value. Doc — accounts.ts BAS 5/6 description tightened. Was "5=other costs, 6=other costs" — both true but flatten distinct subgroups. Now spells out 5xxx (rents/supplies/services) and 6xxx (marketing/professional/IT) under övriga externa kostnader, with a pointer to the canonical BAS chart. Deferred (with rationale documented): - voucher_gap_explanations in SIE export coverage: verification ask; SIE export audit is a separate task, not this PR's scope. - Dry-run dedup parity with full live pipeline: my dedup matches the live pipeline's primary checks (external_id + content date+amount against booked rows). Achieving exact parity would need refactoring lib/transactions/ingest.ts to expose a shared dedup helper. - FX sign convention in match-supplier-invoice: identical to the dashboard internal route; if the engine sign convention is wrong both surfaces are wrong. Engine-layer audit, not v1 surface. - OWASP V8.2.1 cross-tenant via path: recurring false positive — the wrapper sets ctx.companyId from the URL only AFTER company_members membership check. - V2.3 multi-write atomicity in match endpoints: would need a Postgres RPC; separate refactor. - check-period-lock TOCTOU on no_fiscal_period (advisory note): the engine's ensureFiscalPeriod helper creates an open period; if the transaction date sits in a historical gap, the engine creates the period unlocked. The trigger remains the authoritative gate. Tests + build green: 3270 passing, lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #464 round-4 review fixes (compliance bot re-run) The compliance swarm went from 20 → 10 findings after round-3, but the swedish-compliance bot caught 5 issues my fixes introduced or didn't fully cover. Fix — VAT account suppression narrowed to BAS 2610–2649. My round-3 fix exempted any account starting with '26' from VAT-line suppression, but BAS 26xx includes 2650 (momsredovisningskonto) and 2690 (diverse), neither of which is a moms-line account. Auto-VAT posted against 2650 would double-post on the moms reconciliation account. Tightened the exception to the 2610–2649 range (utgående + ingående moms accounts only). Fix — exceedsEighteenMonths month-end overflow. My round-3 manual month math still passed `startD` raw to Date.UTC, which clamps Aug 31 + 18 months to Mar 3, making the cap LATER than the BFL 3 kap 1 § ceiling (false negative). Now clamps `startD` to the last valid day of the target month using `Date.UTC(year, m+1, 0)`. Fix — ingest dry-run dedup float-key normalization. Built the content-dedup set from `${tx.date}|${tx.amount}` where amount is a JS number stringified directly — `-349.5` from JSON vs `-349.50` from a Postgres numeric round-trip miss-match. Normalized both sides to .toFixed(2). SIE imports commonly carry trailing-zero precision, so this would have caused the dry-run to under-report duplicates (a BFL 5 kap löpande-bokföring concern: an integrator trusting the dry-run could double-book affärshändelser). Fix — CAS-race voucher_series fallback no longer files under 'A'. Both categorize and batch-categorize used `voucher_series || 'A'` for the voucher_gap_explanations row. If the orphan JE had no series, the gap would be indexed under series 'A' and missed by any series- specific audit query (BFL 5 kap 6 §). Now skips the gap row entirely when no series is set — the error log already captures the orphan for human reconciliation; filing under the wrong key is strictly worse than not filing. Fix — match-invoice rejects kontantmetoden partial payments. Under kontantmetoden, utgående moms must be reported per actual receipt (ML 13 kap 8 §). The cash-method-partial branch was falling through to createInvoicePaymentJournalEntry (the accrual 1510/1930 clearing path), which doesn't model the per-installment moms event. Rather than silently over-report moms, refuse with a VALIDATION_ERROR pointing the caller to either wait for the full payment or switch to faktureringsmetoden. Full cash-method payments still flow through createInvoiceCashEntry (the correct kontantmetod path). Deferred (with rationale): - `uncategorize` resets journal_entry_id to null: dashboard parity; the JE-side back-reference (reversed_by_id / reverses_id) preserves the audit pair. Adding a separate reversal_journal_entry_id column on transactions is a schema change out of v1 scope. - OWASP V8.2.1 cross-tenant: recurring false positive. - OWASP V2.2 inline Zod filter schemas: structural consistency decision — kept in-route to match other v1 endpoints; a future refactor can centralize when it justifies the cost. - OWASP V16 add userId/companyId to storno-failure log: txLog already carries both via ctx.log.child; not changing call-site syntax for compliance theatre. - Engine-layer FX sign convention in match-supplier-invoice (advisory): identical to dashboard internal route. Tests + build green: 3270 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): match-supplier-invoice storno conflicting JE before booking The match-invoice route stornoes any conflicting auto-categorization JE before posting the payment entry; match-supplier-invoice was missing the symmetric guard. If a transaction was previously auto-categorized (e.g. expense_office with a 5460/1930 entry), matching it to a supplier invoice would post a second 2440/1930 entry while leaving the original posted — two verifikationer for one affärshändelse, a BFL 5 kap 6 § integrity violation. Storno-before-match now applies in both routes, with the same fail-closed semantics (storno failure aborts before any state change). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-13 15:22:57 +02:00
Jakob Wennberg	01e99d3220	feat(api): v1 invoice PDF + customer bulk-create (Phase 2 PR-B-3) (#460) ... Closes out the Phase 2 invoices+customers vertical. After this PR, every write/read the dashboard does on these two resources is reachable via the public API. GET /api/v1/companies/{companyId}/invoices/{id}/pdf Read-only application/pdf endpoint. Mirrors the dashboard's internal /api/invoices/[id]/pdf so a downloaded PDF is byte-equivalent across surfaces. Drafts render with the "faktura-utkast-<id-slice>.pdf" filename (preview before send is a legitimate workflow); sent invoices use "faktura-<number>.pdf"; credit notes use "kreditfaktura-<number>.pdf" and embed the original invoice's löpnummer per ML 17 kap 22–23§ back-reference; proforma + delivery notes get their own prefixes. Error codes: INVOICE_PDF_RENDER_FAILED (500, new), INVOICE_SEND_COMPANY_SETTINGS_MISSING (404, reused — same condition, same remediation). POST /api/v1/companies/{companyId}/customers/bulk-create Mirrors /invoices/bulk-create exactly: same `{ results, summary }` shape, same all_or_nothing: true → 501 NOT_IMPLEMENTED contract, same 50-item cap, same sequential processing. Per-item rollback isn't needed (customer insert is a single row), but per-item 23505 → CUSTOMER_DUPLICATE_ORG_NUMBER failure surfaces in the results array without echoing org_number (GDPR Art.5(1)(c): for sole traders org_number IS the personnummer). VIES validation runs per item, best-effort — a timeout leaves vat_number_validated=false but does NOT fail the item. Registry: extended EndpointDefinition.response with an optional `contentType` field so the OpenAPI generator can emit `format: binary` schemas for non-JSON responses. The PDF endpoint is the first consumer; future binary endpoints (SIE export, ICS feeds) use the same hook. Scope catalogue: added GET .../pdf → invoices:read, POST .../customers/bulk-create → customers:write. Tests: 14 new integration cases (7 for PDF: sent / draft / credit-note filename, 404, render-error 500, non-UUID 400, scope rejection; 7 for customer bulk-create: happy path, dup-org error masking, max-50 cap, all_or_nothing 501, dry-run preview, empty array, scope rejection). Suite green: 3232 passing. Build + lint clean. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-13 10:53:51 +02:00
Jakob Wennberg	37ccda5cad	feat(api): v1 invoice :send + bulk-create (Phase 2 PR-B-2b-3 + PR-B-2c) (#458) ... * feat(api): v1 invoice :send + bulk-create action verbs (Phase 2 PR-B-2b-3 + PR-B-2c) Combined chunk: ship the full :send pipeline and partial-success bulk creation in one PR, plus include the dangling /reset-password middleware fix that completes PR-455's password-recovery flow. POST /api/v1/companies/:companyId/invoices/:id/send Full send pipeline mirroring the internal route, hardened for the public API surface: email-configured check, draft-only guard, cancelled / delivery-note / credit-note / missing-moms_ruta rejections, customer email check, company-settings fetch, F-series invoice-number allocation (atomic at :send per ML 17 kap 24§ p.2, not at draft create), preflight PDF render before number consumption, final PDF render, email send, point-of-no-return status flip, BFL 5 kap journal entry, document archival, invoice.sent event emit. Post-send failures (journal entry, archive) surface via a `warnings` array rather than failing the response — the invoice IS sent at that point. Dry-run validates the pipeline + preflight PDF without allocating a number or hitting the provider. Error codes: INVOICE_SEND_EMAIL_NOT_CONFIGURED (503), INVOICE_SEND_NO_CUSTOMER_EMAIL / _CANCELLED / _COMPANY_SETTINGS_MISSING (400), INVOICE_UPDATE_NOT_DRAFT (409), INVOICE_SEND_PDF_RENDER_FAILED / _NUMBER_ASSIGN_FAILED (500), INVOICE_SEND_PROVIDER_FAILED (502). POST /api/v1/companies/:companyId/invoices/bulk-create Batch create up to 50 invoices in a single call, sequential processing, partial success: `{ results: [{ ok, request_index, data?, error? }], summary: { total, succeeded, failed } }`. Per-item rollback on items insert failure (delete the parent invoice row). Emits invoice.created per success. Dry-run wraps results in a preview without inserting. `all_or_nothing` is accepted but reserved for a future PR. lib/supabase/middleware.ts Add /reset-password bypass before the authenticated-user redirect so password-recovery sessions don't bounce to '/'. This should have landed in PR-455 — the `git add 'app/(auth)'` filter missed the middleware file at lib/. Without this the recovery email link silently fails for the recipient. Tests: 14 new integration tests across both routes (happy path, provider failure, scope rejection, draft-only guard, dry-run shape, bulk partial-success, max-50 enforcement, validation error). Full suite green (3207 passing, 1 unrelated pre-existing pg-real failure). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address PR #458 review — :send + bulk-create hardening Greptile P1: silent zero-row update after email delivery. PostgREST returns { error: null } on 0-row UPDATEs. The post-email status flip used `.eq('status', 'draft')` as an optimistic lock but never inspected the row count, so a concurrent state change (race, double-send from another session) would leave the DB row in 'draft' while the response claimed 'sent' and the email was already gone. Fix: `.select('id')` after the update and check `flipRows.length`; on 0-row miss, push STATUS_UPDATE_FAILED warning AND change the response status to 'draft' so the caller can reconcile. Greptile P1: re-read error swallowed; invoice_number could vanish from the response. After ensureInvoiceNumber, the re-read query destructured only `data`, silently dropping `error`. A transient connection failure would leave `numbered` null and `finalInvoiceNumber` undefined; JSON serialization would then omit the field, violating the documented response schema. Fix: capture `reReadErr`, log a warning, fall back to typed.invoice_number (which was just written by the RPC and is authoritative in-memory). Apply the same fallback at the top-level `ok()` call. Greptile P2 + Compliance Swarm V2.3 + Swedish-compliance kreditfaktura: reject credit notes from :send. The :credit endpoint creates credit notes atomically in 'sent' state with their own number — there is no v1 path that produces a draft credit note, so reaching :send with credited_invoice_id set is misuse or manual DB editing. Allowing it would assign an F-series number to a kreditfaktura (ML 17 kap 22–23§ require a distinct kreditfaktura series and a back-reference that this route would not enforce). Fix: reject with VALIDATION_ERROR pointing at /credit. Removes a stretch of dead code (originalInvoiceNumber lookup, kreditfaktura filename branch) that can never execute now. Greptile P2: all_or_nothing: true silently treated as false. A caller asking for atomic semantics must not get partial-success behaviour with no runtime signal. Fix: reject with new NOT_IMPLEMENTED error (501) plus a details.field pointer. Schema still accepts the flag for forward compatibility once a DB-side RPC ships. New error code added to lib/errors/structured-errors.ts. Tests: 3 new integration cases (credit-note rejection, status-flip no-op warning, all_or_nothing 501). Suite green: 3218 passing. Build clean, lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): PR #458 follow-up — defense-in-depth + comment precision OWASP V8.2.1 (bulk-create): use DB-returned customer.id at insert time instead of input.customer_id. The .eq() pair already enforces company scoping at fetch, but echoing the trusted value from the query makes the guarantee explicit at the call site and immune to refactoring drift. Same change in the dry-run preview shape. Swedish-compliance wording: the credit-note rejection comment now spells out BOTH ML 17 kap 22–23§ requirements — distinct kreditfaktura series AND explicit back-reference to the original invoice's löpnummer — so any future v1 path that does support credit-note send starts from a complete spec. Company-settings select: kept select('*') with an explanatory comment rather than enumerating columns. The InvoicePDF template consumes the full CompanySettings shape; a partial allow-list risks silently breaking rendering, and the table has no sensitive columns today (API tokens, billing data live in scoped tables). Documents the trade-off so the next reviewer doesn't re-litigate. Deliberately not changed: - Math.round → Math.trunc on VAT öre: CLAUDE.md mandates Math.round project-wide; unilateral deviation here would diverge from the bookkeeping engine and POST /invoices. - 207 Multi-Status on partial post-email failures: gnubok's convention is warnings[] in the 200 envelope; a per-route status divergence would break the response contract clients rely on. - Wrapper membership double-check (OWASP V8.2.1 send route): the withApiV1 wrapper sets ctx.companyId from the URL after the membership check — recurring false positive in this swarm. Tests + build + lint clean. 17/17 in the touched suites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-13 10:23:42 +02:00
Jakob Wennberg	cd96e5ec26	feat(api): v1 invoice :mark-paid + :credit action verbs (Phase 2 PR-B-2b combined) (#455) ... * feat(api): v1 invoice :mark-paid + :credit action verbs (Phase 2 PR-B-2b combined) Bigger PR per the user's request. Lands the remaining two journal-entry- centric action verbs together — they share the same lifecycle pattern established in :mark-sent (idempotent, dry-runnable, scope-gated, warnings on partial-state failures). POST /api/v1/companies/:companyId/invoices/:id/mark-paid - Books a payment against a sent / overdue invoice. Updates status to paid (or partially_paid when remaining_amount > 0). Three booking paths: - Faktureringsmetoden (accrual default): Debit 1930 / Credit 1510 via createInvoicePaymentJournalEntry — settles AR. - Kontantmetoden (cash): Debit 1930 / Credit revenue + Credit VAT via createInvoiceCashEntry — revenue recognition happens HERE under cash. - Custom lines (partial payment): caller-supplied balanced journal lines via createJournalEntry directly. Validated for balance (sum debits == sum credits, both > 0) → 400 INVOICE_PAID_LINES_UNBALANCED otherwise. - Optional body: { payment_date?, exchange_rate_difference?, lines? } - Race-condition guard: status update matches .in(['sent','overdue', 'partially_paid']) so a concurrent payment returns 409 INVOICE_PAID_RACE. - Emits invoice.paid (new event type, added to lib/events/types.ts with paymentAmount + paymentDate in the payload). POST /api/v1/companies/:companyId/invoices/:id/credit - Issues a kreditfaktura against a sent / paid / overdue invoice (ML 17 kap 22–23§). Creates a NEW invoice row with: - invoice_number = "KR-<original>" - credited_invoice_id = original id - status = 'sent' - All amounts negated (subtotal, vat_amount, total, items quantities/totals) - Items mirror the original with negated values; inserted in a separate step with company-scoped rollback DELETE on failure. - Flips original invoice to status='credited'. Warns ORIGINAL_NOT_FLIPPED if the flip fails (the credit note still exists; operator reconciles). - Posts reverse journal entry via createCreditNoteJournalEntry (accrual only; cash basis defers to refund time). - Emits credit_note.created (existing event in the bus). Both endpoints: - Use the established wrapper + Idempotency-Key + dry-run + warnings pattern from :mark-sent. - Validate document_type (no delivery_notes), credited_invoice_id (no recursive credits), and status before any mutation. - Use explicit column projections (no SELECT *). - Sanitize pg_message from client responses (kept in logs). - Emit error-level logs on partial-state failures + surface warnings to the caller via meta.warnings. Event types union (lib/events/types.ts) gains invoice.paid; credit uses the existing credit_note.created event. URL convention: plain /verb subpaths (e.g. /invoices/:id/mark-paid), consistent with :mark-sent. Stripe/QuickBooks pattern, not the AIP-style :verb that Next.js routing fights. 17 new tests covering happy paths (accrual + cash for mark-paid), custom-lines balance validation, dry-run preview, document-shape guards, scope, idempotency, race conditions, and credit-of-credit / delivery-note rejection. 3194/3194 vitest pass; build clean; lint clean on v1 paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address PR #455 review + include password-recovery fixes PR #455 review fixes: - Greptile P1 (CLAUDE.md architecture rule): API routes that emit events via eventBus must call ensureInitialized() at module level to wire extension event handlers. Neither :mark-paid (invoice.paid) nor :credit (credit_note.created) had it — nor did the already-merged :mark-sent, POST /invoices, POST /customers, etc. Fixed once at the wrapper layer: ensureInitialized() now runs at module import of lib/api/v1/with-api-v1.ts, so EVERY v1 route gets the init at import time. Single source of truth prevents future routes from forgetting (idempotent guard makes the repeated call safe). Cleaner than per-route copy of the call. - Swarm PI1.3 (low): 0.005 epsilon in mark-paid was undocumented. Added a comment explaining: after rounding to 2 decimals, newRemaining is in steps of 0.01; values ≤ half-an-öre only arise from float artefacts. Pushing back (recurring triage, consistent with prior PRs): - V8.2.1 + CC6.3 × 4 "ctx.companyId vs params.companyId mismatch" — impossible by construction. The wrapper sets ctx.companyId FROM the URL params after the membership check. They are guaranteed equal. - V2.3 + A.8.15 + A.8.28 atomicity / floating-point / partial-failure alerts — same architectural / cross-surface deferred work as prior PRs; matches internal /api/invoices pattern precisely. - V4.5 account_number allowlist — engine validates it. - V2.4 idempotency TOCTOU — wrapper handles via DB unique constraint. - Art.5(1)(f) PII in logs, A.8.11 dry-run preview scope, A.8.15 partial- failure naming, test scope coverage — all recurring triage. Password-recovery flow fixes (included per request — pre-existing working-tree changes the user authored): - app/(auth)/auth/callback/route.ts: when the callback exchanges a recovery token (type='recovery' or next='/reset-password'), redirect directly to /reset-password instead of running onboarding/MFA/ dashboard checks. Previously users clicking the password-reset email got bounced through onboarding. - lib/supabase/middleware.ts: /reset-password no longer bounces authenticated users to / (the recovery flow lands here with a fresh session by design — the user is *supposed* to call updateUser({ password }) on this page). - app/(auth)/login/page.tsx: shows an error banner when ?error=auth_error is set (expired/used recovery link), with a button to request a new one. Wrapped the page in <Suspense> because useSearchParams() now forces dynamic rendering (Next.js 16 static-prerender bail-out otherwise). - app/(auth)/auth/callback/__tests__/route.test.ts: new test file covering the recovery callback path. 3197/3197 vitest pass (3194 prior + 3 from the new auth-callback tests). Build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): mark-paid uses remaining_amount as default payment, not total Real correctness fix from Swedish-compliance review on PR #455. When no customLines is supplied, mark-paid previously defaulted paymentAmount to typed.total. Combined with the race-condition guard that allows the status UPDATE to flip a partially_paid invoice to paid, this could over-credit AR in a race scenario: 1. Invoice in 'sent' status, total=12500, remaining=12500. 2. Concurrent partial payment lands first → status='partially_paid', remaining=7500. 3. The full-payment request's pre-flight saw 'sent' and passed; its UPDATE matches partially_paid (race guard allows it). With the old logic the journal entry was for total=12500 against an AR balance of only 7500 — a 5000 over-credit. Using remaining_amount as the default eliminates this. Same end state in the common case (no prior partial); correct booking in the race. 3197/3197 vitest pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-12 23:52:18 +02:00
Jakob Wennberg	e4186523f9	feat(api): v1 invoice :mark-sent action verb (Phase 2 PR-B-2b-1) (#454) ... * feat(api): v1 invoice :mark-sent action verb (Phase 2 PR-B-2b-1) First invoice action verb. Transitions a DRAFT invoice to 'sent' status — intended for invoices delivered outside gnubok (Peppol, postal, custom SMTP). The full :send pipeline (PDF + email) builds on top of this in PR-B-2b-3. URL convention: plain /verb subpath (e.g. /invoices/:id/mark-sent), not the AIP-style :verb suffix the plan originally proposed. Next.js routes don't support `:` in folder names, and the Stripe/QuickBooks idiom is plain subpaths anyway. The agent-facing docs can still describe the action however we want. What happens on commit: 1. F-series invoice_number allocated atomically via the generate_invoice_number RPC (per the PR-B-2a design — drafts have invoice_number=null until this transition, preserving the unbroken löpnummer series required by ML 17 kap 24§ p.2). 2. Status flips draft → sent. 3. For accrual + real invoices, posts the invoice journal entry via createInvoiceJournalEntry (Debit AR 1510 / Credit revenue 3xxx / Credit output VAT 26xx). Cash basis skips this; booking happens at payment time. 4. Writes journal_entry_id back onto the invoice row. 5. Emits invoice.sent. Race-condition guard: the status update matches .eq('status', 'draft'), so a concurrent transition between pre-flight and update returns 409 INVOICE_UPDATE_NOT_DRAFT. Dry-run: returns a preview of the post-send invoice state including a would_create_journal_entry flag and the resolved accounting_method. invoice_number can't be predicted exactly (atomic sequence allocation) so the preview shows a marker rather than a fake number. PDF archival is deliberately NOT in this PR. The internal route does it, but PDF rendering + document upload is a meaningful surface area that belongs with :send (PR-B-2b-3) where email + PDF land together. Test infrastructure: the makeFlexibleSupabase mock now supports per-table result QUEUES (array form returns results in order across multiple calls; single value returns same result every time). Required to mock the pre-flight read (status=draft) and post-update read (status=sent) on the same `invoices` table inside one request. 9 new tests covering happy path, idempotency, scope, draft-only guard, delivery-note rejection, 404, UUID validation, dry-run preview shape, and cash-method skip. 3174/3174 vitest pass; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address PR #454 review (Greptile + swarm + Swedish compliance) Real bugs / contract violations: - Greptile P1 (guard order): the delivery_note guard ran AFTER the status check, so a sent delivery note returned 409 instead of the documented 400. Reordered: document-shape guards (delivery_note + credit_note + missing moms_ruta) now run before the status check. - Greptile P1 (journal_entry_id write-back): Supabase returns { data, error } and never rejects on DB errors, so a write-back failure produced no log and left the invoice with a real journal entry but no pointer. Now destructured + escalated to error log AND surfaced as a warning in the response. - Swedish: credit notes (credited_invoice_id !== null) were not rejected — they would have been posted via createInvoiceJournalEntry with the wrong sign (Debit AR / Credit revenue instead of the inverse). Now explicitly rejected; credit-note path goes through POST /:id/credit (PR-B-2b-4). - Swedish: moms_ruta now validated in the pre-flight. A null value would silently default to 25% domestic in the journal-entry generator — wrong for reverse-charge / EU-service / zero-rated invoices. Real ML 17 kap 24§ concern. Partial-state visibility (Swedish + Swarm V2.3 + A.8.15 + PI1.3): The response now carries an optional `warnings: [{ code, message }]` field when the status flip succeeded but a follow-up step failed (journal entry creation, event emission, or journal_entry_id write- back). Three warning codes: - JOURNAL_ENTRY_NOT_POSTED — verifikation missing; BFL 5 kap reconciliation required - JOURNAL_ENTRY_ID_WRITEBACK_FAILED — entry exists but invoice row has no pointer - EVENT_EMIT_FAILED — webhook subscribers may miss this transition All three escalate to error-level logs. The architectural fix (transactional Postgres RPC that bundles allocation + status flip + journal entry) is tracked as cross-surface compliance work; the warnings field is the agent-facing signal until that lands. The F-series race window (number allocated before status flip; a concurrent transition can leave a consumed-but-orphaned number, ML 17 kap 24§ p.2 gap) is now explicitly documented in the route docstring rather than hidden in implementation. Same residual issue exists in the internal route; fix needs the transactional RPC. Pushing back (consistent with prior triage): - V8.2.1 explicit ownership check (wrapper handles — false positive) - Cross-tenant IDOR test (duplicates wrapper test coverage) - Pseudonymise IDs in logs (operational value > theoretical risk) - Structured audit event sink (current ctx.log.info IS structured) - Test fixture A.8.33 (NODE_ENV guard in place; Acme AB is canonical synthetic placeholder) - Projection column narrowing (fields ARE used in the flow) - company_settings hard-fail on miss (accrual default is normal) 3 new tests covering credit-note rejection, missing moms_ruta, and the journal-entry-failed warnings path. Plus the delivery-note test now asserts the guard ordering works for sent delivery notes too. 3177/3177 vitest pass; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-12 23:19:21 +02:00
Jakob Wennberg	e96cbe05d0	feat(api): v1 invoice draft writes (Phase 2 PR-B-2a) (#453) ... * feat(api): v1 invoice draft writes (Phase 2 PR-B-2a) POST /api/v1/companies/:companyId/invoices creates a draft invoice, proforma, or delivery note. Reuses the established v1 discipline: - Idempotency-Key mandatory (wrapper option). - Dry-runnable: ?dry_run=true returns the validated would-be invoice + computed items with VAT totals; no DB writes, no number allocation, no event emission. - Explicit column projections (no SELECT *). - Per-item VAT rate validated against the customer's allowed rates from getVatRules() — mixed-rate invoices supported. - Currency conversion via fetchExchangeRate() (best-effort, non-fatal). - F-series number allocation via ensureInvoiceNumber() with soft-cancel rollback if allocation fails — preserves sequence integrity for ML 17 kap 24§ (no gaps in F-series). - invoice.created event emitted for real invoices (not proformas / delivery notes). PATCH /api/v1/companies/:companyId/invoices/:id updates a DRAFT invoice's metadata fields only: - Allowed: invoice_date, due_date, delivery_date, your_reference, our_reference, notes. - NOT allowed (intentional): customer_id, currency, document_type, items, status. Structural changes go through delete-and-recreate (drafts are cheap); status transitions via the action verbs in PR-B-2b. - 409 INVOICE_DELETE_NOT_DRAFT if the invoice has already been sent / paid / credited / cancelled. The error code is shared with DELETE (reused rather than introducing a new "not draft" code). - Race-condition guard: the .update() also matches .eq('status', 'draft') so a concurrent :send between pre-flight and write returns the same 409. Dry-run for invoice DRAFT create uses dryRunPreview() (validation-only) rather than dryRunStaged() — drafts have no journal-entry side effects yet, so there's nothing to stage in pending_operations. The dryRunStaged() helper from PR-B-1 stays unused this PR; PR-B-2b's :send will be its first real consumer (voucher number, journal lines, account deltas). Tests: 12 new (5 POST + 7 PATCH) covering happy path, customer not found, VAT rate violation, dry-run preview shape, scope enforcement, Idempotency-Key requirement, draft-only PATCH guard, forbidden field rejection, UUID validation, empty body. Stubs ensureInvoiceNumber and fetchExchangeRate to keep tests deterministic. 3165/3165 vitest pass; build clean; lint clean on v1 paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address PR #453 review (Greptile + swarm + Swedish compliance) Real fixes (all reviewers agreed): - Greptile P1 + SOC 2 CC6.3: PATCH was reusing INVOICE_DELETE_NOT_DRAFT (httpStatus 400) for a semantically different operation; docstrings + tests claimed 409 while code returned 400. Introduced INVOICE_UPDATE_NOT_DRAFT with httpStatus 409 in structured-errors.ts. PATCH now returns 409 consistently; test name and assertion aligned. - Greptile P1: POST rollback DELETE on items-insert failure now scoped by company_id (defense in depth) AND its error is destructured/logged so a double-failure is visible in audit trails (was previously silent on the rollback path). - Greptile P1: refetch error after invoice insert is now logged with invoiceId + companyId at warn level; the response gracefully falls back to the header-only shape rather than misleading the agent with a 5xx (the data WAS committed). GDPR Art.5(1)(f) × 2, ISO A.8.11 × 2, SOC 2 CC7.2 × 2: client-facing error responses no longer echo raw Postgres pg_message strings (which can interpolate field values from constraint detail). pg_code is kept in the response (machine-readable, no PII leak); pg_message moves to the internal structured log entry only. Applies to INVOICE_CREATE_INSERT_FAILED and INVOICE_CREATE_ITEMS_FAILED. OWASP V2.2: defensive UUID validation on ctx.companyId at POST handler entry. The wrapper already validated membership, but mirroring the detail-route's pattern for path params eliminates a class of edge-case queries with malformed predicates. Swedish compliance (ML 17 kap 24§ p.2 — most substantive finding): ensureInvoiceNumber is NO LONGER called at draft-create. The doc string already said "F-series invoice_number is allocated atomically on the first send action (PR-B-2b)" but the code contradicted it by allocating at POST. Code now matches intent: drafts (invoices and proformas) keep invoice_number=null until :send. Delivery notes continue to allocate their separate D-series number on insert (different sequence, no F-series gap concern). This eliminates the soft-cancel path entirely for the common case where a user creates and abandons a draft — no more legal gaps in the löpnummer series from ordinary workflow. Pushing back on: - Atomicity / Postgres RPC wrapping (V8.2.1 × 2, CC6.1) — substantial refactor; the existing internal /api/invoices POST has the identical multi-step pattern; not a v1 regression. Track for a future RPC- consolidation PR across both surfaces. - Float-point VAT rounding (V2.3, Swedish #3) — matches internal route precisely; consistency over premature decimal-library migration. - TOCTOU rewrite to single UPDATE-WHERE-RETURNING (V8.2.1, CC6.1) — current pre-flight + scoped UPDATE is correct; the suggested cleanup is stylistic. - PATCH response verbose projection (A.8.3, Art.25) — consistency with detail endpoint; the agent that just updated likely wants the full record back. - per-line moms_ruta (Swedish #4) — schema migration; the existing header-only column is what the codebase has. - Event emission failure alerting (A.8.15) — defer to PR-C webhooks. - Test fixture A.8.33 — already addressed (NODE_ENV guard at test bootstrap, clearly synthetic UUIDs). Test fixture UUID v4 fix: COMPANY_ID upgraded to proper v4 format (was 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', which fails Zod 4's .uuid() version-digit check now that the POST handler validates companyId). 3165/3165 vitest pass; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-12 22:58:56 +02:00
Jakob Wennberg	d29b87fb80	feat(api): v1 customer writes (Phase 2 PR-B-1) (#452) ... * feat(api): v1 customer writes (Phase 2 PR-B-1) First slice of Phase 2 PR-B from the agent-native v1 plan. Customer writes are the simplest write surface — no journal entries, no PDF, no email — which makes them the right place to validate the dry-run + idempotency pipeline before applying it to invoice flows in PR-B-2. New endpoints: - POST /api/v1/companies/:companyId/customers (idempotent, dry-runnable) - PATCH /api/v1/companies/:companyId/customers/:id (idempotent, dry-runnable) - DELETE /api/v1/companies/:companyId/customers/:id (soft-delete via archived_at; idempotent re-archiving; dry-runnable; 204) All three require customers:write scope (added to V1_ENDPOINT_SCOPES). All three require Idempotency-Key (mandatory — wrapper option requireIdempotencyKey: true). All three accept ?dry_run=true or X-Dry-Run: true and return a 200 OK preview with X-Dry-Run header set. New shared infrastructure: - lib/api/v1/dry-run.ts — dryRunPreview() (validation-only, no staging) and dryRunStaged() (financial writes; populated in later phases). Defines the preview response shape that POST/PATCH/DELETE share. Future financial writes (invoices, journal entries) will reuse the staged variant. Pre-existing PR-A bugs fixed: - CustomerType enum in customers/route.ts had wrong values ('business', 'eu_individual', 'non_eu'). Canonical enum is ['individual', 'swedish_business', 'eu_business', 'non_eu_business']. Fixed. - INDIVIDUAL_TYPES masking referenced non-existent 'eu_individual'. Only 'individual' refers to a natural person (Swedish sole trader where org_number = personnummer). Wrapper bug fix: - The idempotency body-hashing flow was consuming the original request's body before passing it to the handler. In Node's vitest environment the cloned request's body became empty, so the handler's request.json() returned {}. Fix: read body from a clone for hashing, leave the original intact for the handler. VIES re-validation on PATCH preserves existing best-effort behaviour. Customer.created event emission on POST so future webhook delivery (Phase 2 PR-C) can subscribe. 23 new tests covering happy path, dry-run preview, idempotency-key enforcement, scope checks, UUID validation, duplicate-org conflict, soft-delete semantics. 3150/3150 vitest pass; build clean; lint clean on v1 paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address PR #452 review (Greptile + swarm + Swedish compliance) Real bugs (all reviewers agreed): - POST registerEndpoint example used the old customer_type 'business' enum value that this PR was already fixing. Now uses 'swedish_business' consistently between code, schema, and docs. - DELETE docstring claimed "Refuses to delete if open invoices remain" but the handler unconditionally archived. Swedish compliance flagged this as a real ML 17 kap 24§ concern (archived customer + open invoice can block kreditfaktura issuance). Added the pre-flight check: DELETE now returns 409 CUSTOMER_HAS_INVOICES with the open count when any open invoice (sent / partially_paid / overdue) references the customer. Docstring updated to match. - PATCH advertised archived_at: null for un-archive but did not actually apply it (field missing from updateData iteration). New V1PatchCustomerSchema extends UpdateCustomerSchema with archived_at restricted to literal null — agents can un-archive but cannot fake archive timestamps. The PATCH allowlist now includes archived_at. Defensive cleanups: - POST: VIES validation now resolves BEFORE the insert so vat_number_validated is set atomically in the primary write. Eliminates the stale-response window where the response could show vat_number_validated=false even though the secondary update succeeded. - PATCH: same pattern — VIES re-validation folded into the primary update payload. Single round-trip; response always reflects committed DB state. - CUSTOMER_RESPONSE_COLUMNS dropped vat_number_validated_at (internal timestamp; not declared in the CustomerCreated or CustomerDetail Zod schemas; no documented consumer). Pushing back on: - V8.2.1 cross-tenant membership check — false positive. The wrapper performs the company_members check before invoking the handler (lib/api/v1/with-api-v1.ts ~232). The swarm read handlers in isolation. - A.8.11 PATCH response masking for individual customer_types — deliberate detail-endpoint carve-out per PR-A. List masks, detail doesn't; that's the design. - CC6.3 separate customers:delete scope — every accounting API (Stripe, QuickBooks, Fortnox) conflates write + archive. Splitting violates principle of least surprise. - V4.5 schema allowlist enforcement — Zod already strips unknown keys; defense-in-depth at the DB-write layer is redundant. - A.5.34 eu_individual masking documentation — the value never existed in canonical CustomerTypeSchema; PR-A's enum was a hallucination this PR corrects. Nothing to document beyond the code comment that's now in place. - Swedish: country default 'Sweden' wrong for non-Swedish customer types — breaking schema change; defer. - Swedish: VAT-format regex pre-check before VIES — internal /api/customers route doesn't either; consistency over micro-validation. - Swedish: flag existing reverse-charge invoices when VIES turns vat_number_validated=false — substantial cross-resource workflow; defer to PR-B-2 or a dedicated compliance-tooling PR. - CC7.2 customer.updated / customer.archived event emission — adding new event types touches lib/events/types.ts AND the event-log-handler allowlist; defer to PR-C where webhooks will be the consumer. 3 new tests cover: archive-blocked-by-open-invoices, PATCH archived_at un-archive, PATCH archived_at rejects non-null. 3153/3153 vitest pass; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): second-pass review on PR #452 — defense-in-depth tweaks Two small adjustments after the second compliance-swarm sweep (13 → expected 11 findings, 0 blocking after this commit): - GDPR Art.5(1)(c) defense-in-depth: re-add 'eu_individual' to the INDIVIDUAL_TYPES masking set in the customer LIST handler. The value is not in the canonical CustomerTypeSchema (so new customers can never have it), but the `customer_type` DB column carries no CHECK constraint, so legacy rows from earlier schema iterations could in principle hold it. Masking is free when the value never appears and protective if it ever does. Adding 'eu_individual' as a first-class customer_type for EU natural persons remains a separate product decision the Swedish compliance review surfaced. - ISO 27001:2022 A.8.33: test bootstrap now asserts NODE_ENV === 'test'. Supabase clients are fully mocked, but if a future test refactor accidentally bypassed the mock, this guard fails the run rather than letting fixtures reach production. Stale comments from prior sweep — no action needed, fixes already in commit 5bb63489: - Greptile P1 "DELETE doesn't check open invoices" — handler now does (lines ~430-440 of [id]/route.ts); the inline comment is pinned to the original file lines and hasn't auto-resolved. - Greptile P1 "PATCH archived_at silently ignored" — V1PatchCustomerSchema now accepts archived_at: z.null().optional() and the field is in the iteration list. - Greptile P1 "example uses old enum" — updated to 'swedish_business'. False positive called out: - OWASP V2.4 "dry-run DELETE skips the open-invoice pre-check" — the pre-flight runs BEFORE the dry-run branch ([id]/route.ts ~430-445), so dry-run DELETE on a customer with open invoices DOES return 409. Pushing back (consistent with first-pass triage): - V8.2.1 × 3 cross-tenant check — wrapper does it (with-api-v1.ts ~232); false positive. - V2.3 / CC6.3 archive scope split — every accounting API conflates write + archive. - V4.5 customer_type cross-field invariants — schema-level cross-field validation; defer. - V16 transactional outbox — architectural; defer to PR-C webhooks. - Art.25 + A.5.12 + A.5.34 PATCH/dry-run preview masking — single-record detail context, deliberate carve-out per PR-A. - Swedish 'disputed' status — not in canonical InvoiceStatus enum. - Swedish 3-state VIES validation, personnummer-format check, mandatory country for non-SE types — all schema-level / cross-field; defer. 3153/3153 vitest pass; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): don't echo org_number in 409 conflict response (GDPR Art.5(1)(c)) For customer_type='individual', org_number IS the Swedish personnummer. The 409 CUSTOMER_DUPLICATE_ORG_NUMBER error detail previously included the submitted value, transmitting it through: - The HTTP response body - Server / observability logs - Any HTTP intermediary recording bodies The caller already knows what they submitted; the error code + a { field: 'org_number' } hint is enough. Drops the value from the detail in both POST /customers and PATCH /customers/:id. 3153/3153 tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-12 22:31:46 +02:00
Jakob Wennberg	32ad6da28c	feat(api): v1 invoice + customer reads (Phase 2 PR-A) (#451) ... * feat(api): v1 invoice + customer read endpoints (Phase 2 PR-A) First slice of the Phase 2 invoices vertical. Read-only endpoints landing in this PR; writes + webhooks land in PR-B and PR-C. After all three a developer can ship an end-to-end invoicing integration. New endpoints (all wrapped, scoped, cursor-paginated): - GET /api/v1/companies/:companyId/invoices — list, filters: status, customer_id, document_type, currency. Cursor on (invoice_date DESC, id DESC). Customer name embedded inline; ?expand=customer for full record, ?expand=items for line items. - GET /api/v1/companies/:companyId/invoices/:id — detail with embedded customer. ?expand=items,payments. - GET /api/v1/companies/:companyId/customers — list, filters: customer_type, search (name/org_number prefix), include_archived. Cursor on (created_at ASC, id ASC). - GET /api/v1/companies/:companyId/customers/:id — detail. ?expand=invoices embeds open invoices in a single round-trip. Shared infra: - lib/api/v1/expand.ts — parseExpand() validates ?expand=a,b,c against a per-endpoint allowlist; unknown keys yield VALIDATION_ERROR with the full invalid list and the allowlist (agent-friendly). - All four routes register with the Zod schema registry so they show up in /api/v1/openapi.json with x-action-risk and use-when / do-not-use-for metadata. - Compound keyset filter on both list endpoints (per Greptile review on PR #450) — no skipped or duplicated rows on page boundaries. Tests: - lib/api/v1/__tests__/expand.test.ts (8 tests) - app/api/v1/companies/[companyId]/invoices/__tests__/route.test.ts (10 tests) - app/api/v1/companies/[companyId]/customers/__tests__/route.test.ts (8 tests) Full repo suite green (3127/3127), build clean, lint clean on v1 paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address PR #451 review (Greptile + compliance swarm) - Greptile P1 (customers search) + OWASP V1.2.5: customer search term now escapes both PostgREST .or() delimiters (,()) AND SQL LIKE wildcards (% _ \). '100%' searches for the literal string instead of any customer containing '100'. - OWASP V8.2.1 + V16.1: detail endpoints now UUID-validate the :id path param before touching the database, and no longer echo the raw id in the NOT_FOUND response details. Adds a structured warn log on 404 with the queried (id, companyId) for audit purposes. - V4.5 + Art.25(1) + A.8.3 / A.8.11 + CC6.3 + PI1.3 (~12 findings): every select('*') replaced with explicit column lists per the documented Zod schemas. Includes joined sub-queries — customer:customers(...), items:invoice_items(...), payments:invoice_payments(...). Future schema migrations adding sensitive columns must now update these projections before the field becomes visible on the public API. - A.8.5: hardcoded 'Bearer gnubok_sk_x' in test fixtures replaced with 'Bearer test-fixture-not-a-real-key' to avoid false-positive secret scanner alerts. Fixture UUIDs upgraded to valid v4 format (Zod 4's .uuid() enforces version+variant digits). - Art.5(1)(f): customer-invoices expansion soft-degrade now logs only the error code + message rather than the full Supabase error object. Pushing back on: - Art.5(1)(b) org_number in customer list — Bolagsverket-public data (same triage as PR #450; required by integration use case) - Art.25(2) customer_name always-joined — denormalising via trigger is a real schema migration for a marginal data-flow gain - A.8.15 _partial flag on soft-degrade — ?expand is documented as a hint 50/50 v1 tests; 3131/3131 full suite; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): second-pass review on PR #451 — partial_expansions + fake fixtures Address the residual compliance-swarm findings after the first fix round: - CC6.1 (medium): the customer-detail handler now sets meta.partial_expansions=['invoices'] when the ?expand=invoices subquery fails, signalling the degraded response to the caller without escalating to error-level logs (alert fatigue). The primary resource still returns with an empty invoices array. New ResponseOptions.partialExpansions threaded through buildMeta(). 1 new test for the failure path, plus a happy-path assertion that the flag is absent. - A.8.33 (low): SAMPLE_CUSTOMER fixture's org_number and vat_number replaced with 'TEST-0000-0001' / 'SETEST00000001' — cannot be confused with real Bolagsverket entries or pass external VIES validation. Pushing back on: - CC6.3 (medium) — separate scope for ?expand=items on invoices: every accounting API I know (Stripe, QuickBooks, Fortnox) treats line items as part of the invoice resource. Splitting would violate principle of least surprise for integrators; the plan deliberately treats invoices:read as covering the full invoice including items. 3132/3132 vitest pass; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): third-pass review on PR #451 — PII minimisation refinements Third compliance-swarm sweep (6 → 0 highs, 4 medium, 2 low). Addressed: - Art.5(1)(c) personnummer leakage: customer LIST response now masks org_number AND vat_number for customer_type IN ('individual', 'eu_individual') — for sole traders (enskild firma) org_number IS the personnummer. Business customers' Bolagsverket-public org_numbers stay visible. Detail endpoint (deliberate single-record fetch) unchanged. - Art.5(1)(c) over-broad invoice-list expansion: ?expand=customer on the invoice LIST endpoint now uses a new CUSTOMER_LIST_CONTEXT_COLUMNS projection (id, name, customer_type, email, country, archived_at) — full address/phone/notes/vat_number stay on the customer DETAIL endpoint. Drops PII transmitted in bulk-list contexts by ~60%. - A.8.15 permission-error differentiation: customer-detail soft-degrade for ?expand=invoices now bumps Postgres error class 42 (insufficient privilege, RLS denial) to error-level log so Sentry alerts on misconfigurations. Transient errors stay at warn. - PI1.1 ISO-4217 currency: invoice list ?currency now requires /^[A-Z]{3}$/ instead of accepting any 3-8 char string. Two new tests. Pushing back on: - Art.5(1)(f) UUID logging on 404 — UUIDs have 122 bits of entropy; you cannot enumerate the space, so the "log scraping = enumeration" framing doesn't hold. Operational audit value > theoretical risk. - Art.25(1) notes-by-default in customer DETAIL — kept inline. Detail is a deliberate single-record fetch; the dashboard shows notes inline; agents calling /customers/{id} reasonably expect them. Notes are already excluded from the LIST endpoint AND from the invoice-list ?expand=customer projection (above). 3135/3135 vitest pass; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-12 21:49:22 +02:00
Jakob Wennberg	db592d922d	feat(api): v1 REST API foundation — auth wrapper, scopes, registry, smoke endpoints (#450) ... * feat(api): v1 REST API foundation — auth wrapper, scopes, registry, smoke endpoints Lay the substrate for the public REST API at /api/v1/*: Bearer-auth wrapper that reuses the existing api_keys + idempotency machinery, an extended scope catalogue (companies, events, webhooks, operations, documents, compliance), v1 response envelopes (data + meta with request_id, api_version, audit block, cursor pagination), an error envelope with recovery_hint / docs_url / valid_alternatives derived from the existing structured-error registry, and a Zod schema registry that generates the OpenAPI 3.1 spec with x-action-risk / x-idempotent / x-reversible / x-dry-run-supported extensions. Ships discovery routes (/llms.txt, /.well-known/skills/index.json) and three smoke endpoints (GET /api/v1/health, /api/v1/companies, /api/v1/openapi.json) so the wrapper is exercised end-to-end. Includes the api_keys.mode (test|live) migration and 41 unit tests covering auth, scope, company-membership, idempotency replay, dry-run, pagination, response shape, and scope resolution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): harden v1 foundation — cursor validation, security headers, forensic logs Address compliance-swarm findings on PR #450: - OWASP V2.3: decodeDefaultCursor now validates the cursor's ts as ISO 8601 and id as UUID. A crafted cursor previously could inject untyped strings into a query's .gt(field, value); PostgREST would have rejected them, but validating here keeps the failure mode predictable (stale cursor → reset) rather than 400-ing. - OWASP V3.4: public discovery routes (llms.txt, .well-known/skills, openapi.json) now stamp X-Content-Type-Options: nosniff, Referrer-Policy, X-Frame-Options: DENY. New lib/api/v1/security-headers.ts helper. - OWASP V16: security event logs (missing token, validation failure, insufficient scope, company-membership deny) now include source IP (x-forwarded-for / x-real-ip) and User-Agent for forensic correlation. - OWASP V8.2.1 / ISO A.8.3: GET /api/v1/companies emits a warn log when the PostgREST archived_at filter unexpectedly returns a row with a null company join, surfacing silent data-integrity regressions instead of hiding them behind the existing pickCompany() === null filter. Pushing back on (not changed): - GDPR Art.32 cursor HMAC signing — cursors only paginate within a user's own user_id scope; cross-tenant probe surface doesn't exist yet. - GDPR Art.25 org_number in list — Bolagsverket public-record data, removing forces N+1 fetches to make the response useful. - SOC 2 CC6.3 service-role bypasses RLS — defense-in-depth IS the design; the wrapper's company_members membership check is the technical control. - ISO A.8.12 public OpenAPI spec — intentional, mirrors Stripe/Twilio. 5 new pagination tests cover the cursor validators. 46/46 v1 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auth): detect Supabase duplicate-signup obfuscation on register Supabase obfuscates duplicate signups to prevent user enumeration: when an email already belongs to a confirmed account, signUp returns data.user with identities: [] and no error, and sends no email. Without detecting this case we showed the "check your email" screen to the user, who then waited for a mail that never arrived. Detect the empty-identities response and surface it via duplicateEmail state so the UI can branch on it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): second-pass hardening — CSP, IP truncation, cursor scope comment Address the second compliance-swarm sweep on PR #450: - OWASP V3.2: PUBLIC_SECURITY_HEADERS now includes Content-Security-Policy default-src 'none'; frame-ancestors 'none'. Free win for JSON/text-only public routes (no script, style, image, or form contexts). - GDPR Art.5(1)(f): truncate IPs before logging — IPv4 to /24, IPv6 to /48. Preserves diagnostic value (ASN, abuse-pattern correlation, city-level geolocation) while eliminating point-of-presence identification. Standard pattern used by Google Analytics anonymize_ip. Exported truncateIp() so other surfaces can adopt it. - OWASP V8.2.1: explicit comment in GET /api/v1/companies documenting that the cursor's joined_at is applied AFTER user_id filter, so a tampered cursor can only reorder rows the caller already owns. Cursors deliberately unsigned; trade-off documented. Pushing back on second-pass findings (not changed): - ISO A.8.12 / SOC 2 CC6.3 health/llms.txt/skills exposing service name + API version + MCP URL — these are intentional disclosures for a public 3rd-party developer API; hiding them is theatre. - GDPR Art.32 logging granted scopes on INSUFFICIENT_SCOPE — diagnostic value during incident response outweighs the theoretical privilege-profile leak; an attacker who already breached the log store has bigger problems. - OWASP V2.2 route-level Zod for cursor — decodeDefaultCursor already validates strictly; route-level Zod is stylistic. - GDPR Art.25(2) org_number/entity_type in list — Bolagsverket-public data; entity_type materially affects which API calls make sense. - ISO A.8.15 x-forwarded-for trusted-proxy CIDR — overkill behind Vercel's edge which rewrites the leftmost value. 50/50 v1 tests pass (4 new for truncateIp). Build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): third-pass hardening — Host header injection, anon client, HSTS Address the third compliance-swarm sweep on PR #450: - SOC 2 CC6.1 (3× high): llms.txt, openapi.json, and .well-known/skills built URLs from the inbound Host header. A spoofed Host could poison agent discovery with attacker-controlled endpoints. New lib/api/v1/base-url.ts centralises canonical base-URL derivation via NEXT_PUBLIC_APP_URL (already a required env var per CLAUDE.md). - ISO A.8.2 / A.8.5 (2× high): the wrapper's public-scope code path now uses an anon-key Supabase client (RLS-respecting) instead of the service-role client. A future accidental DB call from a public handler is constrained to anon-accessible rows. Least-privilege at the infrastructure layer. - OWASP V3.2 (medium): PUBLIC_SECURITY_HEADERS now includes Strict-Transport-Security: max-age=31536000; includeSubDomains. - GDPR Art.5(1)(f) (medium): truncateIp now logs a warn when a non-empty x-forwarded-for / x-real-ip payload fails to parse, surfacing spoofed or unexpected proxy values to security monitoring instead of silently dropping them. The raw value is never logged. - CC2.3 (low): llms.txt now links the SECURITY.md disclosure policy with the security@arcim.io reporting address so agents have a clear responsible-disclosure path. Pushing back on third-pass findings (not changed): - Cursor HMAC signing — user_id filter is the authorisation boundary; cursor scope is bounded to within-user rows. Documented in code. - org_number in companies list — Bolagsverket public data; the swarm's "could be enskild firma personnummer" framing isn't accurate (enskild firma org_number IS the personnummer, but it's already in the public Bolagsverket business register). - Health endpoint information disclosure — intentional for a public developer API; matches Stripe/Twilio convention. - llms.txt / skills index MCP URL disclosure — that's the file's purpose. - Cache-Control public on discovery routes — content is by definition public; getCanonicalBaseUrl() removes the previous spoof concern. - Duplicate-email screen — user's own input; out of scope for this PR. 50/50 v1 tests pass; @supabase/supabase-js#createClient mocked so the public-path tests don't need real env vars. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): widen validateApiKey result assertions to include mode field The core-only CI job failed on two pre-existing api-keys.test.ts assertions that used strict toEqual matching against the old (userId, companyId, scopes) shape. The wrapper migration in this PR widened that shape with mode, apiKeyId, and apiKeyName. Update both existing assertions to match the current shape and add a third test that exercises the mode='test' path. 3027/3027 vitest tests now pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): fourth-pass hardening — env guards, IP range check, headers on wrapped routes Address the fourth compliance-swarm sweep on PR #450: - ISO A.5.17 / SOC 2 CC6.1 (high): createAnonClient now fails closed with an explicit Error if NEXT_PUBLIC_SUPABASE_URL or _ANON_KEY are missing, surfacing misconfiguration on the first request instead of throwing deeper in the handler with no context. - GDPR Art.5(1)(f): truncateIp now rejects IPv4 with out-of-range octets (>255). '999.999.999.999' now returns undefined instead of a pseudo-IP that would pollute abuse-pattern analysis. Edge octets (0, 255) still accepted. 2 new tests. - OWASP V3.2 / V3.3: the wrapper's stampHeaders step now applies the full security header set to every wrapped v1 response (CSP, HSTS, X-Frame, Referrer-Policy, X-Content-Type-Options) PLUS X-Robots-Tag: noai, noimageai so authenticated payloads are excluded from AI training sets. Public discovery routes (llms.txt, skills index, openapi.json) deliberately omit X-Robots-Tag — being AI-discoverable is the whole point of those surfaces. - New WRAPPED_RESPONSE_HEADERS export separates the two contexts. Pushing back on: - SOC 2 CC6.1 medium "API key prefix in public docs aids brute force" — inverted logic. Every public API publishes its key prefix specifically so secret scanners (GitHub Advanced Security, GitLeaks) can detect leaks. Stripe (sk_live_), GitHub (ghp_), OpenAI (sk-) all do this. - SOC 2 CC6.3 medium "formal risk register for unsigned cursors" — org -level documentation, outside this PR. Code-comment already documents the trade-off. - SOC 2 CC2.3 low "llms.txt hardcodes security@arcim.io" — same address as SECURITY.md; no drift risk. Flagged separately (not changed): the register-page duplicate-email detection in this branch defeats Supabase's user-enumeration obfuscation (GDPR Art.5(1)(c) × 2, ISO A.8.11). Substantive product decision: UX (no infinite-wait for non-existent accounts) vs security (no enumeration). GitHub and Stripe Atlas pick UX; some pick security. Owner's call. 3029/3029 vitest tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): address Greptile review on PR #450 - P1 (companies/route.ts): keyset pagination was missing its tiebreaker. The cursor encoded (joined_at, id) but the filter only applied .gt('joined_at', ts) — same-joined_at rows on a page boundary could be skipped or duplicated. Also the encoded id was companies.id while the sort was on company_members, mismatched. Fixed: select + sort + encode on company_members.id, apply compound joined_at.gt.{ts} OR (joined_at.eq.{ts} AND id.gt.{cursor_id}) via .or(). Side benefit — eliminates the broken-cursor-on-null-join case (#2) because company_members.id is always present, no null guard needed. - P2 (registry.ts): ZodUnion branch had a dead ternary (['x','y','z','w'].length > 0 ? undefined : 'object') that always yielded undefined. Removed; emit { oneOf: [...] } without top-level type (correct JSON Schema for a union). - P2 (with-api-v1.ts): public-endpoint path was short-circuiting before Bearer-token validation, contradicting the JSDoc and PR description. Now opportunistically validates a supplied token for rate-limit attribution + key tracking; missing/invalid token silently falls back to anon (the route is public by definition, so we don't 401). Two new tests cover both branches. 3031/3031 vitest tests pass; build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-12 20:47:13 +02:00
Jakob Wennberg	eb77ad50b5	feat(mcp): add create_voucher + correct_entry MCP tools (#448) ... * feat(mcp): add create_voucher + correct_entry MCP tools The MCP toolset had no way to post a journal entry outside the preset workflows (categorize_transaction, create_invoice, …). That blocks legitimate flows the engine already supports — K3 capitalization to BAS 1010, period-end accruals, FX adjustments, prepayments, and rättelseposter for foreign reverse-charge VAT that landed on 2641 instead of 2614/2645. create_voucher exposes the existing createJournalEntry() primitive: arbitrary balanced lines, optional fiscal-period auto-resolution, staged for human approval. correct_entry exposes correctEntry() (storno + new corrected entry per BFL 5 kap 5§) so part of a posted verifikation can be fixed without losing the legs that were right. Both are HIGH risk in OPERATION_RISK_TIERS — the arbitrary account/amount/ period inputs make them compliance-critical despite being structurally similar to uncategorize_transaction (medium). Approval flow unchanged; no auto-commit, regardless of trust level. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): address PR #448 review — voucher tools hardening Greptile P1 + compliance bot findings, all in one pass. commitCreateVoucher (commit.ts): - Hardcode source_type to 'manual' instead of reading from params. A future direct-staging path or hand-inserted pending_operations row could otherwise inject 'bank_transaction'/'invoice_created'/etc. and corrupt the audit-trail origin. - Re-validate balance defensively before reaching the engine, so a tampered params row surfaces a clean Swedish 400 instead of an opaque engine error. gnubok_create_voucher (server.ts): - Validate the explicit fiscal_period_id when supplied: confirm it exists, is open (is_closed = false), and that entry_date falls within its span. Without this, a closed/locked period was only caught at commit-time with a generic DB-trigger error. - Throw at staging when any line targets an account that's missing from chart_of_accounts or marked inactive, rather than relying on the approver to spot the advisory flag. - Remove source_type from the staged params blob entirely — the executor ignores it anyway, no point letting it travel through. - Add a comment that the staging-time period-lock check is advisory and the executor is the authoritative guard, so future cleanup doesn't remove either as 'redundant'. Descriptions: - gnubok_correct_entry now explicitly notes that the storno + corrected entries land in the original period (defends against compliance bot's speculative "different period" concern recurring on future reviews). - Both tools' tax_code field gets a note that the BAS account number drives momsdeklaration ruta mapping, not tax_code — guards against an LLM treating tax_code as the VAT-routing dial. commitCorrectEntry (commit.ts): - Add a comment pointing at storno-service.ts:99,102,195,198 to make the "uses original period and date" invariant explicit in this file. Tests: - +2 voucher-executors cases: source_type tampering is ignored, unbalanced params return 400. - +10 new voucher-tools tests (MCP layer): unbalanced, closed explicit period, missing explicit period, entry_date outside period, unknown account, inactive account, happy path + correct_entry registration + unbalanced replacement. 720 tests pass in the impacted suites; full suite 2998/2998. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-12 17:16:39 +02:00
Mattsson	a53a119a2e	Fix/vat parent accounts (#438) ... * feat(enable-banking): add support for account selection and syncing - Updated StoredAccount interface to include an 'enabled' flag for account syncing preferences. - Enhanced ensureFiscalPeriod function to handle overlapping fiscal periods with posted entries and opening balances. - Added tests for fiscal period validation and account syncing logic. - Implemented AccountPickerDialog component for user account selection. - Created API routes for PATCH /accounts and POST /sync to manage account syncing. - Introduced 'pending_selection' status for bank connections to allow user account selection before syncing. - Updated database migration to support new connection status and backfill existing accounts with enabled=true. * feat(enable-banking): implement account selection and consent event logging --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-11 17:12:00 +02:00
Mattsson	e77423d099	feat(transactions): add tool to list transactions without documents and update scope map (#403)	2026-05-06 17:14:59 +02:00
Jakob Wennberg	4131db2894	chore: MCP intent-tools, BankID enrichment table, multi-tenant fixes (#402) ... * chore: MCP intent-tools, BankID enrichment table, multi-tenant fixes MCP server gains six intent-shaped tools that collapse multi-call agent flows into one: vat_close_check, query_journal, auto_match_period, create_supplier_invoice_from_inbox, audit_package, year_end_readiness. Tools wired into TOOL_SCOPE_MAP and OPERATION_RISK_TIERS as appropriate (create_supplier_invoice_from_inbox at medium tier — reversible until approve, but stages a leverantörsskuld). BankID enrichment now persists to a dedicated bankid_enrichment table keyed by user_id. extension_data has been company-scoped (NOT NULL company_id) since the multi-tenant refactor, so every BankID signup has silently been failing the enrichment upsert. Select-company picker reads from the new table. delete_last_voucher (BFNAR 2013:2) needs to clear document_attachments.journal_entry_id before deleting the entry, but the new document immutability trigger blocks that UPDATE. Added the same gnubok.allow_delete transaction-scoped bypass pattern used by the journal-entry/line/retention triggers. pg-real tests cover the happy path, the unauthorized direct UPDATE, and the swap-to-different-entry attempt under the bypass flag. fiscal_periods.no_overlapping_fiscal_periods exclusion was scoped to user_id from before multi-tenant — rebound to company_id so the same user can have overlapping fiscal years across companies they own/are member of. Also adds scripts/seed-demo-account.ts for end-to-end demo seeding (two companies, full FY2025, active FY2026 with mixed state). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pr-402): address review feedback Migrations - Drop 20260506140000_document_journal_entry_immutability_delete_bypass.sql: redundant with 20260506140000_document_journal_entry_immutability_bypass.sql that landed on main while this branch was open. Both share the same gnubok.allow_delete pattern; main's version is what the DB actually has. - Rename 20260506150000_bankid_enrichment_table.sql → 20260506160000_bankid_enrichment_table.sql to clear the timestamp clash with 20260506150000_protect_document_journal_link.sql on main (Supabase branch preview was failing on schema_migrations PK collision). Tests - Drop the swap-under-flag test from delete-last-voucher.pg.test.ts: main's bypass returns NEW unconditionally when gnubok.allow_delete='true', so the swap is permitted. Drop the duplicate happy-path test (already covered by 'clears journal_entry_id on attached documents and deletes the voucher'). Keep the unauthorized-direct-UPDATE test. - Add bankid-enrichment.pg.test.ts covering the SELECT RLS policy: user reads own row, cannot read another user's row, INSERT denied for authenticated. gnubok_query_journal - amount_min/amount_max is applied post-fetch (PostgREST can't OR abs(debit) and abs(credit) cleanly), but PostgREST's count is computed pre-filter. Reporting that as total_lines mislead agents into paginating a tail that was already filtered out. When the amount filter is applied, anchor total_lines and truncated to the filtered set and surface db_matched_pre_amount_filter + amount_filter_applied_post_fetch separately. - Escape `_` in the free-text LIKE filter so a search for "2_441" doesn't match "2X441". VAT close check - Reverse-charge blocker no longer fires on ruta 30 (seller-side domestic omvänd skattskyldighet) — the seller books no VAT, the buyer does, so missing ruta 48 is expected. Now scoped to ruta 31/32 (EU acquisition) where the buyer must book both calculated output (2615) and matching ingående moms (2645). - High-value receipt threshold no longer reads journal_entries.total_amount (column doesn't exist; check silently never fired). Sums debits across the entry's lines, which equals the gross for ordinary purchase entries — comparing a gross figure against the BFL/ML 4 000 SEK threshold per ML 17 kap 26–28 §. seed-demo-account.ts - Require an explicit email argument; refuse to run with the previously hardcoded fallback that would silently target a real user. Ensure email is non-undefined for downstream typing. - Type the supabase fiscal_periods insert result locally so tsc no longer reports 'fp implicitly any' from the loose untyped client. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): adjust fiscal-period-start-day pg test for per-company overlap The pg-real failure on PR #402 was a latent bug surfaced by this branch's fiscal_periods exclusion constraint flip from user_id to company_id (migration 20260506140100). The test was inserting periods that overlapped seedCompany's default 2026-01-01..2026-12-31 period; the previous constraint slipped past it because the test's INSERT didn't set user_id (NULL escapes the WITH = match), so two same-company overlapping periods silently coexisted. Now that the constraint correctly fires per company, pick years that don't overlap with the seeded 2026 period. The trigger's behavior under test (allow mid-month start when no earlier period exists, allow back-dated SIE imports, reject mid-month start when an earlier period exists) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(vat-close-check): correct reverse-charge/import blocker rutor Rutor 30/31/32 are the buyer's calculated utgående moms on reverse- charge purchases (domestic byggtjänster/electronics → 2614 → ruta 30; EU goods → 2624 → ruta 31; EU services → 2634 → ruta 32). The buyer must also book matching ingående moms (2647 inhemskt / 2645 utlandet → ruta 48). The previous fix removed ruta 30 on the basis that it was seller-side; that's incorrect — domestic-RC sellers book no VAT at all (they report only beskattningsunderlag on ruta 41), so 2614 only sees buyer-side entries. Restore ruta 30. Also extend the check to import rutor 60/61/62 (non-EU import VAT declared via momsdeklaration since 2015 — 2615/2625/2635). Same mechanic: importer books output VAT on these rutor and deducts the input side via ruta 48. SaaS-from-AWS / OpenAI / Vercel companies hit this path; without including 60/61/62 the blocker would silently miss their misbookings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): expose ruta 60/61/62 (import VAT) on the local VatReportResult The vat-close-check fix referenced vatReport.rutor.ruta60/61/62 but the MCP server's local VatReportResult type only carries ruta 05-49. Build broke on tsc. Extend the MCP server's slim VAT report to also project import VAT — 2615 → ruta 60 (25%), 2625 → ruta 61 (12%), 2635 → ruta 62 (6%) — and fold those into ruta 49 (att betala/återfå). Mirrors the BAS-to-Ruta mapping in lib/reports/vat-declaration.ts. Output schema and required list updated accordingly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-06 16:41:36 +02:00
Mattsson	5725c25bf1	Logs/improved logging (#398) ... * feat(mcp): add create_transactions tool with /pending approval gate New MCP tool gnubok_create_transactions stages 1–10 transactions per call as pending_operations of type create_transaction (risk: medium). Each item becomes its own card on /pending; on confirm, the executor inserts the row into transactions with import_source='mcp' so MCP-staged ingestion is distinguishable from PSD2 sync. Designed for skill workflows that pull external data (e.g., Airtable) and want the user to gate the writes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bas): strip concatenated group headers from corrupted account names A chart-data import bug had glued the next group's header onto the last account in each preceding group across all eight bas-data class files (e.g. account 2670 read "Utgående moms på försäljning inom EU, OSS 27 PERSONALENS SKATTER, AVGIFTER OCH LÖNEAVDRAG"). The corrupted names surface in transaction dropdowns, ledgers, SIE exports and årsredovisning, and risk VAT miscategorization on the OSS (2670) and blandad-verksamhet (6999) accounts specifically. - Cleans 69 account_name and 64 description fields across class-1..8 files - Adds a regression test asserting no name contains a concatenated header - Ships an idempotent safety-net migration that updates already-seeded chart_of_accounts rows, gated on the corrupted string so user customizations are preserved Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(errors): add structured error codes and handling for various operations - Introduced a new structured error registry in `structured-errors.ts` to standardize error handling across the application. - Added Swedish and English messages for various error scenarios, including validation, authorization, and bookkeeping errors. - Implemented a client-side error toast in `use-error-toast.ts` to display user-friendly error messages with remediation hints. - Created a wrapper for recording operation outcomes in `record-operation.ts`, enhancing audit capabilities for operations. - Developed a provider call wrapper in `with-provider-call.ts` to handle external HTTP calls with structured logging and error mapping. - Added a new SQL migration to extend the processing history with new event types and aggregate types for better operational telemetry. * Refactor supplier API routes to use context-based logging and error handling - Replaced direct Supabase client usage in GET and POST routes with context-based approach using `withRouteContext`. - Enhanced error handling to provide structured error responses for supplier creation and listing. - Updated logging to include request IDs for better traceability. - Introduced new error codes for supplier-related operations. - Refactored tax deadlines cron job to utilize context and improved error handling. - Updated ESLint configuration to enforce logging practices across API and lib directories. - Enhanced arcim migration extension with structured error handling and logging. - Added classification for provider errors to improve user-facing error messages. - Introduced request ID in extension context for better log correlation. * fix(route-context): update DynamicParams type for improved type safety in route handlers * feat(transactions): add 'create_transaction' operation to PendingOperationType * fix(route): ensure companyId is non-nullable in loadAndDeriveAbsence function * fix(route-context): ensure companyId is always non-null by short-circuiting with COMPANY_CONTEXT_MISSING --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-06 11:12:02 +02:00
Jakob Wennberg	94f15b9c6c	feat(invoice-inbox): pin documents to bank transactions + MCP tools (#397) ... * feat(invoice-inbox): pin documents to bank transactions + MCP tools Adds a first-class flow for attaching unmatched inbox documents to bank transactions, separate from the existing supplier-invoice convert path: - new transactions.document_id FK → document_attachments (ON DELETE SET NULL) - POST/DELETE /api/transactions/[id]/attach-document - categorize route propagates the link to journal_entry_id on commit - three new MCP tools: gnubok_list_unmatched_documents, gnubok_get_document_content (5-min signed URL), gnubok_attach_document_to_transaction (staged via pending_operations) - InvoiceInboxWorkspace gains a "Koppla till transaktion" picker dialog ranked by amount-match, plus a "Bilaga" badge in SwipeCategorizationView - regex extraction unchanged; supplier-invoice convert flow unchanged Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoice-inbox): address PR #397 review findings - categorize: destructure { error } from the document-link update so Supabase-level failures are logged instead of silently dropped (BFL 5 kap 6 § receipt-on-verifikation contract). - list_unmatched_documents: emit next_cursor whenever the inbox query may have more rows, not only when the post-filter slice was full; switch to composite (created_at, id) cursor to avoid same-second collisions. - DELETE /attach-document: return 404 when the tx isn't in the company; return 409 when the linked document already has journal_entry_id set (räkenskapsinformation immutability). - risk tier: attach_document_to_transaction medium (was low) — link becomes part of verifikation underlag once categorize propagates it. - pg-real test: stop reusing $2 across uuid + text-concat contexts (Postgres couldn't deduce the parameter type). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(migrations): break duplicate version 20260505120000 (Supabase Preview) Two migrations on main share filename version 20260505120000: - 20260505120000_api_keys_refresh_token.sql (PR #392) - 20260505120000_drop_agent_auto_commit.sql (PR #394) The schema_migrations primary key is (version), so any fresh DB doing `supabase db push` over both files conflicts on the second insert. This is why every PR with a migration since #394 has had Supabase Preview either fail or skip. Renaming _drop_agent_auto_commit to 20260505190027 — that matches the timestamp recorded in prod schema_migrations from when apply_migration was called for it, so future `db push` against prod sees the file as already- applied (no re-run). The migration body is fully idempotent (IF EXISTS on every drop) so a re-run would be a no-op anyway. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoice-inbox): address PR #397 round-2 compliance review Two BFL gaps the compliance bot flagged on the round-1 fix commit: 1. commitAttachDocumentToTransaction silently broke verifikation→underlag if the transaction was categorized between staging and approval. Now reads transactions.journal_entry_id at commit time and, if non-null, also writes document_attachments.journal_entry_id in the same commit so BFL 5 kap 6 § is satisfied regardless of order. 2. Application-layer DELETE check was racy (SELECT then UPDATE) and the FK ON DELETE SET NULL path could null transactions.document_id even for a document that is räkenskapsinformation. Added a BEFORE UPDATE OF document_id trigger on transactions that raises check_violation when the previously-attached document has document_attachments.journal_entry_id set. The app-layer guard stays for friendly Swedish messaging; the trigger is the DB-level safety net. pg-real test extended to cover both directions of the trigger (block detach + block swap) and the happy-path detach when there's no JE link yet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoice-inbox): address PR #397 round-3 compliance review Four findings from the round-2 update of the compliance bot. The first three are genuine compliance gaps; the fourth (preview metadata distinguishing pre- vs post-categorization overwrites) is a UX nicety left for follow-up. 1. transactions.document_id FK switched from ON DELETE SET NULL to RESTRICT (migration 20260506100000). Removes the "trigger ordering" concern: a doc that's pinned to any tx now cannot be deleted at all without explicit detach first. Belt-and-braces with block_document_deletion. 2. commitAttachDocumentToTransaction now does: - pre-check that mirrors the DELETE route's 409 when the existing pinned doc is räkenskapsinformation, so the same Swedish message is returned in both paths; - UPDATE…RETURNING journal_entry_id so the propagation decision uses the post-update state, closing the read-then-write race with concurrent categorize. Either ordering of attach-then-categorize or categorize-then-attach now lands at the same correct final state. 3. Both DELETE /attach-document and the MCP commit path catch the trigger's check_violation (SQLSTATE 23514) and translate to 409 with the Swedish underlag message. The trigger remains the DB-level safety net; the app layer is responsible only for friendly UX. pg-real test rewritten for ON DELETE RESTRICT (blocks deletion of pinned doc; detach-then-delete works). Unit coverage added for commitAttach: 404, two distinct 409 paths (pre-check + trigger-translation), happy-path uncategorized, and propagation when tx was categorized between staging and commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoice-inbox): address PR #397 round-4 compliance review Three of five round-3 findings actioned: 1. commitAttachDocumentToTransaction: surface propagation failure rather than logging-and-continuing. If document_attachments.journal_entry_id can't be set after the transaction has been categorized, the op fails (status 500) with a Swedish message instructing retry. Retry is idempotent — same document_id on the tx, same propagate target. 2. Replace check_violation (23514) matching with a stable "BFL_DOCUMENT_IMMUTABILITY:" message prefix. The trigger now uses default P0001 + tagged message; both the route handler and the executor match on the prefix instead of the generic SQLSTATE. Future unrelated CHECK constraints on transactions can no longer accidentally surface as the räkenskapsinformation message. 3. gnubok_list_unmatched_documents now returns invoice currency alongside amount so an agent can FX-normalise before comparing to transactions.amount. Description updated to make the requirement explicit. Mirrored in the UI: AttachToTransactionDialog ranks same-currency rows by amount distance and pushes cross-currency rows to the bottom of the list. Skipped: - Two-migration window for FK action change is acknowledged as resolved by the bot; deploy-atomicity is an ops concern, not code. - Period-lock check in attach/detach: realistic compliance concern is already covered by the existing immutability trigger (post-categorize) and by the engine's period-lock enforcement (categorize itself). A dedicated period check on pre-categorize attach would only guard against pinning a doc to a tx in a closed period — defensible defense-in-depth, but no active BFL violation. Left for a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoice-inbox): address PR #397 round-5 compliance review Three of four findings actioned. The fourth (block_document_deletion verification) is already covered by 20240101000017_enforcement_triggers.sql which raises when document_attachments.journal_entry_id IS NOT NULL on a posted/reversed entry — confirmed via grep, no code change needed. 1. categorize/route.ts: propagation no longer fires-and-forgets. If document_attachments.journal_entry_id can't be set after the JE has been committed, the response now carries a document_link_warning field with a Swedish retry message. The JE is already committed so we can't roll back, but the client can no longer mistake a partial attach for a clean categorize. 2. Rättelse audit trail (BFL 5 kap 5 §): both the REST POST handler and the MCP commit executor now append a TransactionDocumentReplaced event to processing_history whenever a non-null document_id is overwritten, with previous_document_id and new_document_id in the payload. Best-effort — logging failure must not roll back the (compliant) attach. The previous doc id is also returned in the response so callers see what was displaced. 3. MCP staging preview now exposes the existing doc's identity (existing_document_id, existing_document_file_name) plus an explicit existing_document_is_rakenskapsinformation flag, so a human approver sees "replaces X.pdf with Y.pdf" rather than just a will_overwrite_existing boolean. Mirrors BFL 5 kap 5 § informed-rättelse intent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoice-inbox): close trigger SELECT race (PR #397 round-6) The enforce_transactions_document_immutability trigger SELECTed document_attachments.journal_entry_id without a row lock. A concurrent UPDATE setting journal_entry_id on that row could commit between the trigger's SELECT and its RAISE, letting a detach slip through against a document that just became räkenskapsinformation. Add FOR SHARE to the SELECT inside the trigger. A concurrent journal_entry_id write blocks on our share lock until our transaction commits, so either we observe the propagation and raise, or we run first and the propagation observes our committed detach (which is fine because journal_entry_id was still null at that point). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoice-inbox): bidirectional immutability + richer staging preview (PR #397 round-7) Two of six round-6 findings actioned. The other four are recurring architectural recommendations (atomic audit-log writes, background reconciliation jobs, migration consolidation, anti-join via materialized view) that are properly scoped as follow-up work. 1. document_attachments side of the immutability link (BFL 5 kap 6 § works in both directions). New trigger enforce_document_journal_entry_immutability blocks UPDATE OF journal_entry_id when going from non-null to NULL or to a different uuid. The original null→uuid path (initial propagation in the categorize / commitAttach flows) still works. Migration 20260506130000. 2. gnubok_attach_document_to_transaction staging preview now joins on invoice_inbox_items.extracted_data and surfaces vendor/amount/currency/ invoice_date alongside the existing doc filename/mime metadata. Gives the human approver the same hints the agent saw before choosing the attachment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-06 10:24:57 +02:00
Jakob Wennberg	5c52f24a49	feat(mcp): agent-native improvements — progressive discovery, widgets, skills, telemetry (#393) ... * feat(mcp): agent-native improvements — progressive discovery, widgets, skills, telemetry Four coordinated streams of MCP server improvements that move gnubok toward agent-first design, grounded in Anthropic's Nov 2025 "Code execution with MCP" article and the May 2026 MCP conference talk. Context budget — minimize tools/list payload - New gnubok_search_tools: progressive discovery with name|summary|full detail levels and scope filtering. Agents pull only the schemas they need. - Trimmed all 50 tool descriptions from multi-paragraph blocks (avg ~500-1000 chars) to one-sentence summaries (avg ~120 chars). Args/Returns/Examples blocks dropped — they duplicated inputSchema. - outputSchema declared on every tool; structuredContent emitted on every successful tools/call (was previously only widget-tagged tools). - protocolVersion bumped to 2025-06-18 (negotiates back to 2024-11-05). - Workflow examples consolidated into initialize.instructions. - Net effect: tools/list payload ~43 KB / ~10.8K tokens for 51+ tools, with headroom guard at 20K tokens. MCP applications — server-shipped UI widgets - New widgets/ directory with typed UiWidget contract; receipt-matcher moved out of widget-html.ts (which was deleted) into widgets/receipt-matcher.ts. - New gnubok_vat_review_widget tool + interactive momsdeklaration widget (all 8 rutor with summary card, theme-aware light/dark, copy buttons). - resources/list and resources/read iterate uiWidgets dynamically — adding the next widget is a single file drop. Skills over MCP — domain-knowledge primitive - 5 user-facing SKILL.md-style workflow guides authored from existing .claude/skills/swedish-* development skills: • month-end-close — book → reconcile → VAT (monthly filers) → lock • quarterly-vat-review — ruta-by-ruta map, deadlines, common errors • year-end-close — bokslut, bokslutstransaktioner, lock → year-end → opening balances → close (irreversible) • invoicing-rules — ML 17 kap. 24 §, customer types, ROT/RUT, Peppol • payroll-monthly — salary run → calculate → review → AGI XML - gnubok_list_skills (with optional tag filter) + gnubok_load_skill(slug). Both unscoped — available to any authenticated key. - Each skill also exposed as MCP resource at gnubok://skill/<slug> (text/markdown) for forward compatibility with a future native skills/list primitive. Tool-call telemetry — measure before optimizing further - Three new CoreEvent types (mcp.tool_called, mcp.tools_list_called, mcp.resource_read), all persisted to event_log (30-day TTL, RLS-scoped). - Fire-and-forget emission from the dispatcher — never blocks JSON-RPC response, double-guarded against handler failures. - tools/call instrumented at all four exit points (success, execution error, scope denied, unknown tool). Latency measured tightly around tool.execute() — excludes dispatcher overhead. - tools/list logs returned tool count (informs progressive-discovery adoption); resources/read logs URI + kind discriminator (widget / skill / data / unknown). - No PII or secret material in payloads — only metadata. Out of scope (explicitly deferred): - Code-mode SDK (no production code-mode hosts to consume it yet). - Elicitations (require streamable HTTP transport — bigger architectural lift). - DB lockdown / RPC funnel (foundational; should follow once telemetry tells us where writes actually flow). - CRUD → intent endpoints (frontend coupling — multi-PR effort). Tests: +37 new unit tests across search-tools, output-schema, payload-size, vat-review-widget, skills, telemetry. Existing receipt-matcher test updated for the new structuredContent contract. 2,615 unit tests passing. Production build green. No new lint warnings or errors in changed files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): address PR #393 review findings Greptile (P1 + P2) and the Swedish accounting compliance bot flagged 9 issues across security, data correctness, and skill content. All addressed: Security (P1) - gnubok_search_tools: scope filter now fails closed when __keyScopes is absent. The earlier permissive default leaked the full tool inventory if the dispatcher's hard-coded name check ever silently broke. Marker presence is part of the contract — explicitly empty array also hides scoped tools. Two new test cases pin the fail-closed behaviour. Compliance — VAT (data correctness) - get_vat_report now aggregates 2614/2624/2634 (reverse-charge output VAT) and exposes them as ruta30/ruta31/ruta32 per SKV 4700. ruta48 also picks up 2647 (missing before). ruta49 formula corrected to (10+11+12+30+31+32) − 48. The widget renders the new rutor between the Utgående and Ingående sections. - Widget ruta 05 sub-label updated from "3001+3002+3003" to "all momspliktig försäljning oavsett skattesats" — ruta 05 covers all domestic taxable supplies, not just direct-rate sales. - Refactored: extracted computeVatReport() helper used by both gnubok_get_vat_report and gnubok_vat_review_widget. Removes the rename-fragile tools.find() lookup at runtime. Compliance — payroll - payroll-monthly skill: replaced "born 1958 or earlier = 10.21%" (the 2024 formulation) with the statutory rule "age 66+ on 1 January of the income year (67+ from income year 2026)". Removed the unsourced "age 16–18: 11.78%" row in favour of a current växa-stöd description with explicit Prop. 2025/26:34 reference and a "verify against current Skatteverket tables" caveat. Compliance — skills text - invoicing-rules: added explicit footnote on the 1 April 2026 livsmedel rate change. Restaurang/servering stays at 12 %; livsmedel sold in other forms drops to 6 %. Per Prop. 2025/26:55. - year-end-close: clarified periodiseringsfond cap as "25 % of överskott before this year's avsättning" (IL 30 kap.), removing the ambiguous "skattemässigt resultat" phrasing that could be misread as a circular after-fond computation. Schema correctness - STAGED_OPERATION_SCHEMA.required gains "staged" — every path through stagePendingOperation returns the field, so the schema now matches the contract that MCP clients validate against. Tests: 2,617 passing (+2 for the search-tools fail-closed cases). Production build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): address PR #393 round-2 compliance review Round 2 of the swedish-compliance bot ran after the previous fix-push and flagged four substantive items + a recommendation. All addressed: VAT computation (data correctness) - ruta05 expanded beyond 3001/3002/3003 to cover the common BAS taxable- revenue accounts (3001-3008, 3041-3048, 3051-3058, 3071-3078). Companies that book to 30xx alternates were previously under-reporting taxable turnover; now all standard BAS taxable-revenue numbering contributes. - One-sided reverse-charge warning: when output VAT is booked on 2614/2624/2634 (rutor 30/31/32 > 0) but the matching calculated input VAT (2645) is zero, computeVatReport now returns a Swedish-language warning string. ruta49 is inflated in this case — the warning surfaces the most common reverse-charge error per the swedish-vat skill. The widget renders warnings in a terracotta panel above the summary card. - computeVatReport exported and a focused unit test added — exercises 2647 inclusion in ruta48, reverse-charge balanced/unbalanced cases, and the expanded ruta05 mapping. Fills the gap that prior tools/call integration tests couldn't reach. Skill content - payroll-monthly Step 6: BAS journal-entry example no longer hard-codes 31.42 % on the 7510/2730 lines. The avgift line is now described as "avgift_base × applicable_rate per employee" with explicit aggregation semantics for runs that mix full-rate and reduced-rate employees. Aligns with the Step-4 reduced-rate caveats already in place. - invoicing-rules ROT/RUT block: replaced the bare "30 % / max 50 000 SEK" text with the full year-by-year picture — RUT 50 % / 75 000 max, ROT baseline 30 % / 50 000 max, 2024 H2 doubled ceiling, 2025 May–Dec enhanced 50 % rate. Defaults to "verify against current Skatteverket table" rather than a single hard-coded rate. False positives in the round-2 review (no fix needed; documented for the record): - "Old vat_report path still computes ruta48 without 2647" — the old path was replaced by computeVatReport in the previous push; the bot was reading the diff hunk and conflating it with current behaviour. - "Widget prose says ruta49 = (10+11+12) - 48" — no such prose exists in widgets/vat-review.ts. The skill body has the correct (10+11+12+30+31+32)-48 formula. - "Including 'reversed' status entries in VAT aggregation may over-count cross-period storno" — current behaviour is correct per Skatteverket period-aligned filing: the reversed original stays in its own period, the matching storno (status 'posted') lands in the reversal period, and they net to zero across the full year. Adding code comment to document. Tests: 2,623 passing (+6 for computeVatReport unit tests). Production build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): address PR #393 round-3 compliance review Compliance bot re-ran after the round-2 push and flagged three items: VAT computation - One-sided reverse-charge warning previously only checked 2645 (EU acquisitions). For domestic reverse charge per ML 16:13 (byggtjänster, electronics > 100k SEK, etc.) the matching input lands on 2647 — a correctly-balanced 2614+2647 booking would have falsely fired the warning. Fixed: warning now triggers only when *both* 2645 and 2647 are zero. Updated message text mentions both accounts. Added a test case asserting the no-warning path for 2647-only-input. Skill documentation drift - quarterly-vat-review skill body still showed ruta05 source as "3001 + 3002 + 3003" while the runtime computeVatReport sums 32 BAS taxable-revenue accounts. Updated the skill table to read "3001–3008, 3041–3048, 3051–3058, 3071–3078" so the auditor-facing docs match the implementation. outputSchema upgrade - gnubok_get_vat_report and gnubok_vat_review_widget previously declared outputSchema as the bare { type: 'object' }. Replaced with a shared VAT_REPORT_OUTPUT_SCHEMA constant declaring period, period_label, all 11 rutor (with descriptions referencing source accounts), summary, and warnings. Modern MCP clients that validate structuredContent against outputSchema now have an accurate contract. Added a test asserting the schema is non-trivial and declares every ruta the runtime returns. Tests: 2,625 passing (+2 for the 2647 warning path and the outputSchema shape assertion). Production build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): address PR #393 round-4 compliance review The compliance bot re-ran after the round-3 push with a fresh batch. Real findings fixed; false positives documented. VAT computation - Removed 3004 (Försäljning inom Sverige, momsfri / VAT-exempt) from RUTA_05_ACCOUNTS — round-2's expansion accidentally included it. Ruta 05 is the *taxable* base; exempt sales must NOT contribute. New test pins the exclusion. - Added 3106 (taxable EU goods supply, momspliktig) to RUTA_05_ACCOUNTS. Used when EU buyer's VAT number is invalid or buyer is private. - Added ruta35 — EU intra-community goods supplies, momsfri (account 3108). Previously omitted entirely from the rutor schema; SKV 4700 has it as a distinct box separate from ruta 39 (services) and ruta 40 (export outside EU). VatReportResult, VAT_REPORT_OUTPUT_SCHEMA, the widget table, the copy-summary block, and the quarterly-vat-review skill table all updated. New test covers 3108 → ruta35 mapping. - Strengthened the comment on the posted+reversed status filter to document why current behavior is correct per ML 2023:200 and faktureringsmetoden (the bot's cross-period storno concern is a false positive — see commit message rationale below). Skill content - invoicing-rules: added explicit BFL 5 kap. 6–7 § / ML 17 kap. 22–23 § note that the kreditfaktura itself consumes a sequential number from the same (or dedicated KR-) fakturaserie. The KR- prefix is a display convention; the underlying löpnummer must be unbroken just like the regular series. - payroll-monthly: added the missing "born 1937 or earlier → 0 %" cohort to the rate breakdown. Previously could lead a payroll run to over-pay avgifter on the oldest cohort. False positives in the round-4 review (verified, not changed) - 2644/2648 as reverse-charge inputs: verified against gnubok's actual BAS chart (lib/bookkeeping/bas-data/class-2-equity-liabilities.ts). 2644 does not exist; 2648 is "Vilande ingående moms" (dormant input VAT for cash method), not RC at 6 %. Canonical RC inputs are 2645 (EU) and 2647 (domestic) — both already covered. - Cross-period storno over-count: per ML 2023:200 + Skatteverket faktureringsmetoden, the original sale's VAT belongs to the invoice- date period; the kreditfaktura's reduction belongs to the storno-date period. Including 'reversed' status entries (which still have their original date) is therefore correct. *Excluding* them would under-report the original period and over-credit the reversal period. Added a multi-line comment in computeVatReport documenting this. Tests: 2,627 passing (+2 for ruta35 mapping and 3004 exclusion). Production build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 20:04:10 +02:00
Mattsson	c03582b5c7	Fix/percistent mcp connection (#392) ... * feat(oauth): add support for refresh tokens in OAuth flow and update database schema * feat(prompts): add MCP prompts and corresponding functionality for prompt retrieval * feat(auth): enhance error handling for refresh token operations and validation	2026-05-05 13:48:53 +02:00
Mattsson	bb855d2ddc	Add/ai native supp (#385) ... * feat(branding): implement dynamic branding in service worker and reports * feat(auth): enhance API key scopes and add bookkeeping write scope - Updated transaction write scope description to include additional tools. - Enhanced reports read scope description to reflect new functionality. - Introduced bookkeeping write scope with relevant description. - Updated SCOPE_GROUPS to include bookkeeping domain. - Modified TOOL_SCOPE_MAP to include new bookkeeping operations. - Updated validateApiKey function to return api_key_id and api_key_name for better actor attribution. feat(tests): add unit tests for MCP resource registry - Created tests for data resources to ensure all required fields are present. - Added tests for resource query parsing and retrieval. feat(resources): implement MCP resources for company and accounting data - Added capabilities resource to expose API key capabilities based on granted scopes. - Implemented chart of accounts resource to retrieve active BAS chart. - Created company current resource to fetch active company details. - Developed active fiscal period resource to check posting eligibility. - Implemented recent activity resource to fetch latest journal entries, invoices, and transactions. - Added VAT treatments resource to provide available VAT rates per customer type. feat(pending-operations): introduce risk tiers for operations - Added risk level classification for pending operations to determine auto-commit eligibility. - Implemented functions to classify operation risk levels and identify high-risk operations. feat(migrations): add actor model and risk tier to pending operations - Updated pending_operations table to include actor type and risk level columns. - Enhanced audit_log to mirror actor information for compliance. - Modified validate_and_increment_api_key function to return actor details. - Expanded operation types in pending_operations to include new high-risk operations. * feat: add auto-commit functionality for low-risk pending operations - Implemented shouldAutoCommit function to determine eligibility for auto-commit based on operation type, actor type, and company settings. - Created commitPendingOperation function to handle execution of pending operations with consistent status updates. - Added tests for shouldAutoCommit to cover various scenarios including high-risk operations, user actors, company opt-in status, and monetary thresholds. - Introduced new columns in company_settings for agent_auto_commit_enabled and agent_auto_commit_max_amount to allow companies to opt-in for auto-commit functionality. - Added SQL migration to update the database schema for new auto-commit settings. * feat(idempotency): implement idempotency key handling for safe retries and cleanup * feat: expand API key scopes and pending operations for bookkeeping - Added 'suppliers:write' scope to API key scopes for supplier invoice management. - Updated SCOPE_GROUPS to include the new 'suppliers:write' scope. - Introduced new pending operation types for bookkeeping: close_period, lock_period, run_year_end, set_opening_balances, run_currency_revaluation, explain_voucher_gap, uncategorize_transaction, approve_supplier_invoice, credit_supplier_invoice, and convert_invoice. - Implemented corresponding commit functions for the new operations in the pending operations module. - Enhanced PendingOperation type to include actor model and risk level attributes. - Added tests for new functionality, ensuring proper behavior and constraints in the database. * feat: implement unlockPeriod functionality and related tests * feat: add agent auto-commit settings and related functionality * feat: add attention resource with comprehensive summary of outstanding tasks * feat: enhance pending operations with 'committing' status and immutability checks, improve idempotency handling, and add original voucher reference for credit notes	2026-05-04 11:12:29 +02:00
Mattsson	04dbb31d7e	Salary module (#245) ... * feat: implement salary module with personnummer encryption, salary entries, tax tables, and AGI tracking - Added personnummer encryption and decryption functions for secure storage. - Created salary entries handling for journal entries including gross salary, tax withholding, and employer contributions. - Implemented tax table lookup functionality for calculating tax amounts based on monthly income. - Developed SQL migration for salary module including tables for payroll configuration, tax rates, employees, salary runs, salary run employees, salary line items, and AGI declarations. - Established row-level security policies for all new tables to ensure company-scoped access. * feat: add salary calculation modules for 2026 - Implemented engångsskatt calculation for one-time payments with tax brackets. - Added löneväxling functionality for salary sacrifice to pension, including employer savings and warnings. - Created pain.001 generator for salary batch payments in compliance with Swedish banking standards. - Developed PDF template for payslips, including detailed breakdowns and employer costs. - Generated seed data for Swedish tax tables for 2026, including SQL insert statements. - Implemented traktamente calculations for per diem and mileage allowances, adhering to Skatteverket regulations. - Added seed script for populating tax tables in the database. * feat: Update meal reduction percentages in traktamente calculation fix: Remove obsolete seed script for 2026 tax tables feat: Extend SalaryRunStatus type to include 'corrected' status feat: Implement KU10 XML generation endpoint for annual employee income statements feat: Add endpoint for creating corrections to booked salary runs feat: Implement endpoint for sending payslip PDFs to employees feat: Create KU10 XML generator for annual reporting feat: Add salary transaction matcher for auto-linking bank transactions to salary entries chore: Add database migration for salary correction support * feat: replace select elements with custom Select component for employment and salary types * feat: enhance salary calculations with pension entry and avgifter category support	2026-04-15 11:17:39 +02:00
Jakob Wennberg	b387a77bfd	chore: remove Sentry, consolidate migrations, add test coverage (#244) ... * chore: remove Sentry, consolidate migrations, add test coverage Remove @sentry/nextjs and all Sentry integration code — error tracking now handled by Recapt. Consolidate 22 incremental migrations into a single schema sync migration. Add 6 new test suites (auth, invoice matching, VAT rules, opening balances) and extend report tests with edge cases. Update Docker image name to gnubok, sync crontabs and extension presets, fix CSP missing space, simplify journal entry missing-document dialog. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove viewer bank import migration never applied to production 20260413150000_viewer_bank_import_permissions.sql (PR #234) was merged to main but never applied to the production database. It references current_active_company_id() which does not exist in production either. This breaks fresh installs and Supabase preview branches because the migration runs before the consolidated schema sync. Remove it so the migration chain matches production. The viewer bank import RLS policies should be re-added in a future migration alongside the helper functions they depend on. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: correct delete policies for tables without company_id column Seven tables in the generic delete-policy loop don't have a direct company_id column, causing fresh installs to fail with "column company_id does not exist". Fix by moving them out of the loop: - invoice_items, journal_entry_lines, receipt_line_items, supplier_invoice_items → join through parent table - extension_toggles, notification_settings, push_subscriptions → user-scoped (auth.uid() = user_id) All policies match their existing production definitions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-15 10:52:00 +02:00
Mattsson	cd376e1cad	feat: implement viewer role permissions for bank transaction imports and connections (#234)	2026-04-13 19:43:28 +02:00
Mattsson	a1a816b4a5	Delete features (#218) ... * Implement company and account deletion features - Add event types for company and account deletion to CoreEvent. - Enhance Supabase middleware to handle company context resolution and cookie management for archived companies. - Create API routes for deleting accounts and companies, including necessary validations and event emissions. - Implement tests for account and company deletion endpoints to ensure proper functionality and error handling. - Add retention notice component to inform users about bookkeeping data retention during destructive actions. - Create database migrations to support soft deletion of companies and anonymization of user accounts, ensuring compliance with retention laws. * feat: enhance account deletion process and update user notifications * Add service client for onboarding completion check and update escape hatch visibility * Enhance invite flow and email handling for company members * Refactor company context and RLS policies for active company isolation - Update `switchCompany` to remove unnecessary revalidation as client handles navigation. - Revise `getActiveCompanyId` to prioritize `user_preferences` and validate against non-archived memberships. - Modify `setActiveCompany` to ensure `user_preferences` is the authoritative source while maintaining cookie compatibility. - Enhance middleware to resolve active company using `user_preferences` and fallback to first non-archived membership. - Introduce new API route `/api/company/current` to fetch the active company ID for cross-tab synchronization. - Implement `CompanyTabSync` component for real-time active company enforcement across tabs. - Create migration for RLS policies to enforce single-active-company isolation using `current_active_company_id()`. * feat: implement viewer role enforcement for write permissions - Added `useCanWrite` hook to determine if the current user has write permissions based on their role in the active company. - Updated various components (JournalEntryForm, CustomerForm, DeadlineForm, etc.) to disable write actions and show a lock icon with a tooltip for users without write permissions. - Introduced `requireWritePermission` function to enforce write permissions at the API level, returning a 403 response for viewers. - Created tests to verify the behavior of the viewer role and write permissions. - Added database migration to enforce read-only access for viewers at the database level.	2026-04-11 17:06:32 +02:00
Jakob Wennberg	6d75b9a1bf	feat: BankID authentication via TIC Identity API (#192) ... * feat: add BankID authentication via TIC Identity API Integrate BankID as a login/signup method using the TIC Identity API. Users can authenticate with BankID QR codes (desktop) or deep links (mobile), link BankID to existing accounts, and skip TOTP MFA when BankID is linked. Removes Step 0 (role choice) from onboarding for all users. Adds enrichment data support for pre-filling company details from Bolagsverket during signup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR review — server-side rate limit, unlink clears MFA bypass - Add per-IP rate limit (5s cooldown) on /bankid/start to prevent unbounded billable TIC sessions from unauthenticated callers - Add /bankid/unlink endpoint that deletes bankid_identities AND clears app_metadata.bankid_linked so MFA enforcement resumes after unlink - Update BankIdSettings to call server-side unlink instead of client-side delete Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: move rate limiter to module scope, add BankID logo and year-end skill Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-08 13:44:14 +02:00
Jakob Wennberg	c4a6d16e94	feat: document inbox + fix MCP pending operations user_id (#172) ... * fix: always use business PSU type for bank connections EF (sole trader) users connecting to Nordea got personal accounts because psu_type was set to 'personal' based on entity_type. Since gnubok is accounting software, all bank connections should use 'business' PSU type regardless of entity type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: document inbox extension with AI classification Add invoice-inbox extension for email-based document processing with AI-powered classification, supplier matching, and inbox management. Includes MCP tools for document upload/listing, migration, and supporting changes across document service, API keys, and banking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: disable invoice-inbox extension, use dynamic import in MCP server Keep invoice-inbox out of extensions.config.json until ready for production. MCP server now dynamically imports classifyDocument to avoid breaking when the extension is disabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: correct file size error message in invoice-inbox upload Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-03 16:45:17 +02:00
Mattsson	0dd1f5ebc1	feat: multi-tenant company refactor (GNU-19) (#153) ... * feat: multi-tenant company refactor (GNU-19) Introduce companies table, company_members, and user_preferences to support multiple companies per user. All data scoping changes from user_id to company_id across the entire codebase. Key changes: - Database migration: new tables, company_id on 40+ tables, backfill, RLS rewrite from user_id to company-member-based, updated RPCs - Types: Company, CompanyMember, CompanyRole, UserPreferences types; company_id added to all entity interfaces; companyId on all events - Engine: all 7 core functions take companyId; storno, period, year-end services updated; 16 report generators updated - Middleware: company context resolution (cookie → prefs → first company) - API routes: ~120 routes updated with requireCompanyId() - Frontend: CompanyProvider context, layout/dashboard/onboarding updated - Extensions: context factory, 9 extensions, all lib files updated - Tests: 1880 tests passing, all helpers updated with company_id defaults Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add database migrations for multi-tenant company and team system (GNU-19) Adds company_invitations, company creation RPC, team_members, account deletion RPC, and teams table refactor migrations. Updates base multi-tenant migration with cascading FKs and onboarding_step column. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add team types and update core infrastructure for multi-tenancy (GNU-19) Adds TeamRole, MemberSource, and Team types. Refactors Supabase service client to be stateless, updates middleware for team-aware routing, extends CompanyContext with team/role fields, and updates extension service types to accept companyId. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: thread company_id through business logic functions (GNU-19) Replaces user_id scoping with company_id across all lib modules: bookkeeping, documents, transactions, invoices, reconciliation, tax, deadlines, and import. Updates corresponding tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: thread company_id through API routes and extensions (GNU-19) Updates all existing API routes to extract and pass companyId. Updates enable-banking and arcim-migration extensions for company-scoped transaction ingestion and sync. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add company and team management API routes (GNU-19) Adds CRUD endpoints for company members, company invitations, team members, and team invitations. Includes invite token utilities, email templates, and company switch server action. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add team/company UI components, pages, and dashboard updates (GNU-19) Adds CompanySwitcher, ConsultantEmptyState, Step0RoleChoice, company members and team management panels. Updates dashboard layout for team-aware routing, onboarding for multi-step role choice, and auth callback for team invite acceptance. Ignores supabase/.branches/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add null guards for company in import page (GNU-19) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: move appUrl declaration to outer scope in invite route (GNU-19) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add optional chaining for company.name in members section (GNU-19) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add optional chaining for second company.name in members section (GNU-19) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add null guards for company in extension components (GNU-19) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: pass companyId to executeSIEImport in arcim-migration extension (GNU-19) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: update tests to use companyId instead of userId and improve type handling --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-31 16:41:52 +02:00
Mattsson	550cadcb06	fix: harden auth, cron secrets, and provider flows (GNU-17) (#148) ... - Replace === with crypto.timingSafeEqual in all 7 cron routes via shared lib/auth/cron.ts - Add in-memory rate limiting (60 req/min) and expires_at support to calendar feed - Add exponential backoff on MFA verify after 3 failed attempts - Add 60s cooldown on password reset requests - Validate bank callback auth code format before API call - Redact session IDs from bank sync and callback logs - Validate OAuth redirect_uris against allowlist (claude.ai, claude.com, localhost) - Remove excessive PII/debug console logging from login page Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-29 23:49:09 +02:00
Mattsson	bb473e2c57	Worktree api key scopes (#139) ... * feat: add read/write scopes to API keys API keys now require explicit scopes (e.g. transactions:read, invoices:write) instead of having implicit full access. The create dialog shows grouped checkboxes per domain with read/write split. Legacy keys with null scopes default to read-only. MCP tools/list is filtered by scope and tools/call rejects unauthorized calls. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Enhance API key scopes with suppliers and update descriptions for better clarity * fix: drop function before recreating with changed return type PostgreSQL cannot change return type via CREATE OR REPLACE. Drop the existing function first to avoid SQLSTATE 42P13. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add new migration to drop and recreate function with scopes return type The original migration was already applied, so a new migration is needed to DROP the function first before recreating with the updated return type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add DROP FUNCTION to original migration, remove redundant fix migration Preview branches replay all migrations from scratch. The original migration must DROP the function before recreating it with a changed return type, otherwise PostgreSQL rejects the CREATE OR REPLACE. The separate fix migration is no longer needed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: rename migration to avoid duplicate version in schema_migrations Version 20260325120000 is already recorded in the preview DB from a prior failed apply. Renaming to 20260326130000 so Supabase treats it as a new migration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-26 13:30:26 +01:00
Jakob Wennberg	d61670231f	fix: plain 401 for MCP OAuth discovery (#74) ... * fix: return plain 401 for MCP OAuth discovery and harden auth flow - Return plain HTTP 401 with WWW-Authenticate header (no JSON-RPC body) so Claude Desktop's MCP client can trigger OAuth discovery correctly - Remove unused apiKey import from authorize route - Remove stale codeChallengeMethod parameter from oauth-codes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add Retry-After header to 429 rate limit response Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-21 14:47:47 +01:00
Jakob Wennberg	5d66dd6bfc	feat: MCP server, API keys, OAuth, and KPI dashboard (#72) ... * fix: prevent Chrome auto-translate from crashing React during onboarding Chrome auto-translate modifies DOM text nodes when it detects a Swedish page (lang="sv") in a browser set to English. React does not expect external DOM mutations and throws, crashing the entire component tree into global-error.tsx on every step transition. Add translate="no" and <meta name="google" content="notranslate"> to suppress browser translation. Also fix timezone-unsafe date parsing in fiscal period validation (new Date("YYYY-MM-DD") + getDate() returns local-timezone values, shifting dates by -1 day in Western timezones). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add notranslate meta tag to global-error.tsx for consistency Per review feedback — global-error.tsx renders its own <html> document, so it needs the same <meta name="google" content="notranslate"> tag as layout.tsx to fully suppress Chrome translation on error pages. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add MCP server extension with OAuth, API keys, and KPI dashboard Let users do bookkeeping through Claude Desktop, Claude Code, or any MCP-compatible client. "Show my uncategorized transactions." "Book that as office supplies." "Invoice Acme for 15,000 kr." MCP server (extension): - 10 tools: transactions, categorization, customers, invoices, trial balance, VAT report, KPI report, income statement - JSON-RPC 2.0 protocol (no SDK dependency, works in serverless) - Tool annotations, pagination, input validation per MCP best practices - Same engine as web UI (VAT rules, exchange rates, event emission) API key infrastructure (core): - api_keys table with RLS, rate limiting (100 RPM), scopes column - Atomic rate limit via DB RPC (validate_and_increment_api_key) - Key management API routes + settings UI panel OAuth 2.1 for Claude Desktop connectors: - .well-known/oauth-protected-resource + oauth-authorization-server - Authorization endpoint with consent page - Token endpoint with PKCE verification - Stateless encrypted auth codes (AES-256-GCM, no DB storage) - Dynamic client registration KPI dashboard: - /nyckeltal page with hero cards, operational grid, trend chart - GET /api/reports/kpi endpoint - Gross margin, cash position, expense ratio, avg payment days, VAT liability, revenue/expense trend Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address OAuth security vulnerabilities from code review Critical fixes: - Auth code replay: Track used codes in oauth_used_codes table with unique constraint. Codes are single-use per OAuth 2.1 §4.1.2. - Open redirect: Validate redirect_uri against hardcoded allowlist of known Claude callback URLs + localhost for dev. P1 fixes: - Move API key creation from /authorize to /token endpoint. Keys are only created after PKCE verification, preventing orphaned keys on abandoned OAuth flows. - Add ensureInitialized() to MCP server so event handlers load and transaction.categorized events reach extensions. P2 fixes: - Remove 'plain' from PKCE methods — only S256 is advertised and accepted. - Fix extension count in sectors test (10 → 11 for mcp-server). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove duplicate ensureInitialized() that caused circular import The extension router (ext/[...path]/route.ts) already calls ensureInitialized() before dispatching to handlers. The duplicate call in server.ts created a circular import that Turbopack couldn't resolve, breaking the Vercel build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-21 12:57:14 +01:00
Jakob Wennberg	928a145f9a	feat: upgrade auth to email+password with optional TOTP MFA ... - Replace magic-link-only login with email+password (primary) and magic link (toggle) - Add registration page with strong password validation - Add MFA enrollment (/mfa/enroll) with QR code and manual secret - Add MFA verification (/mfa/verify) with 6-digit TOTP input - Add password reset flow (/reset-password) - Add middleware MFA enforcement gated by NEXT_PUBLIC_REQUIRE_MFA env var - Self-hosted deployments (NEXT_PUBLIC_SELF_HOSTED=true) skip MFA entirely - Add Security tab in Settings for password change and MFA management - Add requireAuth() API route helper with MFA check - Update CLAUDE.md with Authentication section and env var docs - Update Dockerfile and docker-entrypoint.sh for new env var placeholders Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-08 10:17:49 +01:00