Uh oh!
There was an error while loading. Please reload this page.
fix(devx): reject an interpolating template literal in check-cross-package-test-inputs' PATH_LITERAL - #12087
Merged
Conversation
…ckage-test-inputs' PATH_LITERAL
`PATH_LITERAL`'s character class excludes only quote characters, so a
backtick-delimited argument holding no quotes matches it even when it is an
interpolating template — `` `${someVar}` `` reads as the literal segment text
`${someVar}` and `walkLiteral()` counts it as ONE ordinary descent, biasing
the depth walk upward instead of taking the documented cannot-read path. That
inverts the file's own stated invariant ("an argument this scan cannot read
leaves the DEPTH walk where it was ... since the escape verdict is a lower
bound"): an unreadable argument is safe, and a template read as readable was
LESS safe than unreadable.
`PATH_LITERAL` has exactly one call site in this file (inside
`pathExpression()`'s `resolve`/`join` argument walk), so narrowing it moves no
other consumer's verdict. The fix wraps that one call in
`readablePathLiteral()`, which rejects a backtick literal containing `${` and
falls back to the existing unreadable-argument branch (name dropped, depth
preserved) — the resolver itself now makes "never a wrong name" true by
construction, rather than relying on `findEscapingPackages()`'s downstream
`statSync(...).isFile()` filter to drop a fabricated roster entry. A
non-interpolating backtick literal is unaffected and continues to be read
(and named) exactly as a quoted literal would.
Both directions pinned in --self-test with the card's own fixture pair (same
climb, same file, only the middle argument differs), plus a control proving
the narrowing does not overshoot a plain backtick literal.
`NEW_URL_LITERAL` has the identical character-class shape and is filed
separately as #12085 — a structurally different call path, out of scope here.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6yinlianghui
marked this pull request as ready for review
August 25, 2026 08:42
Uh oh!
There was an error while loading. Please reload this page.
yinlianghui pushed a commit
that referenced
this pull request
Aug 25, 2026
…ckage-test-inputs' NEW_URL_LITERAL `NEW_URL_LITERAL`'s character class is byte-identical to `PATH_LITERAL`'s (#11487/#12087) and shares the same blind spot: a backtick-delimited argument holding no quotes matches it even when it is an interpolating template, so `` new URL(`${someVar}`, import.meta.url) `` reads `${someVar}` as the literal segment text and `walkLiteral()` counts it as one ordinary descent — biasing the depth walk upward and, when the climb lands outside the package, adding a fabricated NAME to the roster. Unlike `PATH_LITERAL`'s call site, this one has no "cannot read, keep depth" fallback to route into: `NEW_URL_LITERAL` has exactly one call site, directly inside `pathExpression()`, with no enclosing loop. So the fix (a `readableNewUrlLiteral()` wrapper, mirroring `readablePathLiteral()`'s shape but scoped to this call site rather than sharing it) makes an interpolating match return `null`, which flows straight into `pathExpression()`'s existing "no call matched" path and returns `undefined` for the WHOLE `new URL(...)` seed -- the same outcome as any other unrecognised seed shape, not a depth-kept one. The self-test pins that outcome explicitly (does not flag, no name), plus a control proving a non-interpolating backtick `new URL()` literal is unaffected, and a control proving `${` inside a quoted (non-backtick) literal is ordinary text, never interpolation. Measured (Zone 2.3 of #12085): before this fix, an escaping interpolating `new URL()` seed CAN push a fabricated NAME onto the roster (confirmed via a temporary export of `scanPathExpressions()` and a fixture that climbs out of its package), but `findEscapingPackages()`'s downstream `statSync(...).isFile()` filter throws ENOENT on the fabricated literal and drops it -- the same safety net #11487's Zone 2.3 found for `PATH_LITERAL`. Today's blast radius was therefore smaller than the card's open question implied; this fix closes the gap at the source regardless. Both directions ablated: reverting the call-site wrapper alone (tests intact) turns exactly the two new discriminating self-test cases red and leaves the other 115 green, then the wrapper was restored and reverified at 117/117. Fixes#12085
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#11487
What
scripts/check-cross-package-test-inputs.mjsstates, twice, that an argument the scan cannot read leaves the DEPTH walk unchanged and only drops the NAME — the escape verdict is a lower bound.PATH_LITERAL's character class ([^'"]*) excludes only quote characters, so a backtick-delimited argument holding no quotes matched it even when it was an **interpolating template**: ``${someVar}`` read as the literal segment text${someVar}andwalkLiteral()` counted it as ONE ordinary descent — the exact inverse of the stated trade. An unreadable argument was safe; a template read as readable was less safe than unreadable.Fix:
PATH_LITERALhas exactly one call site in this file (insidepathExpression()'sresolve/joinargument walk — verified by grep before touching it), so narrowing it moves no other consumer's verdict. That call is now routed through a smallreadablePathLiteral()wrapper that rejects a backtick literal containing${and falls back to the existing unreadable-argument branch (name dropped, depth preserved). A single/double-quoted literal is unaffected —${inside one of those is ordinary text, never interpolation.Resolver vs. filter (issue's Zone 2.3 question): before this fix, "never a wrong name" held only because
findEscapingPackages()'s downstreamstatSync(...).isFile()filter happened to drop the fabricated roster entry. After this fix, the resolver itself (pathExpression()) never produces a name for an interpolating template in the first place — the invariant is now true at the point the file's own comment makes the claim, not merely downstream of it.Tests
--self-testgains four cases at the site of the existing "unreadable join() argument" pair, using the issue's own fixture:join(HERE, `${someVar}`, '../../other-pkg/src/y.ts')) — now flags, depth -1`../../other-pkg/src/y.ts`) is unaffected — still flags AND is still correctly namedpackages/other-pkg/src/y.ts, proving the narrowing does not overshootReverse-verified: reverting the call site to the raw
a.match(PATH_LITERAL)(with the fix commit intact, so a real restore point exists) fails exactly the two new cases that assert the fix and none of the others —2/113 self-test case(s) failed. Restored and confirmedgit diff HEADclean before re-running.Derived gate list (
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack scripts/check-cross-package-test-inputs.mjs, re-run against HEAD3a73f5644) — all pass:node scripts/pm/bare-root-worklist.mjs --self-test— unaffected: no new scan-root population declared by this change (none stale, none missing).Scope
NEW_URL_LITERAL(three lines abovePATH_LITERAL) has the identical character-class shape and the identical blind spot in a structurally different call path (new URL(rel, import.meta.url)seeds, notresolve/joinarguments) — confirmed by direct regex test. Left untouched here per the issue's disposition and filed separately as #12085 for its own triage read, since an unmatchedNEW_URL_LITERALdoesn't fall into the same "cannot read, keep depth" branch (the whole seed goes unrecognised instead), so the fix consequence isn't identical.Changeset
None —
skip-changeset: this PR touches onlyscripts/check-cross-package-test-inputs.mjs, a repo gate script underscripts/**, nothing published.Generated by Claude Code