fix(extensions): bound the AST walks over author-supplied source

.changeset/bounded-ast-walk-for-author-source.md

@@ -0,0 +1,45 @@
+---
+'@ifc-lite/extensions': patch
+---
+
+Bound the AST walks over extension-author source so a deeply nested script is
+reported, not fatal.
+
+`validateCode` and `inferCapabilities` both fed an AST parsed from
+author-supplied source to `acorn-walk`'s `walk.simple`, which recurses once per
+AST level. A script nested a few hundred levels deep threw
+`RangeError: Maximum call stack size exceeded` out of the middle of both
+functions, escaping the result shape each one is declared to return. Measured
+here, an 800-level script overflowed and a 700-level one did not, and which of
+the two overflowed moved with test ordering — the failure point tracked whatever
+stack the caller happened to have left.
+
+Both now traverse through a new internal `walkBounded`
+(`src/ast/bounded-walk.ts`), which keeps its own stack on the heap and stops at
+`MAX_AST_DEPTH = 1000` (~500 source levels of `if (1) { … }`, below acorn's own
+parse floor of roughly 1200). It descends using `acorn-walk`'s `base` visitor
+and reports nodes in `walk.simple`'s post-order, so which child positions count
+as nodes — non-computed member properties and object keys stay unvisited — and
+the order they arrive in are unchanged. Behaviour below the bound is identical.
+
+Catching the `RangeError` would have been the smaller change and is the wrong
+one: it makes the accept/reject boundary depend on the remaining call stack, so
+the same script passes on one code path and fails on another. The bound is a
+reported result instead.
+
+What each site returns at the bound:
+
+- **`validateCode`** adds an `invalid_value` error naming the limit and returns
+  `ok: false`. A truncated walk has not proven the source clean; anything below
+  the cut-off went uninspected, so reporting `ok` would be a pass on a partial
+  inspection.
+- **`inferCapabilities`** returns an empty capability set *and* a `parseErrors`
+  entry naming the limit. The capabilities found before the walk stopped are a
+  floor, not the answer. Returning them alone would fail open in both callers:
+  `migrateSavedScripts` treats an empty set as "grant `model.read` and migrate
+  anyway", and the promote dialog renders it as "no `bim.*` calls detected".
+  `parseErrors` is the channel both already use to refuse a script — the
+  migration now skips it and the dialog shows its warning.
+
+No public API change; `walkBounded` is not exported from the package entry
+point.

packages/extensions/src/ast/bounded-walk.test.ts

@@ -0,0 +1,108 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+import { describe, it, expect } from 'vitest';
+import * as acorn from 'acorn';
+import * as walk from 'acorn-walk';
+import { MAX_AST_DEPTH, walkBounded } from './bounded-walk.js';
+
+function parse(src: string): acorn.Node {
+  return acorn.parse(src, { ecmaVersion: 'latest', sourceType: 'module' });
+}
+
+/** `levels` nested `if (1) { … }` blocks around `inner`. */
+function nestIf(levels: number, inner: string): string {
+  return 'if(1){'.repeat(levels) + inner + '}'.repeat(levels);
+}
+
+describe('walkBounded', () => {
+  it('visits the same nodes, in the same order, as walk.simple', () => {
+    const src = `
+      const a = { window: 1, ['self']: 2 };
+      function f(x) { return x.process + eval('1'); }
+      label: for (const k of [1, 2]) { f(k); }
+      class C { document() { return new Function('a'); } }
+      import('./x.js');
+    `;
+    const ast = parse(src);
+
+    const viaAcorn: string[] = [];
+    const types = [
+      'Identifier',
+      'CallExpression',
+      'NewExpression',
+      'ImportExpression',
+      'MemberExpression',
+      'Property',
+      'BlockStatement',
+    ];
+    const visitors: Record<string, (n: acorn.Node) => void> = {};
+    for (const t of types) {
+      visitors[t] = (n) => viaAcorn.push(`${t}@${n.start}-${n.end}`);
+    }
+    walk.simple(ast as acorn.AnyNode, visitors as never);
+
+    const viaBounded: string[] = [];
+    const res = walkBounded(ast, (node, type) => {
+      if (types.includes(type)) {
+        viaBounded.push(`${type}@${node.start as number}-${node.end as number}`);
+      }
+    });
+
+    expect(res.depthExceeded).toBe(false);
+    expect(viaBounded).toEqual(viaAcorn);
+    // Guard against the comparison being vacuous.
+    expect(viaAcorn.length).toBeGreaterThan(12);
+  });
+
+  it('does not visit a non-computed member property, matching acorn-walk', () => {
+    const names: string[] = [];
+    walkBounded(parse('foo.window;'), (node, type) => {
+      if (type === 'Identifier') names.push(node.name as string);
+    });
+    expect(names).toEqual(['foo']);
+  });
+
+  it('does visit a computed member property, matching acorn-walk', () => {
+    const names: string[] = [];
+    walkBounded(parse('foo[window];'), (node, type) => {
+      if (type === 'Identifier') names.push(node.name as string);
+    });
+    expect(names).toEqual(['foo', 'window']);
+  });
+
+  it('completes a deep-but-legal script without reporting depth', () => {
+    // 400 source levels -> ~800 AST levels, under the 1000 bound.
+    const res = walkBounded(parse(nestIf(400, 'x;')), () => {});
+    expect(res.depthExceeded).toBe(false);
+  });
+
+  it('reports depth instead of throwing on a script past the bound', () => {
+    // 800 source levels -> ~1600 AST levels, over the bound. The
+    // recursive acorn-walk this replaced threw RangeError here.
+    const res = walkBounded(parse(nestIf(800, 'x;')), () => {});
+    expect(res.depthExceeded).toBe(true);
+  });
+
+  it('bounds by AST depth, not source depth: two AST levels per if-block', () => {
+    // Pins the constant's meaning. A source nesting of just over
+    // MAX_AST_DEPTH/2 must trip the bound; just under must not.
+    expect(walkBounded(parse(nestIf(MAX_AST_DEPTH / 2 - 20, 'x;')), () => {}).depthExceeded)
+      .toBe(false);
+    expect(walkBounded(parse(nestIf(MAX_AST_DEPTH / 2 + 20, 'x;')), () => {}).depthExceeded)
+      .toBe(true);
+  });
+
+  it('a wide-but-shallow AST is never depth-limited', () => {
+    // Far more nodes than MAX_AST_DEPTH, all at depth ~3. The bound
+    // must be about nesting, not node count.
+    const src = Array.from({ length: MAX_AST_DEPTH * 3 }, (_, i) => `a${i};`).join('\n');
+    let seen = 0;
+    const res = walkBounded(parse(src), (_n, type) => {
+      if (type === 'Identifier') seen++;
+    });
+    expect(res.depthExceeded).toBe(false);
+    expect(seen).toBe(MAX_AST_DEPTH * 3);
+  });
+});

packages/extensions/src/ast/bounded-walk.ts

@@ -0,0 +1,162 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+/**
+ * Depth-bounded, non-recursive AST traversal.
+ *
+ * Every AST walked in this package comes from source an extension
+ * author supplies, so the traversal is attacker-reachable and must not
+ * be able to exhaust the JS call stack. `acorn-walk`'s walkers recurse
+ * once per AST level; a script nested a few hundred levels deep throws
+ * `RangeError: Maximum call stack size exceeded` out of the middle of
+ * whatever function invoked the walk.
+ *
+ * This module is the single traversal used by every AST consumer here.
+ * It keeps its own stack on the heap and stops at {@link MAX_AST_DEPTH},
+ * *reporting* that it stopped rather than throwing. Callers vary the
+ * visitor; they do not re-implement the traversal.
+ *
+ * It descends using `acorn-walk`'s own `base` visitor rather than
+ * enumerating object properties generically, so which child positions
+ * count as nodes is identical to what `walk.simple` would have visited:
+ * non-computed member properties, non-computed object keys and labels
+ * stay unvisited. Nodes are reported in `walk.simple`'s post-order
+ * (children before parent) for the same reason — swapping either would
+ * silently change what the call sites see.
+ *
+ * Deliberately NOT exported from the package entry point — internal
+ * utility, not public API.
+ */
+
+import * as walk from 'acorn-walk';
+
+/**
+ * Maximum AST nesting depth any walk in this package will inspect.
+ *
+ * Real scripts nest a few tens of levels deep; this bound is two orders
+ * of magnitude above that. It exists because the AST comes from
+ * author-controlled source: past this depth the walk stops and the
+ * caller reports a validation failure instead of continuing.
+ *
+ * One `if (1) { … }` source level costs two levels here
+ * (`IfStatement` -> `BlockStatement`), and acorn's own parser gives up
+ * at roughly 1200 *source* levels ("Not enough stack space to parse
+ * input"). That parser limit moves with however much stack the host
+ * happens to have left; this one does not, which is the entire point —
+ * the accept/reject boundary must not depend on the caller's remaining
+ * stack.
+ *
+ * Catching the `RangeError` instead would reintroduce exactly that
+ * dependency. Measured on this repo's suite before the fix, an
+ * unbounded walk over a 600-level script overflowed while a 700-level
+ * one did not, and which of the two overflowed moved with test order.
+ */
+export const MAX_AST_DEPTH = 1000;
+
+/** Minimal structural view of an ESTree node. */
+export interface AstNode {
+  type: string;
+  [key: string]: unknown;
+}
+
+export interface BoundedWalkResult {
+  /**
+   * True if traversal stopped early because a node deeper than
+   * {@link MAX_AST_DEPTH} was reached. When true the visit is
+   * incomplete and the caller MUST treat the result as a failure —
+   * never as "nothing found".
+   */
+  depthExceeded: boolean;
+}
+
+function isAstNode(value: unknown): value is AstNode {
+  return (
+    typeof value === 'object' &&
+    value !== null &&
+    typeof (value as { type?: unknown }).type === 'string'
+  );
+}
+
+interface Frame {
+  node: AstNode;
+  depth: number;
+  /**
+   * `acorn-walk` re-dispatches some nodes under a synthetic type
+   * ("Statement", "Expression", "Function", "Pattern", …). The visitor
+   * key is `override || node.type`, exactly as in `walk.simple`.
+   */
+  override?: string;
+  /** False on the descend pass, true on the report pass. */
+  expanded: boolean;
+}
+
+/**
+ * Visit every node `walk.simple` would have visited, in the same order,
+ * without recursing and without exceeding {@link MAX_AST_DEPTH}.
+ *
+ * The visitor is handed the raw node plus the type key `walk.simple`
+ * would have looked its visitor up under; call sites switch on that
+ * key. One traversal is therefore shared across sites that care about
+ * entirely different node types.
+ *
+ * Returns `{ depthExceeded: true }` if the bound stopped the walk. The
+ * traversal never throws for depth reasons.
+ */
+export function walkBounded(
+  root: unknown,
+  visit: (node: AstNode, type: string) => void,
+): BoundedWalkResult {
+  if (!isAstNode(root)) return { depthExceeded: false };
+
+  const baseVisitor = walk.base as unknown as Record<
+    string,
+    ((node: unknown, state: unknown, c: (child: unknown, state: unknown, override?: string) => void) => void) | undefined
+  >;
+
+  const stack: Frame[] = [{ node: root, depth: 0, expanded: false }];
+
+  while (stack.length > 0) {
+    const frame = stack.pop()!;
+    const type = frame.override ?? frame.node.type;
+
+    if (frame.expanded) {
+      visit(frame.node, type);
+      continue;
+    }
+
+    if (frame.depth > MAX_AST_DEPTH) return { depthExceeded: true };
+
+    const baseFn = baseVisitor[type];
+    if (!baseFn) {
+      // Unknown node type: report it and stop descending, matching
+      // acorn-walk's own behaviour of throwing only on a missing base.
+      visit(frame.node, type);
+      continue;
+    }
+
+    const children: Frame[] = [];
+    baseFn(frame.node, null, (child, _state, override) => {
+      if (!isAstNode(child)) return;
+      children.push({
+        node: child,
+        // `skipThrough` bases re-dispatch the *same* node under a new
+        // key; that is not a step down the tree, so it must not consume
+        // a depth level.
+        depth: child === frame.node ? frame.depth : frame.depth + 1,
+        override,
+        expanded: false,
+      });
+    });
+
+    // Report-after-children, matching walk.simple's post-order: push
+    // the report frame first so it pops last, then the children in
+    // reverse so the LIFO stack takes them in source order.
+    stack.push({ ...frame, expanded: true });
+    for (let i = children.length - 1; i >= 0; i--) {
+      stack.push(children[i]!);
+    }
+  }
+
+  return { depthExceeded: false };
+}

packages/extensions/src/flavor/migrate-scripts.test.ts

@@ -37,6 +37,19 @@ describe('migrateSavedScripts', () => {
     expect(r.skipped).toHaveLength(1);
   });
 
+  it('skips a script too deeply nested to infer capabilities from', () => {
+    // Capability inference stops at its AST depth bound and reports the
+    // stop as a parse error. That must land in `skipped` — the
+    // alternative, an empty capability set, silently falls through to
+    // the `model.read` fallback above and migrates an uninspected
+    // script.
+    const code = `${'if(1){'.repeat(800)}bim.model.write();${'}'.repeat(800)}`;
+    const r = migrateSavedScripts([{ id: 'deep', name: 'Deep', code }]);
+    expect(r.extensions).toHaveLength(0);
+    expect(r.skipped).toHaveLength(1);
+    expect(r.skipped[0].reason).toMatch(/nested more than \d+ AST levels/);
+  });
+
   it('produces a slug-stable extension id', () => {
     const r = migrateSavedScripts([
       { id: 'Count Walls!', name: 'Count Walls!', code: 'bim.query.byType("IfcWall");' },