gnubok_create_customer takes a personnummer (encrypted before approval) but gnubok_update_customer did not, so an existing customer whose personnummer sat in the org-number field could not be corrected via MCP. The REST PATCH already supports it; this closes the MCP/pending-operations gap across its three layers: - tool inputSchema: personal_number (string or null) on the strict whitelist. The tool validates the plaintext before any DB read and mirrors the REST PATCH semantics: masked echo (********-1234 or ********-????) = leave unchanged, explicit null = clear, absent = untouched. Setting is refused unless the row ends up as an individual (GDPR art. 5.1 c), including via a simultaneous type change. - CustomerChangesSchema: personal_number_encrypted (nullable, ciphertext shape per customers_personal_number_check 20260726110000). The plaintext key stays forbidden by .strict() and staging-pii-guard. - update executor: maps the staged ciphertext onto customers .personal_number (set/clear/leave), re-checks the individual-only rule against a tampered row, and returns only personal_number_masked. PII handling: the personnummer is encrypted at staging time (AES-256-GCM, same path as create); pending_operations params carry only the ciphertext and the approval preview only the masked form. Idempotency hashing switches to the masked preview for personnummer-bearing updates (random-IV ciphertext would break retries); other updates keep their previous hash identity. catalogVisibility stays 'search': tools/list is at its 59.95K token ceiling with zero headroom (see DECISIONS.md). Fixes #1876 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5
Claude Fable 5
parent
d80103a2f5
commit
77cacdcf34
@@ -2,6 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createQueuedMockSupabase } from '@/tests/helpers'
|
||||
import { TOOL_SCOPE_MAP } from '@/lib/auth/api-keys'
|
||||
import { OPERATION_RISK_TIERS } from '@/lib/pending-operations/risk-tiers'
|
||||
import { hashRequest } from '@/lib/api/idempotency'
|
||||
import { decryptPersonnummer, encryptPersonnummer } from '@/lib/salary/personnummer'
|
||||
import { tools } from '../server'
|
||||
|
||||
const CUSTOMER_ID = '11111111-1111-4111-8111-111111111111'
|
||||
@@ -41,9 +43,9 @@ describe('gnubok_update_customer: registration', () => {
|
||||
expect(OPERATION_RISK_TIERS.update_customer).toBe('low')
|
||||
})
|
||||
|
||||
it('does not expose personal_number as an input', () => {
|
||||
const properties = tool().inputSchema.properties as Record<string, unknown>
|
||||
expect(properties).not.toHaveProperty('personal_number')
|
||||
it('exposes personal_number in the strict input schema, nullable for clearing', () => {
|
||||
const properties = tool().inputSchema.properties as Record<string, Record<string, unknown>>
|
||||
expect(properties.personal_number).toMatchObject({ type: ['string', 'null'] })
|
||||
})
|
||||
|
||||
it('keeps the wide write schema discoverable through tool search', async () => {
|
||||
@@ -166,3 +168,253 @@ describe('gnubok_update_customer: validation and staging', () => {
|
||||
expect(supabase.from).toHaveBeenNthCalledWith(2, 'pending_operations')
|
||||
})
|
||||
})
|
||||
|
||||
// ── personal_number (#1876) ───────────────────────────────────────────
|
||||
//
|
||||
// Synthetic personnummer, never a real one. REST PATCH semantics
|
||||
// (app/api/customers/[id]/route.ts): plaintext sets (encrypted at staging),
|
||||
// masked echo means unchanged, explicit null clears.
|
||||
const PERSONAL_NUMBER = '19900101-1234'
|
||||
const MASKED = '********-1234'
|
||||
const CIPHERTEXT_SHAPE = /^[0-9a-f]{76,255}$/
|
||||
|
||||
type StagedInsert = {
|
||||
params: { changes: Record<string, unknown> }
|
||||
preview_data: {
|
||||
current: Record<string, unknown>
|
||||
changes: Record<string, unknown>
|
||||
proposed: Record<string, unknown>
|
||||
}
|
||||
}
|
||||
|
||||
describe('gnubok_update_customer: personal_number', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('stages the personnummer encrypted and previews it masked, never in plaintext', async () => {
|
||||
const { supabase, enqueue, findCall } = createQueuedMockSupabase()
|
||||
enqueue({ data: currentCustomer({ customer_type: 'individual', org_number: null }) })
|
||||
enqueue({ data: { id: 'op-pn-1' } })
|
||||
|
||||
const result = (await tool().execute(
|
||||
{ customer_id: CUSTOMER_ID, personal_number: PERSONAL_NUMBER },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
)) as { staged: boolean; preview: StagedInsert['preview_data'] }
|
||||
|
||||
expect(result.staged).toBe(true)
|
||||
expect(result.preview.changes.personal_number_masked).toBe(MASKED)
|
||||
expect(result.preview.proposed.personal_number_masked).toBe(MASKED)
|
||||
expect(result.preview.changes).not.toHaveProperty('personal_number')
|
||||
expect(result.preview.changes).not.toHaveProperty('personal_number_encrypted')
|
||||
expect(JSON.stringify(result)).not.toContain(PERSONAL_NUMBER)
|
||||
|
||||
const inserted = findCall('pending_operations', 'insert')?.[0] as StagedInsert
|
||||
expect(inserted.params.changes.personal_number_encrypted).toMatch(CIPHERTEXT_SHAPE)
|
||||
expect(decryptPersonnummer(inserted.params.changes.personal_number_encrypted as string)).toBe(PERSONAL_NUMBER)
|
||||
expect(inserted.params.changes).not.toHaveProperty('personal_number')
|
||||
expect(inserted.preview_data.changes.personal_number_masked).toBe(MASKED)
|
||||
expect(inserted.preview_data.changes).not.toHaveProperty('personal_number_encrypted')
|
||||
// Nothing persisted carries the plaintext: not params, not preview, not title.
|
||||
expect(JSON.stringify(inserted)).not.toContain(PERSONAL_NUMBER)
|
||||
})
|
||||
|
||||
it.each([MASKED, '********-????'])(
|
||||
'treats the masked echo %s as "leave unchanged" and stages no personnummer change',
|
||||
async (maskedForm) => {
|
||||
const { supabase, enqueue, findCall } = createQueuedMockSupabase()
|
||||
enqueue({ data: currentCustomer({ customer_type: 'individual', org_number: null }) })
|
||||
enqueue({ data: { id: 'op-pn-2' } })
|
||||
|
||||
const result = (await tool().execute(
|
||||
{ customer_id: CUSTOMER_ID, personal_number: maskedForm, city: 'New City' },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
)) as { staged: boolean; preview: StagedInsert['preview_data'] }
|
||||
|
||||
expect(result.staged).toBe(true)
|
||||
expect(result.preview.changes).toEqual({ city: 'New City' })
|
||||
|
||||
const inserted = findCall('pending_operations', 'insert')?.[0] as StagedInsert
|
||||
expect(inserted.params.changes).toEqual({ city: 'New City' })
|
||||
},
|
||||
)
|
||||
|
||||
it('rejects an update whose only field is the masked echo (a no-op)', async () => {
|
||||
const { supabase } = createQueuedMockSupabase()
|
||||
|
||||
await expect(
|
||||
tool().execute(
|
||||
{ customer_id: CUSTOMER_ID, personal_number: MASKED },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
),
|
||||
).rejects.toThrow(/at least one/i)
|
||||
expect(supabase.from).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('stages an explicit null as a clear', async () => {
|
||||
const { supabase, enqueue, findCall } = createQueuedMockSupabase()
|
||||
enqueue({ data: currentCustomer({ customer_type: 'individual', org_number: null }) })
|
||||
enqueue({ data: { id: 'op-pn-3' } })
|
||||
|
||||
const result = (await tool().execute(
|
||||
{ customer_id: CUSTOMER_ID, personal_number: null },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
)) as { staged: boolean; preview: StagedInsert['preview_data'] }
|
||||
|
||||
expect(result.staged).toBe(true)
|
||||
expect(result.preview.changes.personal_number_masked).toBeNull()
|
||||
expect(result.preview.proposed.personal_number_masked).toBeNull()
|
||||
|
||||
const inserted = findCall('pending_operations', 'insert')?.[0] as StagedInsert
|
||||
expect(inserted.params.changes.personal_number_encrypted).toBeNull()
|
||||
})
|
||||
|
||||
it('shows the stored personnummer masked in the current preview, never the ciphertext', async () => {
|
||||
const stored = encryptPersonnummer(PERSONAL_NUMBER)
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({
|
||||
data: currentCustomer({ customer_type: 'individual', org_number: null, personal_number: stored }),
|
||||
})
|
||||
enqueue({ data: { id: 'op-pn-4' } })
|
||||
|
||||
const result = (await tool().execute(
|
||||
{ customer_id: CUSTOMER_ID, city: 'New City' },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
)) as { staged: boolean; preview: StagedInsert['preview_data'] }
|
||||
|
||||
expect(result.preview.current.personal_number_masked).toBe(MASKED)
|
||||
expect(result.preview.proposed.personal_number_masked).toBe(MASKED)
|
||||
const serialized = JSON.stringify(result)
|
||||
expect(serialized).not.toContain(PERSONAL_NUMBER)
|
||||
expect(serialized).not.toContain(stored)
|
||||
})
|
||||
|
||||
it('refuses personal_number on a business customer', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: currentCustomer() }) // swedish_business
|
||||
|
||||
await expect(
|
||||
tool().execute(
|
||||
{ customer_id: CUSTOMER_ID, personal_number: PERSONAL_NUMBER },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
),
|
||||
).rejects.toThrow(/individual/)
|
||||
expect(supabase.from).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('refuses personal_number when the same update turns the customer into a business', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: currentCustomer({ customer_type: 'individual', org_number: null }) })
|
||||
|
||||
await expect(
|
||||
tool().execute(
|
||||
{
|
||||
customer_id: CUSTOMER_ID,
|
||||
customer_type: 'swedish_business',
|
||||
personal_number: PERSONAL_NUMBER,
|
||||
},
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
),
|
||||
).rejects.toThrow(/individual/)
|
||||
})
|
||||
|
||||
it('rejects a malformed personal_number before querying the database', async () => {
|
||||
const { supabase } = createQueuedMockSupabase()
|
||||
|
||||
await expect(
|
||||
tool().execute(
|
||||
{ customer_id: CUSTOMER_ID, personal_number: 'not-a-personnummer' },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
),
|
||||
).rejects.toThrow(/personnummer/)
|
||||
expect(supabase.from).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('dry_run returns the masked preview without staging', async () => {
|
||||
const { supabase, enqueue, findCall } = createQueuedMockSupabase()
|
||||
enqueue({ data: currentCustomer({ customer_type: 'individual', org_number: null }) })
|
||||
|
||||
const result = (await tool().execute(
|
||||
{ customer_id: CUSTOMER_ID, personal_number: PERSONAL_NUMBER, dry_run: true },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never,
|
||||
)) as { staged: boolean; dry_run?: boolean; preview: StagedInsert['preview_data'] }
|
||||
|
||||
expect(result.staged).toBe(false)
|
||||
expect(result.dry_run).toBe(true)
|
||||
expect(result.preview.changes.personal_number_masked).toBe(MASKED)
|
||||
expect(JSON.stringify(result)).not.toContain(PERSONAL_NUMBER)
|
||||
expect(findCall('pending_operations', 'insert')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('replays an identical retry under the same idempotency_key despite the random-IV ciphertext', async () => {
|
||||
const args = {
|
||||
customer_id: CUSTOMER_ID,
|
||||
personal_number: PERSONAL_NUMBER,
|
||||
idempotency_key: '4d9e8b1a-2c3f-4a5b-8c7d-0e1f2a3b4c5d',
|
||||
}
|
||||
|
||||
// First call: miss, stage, store.
|
||||
const first = createQueuedMockSupabase()
|
||||
first.enqueue({ data: currentCustomer({ customer_type: 'individual', org_number: null }) })
|
||||
first.enqueue({ data: null }) // idempotency_keys lookup: miss
|
||||
first.enqueue({ data: { id: 'op-pn-idem' } }) // pending_operations insert
|
||||
first.enqueue({ data: null }) // idempotency_keys store
|
||||
const firstResult = (await tool().execute(args, 'company-1', 'user-1', first.supabase as never)) as {
|
||||
operation_id?: string
|
||||
preview: StagedInsert['preview_data']
|
||||
}
|
||||
expect(firstResult.operation_id).toBe('op-pn-idem')
|
||||
const stored = first.findCall('idempotency_keys', 'insert')?.[0] as {
|
||||
request_hash: string
|
||||
response_body: Record<string, unknown>
|
||||
}
|
||||
// The hash is over the masked changes, which are stable across calls;
|
||||
// hashing params (random-IV ciphertext) would make every retry look like
|
||||
// a different payload and fail with IDEMPOTENCY_KEY_REUSE.
|
||||
expect(stored.request_hash).toBe(
|
||||
hashRequest({
|
||||
operationType: 'update_customer',
|
||||
params: { customer_id: CUSTOMER_ID, changes: firstResult.preview.changes },
|
||||
companyId: 'company-1',
|
||||
}),
|
||||
)
|
||||
expect(JSON.stringify(stored)).not.toContain(PERSONAL_NUMBER)
|
||||
|
||||
// Second call: hit with the stored hash; nothing new is staged.
|
||||
const second = createQueuedMockSupabase()
|
||||
second.enqueue({ data: currentCustomer({ customer_type: 'individual', org_number: null }) })
|
||||
second.enqueue({
|
||||
data: {
|
||||
request_hash: stored.request_hash,
|
||||
response_status: 'success',
|
||||
response_body: stored.response_body,
|
||||
expires_at: '2999-01-01T00:00:00Z',
|
||||
},
|
||||
})
|
||||
const replay = (await tool().execute(args, 'company-1', 'user-1', second.supabase as never)) as {
|
||||
idempotency_replay?: boolean
|
||||
operation_id?: string
|
||||
}
|
||||
expect(replay.idempotency_replay).toBe(true)
|
||||
expect(replay.operation_id).toBe('op-pn-idem')
|
||||
expect(second.findCall('pending_operations', 'insert')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -95,6 +95,7 @@ import {
|
||||
} from '@/lib/customers/personal-number-shape'
|
||||
import {
|
||||
PERSONAL_NUMBER_PLAINTEXT_RE,
|
||||
isMaskedPersonalNumber,
|
||||
maskCustomerPersonalNumber,
|
||||
} from '@/lib/customers/mask-personal-number'
|
||||
import {
|
||||
@@ -5321,6 +5322,10 @@ export const tools: McpTool[] = [
|
||||
city: { type: 'string' },
|
||||
country: { type: 'string' },
|
||||
org_number: { type: 'string' },
|
||||
personal_number: {
|
||||
type: ['string', 'null'],
|
||||
description: 'Personnummer, individual customers only. Encrypted at staging, masked on read. Null clears; a masked value (********-1234) means unchanged.',
|
||||
},
|
||||
vat_number: { type: 'string', description: 'EU VAT numbers are revalidated with VIES when the update is approved.' },
|
||||
language: { type: 'string', enum: ['sv', 'en'] },
|
||||
default_payment_terms: { type: 'integer', minimum: 1 },
|
||||
@@ -5359,6 +5364,35 @@ export const tools: McpTool[] = [
|
||||
if (args[key] !== undefined) changes[key] = args[key]
|
||||
}
|
||||
|
||||
// personal_number never enters `changes` in plaintext: staging-pii-guard
|
||||
// forbids that key in staged payloads. REST PATCH semantics
|
||||
// (app/api/customers/[id]/route.ts): a masked echo of a read
|
||||
// ('********-1234' or '********-????') carries no new value and is
|
||||
// dropped; explicit null clears; a plaintext personnummer is validated
|
||||
// here and staged as AES-256-GCM ciphertext, so the approval preview
|
||||
// only ever sees the masked form.
|
||||
const personalNumberArg = args.personal_number
|
||||
let personalNumber: string | null = null
|
||||
if (personalNumberArg !== undefined) {
|
||||
if (personalNumberArg !== null && typeof personalNumberArg !== 'string') {
|
||||
throw new Error('personal_number must be a string or null.')
|
||||
}
|
||||
const trimmed = personalNumberArg === null ? null : personalNumberArg.trim()
|
||||
if (trimmed === null) {
|
||||
changes.personal_number_encrypted = null
|
||||
} else if (isMaskedPersonalNumber(trimmed)) {
|
||||
// Echo of a read: leave the stored personnummer alone.
|
||||
} else if (!PERSONAL_NUMBER_PLAINTEXT_RE.test(trimmed)) {
|
||||
throw new Error(
|
||||
'personal_number must be a Swedish personnummer: YYYYMMDD-XXXX, YYMMDD-XXXX or the digits alone. '
|
||||
+ 'Null clears the stored value; a masked value leaves it unchanged.',
|
||||
)
|
||||
} else {
|
||||
personalNumber = trimmed
|
||||
changes.personal_number_encrypted = encryptCustomerPersonalNumber(trimmed)
|
||||
}
|
||||
}
|
||||
|
||||
const parsed = UpdateCustomerParamsSchema.safeParse({
|
||||
customer_id: args.customer_id,
|
||||
changes,
|
||||
@@ -5370,7 +5404,7 @@ export const tools: McpTool[] = [
|
||||
|
||||
const { data: current, error } = await supabase
|
||||
.from('customers')
|
||||
.select('id, name, customer_type, customer_number, email, phone, address_line1, address_line2, postal_code, city, country, org_number, vat_number, vat_number_validated, language, default_payment_terms, notes')
|
||||
.select('id, name, customer_type, customer_number, email, phone, address_line1, address_line2, postal_code, city, country, org_number, vat_number, vat_number_validated, language, default_payment_terms, notes, personal_number')
|
||||
.eq('id', parsed.data.customer_id)
|
||||
.eq('company_id', companyId)
|
||||
.maybeSingle()
|
||||
@@ -5378,6 +5412,16 @@ export const tools: McpTool[] = [
|
||||
if (error) throw new Error(`Database error: ${error.message}`)
|
||||
if (!current) throw new Error('Customer not found.')
|
||||
|
||||
// Same guard as gnubok_create_customer and the REST PATCH route: only
|
||||
// individual rows get their identifiers masked on read (GDPR art.
|
||||
// 5.1 c), so a personnummer on a business customer is refused. Checked
|
||||
// against the type the row will END UP with, so a simultaneous type
|
||||
// change cannot smuggle one through.
|
||||
const effectiveCustomerType = (parsed.data.changes.customer_type ?? current.customer_type) as string
|
||||
if (personalNumber && effectiveCustomerType !== 'individual') {
|
||||
throw new Error('personal_number is only allowed for customer_type "individual".')
|
||||
}
|
||||
|
||||
const currentPreview = {
|
||||
customer_id: current.id,
|
||||
name: current.name,
|
||||
@@ -5396,6 +5440,19 @@ export const tools: McpTool[] = [
|
||||
language: current.language ?? 'sv',
|
||||
default_payment_terms: current.default_payment_terms,
|
||||
notes: current.notes ?? null,
|
||||
// The stored value is ciphertext; the preview (and the approval UI
|
||||
// that renders it) only ever sees the masked form.
|
||||
personal_number_masked: maskStoredCustomerPersonalNumber(current.personal_number),
|
||||
}
|
||||
|
||||
// The preview never carries the ciphertext either: a personal_number
|
||||
// change is shown as its masked form (or null when clearing).
|
||||
const { personal_number_encrypted: _stagedCiphertext, ...visibleChanges } = parsed.data.changes
|
||||
const previewChanges: Record<string, unknown> = { ...visibleChanges }
|
||||
if (parsed.data.changes.personal_number_encrypted !== undefined) {
|
||||
previewChanges.personal_number_masked = personalNumber
|
||||
? maskCustomerPersonalNumber(personalNumber)
|
||||
: null
|
||||
}
|
||||
|
||||
return stagePendingOperation(
|
||||
@@ -5407,14 +5464,22 @@ export const tools: McpTool[] = [
|
||||
parsed.data,
|
||||
{
|
||||
current: currentPreview,
|
||||
changes: parsed.data.changes,
|
||||
proposed: { ...currentPreview, ...parsed.data.changes },
|
||||
changes: previewChanges,
|
||||
proposed: { ...currentPreview, ...previewChanges },
|
||||
},
|
||||
actor,
|
||||
undefined,
|
||||
{
|
||||
dryRun: Boolean(args.dry_run),
|
||||
idempotencyKey: typeof args.idempotency_key === 'string' ? args.idempotency_key : undefined,
|
||||
// The ciphertext in params has a random IV, so params differ on
|
||||
// every retry when a personnummer is staged; hash the masked
|
||||
// preview instead (same reasoning as gnubok_create_customer). Only
|
||||
// set for personnummer-bearing updates so every other update keeps
|
||||
// its previous hash identity.
|
||||
...(parsed.data.changes.personal_number_encrypted !== undefined
|
||||
? { idempotencyParams: { customer_id: parsed.data.customer_id, changes: previewChanges } }
|
||||
: {}),
|
||||
},
|
||||
)
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user