fix(salary): stop RLS from failing vab/parental absence registration (#1568)

Migration 20260517135000 rewrote the franvaro-specifikationsnummer trigger
functions to insert audit rows into salary_absence_franvaro_audit, a table
with RLS enabled and zero policies, while leaving the functions SECURITY
INVOKER (its comment claimed implicit SECURITY DEFINER, which is false in
Postgres). Every vab/parental insert from role authenticated (dashboard
absence POST, web /pending approval, in-app Assistenten chat) then failed
with 42501, surfaced as a generic 500, and left no diagnosable trace.

- New migration 20260813120000: ALTER both trigger functions to SECURITY
  DEFINER with search_path pinned to public, pg_temp. No RLS policy is added
  on the audit table: trigger/service-only writes stay the design intent.
- mapInsertError: 42501 now maps to the new bilingual DB_PERMISSION_DENIED
  code instead of INTERNAL_ERROR, and 23514 is split so only the 24h-cap
  trigger's 'Total tid' message becomes ABSENCE_HOURS_CONFLICT; other CHECK
  violations map to VALIDATION_ERROR.
- commitRegisterAbsence/commitDeleteAbsence: log the underlying PG details
  and persist the sanitized structured code in result_data.error_code so the
  next failure is traceable from the op row.
- Dashboard absence route: only ABSENCE_HOURS_CONFLICT passes details.message
  through to the client; every other code shows the registry Swedish message
  instead of raw Postgres text.
- New pg-real regression test locks the authenticated-role parental/vab
  insert path, the shared per-month specnummer sequence, the audit rows, and
  idempotent upsert retries.

Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-08-13 15:12:32 +02:00
committed by GitHub
parent ebeaeec80e
commit c4adc8eb7d
9 changed files with 466 additions and 4 deletions
@@ -82,4 +82,43 @@ describe('POST /api/salary/employees/[id]/absence', () => {
const response = await POST(post({ absence_date: '2026-07-01', absence_type: 'sick', hours: 8 }), params)
expect(response.status).toBe(404)
})
it('does not leak raw PG text for DB failures (42501)', async () => {
enqueue({ data: { id: 'emp-1' } }) // loadEmployee
enqueue({
data: null,
error: {
code: '42501',
message:
'new row violates row-level security policy for table "salary_absence_franvaro_audit"',
},
}) // upsert denied
const response = await POST(post({ absence_date: '2026-07-01', absence_type: 'parental', hours: 8 }), params)
const { status, body } = await parseJsonResponse<{ error: string; code: string }>(response)
expect(status).toBe(500)
expect(body.code).toBe('DB_PERMISSION_DENIED')
expect(body.error).not.toMatch(/row-level security/)
// The registry's Swedish message is shown instead.
expect(body.error).toContain('behörighetsfel')
})
it('still passes the 24h-cap trigger detail through (Swedish, user-facing)', async () => {
enqueue({ data: { id: 'emp-1' } }) // loadEmployee
enqueue({
data: null,
error: {
code: '23514',
message: 'Total tid (arbete + frånvaro) för 2026-07-01 får inte överstiga 24 timmar',
},
}) // 24h cap trips
const response = await POST(post({ absence_date: '2026-07-01', absence_type: 'sick', hours: 20 }), params)
const { status, body } = await parseJsonResponse<{ error: string; code: string }>(response)
expect(status).toBe(409)
expect(body.code).toBe('ABSENCE_HOURS_CONFLICT')
expect(body.error).toContain('Total tid')
})
})
@@ -21,8 +21,13 @@ ensureInitialized()
function errorResponse(code: string, details?: Record<string, unknown>): NextResponse {
const entry = getErrorEntry(code)
// Only the 24h-cap trigger's Swedish text is user-facing detail; for every
// other code details.message is raw Postgres text and must not reach the
// client (the registry message is shown instead).
const message =
(details?.message as string | undefined) ?? entry?.message_sv ?? 'Något gick fel'
(code === 'ABSENCE_HOURS_CONFLICT' ? (details?.message as string | undefined) : undefined) ??
entry?.message_sv ??
'Något gick fel'
return NextResponse.json({ error: message, code }, { status: entry?.httpStatus ?? 500 })
}
+10
View File
@@ -81,6 +81,16 @@ const GENERIC: Record<string, StructuredErrorEntry> = {
message_sv: 'Du har inte behörighet att utföra denna åtgärd.',
message_en: 'Insufficient permissions.',
},
// A Postgres privilege/RLS denial (42501) on a write the application
// expected to succeed: a server-side configuration bug (e.g. a SECURITY
// INVOKER trigger writing to a policy-less RLS table), not a user-permission
// problem. Kept distinct from FORBIDDEN (which blames the user) and from
// INTERNAL_ERROR (which hides the failure mode from diagnostics).
DB_PERMISSION_DENIED: {
httpStatus: 500,
message_sv: 'Ett behörighetsfel i databasen stoppade åtgärden. Kontakta supporten om felet kvarstår.',
message_en: 'A database permission (RLS) denial blocked the write. This indicates a server-side misconfiguration.',
},
NOT_FOUND: {
httpStatus: 404,
message_sv: 'Resursen kunde inte hittas.',
@@ -184,6 +184,49 @@ describe('commitPendingOperation: register_absence', () => {
expect(result.status).toBe('rejected')
expect(result.http_status).toBe(409)
})
it('logs the PG details and persists a sanitized error_code when the upsert fails', async () => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
try {
const { supabase, enqueue, findCalls } = createQueuedMockSupabase()
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
enqueue({ data: { id: 'emp-1' } }) // assertEmployee
enqueue({
data: null,
error: {
code: '42501',
message:
'new row violates row-level security policy for table "salary_absence_franvaro_audit"',
},
}) // upsert denied (the franvaro audit-trigger bug)
enqueue({ data: null, error: null }) // finalize (failed)
const op = makePendingOp({
operation_type: 'register_absence',
params: { employee_id: 'emp-1', from: '2026-03-02', to: '2026-03-02', absence_type: 'parental' },
})
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
expect(result.status).toBe('failed')
expect(result.http_status).toBe(500)
expect(result.code).toBe('DB_PERMISSION_DENIED')
// The op row keeps the structured code so the failure mode is
// diagnosable later, but never the raw PG text.
const updates = findCalls('pending_operations', 'update')
const finalize = updates[updates.length - 1]![0] as { result_data?: Record<string, unknown> }
expect(finalize.result_data).toMatchObject({
error_code: 'DB_PERMISSION_DENIED',
http_status: 500,
})
expect(JSON.stringify(finalize.result_data)).not.toContain('row-level security')
// The raw PG message goes to the log: it is persisted nowhere else.
const logged = consoleError.mock.calls.map((c) => c.join(' ')).join('\n')
expect(logged).toContain('register_absence commit failed')
expect(logged).toContain('row-level security')
} finally {
consoleError.mockRestore()
}
})
})
describe('commitPendingOperation: book_salary_run', () => {
@@ -279,6 +322,34 @@ describe('commitPendingOperation: delete_absence', () => {
expect(result.status).not.toBe('committed')
expect(result.error).toBeDefined()
})
it('logs the PG details and persists a sanitized error_code when the delete fails', async () => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
try {
const { supabase, enqueue, findCalls } = createQueuedMockSupabase()
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
enqueue({ data: { id: 'emp-1' } }) // assertEmployee
enqueue({ data: null, error: { code: '57014', message: 'canceling statement due to statement timeout' } }) // delete fails
enqueue({ data: null, error: null }) // finalize (failed)
const op = makePendingOp({
operation_type: 'delete_absence',
params: { employee_id: 'emp-1', from: '2026-03-02', to: '2026-03-06' },
})
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
expect(result.status).toBe('failed')
expect(result.code).toBe('INTERNAL_ERROR')
const updates = findCalls('pending_operations', 'update')
const finalize = updates[updates.length - 1]![0] as { result_data?: Record<string, unknown> }
expect(finalize.result_data).toMatchObject({ error_code: 'INTERNAL_ERROR' })
const logged = consoleError.mock.calls.map((c) => c.join(' ')).join('\n')
expect(logged).toContain('delete_absence commit failed')
expect(logged).toContain('statement timeout')
} finally {
consoleError.mockRestore()
}
})
})
describe('commitPendingOperation: create_employee', () => {
+24
View File
@@ -4982,9 +4982,23 @@ async function commitRegisterAbsence(
includeWeekends: (params.include_weekends as boolean | undefined) ?? false,
})
if (!result.ok) {
// The user-facing message is generic Swedish; without this log the
// underlying PG error (e.g. the franvaro audit-trigger RLS denial that
// caused five untraceable 500s, feedback 2026-08-13) leaves no trace.
// pgDetails, not details: `details` is the logger record's own field
// for non-object args and would be swallowed by the pretty emitter.
createLogger('commit/register_absence').error('register_absence commit failed', {
code: result.code,
pgDetails: result.details,
employeeId,
absenceType,
from,
to,
})
const entry = getErrorEntry(result.code)
return {
error: entry?.message_sv ?? `Kunde inte registrera frånvaron: ${result.code}`,
errorCode: result.code,
status: entry?.httpStatus ?? 500,
}
}
@@ -5081,9 +5095,19 @@ async function commitDeleteAbsence(
absenceType: (params.absence_type as string | undefined) || undefined,
})
if (!result.ok) {
// Same diagnosability treatment as commitRegisterAbsence: keep the PG
// error in the logs and the registry code on the op row.
createLogger('commit/delete_absence').error('delete_absence commit failed', {
code: result.code,
pgDetails: result.details,
employeeId,
from,
to,
})
const entry = getErrorEntry(result.code)
return {
error: entry?.message_sv ?? `Kunde inte ta bort frånvaron: ${result.code}`,
errorCode: result.code,
status: entry?.httpStatus ?? 500,
}
}
+66
View File
@@ -160,6 +160,72 @@ describe('upsertAbsenceRange', () => {
expect(result.ok).toBe(false)
if (!result.ok) expect(result.code).toBe('ABSENCE_HOURS_CONFLICT')
})
it('maps a non-24h CHECK violation (23514) to VALIDATION_ERROR, not ABSENCE_HOURS_CONFLICT', async () => {
mock.enqueue({ data: { id: EMPLOYEE_ID } })
mock.enqueue({
data: null,
error: {
code: '23514',
message:
'new row for relation "salary_absence_days" violates check constraint "salary_absence_days_hours_check"',
},
})
const result = await upsertAbsenceRange(supabase, {
companyId: COMPANY_ID,
employeeId: EMPLOYEE_ID,
from: '2026-03-02',
to: '2026-03-02',
absenceType: 'sick',
hoursPerDay: 30,
})
expect(result.ok).toBe(false)
if (!result.ok) expect(result.code).toBe('VALIDATION_ERROR')
})
it('maps an RLS/privilege denial (42501) to DB_PERMISSION_DENIED with the PG message in details', async () => {
mock.enqueue({ data: { id: EMPLOYEE_ID } })
mock.enqueue({
data: null,
error: {
code: '42501',
message:
'new row violates row-level security policy for table "salary_absence_franvaro_audit"',
},
})
const result = await upsertAbsenceRange(supabase, {
companyId: COMPANY_ID,
employeeId: EMPLOYEE_ID,
from: '2026-03-02',
to: '2026-03-02',
absenceType: 'parental',
})
expect(result.ok).toBe(false)
if (!result.ok) {
expect(result.code).toBe('DB_PERMISSION_DENIED')
expect(result.details?.message).toMatch(/row-level security/)
}
})
it('keeps unrecognized DB errors as INTERNAL_ERROR', async () => {
mock.enqueue({ data: { id: EMPLOYEE_ID } })
mock.enqueue({ data: null, error: { code: '57014', message: 'canceling statement due to statement timeout' } })
const result = await upsertAbsenceRange(supabase, {
companyId: COMPANY_ID,
employeeId: EMPLOYEE_ID,
from: '2026-03-02',
to: '2026-03-02',
absenceType: 'sick',
})
expect(result.ok).toBe(false)
if (!result.ok) expect(result.code).toBe('INTERNAL_ERROR')
})
})
describe('upsertAbsenceDay', () => {
@@ -0,0 +1,202 @@
import { randomUUID } from 'crypto'
import { describe, expect, it } from 'vitest'
import type { PoolClient } from 'pg'
import { seedCompany } from '@/tests/pg/fixtures'
import { getPool, withUserContext } from '@/tests/pg/setup'
/**
* Regression lock for the franvaro-specifikationsnummer trigger under the
* `authenticated` role (feedback 2026-08-13: register_absence 500 for
* foraldraledighet/VAB).
*
* Migration 20260517135000 made the BEFORE INSERT trigger on
* salary_absence_days write an audit row into salary_absence_franvaro_audit,
* a table with RLS enabled and zero policies, from a SECURITY INVOKER
* function. Every vab/parental INSERT from a non-BYPASSRLS role then failed
* with 42501 while 'sick' (which skips the trigger) kept working. Migration
* 20260813120000 makes both trigger functions SECURITY DEFINER with a pinned
* search_path; these tests fail with /row-level security|permission denied/
* without it.
*
* The audit table deliberately has no policies (trigger/service-only
* writes), so audit assertions run on the superuser pool connection after
* RESET ROLE inside the same transaction (withUserContext always rolls back,
* so nothing persists across tests).
*/
async function insertEmployee(params: {
userId: string
companyId: string
}): Promise<string> {
const id = randomUUID()
// personnummer must be 12 digits; last4 mirrors the last four chars.
const pnr = '199001011234'
await getPool().query(
`INSERT INTO public.employees
(id, user_id, company_id, first_name, last_name, personnummer,
personnummer_last4, employment_start, monthly_salary, tax_table_number)
VALUES ($1, $2, $3, 'Test', 'Person', $4, '1234', '2026-01-01', 30000, 32)`,
[id, params.userId, params.companyId, pnr],
)
return id
}
async function insertAbsenceDayAs(
client: PoolClient,
params: {
companyId: string
employeeId: string
date: string
type: string
},
): Promise<{ id: string; specnummer: number | null }> {
const res = await client.query<{ id: string; franvaro_specifikationsnummer: number | null }>(
`INSERT INTO public.salary_absence_days
(company_id, employee_id, absence_date, absence_type, hours)
VALUES ($1, $2, $3, $4, 8)
RETURNING id, franvaro_specifikationsnummer`,
[params.companyId, params.employeeId, params.date, params.type],
)
return {
id: res.rows[0]!.id,
specnummer: res.rows[0]!.franvaro_specifikationsnummer,
}
}
interface AuditRow {
absence_day_id: string
year_month: string
new_specifikationsnummer: number
trigger_op: string
}
/** Read the audit table as superuser (no policies exist by design). */
async function readAudit(client: PoolClient, employeeId: string): Promise<AuditRow[]> {
await client.query('RESET ROLE')
const res = await client.query<AuditRow>(
`SELECT absence_day_id, year_month, new_specifikationsnummer, trigger_op
FROM public.salary_absence_franvaro_audit
WHERE employee_id = $1
ORDER BY assigned_at, new_specifikationsnummer`,
[employeeId],
)
return res.rows
}
describe('franvaro-specnummer.pg: authenticated-role vab/parental inserts', () => {
it('parental INSERT succeeds under role authenticated and mints the shared per-month sequence + audit rows', async () => {
const a = await seedCompany()
const emp = await insertEmployee({ userId: a.userId, companyId: a.companyId })
await withUserContext(a.userId, async (client) => {
const day1 = await insertAbsenceDayAs(client, {
companyId: a.companyId,
employeeId: emp,
date: '2026-03-02',
type: 'parental',
})
expect(day1.specnummer).toBe(1)
const day2 = await insertAbsenceDayAs(client, {
companyId: a.companyId,
employeeId: emp,
date: '2026-03-03',
type: 'parental',
})
expect(day2.specnummer).toBe(2)
// vab shares the same per-(employee, year-month) sequence.
const vabDay = await insertAbsenceDayAs(client, {
companyId: a.companyId,
employeeId: emp,
date: '2026-03-04',
type: 'vab',
})
expect(vabDay.specnummer).toBe(3)
// A different month restarts the sequence.
const aprilDay = await insertAbsenceDayAs(client, {
companyId: a.companyId,
employeeId: emp,
date: '2026-04-01',
type: 'parental',
})
expect(aprilDay.specnummer).toBe(1)
const audit = await readAudit(client, emp)
expect(audit).toHaveLength(4)
expect(audit.every((r) => r.trigger_op === 'insert')).toBe(true)
const march = audit.filter((r) => r.year_month === '2026-03')
expect(march.map((r) => r.new_specifikationsnummer)).toEqual([1, 2, 3])
expect(march.map((r) => r.absence_day_id)).toEqual([day1.id, day2.id, vabDay.id])
const april = audit.filter((r) => r.year_month === '2026-04')
expect(april.map((r) => r.new_specifikationsnummer)).toEqual([1])
})
})
it('sick days skip the trigger: no specnummer, no audit row', async () => {
const a = await seedCompany()
const emp = await insertEmployee({ userId: a.userId, companyId: a.companyId })
await withUserContext(a.userId, async (client) => {
const sickDay = await insertAbsenceDayAs(client, {
companyId: a.companyId,
employeeId: emp,
date: '2026-03-02',
type: 'sick',
})
expect(sickDay.specnummer).toBeNull()
const audit = await readAudit(client, emp)
expect(audit).toHaveLength(0)
})
})
it('upsert retry (ON CONFLICT DO UPDATE) is idempotent: no error, specnummer unchanged', async () => {
const a = await seedCompany()
const emp = await insertEmployee({ userId: a.userId, companyId: a.companyId })
await withUserContext(a.userId, async (client) => {
const day1 = await insertAbsenceDayAs(client, {
companyId: a.companyId,
employeeId: emp,
date: '2026-03-02',
type: 'parental',
})
expect(day1.specnummer).toBe(1)
// Mirror the PostgREST upsert lib/salary/absence.ts sends: the payload
// columns land in SET, franvaro_specifikationsnummer is never touched.
const retry = await client.query<{ franvaro_specifikationsnummer: number | null }>(
`INSERT INTO public.salary_absence_days
(company_id, employee_id, absence_date, absence_type, hours)
VALUES ($1, $2, '2026-03-02', 'parental', 4)
ON CONFLICT (employee_id, absence_date, absence_type)
DO UPDATE SET hours = EXCLUDED.hours
RETURNING franvaro_specifikationsnummer`,
[a.companyId, emp],
)
expect(retry.rows[0]!.franvaro_specifikationsnummer).toBe(1)
})
})
it('both trigger functions are SECURITY DEFINER with a pinned search_path', async () => {
const res = await getPool().query<{
proname: string
prosecdef: boolean
proconfig: string[] | null
}>(
`SELECT proname, prosecdef, proconfig
FROM pg_proc
WHERE proname IN (
'assign_franvaro_specifikationsnummer',
'assign_franvaro_specifikationsnummer_on_update'
)`,
)
expect(res.rows).toHaveLength(2)
for (const row of res.rows) {
expect(row.prosecdef).toBe(true)
expect(row.proconfig ?? []).toContain('search_path=public, pg_temp')
}
})
})
+15 -3
View File
@@ -91,11 +91,23 @@ function mapInsertError(error: { code?: string; message?: string }): {
code: string
details?: Record<string, unknown>
} {
// The 24h cap trigger raises check_violation when worked + absence > 24h
// for the same date.
if (error.code === '23514' || error.message?.includes('Total tid')) {
// Privilege/RLS denial (42501). Seen when a DB trigger writes to a
// protected table as SECURITY INVOKER (the franvaro audit-table bug fixed
// in migration 20260813120000): a server-side configuration error, kept
// distinct from INTERNAL_ERROR so the failure mode is diagnosable.
if (error.code === '42501') {
return { code: 'DB_PERMISSION_DENIED', details: { message: error.message } }
}
// The 24h cap trigger raises check_violation with 'Total tid' text when
// worked + absence hours exceed 24h for the same date.
if (error.message?.includes('Total tid')) {
return { code: 'ABSENCE_HOURS_CONFLICT', details: { message: error.message } }
}
// Any other CHECK violation (hours range, absence_type enum) is invalid
// input, not an hours conflict.
if (error.code === '23514') {
return { code: 'VALIDATION_ERROR', details: { message: error.message } }
}
return { code: 'INTERNAL_ERROR', details: { message: error.message } }
}
@@ -0,0 +1,33 @@
-- Fix: register_absence 500 for foraldraledighet/VAB from user-scoped surfaces.
--
-- Migration 20260517135000_skatteverket_audit_franvaro_lock.sql created
-- salary_absence_franvaro_audit with ENABLE ROW LEVEL SECURITY and ZERO
-- policies, and rewrote the specifikationsnummer trigger functions to INSERT
-- an audit row into it. That migration's comment claims the trigger "runs
-- SECURITY DEFINER (implicit in plpgsql functions that own the table)"; the
-- claim is false: plpgsql functions default to SECURITY INVOKER, so the audit
-- INSERT executes as the calling role. Under any non-BYPASSRLS role (role
-- authenticated: the dashboard absence POST, the web /pending approval, the
-- in-app Assistenten chat committing staged operations) the INSERT is denied
-- with SQLSTATE 42501, which aborts the whole salary_absence_days write. The
-- trigger fires only for absence_type IN ('vab', 'parental'), which is why
-- exactly those registrations failed with a generic 500 while 'sick' and
-- every other type kept working. Service-role paths (BYPASSRLS) were never
-- affected.
--
-- Fix: make both trigger functions SECURITY DEFINER so the audit INSERT runs
-- as the function owner (the migration runner, which also owns the audit
-- table; RLS is not FORCEd, so the owner is exempt). search_path is pinned
-- because SECURITY DEFINER without it is a privilege-escalation footgun.
--
-- Deliberately NO RLS policy is added on salary_absence_franvaro_audit: the
-- design intent is trigger/service-only writes, and an INSERT policy for
-- authenticated would let clients forge audit rows.
ALTER FUNCTION public.assign_franvaro_specifikationsnummer()
SECURITY DEFINER
SET search_path = public, pg_temp;
ALTER FUNCTION public.assign_franvaro_specifikationsnummer_on_update()
SECURITY DEFINER
SET search_path = public, pg_temp;