Skip to content

fix(engine)!: block naming runtime globals to close reflective sandbox escapes - #481

Merged
rhuanbarreto merged 2 commits into
mainfrom
claude/scanner-block-globals
Jul 16, 2026
Merged

fix(engine)!: block naming runtime globals to close reflective sandbox escapes#481
rhuanbarreto merged 2 commits into
mainfrom
claude/scanner-block-globals

Conversation

@rhuanbarreto

Copy link
Copy Markdown
Contributor

Summary

The module allowlist (#477) stopped rule files from importing dangerous modules — but Bun, process, and the global object are live globals in the rule runtime, reachable with no import at all. The scanner blocked the shapes Bun.spawn and Bun[x], which is the same losing game a module denylist was. Two fire-tested RCEs walked straight around it, both reported "pass": true by archgate check:

// (1) reflection / aliasing / destructuring / global-object aliases
Reflect.get(Bun, "spawn")([...]);        // also: const {spawn}=Bun; const B=Bun;
                                          //       globalThis.Bun.spawn / global.Bun.spawn / self.Bun.spawn
// (2) Function constructor via the .constructor chain === eval — bypasses even the module allowlist
(() => {}).constructor("return import('node:child_process')")();

The fix — apply the allowlist philosophy to globals

Block naming the capability source, not the shapes of using it. Any code reference to Bun, process, globalThis, global, self, Reflect, eval, Function, fetch, WebSocket, XMLHttpRequest, EventSource, or require is refused — in any position except a property-key slot (foo.process, { process: 1 } name a property, not the global, and are fine). Blocking the identifier closes aliasing, destructuring, and reflection in one rule. global/self are included because Bun binds the global object under all three names.

Additionally block .constructor access (dotted + computed-literal) on any receiver — the property-chain route to the Function constructor (= eval).

This simplified the scanner (net −157 lines): the per-shape Bun/process member denylists and the separate eval/Function/fetch/require call checks are gone, subsumed by the single identifier block. First-party and imported scans convergescanImportedRuleSource() now delegates — because a first-party rule runs with full privilege too and a malicious PR can add one.

An audit confirmed zero of the repo's own .rules.ts reference these globals as executable code (every mention is a searched-for string), so there are no false positives.

Known residual (documented + regression-tested)

A property name built at runtime — const c = "constructor"; (() => {})[c] — is unknowable to a static scanner, and blocking all computed access would reject ordinary arr[i]/obj[key]. That route to eval is left to execution-time isolation; the scan is defense-in-depth that raises the bar from a trivial one-liner to runtime string construction, not a jail. A test asserts this residual explicitly so it's a deliberate, known gap.

Test plan

  • tests/engine/rule-scanner-escapes.test.ts — new "reflective and aliased access to runtime globals" block: aliasing, destructuring, Reflect.get, the three global-object aliases, Object.getOwnPropertyDescriptor, the Function-constructor chain (dotted + computed-literal), eval/Function/fetch/require aliasing; plus "legitimate global-adjacent code still passes" (Object.keys, a property named process, a normal ctx-only rule) and the explicit computed-variable residual
  • Fire test — both RCEs now refused end-to-end ("pass": false), no payload written
  • bun run validate — 44/44 ADR rules (dogfooded on the repo's own rules), 1527 tests, lint, typecheck, build

Warning

BREAKING: rule files may no longer name Bun/process/globalThis/etc., even for benign reads (Bun.env, process.platform, Bun.Glob). Rules interact with the project only through ctx; a rule that genuinely needs such data is a ctx feature request. Small false-positive surface: a rule using one of these names as a local variable (self, global) must rename.

ADR

Amends ARCH-024, clause 4: rewrites the "escapes that name no module" model to "block naming the global," documents the .constructor closure, the scan convergence, and the computed-variable-key residual as the static-analysis limit that execution-time isolation answers.

Doc coordination

guides/security.mdx currently names Reflect.get(Bun,"spawn") as a residual bypass (on main, and reworded in #480's rewrite). This PR closes that specific residual, so that note should be updated to reflect the new residual (a runtime-computed .constructor key). To avoid editing soon-replaced text and redundant trilingual i18n churn here, I'll make that doc update on the #480 branch (which owns the rewritten security section) so the two don't conflict — flagging for whoever sequences the merges.

…x escapes

The module allowlist (#477) stopped rule files from importing dangerous
modules, but Bun, process, and the global object are LIVE globals in the
rule runtime — reachable with no import at all. The scanner blocked the
shapes `Bun.spawn` and `Bun[x]`, which is the same losing game a module
denylist was: two fire-tested RCEs walked around it, both reported
"pass": true by archgate check:

  - Reflect.get(Bun, "spawn")([...]) — also via const {spawn}=Bun,
    const B=Bun, globalThis/global/self.Bun.spawn
  - (() => {}).constructor("return import('node:child_process')")() — the
    Function constructor via the .constructor chain is eval, bypassing even
    the module allowlist

Apply the allowlist philosophy to globals: block *naming* the capability
source, not the shapes of using it. Any code reference to Bun, process,
globalThis, global, self, Reflect, eval, Function, fetch, WebSocket,
XMLHttpRequest, EventSource, or require is refused (property-key positions
like `foo.process` and `{process:1}` are fine). Blocking the identifier
closes aliasing/destructuring/reflection in one rule. Additionally block
`.constructor` (dotted + computed-literal) on any receiver — the
property-chain route to Function/eval.

This subsumes and simplifies: the per-shape Bun/process member denylists
and the eval/Function/fetch/require call checks are gone. First-party and
imported scans converge (scanImportedRuleSource delegates) — a first-party
rule runs with full privilege too, and a malicious PR can add one. An audit
confirmed zero of the repo's own .rules.ts reference these globals as code
(every mention is a searched-for string), so there are no false positives.

Known residual, documented and regression-tested: a runtime-computed key
`(() => {})[c]` is unknowable to a static scanner, and blocking all computed
access would reject `obj[key]`. That route to eval is left to execution-time
isolation — the scan is defense-in-depth, not a jail.

BREAKING CHANGE: rule files may no longer name Bun/process/globalThis/etc.,
even for benign reads (Bun.env, process.platform). Rules use ctx only.
Amends ARCH-024; coverage in tests/engine/rule-scanner-escapes.test.ts.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@cursor

cursor Bot commented Jul 16, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_b031761c-eb78-4f82-8d6c-75fce385d999)

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@rhuanbarreto, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 67f99c70-43f7-4d2d-a146-f4f1add16763

📥 Commits

Reviewing files that changed from the base of the PR and between 6c66884 and 5b47495.

📒 Files selected for processing (3)
  • .archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md
  • src/engine/rule-scanner.ts
  • tests/engine/rule-scanner-escapes.test.ts
📝 Walkthrough

Walkthrough

The rule scanner now blocks references to a defined set of dangerous runtime globals and rejects .constructor access through dotted and computed-literal forms. Imported rule scanning delegates to the primary scan path, removing separate imported-only checks. Tests were updated for identifier-based messages and locations, with expanded coverage for reflection, aliasing, constructor chains, scan convergence, and allowed global-adjacent code. The ADR documents the revised boundary, residual limitations, risks, and enforcement guidance.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: blocking runtime globals to prevent reflective sandbox escapes.
Description check ✅ Passed The description is directly related to the changeset and accurately explains the scanner hardening and tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 16, 2026

Copy link
Copy Markdown

Deploying archgate-cli with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5b47495
Status: ✅  Deploy successful!
Preview URL: https://dfc8b419.archgate-cli.pages.dev
Branch Preview URL: https://claude-scanner-block-globals.archgate-cli.pages.dev

View logs

rhuanbarreto added a commit that referenced this pull request Jul 16, 2026
…dent

The "review untrusted rules" note cited Reflect.get(Bun, "spawn") as a
residual bypass. The scanner-hardening PR (#481) closes reflective access,
so that example would become stale. Replace it with the durable residual —
anything built at runtime (a computed property name, a code string) is opaque
to a source scan — which holds regardless of which PR merges first. en + nb +
pt-br (GEN-002). llms-full regenerated.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Code Coverage

Metric Value
Lines 91.1% (7872 / 8645)
Threshold 90% minimum — met
Platforms Linux + Windows

Full HTML report available in workflow artifacts.

Per-directory breakdown
Directory Coverage Lines
src/commands/ 89.0% 1959 / 2200
src/engine/ 93.2% 1755 / 1884
src/formats/ 98.7% 148 / 150
src/helpers/ 90.9% 4010 / 4411

@rhuanbarreto

Copy link
Copy Markdown
Contributor Author

Doc coordination done: the security.mdx residual note (which cited Reflect.get(Bun,"spawn")) is updated on the #480 branch to a merge-order-independent example (anything built at runtime is opaque to a source scan), so it stays accurate whichever of #480/#481 merges first.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md:
- Around line 77-81: Close the executable Function-constructor escape instead of
documenting it as an accepted residual. In src/engine/rule-scanner.ts:378-389,
extend the scanner beyond MemberExpression handling to reject constructor
extraction such as object destructuring, or enforce equivalent execution-time
isolation. In .archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md:77-81,
revise the residual and boundary documentation to reflect the closed path. In
tests/engine/rule-scanner-escapes.test.ts:353-363, replace the permissive
assertion with an end-to-end regression that rejects the constructor-based
execution path.

In `@src/engine/rule-scanner.ts`:
- Around line 327-337: Update checkBannedIdentifier so every matching code
identifier advances the occurrence index, including identifiers used as exempt
property keys, while pushViolation is called only for non-property-key
positions. Ensure emitted occurrence indexes remain aligned with actual global
references in both object-literal keys and regular usages.
- Around line 391-393: Update the import.meta require escape handling near the
existing block so it also rejects computed access such as
import.meta["require"]. Use staticPropName() for the property check, preserving
recognition of both dot and bracket forms, and add a regression test covering
the computed escape.

In `@tests/engine/rule-scanner-escapes.test.ts`:
- Around line 321-329: Add XMLHttpRequest and EventSource cases to the codegen
array in the rule-scanner escape test, covering their direct constructor usage
and matching the existing banned-global escape patterns. Keep the cases
alongside the other reflective global entries.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d3d0df67-352b-4063-8c8e-5321fdc5caf8

📥 Commits

Reviewing files that changed from the base of the PR and between 18db14d and 6c66884.

📒 Files selected for processing (6)
  • .archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md
  • src/engine/rule-scanner.ts
  • tests/engine/rule-scanner-adversarial.test.ts
  • tests/engine/rule-scanner-escapes.test.ts
  • tests/engine/rule-scanner-positions.test.ts
  • tests/engine/rule-scanner.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Smoke Test (Windows) / Windows
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (13)
tests/**/*.test.ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-005-testing-standards.md)

tests/**/*.test.ts: Use Bun's built-in test runner (bun:test) for all test files, and place tests under tests/ mirroring the src/ directory structure with <module-name>.test.ts naming.
Use temp directories (mkdtemp) for filesystem tests, and clean them up in afterEach or afterAll.
Close external SDK instances (servers, clients, transports, connections) in afterEach or afterAll, managing their lifecycle in hooks rather than inside individual test bodies.
When a test creates a temporary git repository and runs git commit, configure local user.email and user.name immediately after git init.
Test public module interfaces, not private implementation details.
Use descriptive test names that explain the expected behavior.
Every runnable test()/it() must contain at least one expect() assertion; smoke tests must make the contract explicit with expect(() => fn()).not.toThrow() or await expect(promise).resolves.toBeUndefined().
Use test.skip, test.skipIf, or test.todo for intentionally empty or disabled tests; do not use bare return or empty callbacks to skip work.
If the first expect() is being added to a previously assertion-less test file, add expect to the bun:test import.
When mocking fetch in tests, assign directly to globalThis.fetch and restore the original or use mock.restore() afterward.
Wrap spyOn() and inline mockImplementation() usage in try/finally, or create and restore spies in hooks, so mockRestore() always runs.
Only raise a per-test timeout above the global bun test --timeout 60000; never set a shorter per-test timeout.
Mock first-party modules with import * as mod plus spyOn(mod, "fn"), not mock.module().
When a test needs to redirect user-scope paths, mock os.homedir() instead of relying on HOME/Bun.env.HOME; restore the spy in test hooks.
Do not depend on network access in unit tests.
Do not leave temp files after test runs.
Do not leave external SDK instances open after tests...

Files:

  • tests/engine/rule-scanner-adversarial.test.ts
  • tests/engine/rule-scanner-positions.test.ts
  • tests/engine/rule-scanner-escapes.test.ts
  • tests/engine/rule-scanner.test.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-006-dependency-policy.md)

**/*.{ts,tsx,js,jsx,mjs,cjs}: Use Bun built-ins for file I/O (Bun.file, Bun.write), HTTP, subprocess execution (Bun.spawn), globbing (Bun.Glob), and testing (bun:test).
Do not use Node.js-specific APIs when Bun alternatives exist; for example, use Bun.file() instead of fs.readFile() for simple reads.
Prefer node: built-in modules such as node:util, node:path, and node:fs over npm alternatives.
Do not use utility libraries for single functions, such as importing lodash for one helper like pick.
Do not use path aliases (tsconfig paths); use relative imports with Bun's native module resolution.
Do not install packages globally during development; use bunx for one-off tools.

Files:

  • tests/engine/rule-scanner-adversarial.test.ts
  • tests/engine/rule-scanner-positions.test.ts
  • tests/engine/rule-scanner-escapes.test.ts
  • tests/engine/rule-scanner.test.ts
  • src/engine/rule-scanner.ts
tests/**/*.ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)

In test files, use _resetPlatformCache() to simulate different platforms instead of mocking or mutating process.platform directly.

Files:

  • tests/engine/rule-scanner-adversarial.test.ts
  • tests/engine/rule-scanner-positions.test.ts
  • tests/engine/rule-scanner-escapes.test.ts
  • tests/engine/rule-scanner.test.ts
{src,tests}/**/*.ts

📄 CodeRabbit inference engine (.archgate/adrs/LEGAL-001-spdx-license-headers.md)

{src,tests}/**/*.ts: Every TypeScript source file in src/ and tests/ must begin with // SPDX-License-Identifier: Apache-2.0 followed by // Copyright 2026 Archgate.
If a TypeScript file has a shebang line (for example #!/usr/bin/env bun in src/cli.ts), the SPDX license header must appear immediately after the shebang.
Use single-line // comments for the SPDX header; do not use block comments (/* */) or alternate license identifiers.

Files:

  • tests/engine/rule-scanner-adversarial.test.ts
  • tests/engine/rule-scanner-positions.test.ts
  • tests/engine/rule-scanner-escapes.test.ts
  • tests/engine/rule-scanner.test.ts
  • src/engine/rule-scanner.ts
tests/engine/rule-scanner*.test.ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md)

Scanner tests MUST cover both property spellings, legitimate rule code, source positions, raw-text violations, obfuscated specifiers, and the documented computed-variable .constructor residual.

Files:

  • tests/engine/rule-scanner-adversarial.test.ts
  • tests/engine/rule-scanner-positions.test.ts
  • tests/engine/rule-scanner-escapes.test.ts
  • tests/engine/rule-scanner.test.ts
**

⚙️ CodeRabbit configuration file

**: This project uses Archgate — an AI governance framework based on
Architecture Decision Records (ADRs). The ADRs in .archgate/adrs/
are the authoritative rules for this codebase. Each ADR has a companion
.rules.ts file with automated checks that run via archgate check.

When reviewing, you must:

  1. Treat ADR violations as blocking issues, not suggestions.
  2. Cite the specific ADR ID when flagging a violation (e.g., "Violates ARCH-006").
  3. Focus on semantic and contextual violations that automated rules cannot catch —
    the .rules.ts files already cover syntactic/structural patterns.
  4. If you are unsure whether something violates an ADR, flag it as a question
    rather than approving it.

Files:

  • tests/engine/rule-scanner-adversarial.test.ts
  • tests/engine/rule-scanner-positions.test.ts
  • tests/engine/rule-scanner-escapes.test.ts
  • tests/engine/rule-scanner.test.ts
  • src/engine/rule-scanner.ts
tests/engine/rule-scanner-escapes.test.ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md)

Every discovered scanner escape MUST first receive a failing regression test, and tests MUST assert that malicious module constructs, global access paths, constructor access, and invisible characters are rejected.

Files:

  • tests/engine/rule-scanner-escapes.test.ts
src/**/*.ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-003-output-formatting.md)

src/**/*.ts: Use styleText(format, text) from node:util for all terminal colors and formatting in CLI source files; do not use raw ANSI escape codes or third-party color libraries.
Commands that produce structured results and support --json must emit machine-readable JSON to stdout with no colors or decorative formatting.
When a command supports --json, use formatJSON() from src/helpers/output.ts for JSON serialization, and pass forcePretty: true when the user explicitly provided --json.
Use isAgentContext() from src/helpers/output.ts to enable auto-JSON behavior for commands that support both human-readable and JSON output modes.
CLI output must not include emoji; use text symbols and colors instead.
Send normal command output to stdout with console.log(), and send errors, warnings, and debug messages to stderr via logError(), logWarn(), and logDebug().
Keep CLI output concise and scannable by using whitespace and alignment instead of long text blocks.
Respect NO_COLOR automatically by relying on styleText; do not add custom color-environment handling in CLI code.
Do not output progress spinners unless there is a TTY check.
Do not assume piped output means agent context when CI is set; CI runners should still receive human-readable output.

src/**/*.ts: Do not re-export symbols from another module in any source file; statements like export { X } from "./other" and export type { X } from "./other" are forbidden.
Import symbols directly from the module that defines them; do not import from a directory path such as ../formats expecting implicit index.ts resolution.

Make large production thresholds injectable via an optional parameter that defaults to the module constant, so tests can supply a small value instead of generating huge fixtures.

src/**/*.ts: Use await Bun.file(path).json() when reading JSON files in Bun TypeScript source code; do not use JSON.parse(await Bun.file(path).text()) or `JSON.parse(fs.readFile...

Files:

  • src/engine/rule-scanner.ts
src/**/!(*platform).ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)

src/**/!(*platform).ts: In src/ TypeScript source files, do not read process.platform directly; use src/helpers/platform.ts (isWindows(), isMacOS(), isLinux(), isWSL(), getPlatformInfo()) for all platform detection.
Use the centralized platform helper instead of duplicating OS/WSL detection logic inline anywhere in src/ TypeScript source.
When behavior differs between Linux and Windows, account for WSL by using isWSL() rather than assuming `

Files:

  • src/engine/rule-scanner.ts
src/engine/rule-scanner.ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-022-ast-aware-rule-context.md)

src/engine/rule-scanner.ts: Keep rule-scanner.ts using the shared parseModule() helper instead of duplicating the parser call inline again.
rule-scanner.ts must continue blocking banned imports, dangerous Bun.* property access, eval/Function, non-literal dynamic import(), and globalThis/process.env mutation in .rules.ts source.
The sandbox must continue to block Bun.spawn and Bun.spawnSync from .rules.ts code.
The sandbox must continue to forbid .rules.ts code from reaching subprocess primitives directly, leaving ctx.ast() as the only sanctioned path.

src/engine/rule-scanner.ts: scanRuleSource() MUST enforce an allowlist of module specifiers via ALLOWED_MODULES; dangerous-module denylists MUST NOT be used.
Only node:path, node:url, node:util, and node:crypto may be allowlisted; bare specifiers and node:module MUST remain blocked.
All module-evaluating constructs—static imports, dynamic imports, re-exports, and export-star declarations—MUST use the same module-specifier check; non-literal dynamic imports MUST be rejected.
References to dangerous runtime globals, including Bun, process, globalThis, global, self, Reflect, eval, Function, fetch, WebSocket, XMLHttpRequest, and require, MUST be blocked by identifier name.
.constructor access MUST be rejected for every receiver in both dotted and computed-literal forms.
Property-name checks MUST recognize both o.name and o["name"] using staticPropName(); runtime-computed keys remain intentionally unsupported.
The raw source pass MUST reject bidi controls, directional marks, and zero-width or invisible characters, while permitting a leading BOM; blocked code points MUST be represented numerically.
Raw-text scanning MUST NOT search for dangerous identifiers or module names; semantic security checks belong to the AST scanner.
Process-internal property names such as binding, dlopen, and _linkedBinding MUST be matched by property name alone s...

Files:

  • src/engine/rule-scanner.ts
src/{helpers,engine}/**/*.ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-002-error-handling.md)

Do not use console.log(), console.warn(), or console.info() directly in helper or engine files; use logInfo() or logWarn() instead.

Files:

  • src/engine/rule-scanner.ts
src/engine/**/*.ts

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md)

src/engine/**/*.ts: In the rules engine, list project files by matching glob patterns in memory against the git-tracked file set rather than walking the filesystem.
Route new engine file listings through listMatchingFiles for rule-facing inputs or matchTrackedFiles for trusted ADR frontmatter patterns.
When the target is a Git repository and respectGitignore is not false, pass the tracked set from getGitTrackedFiles to matching operations.
Per-run RunCaches must share glob results keyed by pattern and tracked mode and file text keyed by absolute path; cached promises should share in-flight work, and returned glob arrays must be copied before exposing them to rules.
Do not cache readJSON results because rules receive mutable objects whose shared mutation could leak between rules.
Do not filter filesystem scan results against the tracked set as a substitute for in-memory matching, and do not hardcode ignore directories; use the Git-derived tracked set instead.

Files:

  • src/engine/rule-scanner.ts
src/engine/{rule-scanner.ts,loader.ts}

📄 CodeRabbit inference engine (.archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md)

First-party and imported rule scans MUST remain converged, with scanImportedRuleSource() delegating to scanRuleSource() unless an imported-only restriction is genuinely required.

Files:

  • src/engine/rule-scanner.ts
🧠 Learnings (1)
📚 Learning: 2026-07-11T13:03:15.386Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 467
File: .archgate/adrs/ARCH-011-consistent-project-root-resolution.md:0-0
Timestamp: 2026-07-11T13:03:15.386Z
Learning: For Markdown files formatted by oxfmt (especially ADRs), avoid inline code spans that contain escaped backticks, e.g. `\`...\`` inside a single `` `...` `` span. oxfmt may mis-parse these and, on re-format, can collapse spaces after later inline code spans on the same line, effectively removing any manually re-added spacing. Instead, rephrase the text so the message stays plain quoted text, and put any embedded command/fragment that needs code formatting (e.g., `archgate init`) in its own separate inline code span; keep surrounding punctuation/spacing outside the code span.

Applied to files:

  • .archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md
🪛 LanguageTool
.archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md

[style] ~73-~73: Consider removing “of” to be more concise
Context: ...name the same capability are unbounded. All of the following reach Bun.spawn without mat...

(ALL_OF_THE)


[style] ~81-~81: Consider an alternative for the overused word “exactly”.
Context: ...cannot resolve the specifier, and it is exactly why this ADR names execution-time isola...

(EXACTLY_PRECISELY)

🔇 Additional comments (6)
.archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md (1)

71-75: LGTM!

Also applies to: 150-156

src/engine/rule-scanner.ts (1)

35-70: LGTM!

Also applies to: 475-489

tests/engine/rule-scanner-adversarial.test.ts (1)

94-101: LGTM!

tests/engine/rule-scanner-escapes.test.ts (1)

18-21: LGTM!

Also applies to: 93-100, 111-119, 159-161, 290-318, 331-352, 365-392

tests/engine/rule-scanner-positions.test.ts (1)

36-38: LGTM!

Also applies to: 138-142, 175-179, 190-195, 205-206, 286-298

tests/engine/rule-scanner.test.ts (1)

48-78: LGTM!

Also applies to: 187-188, 213-250, 303-333

Comment thread .archgate/adrs/ARCH-024-rule-file-sandbox-boundary.md Outdated
Comment thread src/engine/rule-scanner.ts Outdated
Comment thread src/engine/rule-scanner.ts Outdated
Comment thread tests/engine/rule-scanner-escapes.test.ts
Addresses CodeRabbit review on the reflective-globals sandbox hardening:

- Block `.constructor` reached through a destructuring binding pattern
  (`const { constructor: F } = x`) — an ObjectPattern the MemberExpression
  check never visited, reaching the Function constructor (= eval) by a
  different AST path. Covers renamed, computed-string, and shorthand keys.
- Handle the computed spelling `import.meta["require"]` and anchor its
  violation on `import.meta` so it reports a real position instead of
  remapping to line 0 (Bun's transpiler normalises the bracket form).
- Advance the banned-identifier occurrence counter for property-key slots
  too, so a real global reference after a same-named object key remaps to
  the reference rather than the earlier key.
- Add escape-suite coverage for XMLHttpRequest and EventSource, which were
  in the banned-globals set but untested.

Amends ARCH-024 to document the destructuring route and fold the
computed-variable residual into both member and destructuring spellings.
Enforcement stays in tests/engine/rule-scanner-escapes.test.ts.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@cursor

cursor Bot commented Jul 16, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_1869ae46-0dee-4e64-9fe2-bd228f2a0d82)

@rhuanbarreto
rhuanbarreto merged commit 6666df2 into main Jul 16, 2026
23 checks passed
@rhuanbarreto
rhuanbarreto deleted the claude/scanner-block-globals branch July 16, 2026 14:13
@archgatebot archgatebot Bot mentioned this pull request Jul 16, 2026
rhuanbarreto pushed a commit that referenced this pull request Jul 16, 2026
# archgate

## [0.49.0](v0.48.4...v0.49.0)
(2026-07-16)

### ⚠ BREAKING CHANGES

* **engine:** block naming runtime globals to close reflective sandbox
escapes (#481)
* **engine:** close rule-file sandbox escapes via module allowlist
(#477)
* **cli:** emit lean agent-facing JSON payloads by default (#476)

### Features

* **cli:** emit lean agent-facing JSON payloads by default
([#476](#476))
([04affa3](04affa3))
* **engine:** base-revision + comment access for ctx.ast() (closes
[#479](#479))
([#480](#480))
([49feb1f](49feb1f)),
closes [#477](#477)

### Bug Fixes

* **engine:** block naming runtime globals to close reflective sandbox
escapes ([#481](#481))
([6666df2](6666df2)),
closes [#477](#477)
[#480](#480)
* **engine:** close rule-file sandbox escapes via module allowlist
([#477](#477))
([18db14d](18db14d))

---
This PR was generated with
[simple-release](https://github.com/TrigenSoftware/simple-release).

<details>
<summary>📄 Cheatsheet</summary>
<br>



You can configure the bot's behavior through a pull request comment
using the `!simple-release/set-options` command.

### Command Format

````md
!simple-release/set-options

```json
{
  "bump": {},
  "publish": {}
}
```
````

### Useful Parameters

#### Bump

| Parameter | Type | Description |
|-----------|------|-------------|
| `version` | `string` | Force set specific version |
| `as` | `'major' \| 'minor' \| 'patch' \| 'prerelease'` | Release type
|
| `prerelease` | `string` | Pre-release identifier (e.g., "alpha",
"beta") |
| `firstRelease` | `boolean` | Whether this is the first release |
| `skip` | `boolean` | Skip version bump |
| `byProject` | `Record<string, object>` | Per-project bump options for
monorepos |

#### Publish

| Parameter | Type | Description |
|-----------|------|-------------|
| `skip` | `boolean` | Skip publishing |
| `access` | `'public' \| 'restricted'` | Package access level |
| `tag` | `string` | Tag for npm publication |

### Usage Examples

#### Force specific version

````md
!simple-release/set-options

```json
{
  "bump": {
    "version": "2.0.0"
  }
}
```
````

#### Force major bump

````md
!simple-release/set-options

```json
{
  "bump": {
    "as": "major"
  }
}
```
````

#### Create alpha pre-release

````md
!simple-release/set-options

```json
{
  "bump": {
    "prerelease": "alpha"
  }
}
```
````

#### Publish with specific access and tag

````md
!simple-release/set-options

```json
{
  "bump": {
    "prerelease": "beta"
  },
  "publish": {
    "access": "public",
    "tag": "beta"
  }
}
```
````

### Access Restrictions

The command can only be used by users with permissions:
- repository owner
- organization member
- collaborator

### Notes

- The last comment with `!simple-release/set-options` command takes
priority
- JSON must be valid, otherwise the command will be ignored
- Parameters apply only to the current release execution
- The command can be updated by editing the comment or adding a new one


</details>

<!--
  Please do not edit this comment.
  simple-release-pull-request: true
  simple-release-branch-from: release
  simple-release-branch-to: main
-->

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant