fix: similar-sweep currency remediation, security hardening and v1 API fixes (#1215)
* fix(security): gate replace_sie_import behind owner/admin membership The RPC was SECURITY DEFINER with EXECUTE granted to PUBLIC and anon, no company_members lookup, no auth.uid() reference and no unauthorized raise, while setting gnubok.allow_delete to disarm the BFL immutability and retention triggers. Any caller holding a company_id and an import id could hard delete another tenant's verifikationer. Confirmed live in production. Applies the same fail closed owner/admin guard that undo_sie_import already carries (migration 20260624120000), resolving the actor from COALESCE(p_user_id, auth.uid()) so it denies when the role is NULL, then revokes EXECUTE from PUBLIC and anon. search_path and the raised statement_timeout are restated, since CREATE OR REPLACE drops settings that are not repeated. userId is a required parameter on replaceSIEImport: the service client has a NULL auth.uid(), so a caller without an explicit actor now fails to compile rather than hitting the closed gate at runtime. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): validate arcim OAuth callback state server side The callback route is skipAuth and decoded the state parameter as plain base64url JSON, trusting consentId and provider from it. A one time code was minted at flow start and never read. An unauthenticated attacker who learned a consent id could run an OAuth flow on their own provider account and post the callback with a forged state, landing their tokens on another tenant's consent, so the victim's next migration imported the attacker's ledger. State is now an opaque randomBytes(32) pointer to a provider_otc row, consumed by a single atomic UPDATE guarded on used_at IS NULL and expires_at, so a replay loses the row lock race and updates nothing. provider is read from provider_consents rather than trusted from the client. provider_otc already existed for exactly this purpose and was never wired up. Also scopes getConsent to an owning company, closing a cross tenant status oracle where the preview and migrate paths echoed a consent's status before the scoped check ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): scope documents storage to company_id (phase A) The documents bucket policies matched on auth.uid(), and upload keys were documents/{userId}/..., so company membership was never consulted. Removing a member revoked nothing: their session still authenticated and they kept direct Storage read access to every receipt, supplier invoice and bank statement they had uploaded. The same bug was fixed for sie-files in 20260416120000; this bucket was left behind. Phase A is additive. Company scoped policies are added alongside the uploader scoped ones, uploads move to documents/{companyId}/{userId}/..., and reads accept either layout so nothing breaks mid migration. Phase C, which drops the old policies, is gated on the backfill reporting zero remaining legacy prefix objects. The policy compares the company segment as text rather than casting to uuid the way sie-files does: this bucket holds keys whose second segment is not a uuid (MCP audit packages), and Postgres does not guarantee the bucket prefix qual runs before the cast, so a planner reordering would raise 22P02 and fail the whole query instead of filtering the row out. deleteDocument now removes both candidate keys. Removing only the stored pointer would leave a readable orphan copy of a document the user asked to erase. The backfill script is included but has never been run. It defaults to dry run, refuses .env.local by name, and verifies each copy is readable and SHA-256 identical before repointing the row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): enforce events:read scope and membership on /api/events This was the only one of the three validateApiKey call sites with no downstream guard: v1 and the MCP server both check scope and re-verify company membership, this route did neither. An events:read scope existed and was documented as gating the endpoint but was never called, so a legacy key falling back to DEFAULT_SCOPES read the full log. The bound company id went straight from the api_keys row into a service role query, so a key whose user had been removed from the company kept reading. Adds the scope check before any database access, re-verifies company_members with archived_at IS NULL, honours test mode by stamping X-Gnubok-Mode instead of ignoring it, applies minimisePayload so the pull surface can never return a wider payload than the push surface, and replaces the three flat error strings with the canonical envelope. Test key reads are served rather than blocked: TEST_KEY_WRITE_BLOCKED is gated on mutations in with-api-v1, so a read gets the same treatment as every other v1 read endpoint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(bookkeeping): sweep remaining journal_entries!inner embeds A previous refactor removed this pattern from lib/reports and introduced fetchEntryLines, but the class was never swept. Seventeen sites remained and had become the top application consumer of production database time: measured across the resulting query shapes, 32,694 calls and 25,848 seconds of execution, mean 790ms, with shapes averaging 2.6s and 3.0s and maxing at 7,962ms against the 8s statement_timeout, which surfaced to users as 500s on the booking path. PostgREST compiles an embed with filters on the embedded side into a correlated INNER JOIN LATERAL with a parameterized LIMIT, which stops Postgres reordering the join, so each query walked the whole journal_entry_lines table across all tenants. Driving from the entries side instead turns that into two indexed round trips. Converted sites keep their existing shape: the helper reattaches the parent entry under the same key the embed produced. Several conversions also remove a latent silent truncation where an unpaginated query was capped at PostgREST's 1000 row ceiling. Two deliberate exceptions. The free text ilike legs of the MCP display query stay on the embed, because each is capped at legLimit and that cap drives the truncation contract the tool reports, while the helper is unbounded. The accounts route moves to the existing get_account_usage_counts RPC instead, since its embed was a head count and the helper returns rows. commitEntry's write path is untouched: the change there is confined to the read query of the pre-commit dimension rule check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(api): anchor v1 list cursors on created_at Page two returned page one, forever, while still advertising a fresh next_cursor. The three routes sorted by and encoded a Postgres date column, which serializes as YYYY-MM-DD, but decodeDefaultCursor validates the cursor timestamp as full ISO-8601 and returned null, so the keyset filter was never applied and has_more never went false. An integrator syncing verifikat looped on the newest rows indefinitely. The transactions route already solved this and its comment names the trap; the fix was never ported. All three now order and encode on created_at with an id tie break, matching the transactions keyset predicate exactly. ISO_TIMESTAMP is deliberately left alone: relaxing it would silently change sort semantics on the route that currently works. Default ordering therefore moves from business date to insert order. Every business date is still on the row, and the invoices list gains date_from and date_to filters so a date range is still reachable; the other two already had them. The tests use an in-memory PostgREST that actually evaluates the filters, because the repo's pass-through mock cannot catch this class of bug: the bug is that the filter is never sent. They walk to exhaustion with a hard iteration cap, so an unterminated walk fails instead of hanging. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(api): separate dry run from commit in the idempotency hash The request hash was built from url.pathname, which excludes the query string, so a dry run and its commit hashed identically. Following the flow documented in dry-run.ts, re-issuing the request with the same Idempotency-Key returned the cached preview with Idempotent-Replayed set and wrote nothing, while reporting 200. An agent or integrator saw success for a write that never happened. dry_run is folded into the hash only when true, not as an unconditional boolean. Including it as false would change the hash of every ordinary write, and with a 24h idempotency TTL any key in flight across the deploy would fail the request_hash comparison and 409 on a legitimate retry. Both hash call sites now go through one shared helper so they cannot drift into a permanent cache miss, and dry run responses are no longer stored at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: install the Bedrock SDK out of tree in the compliance review The Swedish accounting compliance gate had failed ten consecutive runs and so was posting nothing. With --no-package-lock npm discarded the lockfile and re-resolved the whole tree from package.json, floating @hookform/resolvers to 5.4.3, whose valibot ^1 peer conflicts with the pinned valibot 0.39.0. Installing into the parent of the checkout resolves only that one package, so an unrelated peer conflict can never take the gate down again. Node still finds it because ESM bare specifiers walk up parent node_modules; NODE_PATH would not have worked, as it is CommonJS only. --legacy-peer-deps was rejected because it masks future genuine peer conflicts and still reifies the full tree. The same step's SDK version is aligned from 0.31.0 back to the 0.29.1 that package.json and check:guards enforce after the streaming outage. That drift went unnoticed because the pin guard only inspects package.json and the lockfile, never workflow files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * build(docker): generate crontabs from vercel.json vercel.json defines 16 cron jobs; both Docker crontabs carried 9, and were byte identical to each other. Self hosted deployments therefore never sent recurring invoices, never dispatched webhooks and never cleaned up idempotency keys. tax-deadlines also ran once a year on 2 January instead of daily, and documents/verify weekly instead of daily. Extension crons are included rather than excluded. The Dockerfile copies the whole tree before building, so every extension cron route is compiled into the image regardless of the enabled preset, and each returns 200 when its extension is unconfigured, so curl -sf logs no failure. Two such entries were already present in the crontab for extensions absent from the preset, which settles the intent. documents/verify is treated as drift rather than a self hosted concession: the weekly cadence was present in the hosted crontab too, and the run is capped at 200 documents walking a nulls-first queue, so weekly drains the integrity queue seven times slower on a check that exists for BFL retention. webhooks/dispatch keeps its per minute cadence, adding 1,440 requests a day on self hosted. A gentler tick would silently stretch the first retry, since the retry ladder opens at 60 seconds. SCHEDULE_OVERRIDES is the one line place to change that. A parity test asserts the path sets match minus a documented exclusion list, and ratchets three cron routes that are currently scheduled nowhere so they are named rather than silently rotting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(observability): add a provider agnostic error sink There is no error tracking in this codebase: logs go to console and Vercel retention and nowhere else, nothing alerts on the 16 cron jobs, and seven code comments across lib, app, components and extensions asserted that Sentry captures errors when Sentry is not a dependency. The two most recent bug fixes on this repo were both discovered by customer email. This adds the sink, not a vendor. No dependency is taken: the interface has a no-op default and a registration point, so behaviour is unchanged until an adapter is registered. Releases are tagged from the build id already inlined by next.config.ts. Redaction moved out of lib/logger.ts into a leaf module that both the logger and the sink import, so there is one denylist and no path from application data to a third party can skip the personnummer regex, including direct sink calls that bypass the logger. That matters here because these logs carry personnummer and financial data. verifyCronSecret now reports its own 401s, which covers all 16 jobs without touching a route file and catches the case where CRON_SECRET is rotated without updating the scheduler and every job silently 401s forever. The threshold is one failure rather than the backup alert's three: suppressing the first occurrence is precisely how an outage stays invisible. The seven misleading comments are corrected to describe what the code actually does, including the two cases that still are not covered: the client side one, since the sink is server side, and a warn level call that is not forwarded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: remediate the 2026-07-26 similar-sweep findings across all surfaces Resolves the ~150-finding sweep (dev_docs/similar-sweep-2026-07-26.md) with one agent per finding; every behavioural fix carries a regression test proven to fail at HEAD. Full status, corrections to the sweep, refusals and open decisions in dev_docs/similar-sweep-2026-07-26-remediation-status.md. Structural roots closed: - resolveSekAmountOrNull(): honest SEK resolution refuses instead of booking 1:1; four duplicated toSek closures now refuse via INVOICE_FX_RATE_MISSING - ledger-line-amount.ts: journal_entry_lines.currency labels the document, not the amount; SQL pre-filter decoy proven and fixed - sparse-patch.ts: .partial() does not strip .default() in Zod 4.4.3; the exploitable salary payslip-line PATCH and KPI preferences sinks fixed - tests/schema: migration-replay phantom-column guard (13k+ refs, closed CHECK sets, onConflict targets); found 28 real defects, all fixed, all four baselines now empty - three new ratchet guards: sek-labelled-amount, cross-extension-import, ungated-extension-route Highlights: lawful VAT-rate set on all seven invoice surfaces (ML 6 kap), RC input VAT mismatch wired on web + both MCP callers, missing-underlag resource delegates to the shared RPC predicate, push-notifications consent polarity fail-closed, deadlines undo honours requested state, silent-failure and read-side-fabrication classes fixed across settings/KPI/inbox/Stripe/ Arcim/kassaflodesanalys, error-envelope stringification fixed at 10+ sites with isSwedishUserMessage extended. Also includes the parallel session's MCP invoice tools (update_invoice, recurring schedules, invoice deliveries) which share files with the sweep work and are verified green together. 13 new migrations are NOT applied anywhere; they apply via branch merge. 20260726120000 backfills 1247 supplier-invoice rows. pg tests for new DDL are written but unrun (no local Postgres). Verified: 11088 tests / 881 files green, tsc 0 non-test errors, lint 0 errors, check:guards passing, MCP payload 57475/57500. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(migrations): rename replace_sie_import migration off main's 20260726090000 version origin/main shipped 20260726090000_agent_quota_rpc_caller_guard.sql; keeping our replace_sie_import migration on the same version would abort the Supabase apply with a schema_migrations_pkey duplicate at merge time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(review): remediate pre-publish deep-review findings across all slices A 13-agent review of the full branch diff surfaced 1 critical, 5 high and ~45 further findings; this commit resolves them in one pass: - replace_sie_import / undo_sie_import: p_user_id honored only for service_role callers; any other caller is pinned to auth.uid() (impersonation gate bypass), authz raise errcode 42501 mapped to a Swedish 403 in the route, new caller-guard migration for undo - bulk_book_transactions refuses homogeneous non-SEK batches instead of writing foreign magnitudes into SEK ledger columns - credit-note cap trigger: company-match on credited_invoice_id, no cross-tenant figures in exception text - link_voucher RPCs resolve NULL invoice currency as SEK end to end - personal-number ciphertext CHECK split into NOT VALID + VALIDATE - same-currency foreign settlements clear 1510 at booking rate and book realized diff to 3960/7960; rate-less foreign write paths refuse - receivables revaluation covers partially_paid and outstanding amounts - period lock guard paginates candidates past the PostgREST 1000 cap - documents: service-client storage removals after authz, dual-layout reads in integrity cron and archive export, backfill delete-source sweep actually deletes with hash verification and shared-key grouping - invoice matching normalizes NULL/lowercase currencies (regression), duplicate candidates stop claiming amount matches they never ran - match-invoice aborts on any booking failure (no paid-without-verifikat) - refresh-exchange-rate reverts on concurrent booking (TOCTOU window) - KPI preferences upsert arbiter aligned to the company-scoped constraint - personnummer_last4 stripped from all salary responses incl. MCP tools - worked-hours batch restores destroyed rows on conflict and error paths - MCP: shared duplicate-claim builder (no more 'null kr'), short-circuit on tag_journal_lines overflow, auto_send schedules stage as high risk - observability sink redacts emails/IBANs/API keys and keeps redacted stacks in prod; assorted small guards (safe-return-to /@, dry_run=True, cursor helper off-by-one, OAuth state TTL 10 min, arcim saveMappings call removed) Full dispositions, deferred items and hand-verified accounting numbers are documented in the PR body and DECISIONS.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(personnummer): implement masking and encryption for personal numbers with tests * fix(review): address CI and compliance-bot findings for PR #1215 pg-real: the CI image's auth shim reads the legacy request.jwt.claim.role GUC, so both service-role simulations (runAsServiceRole and the invoice-delivery test's local helper) never satisfied auth.role() = 'service_role' and every legitimate p_user_id path failed closed; the shared helper now sets both GUC shapes plus SET LOCAL ROLE with a fail-loud sanity check, and the delivery test reuses it. The link-voucher migration had recreated both RPCs from pre-rewrite file text, reintroducing the NULL-unsafe membership pattern the null-safe-tenant-guards ratchet bans; both guards now use public.caller_is_company_member() with all currency changes preserved. Compliance bots: the customers export now emits the standard masked form instead of raw AES-256-GCM ciphertext in the Org-/personnummer column, and maskCustomerRow returns a non-round-trippable placeholder on decrypt failure instead of 500ing the list. MCP parity: gnubok_lock_period's staging pre-check now runs the exact countUnbookedInPeriod the commit path enforces (exported from period-service; local mirror deleted), and gnubok_agi_status resolves AGI state run-scoped so a correction run no longer renders as already filed. Declined with evidence: PR-Agent's opening-balances null-zeroing concern (all mergeable columns are NOT NULL with defaults per 20260713101000). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(review): address codex review findings on PR #1215 - restore 20260726140000 to its preview-recorded content and restate the NULL-safe tenant guard under 20260727130000: a recorded migration version never re-runs, so the in-place edit could not reach the preview branch - replace toFixed() with sv-SE two-decimal formatting in the ROT/RUT cap warning texts and update the pinned test expectations - drop the em dash in the fiscal-periods route comment - strip trailing whitespace in import-existing.test.ts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reports): raise timeout on real PDF render tests renderToBuffer does real @react-pdf layout work and exceeds the 5s default when the full suite saturates the CPU; tests pass in isolation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5
Claude Fable 5
parent
a43a8b03cf
commit
f24b26a139
@@ -794,6 +794,34 @@ describe('CreateSupplierInvoiceSchema', () => {
|
||||
expect(result.success).toBe(false)
|
||||
})
|
||||
|
||||
// The DB CHECK supplier_invoices_exchange_rate_check reads
|
||||
// `exchange_rate IS NULL OR (exchange_rate > 0 AND exchange_rate < 100000)`.
|
||||
// The schema had no ceiling at all, so a fat-fingered rate reached Postgres
|
||||
// and came back as a 23514 the route surfaced as a 500. These three pin the
|
||||
// mirror to the constraint, exclusivity included.
|
||||
it('rejects an exchange rate at or above the DB ceiling of 100000', () => {
|
||||
const result = CreateSupplierInvoiceSchema.safeParse(
|
||||
validSupplierInvoice({ exchange_rate: 250000 })
|
||||
)
|
||||
expect(result.success).toBe(false)
|
||||
const issue = result.error?.issues.find((i) => i.path.join('.') === 'exchange_rate')
|
||||
expect(issue?.message).toContain('100 000')
|
||||
})
|
||||
|
||||
it('rejects exactly 100000: the CHECK bound is exclusive', () => {
|
||||
const result = CreateSupplierInvoiceSchema.safeParse(
|
||||
validSupplierInvoice({ exchange_rate: 100000 })
|
||||
)
|
||||
expect(result.success).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts 99999.99, just inside the exclusive ceiling', () => {
|
||||
const result = CreateSupplierInvoiceSchema.safeParse(
|
||||
validSupplierInvoice({ exchange_rate: 99999.99 })
|
||||
)
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts item with legacy quantity/unit_price fields', () => {
|
||||
const result = CreateSupplierInvoiceSchema.safeParse(
|
||||
validSupplierInvoice({
|
||||
@@ -1199,6 +1227,36 @@ describe('MatchInvoiceSchema', () => {
|
||||
const result = MatchInvoiceSchema.safeParse({ invoice_id: 'INV-001' })
|
||||
expect(result.success).toBe(false)
|
||||
})
|
||||
|
||||
// manual_exchange_rate lands in invoice_payments.payment_exchange_rate,
|
||||
// whose CHECK is `> 0 AND < 100000`. The old `.max(100000)` was inclusive:
|
||||
// exactly 100000 passed Zod and then violated the constraint.
|
||||
it('rejects exactly 100000 for manual_exchange_rate (exclusive CHECK)', () => {
|
||||
const result = MatchInvoiceSchema.safeParse({
|
||||
invoice_id: validUuid,
|
||||
manual_exchange_rate: 100000,
|
||||
})
|
||||
expect(result.success).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts 99999.99 for manual_exchange_rate', () => {
|
||||
const result = MatchInvoiceSchema.safeParse({
|
||||
invoice_id: validUuid,
|
||||
manual_exchange_rate: 99999.99,
|
||||
})
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects a zero or negative manual_exchange_rate', () => {
|
||||
expect(MatchInvoiceSchema.safeParse({
|
||||
invoice_id: validUuid,
|
||||
manual_exchange_rate: 0,
|
||||
}).success).toBe(false)
|
||||
expect(MatchInvoiceSchema.safeParse({
|
||||
invoice_id: validUuid,
|
||||
manual_exchange_rate: -11.5,
|
||||
}).success).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('MatchSupplierInvoiceSchema', () => {
|
||||
|
||||
@@ -0,0 +1,274 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { z } from 'zod'
|
||||
import { sparsePatch, sparsePatchBody, isPatchDocument } from '@/lib/api/sparse-patch'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { createMockRequest } from '@/tests/helpers'
|
||||
|
||||
// Mirrors the real hazard shape: a create schema carrying .default() flags,
|
||||
// turned into a patch schema with .partial().
|
||||
const CreateLine = z.object({
|
||||
description: z.string().min(1),
|
||||
amount: z.number(),
|
||||
quantity: z.number().optional(),
|
||||
is_taxable: z.boolean().default(true),
|
||||
is_net_deduction: z.boolean().default(false),
|
||||
sort_order: z.number().int().default(0),
|
||||
account_number: z.string().nullable().optional(),
|
||||
tags: z.array(z.string()).default([]),
|
||||
meta: z.object({ source: z.string().default('manual') }).default({ source: 'manual' }),
|
||||
})
|
||||
|
||||
const UpdateLine = CreateLine.partial()
|
||||
|
||||
describe('the bug this helper exists for', () => {
|
||||
it('.partial() does NOT strip .default(): a one-field parse resurrects every default', () => {
|
||||
// This is the finding, pinned as an executable fact. If a future zod
|
||||
// upgrade changes it, this test tells you the helper can be retired.
|
||||
expect(UpdateLine.parse({ amount: 5500 })).toEqual({
|
||||
amount: 5500,
|
||||
is_taxable: true,
|
||||
is_net_deduction: false,
|
||||
sort_order: 0,
|
||||
tags: [],
|
||||
meta: { source: 'manual' },
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('sparsePatch', () => {
|
||||
it('a one-field patch does not resurrect defaults', () => {
|
||||
const result = sparsePatch(UpdateLine, { amount: 5500 })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ amount: 5500 })
|
||||
expect(Object.keys(result.data)).toEqual(['amount'])
|
||||
})
|
||||
|
||||
it('keeps a default-carrying field when the caller DID send it', () => {
|
||||
const result = sparsePatch(UpdateLine, { is_taxable: false })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ is_taxable: false })
|
||||
})
|
||||
|
||||
it('an explicit null survives as a deliberate clear', () => {
|
||||
const result = sparsePatch(UpdateLine, { account_number: null })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ account_number: null })
|
||||
expect('account_number' in result.data).toBe(true)
|
||||
})
|
||||
|
||||
it('an absent key is untouched (never appears in the output)', () => {
|
||||
const result = sparsePatch(UpdateLine, { description: 'Bonus' })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect('account_number' in result.data).toBe(false)
|
||||
expect('is_taxable' in result.data).toBe(false)
|
||||
expect('sort_order' in result.data).toBe(false)
|
||||
})
|
||||
|
||||
it('null and absent are distinguishable, which is the whole point', () => {
|
||||
const cleared = sparsePatch(UpdateLine, { account_number: null })
|
||||
const untouched = sparsePatch(UpdateLine, {})
|
||||
expect(cleared.success && cleared.data).toEqual({ account_number: null })
|
||||
expect(untouched.success && untouched.data).toEqual({})
|
||||
})
|
||||
|
||||
it('drops an in-process undefined (not expressible in JSON, never intended)', () => {
|
||||
const result = sparsePatch(UpdateLine, { amount: 1, quantity: undefined })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ amount: 1 })
|
||||
expect('quantity' in result.data).toBe(false)
|
||||
})
|
||||
|
||||
it('drops unknown keys rather than passing them to the caller', () => {
|
||||
const result = sparsePatch(UpdateLine, { amount: 1, is_system: true, company_id: 'other' })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ amount: 1 })
|
||||
})
|
||||
|
||||
it('drops prototype-polluting own properties from a JSON body', () => {
|
||||
const rawBody = JSON.parse('{"amount": 1, "__proto__": {"polluted": true}}')
|
||||
const result = sparsePatch(UpdateLine, rawBody)
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ amount: 1 })
|
||||
expect(({} as Record<string, unknown>).polluted).toBeUndefined()
|
||||
})
|
||||
|
||||
describe('nested objects', () => {
|
||||
it('an absent nested key stays absent (its inner defaults do not materialise)', () => {
|
||||
const result = sparsePatch(UpdateLine, { amount: 1 })
|
||||
expect(result.success && 'meta' in result.data).toBe(false)
|
||||
})
|
||||
|
||||
it('a supplied nested key is replaced wholesale, inner defaults included', () => {
|
||||
// Shallow by design: the sink is `SET col = $1`, which replaces the whole
|
||||
// jsonb value, so a half-merged object would write something the caller
|
||||
// never described.
|
||||
const result = sparsePatch(UpdateLine, { meta: {} })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ meta: { source: 'manual' } })
|
||||
})
|
||||
|
||||
it('reports validation errors with their full nested path', () => {
|
||||
const result = sparsePatch(UpdateLine, { meta: { source: 42 } })
|
||||
expect(result.success).toBe(false)
|
||||
if (result.success) return
|
||||
expect(result.error.issues[0].path).toEqual(['meta', 'source'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('arrays', () => {
|
||||
it('an array VALUE is taken wholesale', () => {
|
||||
const result = sparsePatch(UpdateLine, { tags: ['a', 'b'] })
|
||||
expect(result.success && result.data).toEqual({ tags: ['a', 'b'] })
|
||||
})
|
||||
|
||||
it('an explicitly empty array is a real value, not an absence', () => {
|
||||
const result = sparsePatch(UpdateLine, { tags: [] })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ tags: [] })
|
||||
expect('tags' in result.data).toBe(true)
|
||||
})
|
||||
|
||||
it('an array BODY is rejected: a patch document must be a JSON object', () => {
|
||||
const result = sparsePatch(UpdateLine, [{ amount: 1 }])
|
||||
expect(result.success).toBe(false)
|
||||
if (result.success) return
|
||||
expect(result.error.issues[0].message).toContain('JSON object')
|
||||
})
|
||||
})
|
||||
|
||||
it.each([
|
||||
['null', null],
|
||||
['a scalar', 42],
|
||||
['a string', 'amount=1'],
|
||||
])('rejects %s as a body', (_label, body) => {
|
||||
expect(sparsePatch(UpdateLine, body).success).toBe(false)
|
||||
})
|
||||
|
||||
it('propagates schema validation failures unchanged', () => {
|
||||
const result = sparsePatch(UpdateLine, { amount: 'not a number' })
|
||||
expect(result.success).toBe(false)
|
||||
if (result.success) return
|
||||
expect(result.error.issues[0].path).toEqual(['amount'])
|
||||
})
|
||||
|
||||
it('throws when the schema output is not an object (there is nothing to narrow)', () => {
|
||||
const Reshaped = z.object({ a: z.number().optional() }).transform(() => null)
|
||||
expect(() => sparsePatch(Reshaped, { a: 1 })).toThrow(/output is an object/)
|
||||
})
|
||||
|
||||
it('works on a .superRefine()-wrapped schema (where .shape is unreachable)', () => {
|
||||
const Refined = CreateLine.partial().superRefine((data, ctx) => {
|
||||
if (data.amount === 13) {
|
||||
ctx.addIssue({ code: 'custom', message: 'Olyckstal', path: ['amount'] })
|
||||
}
|
||||
})
|
||||
expect(sparsePatch(Refined, { amount: 1 }).success && sparsePatch(Refined, { amount: 1 })).toMatchObject({
|
||||
data: { amount: 1 },
|
||||
})
|
||||
const bad = sparsePatch(Refined, { amount: 13 })
|
||||
expect(bad.success).toBe(false)
|
||||
if (bad.success) return
|
||||
expect(bad.error.issues[0].path).toEqual(['amount'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('isPatchDocument', () => {
|
||||
it.each([
|
||||
[{}, true],
|
||||
[{ a: 1 }, true],
|
||||
[[], false],
|
||||
[null, false],
|
||||
[1, false],
|
||||
['x', false],
|
||||
[undefined, false],
|
||||
])('%s -> %s', (input, expected) => {
|
||||
expect(isPatchDocument(input)).toBe(expected)
|
||||
})
|
||||
})
|
||||
|
||||
describe('sparsePatchBody + validateBody', () => {
|
||||
async function patch(body: unknown) {
|
||||
const request = createMockRequest('/api/thing/1', { method: 'PATCH', body })
|
||||
return validateBody(request, sparsePatchBody(UpdateLine))
|
||||
}
|
||||
|
||||
it('yields only the sent keys through the validateBody pipeline', async () => {
|
||||
const result = await patch({ amount: 5500 })
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({ amount: 5500 })
|
||||
})
|
||||
|
||||
it('keeps an explicit null through the validateBody pipeline', async () => {
|
||||
const result = await patch({ account_number: null })
|
||||
expect(result.success && result.data).toEqual({ account_number: null })
|
||||
})
|
||||
|
||||
it('returns a 400 with the original field path on a schema failure', async () => {
|
||||
const result = await patch({ amount: 'x' })
|
||||
expect(result.success).toBe(false)
|
||||
if (result.success) return
|
||||
expect(result.response.status).toBe(400)
|
||||
const body = await result.response.json()
|
||||
expect(body.errors[0].field).toBe('amount')
|
||||
})
|
||||
|
||||
// Forwarding through ctx.addIssue must not flatten the issue to `custom`:
|
||||
// validateBody puts `code` in the 400 envelope and clients branch on it.
|
||||
it('keeps the original issue CODE, not a flattened `custom`', async () => {
|
||||
const result = await patch({ amount: 'x' })
|
||||
expect(result.success).toBe(false)
|
||||
if (result.success) return
|
||||
const body = await result.response.json()
|
||||
expect(body.errors[0].code).toBe('invalid_type')
|
||||
expect(body.errors[0].code).not.toBe('custom')
|
||||
})
|
||||
|
||||
it('forwards every issue, each with its own nested path and code', async () => {
|
||||
const result = await patch({ meta: { source: 1 }, description: '' })
|
||||
expect(result.success).toBe(false)
|
||||
if (result.success) return
|
||||
const body = await result.response.json()
|
||||
expect(body.errors).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ field: 'meta.source', code: 'invalid_type' }),
|
||||
expect.objectContaining({ field: 'description', code: 'too_small' }),
|
||||
]),
|
||||
)
|
||||
})
|
||||
|
||||
it('produces the same issues a plain validateBody(schema) would', async () => {
|
||||
const bare = await validateBody(
|
||||
createMockRequest('/api/thing/1', { method: 'PATCH', body: { amount: 'x' } }),
|
||||
UpdateLine,
|
||||
)
|
||||
const sparse = await patch({ amount: 'x' })
|
||||
expect(bare.success).toBe(false)
|
||||
expect(sparse.success).toBe(false)
|
||||
if (bare.success || sparse.success) return
|
||||
expect(await sparse.response.json()).toEqual(await bare.response.json())
|
||||
})
|
||||
|
||||
it('returns a 400 when the body is not a JSON object', async () => {
|
||||
const result = await patch([1, 2])
|
||||
expect(result.success).toBe(false)
|
||||
if (result.success) return
|
||||
expect(result.response.status).toBe(400)
|
||||
})
|
||||
|
||||
it('an empty body parses to an empty patch, not to a pile of defaults', async () => {
|
||||
const result = await patch({})
|
||||
expect(result.success).toBe(true)
|
||||
if (!result.success) return
|
||||
expect(result.data).toEqual({})
|
||||
})
|
||||
})
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { z } from 'zod'
|
||||
import { validateBody, validateQuery } from '../validate'
|
||||
import { getErrorMessage } from '@/lib/errors/get-error-message'
|
||||
|
||||
// ============================================================
|
||||
// Helpers
|
||||
@@ -74,13 +75,93 @@ describe('validateBody', () => {
|
||||
if (!result.success) {
|
||||
const body = await result.response.json()
|
||||
expect(result.response.status).toBe(400)
|
||||
expect(body.error).toBe('Validation failed')
|
||||
// Inverted on purpose: the old assertion pinned the constant
|
||||
// 'Validation failed', which is exactly the bug. `error` now carries the
|
||||
// actionable field message so clients that read only `error` show
|
||||
// something useful; `errors[]` keeps the full machine-readable detail and
|
||||
// `type` stays the discriminator.
|
||||
expect(body.error).not.toBe('Validation failed')
|
||||
expect(body.error).toMatch(/^Valideringsfel: /)
|
||||
expect(body.error).toContain('name')
|
||||
expect(body.error).toContain('age')
|
||||
expect(body.type).toBe('validation_error')
|
||||
expect(body.errors).toBeInstanceOf(Array)
|
||||
expect(body.errors.length).toBeGreaterThan(0)
|
||||
}
|
||||
})
|
||||
|
||||
it('summarizes at most three issues and counts the rest', async () => {
|
||||
const WideSchema = z.object({
|
||||
a: z.string(),
|
||||
b: z.string(),
|
||||
c: z.string(),
|
||||
d: z.string(),
|
||||
e: z.string(),
|
||||
})
|
||||
const request = createJsonRequest({})
|
||||
const result = await validateBody(request, WideSchema)
|
||||
|
||||
expect(result.success).toBe(false)
|
||||
if (!result.success) {
|
||||
const body = await result.response.json()
|
||||
expect(body.errors).toHaveLength(5)
|
||||
expect(body.error).toContain('(+2 till)')
|
||||
// Only the first three are named in the prose.
|
||||
expect(body.error).not.toContain('d:')
|
||||
expect(body.error).not.toContain('e:')
|
||||
}
|
||||
})
|
||||
|
||||
it('surfaces the actionable Swedish sentence to a client that reads only body.error', async () => {
|
||||
// Reproduces the failing UI path: pages like arsredovisning/page.tsx and
|
||||
// assets/[id]/dispose/page.tsx forward `body.error` (a string) into
|
||||
// getErrorMessage, which collapsed the constant into "Något gick fel."
|
||||
const SwedishSchema = z.object({
|
||||
period_start: z.string({ message: 'Ange periodens startdatum' }),
|
||||
})
|
||||
const request = await validateBody(createJsonRequest({}), SwedishSchema)
|
||||
|
||||
expect(request.success).toBe(false)
|
||||
if (!request.success) {
|
||||
const body = await request.response.json()
|
||||
|
||||
const shown = getErrorMessage(body.error, { statusCode: 400 })
|
||||
expect(shown).toBe(body.error)
|
||||
expect(shown).toContain('Ange periodens startdatum')
|
||||
expect(shown).not.toBe('Något gick fel. Försök igen.')
|
||||
|
||||
// Clients that forward the whole body keep the existing errors[] path.
|
||||
expect(getErrorMessage(body, { statusCode: 400 })).toContain(
|
||||
'Ange periodens startdatum',
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps the machine-readable discriminator and per-field detail intact', async () => {
|
||||
const request = createJsonRequest({ name: '', age: -5 })
|
||||
const result = await validateBody(request, TestSchema)
|
||||
|
||||
expect(result.success).toBe(false)
|
||||
if (!result.success) {
|
||||
const body = await result.response.json()
|
||||
// A machine consumer branches on `type` / `errors[].code`, never on prose.
|
||||
// Both are pinned to concrete values here: asserting only `typeof` would
|
||||
// keep passing if the codes turned into empty strings.
|
||||
expect(body.type).toBe('validation_error')
|
||||
for (const issue of body.errors as Array<Record<string, unknown>>) {
|
||||
expect(typeof issue.field).toBe('string')
|
||||
expect(typeof issue.message).toBe('string')
|
||||
expect(typeof issue.code).toBe('string')
|
||||
}
|
||||
// Field-keyed lookup, the way a client maps issues onto form inputs.
|
||||
const byField = new Map(
|
||||
(body.errors as Array<{ field: string; code: string }>).map((e) => [e.field, e.code]),
|
||||
)
|
||||
expect(byField.get('name')).toBe('too_small')
|
||||
expect(byField.get('age')).toBe('too_small')
|
||||
}
|
||||
})
|
||||
|
||||
it('returns field paths in error details', async () => {
|
||||
const request = createJsonRequest({ name: 'Alice', age: 'not-a-number' })
|
||||
const result = await validateBody(request, TestSchema)
|
||||
|
||||
+106
-8
@@ -35,6 +35,33 @@ const accountNumber = z.string().regex(/^\d{4}$/, 'Account number must be exactl
|
||||
/** Non-negative monetary amount (>= 0) */
|
||||
const nonNegativeAmount = z.number().nonnegative()
|
||||
|
||||
/**
|
||||
* SEK per one unit of a foreign currency.
|
||||
*
|
||||
* Mirrors the database CHECK that every table storing a rate carries:
|
||||
* `invoices_exchange_rate_check`, `supplier_invoices_exchange_rate_check`,
|
||||
* `invoice_payments_payment_exchange_rate_check` and
|
||||
* `supplier_invoice_payments_payment_exchange_rate_check` all read
|
||||
* `rate IS NULL OR (rate > 0 AND rate < 100000)`. BOTH bounds are exclusive,
|
||||
* so this mirror is `.positive()` + `.lt(100000)`; `.max(100000)` would let
|
||||
* exactly 100000 through the schema and straight into a 23514 violation.
|
||||
*
|
||||
* Without the mirror a plausible fat-fingered rate (250000, a pasted total
|
||||
* instead of a rate) passed validation, hit the constraint in Postgres, and
|
||||
* surfaced as an unexplained 500. Through this primitive it lands in
|
||||
* `validateBody`'s 400 with a message naming the field and the fix.
|
||||
*
|
||||
* The ceiling is a typo guard rather than a precise band: no currency the app
|
||||
* supports comes near it (USD ~10.5, EUR ~11.5, GBP ~13.5).
|
||||
*/
|
||||
const exchangeRate = z
|
||||
.number()
|
||||
.positive('Växelkursen måste vara större än 0')
|
||||
.lt(
|
||||
100000,
|
||||
'Växelkursen måste vara mindre än 100 000. Ange kursen per 1 enhet av valutan, till exempel 11,45 för EUR, inte fakturans belopp.',
|
||||
)
|
||||
|
||||
const invoiceEmailAddress = z
|
||||
.string()
|
||||
.trim()
|
||||
@@ -660,10 +687,16 @@ export const CreateSelfBillingInvoiceSchema = z.object({
|
||||
// Recurring invoice schedule schemas
|
||||
// ============================================================
|
||||
|
||||
// Swedish VAT rates per ML 17 kap 24§ p.9: null means "use customer default
|
||||
// from getAvailableVatRates". Any other value would produce a non-compliant
|
||||
// invoice (buyer cannot deduct ingående moms). Cron-time validation against
|
||||
// the customer's allowed set still runs in executeRecurringSchedule.
|
||||
// Swedish VAT rates per ML 17 kap 24§ p.9. null means "use the customer's
|
||||
// default rate" (getAvailableVatRates), which is 0% for a VAT-validated EU
|
||||
// business or an export customer: huvudregeln, ML 6 kap. 34 §, taxes a B2B
|
||||
// service where the buyer is established. An explicit 25/12/6 is still lawful
|
||||
// for those customers when the supply is taxed where it is performed
|
||||
// (fastighetstjänst, persontransport, korttidsuthyrning, restaurang/catering,
|
||||
// admission to cultural and sports events), so cron-time validation in
|
||||
// executeRecurringSchedule gates on getPermittedVatRates, not on the default.
|
||||
// A rate outside 0/6/12/25 is rejected here: there is no such Swedish rate, and
|
||||
// the buyer could not deduct ingående moms on it.
|
||||
export const RecurringScheduleItemSchema = z.object({
|
||||
description: z.string().min(1, 'Item description is required'),
|
||||
quantity: z.number().positive('Quantity must be positive'),
|
||||
@@ -814,7 +847,17 @@ export const UpdateCustomerSchema = z.object({
|
||||
country: z.string().optional(),
|
||||
org_number: z.string().optional(),
|
||||
vat_number: z.string().optional(),
|
||||
personal_number: z.string().regex(/^(\d{6}|\d{8})[-+]?\d{4}$/, 'Invalid personal number').nullable().optional(),
|
||||
// Plaintext personnummer (validated here, then encrypted by the route), or
|
||||
// the masked form '********-1234' that every read path returns. The route
|
||||
// reads the mask as "leave the stored value alone" and never stores it, so
|
||||
// a client echoing back what it read cannot wipe the personnummer.
|
||||
// CreateCustomerSchema stays strict: on create there is no stored value to
|
||||
// preserve, so a mask there is a client error and earns a 400.
|
||||
personal_number: z
|
||||
.string()
|
||||
.regex(/^(?:(\d{6}|\d{8})[-+]?\d{4}|\*{8}-\d{4})$/, 'Invalid personal number')
|
||||
.nullable()
|
||||
.optional(),
|
||||
language: z.enum(['sv', 'en']).optional(),
|
||||
default_payment_terms: z.number().int().positive().optional(),
|
||||
notes: z.string().optional(),
|
||||
@@ -929,7 +972,9 @@ export const CreateSupplierInvoiceSchema = z.object({
|
||||
due_date: isoDate,
|
||||
delivery_date: optionalIsoDate,
|
||||
currency: CurrencySchema.optional(),
|
||||
exchange_rate: z.number().positive().optional(),
|
||||
// Bounded by the shared `exchangeRate` primitive so the value can never
|
||||
// reach `supplier_invoices_exchange_rate_check` and come back as a 500.
|
||||
exchange_rate: exchangeRate.optional(),
|
||||
vat_treatment: VatTreatmentSchema.optional(),
|
||||
reverse_charge: z.boolean().optional(),
|
||||
payment_reference: z.string().optional(),
|
||||
@@ -1347,13 +1392,16 @@ export const MatchInvoiceSchema = z
|
||||
// settlement. Used when the Riksbanken lookup returns nothing (rate not
|
||||
// published for that date): the dialog surfaces an input so the user can
|
||||
// type the rate from their bank statement. Ignored when tx.currency ===
|
||||
// invoice.currency. The .max() is a sanity ceiling against pasted garbage /
|
||||
// invoice.currency. The ceiling is a sanity guard against pasted garbage /
|
||||
// scientific-notation input silently corrupting the FX-diff posting and
|
||||
// invoice_payments.amount: no supported currency's SEK rate approaches it
|
||||
// (USD~10.5, EUR~11.5, GBP~13.5). It is a guard rail, not a precise band;
|
||||
// the dialog's live preview (paid_in_invoice_currency + FX gain/loss) is
|
||||
// what catches a plausible-but-wrong decimal-shift typo before confirm.
|
||||
manual_exchange_rate: z.number().positive().max(100000).optional(),
|
||||
// It used to be `.max(100000)`, an inclusive ceiling against an exclusive
|
||||
// `payment_exchange_rate < 100000` CHECK: exactly 100000 passed Zod and
|
||||
// died in Postgres. The shared primitive is exclusive on both ends.
|
||||
manual_exchange_rate: exchangeRate.optional(),
|
||||
})
|
||||
.refine((v) => !v.force || !!v.expected_journal_entry_id, {
|
||||
message: 'expected_journal_entry_id is required when force=true',
|
||||
@@ -2492,6 +2540,23 @@ export const UpdateEmployeeSchema = EmployeeSchemaPatchBase.partial().superRefin
|
||||
|
||||
export const EmployeeBenefitTypeSchema = z.enum(['bike', 'car', 'meals', 'housing', 'wellness', 'other'])
|
||||
|
||||
/**
|
||||
* Mirrors the table-level CHECK on employee_benefits (migration
|
||||
* 20260512200100_employee_benefits.sql):
|
||||
*
|
||||
* CHECK (valid_to IS NULL OR valid_to >= valid_from)
|
||||
*
|
||||
* The bound is INCLUSIVE (`>=`): valid_to === valid_from is a legal single-day
|
||||
* benefit, and the run-calculation window is inclusive at both ends too
|
||||
* (`valid_from <= payment_date` AND `valid_to IS NULL OR valid_to >=
|
||||
* payment_date`, lib/salary/run-calculation.ts). A NULL/omitted valid_to means
|
||||
* an open-ended benefit and stays legal. Only a strictly earlier valid_to is
|
||||
* rejected. Shared with the routes so the schema 400 and the route's
|
||||
* merged-state 400 say the same thing.
|
||||
*/
|
||||
export const BENEFIT_PERIOD_ORDER_MESSAGE =
|
||||
'"Gäller till" måste vara samma dag som eller efter "Gäller från". Lämna fältet tomt för en löpande förmån.'
|
||||
|
||||
export const CreateEmployeeBenefitSchema = z.object({
|
||||
benefit_type: EmployeeBenefitTypeSchema,
|
||||
description: z.string().min(1).max(200),
|
||||
@@ -2519,6 +2584,20 @@ export const CreateEmployeeBenefitSchema = z.object({
|
||||
path: ['monthly_value'],
|
||||
})
|
||||
}
|
||||
|
||||
// Validity period: exact mirror of the DB CHECK (see
|
||||
// BENEFIT_PERIOD_ORDER_MESSAGE). Both dates are always fully visible on a
|
||||
// create, so the whole constraint is checkable here and the insert can no
|
||||
// longer trip the CHECK and surface as an opaque 500. ISO YYYY-MM-DD strings
|
||||
// order lexicographically the same as chronologically, so a plain `<` is
|
||||
// exact; `=== undefined` keeps the open-ended case legal.
|
||||
if (data.valid_to !== undefined && data.valid_to < data.valid_from) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
message: BENEFIT_PERIOD_ORDER_MESSAGE,
|
||||
path: ['valid_to'],
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
export const UpdateEmployeeBenefitSchema = z.object({
|
||||
@@ -2529,6 +2608,25 @@ export const UpdateEmployeeBenefitSchema = z.object({
|
||||
valid_to: isoDate.nullable().optional(),
|
||||
metadata: z.record(z.string(), z.unknown()).optional(),
|
||||
is_active: z.boolean().optional(),
|
||||
}).superRefine((data, ctx) => {
|
||||
// Same DB CHECK mirror as the create schema, with the .partial() caveat: an
|
||||
// all-optional body only lets the schema compare the two dates when it
|
||||
// carries BOTH. A single-date PATCH has nothing in-body to compare against
|
||||
// (the other half lives on the stored row), so the route re-checks the merged
|
||||
// stored+patched pair before it writes. `valid_to: null` clears the end date
|
||||
// and stays legal, exactly as `valid_to IS NULL` is in the CHECK.
|
||||
if (
|
||||
data.valid_from !== undefined &&
|
||||
data.valid_to !== undefined &&
|
||||
data.valid_to !== null &&
|
||||
data.valid_to < data.valid_from
|
||||
) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
message: BENEFIT_PERIOD_ORDER_MESSAGE,
|
||||
path: ['valid_to'],
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
export const CreateSalaryRunSchema = z.object({
|
||||
|
||||
@@ -0,0 +1,196 @@
|
||||
/**
|
||||
* Sparse PATCH parsing: keep only the fields the caller actually sent.
|
||||
*
|
||||
* ## The problem
|
||||
*
|
||||
* Zod's `.partial()` makes every key optional; it does NOT strip `.default()`.
|
||||
* Verified against the zod version in this repo (4.4.3):
|
||||
*
|
||||
* ```
|
||||
* const Base = z.object({ amount: z.number(), is_taxable: z.boolean().default(true) })
|
||||
* Base.partial().parse({ amount: 5500 })
|
||||
* // -> { amount: 5500, is_taxable: true } <- is_taxable was NEVER sent
|
||||
* ```
|
||||
*
|
||||
* Routes that spread that result into `.update()` therefore reset every
|
||||
* defaulted column on every PATCH: naming one field silently rewrites the
|
||||
* others. The repo hit this three times independently (an
|
||||
* `EmployeeSchemaPatchBase` re-declaration in lib/api/schemas.ts and a
|
||||
* hand-rolled `rawKeys` intersection in three v1 routes). This module is the
|
||||
* shared version of the `rawKeys` intersection, which is the one that
|
||||
* generalises: it needs nothing from the schema's internals, so it works on
|
||||
* `.partial()`, `.omit()`, and `.superRefine()`-wrapped schemas alike, and it
|
||||
* cannot drift the way a hand-maintained duplicate base shape does.
|
||||
*
|
||||
* ## Semantics
|
||||
*
|
||||
* The output contains exactly the keys that are BOTH (a) literally present as
|
||||
* own properties of the raw JSON body and (b) known to the schema.
|
||||
*
|
||||
* | raw body | in output? |
|
||||
* |-------------------------|-------------------------------------------------|
|
||||
* | `{ "amount": 5500 }` | `amount: 5500` only. Defaults never materialise. |
|
||||
* | `{ "note": null }` | `note: null`. An explicit null is a deliberate |
|
||||
* | | clear and MUST survive: null-vs-absent is the |
|
||||
* | | entire point of this helper. |
|
||||
* | key absent | absent. The column is left untouched. |
|
||||
* | `{ "q": undefined }` | dropped. `undefined` is not expressible in JSON, |
|
||||
* | | so a caller can never have meant it; inventing a |
|
||||
* | | `null` would clear a column nobody asked to |
|
||||
* | | clear. (Only reachable from in-process callers.) |
|
||||
* | unknown key | dropped. The intersection runs over the schema's |
|
||||
* | | parsed output, so unknown keys cannot reach the |
|
||||
* | | database (mass-assignment defense). |
|
||||
* | `{ "tags": [] }` | `tags: []`. An array VALUE is a value: it is |
|
||||
* | | taken wholesale, empty array included. |
|
||||
* | body is an array/scalar | rejected. A patch document must be a JSON object.|
|
||||
*
|
||||
* **Shallow by design.** A key whose value is an object is replaced wholesale,
|
||||
* including any `.default()` Zod filled in *inside* that object. This is
|
||||
* correct for the intended sink: `UPDATE ... SET col = $1` replaces a whole
|
||||
* jsonb column, so a "partially patched" nested object would write a value the
|
||||
* caller never described. If a nested field genuinely needs per-key merge
|
||||
* semantics, merge it against the stored row in the route, explicitly.
|
||||
*
|
||||
* Validation itself is unchanged: the schema parses the raw body first, so
|
||||
* refinements still see the default-filled object exactly as before. Only the
|
||||
* *write set* is narrowed.
|
||||
*
|
||||
* ## When NOT to use this
|
||||
*
|
||||
* The narrowing happens AFTER parsing, so it only helps a sink that writes the
|
||||
* keys it is handed. Two shapes it does not fix:
|
||||
*
|
||||
* 1. **A fixed-column sink** (an upsert that writes a whole row or a whole
|
||||
* jsonb document). Narrowing the patch does not stop the unmentioned columns
|
||||
* from being written; it only makes them `undefined`. Such a route must merge
|
||||
* the sparse patch over the STORED row before writing (see
|
||||
* `app/api/kpi/preferences/route.ts`).
|
||||
* 2. **A CROSS-FIELD `.refine` / `.superRefine`.** The refinement runs on the
|
||||
* default-filled parse, so it judges values the caller never sent: it can
|
||||
* reject a legitimate patch and accept an illegitimate one. Strip the
|
||||
* defaults from the patch base instead (see `EmployeeSchemaPatchBase` in
|
||||
* `lib/api/schemas.ts`), or validate against the stored row in the route.
|
||||
*
|
||||
* A schema whose own top-level `.transform()` reshapes the output is also out of
|
||||
* scope: the intersection runs against the raw body's keys, so invented keys are
|
||||
* dropped. Narrow before transforming, not after.
|
||||
*
|
||||
* ## Usage
|
||||
*
|
||||
* With `validateBody` (cookie-session routes):
|
||||
* ```ts
|
||||
* const validation = await validateBody(request, sparsePatchBody(UpdateThingSchema))
|
||||
* if (!validation.success) return validation.response
|
||||
* if (Object.keys(validation.data).length === 0) { ... nothing to update ... }
|
||||
* ```
|
||||
*
|
||||
* With a raw body already in hand (v1 REST / MCP):
|
||||
* ```ts
|
||||
* const patch = sparsePatch(UpdateThingSchema, rawBody)
|
||||
* if (!patch.success) return v1ErrorResponseFromCode('VALIDATION_ERROR', ...)
|
||||
* ```
|
||||
*/
|
||||
|
||||
import { z } from 'zod'
|
||||
|
||||
/**
|
||||
* Own properties that `JSON.parse` can produce but that must never be carried
|
||||
* into an object literal. `parsed.data` (Zod's output) can never contain them,
|
||||
* so the intersection already blocks them; filtering the present-key set too
|
||||
* makes the intent unambiguous. Mirrors the guard the v1 routes already ship.
|
||||
*/
|
||||
const POLLUTING_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
|
||||
|
||||
export type SparsePatchResult<T> =
|
||||
| { success: true; data: Partial<T> }
|
||||
| { success: false; error: z.ZodError }
|
||||
|
||||
/** A patch document is a plain JSON object: not null, not an array, not a scalar. */
|
||||
export function isPatchDocument(raw: unknown): raw is Record<string, unknown> {
|
||||
return typeof raw === 'object' && raw !== null && !Array.isArray(raw)
|
||||
}
|
||||
|
||||
/** Own enumerable keys of a raw JSON body, minus the prototype-polluting ones. */
|
||||
function presentKeys(rawBody: Record<string, unknown>): Set<string> {
|
||||
return new Set(Object.keys(rawBody).filter((key) => !POLLUTING_KEYS.has(key)))
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse `rawBody` with `schema`, then keep only the keys the caller literally
|
||||
* sent. See the module docblock for the exact null-vs-absent semantics.
|
||||
*/
|
||||
export function sparsePatch<S extends z.ZodType>(
|
||||
schema: S,
|
||||
rawBody: unknown,
|
||||
): SparsePatchResult<z.infer<S>> {
|
||||
if (!isPatchDocument(rawBody)) {
|
||||
return {
|
||||
success: false,
|
||||
error: new z.ZodError([
|
||||
{
|
||||
code: 'invalid_type',
|
||||
expected: 'object',
|
||||
input: rawBody,
|
||||
path: [],
|
||||
message: 'Body must be a JSON object.',
|
||||
},
|
||||
]),
|
||||
}
|
||||
}
|
||||
|
||||
const parsed = schema.safeParse(rawBody)
|
||||
if (!parsed.success) {
|
||||
return { success: false, error: parsed.error }
|
||||
}
|
||||
|
||||
// A schema whose output is not an object has no keys to narrow, so the caller
|
||||
// has misused the helper. Fail loudly instead of throwing an opaque
|
||||
// "Cannot convert undefined or null to object" out of Object.entries.
|
||||
if (!isPatchDocument(parsed.data)) {
|
||||
throw new TypeError(
|
||||
'sparsePatch requires a schema whose output is an object; got ' + typeof parsed.data,
|
||||
)
|
||||
}
|
||||
|
||||
const present = presentKeys(rawBody)
|
||||
const data: Record<string, unknown> = {}
|
||||
// Iterate the PARSED output, not the raw body: unknown keys the schema
|
||||
// stripped must not reappear, and the values must be the coerced/validated
|
||||
// ones. `value !== undefined` drops keys that survived parsing without a
|
||||
// value (see the table above); an explicit `null` is kept.
|
||||
for (const [key, value] of Object.entries(parsed.data)) {
|
||||
if (present.has(key) && value !== undefined) {
|
||||
data[key] = value
|
||||
}
|
||||
}
|
||||
|
||||
return { success: true, data: data as Partial<z.infer<S>> }
|
||||
}
|
||||
|
||||
/**
|
||||
* Wrap a schema so `validateBody()` yields the sparse patch instead of the
|
||||
* default-filled object. Every validation issue is forwarded with its original
|
||||
* `code` and `path`, so the 400 envelope is byte-identical to a plain
|
||||
* `validateBody(request, schema)` (pinned by a test that compares the two).
|
||||
*/
|
||||
export function sparsePatchBody<S extends z.ZodType>(
|
||||
schema: S,
|
||||
): z.ZodType<Partial<z.infer<S>>, unknown> {
|
||||
return z.unknown().transform((raw, ctx) => {
|
||||
const result = sparsePatch(schema, raw)
|
||||
if (!result.success) {
|
||||
// Forwarded by spread, not by passing `issue` straight through: Zod 4
|
||||
// types `addIssue` against the RAW issue shape, and a finalized
|
||||
// `$ZodIssue` is an interface, so it gets no implicit index signature and
|
||||
// will not satisfy that shape. A spread produces an object literal type,
|
||||
// which does, so the original `code` and `path` survive into the 400
|
||||
// envelope with no cast and without flattening to `custom`.
|
||||
for (const issue of result.error.issues) {
|
||||
ctx.addIssue({ ...issue })
|
||||
}
|
||||
return z.NEVER
|
||||
}
|
||||
return result.data
|
||||
})
|
||||
}
|
||||
@@ -81,7 +81,19 @@ describe('nextCursorFromPage', () => {
|
||||
]
|
||||
const cursor = nextCursorFromPage(rows, 2)
|
||||
expect(cursor).not.toBeNull()
|
||||
expect(decodeDefaultCursor(cursor)).toEqual({ ts: '2026-01-03T00:00:00Z', id: ID_C })
|
||||
// The cursor is the LAST row of the trimmed page (rows[limit - 1]), never
|
||||
// rows[limit]: routes paginate with strictly-greater/less predicates, so
|
||||
// encoding the first row of the next page would skip it at the boundary.
|
||||
expect(decodeDefaultCursor(cursor)).toEqual({ ts: '2026-01-02T00:00:00Z', id: ID_B })
|
||||
})
|
||||
|
||||
it('with limit=1 the cursor is the single returned row, so the next page starts at row 2', () => {
|
||||
const rows = [
|
||||
row(ID_A, '2026-01-01T00:00:00Z'),
|
||||
row(ID_B, '2026-01-02T00:00:00Z'),
|
||||
]
|
||||
const cursor = nextCursorFromPage(rows, 1)
|
||||
expect(decodeDefaultCursor(cursor)).toEqual({ ts: '2026-01-01T00:00:00Z', id: ID_A })
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -0,0 +1,472 @@
|
||||
/**
|
||||
* Regression suite for the documented v1 "preview, then commit" flow.
|
||||
*
|
||||
* The contract in `lib/api/v1/dry-run.ts` tells integrators to commit by
|
||||
* re-issuing the SAME request without `dry_run=true`, carrying the SAME
|
||||
* `Idempotency-Key`. That made the preview and the commit indistinguishable to
|
||||
* the idempotency layer (same method, same path, same body): the preview was
|
||||
* cached under the commit's hash and the commit replayed it. 200 OK, header
|
||||
* `Idempotent-Replayed: true`, `{ dry_run: true, preview }` in the body, and
|
||||
* nothing written. An agent reading the status code reported success.
|
||||
*
|
||||
* These tests exercise the wrapper against an in-memory stand-in for the
|
||||
* `idempotency_keys` table, because the bug only appears when a real store
|
||||
* carries state from one request to the next: a per-call `vi.fn()` that always
|
||||
* returns null can never reproduce it.
|
||||
*
|
||||
* Mocking follows `tests/helpers.ts` conventions, but the v1 surface never
|
||||
* touches `@/lib/supabase/server`: it is API-key authenticated and runs on
|
||||
* `createServiceClientNoCookies()` from `@/lib/auth/api-keys`, so that is what
|
||||
* is stubbed here (same as `with-api-v1.test.ts`).
|
||||
*/
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { z } from 'zod'
|
||||
|
||||
beforeAll(() => {
|
||||
// The wrapper's public-scope path fails closed without these; stubbed only
|
||||
// to clear the guard (no Supabase instance is contacted).
|
||||
process.env.NEXT_PUBLIC_SUPABASE_URL ||= 'http://localhost:54321'
|
||||
process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY ||= 'test-anon-key'
|
||||
})
|
||||
|
||||
vi.mock('@/lib/auth/api-keys', async () => {
|
||||
const actual = await vi.importActual<typeof import('@/lib/auth/api-keys')>('@/lib/auth/api-keys')
|
||||
return {
|
||||
...actual,
|
||||
validateApiKey: vi.fn(),
|
||||
createServiceClientNoCookies: vi.fn(),
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@supabase/supabase-js', async () => {
|
||||
const actual = await vi.importActual<typeof import('@supabase/supabase-js')>('@supabase/supabase-js')
|
||||
return {
|
||||
...actual,
|
||||
createClient: vi.fn().mockReturnValue({}),
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@/lib/api/idempotency', async () => {
|
||||
const actual = await vi.importActual<typeof import('@/lib/api/idempotency')>(
|
||||
'@/lib/api/idempotency',
|
||||
)
|
||||
return {
|
||||
...actual,
|
||||
checkIdempotencyKey: vi.fn(),
|
||||
storeIdempotencyResponse: vi.fn(),
|
||||
}
|
||||
})
|
||||
|
||||
import { createServiceClientNoCookies, validateApiKey } from '@/lib/auth/api-keys'
|
||||
import {
|
||||
checkIdempotencyKey,
|
||||
IdempotencyKeyReuseError,
|
||||
storeIdempotencyResponse,
|
||||
} from '@/lib/api/idempotency'
|
||||
import { withApiV1 } from '../with-api-v1'
|
||||
import { dryRunPreview } from '../dry-run'
|
||||
import { created } from '../response'
|
||||
import { registerEndpoint } from '../registry'
|
||||
|
||||
const mockValidate = validateApiKey as ReturnType<typeof vi.fn>
|
||||
const mockServiceClient = createServiceClientNoCookies as ReturnType<typeof vi.fn>
|
||||
const mockCheckIdempotency = checkIdempotencyKey as ReturnType<typeof vi.fn>
|
||||
const mockStoreIdempotency = storeIdempotencyResponse as ReturnType<typeof vi.fn>
|
||||
|
||||
const COMPANY_ID = 'company-1'
|
||||
const USER_ID = 'user-1'
|
||||
const INVOICES_URL = `https://x.test/api/v1/companies/${COMPANY_ID}/invoices`
|
||||
const REQUEST_BODY = { customer_id: 'cust-1', amount: 1250 }
|
||||
|
||||
// The wrapper reads `dryRunSupported` off the registry when a TEST key writes.
|
||||
// Registering the pattern here lets the forced-dry-run path run instead of
|
||||
// short-circuiting to TEST_KEY_WRITE_BLOCKED. Vitest isolates module state per
|
||||
// test file, so this registration is invisible to the rest of the suite.
|
||||
registerEndpoint({
|
||||
operation: 'invoices.create',
|
||||
method: 'POST',
|
||||
path: '/api/v1/companies/:companyId/invoices',
|
||||
summary: 'Create an invoice (test fixture).',
|
||||
description: 'Fixture registration used by the dry-run idempotency tests.',
|
||||
useWhen: 'never: test fixture',
|
||||
doNotUseFor: 'anything outside this test file',
|
||||
pitfalls: [],
|
||||
example: { response: {} },
|
||||
scope: 'invoices:write',
|
||||
risk: 'medium',
|
||||
idempotent: false,
|
||||
reversible: true,
|
||||
dryRunSupported: true,
|
||||
response: { success: z.object({}) },
|
||||
})
|
||||
|
||||
function makeSupabaseStub(membership: { company_id: string; role: string } | null) {
|
||||
return {
|
||||
from: vi.fn().mockReturnValue({
|
||||
select: vi.fn().mockReturnValue({
|
||||
eq: vi.fn().mockReturnValue({
|
||||
eq: vi.fn().mockReturnValue({
|
||||
maybeSingle: vi.fn().mockResolvedValue({ data: membership, error: null }),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
function keyAuth(mode: 'live' | 'test') {
|
||||
return {
|
||||
userId: USER_ID,
|
||||
companyId: COMPANY_ID,
|
||||
apiKeyId: mode === 'test' ? 'ak_test' : 'ak_live',
|
||||
apiKeyName: `${mode} key`,
|
||||
scopes: ['invoices:write'],
|
||||
mode,
|
||||
}
|
||||
}
|
||||
|
||||
interface StoredRow {
|
||||
requestHash: string
|
||||
status: 'success' | 'error'
|
||||
body: Record<string, unknown>
|
||||
}
|
||||
|
||||
/**
|
||||
* In-memory stand-in for the `idempotency_keys` table. Reproduces the three
|
||||
* behaviours the wrapper depends on:
|
||||
* - rows are scoped by (user_id, company_id, key)
|
||||
* - a stored row whose request_hash differs raises IdempotencyKeyReuseError
|
||||
* - insert is first-writer-wins (the unique index swallows the loser)
|
||||
*/
|
||||
function installIdempotencyTable(): Map<string, StoredRow> {
|
||||
const rows = new Map<string, StoredRow>()
|
||||
const rowKey = (userId: string, companyId: string, key: string) =>
|
||||
`${userId}::${companyId}::${key}`
|
||||
|
||||
mockCheckIdempotency.mockImplementation(
|
||||
async (
|
||||
_supabase: unknown,
|
||||
userId: string,
|
||||
companyId: string,
|
||||
key: string,
|
||||
requestHash: string,
|
||||
) => {
|
||||
const row = rows.get(rowKey(userId, companyId, key))
|
||||
if (!row) return null
|
||||
if (row.requestHash !== requestHash) throw new IdempotencyKeyReuseError(key)
|
||||
return { status: row.status, body: row.body }
|
||||
},
|
||||
)
|
||||
|
||||
mockStoreIdempotency.mockImplementation(
|
||||
async (
|
||||
_supabase: unknown,
|
||||
userId: string,
|
||||
companyId: string,
|
||||
key: string,
|
||||
requestHash: string,
|
||||
status: 'success' | 'error',
|
||||
body: Record<string, unknown>,
|
||||
) => {
|
||||
const k = rowKey(userId, companyId, key)
|
||||
if (rows.has(k)) return
|
||||
rows.set(k, { requestHash, status, body })
|
||||
},
|
||||
)
|
||||
|
||||
return rows
|
||||
}
|
||||
|
||||
/**
|
||||
* A write route shaped like the real ones: previews when `ctx.dryRun`, and
|
||||
* otherwise performs the side-effect. `committed` is the observable proof that
|
||||
* the write actually happened, which is the whole point of this suite.
|
||||
*/
|
||||
function makeInvoiceRoute() {
|
||||
const committed: string[] = []
|
||||
const previews: string[] = []
|
||||
let seq = 0
|
||||
|
||||
const route = withApiV1<{ params: Promise<{ companyId: string }> }>(
|
||||
'invoices.create',
|
||||
async (request, ctx) => {
|
||||
const body = (await request.json()) as { customer_id: string }
|
||||
if (ctx.dryRun) {
|
||||
previews.push(body.customer_id)
|
||||
return dryRunPreview(
|
||||
{ customer_id: body.customer_id },
|
||||
{ requestId: ctx.requestId, log: ctx.log },
|
||||
)
|
||||
}
|
||||
seq += 1
|
||||
const id = `inv-${seq}`
|
||||
committed.push(id)
|
||||
return created({ id, customer_id: body.customer_id }, { requestId: ctx.requestId })
|
||||
},
|
||||
{ requireScope: 'invoices:write' },
|
||||
)
|
||||
|
||||
return { route, committed, previews }
|
||||
}
|
||||
|
||||
function companyParams(companyId: string) {
|
||||
return { params: Promise.resolve({ companyId }) }
|
||||
}
|
||||
|
||||
function postInvoice(opts: {
|
||||
key?: string
|
||||
dryRunQuery?: boolean
|
||||
/** Raw value for the ?dry_run= query param (case-sensitivity matrix). */
|
||||
dryRunQueryValue?: string
|
||||
dryRunHeader?: boolean
|
||||
}): Request {
|
||||
const headers: Record<string, string> = {
|
||||
Authorization: 'Bearer gnubok_sk_x',
|
||||
'Content-Type': 'application/json',
|
||||
}
|
||||
if (opts.key) headers['Idempotency-Key'] = opts.key
|
||||
if (opts.dryRunHeader) headers['X-Dry-Run'] = 'true'
|
||||
const url =
|
||||
opts.dryRunQueryValue !== undefined
|
||||
? `${INVOICES_URL}?dry_run=${opts.dryRunQueryValue}`
|
||||
: opts.dryRunQuery
|
||||
? `${INVOICES_URL}?dry_run=true`
|
||||
: INVOICES_URL
|
||||
return new Request(url, {
|
||||
method: 'POST',
|
||||
headers,
|
||||
body: JSON.stringify(REQUEST_BODY),
|
||||
})
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockServiceClient.mockReturnValue(makeSupabaseStub({ company_id: COMPANY_ID, role: 'owner' }))
|
||||
mockValidate.mockResolvedValue(keyAuth('live'))
|
||||
installIdempotencyTable()
|
||||
})
|
||||
|
||||
describe('withApiV1: dry-run then commit on the same Idempotency-Key', () => {
|
||||
// THE missing test. Before the fix this returned the cached preview with
|
||||
// Idempotent-Replayed: true and `committed` stayed empty.
|
||||
it('commits for real after a dry-run preview issued under the same key', async () => {
|
||||
const { route, committed, previews } = makeInvoiceRoute()
|
||||
|
||||
const preview = await route(
|
||||
postInvoice({ key: 'key-1', dryRunQuery: true }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
expect(preview.status).toBe(200)
|
||||
expect(preview.headers.get('X-Dry-Run')).toBe('true')
|
||||
expect(preview.headers.get('Idempotent-Replayed')).toBeNull()
|
||||
const previewBody = await preview.json()
|
||||
expect(previewBody.data.dry_run).toBe(true)
|
||||
expect(previews).toEqual(['cust-1'])
|
||||
expect(committed).toEqual([])
|
||||
|
||||
const commit = await route(postInvoice({ key: 'key-1' }), companyParams(COMPANY_ID))
|
||||
|
||||
expect(commit.status).toBe(201)
|
||||
expect(commit.headers.get('Idempotent-Replayed')).toBeNull()
|
||||
const commitBody = await commit.json()
|
||||
expect(commitBody.data.id).toBe('inv-1')
|
||||
expect(commitBody.data.dry_run).toBeUndefined()
|
||||
expect(committed).toEqual(['inv-1'])
|
||||
})
|
||||
|
||||
it('never writes the dry-run response into the idempotency cache', async () => {
|
||||
const { route } = makeInvoiceRoute()
|
||||
|
||||
await route(postInvoice({ key: 'key-2', dryRunQuery: true }), companyParams(COMPANY_ID))
|
||||
|
||||
expect(mockStoreIdempotency).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not replay a repeated dry-run: each preview re-runs the handler', async () => {
|
||||
const { route, committed, previews } = makeInvoiceRoute()
|
||||
|
||||
const first = await route(
|
||||
postInvoice({ key: 'key-3', dryRunQuery: true }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
const second = await route(
|
||||
postInvoice({ key: 'key-3', dryRunQuery: true }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
|
||||
expect(first.headers.get('Idempotent-Replayed')).toBeNull()
|
||||
expect(second.headers.get('Idempotent-Replayed')).toBeNull()
|
||||
expect(second.status).toBe(200)
|
||||
expect((await second.json()).data.dry_run).toBe(true)
|
||||
expect(previews).toHaveLength(2)
|
||||
expect(committed).toEqual([])
|
||||
})
|
||||
|
||||
// The dry-run flag is part of the request hash, so a key that already
|
||||
// committed for real cannot be re-used for a simulation: that would hand back
|
||||
// a committed result wearing a preview's clothes. 409 is the honest answer.
|
||||
it('rejects a dry-run that re-uses a key which already committed', async () => {
|
||||
const { route, committed } = makeInvoiceRoute()
|
||||
|
||||
await route(postInvoice({ key: 'key-4' }), companyParams(COMPANY_ID))
|
||||
expect(committed).toEqual(['inv-1'])
|
||||
|
||||
const res = await route(
|
||||
postInvoice({ key: 'key-4', dryRunQuery: true }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
|
||||
expect(res.status).toBe(409)
|
||||
const body = await res.json()
|
||||
expect(body.error.code).toBe('IDEMPOTENCY_KEY_REUSE')
|
||||
expect(committed).toEqual(['inv-1'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('withApiV1: idempotent replay of real commits (must not regress)', () => {
|
||||
// Also the lockstep guard for the two hashRequest call sites: the lookup only
|
||||
// finds what the store wrote if both hash the same input. Diverge them and
|
||||
// this test fails with a second write instead of a replay.
|
||||
it('replays the second of two identical commits under the same key', async () => {
|
||||
const { route, committed } = makeInvoiceRoute()
|
||||
|
||||
const first = await route(postInvoice({ key: 'key-5' }), companyParams(COMPANY_ID))
|
||||
const second = await route(postInvoice({ key: 'key-5' }), companyParams(COMPANY_ID))
|
||||
|
||||
expect(first.status).toBe(201)
|
||||
expect(first.headers.get('Idempotent-Replayed')).toBeNull()
|
||||
expect(second.headers.get('Idempotent-Replayed')).toBe('true')
|
||||
expect((await second.json()).data.id).toBe('inv-1')
|
||||
expect(committed).toEqual(['inv-1'])
|
||||
})
|
||||
|
||||
it('still rejects the same key carrying a different body', async () => {
|
||||
const { route, committed } = makeInvoiceRoute()
|
||||
|
||||
await route(postInvoice({ key: 'key-6' }), companyParams(COMPANY_ID))
|
||||
|
||||
const changed = new Request(INVOICES_URL, {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
Authorization: 'Bearer gnubok_sk_x',
|
||||
'Content-Type': 'application/json',
|
||||
'Idempotency-Key': 'key-6',
|
||||
},
|
||||
body: JSON.stringify({ ...REQUEST_BODY, amount: 9999 }),
|
||||
})
|
||||
const res = await route(changed, companyParams(COMPANY_ID))
|
||||
|
||||
expect(res.status).toBe(409)
|
||||
expect((await res.json()).error.code).toBe('IDEMPOTENCY_KEY_REUSE')
|
||||
expect(committed).toEqual(['inv-1'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('withApiV1: dry-run query flag is case-insensitive', () => {
|
||||
// '?dry_run=True' used to fall through the exact-match check and COMMIT for
|
||||
// real while the caller believed it previewed: the one direction this flag
|
||||
// must never fail in.
|
||||
it('previews on ?dry_run=True (mis-cased flag must never commit)', async () => {
|
||||
const { route, committed, previews } = makeInvoiceRoute()
|
||||
|
||||
const res = await route(
|
||||
postInvoice({ key: 'key-10', dryRunQueryValue: 'True' }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
expect((await res.json()).data.dry_run).toBe(true)
|
||||
expect(previews).toHaveLength(1)
|
||||
expect(committed).toEqual([])
|
||||
expect(mockStoreIdempotency).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('previews on ?dry_run=TRUE as well', async () => {
|
||||
const { route, committed } = makeInvoiceRoute()
|
||||
|
||||
const res = await route(
|
||||
postInvoice({ key: 'key-11', dryRunQueryValue: 'TRUE' }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
expect((await res.json()).data.dry_run).toBe(true)
|
||||
expect(committed).toEqual([])
|
||||
})
|
||||
|
||||
it('still commits on non-true values (?dry_run=1, ?dry_run=false)', async () => {
|
||||
const { route, committed, previews } = makeInvoiceRoute()
|
||||
|
||||
const one = await route(
|
||||
postInvoice({ key: 'key-12', dryRunQueryValue: '1' }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
const falsy = await route(
|
||||
postInvoice({ key: 'key-13', dryRunQueryValue: 'false' }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
|
||||
expect(one.status).toBe(201)
|
||||
expect(falsy.status).toBe(201)
|
||||
expect(previews).toHaveLength(0)
|
||||
expect(committed).toEqual(['inv-1', 'inv-2'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('withApiV1: X-Dry-Run header parity with ?dry_run=true', () => {
|
||||
it('previews on the header and still commits on the follow-up request', async () => {
|
||||
const { route, committed, previews } = makeInvoiceRoute()
|
||||
|
||||
const preview = await route(
|
||||
postInvoice({ key: 'key-7', dryRunHeader: true }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
expect(preview.status).toBe(200)
|
||||
expect((await preview.json()).data.dry_run).toBe(true)
|
||||
expect(previews).toHaveLength(1)
|
||||
expect(committed).toEqual([])
|
||||
expect(mockStoreIdempotency).not.toHaveBeenCalled()
|
||||
|
||||
const commit = await route(postInvoice({ key: 'key-7' }), companyParams(COMPANY_ID))
|
||||
|
||||
expect(commit.status).toBe(201)
|
||||
expect(commit.headers.get('Idempotent-Replayed')).toBeNull()
|
||||
expect((await commit.json()).data.id).toBe('inv-1')
|
||||
expect(committed).toEqual(['inv-1'])
|
||||
})
|
||||
|
||||
it('does not replay a repeated header-driven dry-run', async () => {
|
||||
const { route, previews } = makeInvoiceRoute()
|
||||
|
||||
await route(postInvoice({ key: 'key-8', dryRunHeader: true }), companyParams(COMPANY_ID))
|
||||
const second = await route(
|
||||
postInvoice({ key: 'key-8', dryRunHeader: true }),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
|
||||
expect(second.headers.get('Idempotent-Replayed')).toBeNull()
|
||||
expect(previews).toHaveLength(2)
|
||||
})
|
||||
})
|
||||
|
||||
describe('withApiV1: test keys (forced dry-run) never poison the cache', () => {
|
||||
it('lets a live commit through on a key a test key already simulated', async () => {
|
||||
const { route, committed, previews } = makeInvoiceRoute()
|
||||
|
||||
mockValidate.mockResolvedValueOnce(keyAuth('test'))
|
||||
const simulated = await route(postInvoice({ key: 'key-9' }), companyParams(COMPANY_ID))
|
||||
|
||||
expect(simulated.status).toBe(200)
|
||||
expect(simulated.headers.get('X-Gnubok-Mode')).toBe('test')
|
||||
expect((await simulated.json()).data.dry_run).toBe(true)
|
||||
expect(previews).toHaveLength(1)
|
||||
expect(committed).toEqual([])
|
||||
expect(mockStoreIdempotency).not.toHaveBeenCalled()
|
||||
|
||||
const commit = await route(postInvoice({ key: 'key-9' }), companyParams(COMPANY_ID))
|
||||
|
||||
expect(commit.status).toBe(201)
|
||||
expect(commit.headers.get('Idempotent-Replayed')).toBeNull()
|
||||
expect(committed).toEqual(['inv-1'])
|
||||
})
|
||||
})
|
||||
@@ -9,7 +9,13 @@
|
||||
* normal success status (201, 204, etc.). A caller that sees `200`
|
||||
* with `X-Dry-Run` knows the write was NOT committed.
|
||||
* 3. Commit by re-issuing the same request without `dry_run=true`, passing
|
||||
* the same `Idempotency-Key` to guarantee at-most-once semantics.
|
||||
* the same `Idempotency-Key` to guarantee at-most-once semantics. The
|
||||
* wrapper keeps the two apart: a dry-run response is never written to the
|
||||
* idempotency cache, and the dry-run flag is folded into the request hash,
|
||||
* so the commit executes for real instead of replaying the preview.
|
||||
* Order matters. Once a key has committed for real, re-issuing it WITH
|
||||
* `dry_run=true` is rejected as key reuse (409 IDEMPOTENCY_KEY_REUSE)
|
||||
* rather than answered with the committed result dressed up as a preview.
|
||||
*
|
||||
* Two preview shapes are supported:
|
||||
*
|
||||
|
||||
@@ -111,14 +111,19 @@ export function decodeDefaultCursor(cursor: string | null | undefined): DefaultC
|
||||
* cursor for the *next* page (or null when this was the final page).
|
||||
*
|
||||
* Convention: the caller fetches `limit + 1` rows, passes the full slice in,
|
||||
* and we return either the cursor of row[limit] or null when the page wasn't
|
||||
* full. The caller should then trim the slice to `limit` before returning it
|
||||
* to the user.
|
||||
* and we return either the cursor of the LAST ROW OF THE TRIMMED PAGE
|
||||
* (rows[limit - 1]) or null when the page wasn't full. The caller should then
|
||||
* trim the slice to `limit` before returning it to the user.
|
||||
*
|
||||
* Contract: the cursor marks the last row already returned; every v1 route's
|
||||
* keyset predicate is strictly greater/less than the cursor, so encoding
|
||||
* rows[limit] (the first row of the NEXT page) would skip that row at every
|
||||
* page boundary.
|
||||
*/
|
||||
export function nextCursorFromPage<T extends { created_at: string; id: string }>(
|
||||
rows: T[],
|
||||
limit: number,
|
||||
): string | null {
|
||||
if (rows.length <= limit) return null
|
||||
return encodeDefaultCursor(rows[limit])
|
||||
return encodeDefaultCursor(rows[limit - 1])
|
||||
}
|
||||
|
||||
+67
-10
@@ -14,8 +14,11 @@
|
||||
* 4. When the URL contains `companyId`, verifies the API key's user has
|
||||
* access to that company via `company_members`. Multi-company keys are
|
||||
* supported transparently: the URL is the source of truth.
|
||||
* 5. Resolves `Idempotency-Key` (header) and replays cached responses.
|
||||
* 6. Resolves the dry-run flag (`?dry_run=true` query OR `X-Dry-Run` header).
|
||||
* 5. Resolves the dry-run flag (`?dry_run=true` query OR `X-Dry-Run` header).
|
||||
* 6. Resolves `Idempotency-Key` (header) and replays cached responses. The
|
||||
* dry-run flag is part of the cache identity and dry-run responses are
|
||||
* never cached, so a simulation can never be replayed in place of the
|
||||
* real write that follows it.
|
||||
* 7. Invokes the handler with a typed RouteContext.
|
||||
* 8. Stamps `X-Request-Id`, `Gnubok-Version`, `X-RateLimit-Limit` on the
|
||||
* response.
|
||||
@@ -198,12 +201,53 @@ function extractForensicContext(request: Request, log: Logger): { ip: string | u
|
||||
}
|
||||
|
||||
function isDryRun(request: Request, url: URL): boolean {
|
||||
if (url.searchParams.get('dry_run') === 'true') return true
|
||||
// Case-insensitive on BOTH surfaces. The header was always lowercased, but
|
||||
// the query flag used to require exactly 'true', so '?dry_run=True'
|
||||
// committed for real while the caller believed it previewed: the worst
|
||||
// possible parse of a preview flag. Any other value ('1', 'yes', 'false')
|
||||
// stays non-dry-run, unchanged.
|
||||
const queryVal = url.searchParams.get('dry_run')
|
||||
if (queryVal !== null && queryVal.toLowerCase() === 'true') return true
|
||||
const headerVal = request.headers.get(DRY_RUN_HEADER)
|
||||
if (headerVal && headerVal.toLowerCase() === 'true') return true
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* Canonical idempotency hash for a v1 request.
|
||||
*
|
||||
* `dryRun` is part of the identity of the request. The documented commit flow
|
||||
* (see `dry-run.ts`) is "re-issue the exact same request without
|
||||
* `dry_run=true`, same Idempotency-Key", so a simulation and the real write
|
||||
* that follows it share method, path and body. Hashing only those three made
|
||||
* the two indistinguishable: the preview got cached under the commit's hash
|
||||
* and the commit replayed it, returning 200 with `{ dry_run: true, preview }`
|
||||
* while writing nothing.
|
||||
*
|
||||
* The flag is only added to the hashed object when it is TRUE, so an ordinary
|
||||
* (non-dry-run) write hashes byte-identically to previous releases. Idempotency
|
||||
* rows live for 24h; folding `dry_run: false` in unconditionally would make
|
||||
* every key in flight across this deploy fail the `request_hash` comparison and
|
||||
* answer a legitimate retry with IDEMPOTENCY_KEY_REUSE.
|
||||
*
|
||||
* Both the cache lookup and the cache store go through this function: if the
|
||||
* two hash inputs ever drift apart, a stored response can never be found
|
||||
* again, which is a subtler failure than the one this fixes.
|
||||
*/
|
||||
function buildRequestHash(input: {
|
||||
method: string
|
||||
path: string
|
||||
body: unknown
|
||||
dryRun: boolean
|
||||
}): string {
|
||||
return hashRequest({
|
||||
method: input.method,
|
||||
path: input.path,
|
||||
body: input.body,
|
||||
...(input.dryRun ? { dry_run: true } : {}),
|
||||
})
|
||||
}
|
||||
|
||||
async function readBodyForHash(request: Request): Promise<{ body: unknown; cloned: Request }> {
|
||||
// We need the body to hash it, but the handler also needs it. Read from a
|
||||
// CLONE for the hash and pass the original through to the handler: that
|
||||
@@ -406,14 +450,20 @@ export function withApiV1<P extends DynamicParams = { params: Promise<Record<str
|
||||
})
|
||||
}
|
||||
|
||||
// 7. If idempotency-key supplied, check for cached response.
|
||||
// 7. Dry-run resolution. Test keys force it on regardless of the flag.
|
||||
// Resolved BEFORE the idempotency lookup because it feeds the request
|
||||
// hash: a preview and its follow-up commit must never share a cache
|
||||
// entry.
|
||||
const dryRun = isDryRun(request, url) || forceDryRun
|
||||
|
||||
// 8. If idempotency-key supplied, check for cached response.
|
||||
let bodyForHash: unknown = null
|
||||
let workingRequest = request
|
||||
if (idempotencyKey && isMutation && companyId) {
|
||||
const { body, cloned } = await readBodyForHash(request)
|
||||
bodyForHash = body
|
||||
workingRequest = cloned
|
||||
const reqHash = hashRequest({ method: request.method, path, body })
|
||||
const reqHash = buildRequestHash({ method: request.method, path, body, dryRun })
|
||||
try {
|
||||
const hit = await checkIdempotencyKey(supabase, auth.userId, companyId, idempotencyKey, reqHash)
|
||||
if (hit) {
|
||||
@@ -434,9 +484,6 @@ export function withApiV1<P extends DynamicParams = { params: Promise<Record<str
|
||||
}
|
||||
}
|
||||
|
||||
// 8. Dry-run resolution. Test keys force it on regardless of the flag.
|
||||
const dryRun = isDryRun(workingRequest, url) || forceDryRun
|
||||
|
||||
const ctx: ApiV1Context = {
|
||||
requestId,
|
||||
log: userLog.child({ companyId }),
|
||||
@@ -461,10 +508,20 @@ export function withApiV1<P extends DynamicParams = { params: Promise<Record<str
|
||||
}
|
||||
|
||||
// 10. Persist idempotency cache (best-effort).
|
||||
if (idempotencyKey && isMutation && companyId && response.status < 500) {
|
||||
//
|
||||
// Never cache a dry-run: the response describes a write that did not
|
||||
// happen, and caching it under a real Idempotency-Key is exactly how
|
||||
// the documented "preview, then commit with the same key" flow used
|
||||
// to lose the commit. A simulation has nothing worth replaying.
|
||||
if (idempotencyKey && isMutation && companyId && !dryRun && response.status < 500) {
|
||||
try {
|
||||
const body = await response.clone().json().catch(() => ({}))
|
||||
const reqHash = hashRequest({ method: request.method, path, body: bodyForHash })
|
||||
const reqHash = buildRequestHash({
|
||||
method: request.method,
|
||||
path,
|
||||
body: bodyForHash,
|
||||
dryRun,
|
||||
})
|
||||
const status: 'success' | 'error' = response.status >= 400 ? 'error' : 'success'
|
||||
await storeIdempotencyResponse(
|
||||
supabase,
|
||||
|
||||
+32
-1
@@ -21,6 +21,37 @@ interface ValidationOptions {
|
||||
operation?: string
|
||||
}
|
||||
|
||||
/** How many field issues the human-readable `error` summary names before truncating. */
|
||||
const SUMMARY_ISSUE_LIMIT = 3
|
||||
|
||||
/**
|
||||
* Build the human-readable `error` string for a Zod validation failure.
|
||||
*
|
||||
* `errors[]` keeps the full machine-readable detail, but plenty of clients read
|
||||
* only `error`. A constant there ('Validation failed') either reached the user
|
||||
* as English boilerplate in a Swedish UI, or got swallowed by
|
||||
* `getErrorMessage()`'s generic "Något gick fel" fallback because the constant
|
||||
* matches nothing it knows. The 'Valideringsfel' lead-in is load-bearing: it is
|
||||
* what makes `getErrorMessage()` recognize the sentence as an already-Swedish
|
||||
* user message and pass it through verbatim, so clients that forward
|
||||
* `body.error` alone still show the actionable field message.
|
||||
*
|
||||
* `type: 'validation_error'` remains the machine-readable discriminator; nothing
|
||||
* should branch on this prose.
|
||||
*/
|
||||
function summarizeIssues(errors: Array<{ field: string; message: string }>): string {
|
||||
const shown = errors
|
||||
.slice(0, SUMMARY_ISSUE_LIMIT)
|
||||
.map((issue) => (issue.field ? `${issue.field}: ${issue.message}` : issue.message))
|
||||
.filter((text) => text.trim() !== '')
|
||||
|
||||
if (shown.length === 0) return 'Valideringsfel: kontrollera fälten och försök igen.'
|
||||
|
||||
const hidden = errors.length - SUMMARY_ISSUE_LIMIT
|
||||
const more = hidden > 0 ? ` (+${hidden} till)` : ''
|
||||
return `Valideringsfel: ${shown.join('. ')}${more}`
|
||||
}
|
||||
|
||||
function logIssues(
|
||||
options: ValidationOptions | undefined,
|
||||
kind: 'body' | 'query' | 'json',
|
||||
@@ -86,7 +117,7 @@ export async function validateBody<T>(
|
||||
success: false,
|
||||
response: NextResponse.json(
|
||||
{
|
||||
error: 'Validation failed',
|
||||
error: summarizeIssues(errors),
|
||||
type: 'validation_error',
|
||||
errors,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user