Skip to content

Commit 23f54e1

Browse files
dcramercodex
andcommitted
fix(workflow): prevent node module imports in workflow graph
Move chat/serde runtime imports into step execution so workflow modules\ndo not statically pull package graphs that can include Node-only modules.\n\nAdd a workflow boundary check and run it in test/build so violations\nfail before deploy. Co-Authored-By: Codex (GPT-5) <codex@openai.com>
1 parent a9bfea0 commit 23f54e1

3 files changed

Lines changed: 162 additions & 21 deletions

File tree

packages/junior/package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,15 @@
2121
],
2222
"scripts": {
2323
"dev": "next dev",
24-
"build": "next build --webpack",
24+
"build": "pnpm run test:workflow-boundary && next build --webpack",
2525
"build:pkg": "tsup",
2626
"start": "next start",
27-
"test": "pnpm run test:slack-boundary && vitest run",
27+
"test": "pnpm run test:slack-boundary && pnpm run test:workflow-boundary && vitest run",
2828
"test:watch": "vitest",
2929
"preevals": "pnpm run test:slack-boundary",
3030
"evals": "pnpm exec vitest run -c vitest.evals.config.ts",
3131
"test:slack-boundary": "node scripts/check-slack-test-boundary.mjs",
32+
"test:workflow-boundary": "node scripts/check-workflow-node-modules.mjs",
3233
"typecheck": "tsc --noEmit",
3334
"skills:check": "node scripts/check-skills.mjs"
3435
},
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
#!/usr/bin/env node
2+
import fs from "node:fs/promises";
3+
import path from "node:path";
4+
5+
const projectRoot = process.cwd();
6+
const srcRoot = path.join(projectRoot, "src");
7+
const allowedExternal = new Set(["workflow", "workflow/api"]);
8+
9+
const sourceExts = [".ts", ".tsx", ".mts", ".cts", ".js", ".mjs", ".cjs"];
10+
11+
async function walk(dir) {
12+
const entries = await fs.readdir(dir, { withFileTypes: true });
13+
const files = [];
14+
for (const entry of entries) {
15+
const fullPath = path.join(dir, entry.name);
16+
if (entry.isDirectory()) {
17+
files.push(...(await walk(fullPath)));
18+
continue;
19+
}
20+
if (sourceExts.some((ext) => entry.name.endsWith(ext))) {
21+
files.push(fullPath);
22+
}
23+
}
24+
return files;
25+
}
26+
27+
function hasWorkflowDirective(source) {
28+
return /(^|\n)\s*["']use workflow["']\s*;?/.test(source);
29+
}
30+
31+
function extractStaticImports(source) {
32+
const imports = [];
33+
const importFromRegex = /(^|\n)\s*import\s+(?!type\b)[^;\n]*?from\s+["']([^"']+)["']/g;
34+
const sideEffectRegex = /(^|\n)\s*import\s+["']([^"']+)["']/g;
35+
const exportFromRegex = /(^|\n)\s*export\s+[^;\n]*?from\s+["']([^"']+)["']/g;
36+
37+
for (const regex of [importFromRegex, sideEffectRegex, exportFromRegex]) {
38+
let match;
39+
while ((match = regex.exec(source)) !== null) {
40+
imports.push(match[2]);
41+
}
42+
}
43+
return imports;
44+
}
45+
46+
async function fileExists(filepath) {
47+
try {
48+
await fs.access(filepath);
49+
return true;
50+
} catch {
51+
return false;
52+
}
53+
}
54+
55+
async function resolveLocalImport(fromFile, specifier) {
56+
const candidates = [];
57+
if (specifier.startsWith("@/")) {
58+
const rel = specifier.slice(2);
59+
candidates.push(path.join(srcRoot, rel));
60+
} else if (specifier.startsWith("./") || specifier.startsWith("../")) {
61+
candidates.push(path.resolve(path.dirname(fromFile), specifier));
62+
} else {
63+
return undefined;
64+
}
65+
66+
const resolvedCandidates = [];
67+
for (const base of candidates) {
68+
resolvedCandidates.push(base);
69+
for (const ext of sourceExts) {
70+
resolvedCandidates.push(`${base}${ext}`);
71+
resolvedCandidates.push(path.join(base, `index${ext}`));
72+
}
73+
}
74+
75+
for (const candidate of resolvedCandidates) {
76+
if (await fileExists(candidate)) {
77+
const stat = await fs.stat(candidate);
78+
if (stat.isFile()) {
79+
return candidate;
80+
}
81+
}
82+
}
83+
84+
return undefined;
85+
}
86+
87+
function isExternalImport(specifier) {
88+
return !specifier.startsWith("@/") && !specifier.startsWith("./") && !specifier.startsWith("../");
89+
}
90+
91+
const allSourceFiles = await walk(srcRoot);
92+
const workflowEntryFiles = [];
93+
for (const file of allSourceFiles) {
94+
const source = await fs.readFile(file, "utf8");
95+
if (hasWorkflowDirective(source)) {
96+
workflowEntryFiles.push(file);
97+
}
98+
}
99+
100+
const queue = [...workflowEntryFiles];
101+
const visited = new Set();
102+
const violations = [];
103+
104+
while (queue.length > 0) {
105+
const file = queue.shift();
106+
if (!file || visited.has(file)) {
107+
continue;
108+
}
109+
visited.add(file);
110+
111+
const source = await fs.readFile(file, "utf8");
112+
const imports = extractStaticImports(source);
113+
114+
for (const specifier of imports) {
115+
if (isExternalImport(specifier)) {
116+
if (!allowedExternal.has(specifier)) {
117+
violations.push({ file, specifier });
118+
}
119+
continue;
120+
}
121+
122+
const resolvedLocal = await resolveLocalImport(file, specifier);
123+
if (!resolvedLocal) {
124+
violations.push({ file, specifier, missing: true });
125+
continue;
126+
}
127+
queue.push(resolvedLocal);
128+
}
129+
}
130+
131+
if (violations.length > 0) {
132+
console.error("Workflow boundary check failed. Disallowed imports reachable from 'use workflow' files:\n");
133+
for (const violation of violations) {
134+
const relFile = path.relative(projectRoot, violation.file);
135+
if (violation.missing) {
136+
console.error(`- ${relFile}: cannot resolve import '${violation.specifier}'`);
137+
} else {
138+
console.error(`- ${relFile}: disallowed external import '${violation.specifier}'`);
139+
}
140+
}
141+
console.error("\nAllowed external imports in workflow graph: workflow, workflow/api");
142+
process.exit(1);
143+
}
144+
145+
console.log(`Workflow boundary check passed for ${workflowEntryFiles.length} workflow entr${workflowEntryFiles.length === 1 ? "y" : "ies"}.`);

packages/junior/src/chat/workflow/thread-steps.ts

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
import { Message, ThreadImpl } from "chat";
2-
import type { SerializedMessage, SerializedThread, Thread } from "chat";
3-
import { WORKFLOW_DESERIALIZE } from "@workflow/serde";
1+
import type { Message, SerializedMessage, SerializedThread, Thread } from "chat";
42
import type { ThreadMessagePayload } from "@/chat/workflow/types";
53

64
let stateAdapterConnected = false;
@@ -24,20 +22,6 @@ function isSerializedMessage(message: ThreadMessagePayload["message"]): message
2422
return typeof message === "object" && message !== null && (message as { _type?: unknown })._type === "chat:Message";
2523
}
2624

27-
function toRuntimeThread(thread: ThreadMessagePayload["thread"]): Thread {
28-
if (isSerializedThread(thread)) {
29-
return ThreadImpl[WORKFLOW_DESERIALIZE](thread);
30-
}
31-
return thread;
32-
}
33-
34-
function toRuntimeMessage(message: ThreadMessagePayload["message"]): Message {
35-
if (isSerializedMessage(message)) {
36-
return Message[WORKFLOW_DESERIALIZE](message);
37-
}
38-
return message;
39-
}
40-
4125
function getPayloadChannelId(payload: { thread: ThreadMessagePayload["thread"] }): string | undefined {
4226
return payload.thread.channelId;
4327
}
@@ -71,6 +55,13 @@ export async function logThreadMessageFailureStep(
7155

7256
export async function processThreadMessageStep(payload: ThreadMessagePayload, workflowRunId?: string): Promise<void> {
7357
"use step";
58+
const [{ Message, ThreadImpl }, { WORKFLOW_DESERIALIZE }] = await Promise.all([import("chat"), import("@workflow/serde")]);
59+
const threadDeserializer = (ThreadImpl as unknown as Record<PropertyKey, (value: SerializedThread) => Thread>)[
60+
WORKFLOW_DESERIALIZE
61+
];
62+
const messageDeserializer = (Message as unknown as Record<PropertyKey, (value: SerializedMessage) => Message>)[
63+
WORKFLOW_DESERIALIZE
64+
];
7465
const [
7566
{ appSlackRuntime, bot },
7667
{ downloadPrivateSlackFile },
@@ -120,8 +111,12 @@ export async function processThreadMessageStep(payload: ThreadMessagePayload, wo
120111
await getStateAdapter().connect();
121112
stateAdapterConnected = true;
122113
}
123-
const runtimeThread = toRuntimeThread(payload.thread);
124-
const runtimeMessage = toRuntimeMessage(payload.message);
114+
const runtimeThread = isSerializedThread(payload.thread)
115+
? threadDeserializer(payload.thread)
116+
: payload.thread;
117+
const runtimeMessage = isSerializedMessage(payload.message)
118+
? messageDeserializer(payload.message)
119+
: payload.message;
125120
const runtimePayload = {
126121
...payload,
127122
thread: runtimeThread,

0 commit comments

Comments
 (0)