From 80024aa31b98e5e9d0867ff19d90b950ed42562d Mon Sep 17 00:00:00 2001 From: os-elon Date: Fri, 21 Aug 2026 01:08:16 +0000 Subject: [PATCH 1/3] =?UTF-8?q?test(cli):=20put=20the=20serve=E2=86=94boot?= =?UTF-8?q?Stack=20parity=20scan=20on=20the=20shared=20comment=20masker=20?= =?UTF-8?q?(#10453)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #9367 converted six `scripts/check-*.mjs` gates off a private naive `stripComments` and onto the string-aware `maskComments`. This scan was outside that population only because it is a test, and it still carried the original spelling — block pass first, the order that swallows code. `serve.ts` is the conversion's textbook subject: its `5d.` header names the route wildcard `/api/v1/auth/*`, whose opener starts a phantom block comment running to the `webpackIgnore` pragma ten lines below, deleting the `hasAuthPlugin` computation and the auth gate in between. Measured at `5c3faa70d`: the naive strip keeps 1895 code-bearing lines of `serve.ts`, the masker keeps 2141. No verdict moved. Every construction this scan measures already sat outside the swallowed span, so the divergence was latent — which is exactly why it had to be closed by hand rather than by a red build. A new block pins the separator itself on four inputs where the two implementations disagree, so reverting the swap is red rather than silent. The masker's read of both real subjects was also checked byte-for-byte against `@typescript-eslint/parser`'s comment ranges (0 bytes of disagreement either way), which rules out #10427's open `scanSource` desync for this pair. The import escapes the package, so it is declared in `CROSS_PACKAGE_TEST_INPUTS` and hashed by `@objectstack/cli#test`. That gate did not demand it — see #10452. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- ...ve-verify-security-parity.contract.test.ts | 121 ++++++++++++++++-- scripts/check-cross-package-test-inputs.mjs | 13 ++ turbo.json | 3 +- 3 files changed, 128 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts b/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts index 59b45b2fef..f4a9d372a9 100644 --- a/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts +++ b/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts @@ -39,6 +39,11 @@ import { readFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { appSecurityPluginOptions, appDefaultPermissionSetName } from '@objectstack/plugin-security'; +// @ts-expect-error -- the repo's one comment/code separator (#9367) is a plain +// `.mjs` script with no type declarations. This file IS in cli's tsc program, +// so the suppression is a real one, not a phantom: delete the import and tsc +// reports the unused directive. +import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; const HERE = path.dirname(fileURLToPath(import.meta.url)); @@ -69,7 +74,7 @@ function readBootPath(relative: string): string { } /** - * Comments stripped, because this scan is about what the two files DO. + * Comments masked, because this scan is about what the two files DO. * * Both boot sites are heavily commented — with the very construction shapes * being asserted about, since each explains what it replaced — so a scan over @@ -78,19 +83,45 @@ function readBootPath(relative: string): string { * comment-inclusive form would forbid the next author from ever *describing* * the old wiring, which is the opposite of what these files need. * - * Approximate by design, and safe here: the result feeds nothing but the - * `new SecurityPlugin(...)` regex below, so a `//` mangled out of a string - * literal (`'http://localhost:3000'` in `harness.ts`) cannot affect a verdict. - * Do not reuse this for anything that reads string contents. + * This was a private two-regex `stripComments`, block pass first, and this + * paragraph used to call it “approximate by design, and safe here”. It was + * neither. #9367 converted six `scripts/check-*.mjs` gates off that exact + * spelling onto the shared `maskComments`; these scans were outside that + * population only because they are tests, and `serve.ts` is the conversion's + * textbook subject. Its `5d.` header names the route wildcard + * `/api/v1/auth/*`, whose opener starts a PHANTOM block comment running to the + * next real terminator — the `webpackIgnore` pragma ten lines below — which + * deletes the `hasAuthPlugin` computation and the + * `if (!hasAuthPlugin && tierEnabled('auth'))` gate in between. Measured on + * this pair at `5c3faa70d`: the naive strip keeps 1895 code-bearing lines of + * `serve.ts`, `maskComments` keeps 2141. + * + * No verdict below moved on the swap. Every construction this file measures + * already sat outside the swallowed span, which is what made the divergence + * latent rather than live — and is exactly why it had to be closed by hand + * rather than by a red build. What the swap buys is that it stays closed: an + * assertion added about anything inside that span, or a measured construction + * that moves into it, would otherwise go wrong in silence. The separator's own + * behaviour is pinned at the bottom of this file, so reverting it is red. + * + * `maskComments` also BLANKS rather than deletes — every byte offset and line + * number survives — which the offset-walking `securityPluginConstructions` + * below depends on. + * + * Named rather than inlined into the `.map()` so that the pin at the bottom of + * this file exercises THIS binding. A pin that called `maskComments` directly + * would stay green through the one edit it exists to catch — someone restoring + * a private strip here — which is the same shape of hole as the mention-is-not- + * a-call ablation recorded below. */ -function stripComments(source: string): string { - return source.replace(/\/\*[\s\S]*?\*\//g, ' ').replace(/(^|[^:])\/\/[^\n]*/g, '$1'); +function separateCodeFromProse(source: string): string { + return maskComments(source); } const BOOT_PATHS: Array<{ label: string; relative: string; source: string }> = [ { label: 'objectstack serve', relative: 'cli/src/commands/serve.ts' }, { label: 'verify bootStack', relative: 'verify/src/harness.ts' }, -].map((p) => ({ ...p, source: stripComments(readBootPath(p.relative)) })); +].map((p) => ({ ...p, source: separateCodeFromProse(readBootPath(p.relative)) })); /** * Every `new SecurityPlugin(...)` construction in a file, with its argument. @@ -178,3 +209,77 @@ describe('the shared resolution, exercised (#7001)', () => { expect(appSecurityPluginOptions(undefined)).toBeUndefined(); }); }); + +/** + * The separator beneath the parity scan, pinned — #9367's conversion, arriving + * here (#10453). + * + * ⚠️ The naive strip below is a SPECIMEN of what was replaced, never an + * instrument. A future re-derivation grepping this directory for that regex + * will land on this block: it is the pin that the file no longer USES it. + * + * Each case is an ordinary shape carrying a comment opener that is not a + * comment, followed by the construction this file measures and a real docblock + * for a phantom span to close against. The naive strip's block pass runs first, + * so the phantom opens at the opener and closes at the docblock's terminator, + * deleting the construction in between: the scan then reports a boot path that + * constructs no `SecurityPlugin` at all, and reports it as a clean read. The + * masker resolves all four correctly, and the two implementations were checked + * byte-for-byte against `@typescript-eslint/parser`'s comment ranges on both + * real subjects (0 bytes of disagreement either way), which is what rules out + * #10427's open `scanSource` desync for this pair. + */ +describe("the parity scan's comment separator is string-aware (#9367)", () => { + /** The private two-regex strip this file carried until #10453. Specimen only. */ + const naiveStrip = (source: string): string => + source.replace(/\/\*[\s\S]*?\*\//g, ' ').replace(/(^|[^:])\/\/[^\n]*/g, '$1'); + + const CONSTRUCTION = 'new SecurityPlugin(appSecurityPluginOptions(config));'; + const DOCBLOCK = '/** a docblock far below, for a phantom span to close against */'; + + const DISAGREEMENTS: Array<{ label: string; source: string }> = [ + { + // `serve.ts` carries exactly this today, at its `5d.` header. + label: 'a route wildcard in a line comment', + source: ['// The Console expects /api/v1/auth/* to be served by better-auth.', CONSTRUCTION, DOCBLOCK].join('\n'), + }, + { + label: 'a route wildcard in a string literal', + source: ["const authRoutes = '/api/v1/auth/*';", CONSTRUCTION, DOCBLOCK].join('\n'), + }, + { + label: 'a comment opener in a template literal', + source: ['const route = `${base}/api/v1/auth/*`;', CONSTRUCTION, DOCBLOCK].join('\n'), + }, + { + // The nested-template family #10427 measures `scanSource` against. The + // masker answers this one correctly; the case it does NOT is a nested + // template carrying an ESCAPED backtick, which neither subject of this + // scan contains and which is that issue's to close, not this file's. + label: 'a comment opener in a template nested inside an interpolation', + source: ['const msg = `${globs.map((g) => `<${g}>`).join(", ")} /api/v1/auth/*`;', CONSTRUCTION, DOCBLOCK].join('\n'), + }, + ]; + + it.each(DISAGREEMENTS)('$label — the masker keeps the construction', ({ source }) => { + expect(securityPluginConstructions(separateCodeFromProse(source))).toEqual(['appSecurityPluginOptions(config)']); + }); + + it.each(DISAGREEMENTS)('$label — the naive strip loses it', ({ source }) => { + // The direction that matters: not a mangled argument, but a boot path that + // reads as constructing nothing. Two such paths agree with each other + // vacuously, which is the parity claim above passing over no evidence. + expect(securityPluginConstructions(naiveStrip(source))).toEqual([]); + }); + + it('blanks rather than deletes, so the paren walk reads real offsets', () => { + // `securityPluginConstructions` slices by offset into the masked text and + // the failure it throws names one, so a separator that shortens the file + // reports positions that do not exist in it. On the first case the naive + // strip does not merely shift offsets — it collapses 185 bytes to nothing. + const { source } = DISAGREEMENTS[0]; + expect(separateCodeFromProse(source)).toHaveLength(source.length); + expect(separateCodeFromProse(source).split('\n')).toHaveLength(source.split('\n').length); + expect(naiveStrip(source).length).toBeLessThan(source.length); + }); +}); diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index ff30622781..2dfccda22b 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -242,6 +242,18 @@ const CROSS_PACKAGE_TEST_INPUTS = { // designed trade (over-collection can only widen a radius, never narrow one), // and declaring one rarely-touched file is cheaper than teaching the scanner to // tell prose from code, or than rewording a comment to dodge a scanner. + // + // `js-comment-mask.mjs` is the first entry declared for an IMPORT rather than + // a file read: src/commands/serve-verify-security-parity.contract.test.ts + // imports `maskComments` from it to separate code from prose in the two boot + // paths it scans (#10453, adopting #9367's conversion). This gate did NOT + // demand the declaration -- its literal collector recognises path-shaped + // reads, and a relative import specifier that escapes the package is not one + // of the spellings it knows. Declared by hand because the coupling is real + // whatever the collector saw: that scan's verdict is a function of this + // module's masking behaviour, so a change to it has to re-run cli's suite. + // The undetected-import spelling is filed separately as #10452; widening a + // radius by hand is never the reason not to file it. globs: [ 'packages/verify/src/**', 'packages/plugins/plugin-security/src/**', @@ -254,6 +266,7 @@ const CROSS_PACKAGE_TEST_INPUTS = { 'content/docs/deployment/index.mdx', 'content/docs/permissions/authentication.mdx', 'scripts/check-nul-bytes.mjs', + 'scripts/js-comment-mask.mjs', ], }, '@objectstack/lint': { diff --git a/turbo.json b/turbo.json index 568f6e1690..a01449251d 100644 --- a/turbo.json +++ b/turbo.json @@ -72,7 +72,8 @@ "$TURBO_ROOT$/content/docs/deployment/cli.mdx", "$TURBO_ROOT$/content/docs/deployment/index.mdx", "$TURBO_ROOT$/content/docs/permissions/authentication.mdx", - "$TURBO_ROOT$/scripts/check-nul-bytes.mjs" + "$TURBO_ROOT$/scripts/check-nul-bytes.mjs", + "$TURBO_ROOT$/scripts/js-comment-mask.mjs" ] }, "@objectstack/lint#test": { From fc21bfb84b74212feb81fb4375f704473a8abcbd Mon Sep 17 00:00:00 2001 From: os-elon Date: Fri, 21 Aug 2026 02:30:50 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix(cli):=20drop=20the=20now-unused=20@ts-e?= =?UTF-8?q?xpect-error=20=E2=80=94=20the=20masker=20gained=20a=20real=20.d?= =?UTF-8?q?.mts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scripts/js-comment-mask.d.mts` landed on main in 0681a76b8, after this branch forked at 5c3faa70d. It hand-declares `maskComments(source: string): string`, so the import no longer errors and the suppression above it became TS2578 "Unused '@ts-expect-error' directive" — red on `typecheck-workspace`, and red on `typecheck-debt` as a cascade, because the re-measure compares each ledger entry's raw `tsc --noEmit` count against its recorded number and this pushed `@objectstack/cli` above its own. Neither tsc was wrong: the branch's tree had no declaration (directive load-bearing, typecheck clean) and CI's merged tree has one. Reproduced locally only after merging origin/main, which is the honest way to see it. Not a silent deletion — the import is now genuinely typed rather than `any`. Probed by passing `maskComments(12345)` into a `number`, which tsc rejects with TS2345 (parameter is `string`) and TS2322 (return is `string`); the probe was reverted byte-identically. The comment that justified the suppression is replaced rather than left standing, since its claim that the module has "no type declarations" is now false. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- .../serve-verify-security-parity.contract.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts b/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts index f4a9d372a9..35d3dbc67b 100644 --- a/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts +++ b/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts @@ -39,10 +39,11 @@ import { readFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { appSecurityPluginOptions, appDefaultPermissionSetName } from '@objectstack/plugin-security'; -// @ts-expect-error -- the repo's one comment/code separator (#9367) is a plain -// `.mjs` script with no type declarations. This file IS in cli's tsc program, -// so the suppression is a real one, not a phantom: delete the import and tsc -// reports the unused directive. +// The repo's one comment/code separator (#9367). Typed by the hand-written +// `scripts/js-comment-mask.d.mts` next to it, so this import needs no +// suppression and `maskComments` arrives as `(source: string) => string` +// rather than `any` — which is what makes the `source: string` annotation on +// BOOT_PATHS below a real check instead of a formality. import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; const HERE = path.dirname(fileURLToPath(import.meta.url)); From 51a3a986aef2d775ffa50d3a5db29d3faef85183 Mon Sep 17 00:00:00 2001 From: os-elon Date: Fri, 21 Aug 2026 02:39:41 +0000 Subject: [PATCH 3/3] fix(ci): declare the masker's .d.mts as a cli test input too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:cross-package-test-inputs` went red once the previous commit named `scripts/js-comment-mask.d.mts` in the test's prose: the gate's literal collector takes quoted paths without parsing, so a mention forces a declaration. This roster already settles that trade for `check-nul-bytes.mjs` — declare the file rather than reword a comment to dodge a scanner. Here declaring is also the substantively right answer, not just the cheap one. The `.d.mts` is what gives `maskComments` its type, so cli's `tsc --noEmit` verdict is a function of that file — and this branch is the proof: the file arriving on main is what turned an `@ts-expect-error` into a TS2578 and took two typecheck lanes red without cli's own tests ever re-running. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- scripts/check-cross-package-test-inputs.mjs | 12 ++++++++++++ turbo.json | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index 2dfccda22b..d2dd593bec 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -254,6 +254,17 @@ const CROSS_PACKAGE_TEST_INPUTS = { // module's masking behaviour, so a change to it has to re-run cli's suite. // The undetected-import spelling is filed separately as #10452; widening a // radius by hand is never the reason not to file it. + // + // Its `.d.mts` sibling is declared for BOTH reasons this roster records. It + // is named in that test's prose, and the literal collector takes quoted + // paths without parsing, so a mention forces a declaration (the + // `check-nul-bytes.mjs` entry above settles that trade the same way: + // declaring the file beats rewording a comment to dodge a scanner). It is + // also a real input rather than only a mention -- it is what gives + // `maskComments` its type, so cli's `tsc --noEmit` verdict is a function of + // it. Measured, not assumed: this file arriving on main is exactly what + // turned that test's `@ts-expect-error` into a TS2578 and took the + // typecheck lanes red on a branch that never touched it. globs: [ 'packages/verify/src/**', 'packages/plugins/plugin-security/src/**', @@ -267,6 +278,7 @@ const CROSS_PACKAGE_TEST_INPUTS = { 'content/docs/permissions/authentication.mdx', 'scripts/check-nul-bytes.mjs', 'scripts/js-comment-mask.mjs', + 'scripts/js-comment-mask.d.mts', ], }, '@objectstack/lint': { diff --git a/turbo.json b/turbo.json index a01449251d..8210abfd77 100644 --- a/turbo.json +++ b/turbo.json @@ -73,7 +73,8 @@ "$TURBO_ROOT$/content/docs/deployment/index.mdx", "$TURBO_ROOT$/content/docs/permissions/authentication.mdx", "$TURBO_ROOT$/scripts/check-nul-bytes.mjs", - "$TURBO_ROOT$/scripts/js-comment-mask.mjs" + "$TURBO_ROOT$/scripts/js-comment-mask.mjs", + "$TURBO_ROOT$/scripts/js-comment-mask.d.mts" ] }, "@objectstack/lint#test": {