Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,6 +39,12 @@ import { readFileSync } from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { appSecurityPluginOptions, appDefaultPermissionSetName } from '@objectstack/plugin-security';
// 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));

Expand DownExpand Up@@ -69,7 +75,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
Expand All@@ -78,19 +84,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.
Expand DownExpand Up@@ -178,3 +210,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);
});
});
25 changes: 25 additions & 0 deletions scripts/check-cross-package-test-inputs.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -242,6 +242,29 @@ 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.
//
// 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/**',
Expand All@@ -254,6 +277,8 @@ 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',
'scripts/js-comment-mask.d.mts',
],
},
'@objectstack/lint': {
Expand Down
4 changes: 3 additions & 1 deletion turbo.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -72,7 +72,9 @@
"$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",
"$TURBO_ROOT$/scripts/js-comment-mask.d.mts"
]
},
"@objectstack/lint#test": {
Expand Down
Loading