fix(customers): personnummer via MCP lands in personal_number, masked everywhere; MCP payment terms follow settings (#1788)
* fix(customers): personnummer on the MCP path lands in personal_number, masked everywhere; MCP payment terms follow settings Follow-up to #1724 (Discord kalletoxic): the fix reached the web form and the v1 REST API, but not the MCP path, and the web customer list still showed a personnummer raw when it sat in org_number. Personnummer (MCP + every write path): - gnubok_create_customer gets a personal_number input. Until now it had none, so an agent creating a private person either dropped the number or put it in org_number, which nothing masks. Encrypted at staging (personal_number_encrypted + personal_number_masked; personal_number is now a forbidden staging key in staging-pii-guard), the approval preview shows ********-1234, commitCreateCustomer stores the ciphertext as-is. Idempotency hashes the masked preview (new StageOptions.idempotencyParams) because the random-IV ciphertext would make identical retries look like payload changes. - A personnummer-shaped org_number on customer_type=individual is the personnummer in the wrong field: it is moved into personal_number (encrypted) and org_number cleared, on CreateCustomerSchema (web POST, v1 POST, v1 bulk), both PATCH routes, MCP staging, and commitCreateCustomer for in-flight ops. Only a DIFFERENT personnummer next to personal_number is refused (new CUSTOMER_PERSONAL_NUMBER_CONFLICT). The business-type guard from #1724 is unchanged and now also fires at MCP staging, so the user never approves an operation that fails at commit. - Read side: the web customer list and gnubok_list_customers mask a legacy individual row's org_number personnummer instead of showing it raw; list_customers exposes personal_number_masked and never the ciphertext. - scripts/repair-customer-personal-number-in-org-number.ts moves the existing rows (dry run: 134 rows across 10 companies on prod); run by hand with --confirm after deploy. - customer-onboarding skill: EF customers follow the #1724 decision (individual + personal_number); ROT/RUT section names the real field. Payment terms (MCP): - gnubok_create_customer staged `payment_terms || 30`, so resolveDefaultPaymentTerms at commit always saw 30 and the company's invoice_default_days never reached MCP customers. Resolved at staging now, so the preview shows the value the row will get. tools/list payload ceiling 59.75K to 59.85K (descriptions trimmed first, rationale in payload-size.bench.test.ts). apiskill regenerated; no migrations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbLqn9bgZ9NJ5qnZMeC1Bk * fix(scripts): literal update payloads in the personnummer repair script The no-phantom-columns scanner counts a runtime-built update payload as unresolvable and the ceiling (379) had no headroom; two literal payloads keep the guard able to resolve both branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbLqn9bgZ9NJ5qnZMeC1Bk --------- 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:
co-authored by
Claude Fable 5
Jakob Wennberg
parent
a5c8f55127
commit
13b69a2056
@@ -5,7 +5,12 @@ import { validateVatNumber } from '@/lib/vat/vies-client'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
|
||||
import { encryptCustomerPersonalNumber, maskCustomerRow } from '@/lib/customers/protect-personal-number'
|
||||
import { looksLikeSwedishPersonalNumber } from '@/lib/customers/personal-number-shape'
|
||||
import {
|
||||
looksLikeSwedishPersonalNumber,
|
||||
normalizeReroutedPersonalNumber,
|
||||
orgNumberHoldsPersonalNumber,
|
||||
personalNumberDigits,
|
||||
} from '@/lib/customers/personal-number-shape'
|
||||
import { isMaskedPersonalNumber } from '@/lib/customers/mask-personal-number'
|
||||
import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message'
|
||||
|
||||
@@ -107,6 +112,23 @@ export const PATCH = withRouteContext(
|
||||
return errorResponseFromCode('CUSTOMER_ORG_NUMBER_IS_PERSONAL', opLog, { requestId })
|
||||
}
|
||||
|
||||
// The mirror image for individuals: a personnummer submitted as
|
||||
// org_number is the personnummer in the wrong field. It is stored
|
||||
// encrypted in personal_number and org_number is cleared, same as
|
||||
// CreateCustomerSchema does on create. Next to a DIFFERENT plaintext
|
||||
// personal_number in the same body the two conflict.
|
||||
const reroutedPersonalNumber = orgNumberHoldsPersonalNumber(effectiveType, body.org_number)
|
||||
? normalizeReroutedPersonalNumber(body.org_number!)
|
||||
: null
|
||||
if (
|
||||
reroutedPersonalNumber
|
||||
&& personalNumberSubmitted
|
||||
&& body.personal_number
|
||||
&& personalNumberDigits(body.personal_number) !== personalNumberDigits(reroutedPersonalNumber)
|
||||
) {
|
||||
return errorResponseFromCode('CUSTOMER_PERSONAL_NUMBER_CONFLICT', opLog, { requestId })
|
||||
}
|
||||
|
||||
const updateData: Record<string, unknown> = {}
|
||||
if (body.name !== undefined) updateData.name = body.name
|
||||
if (body.customer_type !== undefined) updateData.customer_type = body.customer_type
|
||||
@@ -126,9 +148,13 @@ export const PATCH = withRouteContext(
|
||||
if (body.postal_code !== undefined) updateData.postal_code = body.postal_code
|
||||
if (body.city !== undefined) updateData.city = body.city
|
||||
if (body.country !== undefined) updateData.country = body.country
|
||||
if (body.org_number !== undefined) updateData.org_number = body.org_number
|
||||
if (body.org_number !== undefined) {
|
||||
updateData.org_number = reroutedPersonalNumber ? null : body.org_number
|
||||
}
|
||||
if (body.vat_number !== undefined) updateData.vat_number = body.vat_number
|
||||
if (personalNumberSubmitted) {
|
||||
if (reroutedPersonalNumber && !(personalNumberSubmitted && body.personal_number)) {
|
||||
updateData.personal_number = encryptCustomerPersonalNumber(reroutedPersonalNumber)
|
||||
} else if (personalNumberSubmitted) {
|
||||
// Stored as ciphertext; customers_personal_number_check accepts that
|
||||
// shape only (20260726110000).
|
||||
updateData.personal_number = encryptCustomerPersonalNumber(body.personal_number)
|
||||
|
||||
@@ -405,3 +405,97 @@ describe('personal_number on customer routes', () => {
|
||||
expect(response.status).toBe(404)
|
||||
})
|
||||
})
|
||||
|
||||
// A personnummer submitted as org_number on an individual is the personnummer
|
||||
// in the wrong field: stored encrypted in personal_number, org_number left
|
||||
// empty, on create and on update alike.
|
||||
describe('personnummer submitted as org_number on an individual', () => {
|
||||
const routeParams = { params: Promise.resolve({ id: 'customer-1' }) }
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
eventBus.clear()
|
||||
captured.insert.length = 0
|
||||
captured.update.length = 0
|
||||
queryResult = { data: null, error: null }
|
||||
requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase })
|
||||
requireWriteMock.mockResolvedValue({ ok: true })
|
||||
})
|
||||
|
||||
it('POST stores it encrypted as personal_number and leaves org_number empty', async () => {
|
||||
queryResult = {
|
||||
data: {
|
||||
id: 'customer-1',
|
||||
name: 'Bertil Bengtsson',
|
||||
customer_type: 'individual',
|
||||
org_number: null,
|
||||
personal_number: encryptPersonnummer(PERSONAL_NUMBER),
|
||||
},
|
||||
error: null,
|
||||
}
|
||||
|
||||
const response = await POST(
|
||||
createMockRequest('/api/customers', {
|
||||
method: 'POST',
|
||||
body: { name: 'Bertil Bengtsson', customer_type: 'individual', org_number: PERSONAL_NUMBER },
|
||||
}),
|
||||
{ params: Promise.resolve({}) },
|
||||
)
|
||||
|
||||
const { status, body } = await parseJsonResponse<{ data: { personal_number: string; org_number: string | null } }>(response)
|
||||
expect(status).toBe(200)
|
||||
const inserted = captured.insert[0] as { org_number?: string | null; personal_number?: string | null }
|
||||
expect(inserted.org_number ?? null).toBeNull()
|
||||
expect(inserted.personal_number).toMatch(CIPHERTEXT_SHAPE)
|
||||
expect(decryptPersonnummer(inserted.personal_number!)).toBe(PERSONAL_NUMBER)
|
||||
expect(JSON.stringify(inserted)).not.toContain(PERSONAL_NUMBER)
|
||||
expect(body.data.personal_number).toBe(MASKED)
|
||||
})
|
||||
|
||||
it('PATCH stores it encrypted as personal_number and clears org_number', async () => {
|
||||
queryResult = {
|
||||
data: {
|
||||
id: 'customer-1',
|
||||
customer_type: 'individual',
|
||||
org_number: null,
|
||||
personal_number: encryptPersonnummer(PERSONAL_NUMBER),
|
||||
},
|
||||
error: null,
|
||||
}
|
||||
|
||||
const response = await PATCH(
|
||||
createMockRequest('/api/customers/customer-1', {
|
||||
method: 'PATCH',
|
||||
body: { org_number: PERSONAL_NUMBER },
|
||||
}),
|
||||
routeParams,
|
||||
)
|
||||
|
||||
const { status } = await parseJsonResponse<unknown>(response)
|
||||
expect(status).toBe(200)
|
||||
const updated = captured.update[0] as { org_number?: string | null; personal_number?: string | null }
|
||||
expect(updated.org_number).toBeNull()
|
||||
expect(updated.personal_number).toMatch(CIPHERTEXT_SHAPE)
|
||||
expect(decryptPersonnummer(updated.personal_number!)).toBe(PERSONAL_NUMBER)
|
||||
})
|
||||
|
||||
it('PATCH refuses an org_number that is a different personnummer than personal_number', async () => {
|
||||
queryResult = {
|
||||
data: { id: 'customer-1', customer_type: 'individual' },
|
||||
error: null,
|
||||
}
|
||||
|
||||
const response = await PATCH(
|
||||
createMockRequest('/api/customers/customer-1', {
|
||||
method: 'PATCH',
|
||||
body: { org_number: '19850505-5555', personal_number: PERSONAL_NUMBER },
|
||||
}),
|
||||
routeParams,
|
||||
)
|
||||
|
||||
const { status, body } = await parseJsonResponse<unknown>(response)
|
||||
expect(status).toBe(400)
|
||||
expect(JSON.stringify(body)).toContain('CUSTOMER_PERSONAL_NUMBER_CONFLICT')
|
||||
expect(captured.update).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -26,7 +26,12 @@ import {
|
||||
maskCustomerRow,
|
||||
} from '@/lib/customers/protect-personal-number'
|
||||
import { isMaskedPersonalNumber } from '@/lib/customers/mask-personal-number'
|
||||
import { looksLikeSwedishPersonalNumber } from '@/lib/customers/personal-number-shape'
|
||||
import {
|
||||
looksLikeSwedishPersonalNumber,
|
||||
normalizeReroutedPersonalNumber,
|
||||
orgNumberHoldsPersonalNumber,
|
||||
personalNumberDigits,
|
||||
} from '@/lib/customers/personal-number-shape'
|
||||
|
||||
// v1-only extension: allow PATCH to set archived_at back to null to
|
||||
// un-archive a customer. Restricted to literal `null` so the caller can't
|
||||
@@ -242,7 +247,7 @@ registerEndpoint({
|
||||
'org_number uniqueness is enforced at DB level: 23505 → 409 CUSTOMER_DUPLICATE_ORG_NUMBER.',
|
||||
'VIES re-validation is best-effort and runs only on commit. A VIES timeout does not fail the update.',
|
||||
'personal_number: a plaintext value is stored encrypted (individual customers only); the masked form a read returned (********-1234) means "leave unchanged" and is never stored; null clears it. Changing customer_type away from individual clears any stored personal_number.',
|
||||
'An org_number shaped like a Swedish personnummer is rejected for business customer_types (400 CUSTOMER_ORG_NUMBER_IS_PERSONAL).',
|
||||
'An org_number shaped like a Swedish personnummer is rejected for business customer_types (400 CUSTOMER_ORG_NUMBER_IS_PERSONAL). On an individual it is the personnummer in the wrong field: it is stored encrypted as personal_number and org_number is cleared; next to a different personal_number in the same body it is 400 CUSTOMER_PERSONAL_NUMBER_CONFLICT.',
|
||||
],
|
||||
example: {
|
||||
request: { default_payment_terms: 14, notes: 'New payment terms agreed 2026-05-12.' },
|
||||
@@ -350,6 +355,26 @@ export const PATCH = withApiV1<{ params: Promise<{ companyId: string; id: string
|
||||
})
|
||||
}
|
||||
|
||||
// The mirror image for individuals: a personnummer submitted as
|
||||
// org_number is the personnummer in the wrong field. It is stored
|
||||
// encrypted in personal_number and org_number is cleared, same as
|
||||
// CreateCustomerSchema does on create. Next to a DIFFERENT plaintext
|
||||
// personal_number in the same body the two conflict.
|
||||
const reroutedPersonalNumber = orgNumberHoldsPersonalNumber(effectiveType, body.org_number)
|
||||
? normalizeReroutedPersonalNumber(body.org_number!)
|
||||
: null
|
||||
if (
|
||||
reroutedPersonalNumber
|
||||
&& personalNumberSubmitted
|
||||
&& body.personal_number
|
||||
&& personalNumberDigits(body.personal_number) !== personalNumberDigits(reroutedPersonalNumber)
|
||||
) {
|
||||
return v1ErrorResponseFromCode('CUSTOMER_PERSONAL_NUMBER_CONFLICT', ctx.log, {
|
||||
requestId: ctx.requestId,
|
||||
details: { field: 'org_number' },
|
||||
})
|
||||
}
|
||||
|
||||
// Build the partial update set. Fields explicitly set to undefined in
|
||||
// the body are not in the resulting object (Zod strips undefined). null
|
||||
// IS allowed and means "clear the field" (or, for archived_at, "un-archive").
|
||||
@@ -381,7 +406,10 @@ export const PATCH = withApiV1<{ params: Promise<{ companyId: string; id: string
|
||||
if (body.customer_number !== undefined) {
|
||||
updateData.customer_number = body.customer_number || null
|
||||
}
|
||||
if (personalNumberSubmitted) {
|
||||
if (reroutedPersonalNumber) updateData.org_number = null
|
||||
if (reroutedPersonalNumber && !(personalNumberSubmitted && body.personal_number)) {
|
||||
updateData.personal_number = encryptCustomerPersonalNumber(reroutedPersonalNumber)
|
||||
} else if (personalNumberSubmitted) {
|
||||
// Stored as ciphertext; customers_personal_number_check accepts that
|
||||
// shape only (20260726110000).
|
||||
updateData.personal_number = encryptCustomerPersonalNumber(body.personal_number)
|
||||
|
||||
@@ -1180,3 +1180,96 @@ describe('company payment-terms default on create', () => {
|
||||
expect(supabaseMock.from.mock.calls.some((c) => c[0] === 'customers')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
// A personnummer submitted as org_number on an individual is the personnummer
|
||||
// in the wrong field: stored encrypted in personal_number, org_number left
|
||||
// empty, on create and on update alike. (The pre-2026-08-21 docs told callers
|
||||
// org_number was accepted for individuals; it still is, it just lands right.)
|
||||
describe('personnummer submitted as org_number on an individual (v1)', () => {
|
||||
it('POST stores it encrypted as personal_number and returns org_number empty', async () => {
|
||||
withWriteScope()
|
||||
const supabaseMock = makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
customers: {
|
||||
data: {
|
||||
...SAMPLE_CUSTOMER,
|
||||
customer_type: 'individual',
|
||||
org_number: null,
|
||||
vat_number: null,
|
||||
personal_number: TEST_PERSONAL_NUMBER,
|
||||
},
|
||||
error: null,
|
||||
},
|
||||
})
|
||||
mockServiceClient.mockReturnValue(supabaseMock)
|
||||
|
||||
const res = await createCustomer(
|
||||
makePostRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/customers`, {
|
||||
name: 'Bertil Bengtsson',
|
||||
customer_type: 'individual',
|
||||
org_number: TEST_PERSONAL_NUMBER,
|
||||
}),
|
||||
companyParams(COMPANY_ID),
|
||||
)
|
||||
|
||||
expect(res.status).toBe(201)
|
||||
const inserted = supabaseMock.captured.insert[0] as { org_number?: string | null; personal_number?: string | null }
|
||||
expect(inserted.org_number ?? null).toBeNull()
|
||||
expect(inserted.personal_number).toMatch(CIPHERTEXT_SHAPE)
|
||||
expect(decryptPersonnummer(inserted.personal_number!)).toBe(TEST_PERSONAL_NUMBER)
|
||||
expect(JSON.stringify(inserted)).not.toContain(TEST_PERSONAL_NUMBER)
|
||||
const body = await res.json()
|
||||
expect(body.data.personal_number).toBe(MASKED_PERSONAL_NUMBER)
|
||||
expect(body.data.org_number).toBeNull()
|
||||
})
|
||||
|
||||
it('PATCH stores it encrypted as personal_number and clears org_number', async () => {
|
||||
withWriteScope()
|
||||
const supabaseMock = makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
customers: {
|
||||
data: { ...SAMPLE_CUSTOMER, customer_type: 'individual', org_number: null, personal_number: TEST_PERSONAL_NUMBER },
|
||||
error: null,
|
||||
},
|
||||
})
|
||||
mockServiceClient.mockReturnValue(supabaseMock)
|
||||
|
||||
const res = await updateCustomer(
|
||||
makePatchRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/customers/${CUSTOMER_ID}`, {
|
||||
org_number: TEST_PERSONAL_NUMBER,
|
||||
}),
|
||||
detailParams(COMPANY_ID, CUSTOMER_ID),
|
||||
)
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
const updated = supabaseMock.captured.update[0] as { org_number?: string | null; personal_number?: string | null }
|
||||
expect(updated.org_number).toBeNull()
|
||||
expect(updated.personal_number).toMatch(CIPHERTEXT_SHAPE)
|
||||
expect(decryptPersonnummer(updated.personal_number!)).toBe(TEST_PERSONAL_NUMBER)
|
||||
})
|
||||
|
||||
it('PATCH refuses an org_number that is a different personnummer than personal_number', async () => {
|
||||
withWriteScope()
|
||||
const supabaseMock = makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
customers: {
|
||||
data: { ...SAMPLE_CUSTOMER, customer_type: 'individual' },
|
||||
error: null,
|
||||
},
|
||||
})
|
||||
mockServiceClient.mockReturnValue(supabaseMock)
|
||||
|
||||
const res = await updateCustomer(
|
||||
makePatchRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/customers/${CUSTOMER_ID}`, {
|
||||
org_number: '19850505-5555',
|
||||
personal_number: TEST_PERSONAL_NUMBER,
|
||||
}),
|
||||
detailParams(COMPANY_ID, CUSTOMER_ID),
|
||||
)
|
||||
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error.code).toBe('CUSTOMER_PERSONAL_NUMBER_CONFLICT')
|
||||
expect(supabaseMock.captured.update).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -280,8 +280,8 @@ registerEndpoint({
|
||||
pitfalls: [
|
||||
'Idempotency-Key is mandatory: calls without it return 400 VALIDATION_ERROR.',
|
||||
'org_number uniqueness is enforced at the database level; duplicate inserts return 409 CUSTOMER_DUPLICATE_ORG_NUMBER.',
|
||||
'For Swedish sole traders (customer_type=individual), org_number IS the personnummer. List responses mask it; the create endpoint accepts it as input.',
|
||||
'An org_number shaped like a Swedish personnummer is rejected for business customer_types: create the customer as customer_type=individual (personal_number or org_number) so the number is masked and protected.',
|
||||
'A personnummer-shaped org_number on customer_type=individual is treated as the personnummer submitted in the wrong field: it is stored encrypted as personal_number, returned masked (********-1234), and org_number is left empty. Prefer passing it as personal_number. Next to a different personal_number in the same body it is a 400.',
|
||||
'An org_number shaped like a Swedish personnummer is rejected for business customer_types: create the customer as customer_type=individual with personal_number so the number is masked and protected.',
|
||||
'personal_number is accepted only for customer_type=individual, stored encrypted, and returned in the masked form ********-1234.',
|
||||
'If default_payment_terms is omitted, it defaults to the company setting invoice_default_days, falling back to 30.',
|
||||
'VIES validation runs only on commit. Dry-run skips the external call and leaves vat_number_validated=false in the preview.',
|
||||
|
||||
Reference in New Issue
Block a user