Skip to content

Commit e7f0fa7

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-16882-connect-agent-reachability-copy
2 parents 0f3f9d4 + 65ad77d commit e7f0fa7

6 files changed

Lines changed: 1863 additions & 2 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
fix(spec): the driver-config registry refuses an off-vocabulary id instead of answering with a truthy non-schema
6+
7+
`DRIVER_CONFIG_JSON_SCHEMAS`, `DRIVER_ID_ALIASES` and `DATABASE_DRIVER_ALIASES`
8+
are plain object literals, so all three inherit `Object.prototype`, and every
9+
lookup into them was a bare index. Measured against the built artifact
10+
(`dist/data/index.mjs`) on the repo's Node 22 baseline (v22.22.2), an id that
11+
names an inherited member resolved that member and was handed onward as if it
12+
were a driver:
13+
14+
| call | before | after |
15+
|:--|:--|:--|
16+
| `getDriverConfigJsonSchemaById('memory')` | the JSON Schema | the JSON Schema — unmoved |
17+
| `getDriverConfigJsonSchemaById('constructor')` | `{}` — an EMPTY JSON Schema that accepts every config | `TypeError` naming the id and the legal vocabulary |
18+
| `getDriverConfigJsonSchemaById('toString')` | `'[object Object]'` — a **string**, where the signature promises an object | `TypeError` |
19+
| `getDriverConfigJsonSchemaById('valueOf')` | the registry object itself | `TypeError` |
20+
| `getDriverConfigJsonSchemaById('__proto__')` | `TypeError: … is not a function` | `TypeError`, now naming the id |
21+
| `getDriverConfigJsonSchemaById('nope')` | `TypeError: … is not a function` | `TypeError`, now naming the id |
22+
| `resolveDriverId('constructor')` | the `Object` **function** — truthy, not a driver id | `undefined` |
23+
| `resolveDriverId('__proto__')` | `Object.prototype` — a truthy object | `undefined` |
24+
| `resolveDatabaseDriverId('constructor')` | the `Object` **function** | `undefined` |
25+
| `driverHasLocalDefault('constructor')` | `undefined`, out of a function declared `boolean` | `true`, as its doc promises for an unknown id |
26+
| `resolveDriverId('pg')` / `resolveDriverId(' PostgreSQL ')` | `'postgres'` | `'postgres'` — unmoved |
27+
28+
`getDriverConfigJsonSchemaById` handing back `{}` is the worst of these: an
29+
empty JSON Schema validates anything, so a Studio connection form or a
30+
`DriverDefinitionSchema.configSchema` consumer that asked "what shape must this
31+
config have" was told "any shape at all" and reported success.
32+
33+
The resolvers' half is reachable without a plain-JS consumer. The CLI refuses an
34+
unclaimed operator selection with `if (driverType && !kind)` after calling
35+
`resolveDatabaseDriverId`, so `OS_DATABASE_DRIVER=constructor` produced a truthy
36+
`kind` that is not a driver id and walked past the refusal.
37+
38+
All three lookups now go through an `Object.prototype.hasOwnProperty.call` check.
39+
This narrows and widens nothing: every legal spelling is an own key of its table,
40+
so no value accepted before is refused now, and only answers that were never
41+
inside the declared return types move. The declared signatures are unchanged —
42+
`getDriverConfigJsonSchemaById` stays `(id: BuiltinDriverId) => Record<string, unknown>`
43+
and both resolvers stay `(driver: unknown) => BuiltinDriverId | undefined`.
44+
45+
A null-prototype table was the other available shape and was measured rather than
46+
assumed: a `__proto__: null` object literal does not type-check against the
47+
`Readonly<Record<…>>` annotation at all (TS2353), and the
48+
`Object.assign(Object.create(null), …)` spelling that does compile silently costs
49+
that annotation — a table missing a driver stopped failing to compile (TS2741).

.github/workflows/lint.yml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2407,6 +2407,26 @@ jobs:
24072407
- name: Quick-reference section counts match their tables
24082408
run: pnpm check:quick-reference-counts
24092409

2410+
# #17388: three hand-written pages enumerate @objectstack/spec's importable
2411+
# subpaths or its protocol namespaces, and nothing read any of them. When
2412+
# #16325 step 3 deleted the ./cloud exports entry and added ./marketplace,
2413+
# every MECHANICAL counter of that set moved with it -- the anti-vacuity
2414+
# floor pin, the alias-coverage pin, export-origins, root-meta, llms.txt,
2415+
# quick-reference counts, ~25 in all -- and all three prose enumerations
2416+
# kept advertising cloud and never listed marketplace, until a human swept
2417+
# for them. Step 2 of that same chain was FAILed in review by exactly this
2418+
# class of unlisted counter.
2419+
# It holds TWO derived sets, not one: the exports map's 16 bare ./<name>
2420+
# entries, and the 15 of those whose declared CATEGORY_TITLES title ends in
2421+
# " Protocol" (meta-spelling is a subpath and is titled "Vocabulary", not a
2422+
# protocol). Display names come from that same declaration, so the #5853
2423+
# "Qa Protocol" class of wrong-but-stable spelling is a finding here too.
2424+
# It lives in this job with the other docs guards: the edit that breaks it
2425+
# is a docs edit, so a packages/** paths filter would blind it to its own
2426+
# failure mode.
2427+
- name: Hand-written spec enumerations match the exports map
2428+
run: pnpm check:docs-spec-enumerations
2429+
24102430
# #11752: the "Overlay whitelist (shared-DB tenancy invariant)" table in
24112431
# metadata-lifecycle.mdx is a hand-kept copy of DEFAULT_METADATA_TYPE_REGISTRY
24122432
# sitting directly under the sentence that calls the registry the single

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
"check:corpus-claim-drift": "node scripts/check-corpus-claim-drift.mjs --self-test && node scripts/check-corpus-claim-drift.mjs",
6262
"check:skill-identifier-liveness": "node scripts/check-skill-identifier-liveness.mjs --self-test && node scripts/check-skill-identifier-liveness.mjs",
6363
"check:quick-reference-counts": "node scripts/check-quick-reference-counts.mjs --self-test && node scripts/check-quick-reference-counts.mjs",
64+
"check:docs-spec-enumerations": "node scripts/check-docs-spec-enumerations.mjs --self-test && node scripts/check-docs-spec-enumerations.mjs",
6465
"check:overlay-whitelist-table": "node scripts/check-overlay-whitelist-table.mjs --self-test && node scripts/check-overlay-whitelist-table.mjs",
6566
"check:runtime-services-index": "node scripts/check-runtime-services-index.mjs --self-test && node scripts/check-runtime-services-index.mjs",
6667
"check:skill-frame-sync": "node scripts/check-skill-frame-sync.mjs --self-test && node scripts/check-skill-frame-sync.mjs",

packages/spec/src/data/driver/config-registry.test.ts

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@ import { describe, it, expect } from 'vitest';
55
import { DatasourceSchema } from '../datasource.zod';
66
import {
77
BUILTIN_DRIVER_IDS,
8+
type BuiltinDriverId,
89
DATABASE_DRIVER_SELECTION_ALIASES,
910
DATABASE_DRIVER_SELECTION_IDS,
1011
DRIVER_CONFIG_SCHEMAS,
1112
DRIVER_ID_ALIASES,
13+
driverHasLocalDefault,
1214
getDriverConfigJsonSchemaById,
1315
getDriverConfigSchema,
1416
resolveDatabaseDriverId,
@@ -222,3 +224,149 @@ describe('DATABASE_DRIVER_SELECTION_IDS — what a boot flag may offer (#6969)',
222224
expect(Object.isFrozen(DATABASE_DRIVER_SELECTION_IDS)).toBe(true);
223225
});
224226
});
227+
/**
228+
* The OFF-VOCABULARY population — the one the pins above cannot reach (#16903).
229+
*
230+
* ⚠ Every existing case in this file iterates `BUILTIN_DRIVER_IDS`,
231+
* `DRIVER_ID_ALIASES` or a hand-written canonical spelling: exactly the
232+
* population that behaves. They were all green while
233+
* `getDriverConfigJsonSchemaById('constructor')` returned `{}` — an EMPTY JSON
234+
* Schema that accepts every config it is asked to judge — and while
235+
* `resolveDriverId('constructor')` returned the `Object` FUNCTION out of a
236+
* signature that says `BuiltinDriverId | undefined`. A pin over canonical ids
237+
* alone would be green before and after the guard and would prove nothing, so
238+
* the population is the point of this describe.
239+
*/
240+
describe('driver lookups — an OFF-vocabulary id is refused, never answered with a non-schema (#16903)', () => {
241+
/**
242+
* The consumer that actually reaches these, spelled as the cast it is.
243+
* `getDriverConfigJsonSchemaById` and both resolvers are published
244+
* (`packages/spec/api-surface/data.json`), so "unreachable in-repo" is not
245+
* "unreachable": a plain-JS consumer arrives with zero type checking, and for
246+
* the resolvers the id can also arrive from `OS_DATABASE_DRIVER` or from
247+
* authored `datasource.driver` metadata — which is exactly where
248+
* `constructor` and `toString` show up.
249+
*/
250+
const untypedJsonSchema = (id: string): unknown => getDriverConfigJsonSchemaById(id as BuiltinDriverId);
251+
252+
/**
253+
* Words that resolve an INHERITED member, grouped by what the bare lookup did
254+
* with each before the own-property guard. Measured against the built
255+
* artifact (`dist/data/index.mjs`) on the Node 22 baseline (v22.22.2).
256+
*/
257+
const PROTOTYPE_RESOLVABLE_CALLABLE = [
258+
// Returned a truthy NON-schema — the silent wrong answers, and the reason
259+
// this is a bug rather than a tidy-up. `constructor` ran `Object()` and gave
260+
// `{}`; `toString` gave the STRING '[object Object]'; `valueOf` gave the
261+
// registry object itself.
262+
'constructor',
263+
'toString',
264+
'valueOf',
265+
// Returned a `boolean` (each called with no argument, receiver = the
266+
// registry) where the signature promises an object — which is why a
267+
// truthiness assertion alone cannot catch this family either.
268+
'hasOwnProperty',
269+
'isPrototypeOf',
270+
'propertyIsEnumerable',
271+
];
272+
273+
/**
274+
* Words with no own key AND nothing callable behind them: `__proto__`
275+
* resolved `Object.prototype`, the rest resolved `undefined`. Both spellings
276+
* already threw a `TypeError` off a non-callable, so these are the CONTROLS —
277+
* the guard must not invent a new failure for input that already failed.
278+
*/
279+
const ALREADY_THREW = ['__proto__', 'nope', '', 'com.vendor.snowflake'];
280+
281+
const OFF_VOCABULARY = [...PROTOTYPE_RESOLVABLE_CALLABLE, ...ALREADY_THREW];
282+
283+
it('holds this population HONEST — every word above is outside the vocabulary', () => {
284+
// Without this, a spelling promoted into the table would leave every pin
285+
// below asserting a refusal for a LEGAL id, and they would go on passing
286+
// while meaning the opposite of what they say.
287+
for (const word of OFF_VOCABULARY) {
288+
expect(BUILTIN_DRIVER_IDS as readonly string[], word).not.toContain(word);
289+
expect(Object.prototype.hasOwnProperty.call(DRIVER_ID_ALIASES, word), word).toBe(false);
290+
}
291+
});
292+
293+
it('throws for an id naming a callable Object.prototype member, instead of returning a non-schema', () => {
294+
for (const id of PROTOTYPE_RESOLVABLE_CALLABLE) {
295+
expect(() => untypedJsonSchema(id), id).toThrow(TypeError);
296+
}
297+
});
298+
299+
it('still throws for a plainly absent id, exactly as it always did', () => {
300+
// The control: `__proto__` and an unknown word threw a `TypeError` before
301+
// the guard too. The guard is a narrowing, so this assertion must be green
302+
// on both sides of it — if it moves, the change did more than close a hole.
303+
for (const id of ALREADY_THREW) {
304+
expect(() => untypedJsonSchema(id), id).toThrow(TypeError);
305+
}
306+
});
307+
308+
it('names the offending id and the legal vocabulary in every refusal', () => {
309+
// A bare `.toThrow()` is not a refusal assertion here: two of these words
310+
// already threw. What distinguishes a REFUSAL from the old incidental
311+
// `… is not a function` is that the message names the subject and what was
312+
// expected instead.
313+
for (const id of OFF_VOCABULARY) {
314+
let message = '';
315+
try {
316+
untypedJsonSchema(id);
317+
} catch (error) {
318+
message = (error as Error).message;
319+
}
320+
expect(message, id).toContain('getDriverConfigJsonSchemaById');
321+
expect(message, id).toContain(JSON.stringify(id));
322+
for (const canonical of BUILTIN_DRIVER_IDS) {
323+
expect(message, `${id}${canonical}`).toContain(canonical);
324+
}
325+
}
326+
});
327+
328+
it('still answers every canonical id with its own JSON Schema, unmoved', () => {
329+
// The narrowing must stop at the vocabulary edge: the guard refuses more and
330+
// accepts nothing new, so every in-vocabulary answer is byte-identical.
331+
for (const id of BUILTIN_DRIVER_IDS) {
332+
const json = getDriverConfigJsonSchemaById(id) as { type?: string; properties?: object };
333+
expect(json.type, id).toBe('object');
334+
expect(json.properties, id).toBeTruthy();
335+
}
336+
// …and the memoised identity survives the guard.
337+
expect(getDriverConfigJsonSchemaById('postgres')).toBe(getDriverConfigJsonSchemaById('postgres'));
338+
});
339+
340+
it('resolves an off-vocabulary spelling to `undefined`, never to a truthy non-id', () => {
341+
// `resolveDriverId('constructor')` returned the `Object` FUNCTION and
342+
// `resolveDriverId('__proto__')` returned `Object.prototype` — both truthy,
343+
// neither a `BuiltinDriverId`, out of a signature that admits only
344+
// `BuiltinDriverId | undefined`.
345+
for (const word of OFF_VOCABULARY) {
346+
expect(resolveDriverId(word), word).toBeUndefined();
347+
expect(resolveDatabaseDriverId(word), word).toBeUndefined();
348+
}
349+
});
350+
351+
it('answers `true` for an off-vocabulary driver in driverHasLocalDefault, never `undefined`', () => {
352+
// The declared return is `boolean` and the doc promises `true` for an id the
353+
// table does not know. A truthy non-id from `resolveDriverId` used to index
354+
// `DRIVER_LOCAL_DEFAULT` to `undefined`, so `constructor` and `__proto__`
355+
// came back `undefined` out of a function declared `boolean`.
356+
for (const word of OFF_VOCABULARY) {
357+
expect(typeof driverHasLocalDefault(word), word).toBe('boolean');
358+
expect(driverHasLocalDefault(word), word).toBe(true);
359+
}
360+
});
361+
362+
it('still answers every canonical id from the vocabulary table, unmoved', () => {
363+
// The other side of the same edge, for the resolvers.
364+
for (const id of BUILTIN_DRIVER_IDS) {
365+
expect(resolveDriverId(id), id).toBe(id);
366+
expect(resolveDatabaseDriverId(id), id).toBe(id);
367+
}
368+
expect(resolveDriverId(' PostgreSQL ')).toBe('postgres');
369+
expect(resolveDriverId('sqlite3')).toBe('sqlite');
370+
expect(resolveDatabaseDriverId('sqlite3')).toBeUndefined();
371+
});
372+
});

0 commit comments

Comments
 (0)