Skip to content

Commit 4342c99

Browse files
os-billclaude
andauthored
fix(spec): guard three data lookups against Object.prototype fall-through (#17819)
Fixes #17456 Fixes #17762 **Clause-②: no** — the diff adds no exported symbol, no key on a published payload and no registration; it narrows three existing implementations onto the signatures they already declare. Checked mechanically, not asserted: `node scripts/pm/check-widening-tells.mjs --declaration no --diff …` exits **0** — «7 changed file(s) — 3 judged against a declared surface (no widening tell), 4 NOT MEASURED» (the 4 being the changeset and the three test files, neither of which declares a contract). The known T1 false positive on a re-declared key line did not fire. > Note: generic type brackets are written with **square** brackets in this body (`Record[string, X]` stands for the angle-bracket spelling) so it survives GitHub's body sanitiser intact. ## What was wrong Three lookups indexed a plain object literal with a runtime key, so an off-vocabulary key resolved an **inherited** `Object.prototype` member and was handed out through a signature that admits only declared values: - `canonicalizeSqlType` — `packages/spec/src/data/type-compat.ts`, **both halves** of its alias line (`dialectMap[t]` and `BASE_ALIASES[t]`) - `suggestDefaultValueToken` — `packages/spec/src/data/default-value-shape.ts` - `classifyFilterToken` — `packages/spec/src/data/context-tokens.zod.ts`, where the index sits in a **property position inside an object literal**, which is why #17456's `return TABLE[key];`-keyed survey could not reach it ## Premise falsification, first — all three faces Every claim in either card was re-checked rather than inherited. **Action face.** All three sites were read on `origin/main` (`2eab3beb7f`, fetched) and all three still lacked a guard. Each symbol was located by name; the three `path:line` readings the cards carry (`type-compat.ts:222`, `default-value-shape.ts:297`, `context-tokens.zod.ts:252`) all re-derived to the same lines at this head, but the pins and this body cite symbols, not line numbers. `git log --oneline -20` over each file returns one commit in this checkout — ⚠️ the checkout is **shallow** (`git rev-parse --is-shallow-repository` → `true`), so that is a depth artefact and no ancestry claim rests on it. **Card-reference face.** Read live rather than as the cards describe them: **#15315** — state `completed`, no longer open; **#16903** — state `completed`, no longer open; **#17715** — open; **objectui#9129** — open. All four match what the cards say. (The words for the not-open state are spelled around deliberately: GitHub's reference parser matches a keyword plus a number and ignores the surrounding prose, so a status sentence must not put one near a card number it has no business touching.) **Work-item face.** Each site independently confirmed unguarded in the tree before being touched. ⛔ The two excluded siblings were re-confirmed and **not** touched: `numericColumnFor` is guarded by a `Set` membership test above the index, and `currencyFractionDigits` upper-cases its key. ## ⭐ The measurement this round owed #17762's filer states plainly that they did **not** import and drive the built `classifyFilterToken` — their table came from a minimal literal with the same prototype. That reading is delivered here, against the **BUILT artifact** (`packages/spec/dist/data/index.mjs`) on the repo's Node 22 baseline (v22.22.2), for all three functions, before and after. Population fixed at five, every zero carrying a control that could have come back the other way. | call | BEFORE | AFTER | |:--|:--|:--| | `canonicalizeSqlType('varchar')` — lit control | `'text'` | `'text'` | | `canonicalizeSqlType('timestamptz', 'postgres')` — lit control | `'datetime'` | `'datetime'` | | `canonicalizeSqlType('constructor')` | the `Object` **function** | `'unknown'` | | `canonicalizeSqlType('constructor', d)` — every `SqlDialect` `d` | the `Object` **function** | `'unknown'` | | `canonicalizeSqlType('__proto__')` | `'array'` | `'array'` | | `canonicalizeSqlType('toString' / 'valueOf' / 'nope')` | `'unknown'` | `'unknown'` | | `suggestFieldTypeForSqlType('constructor')` | **`TypeError: Cannot read properties of undefined (reading 'suggested')`** | `undefined` | | `isCompatible('constructor', 'text')` | **`TypeError: … (reading 'exact')`** | `'lossy'` | | `suggestDefaultValueToken('currentuser')` — lit control | `'current_user'` | `'current_user'` | | `suggestDefaultValueToken('constructor')` | the `Object` **function** | `undefined` | | `suggestDefaultValueToken('__proto__')` | `Object.prototype` | `undefined` | | `suggestDefaultValueToken('toString' / 'valueOf' / 'nope')` | `undefined` | `undefined` | | `classifyFilterToken('{current_user}').suggestion` — lit control | `'current_user_id'` | `'current_user_id'` | | `classifyFilterToken('{constructor}').suggestion` | the `Object` **function** | `undefined` | | `classifyFilterToken('{__proto__}').suggestion` | `Object.prototype` | `undefined` | | `classifyFilterToken('{toString}' / '{valueOf}' / '{nope}').suggestion` | `undefined` | `undefined` | Three readings neither card carries: 1. ⭐ **The `__proto__` row on site 1 is not a fall-through.** `'__proto__'` starts with `_`, which is Postgres array notation (`_int4`), and that branch returns `'array'` — a legitimate declared member — before either table is consulted. #17456's table reports `'array'` correctly but does not say why; a reader could mistake it for the defect. A pin now records it so the array rule cannot be quietly dropped either. 2. ⭐ **The defect was not confined to the function's own return.** A non-`CanonicalSqlType` reaches `CANONICAL_TO_FIELD[canonical]`, which is `undefined`, so the two **published sibling accessors** that read the result — `suggestFieldTypeForSqlType` and `isCompatible` — **threw a `TypeError`** on `rawType = 'constructor'`. That is a crash on a published surface, from a `rawType` that arrives off live database introspection. 3. ⭐ **The wider-regex question #17762 asks (its acceptance item 4) is answered, and bounded.** `FILTER_TOKEN_WRAPPED_RE` captures `[^{}]+` — anything but braces — so the reachable key set is *not* the identifier-shaped one objectui's narrower `[a-zA-Z0-9_]+` bounds. What bounds it here is the `toLowerCase()`: of `Object.prototype`'s twelve own property names, exactly **two** are lower-case-stable and therefore namable — `constructor` and `__proto__`. The other ten (`hasOwnProperty`, `toLocaleString`, `isPrototypeOf`, `propertyIsEnumerable`, `toString`, `valueOf`, and the four `__define*` / `__lookup*` accessors) are quiet by that casing accident alone, ⛔ not by a guard. One pin sweeps all twelve so the day a lower-case prototype member is added is a red test rather than a silent regression. ## The fix An `Object.prototype.hasOwnProperty.call` guard at each lookup, returning **that function's own already-declared refusal value** read from its signature — `'unknown'` (site 1's own trailing `return`), `undefined` (site 2's declared return type), and an absent `suggestion` (site 3's optional field). ⛔ No declared return type changes, and no value was invented for this round. The spelling is copied from the two landed siblings in `src/shared/value-domain.zod.ts` and `src/data/driver/config-registry.zod.ts`. ⛔ **No null-prototype table**, per the binding carry-over: the annotation either stops compiling (TS2353) or, in the `Object.assign(Object.create(null), …)` spelling, silently loses its exhaustiveness check (TS2741 stopped firing for a table missing a member). ⛔ **No special-casing of `constructor`** — a guard that names words does not survive the next prototype member. ## The pins, and the ablation that proves they can fail Each site gains a pin whose population is exactly **`constructor`, `toString`, `valueOf`, `__proto__` and a plain unknown word**, plus a lit control proving the real vocabulary still answers. The pre-existing pins are green today because they iterate the canonical vocabulary only — precisely the population that behaves. ⭐ **Ablation, run from the committed state, on-disk proof read first.** All three guards were removed from source; the removal was proven on disk by `grep -c` on each guard's exact text (`1 → 0` for each, against a lit control on a string still present reading `2`) and by three changed blob hashes, ⛔ not by any editor's exit code. The script carried `trap … EXIT INT TERM` with absolute paths. - **mutated** — `Test Files 3 failed (3)` · `Tests 10 failed | 89 passed (99)`. The ten are exactly the prototype-member cases across all three files; **every lit control stayed green**, so the ablation did not simply break the suite. - **restored** — `git checkout HEAD -- …`, proven by an empty `git diff HEAD`, an empty **whole-tree** `git status --porcelain`, and all three blob hashes back at their `HEAD` values (matched, not empty). Then: `Test Files 3 passed (3)` · `Tests 99 passed (99)`. ⭐ No rebuild is involved, and that is a measured fact rather than a shortcut: these tests import their subject by **relative source path**, so vitest resolves `src/`. Had they been consuming `dist/`, a mutation to `src` alone would have left them green — that they moved *is* the proof of the resolution path. ## Changeset — measured `npm pack --dry-run --json` on the **built** package (built first): - **positive control** `dist/data/index.mjs` → **SHIPS**; `dist/data/index.js` → **SHIPS** - **negative controls** `src/data/type-compat.ts`, `src/data/default-value-shape.ts`, `src/data/type-compat.test.ts`, `src/data/context-tokens.test.ts` → **do NOT ship** - `src/data/context-tokens.zod.ts` → **SHIPS** as source too (the manifest's `files` carries `src/**/*.zod.ts`) And the guard text itself reaches the packed artifact: 4 occurrences in each of `dist/data/index.mjs` and `dist/data/index.js`, against a control marker reading 0. ⇒ **A changeset is owed.** Grade **`patch`**: a bug fix in a released package, with no declared signature moved and nothing an author can write removed or renamed. `.changeset/17456-prototype-fallthrough-guards.md`. ⚠️ One sub-premise of the dispatch is falsified and recorded rather than acted on differently: the `issue-N-` filename spelling has **four** precedents in this tree, not zero (`issue-17400-…`, `issue-17461-…`, `issue-17574-…`, `issue-17595-…`). The directive's substance still holds — `card-slug` is the prevailing *numbered* spelling at 48 files — so the filename is unchanged. ## Verification - `pnpm --filter @objectstack/spec typecheck` — **OK**, including `check:test-typecheck` («54 file(s) / 259 error(s) / 144 pinned signature(s)» held, none of them in the three files this PR touches — a zero with a lit control: the same grep over the same log reads 259 for other test files). - `pnpm --filter @objectstack/spec test` — **`Test Files 470 passed | 1 skipped (471)` · `Tests 13378 passed | 1 skipped (13379)`**. - `pnpm lint` (`eslint . --no-inline-config`, repo-wide, ⛔ not narrowed) — **exit 0** at `b280ae299a`, the final commit. - **All 81 derived gate families** (`node scripts/pm/dispatch-gates.mjs`) — exit code captured **before any pipe** for every one. Reconciled: «81 derived, 81 run, **0 NOT-MEASURED**, 0 UNRUN … a DERIVED zero — all 81 recorded an exit code and none of them is 3». Eight needed a prerequisite first — one exit 3 and four exit 1 on a stale `dist`, plus three more exit 3 — and ⛔ none was folded into the green count: the closures were built (`turbo run build`, including the full `packages/*` sweep `lint.yml` performs before those steps) and each was re-run to a real exit 0. ## 验收备注 — out of scope, noted and filed ⭐ **The triage comment on #17456 asked this round to re-run the survey keyed on the index expression itself rather than on the `return TABLE[key]` spelling, and to report the count. Done, and the count is not five.** Keyed on «a table annotated `Record[string, X]` — the widest key type — that is a plain object literal, indexed with a non-literal key», over 944 non-test files in `packages/spec/src`: **76** such tables, **44** with at least one runtime-key index site. Four of those were driven against the built artifact. **Three are live and off-contract** — `normalizeFilterOperator` (`ui/view.zod.ts`, which indexes **twice**, raw and lower-cased, so it has no case-folding accident protecting it and returns the `Object` function for `constructor`, `toString` **and** `valueOf`), `resolveDiscoveryEnvironment` (`api/discovery.zod.ts`, whose docblock promises «a value guaranteed to satisfy DiscoveryEnvironmentSchema») and `pluralToSingular` (`meta-spelling/manifest-collection-spelling.ts`). One is **safe** and is the discriminating control: `suggestFieldType` reaches the inherited member but only truthiness-tests it before wrapping, so nothing off-contract escapes. ⛔ Not fixed here — the scope fence on both cards is exact and the triage comment states it. Filed as **#17818**, unassigned and unlabelled, carrying the survey method, the measured tables with their lit controls, and an explicit NOT MEASURED on the remaining 41 tables. Nothing else is withheld: no other finding was noted and not filed. --- _Generated by [Claude Code](https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH)_ --- _Generated by [Claude Code](https://claude.ai/code)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent ca16dc2 commit 4342c99

7 files changed

Lines changed: 328 additions & 3 deletions

File tree

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
fix(spec): three more lookups refuse an off-vocabulary key instead of handing back an `Object.prototype` member
6+
7+
`BASE_ALIASES` / `DIALECT_ALIASES` (`canonicalizeSqlType`),
8+
`DEFAULT_VALUE_TOKEN_SUGGESTIONS` (`suggestDefaultValueToken`) and
9+
`CONTEXT_TOKEN_SUGGESTIONS` (`classifyFilterToken`) are plain object literals, so
10+
all three inherit `Object.prototype`, and every lookup into them was a bare
11+
index. Measured by importing the BUILT artifact (`dist/data/index.mjs`) on the
12+
repo's Node 22 baseline (v22.22.2) and driving each function — the same way the
13+
two landed siblings in this family were measured — over a fixed population of
14+
five: `constructor`, `toString`, `valueOf`, `__proto__` and a plain unknown word.
15+
16+
| call | before | after |
17+
|:--|:--|:--|
18+
| `canonicalizeSqlType('varchar')` | `'text'` | `'text'` — unmoved |
19+
| `canonicalizeSqlType('timestamptz', 'postgres')` | `'datetime'` | `'datetime'` — unmoved |
20+
| `canonicalizeSqlType('constructor')` | the `Object` **function**, out of a signature that admits only `CanonicalSqlType` string literals | `'unknown'` |
21+
| `canonicalizeSqlType('constructor', <any dialect>)` | the `Object` **function** | `'unknown'` |
22+
| `canonicalizeSqlType('__proto__')` | `'array'` | `'array'` — unmoved; the array-notation rule answers ahead of either table |
23+
| `canonicalizeSqlType('toString' / 'valueOf' / 'nope')` | `'unknown'` | `'unknown'` — unmoved |
24+
| `suggestFieldTypeForSqlType('constructor')` | **`TypeError: Cannot read properties of undefined (reading 'suggested')`** | `undefined` |
25+
| `isCompatible('constructor', 'text')` | **`TypeError: … (reading 'exact')`** | `'lossy'` |
26+
| `suggestDefaultValueToken('currentuser')` | `'current_user'` | `'current_user'` — unmoved |
27+
| `suggestDefaultValueToken('constructor')` | the `Object` **function** | `undefined` |
28+
| `suggestDefaultValueToken('__proto__')` | `Object.prototype` — an **object** | `undefined` |
29+
| `classifyFilterToken('{current_user}').suggestion` | `'current_user_id'` | `'current_user_id'` — unmoved |
30+
| `classifyFilterToken('{constructor}').suggestion` | the `Object` **function**, in a field declared `ContextToken` | `undefined` |
31+
| `classifyFilterToken('{__proto__}').suggestion` | `Object.prototype` | `undefined` |
32+
33+
The two `TypeError` rows are the sharpest consequence and were not previously
34+
recorded: a non-`CanonicalSqlType` reaches `CANONICAL_TO_FIELD[canonical]`, which
35+
is `undefined`, so both published sibling accessors threw on the member read
36+
rather than merely returning something off-contract. `canonicalizeSqlType`'s
37+
`rawType` comes off live database introspection, which is where an
38+
attacker-free, entirely accidental `constructor` actually comes from.
39+
40+
`classifyFilterToken`'s half is the one a type-checked consumer meets: the
41+
declared `suggestion?: ContextToken` was a compile-time guarantee that was false
42+
at runtime, and nothing in the type system would ever have flagged it. Its
43+
wrapped-token regex captures `[^{}]+` — anything but braces — so the reachable
44+
key set is not the identifier-shaped one; what bounds it is the `toLowerCase()`,
45+
which leaves exactly the lower-case-stable prototype members (`constructor`,
46+
`__proto__`) namable today. `toString` / `valueOf` were quiet by that casing
47+
accident alone, not by a guard.
48+
49+
All three sites now go through an `Object.prototype.hasOwnProperty.call` check
50+
returning each function's own already-declared refusal value — `'unknown'`,
51+
`undefined`, and an absent `suggestion` respectively. No declared signature
52+
changes. This narrows and widens nothing an author can reach: every legal
53+
spelling is an own key of its table, so nothing accepted before is refused now,
54+
and only answers that were never inside the declared return types move.
55+
56+
A null-prototype table was the other available shape and is not taken, for the
57+
reason the two landed siblings measured rather than assumed: a `__proto__: null`
58+
object literal does not type-check against the `Record<…>` annotation at all
59+
(TS2353), and the `Object.assign(Object.create(null), …)` spelling that does
60+
compile silently costs that annotation's exhaustiveness check (TS2741 stopped
61+
firing for a table missing a member). A quiet failure is worse than a loud one.

packages/spec/src/data/context-tokens.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,3 +183,67 @@ describe('classifyFilterToken — brace-wrapped by intent (#5586)', () => {
183183
expect(classifyFilterToken(value)).toBeNull();
184184
});
185185
});
186+
187+
/**
188+
* The Object.prototype fall-through pin. Its POPULATION is the point: the
189+
* suite's other suggestion cases iterate the canonical near-miss vocabulary
190+
* only — precisely the population that behaves — which is why this site sat
191+
* green while `classifyFilterToken('{constructor}')` put the `Object` FUNCTION
192+
* into `suggestion`, whose declared type is `ContextToken`.
193+
*/
194+
describe('classifyFilterToken — Object.prototype fall-through in `suggestion`', () => {
195+
// Fixed at five. `toString` / `valueOf` are quiet only because the key is
196+
// lower-cased first (`tostring` names nothing); they are in the population
197+
// so that the accident is pinned as an outcome rather than trusted as a
198+
// guard.
199+
const POPULATION = ['constructor', 'toString', 'valueOf', '__proto__', 'nope'] as const;
200+
201+
it.each(POPULATION)('{%s} carries this branch\'s own declared refusal — an absent suggestion', (word) => {
202+
// `suggestion` is optional in the declared return, so its absence IS the
203+
// refusal value; ⛔ nothing was invented for the fix.
204+
expect(classifyFilterToken(`{${word}}`)).toEqual({ kind: 'unknown', token: word, suggestion: undefined });
205+
expect(classifyFilterToken(`\${${word}}`)).toEqual({ kind: 'unknown', token: word, suggestion: undefined });
206+
});
207+
208+
it('no spelling reachable through the wrapped-token regex yields a non-ContextToken suggestion', () => {
209+
// `FILTER_TOKEN_WRAPPED_RE` captures `[^{}]+` — anything but braces — so the
210+
// reachable key set is NOT the identifier-shaped one objectui's narrower
211+
// `[a-zA-Z0-9_]+` bounds. This sweep is over every own property name of
212+
// `Object.prototype`, which is the set a bracket index can resolve, rather
213+
// than over the two spellings that happen to be noisy today.
214+
for (const name of Object.getOwnPropertyNames(Object.prototype)) {
215+
const out = classifyFilterToken(`{${name}}`);
216+
expect(out).not.toBeNull();
217+
const suggestion = out && 'suggestion' in out ? out.suggestion : undefined;
218+
// Pins the SILENCE: either absent, or a declared member of the union.
219+
expect(suggestion === undefined || (CONTEXT_TOKENS as readonly string[]).includes(suggestion)).toBe(true);
220+
expect(typeof suggestion).not.toBe('function');
221+
expect(typeof suggestion).not.toBe('object');
222+
}
223+
});
224+
225+
it('lit control — a real near miss still gets its suggestion', () => {
226+
// Without this, a guard that refused everything would pass the cases above.
227+
expect(classifyFilterToken('{current_user}')).toEqual({
228+
kind: 'unknown',
229+
token: 'current_user',
230+
suggestion: 'current_user_id',
231+
});
232+
expect(classifyFilterToken('{org_id}')).toEqual({
233+
kind: 'unknown',
234+
token: 'org_id',
235+
suggestion: 'current_org_id',
236+
});
237+
});
238+
239+
it('lit control — the lookup table shadows no prototype member with an own key', () => {
240+
// The reading the fix rests on: the table cannot be "already safe" by
241+
// accident. If a row named `constructor` were ever added, the guard's
242+
// meaning changes and this goes red first.
243+
for (const name of Object.getOwnPropertyNames(Object.prototype)) {
244+
expect(Object.prototype.hasOwnProperty.call(CONTEXT_TOKEN_SUGGESTIONS, name)).toBe(false);
245+
}
246+
// Lit control for the control: a row that IS present.
247+
expect(Object.prototype.hasOwnProperty.call(CONTEXT_TOKEN_SUGGESTIONS, 'current_user')).toBe(true);
248+
});
249+
});

packages/spec/src/data/context-tokens.zod.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,5 +249,33 @@ export function classifyFilterToken(
249249
const token = m[1];
250250
if (isContextToken(token)) return { kind: 'context', token: token as ContextToken };
251251
if (isDateMacroToken(token)) return { kind: 'date-macro', token };
252-
return { kind: 'unknown', token, suggestion: CONTEXT_TOKEN_SUGGESTIONS[token.toLowerCase()] };
252+
// Own-property guard: the table is a plain object literal, so a bare index
253+
// resolves `Object.prototype`'s members for an off-vocabulary token —
254+
// `{constructor}` put the `Object` FUNCTION and `{__proto__}`
255+
// `Object.prototype` itself into `suggestion`, whose declared type is
256+
// `ContextToken`. A TypeScript consumer holds a compile-time guarantee that
257+
// is false at runtime, and nothing in the type system will ever flag it.
258+
//
259+
// The reach is not bounded by the identifier shapes: `FILTER_TOKEN_WRAPPED_RE`
260+
// captures `[^{}]+`, anything but braces. What bounds it is that the key is
261+
// lower-cased, so only the prototype members whose names are already
262+
// lower-case are namable — `constructor` and `__proto__` today. `toString` /
263+
// `valueOf` / `hasOwnProperty` are quiet by that casing accident alone, ⛔ not
264+
// by a guard, and a future lower-case prototype member would join the noisy
265+
// set silently.
266+
//
267+
// The refusal value is this branch's own declared one: `suggestion` is
268+
// optional, so its absence — `undefined` — is what "resembles nothing" already
269+
// means here. The guard narrows: every near-miss that answered before is an
270+
// own key.
271+
//
272+
// ⛔ Not a null-prototype table (TS2353 against the `Readonly<Record<…>>`
273+
// annotation, or a silent loss of its exhaustiveness check via
274+
// `Object.assign(Object.create(null), …)`) and ⛔ not a list of prototype
275+
// member names, which the next prototype member defeats.
276+
const lower = token.toLowerCase();
277+
const suggestion = Object.prototype.hasOwnProperty.call(CONTEXT_TOKEN_SUGGESTIONS, lower)
278+
? CONTEXT_TOKEN_SUGGESTIONS[lower]
279+
: undefined;
280+
return { kind: 'unknown', token, suggestion };
253281
}

packages/spec/src/data/default-value-shape.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ import {
1717
checkLiteralDefaultValue,
1818
discriminateDefaultValueShape,
1919
isExpressionEnvelopeDefault,
20+
suggestDefaultValueToken,
2021
type DefaultValueShape,
2122
} from './default-value-shape';
23+
import { DEFAULT_VALUE_TOKENS } from './default-value-tokens';
2224

2325
describe('#7127 discriminateDefaultValueShape — engine-parity classification', () => {
2426
const CASES: Array<{ label: string; dv: unknown; shape: DefaultValueShape }> = [
@@ -175,3 +177,40 @@ describe('#7127 checkLiteralDefaultValue — the shared stored-form literal chec
175177
expect(v.detail).not.toContain('Unrecognized key');
176178
});
177179
});
180+
181+
/**
182+
* The Object.prototype fall-through pin. Its POPULATION is the point: the
183+
* suite's other suggestion cases iterate the canonical near-miss vocabulary
184+
* only — precisely the population that behaves — which is why this site sat
185+
* green while `suggestDefaultValueToken('constructor')` returned the `Object`
186+
* FUNCTION and `suggestDefaultValueToken('__proto__')` returned
187+
* `Object.prototype`, out of a signature declared `DefaultValueToken |
188+
* undefined`.
189+
*/
190+
describe('suggestDefaultValueToken — Object.prototype fall-through', () => {
191+
// Fixed at five. `toString` / `valueOf` are quiet here only because the key
192+
// is lower-cased first, which is an accident of casing and ⛔ not a guard —
193+
// they belong in the population precisely so the day that stops being true
194+
// is a red test and not a silent regression.
195+
const POPULATION = ['constructor', 'toString', 'valueOf', '__proto__', 'nope'] as const;
196+
197+
it.each(POPULATION)('%s answers this function\'s own declared refusal value', (word) => {
198+
// `undefined` is read from the declared return type, ⛔ not invented here.
199+
expect(suggestDefaultValueToken(word)).toBeUndefined();
200+
});
201+
202+
it.each(POPULATION)('%s never yields a non-string, whatever the answer is', (word) => {
203+
// Pins the SILENCE rather than today's `undefined`: should a prototype
204+
// member ever be shadowed by a real own key, this still holds; what it
205+
// refuses is a function or an object escaping the string union.
206+
const out = suggestDefaultValueToken(word);
207+
expect(out === undefined || DEFAULT_VALUE_TOKENS.includes(out)).toBe(true);
208+
});
209+
210+
it('lit control — the real near-miss vocabulary still answers', () => {
211+
// Without this, a guard that refused everything would pass the cases above.
212+
expect(suggestDefaultValueToken('currentuser')).toBe('current_user');
213+
expect(suggestDefaultValueToken('{now}')).toBe('NOW()');
214+
expect(suggestDefaultValueToken('current_time')).toBe('NOW()');
215+
});
216+
});

packages/spec/src/data/default-value-shape.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,5 +294,25 @@ export function suggestDefaultValueToken(dv: unknown): DefaultValueToken | undef
294294
// guard is a type error, not just awkward.
295295
const key = dv.trim().toLowerCase();
296296
if (isRuntimeDefaultToken(dv)) return undefined;
297+
// Own-property guard: the table is a plain object literal, so a bare index
298+
// resolves `Object.prototype`'s members for an off-vocabulary `dv` —
299+
// `constructor` handed back the `Object` FUNCTION and `__proto__`
300+
// `Object.prototype` itself, out of a signature that admits only a
301+
// `DefaultValueToken` string or `undefined`. `toString` / `valueOf` are quiet
302+
// here only because `key` is lower-cased first (`tostring` names nothing),
303+
// which is an accident of casing and ⛔ not a guard — it does not cover the
304+
// two prototype members whose names are already lower-case, and it would not
305+
// cover a future one.
306+
//
307+
// The refusal value is this function's own declared one, `undefined`, read
308+
// from the return type. The guard narrows: every suggestion that answered
309+
// before is an own key.
310+
//
311+
// ⛔ Not a null-prototype table (TS2353 against the annotation, or a silent
312+
// loss of its exhaustiveness check via `Object.assign(Object.create(null), …)`;
313+
// measured in `src/data/driver/config-registry.zod.ts`'s sibling guard) and
314+
// ⛔ not a list of prototype member names, which the next prototype member
315+
// defeats.
316+
if (!Object.prototype.hasOwnProperty.call(DEFAULT_VALUE_TOKEN_SUGGESTIONS, key)) return undefined;
297317
return DEFAULT_VALUE_TOKEN_SUGGESTIONS[key];
298318
}

packages/spec/src/data/type-compat.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
canonicalizeSqlType,
66
suggestFieldTypeForSqlType,
77
isCompatible,
8+
type SqlDialect,
89
} from './type-compat';
910
import { suggestFieldType } from '../shared/suggestions.zod';
1011

@@ -154,3 +155,88 @@ describe('isCompatible', () => {
154155
expect(isCompatible('geometry', 'number')).toBe(false);
155156
});
156157
});
158+
159+
/**
160+
* The Object.prototype fall-through pin. Its POPULATION is the point: the
161+
* suite's other cases iterate the canonical vocabulary only, which is
162+
* precisely the population that behaves, and that is why this site sat green
163+
* while `canonicalizeSqlType('constructor')` returned the `Object` FUNCTION
164+
* out of a signature that admits only `CanonicalSqlType` string literals.
165+
*
166+
* `rawType` is uncontrolled — it arrives from live database introspection —
167+
* so the population below is the reachable one, not a contrived one.
168+
*/
169+
describe('canonicalizeSqlType — Object.prototype fall-through', () => {
170+
// Fixed at five: the three prototype members a lower-cased key can name or
171+
// nearly name, the assignment-shaped one, and a plain unknown word that
172+
// names nothing at all. Four is not four-fifths of this pin.
173+
const POPULATION = ['constructor', 'toString', 'valueOf', '__proto__', 'nope'] as const;
174+
// Every member of the declared `SqlDialect` union, so the dialect half of the
175+
// guarded line is covered for each table it can select — including the two
176+
// the union declares but `DIALECT_ALIASES` does not populate, which fall
177+
// through to the base map and must answer just as safely.
178+
const DIALECTS: readonly SqlDialect[] = [
179+
'postgres', 'mysql', 'sqlite', 'snowflake', 'bigquery', 'mongo',
180+
];
181+
182+
// The declared return type is a private union, so the pin names it here.
183+
// ⛔ This list pins the SIGNATURE, not today's answers: a member added to
184+
// `CanonicalSqlType` belongs here too.
185+
const CANONICAL: readonly string[] = [
186+
'text', 'integer', 'bigint', 'decimal', 'float', 'boolean', 'date', 'time',
187+
'datetime', 'json', 'uuid', 'binary', 'enum', 'array', 'vector', 'unknown',
188+
];
189+
190+
it.each(POPULATION)('%s resolves to a declared CanonicalSqlType, never a prototype member', (word) => {
191+
// The assertion is on the SHAPE of the answer, not on which word it is:
192+
// what the defect produced was a `function`, and pinning "is a declared
193+
// member of the union" survives a vocabulary change that pinning the
194+
// string `'unknown'` would break.
195+
expect(typeof canonicalizeSqlType(word)).toBe('string');
196+
expect(CANONICAL).toContain(canonicalizeSqlType(word));
197+
for (const dialect of DIALECTS) {
198+
expect(typeof canonicalizeSqlType(word, dialect)).toBe('string');
199+
expect(CANONICAL).toContain(canonicalizeSqlType(word, dialect));
200+
}
201+
});
202+
203+
it('`constructor` is refused with this function\'s own declared refusal value', () => {
204+
// `'unknown'` is the trailing `return` of the function itself, ⛔ not a
205+
// value invented for the fix.
206+
expect(canonicalizeSqlType('constructor')).toBe('unknown');
207+
for (const dialect of DIALECTS) expect(canonicalizeSqlType('constructor', dialect)).toBe('unknown');
208+
});
209+
210+
it('`__proto__` answers `array` from the array-notation rule, ahead of either table', () => {
211+
// Not a fall-through: `__proto__` starts with `_`, which is Postgres array
212+
// notation (`_int4`), and that branch returns before any lookup. Recorded
213+
// so a later reader does not mistake a legitimate declared answer for the
214+
// defect, and so the array rule cannot be quietly dropped.
215+
expect(canonicalizeSqlType('__proto__')).toBe('array');
216+
expect(canonicalizeSqlType('_int4')).toBe('array');
217+
});
218+
219+
it('the published sibling accessors stay total over the same population', () => {
220+
// The defect was not confined to this function's own return: a
221+
// non-`CanonicalSqlType` reaches `CANONICAL_TO_FIELD[canonical]`, which is
222+
// `undefined`, and both accessors below threw a TypeError on the member
223+
// read. That is the consequence a plain-JS caller actually meets.
224+
for (const word of POPULATION) {
225+
expect(() => suggestFieldTypeForSqlType(word)).not.toThrow();
226+
expect(() => isCompatible(word, 'text')).not.toThrow();
227+
for (const dialect of DIALECTS) {
228+
expect(() => suggestFieldTypeForSqlType(word, dialect)).not.toThrow();
229+
expect(() => isCompatible(word, 'text', dialect)).not.toThrow();
230+
}
231+
}
232+
});
233+
234+
it('lit control — the canonical vocabulary is untouched by the guard', () => {
235+
// If the guard narrowed anything it should not, these go red. Both halves
236+
// of the guarded line are represented: the base map and a dialect map.
237+
expect(canonicalizeSqlType('varchar')).toBe('text');
238+
expect(canonicalizeSqlType('numeric(10,2)')).toBe('decimal');
239+
expect(canonicalizeSqlType('timestamptz', 'postgres')).toBe('datetime');
240+
expect(canonicalizeSqlType('objectid', 'mongo')).toBe('text');
241+
});
242+
});

0 commit comments

Comments
 (0)