fix(extensions): bound the entry-source AST walk instead of overflowing the stack

.changeset/source-wrap-banned-construct-recursion.md

@@ -0,0 +1,11 @@
+---
+"@ifc-lite/extensions": patch
+---
+
+Make `wrapEntrySource`'s banned-construct check walk the entire entry-script AST instead of only its top-level statements.
+
+The check existed to flag `import`/`export` syntax at wrap time so extension authors get a clear, early error instead of a confusing runtime failure. It only ever inspected `ast.body`, so any of those constructs written inside a nested function, arrow body, or class method passed silently. In practice the QuickJS sandbox realm has no module loader registered, so a nested dynamic `import(...)` was always going to fail at runtime anyway with an opaque engine error — this change moves that failure earlier and makes it legible, and closes the gap between what the check's name and callers assume ("banned constructs are caught") and what it verified.
+
+The walk now also flags dynamic `import(...)` anywhere it appears, not just static top-level `import`/`export` declarations (which the ECMAScript grammar restricts to the top level regardless of where the walk looks). `eval` and `new Function` are deliberately left alone: both run confined inside the same non-module sandbox realm with no path to the host bridge, and banning them would restrict legitimate extension code for no isolation benefit.
+
+The walk iterates over an explicit stack rather than recursing, and stops at a fixed depth of 1000 AST levels. `wrapEntrySource` returns a `ValidationResult`, so a deeply nested entry script has to come back as a reported error; a recursive walk instead threw a `RangeError` ("Maximum call stack size exceeded") out of the middle of it, at roughly 500 nested blocks. Past the bound the script is now rejected with an `invalid_value` error naming the limit, matching how acorn's own parser already degrades on input it cannot handle. Real entry scripts nest a few tens of levels deep.

packages/extensions/src/host/source-wrap.test.ts

@@ -132,3 +132,53 @@ describe('wrapEntrySource — entryFnName validation', () => {
     expect(r.ok).toBe(true);
   });
 });
+
+describe('wrapEntrySource — nested banned constructs', () => {
+  it('rejects dynamic import() hidden inside a function body', () => {
+    const r = wrapEntrySource(
+      'function activate(ctx) { import("node:fs").then(fs => console.log(fs)); }',
+      { entryFnName: 'activate' },
+    );
+    expect(r.ok).toBe(false);
+    if (!r.ok) {
+      expect(r.errors.some((e) => e.message.toLowerCase().includes('import'))).toBe(true);
+    }
+  });
+});
+
+describe('wrapEntrySource — deeply nested entry scripts', () => {
+  const nested = (levels: number) =>
+    `${'if (1) {'.repeat(levels)}function activate(ctx) {}${'}'.repeat(levels)}`;
+
+  it('reports a validation error instead of throwing on a deeply nested script', () => {
+    // 900 nested `if` blocks is ~1800 AST levels — past MAX_AST_DEPTH,
+    // but still shallow enough that acorn itself parses it, so this
+    // exercises the walk's own bound rather than the parser's.
+    const r = wrapEntrySource(nested(900), { entryFnName: 'activate' });
+
+    expect(r.ok).toBe(false);
+    if (!r.ok) {
+      expect(r.errors).toHaveLength(1);
+      expect(r.errors[0]!.code).toBe('invalid_value');
+      expect(r.errors[0]!.message).toBe('Entry script is nested more than 1000 AST levels deep.');
+    }
+  });
+
+  it('still wraps a script nested well inside the bound', () => {
+    // 400 nested `if` blocks is ~800 AST levels — under MAX_AST_DEPTH.
+    const r = wrapEntrySource(nested(400), { entryFnName: 'activate' });
+    expect(r.ok).toBe(true);
+  });
+
+  it('still flags a banned construct buried under deep-but-legal nesting', () => {
+    const levels = 200;
+    const r = wrapEntrySource(
+      `${'if (1) {'.repeat(levels)}import('node:fs');${'}'.repeat(levels)}function activate(ctx) {}`,
+      { entryFnName: 'activate' },
+    );
+    expect(r.ok).toBe(false);
+    if (!r.ok) {
+      expect(r.errors.some((e) => e.message.includes('Dynamic `import(...)`'))).toBe(true);
+    }
+  });
+});

packages/extensions/src/host/source-wrap.ts

@@ -139,45 +139,140 @@ if (typeof ${entryFn} === 'function') {
 })()`;
 }
 
-interface MaybeNode {
-  type: string;
-  body?: MaybeNode[];
-  source?: { value: unknown };
-}
+/**
+ * Maximum AST nesting depth the banned-construct walk will inspect.
+ *
+ * Real entry scripts nest a few tens of levels deep; this bound is
+ * two orders of magnitude above that. It exists because the AST comes
+ * from extension-author-controlled source: past this depth the walk
+ * stops and reports a validation error instead of continuing. acorn's
+ * own parser gives up at roughly twice this depth in the same process
+ * ("Not enough stack space to parse input"), but that limit moves with
+ * however much stack the host happens to have left; this one does not.
+ *
+ * A script nested deeper than this is rejected with an
+ * `invalid_value` error naming the limit — never a thrown RangeError.
+ */
+const MAX_AST_DEPTH = 1000;
 
 /**
- * Walk the top-level program body looking for constructs we do not
- * support in v1. Returns one ValidationError per offending node.
+ * Walk the *entire* AST — including nested function bodies, arrow
+ * bodies, class methods, and blocks — looking for constructs we do
+ * not support in v1. Returns one ValidationError per offending node.
+ *
+ * Static `import`/`export` declarations are only legal at the top
+ * level of a module per the ECMAScript grammar, so acorn can never
+ * produce them elsewhere; they're included here via the same walk
+ * for a single code path rather than because nesting is possible.
+ * Dynamic `import(...)`, in contrast, is an expression and CAN appear
+ * anywhere an expression can — nested inside a function body, an
+ * arrow, a class method, etc. — which is exactly the gap this walk
+ * closes: the previous top-level-only scan missed it entirely.
+ *
+ * The traversal keeps its own stack on the heap instead of recursing
+ * the way `acorn-walk` does. `wrapEntrySource` is declared to return a
+ * ValidationResult, and a recursive walk over a deeply nested script
+ * escapes that contract by throwing a RangeError out of the middle of
+ * it. Deeply nested input has to come back as a *reported* error, the
+ * same way acorn's own depth failure already does.
  */
 function checkBannedConstructs(ast: acorn.Node): ValidationError[] {
   const errors: ValidationError[] = [];
-  const body = (ast as MaybeNode).body ?? [];
-  for (const node of body) {
-    if (!node || typeof node !== 'object') continue;
+  const stack: Array<{ node: AstNode; depth: number }> = [
+    { node: ast as unknown as AstNode, depth: 0 },
+  ];
+
+  while (stack.length > 0) {
+    const { node, depth } = stack.pop()!;
+
+    if (depth > MAX_AST_DEPTH) {
+      errors.push({
+        path: '',
+        code: 'invalid_value',
+        message: `Entry script is nested more than ${MAX_AST_DEPTH} AST levels deep.`,
+        hint: 'Flatten the script — extract deeply nested blocks into separate helper functions.',
+      });
+      return errors;
+    }
+
     switch (node.type) {
       case 'ImportDeclaration':
         errors.push({
           path: '',
           code: 'invalid_value',
-          message: 'Top-level `import` statements are not supported in extension entry scripts.',
+          message: '`import` statements are not supported in extension entry scripts.',
           hint: 'Inline any helpers, or move them into a separate file referenced via entry.commands / entry.triggers.',
         });
         break;
       case 'ExportNamedDeclaration':
       case 'ExportDefaultDeclaration':
       case 'ExportAllDeclaration':
+        errors.push(exportError());
+        break;
+      case 'ImportExpression':
         errors.push({
           path: '',
           code: 'invalid_value',
-          message: 'Top-level `export` statements are not supported in extension entry scripts.',
-          hint: 'Define the entry function as a top-level declaration (e.g. `async function activate(ctx) {…}`) without `export`.',
+          message: 'Dynamic `import(...)` is not supported in extension entry scripts.',
+          hint: 'Inline any helpers, or move them into a separate file referenced via entry.commands / entry.triggers.',
         });
         break;
     }
+
+    // Push children in reverse so the LIFO stack visits them in source
+    // order, matching the order acorn-walk would have reported in.
+    const children = childNodes(node);
+    for (let i = children.length - 1; i >= 0; i--) {
+      stack.push({ node: children[i]!, depth: depth + 1 });
+    }
   }
+
   return errors;
 }
 
+interface AstNode {
+  type: string;
+  [key: string]: unknown;
+}
+
+function isAstNode(value: unknown): value is AstNode {
+  return (
+    typeof value === 'object' &&
+    value !== null &&
+    typeof (value as { type?: unknown }).type === 'string'
+  );
+}
+
+/**
+ * Every child node of `node`, in property order. Generic over the AST
+ * shape on purpose: an unknown property is either a primitive, a node,
+ * or an array of nodes, and anything else (`loc`, `regex`, …) carries
+ * no `type` string and is skipped.
+ */
+function childNodes(node: AstNode): AstNode[] {
+  const out: AstNode[] = [];
+  for (const key of Object.keys(node)) {
+    const value = node[key];
+    if (Array.isArray(value)) {
+      for (const item of value) {
+        if (isAstNode(item)) out.push(item);
+      }
+    } else if (isAstNode(value)) {
+      out.push(value);
+    }
+  }
+  return out;
+}
+
+function exportError(): ValidationError {
+  return {
+    path: '',
+    code: 'invalid_value',
+    message: '`export` statements are not supported in extension entry scripts.',
+    hint: 'Define the entry function as a top-level declaration (e.g. `async function activate(ctx) {…}`) without `export`.',
+  };
+}
+
 function fail(
   path: string,
   code: import('../types.js').ValidationErrorCode,