* refactor(cli): keep acp-integration off serve internals (#8084)
The dependency direction set in #8084 regressed: native Live Voice
(a5c637b749) added four acp-integration imports of serve/live modules,
because nothing in the repo enforces the boundary the issue defines.
Ownership, measured by consumer rather than by directory:
- capture-screen-context, live-task-tools, live-speak-to-user and
live-backend-instructions each have exactly one production consumer,
acp-integration/session/Session.ts, and import nothing from serve/.
They move to acp-integration/live/ with their tests.
- conversations/session-source is shared by acpAgent and four serve
modules, has no imports, and takes its reader as a parameter, so it
moves to runtime/live-session-source.ts alongside the other neutral
contracts. Renamed because every symbol in it is Live-specific.
Adds a no-restricted-imports rule for acp-integration/** so the next
feature spanning both surfaces gets a lint error pointing at runtime/,
rather than silently reopening the criterion.
No behavior change: moves, import rewrites, and the lint block.
* fix(cli): harden the acp/serve boundary guard (round 2)
- Flag the bare '../serve' directory specifier, which resolves to the
serve/ barrel and skipped the trailing-segment group patterns (also
added to the utils/ guard for symmetry).
- Extend the same boundary to runtime/, the layer the rule directs
authors to, so the #8084 coupling cannot reform one hop away.
- Cover dynamic imports: no-restricted-imports never visits
ImportExpression, so a no-restricted-syntax selector now enforces the
boundary for await import('../serve/...') too. The acp-integration
block moves after the general TS block (flat config lets the last
matching block win per rule) and restates its no-restricted-syntax
selectors so the override drops nothing.
- Document that CI lint is the enforcement point; no fixture test pins
the block.
Verified: synthetic fixtures for all three violation shapes are
rejected; full npm run lint passes with no live violations.
* test(cli): pin serve boundary lint rules
* test(cli): close serve boundary lint gaps
* fix(lint): close serve-boundary entrances and harden the guard
- reject computed dynamic-import sources (concatenation, new URL) and
type-level imports fail-closed; rounds 2-5 each demonstrated a new
per-spelling regex entrance, so non-literal forms are blocked outright
(R4-1)
- rewrite the boundary patterns without nested quantifiers; the previous
shape backtracked exponentially (~4x per two ../ segments, lint-time
ReDoS) (R5-2)
- build the three guarded override blocks no-restricted-syntax arrays from
one shared helper so flat config last-wins cannot silently drop selectors
(R5-3)
- pin the bare-directory barrel specifier in fixtures (R5-4) and add a
string-throw probe pinning the restated selectors in the overrides (R5-5)
- replace the **/serve* static globs with enumerated relative depths so
third-party serve-named packages are never flagged (R5-7)
* fix(lint): correct TSImportType selector path and computed-template handling
- read the type-import specifier at argument.literal.value: @typescript-eslint
wraps it in a TSLiteralType, so argument.value was dead code and the old
fail-closed TSImportType selector over-matched every type-level import
(37 errors in files this PR never touches) (round-6 Critical)
- reject computed template literals (templates containing expressions)
fail-closed; pure-literal templates stay covered by the quasis pattern
selectors — the old blanket TemplateLiteral exemption contradicted the
fail-closed comment above it (round-6 Critical)
- give the fail-closed selectors a distinct message: computed sources
cannot be checked against the boundary, which is not the same policy as
importing serve/ (round-6 suggestion)
- pin the depth-enumeration loop beyond depth 1 with a depth-2 fixture,
pin the fixed type-import selector with a negative typeof-import control,
and pin the computed-template fail-closed path (round-6 suggestion)
* fix(lint): close the remaining round-6 serve-boundary entrances
Complements the previous commit (which fixed the TSImportType selector
path and computed-template fail-closed) with the R4-1 entrances it left
open, each pinned by a fixture:
- percent-encoded segments (`../%73erve/index.js`): Node percent-decodes
segments when mapping the resolved URL to the filesystem, so raw-text
patterns cannot see through them — any `%` in a guarded-tree specifier
is now rejected with a dedicated message.
- static traversal twins: the pattern regexes now run over static
ImportDeclaration/ExportNamedDeclaration/ExportAllDeclaration sources
too, closing `import './../serve/x'`, `import '../runtime/../serve/x'`,
and `import '..//serve/x'`, whose dynamic twins were already blocked.
- leading literal segment: a traversal-anywhere pattern catches
`import('foo/../../../serve/x')` past the dot-slash anchor.
- vitest module-loading calls (vi.mock/doMock/importActual/importMock)
resolve and load the real module, so they get the same patterns plus
fail-closed coverage for computed arguments.
* fix(lint): cover vitest serve-boundary calls
* fix(lint): close the round-7 serve-boundary entrance classes
R4-1 round-7 interim hardening (the durable specifier-resolving custom
rule remains tracked separately):
- case-variant spellings (../Serve/...): every pattern, percent and
quasis attribute regex now carries the i flag, covering the dynamic,
static, vi.*/vitest.* and TSImportType arms.
- ?query/#fragment suffixes: rejected alongside % in all eight
specifier shapes (bundlers/Node strip them when resolving, so
'../serve?x' reaches the same module as '../serve').
- percent-encoded pure-template vitest calls: added the missing
arguments.0.quasis.0.value.cooked twin to the reject list.
- root-absolute and file: literal specifiers: fail-closed rejected in
every literal shape (guarded trees sweep verified clean of both).
- createRequire: its source modules ('module'/'node:module') are
flagged in guarded trees, since the alias escapes the
callee-name="require" arm and Node >=22 require(esm) loads serve/.
Each entrance class is pinned by a fixture case (18/18 green through
the real ESLint API); the three guarded trees lint clean with the new
arms.
* refactor(lint): resolve the serve boundary by resolution, not text (#8084)
R4-1 round-8 decision (maintainer-approved option a): replace the
spelling-by-spelling regex/glob matrix with a local resolution-based
ESLint rule (eslint-rules/no-serve-boundary-cross.js).
Eight review rounds each demonstrated a new spelling escaping the text
matrix (data: URLs, percent-encoding, traversal through a leading literal
segment, baseUrl bare specifiers, createRequire/getBuiltinModule,
TSImportType, aliased vitest loaders, Worker/fork), because every spelling
is just another way to NAME the same target. The new rule resolves each
import-like specifier against the importing file and reports anything
landing inside packages/cli/src/serve/:
- relative specifiers resolved against the importing file
- baseUrl bare specifiers resolved against packages/cli (tsconfig baseUrl
makes `src/serve/...` reachable — the round-8 entrance text never saw)
- file: URLs resolved to concrete paths (case-insensitive, whitespace-trimmed
scheme detection, since the URL parser normalizes both)
- vitest loaders matched alias-proof (v.mock / destructured importActual);
only specifiers resolving INTO serve/ report
- child_process.fork checked; spawn deliberately not (first arg is an
executable, not a module)
- fail-closed on statically-unresolvable sources: computed sources, data:
URLs, traversal-bearing bare specifiers, node:module imports,
process.getBuiltinModule
- case-insensitive path comparison (Serve/ loads serve/ on
case-insensitive filesystems)
Fixture suite reworked to resolution semantics: several round-4..7 fixture
depths corrected to spellings that genuinely resolve into src/serve (the
old depths resolved to packages/cli/serve, outside src/serve, and were
only caught by text matching); new pins for every round-8 entrance and for
the Codex self-review Criticals (aliased loaders, uppercase/whitespace URL
schemes, spawn not an import source). 26/26 pass; guarded trees and the
full cli src lint clean (zero false positives).
Removed: relativeServeImportPatterns, restrictedServeImports,
serveDynamicImportPatterns, serveGuardSyntaxRules and the per-spelling
selector/percent/absolute/createRequire special cases.
* fix(lint): drop the dead serveGuardSyntaxRules helper
The resolution-rule commit removed the mechanism but left the
serveGuardSyntaxRules helper behind — unused (no-unused-vars) and
referencing the already-deleted restrictedServeDynamicImports (no-undef),
which failed CI's repo-wide eslint. The guarded trees inherit
restrictedRequire + restrictedStringThrow from the general TS block, so
nothing is lost.
* fix(lint): close serve boundary resolver gaps
* fix(lint): address serve boundary review suggestions
- R9-2: move the new-URL-with-import.meta check into the NewExpression
visitor with the real MemberExpression base shape; the CallExpression
placement was unreachable and standalone new URL(...) reported nothing
- R9-3: report module-builtin entrances via the moduleBuiltin messageId
instead of the self-contradicting failClosed remediation text
- R9-4: match the destructured fork(...) spelling, not just
child_process.fork(...)
- R9-5/R8-2: fixture pins for re-exports, Worker, fork, require,
vi.doMock and vi.importMock entrances
- R9-7: pin the false branch of static-template concatenation (pure
template literals resolving outside serve stay allowed)
- R10-3: filter the third-party serve-named package pin by ruleId so a
failClosed false positive also turns it red
- R13-2: pin resolution detections on the serveBoundary messageId so
inside-detection degrading to blanket fail-closed cannot ship green
* fix(lint): close round-11 serve boundary gaps
Critical fixes:
- '#name' package-imports specifiers sailed through: stripUrlSuffixes
splits on '#' before the fail-closed check saw it, so the branch was
dead code and '#s' classified outside. Check '#' before suffix
stripping (fixture pins both entrances).
- scheme detection used JS trim(), which keeps non-whitespace C0
controls — '\x01data:…' slipped past while Node's URL parser strips
C0-or-space at the edges and loaded it. Detect schemes on the
WHATWG-normalized form (fixtures added).
- eval("(0,eval)"/globalThis.eval spellings included) and new Function
can embed import('…') the rule cannot resolve — fail closed like
computed sources; no-eval/no-new-func are not enabled in the shared
config and the guarded trees contain no such calls.
- backslashes normalize to '/' under Node's URL-based ESM resolution
(file: URLs are special), so '..\\serve\\x.js' loaded serve/ on
posix while the rule saw a bare specifier. Normalize backslashes
before classification (fixture added).
Hardening + pins:
- fork/Worker arms match object-agnostically (namespace/default-import
spellings no longer evade); Worker skips new URL(spec, import.meta.url)
arguments so the URL arm owns them (no more fail-closed false positive
on the canonical construct, no double report on serve targets).
- new TSImportEqualsDeclaration visitor: import x = require('../serve/…')
emits a working createRequire shim under tsc NodeNext.
- isProcessObject accepts computed properties (globalThis['process']) and
the Reflect.apply arm accepts computed getBuiltinModule; the three
getBuiltinModule arms collapse into one via a shared property matcher.
- vitest loader names lifted into a module-level constant; corrected two
stale comments (fail-closed branches; R5-5 probe description).
- fixtures: root-absolute/file: inside verdicts (repoRoot, messageId),
fork member arm, template cooked values, bare vitest loaders, dynamic
bare-'module' entrances, outside-serve negatives for URL/Worker/fork/
require. Suite 44/44.
* fix(cli): restore live session source import
* fix(lint): clear the two lint errors breaking CI on the boundary rule
Follow-up to the round-11 batch, which landed without running the
repo lint:
- the C0-edge-strip regex legitimately contains control-character
ranges (it mirrors the WHATWG URL parser), so disable
no-control-regex on that line with a rationale comment instead of
rewriting the range.
- drop the unused UTILS_FIXTURE constant from the boundary tests
(no fixture lints a utils/ file).
eslint clean on both files, boundary suite 44/44, prettier clean.
* fix(lint): close bounded serve boundary gaps
* fix(lint): complete the round-12 boundary escape closures
Extends the previous commit (which canonicalized the serve/baseUrl
comparison sides, added staticMemberPropertyName, and closed the
Function-call and Worker-eval-option shapes) with the remaining
round-12 review surface — every demonstrated spelling probed before
and after:
- Callee identity is now shape-tolerant end to end: rightmost-segment
object matching (nested member objects like globalThis.vi / x.cp no
longer evade the object-agnostic arms), renamed loader bindings
resolved from the import declarations (fork-as-f, Worker-as-W),
Reflect.apply/construct unwrapped for guarded targets (fork included),
Function.prototype.call/apply/bind indirection handled (.call unwraps
with shifted args; .apply/.bind fail closed), and the
SequenceExpression unwrap applied uniformly instead of eval-only.
- The string-code execution class fails closed beyond Function/eval
direct calls: .constructor property chains (({}).constructor.constructor,
(function(){}).constructor, AsyncFunction variants), eval.call/apply,
and the node:vm surface (runInThisContext / runInNewContext /
runInContext / compileFunction / new vm.Script, scoped to vm imports).
- The Worker eval option fails closed unless eval is statically false
(a dynamic option or non-object second argument is unverifiable), and
the URL arm's import.meta base restriction reports failClosed on the
construct.
- Fixtures pin each class: shape variants, renamed bindings, Reflect
indirection, call/apply/bind, the string-code family (incl. messageId-
specific failClosed pins for the eval:true Worker and non-url
import.meta bases), bare-directory/baseUrl query-suffix spellings, and
outside-serve allow pins for the export/import-equals arms.
- expectServeBoundaryError now filters on the rule id (all three
messageIds contain 'serve'); the divergent substring negative pins
move to expectNoBoundaryHits.
Suite 52/52; guarded trees lint clean (no false positives from the new
arms); eslint + prettier clean.
* fix(lint): close the round-12 reviewer escape classes (#8084)
Ten Criticals plus five hardenings from the round-12 review, every
class probe-verified before and after:
- R12-1: Worker eval-option analysis now matches runtime object-literal
semantics — the LAST eval key wins (duplicates included), an options
object without eval defaults to false (specifier path, no
over-block), and a spread after the last literal eval is
unverifiable — fail closed.
- R12-2: sequence unwrapping is now a uniform invariant — recursive on
callees in both visitors and applied to object expressions
(rightmostObjectName/isProcessObject), closing (0, require).call,
(0, (0, require)), (0, process).getBuiltinModule, (0, vm).* and
new (0, vm).Script.
- R12-3: call/apply/bind indirection is complete — Function/constructor
forward code (unconditional fail-closed), chained indirection
(x.call.call) fails closed instead of falling through, and vm exec
names plus the fork/vitest alias sets resolve.
- R12-4: Reflect.apply/construct target lists mirror the direct-call
arms — Function (incl. member spellings), the vm exec/Script surface,
and the vitest loaders (member and identifier targets).
- R12-5: alias sets populate in a pre-pass over the module body — ESM
imports are hoisted, so use-before-import now resolves like the
import-first direction.
- R12-6: renamed destructured vitest imports resolve through a new
vitestLoaderAliases set.
- R12-7: a named guarded global (process/globalThis/global) carrying an
opaque computed key fails closed (process-family keeps the dedicated
moduleBuiltin message); object-agnostic arms keep their documented
residue.
- R12-8: .constructor fails closed on variable bodies and expression
templates; statically non-string literals keep the pass-through.
- R12-9: inline lazy vm imports ((await import('node:vm')).*) count as
vm objects in the exec and Script arms.
- R12-11/12/13: checkSource skips statically non-specifier arguments
(no unactionable advice for env objects), the URL arm owns
new URL(spec, import.meta.url) on every entrance (no over-block, one
report on the serve form), and the module builtin reports
moduleBuiltin on every entrance.
Test hardening: R12-10 normalizes the repoRoot pin to forward slashes
(Windows merge-gate determinism), R12-14 pins the four mutation
survivors, R12-15 pre-cleans and catch-cleans the baseUrl symlink
links. Suite 65/65; guarded trees lint clean.
* fix(lint): repair corrupted files from the git-API blob upload
The previous commit (552bc7c8f) was pushed via the GitHub git API with
`-f content=@file` blob payloads that GitHub stored corrupted (9-byte
binary blobs), breaking the eslint config load (SyntaxError) and the CI
Test gate. Re-upload both files with JSON --input payloads whose blob
SHAs match the local git objects byte-for-byte (8270a0d2 / 5a0ebaa2).
No content change beyond restoring the intended files; suite 65/65.
* fix(lint): close the round-13 serve-boundary escape classes (#8084)
- re-normalize backslashes AFTER percent-decoding so %5c/%5C cannot
reintroduce a traversal the pre-decode normalization missed
- on realpath ENOENT canonicalize the deepest existing ancestor and
re-append the missing tail, so symlinked-ancestor checkouts fail
closed instead of open
- Worker eval-option scan: treat an unresolvable computed key like a
spread (unknown), fail closed on a non-computed __proto__ prototype
unless it is statically null, and resolve quoted string-literal keys
- carry the opaque-key fail-closed check through composed callees:
one hop below .call/.apply/.bind, as a Reflect target, and on the
getBuiltinModule object side, with the process-family message
Pins all four classes with executed fixtures plus negative controls;
the guarded trees stay lint-clean.
* fix(lint): close the round-13 binding-hop and callee-opacity escapes (#8084)
* refactor(cli): simplify ACP serve boundary guard
* fix(lint): close the round-21 contract pins and bare-barrel escape (#8084)
* fix(lint): close dynamic-import and js-file holes in the acp/serve guard (#8084)
* fix(lint): make acp/serve dynamic-import guard case-insensitive
The no-restricted-syntax selector for dynamic import() of serve/ was case-sensitive, so a macOS case-variant specifier (`../Serve/...`) would resolve to the daemon barrel without tripping the guard. Add the /i flag and cover case-variant plus computed-specifier behavior.
---------
Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
b455bad5e5