|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// `objectstack serve`'s AuditPlugin registration — the facts the #9863 ruling |
| 4 | +// rests on, mechanised. |
| 5 | +// |
| 6 | +// ## The question, and the answer |
| 7 | +// |
| 8 | +// #9863 asked whether `os serve` should grow an `appAuditPluginOptions(config)` |
| 9 | +// helper mirroring the `appSecurityPluginOptions(config)` sibling six lines |
| 10 | +// above it, so that record-view auditing (`AuditPluginOptions.readAudit`) could |
| 11 | +// be turned on from `objectstack.config.ts` — rather than only by an app |
| 12 | +// putting its OWN configured `new AuditPlugin({ readAudit: … })` in the stack's |
| 13 | +// `plugins` array, where it supersedes the CLI's option-less instance under the |
| 14 | +// declared last-one-wins registration contract (#9864, maintainer ruling |
| 15 | +// 2026-08-19, option B). |
| 16 | +// |
| 17 | +// Ruled NO, on four measurements — the reasoning lives at the registration site |
| 18 | +// in `serve.ts` and in #9863's ruling comment; what lives HERE is the part that |
| 19 | +// has to keep being true: |
| 20 | +// |
| 21 | +// 1. The CLI constructs `AuditPlugin` exactly once, with NO options. That is |
| 22 | +// the ruling itself. Re-opening it means editing this file deliberately, |
| 23 | +// not discovering later that the shape drifted. |
| 24 | +// 2. That construction sits inside the auth-gated pair block and ABOVE the |
| 25 | +// stack `plugins` loop. The ORDER is load-bearing and was, until this file, |
| 26 | +// asserted by nothing: invert it and the CLI's option-less instance |
| 27 | +// supersedes the app's configured one, silently turning record-view |
| 28 | +// auditing back OFF for every deployment that had opted in. Nothing else |
| 29 | +// in the repo goes red on that edit. |
| 30 | +// 3. `@objectstack/verify`'s `bootStack` constructs no `AuditPlugin` at all. |
| 31 | +// This is why the #7001 argument for the security helper does not transfer: |
| 32 | +// that helper exists because TWO boot paths both built a `SecurityPlugin` |
| 33 | +// and silently disagreed about its options. Audit has exactly one boot path |
| 34 | +// with an opinion, so there is no disagreement for a shared helper to close |
| 35 | +// — and `@objectstack/verify` does not even depend on |
| 36 | +// `@objectstack/plugin-audit` (see its package.json), so it cannot grow one |
| 37 | +// by accident. If that changes, the ruling's basis changes with it, and |
| 38 | +// this assertion is what says so. |
| 39 | +// |
| 40 | +// ## Why a source scan rather than a boot |
| 41 | +// |
| 42 | +// Same reason as this directory's `serve-verify-security-parity.contract.test.ts` |
| 43 | +// and `serve-email-config-parity.contract.test.ts`: the failure mode is an EDIT |
| 44 | +// to these files, and every one of the three facts above is invisible to a |
| 45 | +// behavioural test. `serve.ts`'s audit block is reachable only from a live |
| 46 | +// `objectstack serve` boot with `@objectstack/plugin-auth` and |
| 47 | +// `@objectstack/plugin-audit` installed, an auth secret set and no app-supplied |
| 48 | +// AuthPlugin; a unit test that got there would be testing the fixture. The grep |
| 49 | +// that WOULD have caught each edit, mechanised, is the honest instrument. |
| 50 | + |
| 51 | +import { describe, it, expect } from 'vitest'; |
| 52 | +import { readFileSync } from 'node:fs'; |
| 53 | +import path from 'node:path'; |
| 54 | +import { fileURLToPath } from 'node:url'; |
| 55 | +// The separator (#9367) is a plain `.mjs`, but it ships a hand-written `.d.mts` |
| 56 | +// declaration alongside it (#10398), so this import is typed and needs no |
| 57 | +// suppression. A `@ts-expect-error` here is an UNUSED directive, and cli's tsc |
| 58 | +// program does include this file, so tsc fails the build on one. |
| 59 | +import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; |
| 60 | + |
| 61 | +const HERE = path.dirname(fileURLToPath(import.meta.url)); |
| 62 | + |
| 63 | +/** |
| 64 | + * `packages/cli/src/commands/` → `packages/`. Reading verify's harness from |
| 65 | + * here is what makes fact 3 an assertion instead of a comment; |
| 66 | + * `@objectstack/verify` is a real dependency of this package and the read is |
| 67 | + * test-only (tests never ship — `files: ["dist"]`). The glob is already declared |
| 68 | + * for `@objectstack/cli` by `pnpm check:cross-package-test-inputs`, and hashed |
| 69 | + * by `@objectstack/cli#test` in turbo.json, for the sibling parity scan; this |
| 70 | + * file adds a second reader of the same path, not a new radius. |
| 71 | + * |
| 72 | + * That gate is named by its runnable script rather than by its file path on |
| 73 | + * purpose. Its literal collector takes any quoted path without parsing, so |
| 74 | + * spelling the path here — in prose, about a file this test never opens — would |
| 75 | + * demand a declaration for it and put cli's whole suite on every edit of the |
| 76 | + * gate. The reads this scan really makes are the two below. |
| 77 | + */ |
| 78 | +const PACKAGES_DIR = path.resolve(HERE, '../../..'); |
| 79 | + |
| 80 | +/** |
| 81 | + * Absence must be loud (AGENTS.md, Route & surface ownership §3). A scan that |
| 82 | + * reports success because it could not find its subject is worse than no scan: |
| 83 | + * every assertion below is of the form "this shape is present / is not present", |
| 84 | + * and an empty string satisfies half of them for free. |
| 85 | + */ |
| 86 | +function readBootPath(relative: string): string { |
| 87 | + const full = path.join(PACKAGES_DIR, relative); |
| 88 | + try { |
| 89 | + return readFileSync(full, 'utf8'); |
| 90 | + } catch (e) { |
| 91 | + throw new Error( |
| 92 | + `#9863 audit-registration scan cannot read its subject '${relative}' (looked at ${full}). ` + |
| 93 | + 'The file moved or was renamed — repoint this scan; do NOT delete it. The ruling it pins ' + |
| 94 | + `is still in force. (${(e as Error).message})`, |
| 95 | + ); |
| 96 | + } |
| 97 | +} |
| 98 | + |
| 99 | +/** |
| 100 | + * Comments MASKED, because this scan is about what the two files DO. |
| 101 | + * |
| 102 | + * Not optional here, and not caution copied from the sibling scans: `serve.ts`'s |
| 103 | + * audit block DESCRIBES the very construction being counted — it spells |
| 104 | + * `new AuditPlugin({ readAudit: … })` in prose to explain the app-side opt-in it |
| 105 | + * documents, and it names the helper by name to record that the helper was ruled |
| 106 | + * against. Over raw text this file would count two constructions where the code |
| 107 | + * has one, and its own ruling assertion would fail on the sentence stating the |
| 108 | + * ruling. |
| 109 | + * |
| 110 | + * ## Why the SHARED masker rather than a private `stripComments` |
| 111 | + * |
| 112 | + * Because the private ones are a measured defect class here (#9367), and this |
| 113 | + * file walked straight into it. Its first draft used the two-regex strip the two |
| 114 | + * older scans in this directory still carry, block pass first — and `serve.ts` |
| 115 | + * has a route wildcard in a line comment (`/api/v1/auth/*` in the `5d.` header). |
| 116 | + * A regex cannot tell that `/*` from a real opener, so it ran a phantom block |
| 117 | + * comment to the next real terminator ten lines below, inside |
| 118 | + * `import(/* webpackIgnore: true *\/ …)`, deleting the `hasAuthPlugin` |
| 119 | + * computation and the auth gate this file measures against. Measured on this |
| 120 | + * exact pair: the naive strip keeps 1895 code-bearing lines of `serve.ts`, the |
| 121 | + * masker keeps 2098. |
| 122 | + * |
| 123 | + * `maskComments` also BLANKS rather than deletes — spans become spaces, newlines |
| 124 | + * kept — so the offsets the ordering assertion below compares are offsets into |
| 125 | + * the real file rather than into a shrunken copy of it. Cross-checked on both |
| 126 | + * subjects: masker and naive-strip agree on all four anchor counts, and the |
| 127 | + * masker leaves the line count identical (4638 → 4638), which is the property |
| 128 | + * being bought. |
| 129 | + * |
| 130 | + * #10427 (open) has the masker desyncing on nested template literals in 16 |
| 131 | + * files. Neither file read here is among them, and the cross-check above is what |
| 132 | + * says so rather than assuming it. |
| 133 | + */ |
| 134 | + |
| 135 | +const SERVE = maskComments(readBootPath('cli/src/commands/serve.ts')); |
| 136 | +const HARNESS = maskComments(readBootPath('verify/src/harness.ts')); |
| 137 | + |
| 138 | +/** |
| 139 | + * Every `new AuditPlugin(...)` construction in a file, with its argument text. |
| 140 | + * |
| 141 | + * Walks parentheses rather than matching `\(([^)]*)\)`, for the reason the |
| 142 | + * sibling parity scan measured: an options argument is itself brace- and |
| 143 | + * paren-bearing (`appAuditPluginOptions(config)`, `{ readAudit: { objects: [] } }`), |
| 144 | + * and a non-nesting match stops at the first inner `)` and silently reports a |
| 145 | + * truncation. The empty-argument case this file asserts today would be reported |
| 146 | + * identically by both forms, which is exactly how a scan that cannot read the |
| 147 | + * shape it guards passes until the day it matters. |
| 148 | + */ |
| 149 | +function auditPluginConstructions(source: string): string[] { |
| 150 | + const NEW = 'new AuditPlugin('; |
| 151 | + const found: string[] = []; |
| 152 | + for (let i = source.indexOf(NEW); i !== -1; i = source.indexOf(NEW, i + 1)) { |
| 153 | + let depth = 1; |
| 154 | + let j = i + NEW.length; |
| 155 | + for (; j < source.length && depth > 0; j++) { |
| 156 | + if (source[j] === '(') depth++; |
| 157 | + else if (source[j] === ')') depth--; |
| 158 | + } |
| 159 | + if (depth !== 0) throw new Error(`unbalanced \`${NEW}…\` at offset ${i} — the scan cannot read this file`); |
| 160 | + found.push(source.slice(i + NEW.length, j - 1).trim()); |
| 161 | + } |
| 162 | + return found; |
| 163 | +} |
| 164 | + |
| 165 | +/** |
| 166 | + * The offset of an anchor that must appear exactly once. Both "missing" and |
| 167 | + * "appeared twice" are reported as failures rather than folded into an offset |
| 168 | + * comparison, because an ordering assertion between two anchors is meaningless |
| 169 | + * if either is ambiguous — and a duplicated anchor is how a refactor most |
| 170 | + * plausibly arrives. |
| 171 | + */ |
| 172 | +function soleOffset(source: string, anchor: string, role: string): number { |
| 173 | + const first = source.indexOf(anchor); |
| 174 | + if (first === -1) { |
| 175 | + throw new Error( |
| 176 | + `#9863 audit-registration scan: the ${role} anchor \`${anchor}\` is gone from serve.ts. ` + |
| 177 | + 'It was the landmark this scan measured the AuditPlugin registration against. ' + |
| 178 | + 'Repoint the anchor at whatever replaced it — the invariant (the CLI registration ' + |
| 179 | + 'stays inside the auth-gated pair block and ABOVE the stack `plugins` loop) is unchanged.', |
| 180 | + ); |
| 181 | + } |
| 182 | + if (source.indexOf(anchor, first + 1) !== -1) { |
| 183 | + throw new Error( |
| 184 | + `#9863 audit-registration scan: the ${role} anchor \`${anchor}\` now appears more than once ` + |
| 185 | + 'in serve.ts, so "before" and "after" no longer name one place. Give this scan an ' + |
| 186 | + 'unambiguous landmark before trusting its verdict.', |
| 187 | + ); |
| 188 | + } |
| 189 | + return first; |
| 190 | +} |
| 191 | + |
| 192 | +/** The `if (!hasAuthPlugin && tierEnabled('auth'))` block the audit pair lives in. */ |
| 193 | +const AUTH_GATE = "if (!hasAuthPlugin && tierEnabled('auth'))"; |
| 194 | +/** The stack `plugins` loop, i.e. where an app's own configured instance is registered. */ |
| 195 | +const PLUGINS_LOOP = 'for (const plugin of plugins)'; |
| 196 | + |
| 197 | +describe('os serve registers AuditPlugin bare, above the stack `plugins` loop (#9863)', () => { |
| 198 | + it('constructs AuditPlugin exactly once, with NO options — the ruling', () => { |
| 199 | + // The empty string is the whole point: `[]` would mean "never constructed" |
| 200 | + // and `['appAuditPluginOptions(config)']` would mean the ruling was reversed. |
| 201 | + expect(auditPluginConstructions(SERVE)).toEqual(['']); |
| 202 | + }); |
| 203 | + |
| 204 | + it('does not reach for a config-derived audit options helper', () => { |
| 205 | + // Re-opening #9863 is allowed; doing it by accident is not. A helper wired |
| 206 | + // in HERE would take effect only when the app supplies no AuthPlugin of its |
| 207 | + // own and an auth secret is set (see the gate below) — a declared config key |
| 208 | + // whose effect depends on unrelated auth conditions, on a compliance |
| 209 | + // surface. If the ruling is revisited, the capability resolver's |
| 210 | + // `CAPABILITY_PROVIDERS.audit` entry — which is NOT auth-gated and already |
| 211 | + // carries the `configKey` mechanism `analytics` uses — is the site to argue |
| 212 | + // about, and this assertion moves in the same edit as the ruling. |
| 213 | + expect(SERVE).not.toContain('appAuditPluginOptions'); |
| 214 | + }); |
| 215 | + |
| 216 | + it('registers inside the auth-gated pair block and ABOVE the stack `plugins` loop', () => { |
| 217 | + const gate = soleOffset(SERVE, AUTH_GATE, 'auth-gate'); |
| 218 | + const loop = soleOffset(SERVE, PLUGINS_LOOP, 'stack-plugins-loop'); |
| 219 | + const audit = soleOffset(SERVE, 'new AuditPlugin(', 'audit-construction'); |
| 220 | + |
| 221 | + // ABOVE the loop: the half `serve.ts` calls load-bearing. Below it, the |
| 222 | + // CLI's option-less instance would supersede the app's configured one and |
| 223 | + // record-view auditing would be off wherever it had been opted in. |
| 224 | + expect(audit).toBeLessThan(loop); |
| 225 | + |
| 226 | + // INSIDE the auth block: the half nothing had written down. The pair is |
| 227 | + // registered by the `5d. Auto-register AuthPlugin (and paired |
| 228 | + // Security/Audit)` branch, so an app that supplies its own AuthPlugin — or |
| 229 | + // a production boot with no auth secret — gets NO CLI AuditPlugin, and the |
| 230 | + // supersede this card is named after never happens there at all. Hoisting |
| 231 | + // the registration out of the block is a real change of meaning, not a |
| 232 | + // tidy-up, and it is exactly the edit an offset comparison against the gate |
| 233 | + // catches. |
| 234 | + expect(audit).toBeGreaterThan(gate); |
| 235 | + }); |
| 236 | + |
| 237 | + it("verify's bootStack has no AuditPlugin opinion — no #7001-shaped disagreement to close", () => { |
| 238 | + expect(auditPluginConstructions(HARNESS)).toEqual([]); |
| 239 | + }); |
| 240 | +}); |
0 commit comments