Skip to content

#9367's naive comment strip survives in two packages/cli contract TESTS — the fix converted the six gates, not the scans that live beside serve.ts #10453

Description

@os-zhuang

Filed unassigned from the #9863 dev seat (session session_01DdCnBGcHeufjrq7drTD3wt, PR #10450). Found by walking into it: a new scan in that PR copied the local idiom and lost the code it was measuring.

The residue

#9367 (closed by PR #9445) converted six scripts/check-*.mjs gates from a private naive stripComments to the shared string-aware maskComments. Two files carrying the identical spelling were outside that scope because they are tests, not gates:

  • packages/cli/src/commands/serve-verify-security-parity.contract.test.ts
  • packages/cli/src/commands/serve-email-config-parity.contract.test.ts

Both define:

functionstripComments(source: string): string{returnsource.replace(/\/\*[\s\S]*?\*\//g,' ').replace(/(^|[^:])\/\/[^\n]*/g,'$1');}

and both run it over packages/cli/src/commands/serve.ts.

Measured on today's serve.ts (e502a6a8e)

serve.ts's 5d. header comment contains the route wildcard /api/v1/auth/* — the very literal #9367 used as its example. Its /* opens a phantom block comment that closes against import(/* webpackIgnore: true */ authPkg)ten lines below, deleting the region in between, which contains the hasAuthPlugin computation and the if (!hasAuthPlugin && tierEnabled('auth')) gate.

Code-bearing lines of serve.ts surviving each order:

striplines kept
block pass first (what these two files do)1895
line pass first2098
shared maskComments2098, and the line count is preserved (4638 → 4638) because it blanks rather than deletes

The 203-line difference is code.

Is it live today? No — and that is the whole reason to fix it now

Both scans currently measure new SecurityPlugin(…) constructions, and every one of them sits outside the swallowed region, so their verdicts are unaffected and both are green for the right reason. This is a latent hazard with a proven mechanism, exactly as #9367 classified its own six.

What makes it worth closing anyway is that it is contagious, which is not a hypothetical: writing #9863's new serve-audit-registration.contract.test.ts meant reading these two files as the local pattern and copying their stripComments verbatim. The copy's first run failed — loudly, because the new scan asserts its anchors exist — with the auth-gate anchor ... is gone from serve.ts. The anchor had not moved; the stripper had eaten it. A scan whose anchors happened to sit elsewhere would simply have passed.

The blast radius also grows on its own: any future assertion in either file about a construction inside lines ~2132–2142 of serve.ts, or any edit that moves a measured construction into a swallowed span, converts this from latent to live silently.

Fix

Swap both private copies for maskComments from scripts/js-comment-mask.mjs, as #9445 did for the gates. Two notes for whoever takes it:

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions