From d5a7b624559fe4160ace8c73d5bacfc493f3f880 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 01:48:49 +0000 Subject: [PATCH] refactor(stack,supabase,prisma): simplify the log-safe failure helper - Prove the FFI error-code allowlist complete with a Record over ProtectErrorCode, like the auth list, instead of an exported array plus a runtime parity test. - Drop the redundant 'Error' entry from ERROR_CLASSES (kindOf already falls back to it). - Point the call-site comments at describeFailure instead of restating its reasoning. - Replace the try/catch loops in the wildcard tests with toThrow, and share the DynamoDB handleError failure fixture. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012Jmn7nUyrbPUkKiKRBH5Lk --- .../operator-lowering-v3-text-search.test.ts | 11 +---- .../__tests__/no-plaintext-in-errors.test.ts | 12 ++--- .../stack-supabase/src/query-builder.ts | 5 +-- .../stack/__tests__/describe-failure.test.ts | 10 +---- .../dynamodb/resolve-decrypt.test.ts | 22 +++------ .../packages/stack/src/dynamodb/helpers.ts | 8 ++-- .../stack/src/schema/match-defaults.ts | 6 +-- .../src/utils/logger/describe-failure.ts | 45 +++++++++---------- 8 files changed, 41 insertions(+), 78 deletions(-) diff --git a/languages/typescript/packages/stack-prisma/test/v3/operator-lowering-v3-text-search.test.ts b/languages/typescript/packages/stack-prisma/test/v3/operator-lowering-v3-text-search.test.ts index 7d5060c2c..8d381fd8c 100644 --- a/languages/typescript/packages/stack-prisma/test/v3/operator-lowering-v3-text-search.test.ts +++ b/languages/typescript/packages/stack-prisma/test/v3/operator-lowering-v3-text-search.test.ts @@ -128,15 +128,8 @@ describe('cipherstash v3 operator lowering — eqlMatch needle guards', () => { // the caller, who commonly logs it, so it reports the term's length only. it('never echoes the rejected pattern in the wildcard error', () => { for (const needle of ['Qzx%wvy', 'Qzx_wvy']) { - let message = '' - try { - lowerMatch(needle) - } catch (err) { - message = (err as Error).message - } - expect(message, needle).toMatch(/cannot honor/) - expect(message, needle).not.toContain('Qzx') - expect(message, needle).not.toContain('wvy') + expect(() => lowerMatch(needle), needle).toThrow(/cannot honor/) + expect(() => lowerMatch(needle), needle).not.toThrow(/Qzx|wvy/) } }) diff --git a/languages/typescript/packages/stack-supabase/__tests__/no-plaintext-in-errors.test.ts b/languages/typescript/packages/stack-supabase/__tests__/no-plaintext-in-errors.test.ts index 2ba598323..f0b35345c 100644 --- a/languages/typescript/packages/stack-supabase/__tests__/no-plaintext-in-errors.test.ts +++ b/languages/typescript/packages/stack-supabase/__tests__/no-plaintext-in-errors.test.ts @@ -61,15 +61,9 @@ describe('plaintext search terms stay out of errors and logs', () => { it('a like() pattern with unsupported wildcards is not echoed in the error', () => { for (const pattern of ['Qzx%wvy', 'Qzx_wvy']) { - let message = '' - try { - builder().select('id').like('email', pattern) - } catch (err) { - message = (err as Error).message - } - expect(message, pattern).toMatch(/cannot honor/) - expect(message, pattern).not.toContain('Qzx') - expect(message, pattern).not.toContain('wvy') + const like = () => builder().select('id').like('email', pattern) + expect(like, pattern).toThrow(/cannot honor/) + expect(like, pattern).not.toThrow(/Qzx|wvy/) } }) diff --git a/languages/typescript/packages/stack-supabase/src/query-builder.ts b/languages/typescript/packages/stack-supabase/src/query-builder.ts index eecbe3d3d..8dbd0fd87 100644 --- a/languages/typescript/packages/stack-supabase/src/query-builder.ts +++ b/languages/typescript/packages/stack-supabase/src/query-builder.ts @@ -599,10 +599,7 @@ export class EncryptedQueryBuilderImpl< } satisfies DecryptContext) } catch (err) { const message = err instanceof Error ? err.message : String(err) - // Log the failure's kind, never its message: this catch sees errors from - // every layer (query-term validation, encryption, PostgREST), and their - // messages may quote a plaintext value. The full message still reaches - // the caller in the returned `error`. + // Log-safe: see describeFailure. The full message goes back in `error`. logger.error( `Supabase encrypted query failed on table "${this.tableName}" (${describeFailure(err)}).`, ) diff --git a/languages/typescript/packages/stack/__tests__/describe-failure.test.ts b/languages/typescript/packages/stack/__tests__/describe-failure.test.ts index 9a1f4ff0f..328a25f20 100644 --- a/languages/typescript/packages/stack/__tests__/describe-failure.test.ts +++ b/languages/typescript/packages/stack/__tests__/describe-failure.test.ts @@ -1,9 +1,5 @@ -import { PROTECT_ERROR_CODES } from '@cipherstash/protect-ffi' import { describe, expect, it } from 'vitest' -import { - describeFailure, - FFI_ERROR_CODES, -} from '@/utils/logger/describe-failure' +import { describeFailure } from '@/utils/logger/describe-failure' // `describeFailure` is what adapters log in place of an error's message, which // may quote a plaintext value (CIP-5161). Everything it returns must come from @@ -82,8 +78,4 @@ describe('describeFailure', () => { revoke() expect(describeFailure(proxy)).toBe('unknown failure') }) - - it('allowlists exactly the FFI error codes protect-ffi declares', () => { - expect([...FFI_ERROR_CODES].sort()).toEqual([...PROTECT_ERROR_CODES].sort()) - }) }) diff --git a/languages/typescript/packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts b/languages/typescript/packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts index d1838eafd..5873cc694 100644 --- a/languages/typescript/packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts +++ b/languages/typescript/packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts @@ -448,15 +448,13 @@ describe('handleError', () => { // The message may quote a plaintext value (CIP-5161). It is returned to the // caller on the error, but the library's own log names only the failure kind. - it('logs the failure kind and code, never the message', () => { - const failure = Object.assign( - new Error('value "Qzx-secret-wvy" rejected'), - { - code: 'UNKNOWN_COLUMN', - }, - ) + const leakyFailure = () => + Object.assign(new Error('value "Qzx-secret-wvy" rejected'), { + code: 'UNKNOWN_COLUMN', + }) - const error = handleError(failure, 'encryptModel') + it('logs the failure kind and code, never the message', () => { + const error = handleError(leakyFailure(), 'encryptModel') expect(error.message).toContain('Qzx-secret-wvy') const logged = errorLog.mock.calls.flat().join('\n') @@ -469,16 +467,10 @@ describe('handleError', () => { // service, so it gets the same log-safe description as the shared logger. // The full message stays on the returned error and on `errorHandler`'s. it('hands the configured logger a log-safe error, and errorHandler the full one', () => { - const failure = Object.assign( - new Error('value "Qzx-secret-wvy" rejected'), - { - code: 'UNKNOWN_COLUMN', - }, - ) const logged: unknown[] = [] const handled: Error[] = [] - const error = handleError(failure, 'encryptModel', { + const error = handleError(leakyFailure(), 'encryptModel', { logger: { error: (message, err) => logged.push(message, err) }, errorHandler: (err) => handled.push(err), }) diff --git a/languages/typescript/packages/stack/src/dynamodb/helpers.ts b/languages/typescript/packages/stack/src/dynamodb/helpers.ts index 3990d56f1..85b4f05ce 100644 --- a/languages/typescript/packages/stack/src/dynamodb/helpers.ts +++ b/languages/typescript/packages/stack/src/dynamodb/helpers.ts @@ -69,11 +69,9 @@ export function handleError( context, }) - // Log the failure's kind, never its message: it may quote a plaintext value - // (CIP-5161). `options.logger` is a log sink too (typically `console`), so it - // gets a copy of the error carrying the same description instead of the - // message. The full message stays on the returned error and reaches - // `errorHandler`, which hands it back to the caller. + // Log-safe: see describeFailure. `options.logger` is a log sink too, so it + // gets a copy carrying the description; the full message stays on the + // returned error and reaches `errorHandler`. const description = `DynamoDB error in ${context} (${describeFailure(error)}).` logger.error(description) diff --git a/languages/typescript/packages/stack/src/schema/match-defaults.ts b/languages/typescript/packages/stack/src/schema/match-defaults.ts index 5755af8c8..807dd0897 100644 --- a/languages/typescript/packages/stack/src/schema/match-defaults.ts +++ b/languages/typescript/packages/stack/src/schema/match-defaults.ts @@ -154,10 +154,8 @@ export function matchNeedleError( * wildcard the bloom tokenizer cannot honour (an interior `%` or a `_`, which it * would treat as an ordinary character and so silently match nothing). * - * Takes the pattern rather than its length so no caller can be tempted to - * interpolate it: the pattern is the caller's plaintext search term, and the - * message is thrown to callers who may log it. Only its length appears. - * Adapters prefix their surface and append their own remedy. + * Only the pattern's length appears: it is plaintext, and callers may log the + * message. Adapters prefix their surface and append their own remedy. */ export function likeWildcardError(pattern: string): string { return `the ${[...pattern].length}-character search pattern has wildcards fuzzy free-text matching cannot honor (an interior "%" or a "_").` diff --git a/languages/typescript/packages/stack/src/utils/logger/describe-failure.ts b/languages/typescript/packages/stack/src/utils/logger/describe-failure.ts index 7107b6529..3a86e9bb4 100644 --- a/languages/typescript/packages/stack/src/utils/logger/describe-failure.ts +++ b/languages/typescript/packages/stack/src/utils/logger/describe-failure.ts @@ -1,7 +1,7 @@ // Type-only imports: adapter-kit must stay importable without loading the // native binding or the auth module, so the allowlists below are written out -// and the types prove them complete (`describe-failure.test.ts` checks the FFI -// list against the runtime one). +// as `Record`s over the code unions, which makes the compiler prove them +// complete. import type { AuthErrorCode } from '@cipherstash/auth' import type { ProtectErrorCode } from '@cipherstash/protect-ffi' import { EncryptionErrorTypes } from '@/errors' @@ -45,8 +45,8 @@ export function describeFailure(err: unknown): string { } } +// Any other name is reported as plain `Error`. const ERROR_CLASSES: ReadonlySet = new Set([ - 'Error', 'AggregateError', 'EvalError', 'RangeError', @@ -59,26 +59,25 @@ const ERROR_CLASSES: ReadonlySet = new Set([ 'EncryptionFailedError', ]) -/** Every FFI error code. `describe-failure.test.ts` pins it to the runtime list. */ -export const FFI_ERROR_CODES = [ - 'INVARIANT_VIOLATION', - 'UNKNOWN_QUERY_OP', - 'UNKNOWN_COLUMN', - 'MISSING_INDEX', - 'INVALID_QUERY_INPUT', - 'SHORT_MATCH_NEEDLE', - 'INVALID_JSON_PATH', - 'STE_VEC_REQUIRES_JSON_CAST_AS', - 'MATCH_REQUIRES_TEXT', - 'UNSUPPORTED_CONFIG_VERSION', - 'INVALID_EQL_VERSION', - 'EQL_V3_UNSUPPORTED_COLUMN', - 'EQL_V3_CONVERSION_FAILED', - 'INVALID_CIPHERTEXT', - 'UNKNOWN', -] as const satisfies readonly ProtectErrorCode[] +// A `Record` over each union, so the compiler rejects a missing or unknown code. +const FFI_ERROR_CODES: Record = { + INVARIANT_VIOLATION: true, + UNKNOWN_QUERY_OP: true, + UNKNOWN_COLUMN: true, + MISSING_INDEX: true, + INVALID_QUERY_INPUT: true, + SHORT_MATCH_NEEDLE: true, + INVALID_JSON_PATH: true, + STE_VEC_REQUIRES_JSON_CAST_AS: true, + MATCH_REQUIRES_TEXT: true, + UNSUPPORTED_CONFIG_VERSION: true, + INVALID_EQL_VERSION: true, + EQL_V3_UNSUPPORTED_COLUMN: true, + EQL_V3_CONVERSION_FAILED: true, + INVALID_CIPHERTEXT: true, + UNKNOWN: true, +} -// A `Record` over the union, so the compiler rejects a missing or unknown code. const AUTH_ERROR_CODES: Record = { REQUEST_ERROR: true, ACCESS_DENIED: true, @@ -105,7 +104,7 @@ const AUTH_ERROR_CODES: Record = { const KNOWN_IDENTIFIERS: ReadonlySet = new Set([ ...Object.values(EncryptionErrorTypes), - ...FFI_ERROR_CODES, + ...Object.keys(FFI_ERROR_CODES), ...Object.keys(AUTH_ERROR_CODES), ])