From d972ade968906a143d5d81c5d106b6aa333ae09a Mon Sep 17 00:00:00 2001 From: qqqys Date: Tue, 25 Aug 2026 15:00:20 +0800 Subject: [PATCH 1/5] feat(core): trust a generated-scripts root for workflow scriptPath loads `Workflow({scriptPath})` and `workflow({scriptPath})` only load files that resolve inside the two saved-workflow directories, and every file in those directories is also a `/` slash command. A tool that generates a workflow script for a single run therefore had no place to put it: writing into `.qwen/workflows` hands the user a permanent command for a run that is already over, and any other path is refused by the loader. Add `/workflows/generated` as a third trusted root for `{scriptPath}` loads only. It lives in the runtime dir beside the run snapshots and journals, is never enumerated by `listSavedWorkflows`, and cannot be reached by `workflow('')`. The same realpath boundary check and symlinked-root refusal apply. The tool description stops claiming that `scriptPath` accepts a path anywhere. Part of #8769. Claude-Session: https://claude.ai/code/session_017cUwuTey4APA8wAyAM6ScS --- .../src/agents/runtime/workflow-saved.test.ts | 97 ++++++++++++++++++- .../core/src/agents/runtime/workflow-saved.ts | 35 +++++-- packages/core/src/config/storage.test.ts | 12 +++ packages/core/src/config/storage.ts | 16 +++ packages/core/src/tools/workflow/workflow.ts | 17 ++-- 5 files changed, 162 insertions(+), 15 deletions(-) diff --git a/packages/core/src/agents/runtime/workflow-saved.test.ts b/packages/core/src/agents/runtime/workflow-saved.test.ts index 1ca1d45c521..1e08331d7e5 100644 --- a/packages/core/src/agents/runtime/workflow-saved.test.ts +++ b/packages/core/src/agents/runtime/workflow-saved.test.ts @@ -180,7 +180,100 @@ describe('workflow-saved', () => { { scriptPath: outside }, fakeConfig(projectDir), ), - ).rejects.toThrow(/outside the saved-workflow directories/); + ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); + }); + }); + + 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 () => { + // `/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'); + await expect( + resolveSavedWorkflowScript( + { scriptPath: sibling }, + fakeConfig(projectDir), + ), + ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); + }); + + 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 saved-workflow and generated-workflow/); + }); + + 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'); + await expect( + resolveSavedWorkflowScript( + { scriptPath: path.join(generatedDir, 'leak.js') }, + fakeConfig(projectDir), + ), + ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); + } finally { + await fs.rm(external, { recursive: true, force: true }); + } }); }); @@ -378,7 +471,7 @@ describe('workflow-saved', () => { const p = path.join(projectWorkflowsDir, 'leak.js'); await expect( resolveSavedWorkflowScript({ scriptPath: p }, fakeConfig(projectDir)), - ).rejects.toThrow(/outside the saved-workflow directories/); + ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); }); it('save into a symlinked root is refused (no write-through)', async () => { diff --git a/packages/core/src/agents/runtime/workflow-saved.ts b/packages/core/src/agents/runtime/workflow-saved.ts index fcae700c943..2d689624d2f 100644 --- a/packages/core/src/agents/runtime/workflow-saved.ts +++ b/packages/core/src/agents/runtime/workflow-saved.ts @@ -16,6 +16,12 @@ * Precedence: when the same `.js` exists in both scopes, the * project-level file wins (matches `FileCommandLoader`'s project-over-user * precedence for custom commands). + * + * A third root, `/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'; @@ -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 @@ -139,10 +158,11 @@ async function listJsFiles(dir: string): Promise { /** * 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, @@ -151,7 +171,7 @@ async function readWorkflowFileSecurely( const real = await fs.realpath(filePath); // throws ENOENT if absent const dirs = ( await Promise.all( - getSavedWorkflowDirs(config).map(async ({ dir }) => { + 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. @@ -167,7 +187,7 @@ async function readWorkflowFileSecurely( const inside = dirs.some((d) => real === d || real.startsWith(d + path.sep)); if (!inside) { throw new Error( - `refusing to load a workflow file outside the saved-workflow directories: '${filePath}'.`, + `refusing to load a workflow file outside the saved-workflow and generated-workflow directories: '${filePath}'.`, ); } return fs.readFile(real, 'utf8'); @@ -200,7 +220,8 @@ export async function listSavedWorkflows( /** * Resolve `workflow('')` or `workflow({scriptPath})` to a loaded * script. The string form looks up `.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 diff --git a/packages/core/src/config/storage.test.ts b/packages/core/src/config/storage.test.ts index 734c5997f0c..f3e7c46856d 100644 --- a/packages/core/src/config/storage.test.ts +++ b/packages/core/src/config/storage.test.ts @@ -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); diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index 048a297744e..bb39d2035cc 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -680,6 +680,22 @@ export class Storage { return path.join(this.getProjectDir(), 'workflows'); } + /** + * Generated-workflow scripts directory: `/workflows/generated`. + * A trusted root for `Workflow({scriptPath})` / `workflow({scriptPath})` + * that is NOT a saved-workflow scope: scripts here are never listed as + * `/` slash commands and cannot be reached by `workflow('')`. + * 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. */ diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index 45878edafce..c7a2dd79dec 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -178,11 +178,16 @@ const WORKFLOW_PARAM_SCHEMA = { scriptPath: { type: 'string', description: - 'Optional. Absolute path to a saved workflow `.js` file to load and ' + - 'run instead of inline `script`. Primarily set by the `/` ' + - 'saved-workflow slash command. Provide exactly ONE of `script` or ' + - '`scriptPath`. The file is read at execution time, so edits to a ' + - 'saved workflow take effect on the next run.', + 'Optional. Absolute path to a workflow `.js` file to load and run ' + + 'instead of inline `script`. Primarily set by the `/` ' + + 'saved-workflow slash command; a tool that generated a script for ' + + 'this run hands you its path the same way. The file must resolve ' + + 'inside a saved-workflow directory (`.qwen/workflows`, ' + + '`~/.qwen/workflows`) or the generated-scripts root ' + + '(`/workflows/generated`) — any other path is refused. ' + + 'Provide exactly ONE of `script` or `scriptPath`. The file is read ' + + 'at execution time, so edits to a saved workflow take effect on the ' + + 'next run.', }, args: { description: @@ -843,7 +848,7 @@ Reach for one to be comprehensive (decompose the work and cover every part in pa **Runtime** — see the \`script\` parameter for the detailed authoring contract. -\`phase(title)\`, \`log(msg)\`, \`agent(prompt, opts?)\`, \`parallel(thunks)\`, \`pipeline(items, ...stages)\`, \`workflow(nameOrRef, args?)\`, plus the \`args\` and \`budget\` globals. \`workflow()\` runs a saved workflow inline under this run's caps and nests one level only — a workflow reached through \`workflow()\` cannot call \`workflow()\` itself, and doing so throws. Saved workflows are \`.js\` files under \`/.qwen/workflows\` (project scope, also surfaced as \`/\` slash commands) or \`~/.qwen/workflows\` (user scope, lower precedence when both define the same name); \`workflow('')\` resolves against those two directories, while \`scriptPath\` takes an absolute path to a script anywhere. Default \`max(1, min(16, cpus-2))\` agents in flight per run (\`${MAX_WORKFLOW_CONCURRENCY_ENV}\`), up to ${DEFAULT_MAX_AGENTS_PER_RUN} agents total (\`${MAX_WORKFLOW_AGENTS_ENV}\`), under a 30-minute wall-clock cap per run (\`QWEN_CODE_MAX_WORKFLOW_SECONDS\`) — a fan-out near the agent cap will not fit inside the default cap. Each subagent attempt is separately capped at ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS} turns (\`${WORKFLOW_SUBAGENT_MAX_TURNS_ENV}\`) and ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TIME_MINUTES} minutes (\`${WORKFLOW_SUBAGENT_MAX_MINUTES_ENV}\`) — an attempt that hits either becomes \`null\` in \`parallel()\`/\`pipeline()\`, indistinguishable from a missing agent, so raise them for legitimately long work. A per-run output-token cap may also be in effect: read \`budget.total\` (\`null\` = uncapped) before committing to a large fan-out, because once the cap is reached every further \`agent()\` call is refused — a bare sequential \`await agent()\` sees the rejection, while inside \`parallel()\`/\`pipeline()\` the refused slot becomes \`null\` and the script keeps running on partial results. Per-call \`agent({ schema, agentType, model, isolation: 'worktree', workingDir, stallMs })\` covers structured-output contracts, declarative-agent selection, model override, git-worktree-isolated subagents, pinning an agent to a caller-owned worktree, and the no-progress stall watchdog (\`stallMs: 0\` disables it). \`resumeFromRunId\` resumes a prior run — agent() calls whose rolling prefix-hash matches the journal are served from cache for the longest unchanged prefix. Runs appear in the background-tasks view and the \`/workflows\` dialog (live phase tree, token usage, cooperative pause/resume, cancel); \`run_in_background: true\` returns a run handle immediately in the interactive TUI and delivers completion through the conversation. Scripts run in a node:vm sandbox with no filesystem or shell access — all I/O happens through the spawned agents. +\`phase(title)\`, \`log(msg)\`, \`agent(prompt, opts?)\`, \`parallel(thunks)\`, \`pipeline(items, ...stages)\`, \`workflow(nameOrRef, args?)\`, plus the \`args\` and \`budget\` globals. \`workflow()\` runs a saved workflow inline under this run's caps and nests one level only — a workflow reached through \`workflow()\` cannot call \`workflow()\` itself, and doing so throws. Saved workflows are \`.js\` files under \`/.qwen/workflows\` (project scope, also surfaced as \`/\` slash commands) or \`~/.qwen/workflows\` (user scope, lower precedence when both define the same name); \`workflow('')\` resolves against those two directories, while \`scriptPath\` takes an absolute path to a script inside either of them or inside the generated-scripts root (\`/workflows/generated\`, where a tool emitting a one-run script writes it — never a slash command, never resolvable by name); a path outside those roots is refused. Default \`max(1, min(16, cpus-2))\` agents in flight per run (\`${MAX_WORKFLOW_CONCURRENCY_ENV}\`), up to ${DEFAULT_MAX_AGENTS_PER_RUN} agents total (\`${MAX_WORKFLOW_AGENTS_ENV}\`), under a 30-minute wall-clock cap per run (\`QWEN_CODE_MAX_WORKFLOW_SECONDS\`) — a fan-out near the agent cap will not fit inside the default cap. Each subagent attempt is separately capped at ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS} turns (\`${WORKFLOW_SUBAGENT_MAX_TURNS_ENV}\`) and ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TIME_MINUTES} minutes (\`${WORKFLOW_SUBAGENT_MAX_MINUTES_ENV}\`) — an attempt that hits either becomes \`null\` in \`parallel()\`/\`pipeline()\`, indistinguishable from a missing agent, so raise them for legitimately long work. A per-run output-token cap may also be in effect: read \`budget.total\` (\`null\` = uncapped) before committing to a large fan-out, because once the cap is reached every further \`agent()\` call is refused — a bare sequential \`await agent()\` sees the rejection, while inside \`parallel()\`/\`pipeline()\` the refused slot becomes \`null\` and the script keeps running on partial results. Per-call \`agent({ schema, agentType, model, isolation: 'worktree', workingDir, stallMs })\` covers structured-output contracts, declarative-agent selection, model override, git-worktree-isolated subagents, pinning an agent to a caller-owned worktree, and the no-progress stall watchdog (\`stallMs: 0\` disables it). \`resumeFromRunId\` resumes a prior run — agent() calls whose rolling prefix-hash matches the journal are served from cache for the longest unchanged prefix. Runs appear in the background-tasks view and the \`/workflows\` dialog (live phase tree, token usage, cooperative pause/resume, cancel); \`run_in_background: true\` returns a run handle immediately in the interactive TUI and delivers completion through the conversation. Scripts run in a node:vm sandbox with no filesystem or shell access — all I/O happens through the spawned agents. **Scout first, then orchestrate** From 3a555a182f865bc6f5db85610adbc184d8add6c4 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Tue, 25 Aug 2026 11:00:20 +0000 Subject: [PATCH 2/5] fix(core): disambiguate the generated-workflow root and name refusal roots (#9987) Co-authored-by: Qwen-Coder --- .../src/agents/runtime/workflow-saved.test.ts | 65 ++++++++++++++++--- .../core/src/agents/runtime/workflow-saved.ts | 2 +- packages/core/src/tools/workflow/workflow.ts | 5 +- 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/packages/core/src/agents/runtime/workflow-saved.test.ts b/packages/core/src/agents/runtime/workflow-saved.test.ts index 1e08331d7e5..dc8c73079ef 100644 --- a/packages/core/src/agents/runtime/workflow-saved.test.ts +++ b/packages/core/src/agents/runtime/workflow-saved.test.ts @@ -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, @@ -180,7 +186,7 @@ describe('workflow-saved', () => { { scriptPath: outside }, fakeConfig(projectDir), ), - ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); + ).rejects.toThrow(/outside the workflow script roots/); }); }); @@ -233,12 +239,16 @@ describe('workflow-saved', () => { const sibling = path.join(`${generatedDir}-evil`, 'x.js'); await fs.mkdir(path.dirname(sibling), { recursive: true }); await fs.writeFile(sibling, `return 'pwned';`, 'utf8'); - await expect( - resolveSavedWorkflowScript( - { scriptPath: sibling }, - fakeConfig(projectDir), - ), - ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); + 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 () => { @@ -252,7 +262,7 @@ describe('workflow-saved', () => { { scriptPath: link }, fakeConfig(projectDir), ), - ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); + ).rejects.toThrow(/outside the workflow script roots/); }); it('refuses a symlinked generated root', async () => { @@ -270,13 +280,48 @@ describe('workflow-saved', () => { { scriptPath: path.join(generatedDir, 'leak.js') }, fakeConfig(projectDir), ), - ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); + ).rejects.toThrow(/outside the workflow script roots/); } 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); + } + }); + }); + // Security (#2): the string-name form must not escape the saved dirs. describe('resolveSavedWorkflowScript — name traversal', () => { it('rejects a traversal name before any path join', async () => { @@ -471,7 +516,7 @@ describe('workflow-saved', () => { const p = path.join(projectWorkflowsDir, 'leak.js'); await expect( resolveSavedWorkflowScript({ scriptPath: p }, fakeConfig(projectDir)), - ).rejects.toThrow(/outside the saved-workflow and generated-workflow/); + ).rejects.toThrow(/outside the workflow script roots/); }); it('save into a symlinked root is refused (no write-through)', async () => { diff --git a/packages/core/src/agents/runtime/workflow-saved.ts b/packages/core/src/agents/runtime/workflow-saved.ts index 2d689624d2f..39ac6197994 100644 --- a/packages/core/src/agents/runtime/workflow-saved.ts +++ b/packages/core/src/agents/runtime/workflow-saved.ts @@ -187,7 +187,7 @@ async function readWorkflowFileSecurely( const inside = dirs.some((d) => real === d || real.startsWith(d + path.sep)); if (!inside) { throw new Error( - `refusing to load a workflow file outside the saved-workflow and generated-workflow directories: '${filePath}'.`, + `refusing to load a workflow file outside the workflow script roots (checked: ${dirs.join(', ')}): '${filePath}'.`, ); } return fs.readFile(real, 'utf8'); diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index c7a2dd79dec..79794ef172a 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -184,7 +184,8 @@ const WORKFLOW_PARAM_SCHEMA = { 'this run hands you its path the same way. The file must resolve ' + 'inside a saved-workflow directory (`.qwen/workflows`, ' + '`~/.qwen/workflows`) or the generated-scripts root ' + - '(`/workflows/generated`) — any other path is refused. ' + + '(`$QWEN_CODE_PROJECT_DIR/workflows/generated` — the per-project ' + + 'runtime dir, not the project tree) — any other path is refused. ' + 'Provide exactly ONE of `script` or `scriptPath`. The file is read ' + 'at execution time, so edits to a saved workflow take effect on the ' + 'next run.', @@ -848,7 +849,7 @@ Reach for one to be comprehensive (decompose the work and cover every part in pa **Runtime** — see the \`script\` parameter for the detailed authoring contract. -\`phase(title)\`, \`log(msg)\`, \`agent(prompt, opts?)\`, \`parallel(thunks)\`, \`pipeline(items, ...stages)\`, \`workflow(nameOrRef, args?)\`, plus the \`args\` and \`budget\` globals. \`workflow()\` runs a saved workflow inline under this run's caps and nests one level only — a workflow reached through \`workflow()\` cannot call \`workflow()\` itself, and doing so throws. Saved workflows are \`.js\` files under \`/.qwen/workflows\` (project scope, also surfaced as \`/\` slash commands) or \`~/.qwen/workflows\` (user scope, lower precedence when both define the same name); \`workflow('')\` resolves against those two directories, while \`scriptPath\` takes an absolute path to a script inside either of them or inside the generated-scripts root (\`/workflows/generated\`, where a tool emitting a one-run script writes it — never a slash command, never resolvable by name); a path outside those roots is refused. Default \`max(1, min(16, cpus-2))\` agents in flight per run (\`${MAX_WORKFLOW_CONCURRENCY_ENV}\`), up to ${DEFAULT_MAX_AGENTS_PER_RUN} agents total (\`${MAX_WORKFLOW_AGENTS_ENV}\`), under a 30-minute wall-clock cap per run (\`QWEN_CODE_MAX_WORKFLOW_SECONDS\`) — a fan-out near the agent cap will not fit inside the default cap. Each subagent attempt is separately capped at ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS} turns (\`${WORKFLOW_SUBAGENT_MAX_TURNS_ENV}\`) and ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TIME_MINUTES} minutes (\`${WORKFLOW_SUBAGENT_MAX_MINUTES_ENV}\`) — an attempt that hits either becomes \`null\` in \`parallel()\`/\`pipeline()\`, indistinguishable from a missing agent, so raise them for legitimately long work. A per-run output-token cap may also be in effect: read \`budget.total\` (\`null\` = uncapped) before committing to a large fan-out, because once the cap is reached every further \`agent()\` call is refused — a bare sequential \`await agent()\` sees the rejection, while inside \`parallel()\`/\`pipeline()\` the refused slot becomes \`null\` and the script keeps running on partial results. Per-call \`agent({ schema, agentType, model, isolation: 'worktree', workingDir, stallMs })\` covers structured-output contracts, declarative-agent selection, model override, git-worktree-isolated subagents, pinning an agent to a caller-owned worktree, and the no-progress stall watchdog (\`stallMs: 0\` disables it). \`resumeFromRunId\` resumes a prior run — agent() calls whose rolling prefix-hash matches the journal are served from cache for the longest unchanged prefix. Runs appear in the background-tasks view and the \`/workflows\` dialog (live phase tree, token usage, cooperative pause/resume, cancel); \`run_in_background: true\` returns a run handle immediately in the interactive TUI and delivers completion through the conversation. Scripts run in a node:vm sandbox with no filesystem or shell access — all I/O happens through the spawned agents. +\`phase(title)\`, \`log(msg)\`, \`agent(prompt, opts?)\`, \`parallel(thunks)\`, \`pipeline(items, ...stages)\`, \`workflow(nameOrRef, args?)\`, plus the \`args\` and \`budget\` globals. \`workflow()\` runs a saved workflow inline under this run's caps and nests one level only — a workflow reached through \`workflow()\` cannot call \`workflow()\` itself, and doing so throws. Saved workflows are \`.js\` files under \`/.qwen/workflows\` (project scope, also surfaced as \`/\` slash commands) or \`~/.qwen/workflows\` (user scope, lower precedence when both define the same name); \`workflow('')\` resolves against those two directories, while \`scriptPath\` takes an absolute path to a script inside either of them or inside the generated-scripts root (\`$QWEN_CODE_PROJECT_DIR/workflows/generated\` — the per-project runtime dir, not the project tree — where a tool emitting a one-run script writes it; never a slash command, never resolvable by name); a path outside those roots is refused. Default \`max(1, min(16, cpus-2))\` agents in flight per run (\`${MAX_WORKFLOW_CONCURRENCY_ENV}\`), up to ${DEFAULT_MAX_AGENTS_PER_RUN} agents total (\`${MAX_WORKFLOW_AGENTS_ENV}\`), under a 30-minute wall-clock cap per run (\`QWEN_CODE_MAX_WORKFLOW_SECONDS\`) — a fan-out near the agent cap will not fit inside the default cap. Each subagent attempt is separately capped at ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS} turns (\`${WORKFLOW_SUBAGENT_MAX_TURNS_ENV}\`) and ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TIME_MINUTES} minutes (\`${WORKFLOW_SUBAGENT_MAX_MINUTES_ENV}\`) — an attempt that hits either becomes \`null\` in \`parallel()\`/\`pipeline()\`, indistinguishable from a missing agent, so raise them for legitimately long work. A per-run output-token cap may also be in effect: read \`budget.total\` (\`null\` = uncapped) before committing to a large fan-out, because once the cap is reached every further \`agent()\` call is refused — a bare sequential \`await agent()\` sees the rejection, while inside \`parallel()\`/\`pipeline()\` the refused slot becomes \`null\` and the script keeps running on partial results. Per-call \`agent({ schema, agentType, model, isolation: 'worktree', workingDir, stallMs })\` covers structured-output contracts, declarative-agent selection, model override, git-worktree-isolated subagents, pinning an agent to a caller-owned worktree, and the no-progress stall watchdog (\`stallMs: 0\` disables it). \`resumeFromRunId\` resumes a prior run — agent() calls whose rolling prefix-hash matches the journal are served from cache for the longest unchanged prefix. Runs appear in the background-tasks view and the \`/workflows\` dialog (live phase tree, token usage, cooperative pause/resume, cancel); \`run_in_background: true\` returns a run handle immediately in the interactive TUI and delivers completion through the conversation. Scripts run in a node:vm sandbox with no filesystem or shell access — all I/O happens through the spawned agents. **Scout first, then orchestrate** From 9254a2c58b938b98428c1d80f98f85ce8346dd19 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Tue, 25 Aug 2026 17:34:59 +0000 Subject: [PATCH 3/5] fix(core): label generated workflow scripts and name refused symlinked roots (#9987) --- .../src/agents/runtime/workflow-saved.test.ts | 31 ++++++--- .../core/src/agents/runtime/workflow-saved.ts | 38 ++++++----- .../core/src/tools/workflow/workflow.test.ts | 63 +++++++++++++++++-- packages/core/src/tools/workflow/workflow.ts | 52 +++++++++++---- 4 files changed, 142 insertions(+), 42 deletions(-) diff --git a/packages/core/src/agents/runtime/workflow-saved.test.ts b/packages/core/src/agents/runtime/workflow-saved.test.ts index dc8c73079ef..4f5564840d5 100644 --- a/packages/core/src/agents/runtime/workflow-saved.test.ts +++ b/packages/core/src/agents/runtime/workflow-saved.test.ts @@ -275,12 +275,18 @@ describe('workflow-saved', () => { ); await fs.mkdir(path.dirname(generatedDir), { recursive: true }); await fs.symlink(external, generatedDir, 'dir'); - await expect( - resolveSavedWorkflowScript( - { scriptPath: path.join(generatedDir, 'leak.js') }, - fakeConfig(projectDir), - ), - ).rejects.toThrow(/outside the workflow script roots/); + 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 }); } @@ -514,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 workflow script roots/); + 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 () => { diff --git a/packages/core/src/agents/runtime/workflow-saved.ts b/packages/core/src/agents/runtime/workflow-saved.ts index 39ac6197994..27b528991f4 100644 --- a/packages/core/src/agents/runtime/workflow-saved.ts +++ b/packages/core/src/agents/runtime/workflow-saved.ts @@ -169,25 +169,31 @@ async function readWorkflowFileSecurely( config: Config, ): Promise { const real = await fs.realpath(filePath); // throws ENOENT if absent - const dirs = ( - 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 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 workflow script roots (checked: ${dirs.join(', ')}): '${filePath}'.`, + `refusing to load a workflow file outside the workflow script roots (checked: ${dirs.join(', ')}${refusedNote}): '${filePath}'.`, ); } return fs.readFile(real, 'utf8'); diff --git a/packages/core/src/tools/workflow/workflow.test.ts b/packages/core/src/tools/workflow/workflow.test.ts index 60faeeb0dd0..951d4cba812 100644 --- a/packages/core/src/tools/workflow/workflow.test.ts +++ b/packages/core/src/tools/workflow/workflow.test.ts @@ -48,6 +48,16 @@ function configWithRegistry(): { return { config, registry }; } +/** + * The scriptPath approval/description surface classifies the path against + * the generated-scripts root, so it needs a config with a real `storage`. + */ +function configWithStorage(): Config { + return { + storage: new Storage(path.join(os.tmpdir(), 'workflow-label-test')), + } as unknown as Config; +} + describe('WorkflowTool', () => { it('has the registered name and display name', () => { const tool = new WorkflowTool(fakeConfig()); @@ -314,9 +324,10 @@ await agent('scan package.json') }); it('scopes a saved-workflow grant to the path that was approved', async () => { - const details = (await detailsFor({ - scriptPath: '/home/u/.qwen/workflows/audit.js', - })) as { hideAlwaysAllow?: boolean; permissionRules?: string[] }; + const details = (await detailsFor( + { scriptPath: '/home/u/.qwen/workflows/audit.js' }, + configWithStorage(), + )) as { hideAlwaysAllow?: boolean; permissionRules?: string[] }; expect(details.hideAlwaysAllow).toBeFalsy(); expect(details.permissionRules).toHaveLength(1); @@ -345,6 +356,50 @@ await agent('scan package.json') expect(wouldAllow({ script: 'await agent("x")' })).toBe(false); }); + // A generated-root script is a throwaway artifact a tool emitted for this + // run. Labeling it as a saved workflow would have the user approve — and + // maybe pre-approve the path rule — under a wrong identity. + it('labels a generated-root scriptPath as a generated script, not a saved workflow', async () => { + const storage = new Storage( + path.join(os.tmpdir(), 'workflow-label-test'), + ); + const config = { storage } as unknown as Config; + const scriptPath = path.join( + storage.getGeneratedWorkflowsDir(), + 'fanout-1a2b3c.js', + ); + const tool = new WorkflowTool(config); + expect(tool.build({ scriptPath }).getDescription()).toBe( + 'Run generated workflow script (fanout-1a2b3c.js)', + ); + const details = (await detailsFor({ scriptPath }, config)) as { + prompt: string; + }; + expect(details.prompt).toContain( + `Generated workflow script: ${scriptPath}`, + ); + expect(details.prompt).not.toContain('Saved workflow'); + }); + + it('keeps the saved-workflow label for a saved-root scriptPath', async () => { + const storage = new Storage( + path.join(os.tmpdir(), 'workflow-label-test'), + ); + const config = { storage } as unknown as Config; + const scriptPath = path.join( + storage.getProjectWorkflowsDir(), + 'deep-research.js', + ); + const tool = new WorkflowTool(config); + expect(tool.build({ scriptPath }).getDescription()).toBe( + 'Run saved workflow (deep-research.js)', + ); + const details = (await detailsFor({ scriptPath }, config)) as { + prompt: string; + }; + expect(details.prompt).toContain(`Saved workflow: ${scriptPath}`); + }); + // The cost warning used to arrive only after a successful run — i.e. // after the spend it warns about, and never at all on the failure path. it('warns about token cost before the spend, exactly once', async () => { @@ -452,7 +507,7 @@ await agent('scan package.json') }); it('build() accepts a scriptPath without inline script', () => { - const tool = new WorkflowTool(fakeConfig()); + const tool = new WorkflowTool(configWithStorage()); const invocation = tool.build({ scriptPath: '/abs/deep-research.js', }); diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index 79794ef172a..6e15d1bae6d 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -72,11 +72,12 @@ export interface WorkflowParams { */ script?: string; /** - * P7b: absolute path to a saved workflow `.js` file to load and run - * instead of inline `script`. Set by the `/` saved-workflow slash - * command (`SavedWorkflowLoader`). Read at execution time so edits to the - * saved file take effect on the next run; the resolved path is recorded on - * the registry entry as run provenance. + * P7b: absolute path to a workflow `.js` file to load and run instead of + * inline `script` — a saved workflow, set by the `/` slash command + * (`SavedWorkflowLoader`), or a one-run script a tool generated under the + * generated-scripts root. Read at execution time so edits to the file take + * effect on the next run; the resolved path is recorded on the registry + * entry as run provenance. */ scriptPath?: string; /** Optional structured value bound to the `args` global inside the script. */ @@ -257,7 +258,13 @@ class WorkflowToolInvocation extends BaseToolInvocation< return `Run workflow: ${sanitizeLine(meta.name)}`; } if (this.params.scriptPath && this.params.script === undefined) { - return `Run saved workflow (${path.basename(this.params.scriptPath)})`; + const kind = isGeneratedWorkflowScriptPath( + this.config, + this.params.scriptPath, + ) + ? 'generated workflow script' + : 'saved workflow'; + return `Run ${kind} (${path.basename(this.params.scriptPath)})`; } return `Run a workflow script (${this.params.script?.length ?? 0} chars)`; } @@ -297,7 +304,7 @@ class WorkflowToolInvocation extends BaseToolInvocation< _abortSignal: AbortSignal, ): Promise { const meta = this.resolveMeta(); - const body = buildConfirmationPrompt(this.params, meta); + const body = buildConfirmationPrompt(this.params, meta, this.config); // The cost warning belongs before the spend, not after it. The registry // latch flips on read, so surfacing it here means the post-hoc copy on @@ -636,16 +643,32 @@ function readMetaForConfirmation(script: string): WorkflowMeta | null { } } +/** + * True when a `scriptPath` points inside the generated-scripts root. Such a + * script is a throwaway artifact a tool emitted for this run, not a workflow + * the user saved — the approval surface labels it accordingly so a grant is + * given under the right identity. Lexical match on purpose: this only picks + * a label; the security check is the realpath boundary in the loader. + */ +function isGeneratedWorkflowScriptPath( + config: Config, + scriptPath: string, +): boolean { + const root = config.storage.getGeneratedWorkflowsDir(); + return scriptPath === root || scriptPath.startsWith(root + path.sep); +} + /** * The body of the approval dialog: what this workflow says it will do. * - * Everything here comes from `meta` and the call's own parameters — never from - * executing the script. When `meta` is absent or unreadable the dialog still - * renders, just with less to say. + * Everything here comes from `meta`, the call's own parameters, and the + * configured script roots — never from executing the script. When `meta` is + * absent or unreadable the dialog still renders, just with less to say. */ function buildConfirmationPrompt( params: WorkflowParams, meta: WorkflowMeta | null, + config: Config, ): string { const lines: string[] = []; @@ -653,7 +676,10 @@ function buildConfirmationPrompt( lines.push(`Workflow: ${sanitizeLine(meta.name)}`); lines.push(sanitizeLine(meta.description)); } else if (params.scriptPath) { - lines.push(`Saved workflow: ${sanitizeLine(params.scriptPath)}`); + const label = isGeneratedWorkflowScriptPath(config, params.scriptPath) + ? 'Generated workflow script' + : 'Saved workflow'; + lines.push(`${label}: ${sanitizeLine(params.scriptPath)}`); } else { lines.push('Workflow: (the script declares no meta block)'); } @@ -904,9 +930,9 @@ export class WorkflowTool extends BaseDeclarativeTool< const hasPath = typeof params.scriptPath === 'string' && params.scriptPath.length > 0; // XOR: inline `script` (LLM authoring) or `scriptPath` (a saved-workflow - // slash command), never both, never neither. + // slash command or a generated script), never both, never neither. if (!hasScript && !hasPath) { - return 'WorkflowTool: provide `script` (inline source) or `scriptPath` (a saved workflow file).'; + return 'WorkflowTool: provide `script` (inline source) or `scriptPath` (a workflow script file).'; } if (hasScript && hasPath) { return 'WorkflowTool: provide exactly one of `script` or `scriptPath`, not both.'; From b5f0eb63040c5ae95b7df60f7578efc0788bc02a Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Tue, 25 Aug 2026 19:44:09 +0000 Subject: [PATCH 4/5] fix(core): classify workflow scriptPath labels by normalized path (#9987) --- .../core/src/tools/workflow/workflow.test.ts | 55 +++++++++++++++++++ packages/core/src/tools/workflow/workflow.ts | 10 ++-- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/core/src/tools/workflow/workflow.test.ts b/packages/core/src/tools/workflow/workflow.test.ts index 951d4cba812..050da254166 100644 --- a/packages/core/src/tools/workflow/workflow.test.ts +++ b/packages/core/src/tools/workflow/workflow.test.ts @@ -400,6 +400,61 @@ await agent('scan package.json') expect(details.prompt).toContain(`Saved workflow: ${scriptPath}`); }); + // The loader canonicalizes `scriptPath` with realpath, so a `..`-laced + // path can load a file far from its raw spelling. Classifying the raw + // string would show the opposite identity from what actually loads. + it('classifies a ..-laced scriptPath by its normalized location', async () => { + const storage = new Storage( + path.join(os.tmpdir(), 'workflow-label-test'), + ); + const config = { storage } as unknown as Config; + // Raw string sits inside the generated root; the `..` segments climb + // out of it, so the normalized path is no longer under the root. + const scriptPath = [ + storage.getGeneratedWorkflowsDir(), + '..', + '..', + '..', + '..', + 'workflows', + 'audit.js', + ].join(path.sep); + const tool = new WorkflowTool(config); + expect(tool.build({ scriptPath }).getDescription()).toBe( + 'Run saved workflow (audit.js)', + ); + const details = (await detailsFor({ scriptPath }, config)) as { + prompt: string; + }; + expect(details.prompt).toContain(`Saved workflow: ${scriptPath}`); + }); + + // The loader trusts the whole subtree under the generated root (writers + // nest per session), so the label must follow nested scripts too, not + // just files sitting directly under the root. + it('labels a nested generated-root scriptPath as a generated script', async () => { + const storage = new Storage( + path.join(os.tmpdir(), 'workflow-label-test'), + ); + const config = { storage } as unknown as Config; + const scriptPath = path.join( + storage.getGeneratedWorkflowsDir(), + 's-abc', + 'fanout.js', + ); + const tool = new WorkflowTool(config); + expect(tool.build({ scriptPath }).getDescription()).toBe( + 'Run generated workflow script (fanout.js)', + ); + const details = (await detailsFor({ scriptPath }, config)) as { + prompt: string; + }; + expect(details.prompt).toContain( + `Generated workflow script: ${scriptPath}`, + ); + expect(details.prompt).not.toContain('Saved workflow'); + }); + // The cost warning used to arrive only after a successful run — i.e. // after the spend it warns about, and never at all on the failure path. it('warns about token cost before the spend, exactly once', async () => { diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index 6e15d1bae6d..eecd8fa5b8d 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -24,6 +24,7 @@ import { type ToolLocation, } from '../tools.js'; import { stripAnsiAndControl } from '../../utils/textUtils.js'; +import { isWithinRoot } from '../../utils/fileUtils.js'; import { extractAndStripMeta, type WorkflowMeta, @@ -647,15 +648,16 @@ function readMetaForConfirmation(script: string): WorkflowMeta | null { * True when a `scriptPath` points inside the generated-scripts root. Such a * script is a throwaway artifact a tool emitted for this run, not a workflow * the user saved — the approval surface labels it accordingly so a grant is - * given under the right identity. Lexical match on purpose: this only picks - * a label; the security check is the realpath boundary in the loader. + * given under the right identity. Both sides are resolved before comparing, + * so a `..`-laced path is classified where it canonically points — the same + * location the loader decides to load. This only picks a label; the security + * check is the realpath boundary in the loader. */ function isGeneratedWorkflowScriptPath( config: Config, scriptPath: string, ): boolean { - const root = config.storage.getGeneratedWorkflowsDir(); - return scriptPath === root || scriptPath.startsWith(root + path.sep); + return isWithinRoot(scriptPath, config.storage.getGeneratedWorkflowsDir()); } /** From a25c7b13f01c481632070b36d23da931072a9fab Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Tue, 25 Aug 2026 21:45:26 +0000 Subject: [PATCH 5/5] fix(core): classify workflow confirmation labels by canonical path (#9987) --- .../core/src/agents/runtime/workflow-saved.ts | 4 +- .../core/src/tools/workflow/workflow.test.ts | 111 ++++++++++++++---- packages/core/src/tools/workflow/workflow.ts | 72 ++++++++++-- 3 files changed, 151 insertions(+), 36 deletions(-) diff --git a/packages/core/src/agents/runtime/workflow-saved.ts b/packages/core/src/agents/runtime/workflow-saved.ts index 27b528991f4..7c50eb071fe 100644 --- a/packages/core/src/agents/runtime/workflow-saved.ts +++ b/packages/core/src/agents/runtime/workflow-saved.ts @@ -107,7 +107,7 @@ export function getWorkflowScriptRoots(config: Config): string[] { } /** - * True when a saved-workflow root dir is itself a symlink. `readWorkflowFileSecurely` + * True when a workflow script root dir is itself a symlink. `readWorkflowFileSecurely` * realpaths the root so it can tolerate symlinked *ancestors* (e.g. a project under * macOS `/tmp -> /private/tmp`); but that same laundering turns a checked-in * `.qwen/workflows -> /outside` link into the allowed boundary — letting discovery @@ -117,7 +117,7 @@ export function getWorkflowScriptRoots(config: Config): string[] { * all three operations. A missing dir (the common case) is not a symlink, so this * is transparent until someone actually links the dir. */ -async function isSymlinkedRoot(dir: string): Promise { +export async function isSymlinkedRoot(dir: string): Promise { return fs .lstat(dir) .then((st) => st.isSymbolicLink()) diff --git a/packages/core/src/tools/workflow/workflow.test.ts b/packages/core/src/tools/workflow/workflow.test.ts index 050da254166..1a5386f8a55 100644 --- a/packages/core/src/tools/workflow/workflow.test.ts +++ b/packages/core/src/tools/workflow/workflow.test.ts @@ -51,11 +51,12 @@ function configWithRegistry(): { /** * The scriptPath approval/description surface classifies the path against * the generated-scripts root, so it needs a config with a real `storage`. + * The `storage` handle comes back too — the label tests derive their + * expected roots from it (same shape as {@link configWithRegistry}). */ -function configWithStorage(): Config { - return { - storage: new Storage(path.join(os.tmpdir(), 'workflow-label-test')), - } as unknown as Config; +function configWithStorage(): { config: Config; storage: Storage } { + const storage = new Storage(path.join(os.tmpdir(), 'workflow-label-test')); + return { config: { storage } as unknown as Config, storage }; } describe('WorkflowTool', () => { @@ -326,7 +327,7 @@ await agent('scan package.json') it('scopes a saved-workflow grant to the path that was approved', async () => { const details = (await detailsFor( { scriptPath: '/home/u/.qwen/workflows/audit.js' }, - configWithStorage(), + configWithStorage().config, )) as { hideAlwaysAllow?: boolean; permissionRules?: string[] }; expect(details.hideAlwaysAllow).toBeFalsy(); expect(details.permissionRules).toHaveLength(1); @@ -360,10 +361,7 @@ await agent('scan package.json') // run. Labeling it as a saved workflow would have the user approve — and // maybe pre-approve the path rule — under a wrong identity. it('labels a generated-root scriptPath as a generated script, not a saved workflow', async () => { - const storage = new Storage( - path.join(os.tmpdir(), 'workflow-label-test'), - ); - const config = { storage } as unknown as Config; + const { config, storage } = configWithStorage(); const scriptPath = path.join( storage.getGeneratedWorkflowsDir(), 'fanout-1a2b3c.js', @@ -382,10 +380,7 @@ await agent('scan package.json') }); it('keeps the saved-workflow label for a saved-root scriptPath', async () => { - const storage = new Storage( - path.join(os.tmpdir(), 'workflow-label-test'), - ); - const config = { storage } as unknown as Config; + const { config, storage } = configWithStorage(); const scriptPath = path.join( storage.getProjectWorkflowsDir(), 'deep-research.js', @@ -404,10 +399,7 @@ await agent('scan package.json') // path can load a file far from its raw spelling. Classifying the raw // string would show the opposite identity from what actually loads. it('classifies a ..-laced scriptPath by its normalized location', async () => { - const storage = new Storage( - path.join(os.tmpdir(), 'workflow-label-test'), - ); - const config = { storage } as unknown as Config; + const { config, storage } = configWithStorage(); // Raw string sits inside the generated root; the `..` segments climb // out of it, so the normalized path is no longer under the root. const scriptPath = [ @@ -433,10 +425,7 @@ await agent('scan package.json') // nest per session), so the label must follow nested scripts too, not // just files sitting directly under the root. it('labels a nested generated-root scriptPath as a generated script', async () => { - const storage = new Storage( - path.join(os.tmpdir(), 'workflow-label-test'), - ); - const config = { storage } as unknown as Config; + const { config, storage } = configWithStorage(); const scriptPath = path.join( storage.getGeneratedWorkflowsDir(), 's-abc', @@ -455,6 +444,84 @@ await agent('scan package.json') expect(details.prompt).not.toContain('Saved workflow'); }); + // The loader decides loadability with `fs.realpath`, so a scriptPath + // whose spelling diverges from its realpath (a symlink crossing roots) + // must be labeled by the content that actually loads — classifying the + // raw string shows the opposite identity from what runs. + it('labels a symlinked scriptPath by the content that loads', async () => { + const projectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'wf-lbl-')); + const runtimeDir = await fs.mkdtemp(path.join(os.tmpdir(), 'wf-lbl-rt-')); + try { + const storage = new Storage(projectDir, runtimeDir); + const config = { storage } as unknown as Config; + const generatedDir = storage.getGeneratedWorkflowsDir(); + const savedDir = storage.getProjectWorkflowsDir(); + await fs.mkdir(generatedDir, { recursive: true }); + await fs.mkdir(savedDir, { recursive: true }); + + // A generated-spelled link whose realpath is a saved workflow. + await fs.writeFile(path.join(savedDir, 'deploy.js'), 'return 1;'); + const generatedSpelling = path.join(generatedDir, 'run.js'); + await fs.symlink(path.join(savedDir, 'deploy.js'), generatedSpelling); + + // A saved-spelled link whose realpath is a generated script. + const nested = path.join(generatedDir, 's-abc'); + await fs.mkdir(nested, { recursive: true }); + await fs.writeFile(path.join(nested, 'throwaway.js'), 'return 1;'); + const savedSpelling = path.join(savedDir, 'toolgen.js'); + await fs.symlink(path.join(nested, 'throwaway.js'), savedSpelling); + + const savedLoaded = (await detailsFor( + { scriptPath: generatedSpelling }, + config, + )) as { prompt: string }; + expect(savedLoaded.prompt).toContain( + `Saved workflow: ${generatedSpelling}`, + ); + expect(savedLoaded.prompt).not.toContain('Generated workflow script'); + + const generatedLoaded = (await detailsFor( + { scriptPath: savedSpelling }, + config, + )) as { prompt: string }; + expect(generatedLoaded.prompt).toContain( + `Generated workflow script: ${savedSpelling}`, + ); + expect(generatedLoaded.prompt).not.toContain('Saved workflow'); + } finally { + await fs.rm(projectDir, { recursive: true, force: true }); + await fs.rm(runtimeDir, { recursive: true, force: true }); + } + }); + + // The loader refuses a symlinked generated-scripts root outright, so the + // dialog must not claim a generated identity for a path under one — the + // content it appears to name will not load as a generated script. + it('labels nothing generated through a symlinked generated root', async () => { + const projectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'wf-lbl-')); + const runtimeDir = await fs.mkdtemp(path.join(os.tmpdir(), 'wf-lbl-rt-')); + const external = await fs.mkdtemp(path.join(os.tmpdir(), 'wf-lbl-ext-')); + try { + const storage = new Storage(projectDir, runtimeDir); + const config = { storage } as unknown as Config; + const generatedDir = storage.getGeneratedWorkflowsDir(); + await fs.mkdir(path.dirname(generatedDir), { recursive: true }); + await fs.writeFile(path.join(external, 'leak.js'), 'return 1;'); + await fs.symlink(external, generatedDir, 'dir'); + const scriptPath = path.join(generatedDir, 'leak.js'); + + const details = (await detailsFor({ scriptPath }, config)) as { + prompt: string; + }; + expect(details.prompt).toContain(`Saved workflow: ${scriptPath}`); + expect(details.prompt).not.toContain('Generated workflow script'); + } finally { + await fs.rm(projectDir, { recursive: true, force: true }); + await fs.rm(runtimeDir, { recursive: true, force: true }); + await fs.rm(external, { recursive: true, force: true }); + } + }); + // The cost warning used to arrive only after a successful run — i.e. // after the spend it warns about, and never at all on the failure path. it('warns about token cost before the spend, exactly once', async () => { @@ -562,7 +629,7 @@ await agent('scan package.json') }); it('build() accepts a scriptPath without inline script', () => { - const tool = new WorkflowTool(configWithStorage()); + const tool = new WorkflowTool(configWithStorage().config); const invocation = tool.build({ scriptPath: '/abs/deep-research.js', }); diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index eecd8fa5b8d..98e1999809b 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -63,6 +63,8 @@ import { WorkflowScriptNotLaunchedError, type WorkflowRunHandle, } from '../../agents/runtime/workflow-runner.js'; +import { isSymlinkedRoot } from '../../agents/runtime/workflow-saved.js'; +import { promises as fs } from 'node:fs'; import * as path from 'node:path'; import type { WorkflowTask } from '../../agents/workflow-run-registry.js'; @@ -301,11 +303,23 @@ class WorkflowToolInvocation extends BaseToolInvocation< * pre-approved — but scoped to that path. The rule is built with the * same helpers the matcher uses so a tool rename moves both sides. */ - override getConfirmationDetails( + override async getConfirmationDetails( _abortSignal: AbortSignal, ): Promise { const meta = this.resolveMeta(); - const body = buildConfirmationPrompt(this.params, meta, this.config); + // The consent surface classifies canonically (the loader's own + // normalization) so the label matches the content that actually loads. + const isGeneratedScriptPath = + this.params.scriptPath !== undefined && + (await isGeneratedWorkflowScriptPathCanonical( + this.config, + this.params.scriptPath, + )); + const body = buildConfirmationPrompt( + this.params, + meta, + isGeneratedScriptPath, + ); // The cost warning belongs before the spend, not after it. The registry // latch flips on read, so surfacing it here means the post-hoc copy on @@ -337,7 +351,7 @@ class WorkflowToolInvocation extends BaseToolInvocation< // No-op: persistence is handled by coreToolScheduler via PM rules. }, }; - return Promise.resolve(details); + return details; } override async execute( @@ -647,11 +661,13 @@ function readMetaForConfirmation(script: string): WorkflowMeta | null { /** * True when a `scriptPath` points inside the generated-scripts root. Such a * script is a throwaway artifact a tool emitted for this run, not a workflow - * the user saved — the approval surface labels it accordingly so a grant is - * given under the right identity. Both sides are resolved before comparing, - * so a `..`-laced path is classified where it canonically points — the same - * location the loader decides to load. This only picks a label; the security - * check is the realpath boundary in the loader. + * the user saved — the transcript surface labels it accordingly. Normalizes + * `..` lexically only (no disk I/O, so it can back the synchronous + * `getDescription()`); where a spelling diverges from its realpath (a + * symlinked file, or a symlinked ancestor) it can disagree with the content + * the loader reads — the confirmation dialog therefore classifies with + * {@link isGeneratedWorkflowScriptPathCanonical} instead. This only picks a + * label; the security check is the realpath boundary in the loader. */ function isGeneratedWorkflowScriptPath( config: Config, @@ -660,17 +676,49 @@ function isGeneratedWorkflowScriptPath( return isWithinRoot(scriptPath, config.storage.getGeneratedWorkflowsDir()); } +/** + * Canonical provenance classification for the confirmation dialog: the same + * normalization the loader applies, so the label matches the content that + * actually loads. Both sides go through `fs.realpath` (resolving `..` AND + * symlinks), with a lexical fallback where a side does not exist yet — and a + * symlinked generated root counts as not-generated because the loader + * refuses it outright. + */ +async function isGeneratedWorkflowScriptPathCanonical( + config: Config, + scriptPath: string, +): Promise { + const root = config.storage.getGeneratedWorkflowsDir(); + if (await isSymlinkedRoot(root)) return false; + let realScriptPath: string; + try { + realScriptPath = await fs.realpath(scriptPath); + } catch { + // Nothing loads under a spelling that is not on disk, so classify the + // raw string the same way the synchronous surface does. + return isWithinRoot(scriptPath, root); + } + let realRoot: string; + try { + realRoot = await fs.realpath(root); + } catch { + realRoot = path.resolve(root); + } + return isWithinRoot(realScriptPath, realRoot); +} + /** * The body of the approval dialog: what this workflow says it will do. * * Everything here comes from `meta`, the call's own parameters, and the - * configured script roots — never from executing the script. When `meta` is - * absent or unreadable the dialog still renders, just with less to say. + * caller's provenance classification — never from executing the script. + * When `meta` is absent or unreadable the dialog still renders, just with + * less to say. */ function buildConfirmationPrompt( params: WorkflowParams, meta: WorkflowMeta | null, - config: Config, + isGeneratedScriptPath: boolean, ): string { const lines: string[] = []; @@ -678,7 +726,7 @@ function buildConfirmationPrompt( lines.push(`Workflow: ${sanitizeLine(meta.name)}`); lines.push(sanitizeLine(meta.description)); } else if (params.scriptPath) { - const label = isGeneratedWorkflowScriptPath(config, params.scriptPath) + const label = isGeneratedScriptPath ? 'Generated workflow script' : 'Saved workflow'; lines.push(`${label}: ${sanitizeLine(params.scriptPath)}`);