fix(sdk,core,build): SDK hardening pass (#3670)
## Summary Five hardening fixes across `@trigger.dev/sdk`, `@trigger.dev/core`, and `@trigger.dev/build`. - `tasks.triggerAndSubscribe` now forwards caller `requestOptions` (custom API keys, per-request overrides) to the underlying `apiClient.triggerTask` call instead of silently dropping them. - `SSEStreamSubscription` no longer retries permanent client errors forever. The default `nonRetryableStatuses` widens from `[404, 410]` to `[400, 404, 409, 410, 422]`, so a malformed session-stream request fails fast instead of busy-looping under bounded backoff. - Session writer falls back to manually wiring the caller's `AbortSignal` on Node 18, where `AbortSignal.any` is unavailable. Caller-driven cancellation now propagates on every supported runtime. - `TriggerChatTransport` throws immediately when a `chat.handover` response is missing `X-Trigger-Chat-Access-Token`, instead of silently downgrading every subsequent turn back to the handover path. `dispose()` aborts every active `session.out` subscription before tearing the coordinator down, so unmount/navigation no longer leaves SSE readers in flight. - Removed the experimental `@trigger.dev/build/extensions/secureExec` build extension. It will return alongside the sandbox feature it was built to support. ## Test plan - [ ] `pnpm run build --filter @trigger.dev/sdk --filter @trigger.dev/core --filter @trigger.dev/build` - [ ] `pnpm --filter @trigger.dev/sdk test --run` (183 tests, including chat / chat-server / sessions / handover) - [ ] `pnpm --filter @trigger.dev/core test --run` - [ ] Manually trigger a `chat.handover` whose response strips `X-Trigger-Chat-Access-Token`, and confirm the transport throws synchronously rather than degrading. - [ ] Unmount a chat UI mid-stream and confirm the active `session.out` SSE connection closes immediately.
This commit is contained in:
@@ -31,8 +31,7 @@
|
||||
"./extensions/typescript": "./src/extensions/typescript.ts",
|
||||
"./extensions/puppeteer": "./src/extensions/puppeteer.ts",
|
||||
"./extensions/playwright": "./src/extensions/playwright.ts",
|
||||
"./extensions/lightpanda": "./src/extensions/lightpanda.ts",
|
||||
"./extensions/secureExec": "./src/extensions/secureExec.ts"
|
||||
"./extensions/lightpanda": "./src/extensions/lightpanda.ts"
|
||||
},
|
||||
"sourceDialects": [
|
||||
"@triggerdotdev/source"
|
||||
@@ -66,9 +65,6 @@
|
||||
],
|
||||
"extensions/lightpanda": [
|
||||
"dist/commonjs/extensions/lightpanda.d.ts"
|
||||
],
|
||||
"extensions/secureExec": [
|
||||
"dist/commonjs/extensions/secureExec.d.ts"
|
||||
]
|
||||
}
|
||||
},
|
||||
@@ -211,17 +207,6 @@
|
||||
"types": "./dist/commonjs/extensions/lightpanda.d.ts",
|
||||
"default": "./dist/commonjs/extensions/lightpanda.js"
|
||||
}
|
||||
},
|
||||
"./extensions/secureExec": {
|
||||
"import": {
|
||||
"@triggerdotdev/source": "./src/extensions/secureExec.ts",
|
||||
"types": "./dist/esm/extensions/secureExec.d.ts",
|
||||
"default": "./dist/esm/extensions/secureExec.js"
|
||||
},
|
||||
"require": {
|
||||
"types": "./dist/commonjs/extensions/secureExec.d.ts",
|
||||
"default": "./dist/commonjs/extensions/secureExec.js"
|
||||
}
|
||||
}
|
||||
},
|
||||
"main": "./dist/commonjs/index.js",
|
||||
|
||||
@@ -1,172 +0,0 @@
|
||||
import { BuildTarget } from "@trigger.dev/core/v3";
|
||||
import { BuildManifest } from "@trigger.dev/core/v3/schemas";
|
||||
import { BuildContext, BuildExtension } from "@trigger.dev/core/v3/build";
|
||||
import { dirname, resolve, join } from "node:path";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import { readPackageJSON } from "pkg-types";
|
||||
|
||||
export type SecureExecOptions = {
|
||||
/**
|
||||
* Packages available inside the sandbox at runtime.
|
||||
*
|
||||
* These are `require()`'d inside the V8 isolate at runtime — the bundler
|
||||
* never sees them statically. They are marked external and installed as
|
||||
* deploy dependencies.
|
||||
*
|
||||
* @example
|
||||
* ```ts
|
||||
* secureExec({ packages: ["jszip", "lodash"] })
|
||||
* ```
|
||||
*/
|
||||
packages?: string[];
|
||||
};
|
||||
|
||||
/**
|
||||
* Build extension for [secure-exec](https://secureexec.dev) — run untrusted
|
||||
* JavaScript/TypeScript in V8 isolates with configurable permissions.
|
||||
*
|
||||
* Handles the esbuild workarounds needed for secure-exec's runtime
|
||||
* `require.resolve` calls, native binaries, and module-scope resolution.
|
||||
*
|
||||
* @example
|
||||
* ```ts
|
||||
* import { secureExec } from "@trigger.dev/build/extensions/secureExec";
|
||||
*
|
||||
* export default defineConfig({
|
||||
* build: {
|
||||
* extensions: [secureExec()],
|
||||
* },
|
||||
* });
|
||||
* ```
|
||||
*/
|
||||
export function secureExec(options?: SecureExecOptions): BuildExtension {
|
||||
return new SecureExecExtension(options ?? {});
|
||||
}
|
||||
|
||||
class SecureExecExtension implements BuildExtension {
|
||||
public readonly name = "SecureExecExtension";
|
||||
|
||||
private userPackages: string[];
|
||||
|
||||
constructor(options: SecureExecOptions) {
|
||||
this.userPackages = options.packages ?? [];
|
||||
}
|
||||
|
||||
externalsForTarget(_target: BuildTarget) {
|
||||
return [
|
||||
// esbuild must not be bundled — it locates its native binary via a
|
||||
// relative path from its JS API entry point. secure-exec uses esbuild
|
||||
// at runtime to bundle polyfills for sandbox code.
|
||||
"esbuild",
|
||||
// User-specified packages are require()'d inside the V8 sandbox at
|
||||
// runtime — the bundler never sees them statically.
|
||||
...this.userPackages,
|
||||
];
|
||||
}
|
||||
|
||||
onBuildStart(context: BuildContext) {
|
||||
context.logger.debug(`Adding ${this.name} esbuild plugins`);
|
||||
|
||||
// Plugin 1: Replace node-stdlib-browser with pre-resolved paths.
|
||||
//
|
||||
// Trigger's ESM shim anchors require.resolve() to the chunk path, so
|
||||
// node-stdlib-browser's runtime require.resolve("./mock/empty.js") breaks.
|
||||
// Fix: load the real node-stdlib-browser at build time (where require.resolve
|
||||
// works), capture the resolved path map, and inline it as a static export.
|
||||
const workingDir = context.workingDir;
|
||||
context.registerPlugin({
|
||||
name: "secure-exec-stdlib-resolver",
|
||||
setup(build) {
|
||||
build.onResolve({ filter: /^node-stdlib-browser$/ }, () => ({
|
||||
path: "node-stdlib-browser",
|
||||
namespace: "secure-exec-nsb-resolved",
|
||||
}));
|
||||
build.onLoad({ filter: /.*/, namespace: "secure-exec-nsb-resolved" }, () => {
|
||||
const buildRequire = createRequire(join(workingDir, "package.json"));
|
||||
const resolved = buildRequire("node-stdlib-browser");
|
||||
return {
|
||||
contents: `export default ${JSON.stringify(resolved)};`,
|
||||
loader: "js",
|
||||
};
|
||||
});
|
||||
},
|
||||
});
|
||||
|
||||
// Plugin 2: Inline bridge.js at build time.
|
||||
//
|
||||
// bridge-loader.js in @secure-exec/node(js) uses __dirname and
|
||||
// require.resolve("@secure-exec/core") at module scope to locate
|
||||
// dist/bridge.js on disk. This fails in Trigger's bundled output.
|
||||
// Fix: read bridge.js content at build time and inline it as a
|
||||
// string literal so no runtime filesystem resolution is needed.
|
||||
//
|
||||
context.registerPlugin({
|
||||
name: "secure-exec-bridge-inline",
|
||||
setup(build) {
|
||||
build.onLoad(
|
||||
{ filter: /[\\/]@secure-exec[\\/]node[\\/]dist[\\/]bridge-loader\.js$/ },
|
||||
(args) => {
|
||||
try {
|
||||
const buildRequire = createRequire(args.path);
|
||||
const coreEntry = buildRequire.resolve("@secure-exec/core");
|
||||
const coreRoot = resolve(dirname(coreEntry), "..");
|
||||
const bridgeCode = readFileSync(join(coreRoot, "dist", "bridge.js"), "utf8");
|
||||
|
||||
return {
|
||||
contents: [
|
||||
`import { getIsolateRuntimeSource } from "@secure-exec/core";`,
|
||||
`const bridgeCodeCache = ${JSON.stringify(bridgeCode)};`,
|
||||
`export function getRawBridgeCode() { return bridgeCodeCache; }`,
|
||||
`export function getBridgeAttachCode() { return getIsolateRuntimeSource("bridgeAttach"); }`,
|
||||
].join("\n"),
|
||||
loader: "js",
|
||||
};
|
||||
} catch {
|
||||
// If we can't inline the bridge, let the normal loader handle it.
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
);
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async onBuildComplete(context: BuildContext, _manifest: BuildManifest) {
|
||||
if (context.target === "dev") {
|
||||
return;
|
||||
}
|
||||
|
||||
context.logger.debug(`Adding ${this.name} deploy dependencies`);
|
||||
|
||||
const dependencies: Record<string, string> = {};
|
||||
|
||||
// Resolve versions for user-specified sandbox packages
|
||||
for (const pkg of this.userPackages) {
|
||||
try {
|
||||
const modulePath = await context.resolvePath(pkg);
|
||||
if (!modulePath) {
|
||||
dependencies[pkg] = "latest";
|
||||
continue;
|
||||
}
|
||||
|
||||
const packageJSON = await readPackageJSON(dirname(modulePath));
|
||||
dependencies[pkg] = packageJSON.version ?? "latest";
|
||||
} catch {
|
||||
context.logger.warn(
|
||||
`Could not resolve version for sandbox package ${pkg}, defaulting to latest`
|
||||
);
|
||||
dependencies[pkg] = "latest";
|
||||
}
|
||||
}
|
||||
|
||||
context.addLayer({
|
||||
id: "secureExec",
|
||||
dependencies,
|
||||
image: {
|
||||
// isolated-vm requires native compilation tools
|
||||
pkgs: ["python3", "make", "g++"],
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -233,8 +233,10 @@ export class SSEStreamSubscription implements StreamSubscription {
|
||||
// reset the timer naturally.
|
||||
stallTimeoutMs?: number;
|
||||
// HTTP statuses that should NOT be retried — fail the stream
|
||||
// permanently. `404` (stream gone) and `410` (session closed)
|
||||
// are sensible defaults; tune per-caller for other 4xx.
|
||||
// permanently. Defaults cover the permanent client-error set:
|
||||
// `400` (bad request), `404` (stream gone), `409` (conflict),
|
||||
// `410` (session closed), `422` (unprocessable). Tune per-caller
|
||||
// for other 4xx.
|
||||
nonRetryableStatuses?: readonly number[];
|
||||
// Optional fetch override. Used by transports that need to route
|
||||
// the SSE connect through a custom path (proxy, custom headers,
|
||||
@@ -249,7 +251,9 @@ export class SSEStreamSubscription implements StreamSubscription {
|
||||
this.retryJitter = options.retryJitter ?? 0.5;
|
||||
this.fetchTimeoutMs = options.fetchTimeoutMs ?? 30_000;
|
||||
this.stallTimeoutMs = options.stallTimeoutMs ?? 0;
|
||||
this.nonRetryableStatuses = new Set(options.nonRetryableStatuses ?? [404, 410]);
|
||||
this.nonRetryableStatuses = new Set(
|
||||
options.nonRetryableStatuses ?? [400, 404, 409, 410, 422]
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -74,8 +74,7 @@ type PublicTokenPermissionProperties = {
|
||||
*
|
||||
* `read:sessions:{id}` lets the bearer read both the `.out` and `.in`
|
||||
* channels and list runs on the session. `write:sessions:{id}` lets the
|
||||
* bearer append to the session's channels. `trigger:sessions:{id}` permits
|
||||
* triggering new runs on the session.
|
||||
* bearer append to the session's channels and create new runs against it.
|
||||
*/
|
||||
sessions?: string | string[];
|
||||
};
|
||||
|
||||
@@ -671,17 +671,23 @@ export class TriggerChatTransport implements ChatTransport<UIMessage> {
|
||||
}
|
||||
|
||||
// Hydrate session state from response headers so subsequent turns
|
||||
// skip the endpoint and write directly to session.in.
|
||||
// skip the endpoint and write directly to session.in. Failing fast
|
||||
// when the header is missing avoids a quiet degraded state where
|
||||
// every later turn re-runs the handover route instead of taking
|
||||
// the slim-wire path.
|
||||
const accessToken = response.headers.get("X-Trigger-Chat-Access-Token");
|
||||
const chatId = args.chatId;
|
||||
if (accessToken) {
|
||||
const state: ChatSessionState = {
|
||||
publicAccessToken: accessToken,
|
||||
isStreaming: true,
|
||||
};
|
||||
this.sessions.set(chatId, state);
|
||||
this.notifySessionChange(chatId, state);
|
||||
if (!accessToken) {
|
||||
throw new Error(
|
||||
"chat.handover response is missing the X-Trigger-Chat-Access-Token header. chat.agent's handover endpoint must echo the session PAT so the transport can hydrate."
|
||||
);
|
||||
}
|
||||
const state: ChatSessionState = {
|
||||
publicAccessToken: accessToken,
|
||||
isStreaming: true,
|
||||
};
|
||||
this.sessions.set(chatId, state);
|
||||
this.notifySessionChange(chatId, state);
|
||||
|
||||
// Filter the parsed UIMessage stream:
|
||||
// - Drop control chunks (`trigger:turn-complete`,
|
||||
@@ -953,6 +959,14 @@ export class TriggerChatTransport implements ChatTransport<UIMessage> {
|
||||
this.coordinator?.removeMessagesListener(fn);
|
||||
}
|
||||
dispose(): void {
|
||||
// Tear down any open session.out subscriptions before the coordinator
|
||||
// goes away. Otherwise controllers in `activeStreams` keep reading
|
||||
// until they time out, leaking network and memory on every
|
||||
// unmount/navigation.
|
||||
for (const controller of this.activeStreams.values()) {
|
||||
controller.abort();
|
||||
}
|
||||
this.activeStreams.clear();
|
||||
this.coordinator?.dispose();
|
||||
this.coordinator = null;
|
||||
}
|
||||
|
||||
@@ -451,9 +451,30 @@ export class SessionOutputChannel {
|
||||
const readableStreamSource = ensureReadableStream(value);
|
||||
|
||||
const abortController = new AbortController();
|
||||
const combinedSignal = options?.signal
|
||||
? AbortSignal.any?.([options.signal, abortController.signal]) ?? abortController.signal
|
||||
: abortController.signal;
|
||||
// `AbortSignal.any` lands in Node 20.3; the SDK still supports Node
|
||||
// 18.20+. On older runtimes fall back to wiring `options.signal` into
|
||||
// `abortController` manually so caller-driven cancellation propagates.
|
||||
let combinedSignal: AbortSignal = abortController.signal;
|
||||
// Set in the Node 18 fallback path so the caller's `signal.addEventListener`
|
||||
// registration can be cleared once the stream finishes. Without this, a
|
||||
// long-lived caller signal (e.g. one reused across many `writer()` calls)
|
||||
// accumulates listeners on every completed turn.
|
||||
let removeCallerAbortListener: (() => void) | undefined;
|
||||
if (options?.signal) {
|
||||
if (typeof AbortSignal.any === "function") {
|
||||
combinedSignal = AbortSignal.any([options.signal, abortController.signal]);
|
||||
} else {
|
||||
const callerSignal = options.signal;
|
||||
if (callerSignal.aborted) {
|
||||
abortController.abort(callerSignal.reason);
|
||||
} else {
|
||||
const onCallerAbort = () => abortController.abort(callerSignal.reason);
|
||||
callerSignal.addEventListener("abort", onCallerAbort, { once: true });
|
||||
removeCallerAbortListener = () =>
|
||||
callerSignal.removeEventListener("abort", onCallerAbort);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve the init promise eagerly so we can capture which one this
|
||||
// writer uses for reactive invalidation below.
|
||||
@@ -499,9 +520,11 @@ export class SessionOutputChannel {
|
||||
// from surfacing as unhandled.
|
||||
instance.wait().then(
|
||||
() => {
|
||||
removeCallerAbortListener?.();
|
||||
span.end();
|
||||
},
|
||||
() => {
|
||||
removeCallerAbortListener?.();
|
||||
if (this.#initPromise === writerInitPromise) {
|
||||
this.#initPromise = undefined;
|
||||
}
|
||||
@@ -516,6 +539,7 @@ export class SessionOutputChannel {
|
||||
},
|
||||
};
|
||||
} catch (error) {
|
||||
removeCallerAbortListener?.();
|
||||
if (error instanceof Error && error.name === "AbortError") {
|
||||
span.end();
|
||||
throw error;
|
||||
|
||||
@@ -2585,7 +2585,8 @@ async function triggerAndSubscribe_internal<TIdentifier extends string, TPayload
|
||||
debounce: options?.debounce,
|
||||
},
|
||||
},
|
||||
{}
|
||||
{},
|
||||
requestOptions
|
||||
);
|
||||
|
||||
// Set attributes after trigger so the dashboard can link to the child run
|
||||
|
||||
Reference in New Issue
Block a user