workflow: total, contained rendering of hostile thrown script values
Codex code-review round 2: errorText() read .stack/.message as plain property gets and fell back to String(error) — a script throwing a value with a throwing accessor (or toString/Symbol.toPrimitive) ran realm code in drive()'s catch and made WorkflowRun.result REJECT, which the detached workflow/end hook turned into an unhandledRejection (process death under dsh-app-boot). Replaced with describeThrown in dsh-workflow-vm/realm: total (never throws), proxy-labelling before any inspection, own-descriptor reads, String() only on primitives, and a CONTAINED stack-getter invocation — modern V8 (Node >= 22) makes stack an own ACCESSOR on genuine Errors, so refusing all accessors would lose every real stack and the lineOffset line numbers; a hostile getter's throw is swallowed and rendering falls back to message. The meta-literal eval catch had the same String(error) exposure and now uses the same renderer. Regression tests: a hostile-thrown-values table through the real engine (throwing stack/message getters, data stack, setter-only stack, proxy, Symbol.toPrimitive, function, null) asserting result resolves 'error' with the expected rendering and NO unhandledRejection fires; a meta-path hostile throw mapping to META_INVALID.
This commit is contained in:
@@ -30,7 +30,7 @@ One deliberate strictness DIVERGENCE from CC: hook misuse — unknown or deferre
|
|||||||
|
|
||||||
**Realm boundary**: values entering the host (meta, hook options, schemas, the return value) go through `materializeFromRealm` — a descriptor walk that NEVER invokes accessors (the repo's `isJsonValue` is prototype-strict and getter-invoking, so it cannot run first; it would reject every cross-realm object and let realm code run outside the timed window) and rejects loud everything JSON cannot carry, proxies included (the trap-free `util.types.isProxy`, checked before any inspection, so realm-side traps never run on the host stack), copying via `Object.defineProperty` so a `"__proto__"` key becomes a data property, never a prototype mutation. Values entering the realm (`args`, `agent()` results) are rebuilt INSIDE the realm via the context's own `JSON.parse`, and `parallel`/`pipeline` resolve to realm-built arrays, so the script never holds a live host-prototype object. Realm functions (stages, thunks) are called, never materialized.
|
**Realm boundary**: values entering the host (meta, hook options, schemas, the return value) go through `materializeFromRealm` — a descriptor walk that NEVER invokes accessors (the repo's `isJsonValue` is prototype-strict and getter-invoking, so it cannot run first; it would reject every cross-realm object and let realm code run outside the timed window) and rejects loud everything JSON cannot carry, proxies included (the trap-free `util.types.isProxy`, checked before any inspection, so realm-side traps never run on the host stack), copying via `Object.defineProperty` so a `"__proto__"` key becomes a data property, never a prototype mutation. Values entering the realm (`args`, `agent()` results) are rebuilt INSIDE the realm via the context's own `JSON.parse`, and `parallel`/`pipeline` resolve to realm-built arrays, so the script never holds a live host-prototype object. Realm functions (stages, thunks) are called, never materialized.
|
||||||
|
|
||||||
**Containment**: every hook-returned promise carries a no-op rejection consumer, so a script that drops a promise cannot surface an unhandled rejection when cancellation rejects it — `dsh-app-boot` exits the process on unhandled rejections. Caps (`maxConcurrentAgents` auto = `min(16, cores - 2)`, `maxTotalAgents` 1000, `maxItemsPerCall` 4096) and timeouts are validated Config, not literals.
|
**Containment**: every hook-returned promise carries a no-op rejection consumer, so a script that drops a promise cannot surface an unhandled rejection when cancellation rejects it — `dsh-app-boot` exits the process on unhandled rejections. Thrown script values (and meta-evaluation throws) are rendered by the total `describeThrown` — fixed labels for proxies/functions, own-descriptor reads, `String()` only on primitives, and a CONTAINED stack-getter call (modern V8 makes `stack` an own accessor on real Errors; a hostile getter's throw is swallowed) — so a hostile thrown value can neither escape the catch path raw nor make `result` reject. Caps (`maxConcurrentAgents` auto = `min(16, cores - 2)`, `maxTotalAgents` 1000, `maxItemsPerCall` 4096) and timeouts are validated Config, not literals.
|
||||||
|
|
||||||
### The consumer (dsh-tool-workflow)
|
### The consumer (dsh-tool-workflow)
|
||||||
|
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ Values ENTERING the host (the meta literal, hook options/schemas, the script's r
|
|||||||
|
|
||||||
## Limits, cancellation, disposal
|
## Limits, cancellation, disposal
|
||||||
|
|
||||||
Per-run: a concurrency semaphore (`maxConcurrentAgents`), a total-`agent()` cap (`maxTotalAgents`), and a per-call item cap (`maxItemsPerCall`), all config. `cancel()` aborts every child (a shared `AbortSignal`), rejects waiting `agent()` slots, and makes every future hook call throw `CANCELLED` — the script dies at its next await and the run settles `cancelled`; a cancellation that lands before the body runs (or before it settles) reports `cancelled` even if the script itself needed no hooks. Once a run settles, stray children a script fired without awaiting are aborted too, and `dispose()` waits for those children to finish disposing (bounded by the grace) before returning. Every hook-returned promise carries a no-op rejection consumer, so a dropped promise cannot surface an unhandled rejection (the app boot layer exits the process on those).
|
Per-run: a concurrency semaphore (`maxConcurrentAgents`), a total-`agent()` cap (`maxTotalAgents`), and a per-call item cap (`maxItemsPerCall`), all config. `cancel()` aborts every child (a shared `AbortSignal`), rejects waiting `agent()` slots, and makes every future hook call throw `CANCELLED` — the script dies at its next await and the run settles `cancelled`; a cancellation that lands before the body runs (or before it settles) reports `cancelled` even if the script itself needed no hooks. Once a run settles, stray children a script fired without awaiting are aborted too, and `dispose()` waits for those children to finish disposing (bounded by the grace) before returning. Every hook-returned promise carries a no-op rejection consumer, so a dropped promise cannot surface an unhandled rejection (the app boot layer exits the process on those), and thrown script values are rendered by the total `describeThrown` (proxy-labelling, own-descriptor reads, a contained stack-getter call), so a hostile throw (`{ get stack() { throw ... } }`) cannot make `result` reject.
|
||||||
|
|
||||||
**Documented limitations** (the accepted cost of the in-process mechanism; the seam exists so a worker-thread/isolated-vm engine can swap in): vm is NOT a security boundary — scripts are model-written, the same trust level as the model's bash access — and the vm `timeout` covers only the initial synchronous slice, so a pathological synchronous spin after the first await cannot be killed; `dispose()` waits `disposeGraceMs` then ABANDONS such a script (its settlement stays contained, but an abandoned spin would still occupy the event loop).
|
**Documented limitations** (the accepted cost of the in-process mechanism; the seam exists so a worker-thread/isolated-vm engine can swap in): vm is NOT a security boundary — scripts are model-written, the same trust level as the model's bash access — and the vm `timeout` covers only the initial synchronous slice, so a pathological synchronous spin after the first await cannot be killed; `dispose()` waits `disposeGraceMs` then ABANDONS such a script (its settlement stays contained, but an abandoned spin would still occupy the event loop).
|
||||||
|
|
||||||
|
|||||||
@@ -20,7 +20,7 @@
|
|||||||
import * as vm from 'node:vm'
|
import * as vm from 'node:vm'
|
||||||
import { WorkflowError } from '@deepseek-ai/dsh-workflow'
|
import { WorkflowError } from '@deepseek-ai/dsh-workflow'
|
||||||
import type { WorkflowMeta, WorkflowPhase } from '@deepseek-ai/dsh-workflow'
|
import type { WorkflowMeta, WorkflowPhase } from '@deepseek-ai/dsh-workflow'
|
||||||
import { materializeFromRealm, MaterializeError } from './realm.ts'
|
import { materializeFromRealm, MaterializeError, describeThrown } from './realm.ts'
|
||||||
|
|
||||||
/** The result of {@link extractMeta}: the validated meta and the runnable body. */
|
/** The result of {@link extractMeta}: the validated meta and the runnable body. */
|
||||||
export interface ExtractedScript {
|
export interface ExtractedScript {
|
||||||
@@ -174,7 +174,10 @@ export function extractMeta(script: string, evalTimeoutMs: number): ExtractedScr
|
|||||||
// below are part of the same boundary.
|
// below are part of the same boundary.
|
||||||
evaluated = vm.runInNewContext(`(${literal})`, undefined, { timeout: evalTimeoutMs })
|
evaluated = vm.runInNewContext(`(${literal})`, undefined, { timeout: evalTimeoutMs })
|
||||||
} catch (error: unknown) {
|
} catch (error: unknown) {
|
||||||
throw new WorkflowError(`meta block failed to evaluate as a pure literal: ${String(error)}`, 'META_INVALID', { cause: error })
|
// describeThrown, not String(): an expression in the literal can THROW a
|
||||||
|
// hostile value (a throwing toString/accessor), and this catch must map
|
||||||
|
// it to META_INVALID rather than let realm code run or a raw error escape.
|
||||||
|
throw new WorkflowError(`meta block failed to evaluate as a pure literal: ${describeThrown(error)}`, 'META_INVALID', { cause: error })
|
||||||
}
|
}
|
||||||
let data: unknown
|
let data: unknown
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -27,6 +27,11 @@
|
|||||||
* chain, so the engine rebuilds inbound values INSIDE the realm via the
|
* chain, so the engine rebuilds inbound values INSIDE the realm via the
|
||||||
* context's own `JSON.parse` (see the runtime).
|
* context's own `JSON.parse` (see the runtime).
|
||||||
*
|
*
|
||||||
|
* {@link describeThrown} is the same discipline for the one place realm
|
||||||
|
* values reach the host WITHOUT materialization: rendering a thrown value
|
||||||
|
* for a failure report. It never throws; the only realm code it can invoke
|
||||||
|
* is a stack getter, contained (see its doc).
|
||||||
|
*
|
||||||
* @module @deepseek-ai/dsh-workflow-vm/realm
|
* @module @deepseek-ai/dsh-workflow-vm/realm
|
||||||
*/
|
*/
|
||||||
|
|
||||||
@@ -40,6 +45,73 @@ export class MaterializeError extends Error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Render a value THROWN by realm code (a script failure, a meta-literal
|
||||||
|
* evaluation failure) as text, without ever throwing itself — the callers sit
|
||||||
|
* in catch blocks whose totality is a seam contract (`WorkflowRun.result`
|
||||||
|
* never rejects). Plain property reads and `String(value)` are hostile-value
|
||||||
|
* hazards (`{ get stack() { throw ... } }`, a throwing
|
||||||
|
* `toString`/`Symbol.toPrimitive`), so: proxies render as a fixed label
|
||||||
|
* (trap-free `isProxy`, before any inspection); `message` is read as an OWN
|
||||||
|
* DATA descriptor only; everything else object-shaped renders as
|
||||||
|
* `[object Object]` without being touched; only primitives (which cannot
|
||||||
|
* carry code) reach `String()`. The one exception is the `stack` getter —
|
||||||
|
* modern V8 makes `stack` an own ACCESSOR on genuine `Error`s, so it is
|
||||||
|
* invoked (that is how real stacks, with the script's own line numbers via
|
||||||
|
* the compile lineOffset, are obtained) but CONTAINED: a hostile getter's
|
||||||
|
* throw is swallowed and rendering falls back to message. Detection is
|
||||||
|
* structural, not `instanceof` — a realm Error is not an instance of the host
|
||||||
|
* class.
|
||||||
|
* @param error - the thrown value, of any shape and any realm.
|
||||||
|
* @returns human-readable text for the failure report; prefers the stack.
|
||||||
|
*/
|
||||||
|
export function describeThrown(error: unknown): string {
|
||||||
|
switch (typeof error) {
|
||||||
|
case 'object':
|
||||||
|
break
|
||||||
|
case 'function':
|
||||||
|
return '[thrown function]'
|
||||||
|
default:
|
||||||
|
// Primitives (string/number/boolean/bigint/symbol/undefined): String()
|
||||||
|
// cannot reach user code on these.
|
||||||
|
return String(error)
|
||||||
|
}
|
||||||
|
if (error === null) return 'null'
|
||||||
|
if (types.isProxy(error)) return '[thrown proxy]'
|
||||||
|
const stack = readStack(error)
|
||||||
|
if (typeof stack === 'string' && stack.length > 0) return stack
|
||||||
|
const message = ownDataProperty(error, 'message')
|
||||||
|
if (typeof message === 'string') return message
|
||||||
|
return '[object Object]'
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Read `error.stack`, tolerating both descriptor shapes: an own DATA property
|
||||||
|
* (older V8, plain objects) and the modern own ACCESSOR pair (the Error Stack
|
||||||
|
* Accessor proposal). Invoking the getter is the only way to obtain a real
|
||||||
|
* stack; on a hostile object that getter is user code, so the call is
|
||||||
|
* contained — a throw yields `undefined` (the caller falls back to message),
|
||||||
|
* and a synchronous spin is the engine's already-accepted post-await
|
||||||
|
* limitation (a script can spin directly just the same).
|
||||||
|
*/
|
||||||
|
function readStack(error: object): unknown {
|
||||||
|
const descriptor = Object.getOwnPropertyDescriptor(error, 'stack')
|
||||||
|
if (descriptor === undefined) return undefined
|
||||||
|
if ('value' in descriptor) return descriptor.value
|
||||||
|
if (typeof descriptor.get !== 'function') return undefined
|
||||||
|
try {
|
||||||
|
return descriptor.get.call(error)
|
||||||
|
} catch {
|
||||||
|
return undefined // a hostile stack getter threw; message/fallback renders instead
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** An own DATA property's value (`undefined` for absent or accessor); never invokes user code on a non-proxy object. */
|
||||||
|
function ownDataProperty(value: object, key: string): unknown {
|
||||||
|
const descriptor = Object.getOwnPropertyDescriptor(value, key)
|
||||||
|
return descriptor !== undefined && 'value' in descriptor ? descriptor.value : undefined
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Whether an object's prototype chain is data-shaped: `null`, or a prototype
|
* Whether an object's prototype chain is data-shaped: `null`, or a prototype
|
||||||
* whose own prototype is `null` (the realm's `Object.prototype` — which we
|
* whose own prototype is `null` (the realm's `Object.prototype` — which we
|
||||||
|
|||||||
@@ -41,7 +41,7 @@ import type {
|
|||||||
WorkflowMeta,
|
WorkflowMeta,
|
||||||
WorkflowResult,
|
WorkflowResult,
|
||||||
} from '@deepseek-ai/dsh-workflow'
|
} from '@deepseek-ai/dsh-workflow'
|
||||||
import { materializeFromRealm, MaterializeError } from './realm.ts'
|
import { materializeFromRealm, MaterializeError, describeThrown } from './realm.ts'
|
||||||
|
|
||||||
/** The per-run knobs the engine resolves from its Config. */
|
/** The per-run knobs the engine resolves from its Config. */
|
||||||
export interface ExecutionLimits {
|
export interface ExecutionLimits {
|
||||||
@@ -97,21 +97,6 @@ function outputText(blocks: ContentBlock[]): string {
|
|||||||
.join('')
|
.join('')
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Render a script failure for the result: prefer the stack (it carries the
|
|
||||||
* script's own line numbers via the compile lineOffset), then the message.
|
|
||||||
* STRUCTURAL detection, not `instanceof Error` — a realm-thrown Error is not
|
|
||||||
* an instance of the host Error class.
|
|
||||||
*/
|
|
||||||
function errorText(error: unknown): string {
|
|
||||||
if (typeof error === 'object' && error !== null) {
|
|
||||||
const maybe = error as { stack?: unknown; message?: unknown }
|
|
||||||
if (typeof maybe.stack === 'string' && maybe.stack.length > 0) return maybe.stack
|
|
||||||
if (typeof maybe.message === 'string') return maybe.message
|
|
||||||
}
|
|
||||||
return String(error)
|
|
||||||
}
|
|
||||||
|
|
||||||
/** A short display label derived from the prompt when the script passes none. */
|
/** A short display label derived from the prompt when the script passes none. */
|
||||||
function defaultLabel(prompt: string): string {
|
function defaultLabel(prompt: string): string {
|
||||||
const newline = prompt.indexOf('\n')
|
const newline = prompt.indexOf('\n')
|
||||||
@@ -240,7 +225,10 @@ export class WorkflowExecution {
|
|||||||
if (error instanceof WorkflowError && error.code === 'CANCELLED') {
|
if (error instanceof WorkflowError && error.code === 'CANCELLED') {
|
||||||
return { value: null, stopReason: 'cancelled', error: error.message, agentsStarted: this.started }
|
return { value: null, stopReason: 'cancelled', error: error.message, agentsStarted: this.started }
|
||||||
}
|
}
|
||||||
return { value: null, stopReason: 'error', error: errorText(error), agentsStarted: this.started }
|
// describeThrown is total and trap-free: a hostile thrown value (a
|
||||||
|
// throwing accessor, a proxy) cannot make this catch throw — drive()
|
||||||
|
// resolving is the `result` never-rejects seam contract.
|
||||||
|
return { value: null, stopReason: 'error', error: describeThrown(error), agentsStarted: this.started }
|
||||||
} finally {
|
} finally {
|
||||||
// Reap strays: a script that fired agent() calls without awaiting them
|
// Reap strays: a script that fired agent() calls without awaiting them
|
||||||
// leaves live children behind after settlement — abort them all. (The
|
// leaves live children behind after settlement — abort them all. (The
|
||||||
|
|||||||
@@ -116,6 +116,13 @@ return 2`
|
|||||||
expect(error.message).toContain('proxies cannot cross')
|
expect(error.message).toContain('proxies cannot cross')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('a meta expression THROWING a hostile value maps to META_INVALID — rendering runs no realm code', () => {
|
||||||
|
const error = bad('export const meta = { name: (() => { throw { get stack() { throw new Error("boom") }, toString() { throw new Error("boom") } } })(), description: "d" }\nreturn 1')
|
||||||
|
expect(error.code).toBe('META_INVALID')
|
||||||
|
expect(error.message).toContain('pure literal')
|
||||||
|
expect(error.message).toContain('[object Object]')
|
||||||
|
})
|
||||||
|
|
||||||
it('rejects shape violations with EVERY violation listed (META_INVALID)', () => {
|
it('rejects shape violations with EVERY violation listed (META_INVALID)', () => {
|
||||||
const error = bad('export const meta = { description: 7, bogus: 1 }\nreturn 1')
|
const error = bad('export const meta = { description: 7, bogus: 1 }\nreturn 1')
|
||||||
expect(error.code).toBe('META_INVALID')
|
expect(error.code).toBe('META_INVALID')
|
||||||
|
|||||||
@@ -589,6 +589,39 @@ describe('dsh-workflow-vm', () => {
|
|||||||
expect(result.error).toBe('[object Object]')
|
expect(result.error).toBe('[object Object]')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('hostile thrown values render contained: result NEVER rejects, no unhandled rejection', async () => {
|
||||||
|
const unhandled: unknown[] = []
|
||||||
|
const onUnhandled = (reason: unknown): void => { unhandled.push(reason) }
|
||||||
|
process.on('unhandledRejection', onUnhandled)
|
||||||
|
try {
|
||||||
|
const { ctx, parent } = await setup()
|
||||||
|
// Each thrown value would run realm code (or throw) under a plain
|
||||||
|
// property read or String(); rendering must stay total — the only
|
||||||
|
// permitted realm call is the CONTAINED stack getter.
|
||||||
|
const cases: [string, string][] = [
|
||||||
|
["throw { get stack() { throw new Error('stack getter threw') } }", '[object Object]'],
|
||||||
|
["throw { get stack() { throw new Error('x') }, message: 'getter threw, message renders' }", 'getter threw, message renders'],
|
||||||
|
["throw { get message() { throw new Error('message getter ran') } }", '[object Object]'],
|
||||||
|
["throw { stack: 'custom data stack' }", 'custom data stack'],
|
||||||
|
["throw (() => { const o = { message: 'setter-only stack' }; Object.defineProperty(o, 'stack', { set() {} }); return o })()", 'setter-only stack'],
|
||||||
|
["throw new Proxy({}, { getOwnPropertyDescriptor() { throw new Error('trap ran') } })", '[thrown proxy]'],
|
||||||
|
["throw { [Symbol.toPrimitive]() { throw new Error('toPrimitive ran') } }", '[object Object]'],
|
||||||
|
['throw () => 1', '[thrown function]'],
|
||||||
|
['throw null', 'null'],
|
||||||
|
]
|
||||||
|
for (const [body, rendered] of cases) {
|
||||||
|
const result = await run(ctx, parent, script(body))
|
||||||
|
expect(result.stopReason).toBe('error')
|
||||||
|
expect(result.error).toBe(rendered)
|
||||||
|
}
|
||||||
|
// Let any stray rejection reach the process hook before asserting.
|
||||||
|
await new Promise(resolve => setTimeout(resolve, 20))
|
||||||
|
expect(unhandled).toEqual([])
|
||||||
|
} finally {
|
||||||
|
process.off('unhandledRejection', onUnhandled)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
it('falls back to the message for an Error whose stack was stripped', async () => {
|
it('falls back to the message for an Error whose stack was stripped', async () => {
|
||||||
const { ctx, parent } = await setup()
|
const { ctx, parent } = await setup()
|
||||||
const result = await run(ctx, parent, script(`
|
const result = await run(ctx, parent, script(`
|
||||||
|
|||||||
Reference in New Issue
Block a user