Make source-policy gates cover every bundled source extension or ban JS under src

[Brad-Edwards]

Review finding

The source-policy scanner only walks .ts files under src, but the project tooling can bundle JavaScript if tracked JavaScript is added under the application source tree.

Evidence

• tests/runtime/source-policy.ts:42 documents the scanner as returning every .ts file.
• tests/runtime/source-policy.ts:47 implements walkTsFiles().
• tests/runtime/source-policy.ts:59 includes only entries ending in .ts.
• The file header describes the policy as a scan over src/**/*.ts, not over every source extension Vite may bundle.

Impact

Future tracked .js, .jsx, .mjs, .cjs, or .tsx files under src could bypass source-policy checks such as dynamic import, eval, or policy-boundary scans. There are no tracked JavaScript source files today, so this is a preventive process gap rather than a current exploit.

Recommended fix

Either formally ban bundled JavaScript source files under src, or expand the source-policy scanner to include every extension that can participate in the bundle.

Acceptance checks

• The repository has an explicit test or policy gate that rejects tracked unsupported source extensions under src, or the scanner includes those extensions.
• If JavaScript is allowed, Q007-style tests prove forbidden dynamic import/eval patterns are detected in .js fixtures.
• Comments and test names no longer describe the protected surface as only src/**/*.ts unless that is the enforced rule.

[Brad-Edwards]

🛠️ Picked up by /implement — driver Codex, branch 152-source-policy-extensions, 2026-05-24T04:29:55Z.

[Brad-Edwards]

gc workflow phase recorded: preflight (issue #152). Posted by the MCP server to enforce ordering between workflow steps (issue #794 MVP-2). Do not edit or delete — used by downstream tools to gate phase prerequisites.

[Brad-Edwards]

gc workflow phase recorded: plan (issue #152). Posted by the MCP server to enforce ordering between workflow steps (issue #794 MVP-2). Do not edit or delete — used by downstream tools to gate phase prerequisites.

Implementation plan

Scope: requirement-free issue #152. Acceptance criteria are the three bullets in the issue body.

Plan:

• Update tests/runtime/source-policy.ts so the canonical file inventory is extension-set based, not TypeScript-only. The set will include executable Vite/esbuild module sources: .ts, .tsx, .js, .jsx, .mjs, .cjs, .mts, .cts; declaration files such as .d.ts stay out of scope because they are not bundled runtime code.
• Rename the exported walker from TS-specific language to source-policy language and update policy tests that consume it. Keep all policy scanners on the same helper instead of adding a second scanner or per-policy glob.
• Add failing-first Q007 scanner coverage proving JavaScript source text is parsed and forbidden eval / dynamic import patterns are detected for .js inputs, and add walker coverage for the supported extension set.
• Update comments, test names, and design/trust prose that currently describe the protected surface as src/**/*.ts.
• Add changelog.d/152.fixed.md because the diff changes the source-policy gate behavior.

Design checks:

• Security: existing AST predicates, exemption parsing, bounded findings, and relative path diagnostics remain the enforcement layer; no regex-only scanner, runtime validator, or advisory mode is added.
• Maintainability: tests/runtime/source-policy.ts remains the canonical inventory and parser helper; policy tests reuse it.
• Extensibility: future source-extension changes require editing one extension set plus fixtures, not every policy.
• Whole-repo view: the change flows through Vitest/CI via pnpm test; Vite can bundle JavaScript even when TypeScript does not type-check JS, so the source-policy surface must not depend on tsconfig allowJs behavior.

[Brad-Edwards]

gc_codex_review — cycle 1 of 1 (pre-push) on issue #152 (branch 152-source-policy-extensions)

Core review

Verdict: ship

This change is shaped correctly around the existing source-policy seam: it widens the canonical walker from TypeScript-only files to executable JS/TS module extensions, keeps declaration files out of runtime-policy scope, and updates the shared parser to select the right TypeScript ScriptKind for JS/JSX/TSX inputs. The docs and policy tests move to the same vocabulary, and the new inventory self-test pins the boundary where it belongs. It does not bypass an incumbent helper or foreclose the next obvious variation; future policy gates can continue to consume the same shared walker/parser contract.

No blocking findings.

Security review

Verdict: ship

This change is shaped correctly: it moves the source-policy boundary from a TypeScript-only glob to an explicit executable-source-module inventory, keeps the shared scanner as the seam, and updates the policy gates to consume that seam rather than duplicating extension logic per rule. Security-wise, this broadens existing static controls over code-execution and import-boundary surfaces without introducing a new runtime input path, auth boundary, secret-handling path, or user-controlled file execution surface. I would ship this.

No blocking findings.

[Brad-Edwards]

gc_codex_review pre-push cycle 1 of 1 complete for issue #152 on branch '152-source-policy-extensions'. Posted by the MCP server to enforce the pre-push hard-cap-1 contract (issues #796, #804, #906). Do not edit or delete — used by the next gc_codex_review (uncommitted) invocation to count cycles.

[Brad-Edwards]

Review decision record — codex cycle 1 (issue #152)

Reviewer: codex
Cycle: 1

Architectural read:

Core reviewer: This change is shaped correctly around the existing source-policy seam: it widens the canonical walker from TypeScript-only files to executable JS/TS module extensions, keeps declaration files out of runtime-policy scope, and updates the shared parser to select the right TypeScript ScriptKind for JS/JSX/TSX inputs. The docs and policy tests move to the same vocabulary, and the new inventory self-test pins the boundary where it belongs. It does not bypass an incumbent helper or foreclose the next obvious variation; future policy gates can continue to consume the same shared walker/parser contract.

Security reviewer: This change is shaped correctly: it moves the source-policy boundary from a TypeScript-only glob to an explicit executable-source-module inventory, keeps the shared scanner as the seam, and updates the policy gates to consume that seam rather than duplicating extension logic per rule. Security-wise, this broadens existing static controls over code-execution and import-boundary surfaces without introducing a new runtime input path, auth boundary, secret-handling path, or user-controlled file execution surface. I would ship this.

Blocking findings: 0 (clean run)

[Brad-Edwards]

gc_test_quality_review cycle 1 of 1 — issue #152

Reviewer: test-quality (claude-sonnet-4-6 via gc_test_quality_review)
Branch: 152-source-policy-extensions
Cycle: 1 / 1
Findings: 1

Finding 1 — [warning] tests/runtime/policy-q004-resource-cleanup.test.ts — multiple it.each blocks

Problem: Tests assert expect(findings.length).toBeGreaterThan(0) and expect(labels.some(l => l.startsWith(expectedLabel))).toBe(true). A scanner that fires for the wrong reason but emits any finding whose label happens to start with the expected prefix would pass. Equally, toBeGreaterThan(0) does not detect phantom extra findings that would indicate the scanner over-fires.
Why it matters: If the listener-root matcher were accidentally widened (e.g., flagging signal.addEventListener alongside document.addEventListener), the positive tests would still pass because findings.length > 0 and the label prefix matches. Label-text regressions (changed wording, wrong diagnostic category) also go undetected with prefix matching.
Fix: Replace expect(findings.length).toBeGreaterThan(0) with expect(findings).toHaveLength(1) (or the exact expected count). Replace expect(labels.some(l => l.startsWith(expectedLabel))).toBe(true) with expect(findings[0]?.label).toBe(exactLabel) or .toContain(expectedLabel) where the exact string is impractical. Where a single input genuinely produces multiple findings, assert the exact count and enumerate expected labels.

[Brad-Edwards]

gc_test_quality_review cycle 1 of 1 complete for issue #152 on branch '152-source-policy-extensions'. Posted by the MCP server to enforce the gc_test_quality_review hard-cap-1 contract (issue #884 follow-up, default lowered in #906). Do not edit or delete — used by the next gc_test_quality_review invocation to count cycles.

[Brad-Edwards]

Review decision record — test-quality cycle 1 (issue #152)

Reviewer: test-quality
Cycle: 1

Architectural read:

This branch exports SOURCE_POLICY_EXTENSIONS from source-policy.ts and wires it into the Q007 file-inventory self-test, closing a coverage gap where the extension list and the walker could drift apart undetected. The broader test-file set continues the established two-layer pattern: per-policy scanner self-tests (inline synthetic TypeScript → findings assertions) backed by runtime-tree compliance sweeps. The seam is correct — shared helper exports exactly the constants tests need, self-tests exercise both flagging and non-flagging paths, and runtime-tree tests enforce the 'no violations in production code' invariant. The one recurring weakness is a subset of Q004 tests that uses a weaker assertion idiom than the rest of the suite.

Blocking findings: 1

Finding 1 — class (9 instances)

• ID: F1
• Title: (no title)
• Decision: fix
• Rationale: Addressed by next cycle
• Instances:
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags %s with label %s' (global addEventListener/removeEventListener it.each)
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags wrapper-rooted listener %swith label%s'
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags new %s(() => undefined)' (observer construction it.each)
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags wrapper-rooted %s' (observer it.each)
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags %s' (prototype monkey-patch it.each)
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags compound assignment %s'
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags mutation-API monkey-patch %s'
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags %s' (ambient-document leaf it.each)
  • tests/runtime/policy-q004-resource-cleanup.test.ts — 'flags wrapper-rooted %swith label starting%s'