Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
159 changes: 155 additions & 4 deletions packages/core/src/agents/runtime/workflow-saved.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import * as os from 'node:os';
import * as path from 'node:path';
import type { Config } from '../../config/config.js';
import { Storage } from '../../config/storage.js';
import { getShellContextEnvVars } from '../../services/shellContextEnv.js';
import {
registerSessionProjectDir,
sessionIdContext,
unregisterSessionProjectDir,
} from '../../utils/sessionIdContext.js';
import {
listSavedWorkflows,
resolveSavedWorkflowScript,
Expand Down Expand Up @@ -180,7 +186,145 @@ describe('workflow-saved', () => {
{ scriptPath: outside },
fakeConfig(projectDir),
),
).rejects.toThrow(/outside the saved-workflow directories/);
).rejects.toThrow(/outside the workflow script roots/);
});
});

describe('resolveSavedWorkflowScript — generated-scripts root', () => {
let generatedDir: string;

beforeEach(() => {
generatedDir = new Storage(projectDir).getGeneratedWorkflowsDir();
});

it('reads a {scriptPath} under the generated root', async () => {
await fs.mkdir(generatedDir, { recursive: true });
const p = path.join(generatedDir, 'qwen-review-1a2b3c.js');
await fs.writeFile(p, `return 'generated';`, 'utf8');
const resolved = await resolveSavedWorkflowScript(
{ scriptPath: p },
fakeConfig(projectDir),
);
expect(resolved.script).toBe(`return 'generated';`);
expect(resolved.name).toBe('qwen-review-1a2b3c');
});

it('trusts the whole subtree, so a writer may nest per session', async () => {
const nested = path.join(generatedDir, 's-abc', 'fanout.js');
await fs.mkdir(path.dirname(nested), { recursive: true });
await fs.writeFile(nested, `return 'nested';`, 'utf8');
const resolved = await resolveSavedWorkflowScript(
{ scriptPath: nested },
fakeConfig(projectDir),
);
expect(resolved.script).toBe(`return 'nested';`);
});

it('is neither listed as a slash command nor resolvable by name', async () => {
await fs.mkdir(generatedDir, { recursive: true });
await fs.writeFile(
path.join(generatedDir, 'emitted.js'),
`return 'generated';`,
'utf8',
);
expect(await listSavedWorkflows(fakeConfig(projectDir))).toEqual([]);
await expect(
resolveSavedWorkflowScript('emitted', fakeConfig(projectDir)),
).rejects.toThrow(/no workflow with that name.*\(none\)/);
});

it('refuses a sibling of the generated root (no prefix match)', async () => {
// `<runs>/generated-evil/x.js` shares the string prefix `generated`
// with the root but is not inside it.
const sibling = path.join(`${generatedDir}-evil`, 'x.js');
await fs.mkdir(path.dirname(sibling), { recursive: true });
await fs.writeFile(sibling, `return 'pwned';`, 'utf8');
const attempt = resolveSavedWorkflowScript(
{ scriptPath: sibling },
fakeConfig(projectDir),
);
await expect(attempt).rejects.toThrow(
/outside the workflow script roots \(checked: /,
);
// The refusal names every root it checked — the generated one lives in
// the runtime dir, which no debugger would guess from the path alone.
await expect(attempt).rejects.toThrow(generatedDir);
});

it('refuses a file that symlinks out of the generated root', async () => {
const outside = path.join(projectDir, 'secret.js');
await fs.writeFile(outside, `return 'EXFILTRATED';`, 'utf8');
await fs.mkdir(generatedDir, { recursive: true });
const link = path.join(generatedDir, 'link.js');
await fs.symlink(outside, link, 'file');
await expect(
resolveSavedWorkflowScript(
{ scriptPath: link },
fakeConfig(projectDir),
),
).rejects.toThrow(/outside the workflow script roots/);
});

it('refuses a symlinked generated root', async () => {
const external = await fs.mkdtemp(path.join(os.tmpdir(), 'wf-gen-evil-'));
try {
await fs.writeFile(
path.join(external, 'leak.js'),
`return 'EXFILTRATED';`,
'utf8',
);
await fs.mkdir(path.dirname(generatedDir), { recursive: true });
await fs.symlink(external, generatedDir, 'dir');
const attempt = resolveSavedWorkflowScript(
{ scriptPath: path.join(generatedDir, 'leak.js') },
fakeConfig(projectDir),
);
await expect(attempt).rejects.toThrow(
/outside the workflow script roots/,
);
// The refused root stays visible in the message: a checked-list it
// is silently absent from reads as if the loader never considered it.
await expect(attempt).rejects.toThrow(
`(checked: ${new Storage(projectDir).getProjectWorkflowsDir()}, ${Storage.getUserWorkflowsDir()}; refused symlinked root: ${generatedDir})`,
);
} finally {
await fs.rm(external, { recursive: true, force: true });
}
});
});

describe('resolveSavedWorkflowScript — subprocess reachability contract', () => {
it('loads a script at $QWEN_CODE_PROJECT_DIR/workflows/generated', async () => {
const sessionId = 'wf-contract';
// Mirror Config at session start: publish the storage project dir
// under the session id, then read back what a subprocess is handed.
registerSessionProjectDir(
sessionId,
new Storage(projectDir).getProjectDir(),
);
try {
const env = sessionIdContext.run(sessionId, () =>
getShellContextEnvVars(),
);
expect(env['QWEN_CODE_PROJECT_DIR']).toBeDefined();
// Literal path segments on purpose: this test must fail if EITHER
// half of the contract moves — the env export or the loader root.
const p = path.join(
env['QWEN_CODE_PROJECT_DIR'],
'workflows',
'generated',
'x.js',
);
await fs.mkdir(path.dirname(p), { recursive: true });
await fs.writeFile(p, `return 'composed';`, 'utf8');
const resolved = await resolveSavedWorkflowScript(
{ scriptPath: p },
fakeConfig(projectDir),
);
expect(resolved.script).toBe(`return 'composed';`);
} finally {
unregisterSessionProjectDir(sessionId);
}
});
});

Expand Down Expand Up @@ -376,9 +520,16 @@ describe('workflow-saved', () => {

it('{scriptPath} into a symlinked root is refused', async () => {
const p = path.join(projectWorkflowsDir, 'leak.js');
await expect(
resolveSavedWorkflowScript({ scriptPath: p }, fakeConfig(projectDir)),
).rejects.toThrow(/outside the saved-workflow directories/);
const attempt = resolveSavedWorkflowScript(
{ scriptPath: p },
fakeConfig(projectDir),
);
await expect(attempt).rejects.toThrow(
/outside the workflow script roots/,
);
await expect(attempt).rejects.toThrow(
`refused symlinked root: ${projectWorkflowsDir}`,
);
});

it('save into a symlinked root is refused (no write-through)', async () => {
Expand Down
69 changes: 48 additions & 21 deletions packages/core/src/agents/runtime/workflow-saved.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
* Precedence: when the same `<name>.js` exists in both scopes, the
* project-level file wins (matches `FileCommandLoader`'s project-over-user
* precedence for custom commands).
*
* A third root, `<projectDir>/workflows/generated`
* (`Storage.getGeneratedWorkflowsDir`), is trusted for `{scriptPath}` loads
* only. Scripts a tool generates for a single run go there: they are neither
* listed as slash commands nor resolvable by name, so emitting one never
* hands the user a command for a run that is already over.
*/

import { promises as fs } from 'node:fs';
Expand Down Expand Up @@ -87,6 +93,19 @@ export function getSavedWorkflowDirs(config: Config): Array<{
];
}

/**
* Every directory a `{scriptPath}` may resolve into: both saved scopes plus
* the generated-scripts root. Name resolution and discovery deliberately use
* {@link getSavedWorkflowDirs} instead — a generated script is loadable by
* path, never addressable by name.
*/
export function getWorkflowScriptRoots(config: Config): string[] {
return [
...getSavedWorkflowDirs(config).map(({ dir }) => dir),
config.storage.getGeneratedWorkflowsDir(),
];
}

/**
* True when a saved-workflow root dir is itself a symlink. `readWorkflowFileSecurely`
* realpaths the root so it can tolerate symlinked *ancestors* (e.g. a project under
Expand Down Expand Up @@ -139,35 +158,42 @@ async function listJsFiles(dir: string): Promise<string[]> {

/**
* Read a candidate workflow file, but only after proving its canonical real
* path stays inside one of the saved-workflow directories. `fs.realpath`
* resolves both `..` and symlinks, so this single check defeats path
* traversal (a `name`/`scriptPath` containing `..`) AND symlink escape (a
* file inside the dir that links out). Throws otherwise.
* path stays inside one of the workflow script roots (the saved-workflow
* directories or the generated-scripts root). `fs.realpath` resolves both
* `..` and symlinks, so this single check defeats path traversal (a
* `name`/`scriptPath` containing `..`) AND symlink escape (a file inside the
* dir that links out). Throws otherwise.
*/
async function readWorkflowFileSecurely(
filePath: string,
config: Config,
): Promise<string> {
const real = await fs.realpath(filePath); // throws ENOENT if absent
const dirs = (
await Promise.all(
getSavedWorkflowDirs(config).map(async ({ dir }) => {
// Exclude a symlinked root: realpath(dir) would launder a
// `.qwen/workflows -> /outside` link into the allowed boundary, so a
// file resolving under the link's target would pass the check below.
if (await isSymlinkedRoot(dir)) return null;
try {
return await fs.realpath(dir);
} catch {
return path.resolve(dir);
}
}),
)
).filter((d): d is string => d !== null);
const roots = await Promise.all(
getWorkflowScriptRoots(config).map(async (dir) => {
// Exclude a symlinked root: realpath(dir) would launder a
// `.qwen/workflows -> /outside` link into the allowed boundary, so a
// file resolving under the link's target would pass the check below.
if (await isSymlinkedRoot(dir)) return { dir, real: null };
try {
return { dir, real: await fs.realpath(dir) };
} catch {
return { dir, real: path.resolve(dir) };
}
}),
);
const dirs = roots.flatMap((r) => (r.real === null ? [] : [r.real]));
const inside = dirs.some((d) => real === d || real.startsWith(d + path.sep));
if (!inside) {
// Keep refused-but-considered roots visible: dropping a symlinked root
// from the list reads as if the loader never considered it at all.
const refused = roots.flatMap((r) => (r.real === null ? [r.dir] : []));
const refusedNote =
refused.length > 0
? `; refused symlinked ${refused.length === 1 ? 'root' : 'roots'}: ${refused.join(', ')}`
: '';
throw new Error(
`refusing to load a workflow file outside the saved-workflow directories: '${filePath}'.`,
`refusing to load a workflow file outside the workflow script roots (checked: ${dirs.join(', ')}${refusedNote}): '${filePath}'.`,
);
}
return fs.readFile(real, 'utf8');
Expand Down Expand Up @@ -200,7 +226,8 @@ export async function listSavedWorkflows(
/**
* Resolve `workflow('<name>')` or `workflow({scriptPath})` to a loaded
* script. The string form looks up `<name>.js` in project then user scope;
* the `{scriptPath}` form reads the file at the given path directly.
* the `{scriptPath}` form reads the file at the given path directly, which
* may sit in either saved scope or under the generated-scripts root.
*
* Throws with an actionable, available-names message on a miss — the
* message text mirrors upstream so scripts written against either runtime
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/config/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,18 @@ describe('Storage – runtime path methods use getRuntimeBaseDir', () => {
expect(storage.getProjectDir()).toContain(path.join(customDir, 'projects'));
});

it('getGeneratedWorkflowsDir sits under the project workflow runs dir', () => {
const customDir = path.resolve('custom');
Storage.setRuntimeBaseDir(customDir);
const storage = new Storage('/tmp/project');
expect(storage.getGeneratedWorkflowsDir()).toBe(
path.join(storage.getWorkflowRunsDir(), 'generated'),
);
expect(storage.getGeneratedWorkflowsDir()).toContain(
path.join(customDir, 'projects'),
);
});

it('getProjectTempDir uses custom runtime base dir', () => {
const customDir = path.resolve('custom');
Storage.setRuntimeBaseDir(customDir);
Expand Down
16 changes: 16 additions & 0 deletions packages/core/src/config/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,22 @@ export class Storage {
return path.join(this.getProjectDir(), 'workflows');
}

/**
* Generated-workflow scripts directory: `<projectDir>/workflows/generated`.
* A trusted root for `Workflow({scriptPath})` / `workflow({scriptPath})`
* that is NOT a saved-workflow scope: scripts here are never listed as
* `/<name>` slash commands and cannot be reached by `workflow('<name>')`.
* It exists for tooling that emits a script for one run (a CLI subcommand
* generating a fan-out for the model to dispatch) — such a script has no
* business in the user's command namespace, and the runtime dir keeps it
* out of the project tree. Layout below the root is the writer's; the
* loader trusts the whole subtree. A subprocess reaches it as
* `$QWEN_CODE_PROJECT_DIR/workflows/generated`.
*/
getGeneratedWorkflowsDir(): string {
return path.join(this.getWorkflowRunsDir(), 'generated');
}

/**
* Path to the persisted snapshot of a completed workflow run.
*/
Expand Down
Loading
Loading