Skip to content

ctx.ast(): add base-revision access so sandbox hardening (#477) doesn't strand legitimate rules #479

Description

@rhuanbarreto

Summary

RuleContext.ast() (ARCH-022) can only parse a current, on-disk, working-tree file, and its output omits comments and docstrings. Two common and legitimate rule patterns need capabilities beyond that, and today the only ways to get them are (a) shelling out to a subprocess or (b) hand-rolled line/regex heuristics.

Option (a) is precisely what the rule-file sandbox forbids, and #477 has now tightened that sandbox from a module denylist to an allowlist. That closes a real RCE (see #477 / ARCH-024), but it also removes the escape hatch these rule patterns were relying on — with no sanctioned replacement. This issue proposes that replacement so the sandbox hardening does not strand legitimate rules.

Background

  • fix(engine)!: close rule-file sandbox escapes via module allowlist #477 replaces the .rules.ts module denylist with an allowlist (node:path, node:url, node:util, node:crypto only). A rule can no longer import node:child_process or otherwise reach a subprocess directly.
  • ARCH-022 already states the intended boundary: "A rule author MUST NEVER be able to reach Bun.spawn, child_process, or any other subprocess/filesystem primitive directly; ctx.ast() is the only door." For that promise to be honest, ctx.ast() has to be capable enough that authors don't need another door.
  • ARCH-024 (added in fix(engine)!: close rule-file sandbox escapes via module allowlist #477) documents the boundary and names execution-time isolation as the long-term direction. This issue is the near-term companion: widen the sanctioned door so the boundary can actually hold.

The capability gaps

Gap 1 — no base-revision access (this is the one that motivates a subprocess)

A frequent rule shape is a "documentation-only change" waiver: relax some requirement (a version bump, a changelog entry, a review gate) when a changed file differs from its base revision only in comments/docstrings — i.e. its executable structure is unchanged.

Implementing this requires comparing a file against its base git revision:

  1. get the base revision's source (git show <base>:<path>), and
  2. parse both revisions structurally and compare, ignoring comments/formatting.

ctx.ast(path, language) and ctx.readFile(path) both take an on-disk working-tree path only. There is no way to reach the base revision through ctx. So a rule author must run git show themselves and parse its output (which ctx.ast() also can't do, since it takes a path, not a source string) — both of which require a subprocess. This is the capability gap that pushes authors to the exact pattern the sandbox now blocks.

Notably, the engine already resolves the base ref (resolveBaseRef in src/engine/git-files.ts) and already computes changedFiles relative to it. The base ref exists inside the engine; it simply isn't exposed to ctx.

Gap 2 — comments/docstrings are not in the AST

parseJsModule (src/engine/js-parser.ts) calls meriyah without comment collection, and Python's ast module has no comment nodes at all. So any comment-governance rule — e.g. a policy on comment length, style, or content — cannot be written against ctx.ast(); it has to fall back to line-by-line regex heuristics that are fragile across languages and comment syntaxes.

This gap does not force a subprocess (comment analysis can limp along on heuristics), but it is the same root cause: the AST view is not rich enough, so authors route around it.

Why now

The two changes are halves of one thing. Tightening the sandbox (#477) without providing these capabilities means any project relying on a base-revision comparison rule will, on upgrading, have that rule refused by the scanner with no supported alternative. The capability MUST ship in the same release as — or a release before — the sandbox tightening reaches users, or the tightening breaks legitimate rules.

Proposed design

Two independent pieces, gated on ARCH-022 review since they extend ctx.ast().

A. Base-revision structural access (addresses Gap 1)

Expose the already-resolved base ref through ctx, and let ast()/a source reader target it. Sketch:

// Read a file's source at the comparison base ref. null if the path
// did not exist at that ref (added file) or no base is resolvable.
ctx.fileAtBase(path: string): Promise<string | null>;

// Parse a file at the base ref, same return shapes as ctx.ast today.
ctx.ast(path: string, language: AstLanguage, opts?: { rev: "base" }): Promise<AstNode>;

A documentation-only waiver then needs no subprocess:

const before = await ctx.ast(file, "python", { rev: "base" });
const after = await ctx.ast(file, "python");
if (before && structurallyEqualIgnoringPositions(before, after)) {
  // executable structure unchanged -> documentation-only
}

Implementation notes:

  • The base-revision read runs git show <base>:<path> inside src/engine/git-files.ts — the existing sanctioned git subprocess site — so no new privileged surface is introduced and the four ARCH-022 guardrails still gate every parser invocation.
  • Comment/docstring insensitivity largely comes for free: both meriyah and Python's ast already drop comments, so a comment-only diff yields identical trees. Position attributes (loc, lineno, col_offset) should be strippable by the caller (or excluded by an equality helper) so formatting-only diffs also compare equal. Docstrings are real string nodes and remain the author's responsibility to ignore if they wish.
  • Fail-closed semantics should match ctx.ast()'s existing throw contract: unresolvable base, missing interpreter, or parse failure throws rather than silently returning a false "equal."

B. Comments in the AST (addresses Gap 2)

Optionally attach comments to ctx.ast() output:

const tree = await ctx.ast(file, "typescript", { comments: true });
// tree.comments: Array<{ type: "line" | "block", value: string, loc: {...} }>

Per language:

  • TS/JS — meriyah supports comment collection (onComment); low cost.
  • Pythonast has no comments; would require the tokenize module in the serializer subprocess.
  • RubyRipper.lex (not Ripper.sexp) surfaces comment tokens.

This cuts against ARCH-022's deliberate "AST shapes are not unified across languages" stance and adds per-language work, so it warrants its own design pass and possibly its own ADR amendment. It is separable from A and lower priority (it does not unblock a subprocess).

Suggested sequencing

  1. Ship A (base-revision access) so the primary subprocess motive disappears.
  2. Land it in the same release as — or before — fix(engine)!: close rule-file sandbox escapes via module allowlist #477's sandbox tightening reaches users, so no legitimate rule is stranded.
  3. Treat B (comments in AST) as a follow-up, evaluated against ARCH-022's unification stance.

Acceptance criteria

  • A rule can obtain a structural view of a file at the comparison base ref through ctx, with no .rules.ts subprocess or filesystem access.
  • The base-revision read is confined to the sanctioned git site (src/engine/git-files.ts) and every parser invocation still passes the ARCH-022 guardrail ordering.
  • Failure modes (no base, added/deleted file, parse error, missing interpreter) throw per the existing ctx.ast() contract — never a silent false-equal.
  • Documented example: a documentation-only-change waiver implemented with zero subprocess use.
  • ARCH-022 amended (or a companion ADR added) to cover the base-revision surface.

Out of scope

  • Execution-time isolation of .rules.ts (a worker/subprocess/restricted-resolver sandbox) — tracked separately; ARCH-024 names it as the direction for strengthening the boundary's nature.
  • Unifying AST node shapes across languages — explicitly rejected by ARCH-022.

References

  • fix(engine)!: close rule-file sandbox escapes via module allowlist #477 — rule-file sandbox escapes / module allowlist (the change that makes this urgent)
  • ARCH-022 — AST-Aware Rule Context (ctx.ast())
  • ARCH-024 — Rule File Sandbox Boundary
  • src/engine/git-files.ts — existing sanctioned git subprocess site and base-ref resolution
  • src/engine/ast-support.ts, src/engine/js-parser.ts — current ctx.ast() parser implementations

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions