Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/one-ast-depth-bound-for-author-source.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@ifc-lite/extensions": patch
---

Give the package's author-source AST walks one depth bound instead of two copies of the same number.

`host/source-wrap.ts`'s `checkBannedConstructs` (#3025) and `ast/bounded-walk.ts` (#3027) landed the same day, each declaring its own private `MAX_AST_DEPTH = 1000`. Both bound traversals over extension-author-supplied source, and both refuse a script that exceeds the bound — so moving one alone would have opened a band where `wrapEntrySource` accepts a script that `validateCode` refuses, or the reverse, with nothing to catch it. `source-wrap.ts` now imports the constant from `ast/bounded-walk.ts`. No behaviour change: the value is the same 1000 it already was.

`bounded-walk.ts`'s module doc claimed to be "the single traversal used by every AST consumer here" and that callers "do not re-implement the traversal", which `checkBannedConstructs` had never been true of — it walks child properties generically, a superset of the positions `acorn-walk`'s `base` descends through. The doc now says which walks go through the module and which does not.
14 changes: 13 additions & 1 deletion packages/extensions/src/ast/bounded-walk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,23 @@
* `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.
*
* Two of the package's three author-source walks go through here:
* `validate/code.ts` and `inference/capability.ts`. The third,
* `host/source-wrap.ts`'s `checkBannedConstructs`, keeps its own
* heap-stack traversal — it enumerates child properties generically
* instead of descending through `acorn-walk`'s `base`, so it visits a
* superset of these positions. It does not keep its own limit: it
* imports {@link MAX_AST_DEPTH} from here, so raising or lowering the
* bound moves both walks rather than opening a band where one gate on
* author-supplied source accepts what the next refuses. (The two count
* a level slightly differently — generic property nesting versus
* `base`'s child dispatch — so they are not guaranteed to cut the same
* script at the same node, only to share the same budget.)
*
* 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:
Expand Down
21 changes: 11 additions & 10 deletions packages/extensions/src/host/source-wrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

import * as acorn from 'acorn';
import type { ValidationError, ValidationResult } from '../types.js';
import { MAX_AST_DEPTH as SHARED_MAX_AST_DEPTH } from '../ast/bounded-walk.js';

export interface SourceWrapOptions {
/** Name of the entry function to invoke (e.g. "activate"). */
Expand Down Expand Up @@ -142,18 +143,18 @@ if (typeof ${entryFn} === 'function') {
/**
* 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.
* Shared with `ast/bounded-walk.ts`, which bounds the package's other
* two author-source walks (`validateCode`, `inferCapabilities`). It was a
* private `1000` here against that module's `1000`: the same number twice,
* with nothing keeping them equal. A script that `wrapEntrySource` accepts
* and `validateCode` refuses — or the reverse — is a bundle that passes one
* gate on author-supplied source and fails the next, and moving either
* constant alone would have produced exactly that band silently.
*
* A script nested deeper than this is rejected with an
* `invalid_value` error naming the limit — never a thrown RangeError.
* 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;
const MAX_AST_DEPTH = SHARED_MAX_AST_DEPTH;

/**
* Walk the *entire* AST — including nested function bodies, arrow
Expand Down
Loading