Skip to content

Commit f9ebb7d

Browse files
fix(plugin-auth): the identity import asks each lookup target its declared exposure before matching a cell (#22800) (#22866)
Fixes #22800 Clause-②: no (narrowing) This PR carries census row 9 of #22661 (seat 1 ticks the parent's checklist and closes the parent; this PR does not). The identity import handed `runImport` a run protocol without `getMetaItem`, so the reference resolution matched its lookup cells without asking the lookup target its declared exposure. The import already held that reader and gave it to the parser a few lines earlier. Classes, positions and functions only, per the card. ## What changed - **One reader, two halves.** `runAdminImportUsers` (`packages/plugins/plugin-auth/src/admin-import-users.ts`) binds the declaration reader it already holds (`IdentityImportDeps.getMetaItem`) once, as `metaReader`. It hands `metaReader` to `prepareImportRequest`, which reads `sys_user`'s fields, so lookup cells become references at all. It also spreads `metaReader` into the run protocol (`const protocol: ImportProtocolLike`). With one binding the two cannot disagree: whenever a cell is a reference, its target is judged. - **The decision is the family's, unchanged.** `servesReferenceTarget` (`packages/core/src/utils/import-runner.ts`, landed by #22739) now asks this door's targets for `list`, the same way it asks for every other `runImport` caller. There is no second rule, and no code in `@objectstack/core` or `@objectstack/rest` changes; the one `@objectstack/core` edit is a TSDoc sentence (see Acceptance notes). The import's reads run under the system identity. That gives no exemption, because the decision takes no caller. - **The answer for a refusing target** is the precedent's answer. Every cell gets `reference_not_found`: one naming a record, one naming none, and a pasted id. Nothing of the target is read. A target with no `enable` block, or with a whitelist that grants `list`, resolves exactly as before. No new error code, no status change. - **Signature measured, not adapted.** `deps.getMetaItem` takes `{ type: string; name: string }`. `GetMetaItemRequest` requires exactly those two members, and the rest are optional. So the dep is assignable to `ImportProtocolLike.getMetaItem` by parameter contravariance, and its promise of `any` is assignable to a promise of `unknown`. `tsc` is green and no adapter was written. The producer is `auth-plugin.ts`, which forwards the protocol service's `getMetaItem`. That method answers the `{ type, name, item }` envelope `servesReferenceTarget` reads (`item.enable`), the same envelope the parser already reads (`item`). ## Measured first: which targets `sys_user`'s reference fields name Read on `origin/main` `eed637c09f`, from `packages/platform-objects/src/identity/sys-user.object.ts`: | field | target | target's declaration | serves `list` | |---|---|---|---| | `manager_id` | `sys_user` | `apiEnabled: true`, `apiMethods: ['get', 'list', 'update', 'bulk']` | yes | | `primary_business_unit_id` | `sys_business_unit` | `apiEnabled: true`, no `apiMethods` (default-open) | yes | - **The null.** No current target refuses `list`, so no shipped import answer changes. The wiring is pinned anyway, as the card directs, so a future refusing declaration is judged. - **The registry injects nothing into `sys_user`.** It is `managedBy: 'better-auth'`, and `resolveInjectedSystemColumns` returns no columns for that row. So these two lookups are the whole reference population. - **Grep control.** The same pattern found the two `Field.lookup` calls plus one inline action param (`type: 'lookup'`, a dialog param and not a field). No in-repo object extension targets `sys_user`. - **The manager column never reaches `runImport`.** The import strips `manager_id` before the run, and its own second pass links managers. In practice, `primary_business_unit_id` is the cell this door matches. - **Not measured:** deployed or cloud-held extensions of `sys_user`. ## Pins All in the `[#22800]` block of `packages/plugins/plugin-auth/src/admin-import-users.test.ts` (6 cases). They use fixture objects only and reuse the file's already-pinned engine double, with only `find` overridden. The block adds no new engine double and leaves the double ledger untouched. - Two refusing declarations, `apiEnabled: false` and a whitelist granting `get` but not `list`. A match, a miss and an id all answer `reference_not_found`. The target is never read, and no user is created. - Two serving controls, no `enable` block and a whitelist granting `list`. A match and an id resolve, and a miss is reported, as before. - The target is asked through the import's reader, once per import, by its own name. - A control without a declaration reader: no cell is a reference, so the target is neither asked nor read. The decision's own matrix (every declaration, fail-closed) stays pinned once, in `packages/core/src/utils/import-runner-reference-exposure.test.ts`. **Census pin, row 9.** `packages/core/src/security/second-object-read-exposure.pin.test.ts` had no row for this door to move. The identity import never calls `referenceTargetOf`. It takes its targets from the `import-field-meta.ts` map, which is already classified `decided`. So the door is named in the docblock, in the shape row 8 used for the MCP stdio bridge: it reaches a classified read without calling the arbiter, and it is held by its own package's pin. No `CALLERS` row is added. A row there would turn the pin's "classified files still call the arbiter" leg red. ## Ablation Run on committed `49e162847a` through `scripts/ablation-replace.mjs`. The anchor was the protocol's `...metaReader,` spread (deleted), hit 1 → 0. The blob went `7be1a3b399e7` → `c5bf2e31b008`. The subject is imported by relative path from source, so no `dist/` hop is involved. - With the wiring removed, 3 cases go red and 3 stay green. The red ones are both refusing legs (the match and id cells come back `ok: true`, the pre-fix behaviour) and the "asked through the reader" leg (only `sys_user` was asked). The green ones are the two serving controls and the no-reader control. - Restore is proven: the blob equals HEAD (`7be1a3b399e7`), and `git diff HEAD` is empty. - The same ablation on the first head `574f795aee`, with the pins in a separate file, gave the same 3 / 3. ## Verification Each reading names its commit. The final head is `8c40d537f4`. That commit changes the changeset and one TSDoc sentence in `import-runner.ts`; `plugin-auth`'s source is unchanged since `49e162847a` (blob `7be1a3b399e7`), so the `plugin-auth` readings and the ablation below stand. - **Build.** `turbo run build --filter='@objectstack/plugin-auth^...'` ran 27 tasks, all successful, at `574f795aee`. The sources of those 27 are unchanged since. `@objectstack/plugin-auth` was built at `4405bc2cb0`, and `admin-import-users.ts` is unchanged since (blob `7be1a3b399e7`). - **`@objectstack/plugin-auth` at `49e162847a`.** `vitest run`: 136 files, 2786 passed, 10 skipped. `typecheck` exit 0, including `check:test-typecheck`. The `[#22800]` block adds no test-layer error: the file's 11 ledgered errors all sit above it. - **`@objectstack/core` at `8c40d537f4`.** Rebuilt; `dist/index.d.ts` carries the reworded sentence. The census pin passes 4 / 4 (`--project repo`), the 12 `import-runner*.test.ts` files pass 138 / 138, and `typecheck` exits 0. - **Identity import files at `49e162847a`.** `admin-import-users.test.ts` and `admin-import-users-manager-pass.test.ts` together: 56 / 56. - **Lint, narrowed and proven (`8c40d537f4`).** `eslint --no-inline-config --format json` on the 4 changed `.ts` files gives 4 files, 0 errors, 0 warnings. Each file is in the linted population: `isPathIgnored` is false and a config resolves for each. `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`), so this diff cannot move the verdict on any untouched file. Repo-wide `pnpm lint` is CI's. - **Gates at `8c40d537f4`.** `dispatch-gates --commands` derived 68 commands: the earlier 67 plus `check:error-code-casing`, now that `import-runner.ts` is in the diff. All 68 exited 0. `check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET) on its first run in the battery and 0 when re-run once the workspace was built. `--ran` with exit codes: 68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN. The roster families keyed to these paths (`check-changeset-fixed`, `check:auth-mount-ledger`, `check:authz-resolver`, `check:filter-alias-parity`) all exited 0. `check-adr-0087-registration` accepts the changeset as `[BREAKING+bang+clause-②-narrowing] not-required (no-migration-prescription)`. `check-changeset-no-major`, given a `pull_request` payload carrying `Clause-②: no (narrowing)`, exits 0. - **Not merged with `origin/main`.** Main has moved 9 commits (to `8532783f65`) with no overlap with this PR's paths. Two gate inputs changed there: `scripts/engine-double-contract.pinned.json` dropped two rows for a removed `plugin-security` test and none of this file's rows, and `scripts/cross-package-test-inputs.mjs` declared two inputs for a `cli` test. CI's merge ref validates the joint tree. ## Acceptance notes - **One TSDoc sentence in `@objectstack/core`, reworded here.** The `ImportProtocolLike.getMetaItem` docblock (`packages/core/src/utils/import-runner.ts`, `:214`) gave "plugin-auth's identity import" as the example of a protocol without the member. That sentence now states the rule without the example and says the identity import carries the member (#22800). It ships in core's `dist/index.d.ts`. No other line of the file is touched. The `:204` mention concerns `validateData`, which the identity import still omits, so it stays true. A comment changes no declaration, so `@objectstack/core` takes no changeset entry. - **The pending #22739 changeset says the opposite of this one.** `.changeset/22739-import-reference-target-exposure.md`, in its Bindings bullet, says the identity import "is not judged". Neither changeset has been released yet, so the next CHANGELOG would carry both sentences. This PR's changeset states the change as FROM → TO, so a reader sees the order. Carrier: whoever compiles the release notes; no seat is named. - **Clause-② is `no (narrowing)`**, per the seat's answer on this card. The family spelled this narrowing with the arm both times (#22739, the MCP bridge row), and a narrowing at zero measured producers is still a narrowing. The changeset is BREAKING (`fix(plugin-auth)!:`), `minor`, and carries an ADR-0087 `not-required (no-migration-prescription)` disposition. - The other residual, a future `runImport` caller without the member, stays with the parent's close-out, as the card says. --- _Generated by [Claude Code](https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6d5191e commit f9ebb7d

5 files changed

Lines changed: 171 additions & 6 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/plugin-auth': minor
3+
---
4+
5+
fix(plugin-auth)!: the identity import asks each lookup target its declared exposure before matching a cell, like every other `runImport` caller (#22800)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export or stored shape is removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is one runtime read: the identity import stops matching cells against a lookup target whose existing `enable` declaration already refuses `list` on every data route (the remedy is the declaration the author already wrote). The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and no exported declaration is removed or narrowed (not runtime-interface-only or type-surface-only); `IdentityImportDeps` keeps its shape and only its doc comment changes. -->
10+
11+
**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
12+
13+
The identity import (`runAdminImportUsers`) parses its payload with `prepareImportRequest` and runs its rows through `runImport`, with a run protocol of its own that writes through better-auth. Its `sys_user` field map, which decides which cells are references, is read through the import's declaration reader (`IdentityImportDeps.getMetaItem`).
14+
15+
**FROM.** The run protocol was built without that reader, so `runImport`'s `servesReferenceTarget` answered "served" without asking: a lookup cell was matched against its TARGET object even when the target's declaration refuses `list` (`enable.apiEnabled: false`, or an `apiMethods` whitelist without `list`). A cell naming an existing record resolved and a cell naming none answered `reference_not_found`, so the row report told the two apart.
16+
17+
**TO.** The run protocol carries the same reader the parser is handed (one binding, so whenever a cell is a reference its target is judged). A target the spec's decision (`canServeApiOperation`, for `list`) does not serve answers every cell, naming a record or not, a pasted id included, `reference_not_found`, and nothing of the target is read. A target with no `enable` block, or a whitelist that grants `list`, resolves exactly as before. The decision takes no caller, so the system identity the import's reads run under is answered like any other caller. No new error code, no status change.
18+
19+
**Measured producers.** `sys_user` declares two lookups: `manager_id` into `sys_user` (the import strips that column before `runImport`; its own second pass links managers) and `primary_business_unit_id` into `sys_business_unit`. Both targets serve `list`, and the registry injects no columns into `sys_user` (`managedBy: 'better-auth'`), so no shipped import answer changes. Deployed object extensions of `sys_user` were NOT MEASURED.
20+
21+
**Fix, if an import relied on the old answer:** the target's own declaration decides. Grant `list` in its `enable.apiMethods`, or drop `enable.apiEnabled: false`, if the object is meant to be matched by import.

‎packages/core/src/security/second-object-read-exposure.pin.test.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,14 @@
4848
* targets are resolved inside `relationConditionSites` (`@objectstack/core`),
4949
* which is the caller classified below. The bridge asks each target the
5050
* decision for `list` (`enforceRelationTargetExposure`, #22777), held by its
51-
* own package's pin rather than by a row here.
51+
* own package's pin rather than by a row here. Nor, for the same reason, a
52+
* door that drives `runImport` with a protocol of its own: plugin-auth's
53+
* identity import takes its cells' targets from the `import-field-meta.ts` map
54+
* classified below, and its run protocol carries the declaration reader the
55+
* import already holds, so `servesReferenceTarget` asks each target the
56+
* decision for `list` there too (#22800), held by its own package's pin
57+
* (`plugin-auth/src/admin-import-users.test.ts`, its `[#22800]` block) rather
58+
* than by a row here.
5259
*
5360
* The scan surface and its prefilter follow `row-serving-door-exposure.pin.test.ts`
5461
* in this directory: git's authored-file list, never a directory crawl, and `.ts`

‎packages/core/src/utils/import-runner.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,9 @@ export interface ImportProtocolLike {
211211
* [#22739] The object-declaration read, the one the door's import template
212212
* and the connector pull already make. The reference resolution reads a
213213
* lookup TARGET's `enable` block through it ({@link servesReferenceTarget}).
214-
* A protocol without it (plugin-auth's identity import, whose reads run
215-
* under the system identity) has no declaration to judge, and none is
216-
* fabricated for it.
214+
* A protocol without it has no declaration to judge, and none is fabricated
215+
* for it; plugin-auth's identity import, whose reads run under the system
216+
* identity, carries it like every other caller (#22800).
217217
*/
218218
getMetaItem?(request: GetMetaItemRequest): Promise<unknown>;
219219
}

‎packages/plugins/plugin-auth/src/admin-import-users.test.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,3 +660,117 @@ describe('[#17422] the import protocol literal is typed BY `ImportProtocolLike`'
660660
});
661661
}
662662
});
663+
664+
/**
665+
* [#22800] The identity import's lookup cells are judged like every other
666+
* `runImport` caller's: the run protocol carries the declaration reader the
667+
* import already holds (the one the parser reads `sys_user`'s fields through),
668+
* so `servesReferenceTarget` (`@objectstack/core`, #22739) asks each lookup
669+
* TARGET its declared exposure for `list` before a cell is matched against it.
670+
*
671+
* A target whose declaration refuses `list` answers as a target the caller
672+
* cannot read: `reference_not_found` for a cell naming a record, a cell naming
673+
* none and a pasted id alike, and nothing of the target is read. A serving
674+
* target is unchanged. The import's reads run under the system identity, which
675+
* is no exemption: the decision takes no caller.
676+
*
677+
* Fixture objects only. The decision's own matrix (every refusing and serving
678+
* declaration, fail-closed) is pinned once, in
679+
* `packages/core/src/utils/import-runner-reference-exposure.test.ts`; this
680+
* block pins that THIS door asks it. The engine is `makeDeps`' pinned double,
681+
* with only its `find` overridden to answer the fixture target.
682+
*/
683+
describe('[#22800] the identity import asks each lookup target its declared exposure, for list', () => {
684+
const TARGET = 'fx_unit';
685+
const RECORD = { id: 'fx_1', name: 'Existing' };
686+
const LOOKUP = 'fx_unit_id';
687+
/** The `sys_user` declaration the reader serves: two plain columns and one lookup into the fixture target. */
688+
const SYS_USER = {
689+
name: 'sys_user',
690+
fields: {
691+
email: { type: 'text', label: 'Email' },
692+
name: { type: 'text', label: 'Name' },
693+
[LOOKUP]: { type: 'lookup', label: 'Unit', reference: TARGET },
694+
},
695+
};
696+
/** A cell naming the record, a cell naming none, and the record's id. */
697+
const ROWS = [
698+
{ email: 'a@fx.test', [LOOKUP]: 'Existing' },
699+
{ email: 'b@fx.test', [LOOKUP]: 'Missing' },
700+
{ email: 'c@fx.test', [LOOKUP]: 'fx_1' },
701+
];
702+
703+
/** The target's declaration is `enable` (absent = no `enable` block); `reader: false` composes no declaration reader at all. */
704+
function exposureDeps(enable: unknown, opts: { reader?: boolean } = {}) {
705+
const m = makeDeps();
706+
const engine = m.deps.getDataEngine()!;
707+
const find = vi.fn(async (object: string, q: { where?: Record<string, unknown> }) => {
708+
if (object !== TARGET) return engine.find(object, q);
709+
const [[field, value]] = Object.entries(q.where ?? {});
710+
return (RECORD as Record<string, unknown>)[field] === value ? [RECORD] : [];
711+
});
712+
const getMetaItem = vi.fn(async ({ name }: { type: string; name: string }) => {
713+
if (name === 'sys_user') return { type: 'object', name, item: SYS_USER };
714+
if (name === TARGET) return { type: 'object', name, item: { name, ...(enable === undefined ? {} : { enable }) } };
715+
return { type: 'object', name, item: undefined };
716+
});
717+
const deps: IdentityImportDeps = {
718+
...m.deps,
719+
getDataEngine: () => ({ ...engine, find }),
720+
...(opts.reader === false ? {} : { getMetaItem }),
721+
};
722+
return { ...m, deps, find, getMetaItem };
723+
}
724+
725+
type RowVerdict = { ok: boolean; code?: string; field?: string };
726+
const verdict = (r: RowVerdict) => ({ ok: r.ok, code: r.code, field: r.field });
727+
const NOT_FOUND = { ok: false, code: 'reference_not_found', field: LOOKUP };
728+
const OK = { ok: true, code: undefined, field: undefined };
729+
const targetReads = (h: ReturnType<typeof exposureDeps>) => h.find.mock.calls.filter(([object]) => object === TARGET);
730+
731+
async function rowVerdicts(h: ReturnType<typeof exposureDeps>) {
732+
const res = await runAdminImportUsers(h.deps, makeRequest({ passwordPolicy: 'none', format: 'json', rows: ROWS }), ACTOR);
733+
expect(res.status).toBe(200);
734+
return (res.body.data as { rows: RowVerdict[] }).rows.map(verdict);
735+
}
736+
737+
for (const [label, enable] of [
738+
['apiEnabled: false', { apiEnabled: false }],
739+
['a whitelist granting get but not list', { apiMethods: ['get'] }],
740+
] as const) {
741+
it(`${label}: a match, a miss and an id all answer reference_not_found, and the target is never read`, async () => {
742+
const h = exposureDeps(enable);
743+
expect(await rowVerdicts(h)).toEqual([NOT_FOUND, NOT_FOUND, NOT_FOUND]);
744+
expect(targetReads(h)).toEqual([]);
745+
expect(h.createUser).not.toHaveBeenCalled();
746+
});
747+
}
748+
749+
for (const [label, enable] of [
750+
['no enable block', undefined],
751+
['a whitelist granting list', { apiMethods: ['list'] }],
752+
] as const) {
753+
it(`CONTROL ${label}: a match and an id resolve, a miss is reported, as before`, async () => {
754+
const h = exposureDeps(enable);
755+
expect(await rowVerdicts(h)).toEqual([OK, NOT_FOUND, OK]);
756+
expect(targetReads(h).length).toBeGreaterThan(0);
757+
expect(h.createUser).toHaveBeenCalledTimes(2);
758+
});
759+
}
760+
761+
it('the target is asked through the reader the import holds, once per import, by its own name', async () => {
762+
const h = exposureDeps({ apiEnabled: false });
763+
await rowVerdicts(h);
764+
expect(h.getMetaItem.mock.calls.map(([ref]) => ref)).toEqual([
765+
{ type: 'object', name: 'sys_user' },
766+
{ type: 'object', name: TARGET },
767+
]);
768+
});
769+
770+
it('CONTROL without a declaration reader no cell is a reference: the target is neither asked nor read', async () => {
771+
const h = exposureDeps({ apiEnabled: false }, { reader: false });
772+
expect(await rowVerdicts(h)).toEqual([OK, OK, OK]);
773+
expect(h.getMetaItem).not.toHaveBeenCalled();
774+
expect(targetReads(h)).toEqual([]);
775+
});
776+
});

‎packages/plugins/plugin-auth/src/admin-import-users.ts‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,12 @@ export interface IdentityImportDeps {
159159
/** better-auth server api (createUser + requestPasswordReset). */
160160
getAuthApi(): Promise<any>;
161161
getDataEngine(): IdentityImportEngine | undefined;
162-
/** Optional metadata reader for field coercion (best-effort). */
162+
/**
163+
* Optional object-declaration reader. It feeds field coercion (best-effort:
164+
* without it no cell is coerced and none is a reference), and [#22800] it is
165+
* the run protocol's `getMetaItem`, so each lookup target is asked its
166+
* declared exposure before a cell is matched against it.
167+
*/
163168
getMetaItem?(ref: { type: string; name: string }): Promise<any>;
164169
phoneNumberEnabled(): boolean;
165170
emailServiceAvailable(): boolean;
@@ -441,6 +446,17 @@ export async function runAdminImportUsers(
441446
const engine = deps.getDataEngine();
442447
if (!engine) return fail(503, 'SERVICE_UNAVAILABLE', 'Data engine unavailable');
443448

449+
// [#22800] The ONE object-declaration reader this import holds, handed to
450+
// both halves that read declarations: the parser, which reads `sys_user`'s
451+
// fields (so lookup cells become references at all), and the run protocol
452+
// below, through which `runImport`'s reference resolution asks each lookup
453+
// TARGET its declared exposure (`servesReferenceTarget`, #22739). One
454+
// binding, so the two can never disagree: whenever a cell is a reference,
455+
// its target is judged. "Reads run under the system identity" is no
456+
// exemption from that decision, which takes no caller.
457+
const metaReader: Pick<ImportProtocolLike, 'getMetaItem'> =
458+
deps.getMetaItem ? { getMetaItem: deps.getMetaItem } : {};
459+
444460
// ── Payload parsing (shared generic-import parser) ───────────────────
445461
const prep = await prepareImportRequest(
446462
{
@@ -453,7 +469,7 @@ export async function runAdminImportUsers(
453469
skipBlankMatchKey: true,
454470
},
455471
{
456-
p: { ...(deps.getMetaItem ? { getMetaItem: deps.getMetaItem } : {}) },
472+
p: metaReader,
457473
objectName: 'sys_user',
458474
maxRows: IMPORT_USERS_MAX_ROWS,
459475
},
@@ -507,6 +523,13 @@ export async function runAdminImportUsers(
507523
}
508524

509525
const protocol: ImportProtocolLike = {
526+
// [#22800] The declaration reader the parser was handed above. With it,
527+
// a lookup cell's target is asked its exposure for `list` before any
528+
// `findData` matches the cell; a target that refuses answers as one the
529+
// caller cannot read (`reference_not_found`), as for every other
530+
// `runImport` caller.
531+
...metaReader,
532+
510533
// [#16638] findExisting path: the CANONICAL QueryAST against sys_user.
511534
// `runImport` hands this handle a `FindDataRequest`, and the `QuerySchema`
512535
// that request declares carries `where` / `limit` — it declares neither

0 commit comments

Comments
 (0)