feat(FN-4366): enforce readonly tool allowlist for workflow steps

Added a readonly tool allowlist enforcement for workflow steps, blocking execution of state-mutating tools (`fn_task_update`, `fn_task_move`, etc.) during workflow step runs. The policy is wired into the executor and merger execution paths, with tests covering allowlist enforcement and a documentati

Fusion-Task-Id: FN-4366

packages/engine/src/__tests__/pi-create-fn-agent.test.ts

@@ -1266,7 +1266,7 @@ describe("createFnAgent", () => {
     expect(createSessionArgs.tools).toBeUndefined();
   });
 
-  it("passes opt-in builtin web tool allowlist to createAgentSession", async () => {
+  it("intersects readonly builtin allowlist with readonly policy", async () => {
     const { createFnAgent } = await import("../pi.js");
 
     await createFnAgent({
@@ -1282,9 +1282,9 @@ describe("createFnAgent", () => {
       "grep",
       "find",
       "ls",
-      "WebSearch",
-      "WebFetch",
     ]));
+    expect(createSessionArgs.tools).not.toContain("WebSearch");
+    expect(createSessionArgs.tools).not.toContain("WebFetch");
   });
 
   it("keeps caller customTools in coding sessions", async () => {

packages/engine/src/__tests__/workflow-step-readonly-allowlist.test.ts

@@ -0,0 +1,71 @@
+import { describe, it, expect } from "vitest";
+import {
+  DENIED_IN_READONLY,
+  READONLY_ALLOWLIST,
+  ReadonlyViolationError,
+  filterCustomToolsForReadonly,
+  isReadonlyAllowed,
+} from "../workflow-step-tool-policy.js";
+
+describe("workflow-step readonly allowlist policy", () => {
+  it("exposes expected readonly allowlist", () => {
+    expect(READONLY_ALLOWLIST).toEqual([
+      "read",
+      "grep",
+      "find",
+      "ls",
+      "fn_web_fetch",
+      "fn_task_show",
+      "fn_task_list",
+      "fn_insight_list",
+      "fn_insight_show",
+      "fn_list_agents",
+      "fn_get_agent_config",
+    ]);
+    expect(isReadonlyAllowed("read")).toBe(true);
+    expect(isReadonlyAllowed(" edit ")).toBe(false);
+  });
+
+  it("denies write/mutation tool names and keeps readonly custom tools", () => {
+    expect(DENIED_IN_READONLY).toEqual(expect.arrayContaining<string>([
+      "edit",
+      "write",
+      "bash",
+      "fn_task_create",
+      "fn_spawn_agent",
+      "fn_delegate_task",
+      "fn_update_agent_config",
+      "fn_agent_create",
+      "fn_agent_delete",
+      "fn_task_plan",
+      "fn_mission_create",
+      "fn_mission_delete",
+      "fn_milestone_add",
+      "fn_slice_add",
+      "fn_feature_add",
+      "fn_slice_activate",
+      "fn_feature_link_task",
+      "fn_agent_stop",
+      "fn_agent_start",
+    ]));
+
+    const filtered = filterCustomToolsForReadonly([
+      { name: "read" } as any,
+      { name: "fn_task_list" } as any,
+      { name: "edit" } as any,
+      { name: "fn_task_update" } as any,
+    ]);
+
+    expect(filtered.allowed.map((tool) => tool.name)).toEqual(["read", "fn_task_list"]);
+    expect(filtered.denied).toEqual(["edit"]);
+  });
+
+  it("captures readonly violation error shape", () => {
+    const err = new ReadonlyViolationError("FN-4366", "Frontend UX Design", "edit");
+    expect(err.code).toBe("READONLY_VIOLATION");
+    expect(err.taskId).toBe("FN-4366");
+    expect(err.stepName).toBe("Frontend UX Design");
+    expect(err.toolName).toBe("edit");
+    expect(err.message).toContain("[readonly-violation]");
+  });
+});

packages/engine/src/executor.ts

@@ -68,6 +68,7 @@ import {
 import { buildPromptLayers, collapsePromptLayers } from "./prompt-layers.js";
 import type { AgentReflectionService } from "./agent-reflection.js";
 import { createRunAuditor, generateSyntheticRunId, type EngineRunContext } from "./run-audit.js";
+import { ReadonlyViolationError, filterCustomToolsForReadonly } from "./workflow-step-tool-policy.js";
 import { evaluateSpecStaleness, getPromptPath } from "./spec-staleness.js";
 import {
   createAgentCreateTool,
@@ -735,6 +736,7 @@ export class TaskExecutor {
   private activeStepExecutors = new Map<string, StepSessionExecutor>();
   /** Active pre-merge workflow step sessions per task. */
   private activeWorkflowStepSessions = new Map<string, AgentSession>();
+  private readonlyWorkflowStepAuditDone = false;
   /**
    * Reviewer subagent sessions per task. Reviewers (`reviewer.ts`) create their
    * own AgentSessions that aren't part of `activeSessions`/`activeStepExecutors`,
@@ -6018,6 +6020,7 @@ ${failureFeedback}
     settings: Settings,
     taskEnv?: NodeJS.ProcessEnv,
   ): Promise<WorkflowStepResult | "deferred-paused"> {
+    await this.auditReadonlyWorkflowStepPromptsOnce(task.id);
     // Check if task has enabled workflow steps
     const currentTask = await this.store.getTask(task.id);
     if (!currentTask.enabledWorkflowSteps?.length) return { allPassed: true };
@@ -6550,6 +6553,15 @@ and show an appropriate message to the user.\`
         ? await this.options.agentStore.getAgent(task.assignedAgentId).catch(() => null)
         : null;
       const workflowRuntimeHint = extractRuntimeHint(workflowAgent?.runtimeConfig);
+      const readonlyCustomTools = toolMode === "readonly"
+        ? filterCustomToolsForReadonly([])
+        : { allowed: [] as ToolDefinition[], denied: [] as string[] };
+      if (toolMode === "readonly" && readonlyCustomTools.denied.length > 0) {
+        await this.store.logEntry(
+          task.id,
+          `[readonly-violation] Workflow step '${workflowStep.name}' dropped denied custom tools: ${readonlyCustomTools.denied.join(", ")}`,
+        );
+      }
 
       const { session } = await createResolvedAgentSession({
         sessionPurpose: "executor",
@@ -6566,6 +6578,7 @@ and show an appropriate message to the user.\`
         taskEnv,
         // Skill selection: use assigned agent skills if available, otherwise role fallback
         ...(skillContext.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}),
+        ...(readonlyCustomTools.allowed.length > 0 ? { customTools: readonlyCustomTools.allowed } : {}),
       });
 
       executorLog.log(`${task.id}: workflow step '${workflowStep.name}' using model ${describeModel(session)}${useOverride && attemptLabel === "primary" ? " (workflow step override)" : ""}${attemptLabel === "fallback" ? " (fallback after timeout)" : ""}`);
@@ -6646,6 +6659,15 @@ and show an appropriate message to the user.\`
       } catch (err: unknown) {
         await agentLogger.flush();
         try { session.dispose(); } catch { /* best-effort */ }
+        if ((err instanceof ReadonlyViolationError) || ((err as { code?: string } | null)?.code === "READONLY_VIOLATION")) {
+          const violation = err as ReadonlyViolationError;
+          const deniedTool = violation.toolName || "unknown";
+          await this.store.logEntry(
+            task.id,
+            `[readonly-violation] Workflow step '${workflowStep.name}' attempted denied tool '${deniedTool}'`,
+          );
+          return { success: false, error: `[readonly-violation] ${violation.message}` };
+        }
         const errorMessage = err instanceof Error ? err.message : String(err);
         return { success: false, error: errorMessage };
       } finally {
@@ -6675,6 +6697,28 @@ and show an appropriate message to the user.\`
     return runOnce(fallback.provider, fallback.modelId, "fallback");
   }
 
+  private async auditReadonlyWorkflowStepPromptsOnce(taskId: string): Promise<void> {
+    if (this.readonlyWorkflowStepAuditDone) return;
+    this.readonlyWorkflowStepAuditDone = true;
+    const tokens = ["edit", "write", "commit", "stage", "modify"];
+    try {
+      const steps = await this.store.listWorkflowSteps();
+      for (const step of steps) {
+        if ((step.mode || "prompt") !== "prompt" || (step.toolMode || "readonly") !== "readonly") continue;
+        const prompt = step.prompt || "";
+        for (const token of tokens) {
+          const re = new RegExp(`\\b${token}\\b`, "i");
+          if (re.test(prompt)) {
+            executorLog.warn(`[workflow-step-audit] readonly step "${step.name}" prompt contains write-implying token "${token}" — re-review intended scope (no auto-migration performed)`);
+            break;
+          }
+        }
+      }
+    } catch (error) {
+      executorLog.warn(`${taskId}: failed readonly workflow-step prompt audit: ${formatError(error)}`);
+    }
+  }
+
   private MAX_WORKTREE_RETRIES = 3;
   private WORKTREE_RETRY_DELAYS = [100, 500, 1000]; // ms
 

packages/engine/src/merger.ts

@@ -78,6 +78,7 @@ import { createWebFetchTool } from "./agent-tools.js";
 import { auditSquashMerge, MERGER_MAIN_OVERLAP_LOOKBACK_COMMITS, type PostMergeAuditStrategy, type SquashAuditFindings } from "./merger-squash-audit.js";
 import { detectMergeOverlap, restoreBranchWinsFiles } from "./merger-overlap-guard.js";
 import { checkDiffVolume, DiffVolumeRegressionError } from "./merger-diff-volume-gate.js";
+import { ReadonlyViolationError, filterCustomToolsForReadonly } from "./workflow-step-tool-policy.js";
 
 export { DiffVolumeRegressionError } from "./merger-diff-volume-gate.js";
 
@@ -8920,6 +8921,15 @@ If issues are found that need attention, describe them clearly and include concr
     const postMergeSystemPrompt = buildSystemPromptWithInstructions(systemPrompt, postMergeInstructions);
 
     const mergerRuntimeHint = extractRuntimeHint(assignedAgent?.runtimeConfig);
+    const readonlyCustomTools = toolMode === "readonly"
+      ? filterCustomToolsForReadonly([])
+      : { allowed: [] as ToolDefinition[], denied: [] as string[] };
+    if (toolMode === "readonly" && readonlyCustomTools.denied.length > 0) {
+      await store.logEntry(
+        taskId,
+        `[readonly-violation] Post-merge workflow step '${workflowStep.name}' dropped denied custom tools: ${readonlyCustomTools.denied.join(", ")}`,
+      );
+    }
     const { session } = await createResolvedAgentSession({
       sessionPurpose: "merger",
       runtimeHint: mergerRuntimeHint,
@@ -8934,6 +8944,7 @@ If issues are found that need attention, describe them clearly and include concr
       defaultThinkingLevel: settings.defaultThinkingLevel,
       // Skill selection: use assigned agent skills if available, otherwise role fallback
       ...(postMergeSkillContext?.skillSelectionContext ? { skillSelection: postMergeSkillContext.skillSelectionContext } : {}),
+      ...(readonlyCustomTools.allowed.length > 0 ? { customTools: readonlyCustomTools.allowed } : {}),
       taskId,
       onFallbackModelUsed: createFallbackModelObserver({
         agent: "merger",
@@ -8970,6 +8981,11 @@ If issues are found that need attention, describe them clearly and include concr
     return { success: true, output };
   } catch (err: any) {
     await agentLogger.flush();
+    if ((err instanceof ReadonlyViolationError) || err?.code === "READONLY_VIOLATION") {
+      const deniedTool = err?.toolName || "unknown";
+      await store.logEntry(taskId, `[readonly-violation] Post-merge workflow step '${workflowStep.name}' attempted denied tool '${deniedTool}'`);
+      return { success: false, error: `[readonly-violation] ${err?.message ?? "Readonly policy violation"}` };
+    }
     return { success: false, error: err.message };
   }
 }

packages/engine/src/pi.ts

@@ -57,6 +57,7 @@ import {
 } from "./agent-action-gate.js";
 import { resolvePermanentAgentToolDecision } from "./permanent-agent-gating.js";
 import type { SystemPromptLayers } from "./prompt-layers.js";
+import { READONLY_ALLOWLIST, filterCustomToolsForReadonly, isReadonlyAllowed } from "./workflow-step-tool-policy.js";
 
 export interface AgentResult {
   session: AgentSession;
@@ -1694,23 +1695,19 @@ export async function createFnAgent(options: AgentOptions): Promise<AgentResult>
       }
     : undefined;
 
-  const tools =
-    options.tools === "readonly"
-      ? [
-          createReadTool(options.cwd),
-          createGrepTool(options.cwd),
-          createFindTool(options.cwd),
-          createLsTool(options.cwd),
-        ]
-      : [
-          createReadTool(options.cwd),
-          createBashTool(options.cwd, bashToolOptions),
-          createEditTool(options.cwd),
-          createWriteTool(options.cwd),
-          createGrepTool(options.cwd),
-          createFindTool(options.cwd),
-          createLsTool(options.cwd),
-        ];
+  const isReadonly = options.tools === "readonly";
+  const builtins = [
+    createReadTool(options.cwd),
+    createBashTool(options.cwd, bashToolOptions),
+    createEditTool(options.cwd),
+    createWriteTool(options.cwd),
+    createGrepTool(options.cwd),
+    createFindTool(options.cwd),
+    createLsTool(options.cwd),
+  ] as ToolDefinition[];
+  const tools = isReadonly
+    ? builtins.filter((tool) => isReadonlyAllowed(tool.name))
+    : builtins;
   // Suppress lint about unused presets — kept in scope for incremental migration.
   void createCodingTools;
   void createReadOnlyTools;
@@ -1807,7 +1804,6 @@ export async function createFnAgent(options: AgentOptions): Promise<AgentResult>
   // since heartbeat/reviewer flows explicitly provide engine-owned tools.
   // This keeps summarizer/compaction sessions safe while retaining intended
   // delegation/memory tools for readonly engine sessions.
-  const isReadonly = options.tools === "readonly";
   const effectiveExtensionPaths = isReadonly ? [] : hostExtensionPaths;
   if (isReadonly && hostExtensionPaths.length > 0) {
     piLog.log(`readonly session — host extensions (${hostExtensionPaths.length}) skipped`);
@@ -1836,9 +1832,18 @@ export async function createFnAgent(options: AgentOptions): Promise<AgentResult>
     // suppress the defaults with `noTools: "builtin"` and register our wrapped
     // tools through `customTools` instead. The wrapped tools preserve the same
     // names (`read`, `bash`, ...) as the built-ins they replace.
+    const readonlyFilteredCustomTools = isReadonly
+      ? filterCustomToolsForReadonly(options.customTools ?? [])
+      : { allowed: options.customTools ?? [], denied: [] };
+    if (isReadonly && readonlyFilteredCustomTools.denied.length > 0) {
+      piLog.warn(
+        `[pi] readonly mode: dropped ${readonlyFilteredCustomTools.denied.length} denied custom tool(s): ${readonlyFilteredCustomTools.denied.join(", ")}`,
+      );
+    }
+
     const toolChainStart: ToolDefinition[] = [
       ...(tools as ToolDefinition[]),
-      ...(options.customTools ?? []),
+      ...readonlyFilteredCustomTools.allowed,
     ];
     const toolsWithPermanentGating = wrapToolsWithPermanentAgentGating(
       toolChainStart,
@@ -1885,10 +1890,13 @@ export async function createFnAgent(options: AgentOptions): Promise<AgentResult>
     };
 
     if (options.builtinToolsAllowlist && options.builtinToolsAllowlist.length > 0) {
+      const safeBuiltinAllowlist = isReadonly
+        ? options.builtinToolsAllowlist.filter((name) => READONLY_ALLOWLIST.includes(name as (typeof READONLY_ALLOWLIST)[number]))
+        : options.builtinToolsAllowlist;
       createSessionOptions.tools = [
         ...new Set([
           ...customToolList.map((tool) => tool.name),
-          ...options.builtinToolsAllowlist,
+          ...safeBuiltinAllowlist,
         ]),
       ].sort();
     }

packages/engine/src/workflow-step-tool-policy.ts

@@ -0,0 +1,65 @@
+import type { ToolDefinition } from "@mariozechner/pi-coding-agent";
+import { TASK_AGENT_MUTATION_TOOLS } from "./gating-classifications.js";
+
+export const READONLY_ALLOWLIST = [
+  "read",
+  "grep",
+  "find",
+  "ls",
+  "fn_web_fetch",
+  "fn_task_show",
+  "fn_task_list",
+  "fn_insight_list",
+  "fn_insight_show",
+  "fn_list_agents",
+  "fn_get_agent_config",
+] as const;
+
+const WRITE_BUILTIN_TOOLS = ["edit", "write", "bash"] as const;
+
+export const DENIED_IN_READONLY = [
+  ...WRITE_BUILTIN_TOOLS,
+  ...Array.from(TASK_AGENT_MUTATION_TOOLS).sort(),
+] as const;
+
+const READONLY_ALLOWLIST_SET = new Set<string>(READONLY_ALLOWLIST);
+const DENIED_IN_READONLY_SET = new Set<string>(DENIED_IN_READONLY);
+
+// Note: fn_task_browse_github_issues is read-only by behavior, but readonly sessions
+// intentionally exclude host extensions in pi.ts, so it remains absent by default.
+
+export class ReadonlyViolationError extends Error {
+  readonly code = "READONLY_VIOLATION" as const;
+
+  constructor(
+    public readonly taskId: string,
+    public readonly stepName: string,
+    public readonly toolName: string,
+  ) {
+    super(`[readonly-violation] ${stepName} attempted to use denied tool "${toolName}" for task ${taskId}`);
+    this.name = "ReadonlyViolationError";
+  }
+}
+
+export function isReadonlyAllowed(toolName: string): boolean {
+  return READONLY_ALLOWLIST_SET.has(toolName.trim());
+}
+
+export function filterCustomToolsForReadonly(tools: ToolDefinition[]): { allowed: ToolDefinition[]; denied: string[] } {
+  const allowed: ToolDefinition[] = [];
+  const denied: string[] = [];
+
+  for (const tool of tools) {
+    const name = tool.name?.trim() ?? "";
+    if (!name) continue;
+    if (isReadonlyAllowed(name)) {
+      allowed.push(tool);
+      continue;
+    }
+    if (DENIED_IN_READONLY_SET.has(name)) {
+      denied.push(name);
+    }
+  }
+
+  return { allowed, denied };
+}