diff --git a/.changeset/import-protocol-implementor-typed.md b/.changeset/import-protocol-implementor-typed.md new file mode 100644 index 0000000000..4cb5cf8fe0 --- /dev/null +++ b/.changeset/import-protocol-implementor-typed.md @@ -0,0 +1,13 @@ +--- +"@objectstack/plugin-auth": patch +--- + +fix(plugin-auth): let `ImportProtocolLike` type the admin import protocol's members (#17422) + +`admin-import-users.ts` is the only hand-written in-repo implementor of the runner's `ImportProtocolLike`, and it annotated all three required members `args: any`. An explicit parameter annotation wins over the contextual type, so #16952's newly declared request dialect held every implementor except this one — the one with a demonstrated history: before #16950 this file read `args?.query?.$filter ?? {}`, the runner moved to the canonical spelling, the read went `undefined`, and the `?? {}` default degraded the import's duplicate probe into match-everything, so `POST /api/v1/auth/admin/import-users` updated the wrong users without a sound. + +The three annotations are deleted, so `findData` / `createData` / `updateData` are typed by the contract they implement. Measured: with the annotations gone, reading a retired wire alias (`args.query?.$filter`) is `TS2339 Property '$filter' does not exist on type 'QueryInput'`; with `args: any` restored the identical probe type-checks at exit 0. + +`FindDataRequest` declares `query` optional, so `findData` now states its refusal in code — a thrown `Error` carrying the already-registered `INVALID_REQUEST` code — instead of relying on an incidental `TypeError` from a property read on `undefined`. No `??` fallback and no optional chaining were added: both spell match-everything, which is the defect this closes. + +No API, request body, response shape or exported signature changes. A caller that reaches `findData` through `runImport` always supplies `query`, so no supported call moves; only a protocol call that was already failing now fails with a code attached. diff --git a/packages/plugins/plugin-auth/src/admin-import-users.test.ts b/packages/plugins/plugin-auth/src/admin-import-users.test.ts index 592925a75a..59cb105460 100644 --- a/packages/plugins/plugin-auth/src/admin-import-users.test.ts +++ b/packages/plugins/plugin-auth/src/admin-import-users.test.ts @@ -1,12 +1,18 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, vi } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { assertEngineUpdateDispatch } from '@objectstack/objectql'; import { runAdminImportUsers, IMPORT_USERS_MAX_ROWS, type IdentityImportDeps } from './admin-import-users.js'; import type { AdminActor } from './admin-user-endpoints.js'; const ACTOR: AdminActor = { id: 'admin-1', email: 'admin@example.com' }; +const HERE = dirname(fileURLToPath(import.meta.url)); +const IMPORT_USERS_SOURCE = readFileSync(resolve(HERE, 'admin-import-users.ts'), 'utf8'); + function makeRequest(body: unknown): Request { return new Request('http://localhost/api/v1/auth/admin/import-users', { method: 'POST', @@ -624,3 +630,33 @@ describe('runAdminImportUsers — CSV payloads', () => { expect(m.createUser.mock.calls.map((c) => c[0].body.email).sort()).toEqual(['c1@x.co', 'c2@x.co']); }); }); + +/** + * [#17422] The IMPLEMENTOR half of #16952's contract. + * + * `ImportProtocolLike` types the three required members, but an EXPLICIT + * parameter annotation wins over a contextual type — so `findData(args: any)` + * opts this file back out of the contract while `tsc --noEmit` stays green. + * Measured on #17422 in this package: with the annotation restored, a probe + * reading the retired wire alias (`args.query?.$filter` — the pre-#16950 read + * whose `?? {}` default degraded the duplicate probe into match-everything) + * type-checks at exit 0; with the annotation gone the same probe is + * `TS2339 Property '$filter' does not exist on type 'QueryInput'`. + * + * ⇒ Nothing else in the repo can see that difference. The behavioural upsert + * tests above discriminate the CONSEQUENCE (ablated to the historical read, + * two of them go red) but not the opt-out itself: re-annotating the parameter + * leaves every one of them green and every gate green. This is that guard. + */ +describe('[#17422] the import protocol literal is typed BY `ImportProtocolLike`', () => { + it('binds the literal to the exported contract', () => { + expect(IMPORT_USERS_SOURCE).toContain('const protocol: ImportProtocolLike = {'); + }); + + for (const member of ['findData', 'createData', 'updateData'] as const) { + it(`leaves \`${member}\`'s parameter unannotated, so the contract types it`, () => { + expect(IMPORT_USERS_SOURCE).toContain(`async ${member}(args) {`); + expect(IMPORT_USERS_SOURCE).not.toMatch(new RegExp(`async\\s+${member}\\s*\\(\\s*args\\s*:`)); + }); + } +}); diff --git a/packages/plugins/plugin-auth/src/admin-import-users.ts b/packages/plugins/plugin-auth/src/admin-import-users.ts index a557f3ca47..a371c6d7c7 100644 --- a/packages/plugins/plugin-auth/src/admin-import-users.ts +++ b/packages/plugins/plugin-auth/src/admin-import-users.ts @@ -362,17 +362,34 @@ export async function runAdminImportUsers( // to `{}` stops constraining anything, so the duplicate probe matches rows // it was given no key for and the upsert updates the WRONG user. One // dialect, read straight — a request that arrives without a `query` is a - // caller defect and costs a loud TypeError, not a silent match-everything. - async findData(args: any) { - const where = args.query.where; - const limit = args.query.limit; + // caller defect and is refused loudly, not softened into match-everything. + // + // [#17422] The parameter is deliberately UNANNOTATED: `ImportProtocolLike` + // types it, and an explicit annotation here would win over that contextual + // type and opt this implementor back out of the contract (the runner's own + // docblock says so). `FindDataRequest` declares `query` OPTIONAL, so the + // contract makes this file write its refusal down instead of leaving it as + // an incidental TypeError from a property read on `undefined`. + // ⛔ Not `args.query ?? {}` and ⛔ not `args.query?.where`: both spell + // match-everything, which is the exact regression this protocol's history + // is about. + async findData(args) { + const query = args.query; + if (!query) { + throw Object.assign( + new Error('import-users: findData was called without a query — refusing to match every user'), + { code: 'INVALID_REQUEST' }, + ); + } + const where = query.where; + const limit = query.limit; return engine.find(args.object, { where, limit, context: SYSTEM_CTX } as any); }, // One better-auth create per row — hashing + credential sys_account. // Deliberately NO createManyData: there is no safe bulk primitive for // identities, and scrypt dominates the cost anyway. - async createData(args: any) { + async createData(args) { const data: Record = args?.data ?? {}; const email: string = typeof data.email === 'string' && data.email.length > 0 ? data.email @@ -431,7 +448,7 @@ export async function runAdminImportUsers( // Upsert updates touch PROFILE fields only — never email, never anything // credential- or system-managed. An empty filtered patch is a no-op. - async updateData(args: any) { + async updateData(args) { const patch: Record = {}; for (const [k, v] of Object.entries(args?.data ?? {})) { if (UPDATE_ALLOWED_FIELDS.has(k) && v !== undefined && v !== null && v !== '') patch[k] = v;