From e22603acf11f6b89e4c9b467169a67022cf682ac Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 05:56:44 +0000 Subject: [PATCH] fix(rest): anchor looksLikeMissingRelation on the driver's quoted template (#8264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mapDataError`'s Postgres limb read `relation` and `does not exist` anywhere in the message, not necessarily the same sentence, so ordinary business prose using both words matched. Anchored on the quoted identifier Postgres always emits, mirroring #8132's anchor for the shared `looksLikeInternalErrorLeak` predicate in @objectstack/types (deliberately NOT reused here — it answers a different question and its other limbs are unrelated to this file's 404-vs-500 attribution). Both call sites of the predicate are covered: the DATA_STORE_FAULT (500) gate the issue named, and the looksLikeUnknownObject (404) limb the issue's own text did not measure. Tests pin both decision paths. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P7vaLs7bhBPi9m3JyzkhDj --- ...anchor-missing-relation-quoted-template.md | 39 +++++++++ packages/rest/src/rest-server.ts | 39 ++++++++- .../src/rest-unknown-object-heuristic.test.ts | 81 +++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 .changeset/anchor-missing-relation-quoted-template.md diff --git a/.changeset/anchor-missing-relation-quoted-template.md b/.changeset/anchor-missing-relation-quoted-template.md new file mode 100644 index 0000000000..d6136e73ac --- /dev/null +++ b/.changeset/anchor-missing-relation-quoted-template.md @@ -0,0 +1,39 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): anchor `looksLikeMissingRelation` on the driver's quoted template (#8264) + +`mapDataError`'s Postgres limb read `relation` and `does not exist` anywhere in +the message, not necessarily the same sentence — so ordinary business prose +using both words (`This relation does not exist in the diagram`) matched. +`does not exist` is ordinary business English; #8132 already anchored the +shared `@objectstack/types` leak predicate on the driver's own quoted +template for exactly this reason, and pinned the identical string as a +negative case. This file's copy of the same question was not covered by that +change (different package, different call site) and kept the loose reading. + +Anchored the same way here — a quoted identifier required between `relation` +and `does not exist` — as a locally-owned pattern rather than a call into the +shared leak predicate: that +predicate answers a different question ("may this be withheld from the +client"), and its other limbs (`sqlite_`, `unique constraint`, `foreign key`, +a bare SQL statement) have nothing to do with this file's question (is this +specifically an unknown-relation condition, for the 404-vs-500 split +`looksLikeMissingRelation` feeds). `relation-sub-object.ts` documents "two +widths, on purpose" for a neighbouring pair of consumers that ask genuinely +different questions; that does not extend to the two USES inside this file, +which both ask the same question and share one predicate correctly. + +**Both of the predicate's two call sites are covered, not just the reported +one:** the `DATA_STORE_FAULT` (500) gate the issue named, and the +`looksLikeUnknownObject` (404) limb the issue's own text did not measure. A +business message no longer gets mislabelled a `DATABASE_ERROR`, and a +crafted unquoted-but-attributable message no longer gets silently answered +`OBJECT_NOT_FOUND` — both now fall through to the generic, still-sanitised +terminal fault, which is the direction the branch's own #5462 comment already +argues for ("the safe way to be wrong is loud"). + +No reachable production path producing the unanchored shape was found at this +call site — this is consistency/invariant restoration between two spellings +of one question, not a fix for a demonstrated live misclassification. diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 6a58de8f15..8a1819ce29 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -437,6 +437,17 @@ function declaredHttpStatus(error: any): number | undefined { return declared; } +/** + * [#8264] Postgres' missing-relation template, anchored on the QUOTED + * identifier the driver always emits — never on the bare "does not exist" + * tail, which is ordinary business English. Module-scoped (not re-compiled + * per {@link mapDataError} call) and named, not inlined, so both of its + * readers share the literal same pattern. See the long note above + * `looksLikeMissingRelation`'s definition, further down this file, for why + * this is one width, not "two widths, on purpose". + */ +const RELATION_DOES_NOT_EXIST = /\brelation\s+["'`][^"'`]+["'`]\s+does not exist/i; + function missingRelationIsObject(raw: string, object: string | undefined): boolean { if (!object) return false; const named = @@ -1051,9 +1062,35 @@ export function mapDataError(error: any, object?: string): { status: number; bod // `code: 'OBJECT_NOT_FOUND'` and is matched far above) is the primary // producer of this 404 anyway; the driver-string limb has been a legacy // safety net since. + // + // [#8264] The Postgres limb used to be a two-`includes()` conjunction — + // `relation` and `does not exist` anywhere in the message, not necessarily + // the same sentence. `does not exist` is ordinary business English ("This + // relation does not exist in the diagram" — the exact negative case + // `error-leak.test.ts` pins for #8132's shared leak predicate), so that + // reading could re-verdict a legitimate business message through EITHER + // consumer below: the 500 gate right here, or the `looksLikeUnknownObject` + // 404 limb two lines further down (both read this same const). Anchored on + // Postgres' own errmsg template — a QUOTED identifier — the same technique + // #8132 used for `looksLikeInternalErrorLeak` in `@objectstack/types`. + // + // Deliberately NOT a call into that shared predicate: it answers a + // different question ("may this message be withheld from the client at + // all?"), and its other limbs — `sqlite_`, `unique constraint`, + // `foreign key`, a bare SQL statement — have nothing to do with THIS + // question (is this specifically an unknown-relation condition, for the + // 404-vs-500 split below?). `relation-sub-object.ts` documents "two + // widths, on purpose" for a neighbouring pair of consumers for exactly + // this reason — different questions get different patterns even when they + // share a substring. That precedent does NOT extend to the two USES right + // here, though: both the 500 gate and the 404 limb are asking this file's + // one question, and `missingRelationIsObject` below already gates the 500 + // path on attribution — so one width for both is correct, not "two + // widths, on purpose" a second time. See the reverse-verification note in + // `rest-unknown-object-heuristic.test.ts` for both paths measured. const looksLikeMissingRelation = lower.includes('no such table') || - (lower.includes('relation') && lower.includes('does not exist')) || + RELATION_DOES_NOT_EXIST.test(raw) || lower.includes('table not found'); if (looksLikeMissingRelation && !missingRelationIsObject(raw, object)) { return DATA_STORE_FAULT(); diff --git a/packages/rest/src/rest-unknown-object-heuristic.test.ts b/packages/rest/src/rest-unknown-object-heuristic.test.ts index e6f0a092ca..cf22090b20 100644 --- a/packages/rest/src/rest-unknown-object-heuristic.test.ts +++ b/packages/rest/src/rest-unknown-object-heuristic.test.ts @@ -488,3 +488,84 @@ describe('[#5462] the declared-status band is untouched', () => { expect(r.body.field).toBe('label'); }); }); + +// --------------------------------------------------------------------------- +// 5. [#8264] The Postgres limb is anchored on the quoted template — both +// decision paths this one const feeds +// --------------------------------------------------------------------------- +// +// `looksLikeMissingRelation` used to read `relation` and `does not exist` +// anywhere in the message, not necessarily the same sentence, so ordinary +// business prose using both words matched — `error-leak.test.ts` pins the +// identical negative case (`'This relation does not exist in the diagram'`) +// for the shared #8132 leak predicate this heuristic was never wired to. +// This section pins the same anchor here, for BOTH of this file's readers of +// the const: the 500 gate right where it is defined, and the +// `looksLikeUnknownObject` 404 limb a few lines below it. +// +// --------------------------------------------------------------------------- +// Reverse verification, direction predicted BEFORE running +// --------------------------------------------------------------------------- +// Restoring the old `(lower.includes('relation') && lower.includes('does not +// exist'))` conjunction: +// +// §5a (decision 1, the 500 gate) RED — the business message reverts to +// `DATABASE_ERROR`/500 instead of the +// generic terminal `INTERNAL_ERROR` +// §5b (decision 2, the 404 limb) RED — the crafted unattributed-but- +// word-matching message reverts to a +// SILENT 404 `OBJECT_NOT_FOUND` +// §5c/§5d (real driver phrasings) GREEN — untouched; every case here is +// already quoted, matching both the +// old and the new predicate +// +// Both are the ordinary RED direction, not one of the inverted families. +// Measured after predicting it; the run is quoted in the PR. + +describe('[#8264] anchored on the driver quoted template, not a bare conjunction', () => { + it('§5a decision 1 (the 500 gate): business prose using both words is no longer DATA_STORE_FAULT', () => { + // The card's own counter-example. No `object`, so it could never be + // attributed either way — the fixed predicate simply stops calling it + // a missing-relation condition at all, and it falls through to the + // generic terminal fault instead of the DATABASE-flavoured one. + const r = mapDataError(driverError('This relation does not exist in the diagram')); + expect(r.status).toBe(500); + expect(r.body.code).not.toBe('DATABASE_ERROR'); + expect(r.body.code).toBe('INTERNAL_ERROR'); + }); + + it('§5a holds with an object present too, and does not spill into the 404 limb either', () => { + const r = mapDataError(driverError('This relation does not exist in the diagram'), 'diagram'); + expect(r.body.code).not.toBe('DATABASE_ERROR'); + expect(r.body.code).not.toBe('OBJECT_NOT_FOUND'); + }); + + it('§5b decision 2 (the 404 limb): an unquoted "relation does not exist" no longer silently 404s', () => { + // Crafted to isolate decision 2, which the card's own example cannot + // reach: `missingRelationIsObject`'s Postgres branch tolerates + // UNQUOTED names (it answers a different, narrower question — which + // relation, once one is already suspected), so it still extracts + // `acct` and matches it to the object. Under the OLD predicate this + // fell through to the `looksLikeMissingRelation` OR-limb of + // `looksLikeUnknownObject` and silently answered 404 — the exact + // second consequence the card's own text never measured. + const r = mapDataError(driverError('Sorry, relation acct does not exist in our records'), 'acct'); + expect(r.status).not.toBe(404); + expect(r.body.code).not.toBe('OBJECT_NOT_FOUND'); + }); + + it('§5c quoted forms — every quote style Postgres could use — still trip the 500 gate', () => { + for (const quote of ['"', "'", '`']) { + const msg = `relation ${quote}sys_metadata${quote} does not exist`; + const r = mapDataError(driverError(msg)); + expect(r.status, msg).toBe(500); + expect(r.body.code, msg).toBe('DATABASE_ERROR'); + } + }); + + it('§5d the quoted form still attributes to 404 when the relation IS the object — decision 2, real case, unchanged', () => { + const r = mapDataError(driverError('relation "ghost" does not exist'), 'ghost'); + expect(r.status).toBe(404); + expect(r.body.code).toBe('OBJECT_NOT_FOUND'); + }); +});