|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#10532] `organization/add-member` — `organizationId` falls back to the |
| 5 | + * caller's active organization; `teamId` does NOT. |
| 6 | + * |
| 7 | + * ## What this pin watches, and why it reads the VENDOR |
| 8 | + * |
| 9 | + * Three of our source comments used to assert the symmetric version of that |
| 10 | + * sentence ("organizationId/teamId default to the caller's active org/team when |
| 11 | + * omitted"). better-auth 1.7.1 implements only the org half. The comments are |
| 12 | + * corrected, and the docs written for #10050 now state the asymmetry to users |
| 13 | + * — so the standing risk is no longer a wrong comment but a **vendor bump that |
| 14 | + * ADDS an active-team fallback**, which would make our (now correct) docs wrong |
| 15 | + * with nothing watching. |
| 16 | + * |
| 17 | + * Therefore this pin asserts a fact about the INSTALLED vendor artifact, read |
| 18 | + * out of `node_modules` at test time. It deliberately does NOT assert anything |
| 19 | + * about our own comment text or our own constants: an assertion whose both |
| 20 | + * sides derive from files in this repo proves only that we wrote what we wrote, |
| 21 | + * and could not fail for the reason it exists. |
| 22 | + * |
| 23 | + * ## The zero-hit discipline |
| 24 | + * |
| 25 | + * "There is no active-team fallback" is a NEGATIVE finding, and a negative |
| 26 | + * finding from a detector nobody proved is worthless — a typo'd matcher reports |
| 27 | + * the same clean absence as a real one. So every negative leg below is paired |
| 28 | + * with a POSITIVE control that runs the SAME matcher over a neighbouring fact |
| 29 | + * known to be present: |
| 30 | + * |
| 31 | + * - `SESSION_ACTIVE_FALLBACK` must MATCH the `orgId` binding (which really |
| 32 | + * does fall back) before its non-match on the `teamId` binding counts; |
| 33 | + * - `activeOrganizationId` must occur in the file before the zero count for |
| 34 | + * `activeTeam*` counts. |
| 35 | + * |
| 36 | + * Measured 2026-08-21 on the installed better-auth 1.7.1, |
| 37 | + * `dist/plugins/organization/routes/crud-members.mjs`, inside `addMember`: |
| 38 | + * |
| 39 | + * const orgId = ctx.body.organizationId || session?.session.activeOrganizationId; |
| 40 | + * const teamId = "teamId" in ctx.body ? ctx.body.teamId : void 0; |
| 41 | + * |
| 42 | + * `activeOrganizationId` occurs 8 times in that file; `activeTeamId`, 0. |
| 43 | + */ |
| 44 | + |
| 45 | +import { describe, it, expect } from 'vitest'; |
| 46 | +import { readFileSync, existsSync } from 'node:fs'; |
| 47 | +import { createRequire } from 'node:module'; |
| 48 | +import { dirname, join } from 'node:path'; |
| 49 | + |
| 50 | +/** |
| 51 | + * Locate this package by walking up from the CWD — the idiom |
| 52 | + * `member-role-canonical.test.ts` / `rate-limit-storage-isolation.test.ts` |
| 53 | + * established here and state the reason for: plugin-auth is CJS-typed (no |
| 54 | + * `"type": "module"`, it publishes `dist/index.js` as CommonJS), so under |
| 55 | + * `module: NodeNext` `import.meta` is a TS1470 in this package however well it |
| 56 | + * runs under vitest. |
| 57 | + */ |
| 58 | +function findUp(predicate: (dir: string) => boolean, what: string): string { |
| 59 | + let dir = process.cwd(); |
| 60 | + for (;;) { |
| 61 | + if (predicate(dir)) return dir; |
| 62 | + const parent = dirname(dir); |
| 63 | + if (parent === dir) throw new Error(`could not locate ${what}`); |
| 64 | + dir = parent; |
| 65 | + } |
| 66 | +} |
| 67 | + |
| 68 | +const PKG = findUp((dir) => { |
| 69 | + const manifest = join(dir, 'package.json'); |
| 70 | + if (!existsSync(manifest)) return false; |
| 71 | + const { name } = JSON.parse(readFileSync(manifest, 'utf8')) as { name?: string }; |
| 72 | + return name === '@objectstack/plugin-auth'; |
| 73 | +}, 'the @objectstack/plugin-auth package root'); |
| 74 | + |
| 75 | +// Resolved from THIS package, so the file read is the better-auth this package |
| 76 | +// is pinned to — not whatever a hoist happens to put at the repo root. The read |
| 77 | +// goes through `node_modules` at test time; there is no checked-in copy of the |
| 78 | +// vendor text for it to fall back to, which is what the ablation in the PR body |
| 79 | +// demonstrates (mutate the file under `node_modules` and this suite reddens). |
| 80 | +const require_ = createRequire(join(PKG, 'probe.js')); |
| 81 | +/** `…/better-auth/dist/index.mjs` → `…/better-auth/dist`. */ |
| 82 | +const VENDOR_DIST = dirname(require_.resolve('better-auth')); |
| 83 | +const CRUD_MEMBERS = join(VENDOR_DIST, 'plugins', 'organization', 'routes', 'crud-members.mjs'); |
| 84 | +const VENDOR_SOURCE = readFileSync(CRUD_MEMBERS, 'utf8'); |
| 85 | + |
| 86 | +/** |
| 87 | + * The whole top-level `const <name> = …` declaration, up to the next top-level |
| 88 | + * `const` (declarations inside the body are indented, so a line-start `const` |
| 89 | + * marks the next sibling). Throws when the declaration is gone — the drift |
| 90 | + * tripwire: a vendor upgrade that restructures the module fails loudly instead |
| 91 | + * of leaving a pin that matches nothing and passes. |
| 92 | + */ |
| 93 | +function topLevelDecl(source: string, name: string): string { |
| 94 | + const start = source.indexOf(`\nconst ${name} = `); |
| 95 | + if (start < 0) { |
| 96 | + throw new Error( |
| 97 | + `[#10532] could not find the top-level \`${name}\` declaration in ${CRUD_MEMBERS}. ` + |
| 98 | + `better-auth restructured this module — re-read \`addMember\` and re-decide whether ` + |
| 99 | + `\`teamId\` still has no active-team fallback (our comments and the ` + |
| 100 | + `content/docs/permissions/authentication.mdx "Attaching an existing user to an ` + |
| 101 | + `organization" section both state that it does not).`, |
| 102 | + ); |
| 103 | + } |
| 104 | + const rest = source.slice(start + 1); |
| 105 | + const next = rest.indexOf('\nconst '); |
| 106 | + return next < 0 ? rest : rest.slice(0, next); |
| 107 | +} |
| 108 | + |
| 109 | +/** |
| 110 | + * The initialiser of a single-line `const <name> = …;` binding inside a slice, |
| 111 | + * returned as vendor BYTES. Throws when absent, for the same reason as above. |
| 112 | + */ |
| 113 | +function bindingInitializer(slice: string, name: string): string { |
| 114 | + const match = new RegExp(`\\n\\s*const ${name} = ([^;\\n]+);`).exec(slice); |
| 115 | + if (!match) { |
| 116 | + throw new Error( |
| 117 | + `[#10532] could not find the \`const ${name} = …;\` binding inside \`addMember\` in ` + |
| 118 | + `${CRUD_MEMBERS}. better-auth rewrote the handler — re-read it and re-decide whether ` + |
| 119 | + `\`teamId\` still has no active-team fallback.`, |
| 120 | + ); |
| 121 | + } |
| 122 | + return match[1]!; |
| 123 | +} |
| 124 | + |
| 125 | +/** |
| 126 | + * "This binding falls back to something on the caller's session." One matcher, |
| 127 | + * used on both bindings, so the negative leg is only ever read after the same |
| 128 | + * matcher has been shown to fire on the positive one. |
| 129 | + */ |
| 130 | +const SESSION_ACTIVE_FALLBACK = /session[\s\S]*\bactive[A-Z]\w*/; |
| 131 | + |
| 132 | +const ADD_MEMBER = topLevelDecl(VENDOR_SOURCE, 'addMember'); |
| 133 | + |
| 134 | +const occurrences = (needle: RegExp): number => |
| 135 | + VENDOR_SOURCE.match(new RegExp(needle.source, 'g'))?.length ?? 0; |
| 136 | + |
| 137 | +describe('[#10532] better-auth `addMember` — the org/team fallback asymmetry', () => { |
| 138 | + it('CONTROL: the `orgId` binding really does fall back to the session active org', () => { |
| 139 | + // Positive control for BOTH instruments used below: it proves the vendor |
| 140 | + // file was located and sliced, and that `SESSION_ACTIVE_FALLBACK` fires on |
| 141 | + // a binding that carries a session fallback. Without this leg passing, the |
| 142 | + // non-match in the next test is not evidence of anything. |
| 143 | + const orgId = bindingInitializer(ADD_MEMBER, 'orgId'); |
| 144 | + expect(orgId).toMatch(SESSION_ACTIVE_FALLBACK); |
| 145 | + expect(orgId).toContain('activeOrganizationId'); |
| 146 | + }); |
| 147 | + |
| 148 | + it('the `teamId` binding has NO fallback — it reads the request body and nothing else', () => { |
| 149 | + const teamId = bindingInitializer(ADD_MEMBER, 'teamId'); |
| 150 | + // Same matcher that just fired on `orgId`. |
| 151 | + expect(teamId).not.toMatch(SESSION_ACTIVE_FALLBACK); |
| 152 | + // …and positively: the only source of the value is the request body. |
| 153 | + expect(teamId).toContain('ctx.body'); |
| 154 | + expect(teamId).not.toContain('session'); |
| 155 | + }); |
| 156 | + |
| 157 | + it('no active-team read anywhere in the module (with the neighbouring positive control)', () => { |
| 158 | + // The control first: if this ever hits 0 the grep itself is broken and the |
| 159 | + // zero below means nothing. |
| 160 | + expect(occurrences(/activeOrganizationId/)).toBeGreaterThan(0); |
| 161 | + expect(occurrences(/active[Tt]eam\w*/)).toBe(0); |
| 162 | + }); |
| 163 | + |
| 164 | + it('the `if (teamId)` branches stay skipped when teamId is absent — no re-resolution later', () => { |
| 165 | + // The remaining way the vendor could acquire an active team is to |
| 166 | + // re-resolve it after the binding. Same control shape: the guard we expect |
| 167 | + // to be there is asserted present, then the re-resolution is asserted |
| 168 | + // absent across the whole handler. |
| 169 | + expect(ADD_MEMBER).toMatch(/if \(teamId\)/); |
| 170 | + expect(ADD_MEMBER).not.toMatch(/teamId\s*(?:\|\||\?\?)=?/); |
| 171 | + }); |
| 172 | +}); |
0 commit comments