check-boundaries generic RULES do not fail closed on computed dynamic imports

[BorisTyshkevich]

Status

The original backtick/no-substitution-template bypass reported here was fixed in #646:

import(`@altinity/clickhouse-http`)

is now recognized, and the package-boundary rules that needed stronger syntax handling were moved to the real TypeScript-parser-backed helpers during #646/#653.

The remaining problem is narrower: the generic regex-backed RULES in build/check-boundaries.mjs still do not fail closed on dynamic imports whose argument is not a statically provable string literal.

Problem

extractSpecifiers() only returns module specifiers it can recognize textually. That is sufficient for ordinary static imports/exports and for quoted or no-substitution-template dynamic imports, but it silently skips computed dynamic imports such as:

import(specifier)
import(`../${name}.js`)
import(prefix + '/module.js')

For a source tree guarded only by the generic RULES loop, a computed dynamic import therefore receives no boundary decision at all. The check does not prove that the import is allowed; it simply fails to see a specifier.

This is no longer the broad package-boundary hole described in the original filing: later #630 phases moved Rule C/Rule D and the final package guards to the real parser. The remaining scope is the generic internal-layer rules that still intentionally use extractSpecifiers().

Why it matters

These guards are intended to enforce source-level dependency direction. A rule that silently ignores an import form it cannot classify is fail-open: a future dynamic import can cross a forbidden layer without check:arch noticing.

There is no known production use of computed dynamic imports under the currently guarded trees, so this is architecture-hardening rather than a live product regression.

Intended fix

For directories covered by the generic RULES, make dynamic-import handling fail closed:

• allow a dynamic import only when its argument is statically provable as a string-like module specifier supported by the checker;
• continue to accept single-quoted, double-quoted, and no-substitution-template literals;
• reject computed/non-static import(...) expressions in guarded source instead of silently skipping them;
• preserve the existing fast path for ordinary static import/export forms unless moving the generic rules to the shared real-parser helper is simpler and measurably acceptable.

The diagnostic should identify the file and explain that the dynamic import cannot be statically checked against the architecture boundary.

Tests

Add sabotage coverage for at least:

• quoted dynamic import crossing a forbidden boundary;
• no-substitution-template dynamic import crossing a forbidden boundary;
• computed template dynamic import, e.g. import(`../${name}.js`);
• identifier argument, e.g. import(specifier);
• concatenated argument, e.g. import('../' + name);
• existing export * from, named re-export, and namespace re-export coverage remains intact.

The production checker and its policy mirror should prove the same fail-closed behavior.

Acceptance criteria

• A guarded source file cannot use a computed/non-static dynamic import without check:arch failing.
• Single-quoted, double-quoted, and no-substitution-template dynamic imports remain analyzable.
• A statically analyzable dynamic import crossing a forbidden RULES boundary is rejected.
• Existing legal imports remain accepted.
• Sabotage tests cover computed-template, identifier, and concatenated dynamic-import arguments.
• The change does not reintroduce a second unsound hand-rolled parser where the shared TypeScript-parser helper is the more appropriate mechanism.

History

Originally filed during review of #641 after finding that the then-current dynamic-import regex matched only single- and double-quoted arguments. That exact bypass was fixed in #646. This issue now tracks only the remaining fail-open behavior for non-static dynamic imports in the generic regex-backed architecture rules.

[BorisTyshkevich]

Ship log — #642

Phase	Status	PR
(unphased)	shipped	#668

Handoff

• Landed: build/lib/check-legacy-owners.mjs gains a shared parser-backed
  findDynamicImportUsages/mightContainDynamicImport pair; build/check-boundaries.mjs's
  generic RULES loop and Rule B now classify every dynamic import(...) in a
  guarded file via that pair instead of the old dynamic-import regex arm
  (removed from extractSpecifiers, renamed extractStaticSpecifiers).
  Uncheckable arguments (identifier, computed template, concatenation,
  conditional, TypeScript inline import-type non-literal, or otherwise) are
  unconditional violations; statically analyzable arguments (single/double-
  quoted, no-substitution-template, including inline type T = import('x').Foo
  forms) flow through the existing relative-resolution/forbidden-prefix logic
  unchanged. Rule C/Rule D and other already-parser-backed package guards
  (feat(#630): extract ClickHouse SQL quoting and type-expression grammar (phase 5) #646/feat(#630): make @altinity/clickhouse-http standalone-buildable, retire client-web vendor spike (phase 8) #653) are untouched.
• Decisions taken (not in the issue): the plan's own "mandatory audit"
  of every other regex/prefilter in check-boundaries.mjs found no other
  instance of the fail-open pattern this issue targets (two ChatGPT findings
  proposing to widen scope to extractStaticSpecifiers's static-only regex
  and to mightReferenceRetiredTopLevelApi's identifier-only threat model
  were independently re-verified and rejected across two passes — both are
  real, pre-existing, already-documented, out-of-scope design boundaries
  from earlier closed issues, not defects this PR introduced).
• Deviations from the spec: none — all six issue acceptance bullets
  claimed, none deferred, per the approved plan.
• Deferred acceptance criteria: none.
• Follow-ups filed: chatgpt-review: extractReportedMetadata misextracts reviewed SHA when Markdown-bolded #669 (inbox) — a real chatgpt-review tooling bug
  found while shipping this unit: extractReportedMetadata's SHA-extraction
  regexes don't tolerate Markdown bold formatting around a backtick-wrapped
  SHA (**\`**`), silently falling back to "first 40-hex string in the
  text" and reporting the WRONG (earlier/previous) SHA as the reviewed head
  on multi-pass PR reviews. Confirmed by manually reading the actual GitHub
  PR review comments, which correctly named the current head both times —
  this is purely an extraction bug, not a review defect, and did not affect
  the correctness of this unit's own certification (independently manually
  verified before merging).
• Plan review: approved on pass 1 (ChatGPT-author/Fable-review loop),
  conversation https://chatgpt.com/c/6a79c43b-f11c-83eb-93b7-f4378d063f32.
• Review outcome: certified at head a2318aa2612ab65f1f059649ad51506744b667d4
  after 3 code-review passes — pass 1 found and fixed one real regression
  (TypeScript inline import(...) type expressions bypassing the new
  classifier, import('../workspace/model.js')-shaped but as an
  ImportTypeNode rather than a CallExpression); passes 2 and 3 both
  correctly re-rejected the same two out-of-scope findings on adversarial
  re-verification; pass 3 verdict SHIP, no accepted findings. Reviewed-SHA
  match confirmed manually (see chatgpt-review: extractReportedMetadata misextracts reviewed SHA when Markdown-bolded #669). Merged as 358924a.