root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(review): strip approval-bypass flags in agent workflow authoring for chat/planning lanes

Browse Source
This commit is contained in:
gsxdsm
2026-06-05 01:05:51 -07:00
parent bd3f26e82a
commit a47ea3f7a1
9 changed files with 269 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

79
packages/core/src/__tests__/strip-approval-bypass-flags.test.ts Normal file
View File

@@ -0,0 +1,79 @@
import { describe, it, expect } from "vitest";
import { stripApprovalBypassFlags } from "../workflow-ir.js";
import type { WorkflowIr } from "../workflow-ir-types.js";
/**
* P0 security helper: removes the CLI-approval-bypass flags
* (`cliSkipApproval`/`autoApprove`) from every node config, recursing into
* foreach `config.template.nodes` at any nesting depth.
*/
describe("stripApprovalBypassFlags", () => {
it("removes both flags from a top-level node config and reports stripped:true", () => {
const ir = {
version: "v1",
name: "wf",
nodes: [{ id: "n1", kind: "prompt", config: { cliSkipApproval: true, autoApprove: true, name: "x" } }],
edges: [],
} as unknown as WorkflowIr;
const { ir: out, stripped } = stripApprovalBypassFlags(ir);
expect(stripped).toBe(true);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const cfg = (out as any).nodes[0].config;
expect(cfg.cliSkipApproval).toBeUndefined();
expect(cfg.autoApprove).toBeUndefined();
expect(cfg.name).toBe("x"); // unrelated config preserved
});
it("strips nested foreach-in-foreach template nodes (arbitrary depth)", () => {
const ir = {
version: "v1",
name: "wf",
nodes: [
{
id: "outer",
kind: "foreach",
config: {
template: {
nodes: [
{
id: "inner-foreach",
kind: "foreach",
config: {
template: {
nodes: [
{ id: "deep", kind: "step-execute", config: { autoApprove: true } },
],
edges: [],
},
},
},
],
edges: [],
},
},
},
],
edges: [],
} as unknown as WorkflowIr;
const { stripped } = stripApprovalBypassFlags(ir);
expect(stripped).toBe(true);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const deep = (ir as any).nodes[0].config.template.nodes[0].config.template.nodes[0];
expect(deep.config.autoApprove).toBeUndefined();
});
it("returns stripped:false when no flags present", () => {
const ir = {
version: "v1",
name: "wf",
nodes: [{ id: "n1", kind: "prompt", config: { name: "x" } }],
edges: [],
} as unknown as WorkflowIr;
expect(stripApprovalBypassFlags(ir).stripped).toBe(false);
});
it("tolerates a non-array nodes field", () => {
const ir = { version: "v1", name: "wf" } as unknown as WorkflowIr;
expect(stripApprovalBypassFlags(ir).stripped).toBe(false);
});
});

1
packages/core/src/index.ts
View File

@@ -48,6 +48,7 @@ export {
export {
parseWorkflowIr,
serializeWorkflowIr,
stripApprovalBypassFlags,
WorkflowIrError,
DEFAULT_WORKFLOW_COLUMN_IDS,
} from "./workflow-ir.js";

36
packages/core/src/workflow-ir.ts
View File

@@ -906,3 +906,39 @@ export function downgradeIrToV1IfPure(ir: WorkflowIr): WorkflowIr {
export function serializeWorkflowIr(ir: WorkflowIr): string {
return JSON.stringify(ir, null, 2);
}
/**
* Strip the trust-escalating `cliSkipApproval`/`autoApprove` flags from every
* node config in an IR, recursing into foreach `config.template.nodes` at any
* nesting depth (foreach-in-foreach). Mutates the passed IR in place and returns
* it alongside a `stripped` flag indicating whether anything was removed.
*
* These flags bypass the CLI first-run approval gate (see executor.ts). They are
* legitimate only for workflows authored through the trusted dashboard editor /
* executor lane; on prompt-injectable surfaces (chat/planning authoring tools,
* import, AI design) they must be removed at the write boundary.
*/
export function stripApprovalBypassFlags(ir: WorkflowIr): { ir: WorkflowIr; stripped: boolean } {
const nodes = (ir as { nodes?: WorkflowIrNode[] }).nodes;
if (!Array.isArray(nodes)) return { ir, stripped: false };
let stripped = false;
const stripNode = (node: WorkflowIrNode): void => {
const cfg = node.config as Record<string, unknown> | undefined;
if (cfg && typeof cfg === "object") {
if ("cliSkipApproval" in cfg) {
delete cfg.cliSkipApproval;
stripped = true;
}
if ("autoApprove" in cfg) {
delete cfg.autoApprove;
stripped = true;
}
const template = (cfg as { template?: { nodes?: unknown } }).template;
if (template && Array.isArray(template.nodes)) {
for (const inner of template.nodes as WorkflowIrNode[]) stripNode(inner);
}
}
};
for (const node of nodes) stripNode(node);
return { ir, stripped };
}

2
packages/dashboard/src/chat.ts
View File

@@ -1616,7 +1616,7 @@ export class ChatManager {
// is available. The chat lane has no ambient task, so fn_workflow_select
// has no default target — an agent must pass an explicit task_id.
const workflowTools = this.taskStore
? createWorkflowAuthoringTools(this.taskStore, "")
? createWorkflowAuthoringTools(this.taskStore, "", { stripApprovalFlags: true })
: [];
const customTools = [...messagingTools, ...workflowTools];

4
packages/dashboard/src/planning.ts
View File

@@ -848,7 +848,7 @@ export async function createSession(
builtinToolsAllowlist: [...PLANNING_BUILTIN_WEB_TOOLS],
customTools: [
...createPlanningBoardTools(store),
...createWorkflowAuthoringTools(store, PLANNING_NO_AMBIENT_TASK_ID),
...createWorkflowAuthoringTools(store, PLANNING_NO_AMBIENT_TASK_ID, { stripApprovalFlags: true }),
],
onThinking: () => {
// Non-streaming path ignores thinking output
@@ -1392,7 +1392,7 @@ async function createPlanningAgent(
builtinToolsAllowlist: [...PLANNING_BUILTIN_WEB_TOOLS],
customTools: [
...createPlanningBoardTools(store),
...createWorkflowAuthoringTools(store, PLANNING_NO_AMBIENT_TASK_ID),
...createWorkflowAuthoringTools(store, PLANNING_NO_AMBIENT_TASK_ID, { stripApprovalFlags: true }),
],
...(modelProvider && modelId
? {

31
packages/dashboard/src/routes/register-workflow-routes.ts
View File

@@ -1,5 +1,5 @@
import type { WorkflowDefinition, WorkflowDefinitionKind, WorkflowIr, WorkflowIrNode } from "@fusion/core";
import { ColumnTraitValidationError, OccupiedColumnsError, InvalidRehomeTargetError, WorkflowCompileError, WorkflowIrError, SCHEMA_VERSION, assertColumnTraitsValid, compileWorkflowToSteps, layoutForIr, listTraits, listStepParsers, parseWorkflowIr, resolvePlanningSettingsModel } from "@fusion/core";
import { ColumnTraitValidationError, OccupiedColumnsError, InvalidRehomeTargetError, WorkflowCompileError, WorkflowIrError, SCHEMA_VERSION, assertColumnTraitsValid, compileWorkflowToSteps, layoutForIr, listTraits, listStepParsers, parseWorkflowIr, resolvePlanningSettingsModel, stripApprovalBypassFlags } from "@fusion/core";
import { createFnAgent as engineCreateFnAgent, validateCodeNodeSources } from "@fusion/engine";
import { ApiError, badRequest, conflict, notFound, rateLimited } from "../api-error.js";
import { emitWorkflowSseEvent } from "../sse.js";
@@ -755,31 +755,12 @@ function assertImportTraitsValid(ir: WorkflowIr): void {
}
/** Strip `cliSkipApproval`/`autoApprove` from every node config in the IR,
* including configs nested inside foreach `template.nodes`. Returns true when
* anything was removed so the response can flag it (R10 trust boundary). */
* including configs nested inside foreach `template.nodes` (any depth). Returns
* true when anything was removed so the response can flag it (R10 trust
* boundary). Delegates to the shared @fusion/core helper so the route and the
* chat/planning authoring tools cannot diverge. */
function stripApprovalFlags(ir: WorkflowIr): boolean {
const nodes = (ir as { nodes?: WorkflowIrNode[] }).nodes;
if (!Array.isArray(nodes)) return false;
let stripped = false;
const stripNode = (node: WorkflowIrNode): void => {
const cfg = node.config as Record<string, unknown> | undefined;
if (cfg && typeof cfg === "object") {
if ("cliSkipApproval" in cfg) {
delete cfg.cliSkipApproval;
stripped = true;
}
if ("autoApprove" in cfg) {
delete cfg.autoApprove;
stripped = true;
}
const template = (cfg as { template?: { nodes?: unknown } }).template;
if (template && Array.isArray(template.nodes)) {
for (const inner of template.nodes as WorkflowIrNode[]) stripNode(inner);
}
}
};
for (const node of nodes) stripNode(node);
return stripped;
return stripApprovalBypassFlags(ir).stripped;
}
/** Collect non-blocking warnings for script nodes (and any config carrying a

90
packages/engine/src/__tests__/agent-workflow-tools-exposure.test.ts
View File

@@ -54,3 +54,93 @@ describe("workflow tool exposure (engine factories)", () => {
expect(createWorkflowDeleteTool(fakeStore).name).toBe("fn_workflow_delete");
});
});
/**
* P0 security: the chat/planning lanes pass {stripApprovalFlags:true} so the
* create/update tools cannot persist a CLI-approval-bypass smuggled through a
* prompt-injectable agent lane. The executor lane omits the option (project-
* owner escape hatch) and the flags pass through unchanged.
*/
describe("workflow authoring tools approval-flag stripping", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function captureStore(): { store: TaskStore; captured: { ir?: any } } {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const captured: { ir?: any } = {};
const store = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
createWorkflowDefinition: async (input: any) => {
captured.ir = input.ir;
return { id: "wf-1", name: input.name };
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
updateWorkflowDefinition: async (_id: string, input: any) => {
captured.ir = input.ir;
return { id: "wf-1", name: input.name ?? "wf" };
},
} as unknown as TaskStore;
return { store, captured };
}
// The ToolDefinition.execute signature requires (id, params, signal, onUpdate,
// ctx); tests only need id+params, so pass undefined for the rest and cast the
// result to read the optional `isError`/`content` shape these tools return.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const run = (tool: { execute: (...a: any[]) => Promise<any> }, params: unknown) =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
tool.execute("call-1", params, undefined, undefined, undefined) as Promise<{
// eslint-disable-next-line @typescript-eslint/no-explicit-any
isError?: boolean; content: { type: string; text?: string }[];
}>;
const irWithFlags = () => ({
version: "v1" as const,
name: "wf",
nodes: [
{ id: "n1", kind: "prompt", config: { cliSkipApproval: true, name: "x" } },
{
id: "fe",
kind: "foreach",
config: {
template: {
nodes: [
{ id: "inner", kind: "step-execute", config: { autoApprove: true } },
],
edges: [],
},
},
},
],
edges: [],
});
it("strips both flags (incl. foreach template) on create with stripApprovalFlags:true", async () => {
const { store, captured } = captureStore();
const tool = createWorkflowCreateTool(store, { stripApprovalFlags: true });
const res = await run(tool, { name: "wf", ir: irWithFlags() });
expect(res.isError).toBeFalsy();
expect(captured.ir.nodes[0].config.cliSkipApproval).toBeUndefined();
expect(captured.ir.nodes[1].config.template.nodes[0].config.autoApprove).toBeUndefined();
const text = res.content.map((c) => c.text ?? "").join("");
expect(text).toContain("approval-bypass flags removed");
});
it("strips flags on update with stripApprovalFlags:true", async () => {
const { store, captured } = captureStore();
const tool = createWorkflowUpdateTool(store, { stripApprovalFlags: true });
const res = await run(tool, { workflow_id: "wf-1", ir: irWithFlags() });
expect(res.isError).toBeFalsy();
expect(captured.ir.nodes[0].config.cliSkipApproval).toBeUndefined();
expect(captured.ir.nodes[1].config.template.nodes[0].config.autoApprove).toBeUndefined();
});
it("executor lane (no option) passes both flags through unchanged", async () => {
const { store, captured } = captureStore();
const tool = createWorkflowCreateTool(store);
const res = await run(tool, { name: "wf", ir: irWithFlags() });
expect(res.isError).toBeFalsy();
expect(captured.ir.nodes[0].config.cliSkipApproval).toBe(true);
expect(captured.ir.nodes[1].config.template.nodes[0].config.autoApprove).toBe(true);
const text = res.content.map((c) => c.text ?? "").join("");
expect(text).not.toContain("approval-bypass flags removed");
});
});

56
packages/engine/src/agent-tools.ts
View File

@@ -12,7 +12,7 @@ import { existsSync } from "node:fs";
import { createHash } from "node:crypto";
import { join, relative, resolve } from "node:path";
import type { AgentStore, AgentState, AgentCapability, AgentUpdateInput, TaskDocument, TaskDocumentCreateInput, TaskStore, RunMutationContext, MessageStore, Message, SourceType, Settings, ResearchRun, ResearchRunStatus, TaskCreateInput, ReflectionStore, ApprovalRequestStore, ProjectSettings, ChatStore } from "@fusion/core";
import { listTraits, isBuiltinWorkflowId } from "@fusion/core";
import { listTraits, isBuiltinWorkflowId, stripApprovalBypassFlags } from "@fusion/core";
import { promoteHeldTask } from "./hold-release.js";
import { DASHBOARD_USER_ID, canAgentTakeImplementationTaskForExplicitRouting, dailyMemoryPath, ensureOpenClawMemoryFiles, extractAgentProvisioningRequest, formatRoleMismatchReason, getMemoryBackendCapabilities, getProjectMemory, isEphemeralAgent, memoryLongTermPath, normalizeMessageParticipant, reconcileDeterministicDuplicate, resolveAgentProvisioningPolicy, resolveMemoryBackend, resolveResearchSettings, resolveTaskGithubTracking, resolveTitleSummarizerSettingsModel, runDeterministicDuplicateGuard, scheduleQmdProjectMemoryRefresh, searchProjectMemory, shouldSkipBackgroundQmdRefresh, summarizeTitle } from "@fusion/core";
import { ResearchOrchestrator } from "./research-orchestrator.js";
@@ -1201,7 +1201,10 @@ export function createTaskPromoteTool(store: TaskStore, currentTaskId: string):
* Create a `fn_workflow_create` tool — a thin wrapper over the store's workflow
* definition create. The IR is validated server-side; a malformed graph rejects.
*/
export function createWorkflowCreateTool(store: TaskStore): ToolDefinition {
export function createWorkflowCreateTool(
store: TaskStore,
opts?: WorkflowAuthoringToolOptions,
): ToolDefinition {
return {
name: "fn_workflow_create",
label: "Create Workflow",
@@ -1226,16 +1229,23 @@ export function createWorkflowCreateTool(store: TaskStore): ToolDefinition {
parameters: workflowCreateParams,
execute: async (_id: string, params: Static<typeof workflowCreateParams>) => {
try {
let approvalNote = "";
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let ir = params.ir as any;
if (opts?.stripApprovalFlags) {
const result = stripApprovalBypassFlags(ir);
ir = result.ir;
if (result.stripped) approvalNote = " (approval-bypass flags removed)";
}
const created = await store.createWorkflowDefinition({
name: params.name,
description: params.description,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
ir: params.ir as any,
ir,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
layout: params.layout as any,
});
return {
content: [{ type: "text" as const, text: `Created workflow ${created.id} (${created.name}).` }],
content: [{ type: "text" as const, text: `Created workflow ${created.id} (${created.name}).${approvalNote}` }],
details: { workflowId: created.id, name: created.name },
};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -1256,7 +1266,10 @@ export function createWorkflowCreateTool(store: TaskStore): ToolDefinition {
* throws an OccupiedColumnsError; we surface it as a structured response carrying
* the per-column occupant counts so the agent can retry with `rehome_to`.
*/
export function createWorkflowUpdateTool(store: TaskStore): ToolDefinition {
export function createWorkflowUpdateTool(
store: TaskStore,
opts?: WorkflowAuthoringToolOptions,
): ToolDefinition {
return {
name: "fn_workflow_update",
label: "Update Workflow",
@@ -1270,17 +1283,24 @@ export function createWorkflowUpdateTool(store: TaskStore): ToolDefinition {
parameters: workflowUpdateParams,
execute: async (_id: string, params: Static<typeof workflowUpdateParams>) => {
try {
let approvalNote = "";
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let ir = params.ir as any;
if (opts?.stripApprovalFlags && ir !== undefined && ir !== null) {
const result = stripApprovalBypassFlags(ir);
ir = result.ir;
if (result.stripped) approvalNote = " (approval-bypass flags removed)";
}
const updated = await store.updateWorkflowDefinition(params.workflow_id, {
name: params.name,
description: params.description,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
ir: params.ir as any,
ir,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
layout: params.layout as any,
rehomeTo: params.rehome_to,
});
return {
content: [{ type: "text" as const, text: `Updated workflow ${updated.id} (${updated.name}).` }],
content: [{ type: "text" as const, text: `Updated workflow ${updated.id} (${updated.name}).${approvalNote}` }],
details: { workflowId: updated.id, name: updated.name },
};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -1410,16 +1430,30 @@ export function createTraitListTool(): ToolDefinition {
* the default task for `fn_workflow_select`; lanes with no ambient task pass a
* placeholder (an agent can still target any task via the `task_id` param).
*/
/**
* Options for the workflow authoring tool set.
*
* `stripApprovalFlags` removes the `cliSkipApproval`/`autoApprove` approval-
* bypass flags from every node config (incl. nested foreach templates) before
* the create/update tools persist the IR. Chat and planning lanes are prompt-
* injectable, so they pass `true`; the executor lane (project-owner escape
* hatch) omits it so the dashboard editor can deliberately set those flags.
*/
export interface WorkflowAuthoringToolOptions {
stripApprovalFlags?: boolean;
}
export function createWorkflowAuthoringTools(
store: TaskStore,
currentTaskId: string,
opts?: WorkflowAuthoringToolOptions,
): ToolDefinition[] {
return [
createWorkflowListTool(store),
createWorkflowGetTool(store),
createWorkflowSelectTool(store, currentTaskId),
createWorkflowCreateTool(store),
createWorkflowUpdateTool(store),
createWorkflowCreateTool(store, opts),
createWorkflowUpdateTool(store, opts),
createWorkflowDeleteTool(store),
createTraitListTool(),
];

12
packages/engine/src/executor.ts
View File

@@ -4575,9 +4575,15 @@ export class TaskExecutor {
//
// SECURITY: both flags are intentional project-owner-only escape hatches.
// They are only reachable by someone who can author/edit a workflow
// definition for this project — the same trust boundary that already
// lets them add named scripts. They are NOT untrusted-input surfaces,
// and neither is enforced at the IR-validation layer.
// definition for this project through the trusted dashboard editor /
// executor lane — the same trust boundary that already lets them add
// named scripts. They are NOT enforced at the IR-validation layer.
// Prompt-injectable surfaces strip these flags at the write boundary
// before persisting: the import / AI-design routes (stripApprovalFlags
// in register-workflow-routes.ts) and the chat/planning workflow
// authoring tools (createWorkflowAuthoringTools(..., {stripApprovalFlags:
// true}) in chat.ts / planning.ts) — all via stripApprovalBypassFlags in
// @fusion/core. Only the executor lane keeps these flags intact.
const skipApproval = cfg.cliSkipApproval === true || cfg.autoApprove === true;
if (!skipApproval && !(await this.store.isWorkflowCliCommandApproved(rawCommand))) {
return this.pauseForCliApproval(node, live, rawCommand);