diff --git a/docs/security/protected-path-edits.md b/docs/security/protected-path-edits.md new file mode 100644 index 00000000..d8ea259b --- /dev/null +++ b/docs/security/protected-path-edits.md @@ -0,0 +1,108 @@ +# Protected-path edit grants + +## What are Claude Code protected paths? + +Claude Code maintains a hardcoded set of **protected paths** that are never +auto-approved for writes, even in `acceptEdits` permission mode. The relevant +prefixes for phax are: + +- `.claude/` (except `.claude/worktrees/`) + +Other protected prefixes (`.git/`, `.vscode/`, `.idea/`) are enforced by Claude +Code but are out of scope for phax grants. + +## Why headless runs cannot reach them + +phax runs Claude Code headless with `--permission-mode acceptEdits`, which +auto-approves edits inside the writable directories (the worktree root plus any +`--add-dir` paths). Protected paths are checked **before** that permission +evaluation, so an `Edit(.claude/**)` entry in `permissions.allow` has no effect — +the write is silently denied. + +The only full overrides are: + +- `--permission-mode bypassPermissions` — drops the entire jail (Bash allow-list + and filesystem bounds), unacceptable for secure runs. +- A `PreToolUse` **hook** that returns an explicit `allow` decision for a single + tool call. + +## The PreToolUse hook approach + +phax generates a narrow `PreToolUse` hook scoped to exactly the protected paths +a phase declares and passes it to `claude` via `--settings`. The hook: + +1. Receives the tool name and input as JSON on stdin from Claude Code. +2. Calls the domain decision (`decideProtectedPathApproval`). +3. Emits `{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow"}}` + for an approved path; emits nothing (exits 0) otherwise. + +The rest of the secure jail — Bash allow-list, filesystem bounds — is untouched. + +## Trust model: plan declares, config grants + +The operator opts in at the `phax.json` level; a plan then declares what it +needs. phax enforces that a declared protected path must be covered by an +opted-in prefix — a plan cannot widen its own protected-write surface. + +### Operator opt-in (`phax.json`) + +Add protected path-prefixes to `security.filesystem.allowWriteProtected`: + +```json +{ + "security": { + "filesystem": { + "allowWriteProtected": [".claude/skills/"] + } + } +} +``` + +An absent or empty array means the feature is off and behavior is unchanged. + +### Phase declaration (`plan.md`) + +List the specific protected files in the phase's planned-file sections: + +```markdown +### Planned files to edit + +- .claude/skills/phax-planning/SKILL.md +``` + +### Preflight enforcement + +Before spawning the phase agent, phax resolves each declared protected path +against the opted-in prefixes: + +- **Covered** (declared path falls under an `allowWriteProtected` prefix) → + path is added to `approvedProtectedPaths`; the hook will approve it at + runtime. +- **Uncovered** (protected but not opted into by config) → preflight fails with + a `SecurityPreflightError` naming the phase and the offending path. + +Non-protected paths are never checked here. + +## Required PHAX security configuration changes + +When a plan phase needs to edit a `.claude/**` file, add the covering prefix to +`phax.json` before running. Without it the preflight will fail before any agent +spawns. + +Example: to allow editing `.claude/skills/phax-planning/SKILL.md`, add: + +```json +"security": { + "filesystem": { + "allowWriteProtected": [".claude/skills/"] + } +} +``` + +## Provider scope + +Protected paths are a Claude Code concept. The codex and mistral-vibe adapters +sandbox the filesystem at the worktree level and do not block `.claude/**`, so +they need no hook. The `approvedProtectedPaths` field is computed and recorded in +`security.json` for all providers (audit parity), but only the Claude Code +adapter consumes it to generate a hook. diff --git a/phax.schema.json b/phax.schema.json index 5705868d..df17b728 100644 --- a/phax.schema.json +++ b/phax.schema.json @@ -119,6 +119,13 @@ "items": { "$ref": "#/$defs/NonEmptyString" } + }, + "allowWriteProtected": { + "type": "array", + "description": "Protected path-prefixes (e.g. \".claude/skills/\") the operator opts into allowing a phase to edit via a scoped PreToolUse hook. A phase declaring a file under one of these prefixes receives a narrow edit grant; a declared protected file not covered by any prefix fails preflight.", + "items": { + "$ref": "#/$defs/NonEmptyString" + } } }, "additionalProperties": false diff --git a/scripts/generate-usage-spec.ts b/scripts/generate-usage-spec.ts index 865e9dcb..1d905ad1 100644 --- a/scripts/generate-usage-spec.ts +++ b/scripts/generate-usage-spec.ts @@ -64,6 +64,12 @@ function emitArg(arg: Argument, indent: string): string { return `${indent}arg "${name}"`; } +// Internal/hook subcommands use a __ prefix by convention and are excluded +// from the usage spec. +function isInternalCommand(cmd: Command): boolean { + return cmd.name().startsWith("__"); +} + function emitCommand(cmd: Command, indent: string, parentPath = ""): string[] { const cmdPath = parentPath ? `${parentPath} ${cmd.name()}` : cmd.name(); const lines: string[] = [`${indent}cmd "${cmd.name()}" {`]; @@ -90,6 +96,7 @@ function emitCommand(cmd: Command, indent: string, parentPath = ""): string[] { } for (const sub of cmd.commands) { + if (isInternalCommand(sub)) continue; lines.push(""); lines.push(...emitCommand(sub, inner, cmdPath)); } @@ -136,8 +143,9 @@ export function generateUsageSpec(): string { } lines.push(``); - // All top-level commands. + // All visible top-level commands (internal __ commands are excluded). for (const cmd of program.commands) { + if (isInternalCommand(cmd)) continue; lines.push(...emitCommand(cmd, "")); lines.push(``); } diff --git a/src/app/executePlan.ts b/src/app/executePlan.ts index 72c4e23d..1c2f9abe 100644 --- a/src/app/executePlan.ts +++ b/src/app/executePlan.ts @@ -64,6 +64,7 @@ import { checkRequiredCommands, computeFrozenAgentCommands, } from "../domain/security/agentCommands.js"; +import { resolveProtectedApprovals } from "../domain/security/protectedPaths.js"; import { resolveSecurityPolicy } from "../domain/security/resolvePolicy.js"; import { cleanupPhase } from "./cleanup.js"; import { commitPhase } from "./commit.js"; @@ -513,6 +514,16 @@ export function executePlan( requiredCommands: plan.run.requiredCommands, provider: binding.provider, }); + const resumePlannedPaths = [ + ...phase.plannedFilesToCreate, + ...phase.plannedFilesToEdit, + ...phase.optionalFilesToEdit, + ]; + const resumeProtectedApprovals = resolveProtectedApprovals({ + plannedPaths: resumePlannedPaths, + allowWriteProtected: securityPolicy.filesystem.allowWriteProtected, + worktreeRoot: worktreePath as string, + }); agentOptions = { provider: binding.provider, model: binding.model, @@ -520,6 +531,7 @@ export function executePlan( cwd: worktreePath as string, security: securityPolicy, agentCommands: resumeFrozenResult.records.map((r) => r.command), + approvedProtectedPaths: resumeProtectedApprovals.approved, outputJsonlPath: join(phaseFolderPath, "output.jsonl"), phaseFolderPath, }; @@ -607,6 +619,35 @@ export function executePlan( worktreePath: worktreePath as string, config: config.security, }); + + // Preflight: verify all declared protected paths are covered by the + // operator's allowWriteProtected opt-in before spawning the agent. + const phasePlannedPaths = [ + ...phase.plannedFilesToCreate, + ...phase.plannedFilesToEdit, + ...phase.optionalFilesToEdit, + ]; + const protectedApprovals = resolveProtectedApprovals({ + plannedPaths: phasePlannedPaths, + allowWriteProtected: securityPolicy.filesystem.allowWriteProtected, + worktreeRoot: worktreePath as string, + }); + // Protected paths only block in secure mode (that is where Claude Code's + // acceptEdits sandbox is active). In unsafe/isolated mode there is no + // jail to circumvent, so the hook is irrelevant and we skip the check. + if (securityPolicy.mode === "secure" && protectedApprovals.uncovered.length > 0) { + return yield* Effect.fail( + new SecurityPreflightError({ + message: [ + `Security preflight failed: phase "${phase.id}" declares ${protectedApprovals.uncovered.length} protected path(s) not covered by security.filesystem.allowWriteProtected in phax.json.`, + `Uncovered: ${protectedApprovals.uncovered.map((p) => `"${p}"`).join(", ")}`, + `Add a matching prefix to security.filesystem.allowWriteProtected in phax.json before running.`, + ].join("\n"), + missing: protectedApprovals.uncovered, + }), + ); + } + const securityFilter: SecurityFilter = (provider) => { if (securityMode !== "secure") { return { allowed: true }; @@ -689,6 +730,7 @@ export function executePlan( marks: postureMarks, agentCommands: frozenResult.records, providerSkippedForSecurity: resolution.skippedForSecurity ?? [], + approvedProtectedPaths: protectedApprovals.approved, }; yield* fs.writeAtomic( join(phaseFolderPath, "security.json"), @@ -744,6 +786,7 @@ export function executePlan( cwd: worktreePath as string, security: securityPolicy, agentCommands: frozenResult.records.map((r) => r.command), + approvedProtectedPaths: protectedApprovals.approved, outputJsonlPath: join(phaseFolderPath, "output.jsonl"), phaseFolderPath, }; diff --git a/src/cli/commands/approveProtectedPath.ts b/src/cli/commands/approveProtectedPath.ts new file mode 100644 index 00000000..a9a051d2 --- /dev/null +++ b/src/cli/commands/approveProtectedPath.ts @@ -0,0 +1,71 @@ +import { decideProtectedPathApproval } from "../../domain/security/protectedPaths.js"; +import { + parseClaudeHookPayload, + PHAX_APPROVED_PATHS_ENV, +} from "../../schemas/claudeHookPayload.js"; + +const ALLOW_OUTPUT = JSON.stringify({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "allow", + }, +}); + +function readApprovedPaths(): readonly string[] { + const raw = process.env[PHAX_APPROVED_PATHS_ENV]; + if (!raw) return []; + try { + const parsed: unknown = JSON.parse(raw); + if (Array.isArray(parsed) && parsed.every((x) => typeof x === "string")) { + return parsed as string[]; + } + return []; + } catch { + return []; + } +} + +async function readStdin(): Promise { + return new Promise((resolve) => { + let data = ""; + process.stdin.setEncoding("utf8"); + process.stdin.on("data", (chunk: string) => { + data += chunk; + }); + process.stdin.on("end", () => resolve(data)); + process.stdin.resume(); + }); +} + +/** + * Thin CLI entry point for the Claude Code PreToolUse hook. + * + * Reads the hook payload from stdin, decodes it, and calls the domain + * decision with the approved paths from the PHAX_APPROVED_PATHS env var. + * On "allow" prints the Claude permissionDecision JSON; on "defer" prints + * nothing and exits 0 so Claude's normal protected-path handling applies. + * + * Contains no business logic — all decisions are in decideProtectedPathApproval. + */ +export async function runApproveProtectedPath(): Promise { + const approvedAbsolutePaths = readApprovedPaths(); + const stdinText = await readStdin(); + const payload = parseClaudeHookPayload(stdinText.trim()); + + if (payload === undefined) { + // Unrecognized payload — defer to Claude's default handling. + return 0; + } + + const decision = decideProtectedPathApproval({ + approvedAbsolutePaths, + toolName: payload.tool_name, + filePath: payload.tool_input.file_path, + }); + + if (decision === "allow") { + process.stdout.write(ALLOW_OUTPUT + "\n"); + } + + return 0; +} diff --git a/src/cli/introspect.ts b/src/cli/introspect.ts index 8efbccb6..8cfd81a8 100644 --- a/src/cli/introspect.ts +++ b/src/cli/introspect.ts @@ -7,6 +7,12 @@ export interface CommandNode { subcommands: CommandNode[]; } +// Internal/hook subcommands use a __ prefix by convention and are excluded +// from the public CLI surface (usage spec, help, parity gate). +function isInternalCommand(cmd: Command): boolean { + return cmd.name().startsWith("__"); +} + function walkCommand(cmd: Command): CommandNode { const flags = cmd.options .map((opt) => opt.long) @@ -17,7 +23,7 @@ function walkCommand(cmd: Command): CommandNode { return { name: cmd.name(), flags, - subcommands: cmd.commands.map(walkCommand), + subcommands: cmd.commands.filter((sub) => !isInternalCommand(sub)).map(walkCommand), }; } diff --git a/src/cli/program.ts b/src/cli/program.ts index 72ffe5ba..e3220e30 100644 --- a/src/cli/program.ts +++ b/src/cli/program.ts @@ -31,6 +31,7 @@ import { runInit } from "./commands/init.js"; import { registerSchemaCommand } from "./commands/schema.js"; import { runCompletions } from "./commands/completions.js"; import { runReport } from "./commands/report.js"; +import { runApproveProtectedPath } from "./commands/approveProtectedPath.js"; export function buildProgram(): Command { const program = new Command(); @@ -384,6 +385,16 @@ export function buildProgram(): Command { registerSkillsCommand(program, consoleOutput); registerSchemaCommand(program, consoleOutput); + // Hidden subcommand invoked by the generated Claude Code PreToolUse hook; + // not intended for direct user use. + const approveCmd = new Command("__approve-protected-path") + .description("Internal: Claude Code PreToolUse hook for protected-path approval") + .action(async () => { + const exitCode = await runApproveProtectedPath(); + process.exit(exitCode); + }); + program.addCommand(approveCmd, { hidden: true }); + // Wire long help and examples into the runtime --help output after all // registrations so commands from *Register.ts files are covered without // touching those files. diff --git a/src/domain/security/protectedPaths.ts b/src/domain/security/protectedPaths.ts new file mode 100644 index 00000000..fec7f89c --- /dev/null +++ b/src/domain/security/protectedPaths.ts @@ -0,0 +1,131 @@ +import * as path from "node:path"; + +/** + * Repo-relative directory prefixes that Claude Code treats as protected and + * that phax may offer to approve via a scoped PreToolUse hook. + * + * Claude Code additionally protects `.git/`, `.vscode/`, `.idea/`, and other + * paths; those are intentionally out of scope here. This constant governs only + * what phax is willing to grant a hook approval for. + * + * `.claude/worktrees/` is writable under acceptEdits and is excluded by + * `isProtectedPath`. + */ +export const CLAUDE_PROTECTED_PREFIXES: readonly string[] = [".claude/"]; + +const CLAUDE_PROTECTED_EXCLUSIONS: readonly string[] = [".claude/worktrees/"]; + +const APPROVABLE_TOOL_NAMES = new Set(["Edit", "Write", "MultiEdit"]); + +function normalizeRepoRelative(input: string): string { + const stripped = input.replace(/\\/g, "/").replace(/^\.\/+/, ""); + const normalized = path.posix.normalize(stripped); + return normalized.startsWith("./") ? normalized.slice(2) : normalized; +} + +function prefixCovers(prefix: string, candidate: string): boolean { + const normalizedPrefix = normalizeRepoRelative(prefix); + const bare = normalizedPrefix.endsWith("/") ? normalizedPrefix.slice(0, -1) : normalizedPrefix; + if (!bare) return false; + if (candidate === bare) return true; + return candidate.startsWith(bare + "/"); +} + +export function isProtectedPath(repoRelativePosixPath: string): boolean { + const normalized = normalizeRepoRelative(repoRelativePosixPath); + if (!normalized || normalized.startsWith("../") || normalized === "..") { + return false; + } + for (const exclusion of CLAUDE_PROTECTED_EXCLUSIONS) { + if (prefixCovers(exclusion, normalized)) return false; + } + for (const prefix of CLAUDE_PROTECTED_PREFIXES) { + if (prefixCovers(prefix, normalized)) return true; + } + return false; +} + +export interface ResolveProtectedApprovalsInput { + readonly plannedPaths: readonly string[]; + readonly allowWriteProtected: readonly string[]; + readonly worktreeRoot: string; +} + +export interface ResolveProtectedApprovalsResult { + readonly approved: readonly string[]; + readonly uncovered: readonly string[]; +} + +function toAbsolutePosix(worktreeRoot: string, repoRelative: string): string { + const normalizedRoot = worktreeRoot.replace(/\\/g, "/").replace(/\/+$/, ""); + const joined = `${normalizedRoot}/${repoRelative}`; + return path.posix.normalize(joined); +} + +/** + * Partition a phase's declared planned paths into protected paths that the + * operator's `allowWriteProtected` prefixes cover (returned as absolute POSIX + * paths) and protected paths that fall outside any configured prefix. Both + * outputs deduplicate while preserving input order. + * + * Non-protected paths are ignored entirely. + */ +export function resolveProtectedApprovals( + input: ResolveProtectedApprovalsInput, +): ResolveProtectedApprovalsResult { + const approved: string[] = []; + const uncovered: string[] = []; + const seenApproved = new Set(); + const seenUncovered = new Set(); + + for (const raw of input.plannedPaths) { + const normalized = normalizeRepoRelative(raw); + if (!normalized || !isProtectedPath(normalized)) continue; + + const covered = input.allowWriteProtected.some((prefix) => prefixCovers(prefix, normalized)); + + if (covered) { + const absolute = toAbsolutePosix(input.worktreeRoot, normalized); + if (!seenApproved.has(absolute)) { + seenApproved.add(absolute); + approved.push(absolute); + } + } else { + if (!seenUncovered.has(normalized)) { + seenUncovered.add(normalized); + uncovered.push(normalized); + } + } + } + + return { approved, uncovered }; +} + +export interface DecideProtectedPathApprovalInput { + readonly approvedAbsolutePaths: readonly string[]; + readonly toolName: string; + readonly filePath: string | undefined; +} + +/** + * Runtime decision for a single Claude Code PreToolUse invocation. Returns + * `"allow"` only when the tool is one of Edit/Write/MultiEdit and the + * resolved absolute `filePath` exactly matches an approved path. Otherwise + * `"defer"` — the hook emits nothing and Claude's normal protected-path + * handling applies. + */ +export function decideProtectedPathApproval( + input: DecideProtectedPathApprovalInput, +): "allow" | "defer" { + if (!APPROVABLE_TOOL_NAMES.has(input.toolName)) return "defer"; + if (!input.filePath) return "defer"; + + const normalized = path.posix.normalize(input.filePath.replace(/\\/g, "/")); + if (!path.posix.isAbsolute(normalized)) return "defer"; + + for (const approved of input.approvedAbsolutePaths) { + const normalizedApproved = path.posix.normalize(approved.replace(/\\/g, "/")); + if (normalizedApproved === normalized) return "allow"; + } + return "defer"; +} diff --git a/src/domain/security/resolvePolicy.ts b/src/domain/security/resolvePolicy.ts index 41e5244e..ec155c0e 100644 --- a/src/domain/security/resolvePolicy.ts +++ b/src/domain/security/resolvePolicy.ts @@ -17,7 +17,9 @@ export function resolveSecurityPolicy(input: ResolvePolicyInput): SecurityPolicy if (mode === "unsafe") { return { mode: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + // allowWriteProtected is a secure-mode concept; unsafe mode already + // drops the jail entirely, so the hook is never generated. + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: config.network.profile }, mcp: { mode: config.mcp.mode, allow: [] }, agentCommands: config.agentCommands, @@ -47,7 +49,11 @@ export function resolveSecurityPolicy(input: ResolvePolicyInput): SecurityPolicy // network_access=false; broader profiles permit subprocess network. return { mode, - filesystem: { allowRead, allowWrite }, + filesystem: { + allowRead, + allowWrite, + allowWriteProtected: config.filesystem.allowWriteProtected, + }, network: { profile: config.network.profile }, mcp: { mode: config.mcp.mode, allow: config.mcp.allow }, agentCommands: config.agentCommands, diff --git a/src/domain/security/resolveReviewPolicy.ts b/src/domain/security/resolveReviewPolicy.ts index ad40e2a4..f6d0ae86 100644 --- a/src/domain/security/resolveReviewPolicy.ts +++ b/src/domain/security/resolveReviewPolicy.ts @@ -23,7 +23,9 @@ export function resolveReviewSecurityPolicy(input: ResolveReviewPolicyInput): Se return { mode, - filesystem: { allowRead, allowWrite }, + // allowWriteProtected is always [] for the review phase — the reviewer has + // read-only access to the worktree and no protected-path grant is appropriate. + filesystem: { allowRead, allowWrite, allowWriteProtected: [] }, // Override to tightest network and MCP settings — the reviewer has no legitimate // reason to reach external APIs beyond the provider CLI (which runs outside the sandbox). network: { profile: "provider-only" }, diff --git a/src/domain/security/types.ts b/src/domain/security/types.ts index 4a7df62e..3b15b18a 100644 --- a/src/domain/security/types.ts +++ b/src/domain/security/types.ts @@ -7,6 +7,7 @@ export interface SecurityPolicy { readonly filesystem: { readonly allowRead: readonly string[]; readonly allowWrite: readonly string[]; + readonly allowWriteProtected: readonly string[]; }; readonly network: { readonly profile: NetworkProfile }; readonly mcp: { readonly mode: McpMode; readonly allow: readonly string[] }; diff --git a/src/infra/providers/claudeCode.ts b/src/infra/providers/claudeCode.ts index ccc23ce5..808f266f 100644 --- a/src/infra/providers/claudeCode.ts +++ b/src/infra/providers/claudeCode.ts @@ -1,8 +1,8 @@ import { Effect, Either } from "effect"; import { spawn } from "node:child_process"; -import { createWriteStream } from "node:fs"; +import { createWriteStream, mkdirSync, writeFileSync } from "node:fs"; import { mkdir } from "node:fs/promises"; -import { dirname } from "node:path"; +import { dirname, join } from "node:path"; import type { AgentRunOptions, AgentRunResult, @@ -27,6 +27,7 @@ import { } from "../../schemas/claudeOutput.js"; import { persistSessionId } from "./sessionWriter.js"; import { writeAgentErrorLog } from "./agentErrorLog.js"; +import { buildProtectedPathHookSettings } from "./protectedPathHookSettings.js"; function wrapFsError(err: unknown): FsError { return new FsError({ @@ -182,7 +183,11 @@ function buildSecureClaudeFlags( return ["--permission-mode", "acceptEdits", ...addDirs, ...shellFlags, ...mcpFlags]; } -export function buildArgs(options: AgentRunOptions, resumeSessionId?: string): string[] { +export function buildArgs( + options: AgentRunOptions, + resumeSessionId?: string, + settingsFilePath?: string, +): string[] { // `claude` requires `--verbose` whenever `--print` is paired with // `--output-format=stream-json`; without it the CLI exits with code 1. const common = ["--print", "--output-format", "stream-json", "--verbose"]; @@ -208,7 +213,17 @@ export function buildArgs(options: AgentRunOptions, resumeSessionId?: string): s return buildSecureClaudeFlags(options.security, options.cwd, options.agentCommands ?? []); })(); - const args = [...common, ...modeFlags, "--model", options.model, "--effort", options.effort]; + const settingsFlags = settingsFilePath !== undefined ? ["--settings", settingsFilePath] : []; + + const args = [ + ...common, + ...modeFlags, + ...settingsFlags, + "--model", + options.model, + "--effort", + options.effort, + ]; if (resumeSessionId) { args.push("--resume", resumeSessionId); } @@ -314,6 +329,36 @@ export function runClaudeCompletion( }); } +const HOOK_SUBCOMMAND = "__approve-protected-path"; +const SETTINGS_FILE_NAME = "claude-protected-approval.settings.json"; + +/** + * Write the protected-path hook settings file for a phase and return its + * absolute path. Returns undefined when there are no approved paths or no + * phase folder. Uses sync I/O (like writeAgentErrorLog) — never throws. + */ +export function writeProtectedPathSettings( + phaseFolderPath: string | undefined, + approvedProtectedPaths: readonly string[] | undefined, +): string | undefined { + if (!phaseFolderPath || !approvedProtectedPaths || approvedProtectedPaths.length === 0) { + return undefined; + } + try { + const settingsPath = join(phaseFolderPath, SETTINGS_FILE_NAME); + const settings = buildProtectedPathHookSettings( + approvedProtectedPaths, + `phax ${HOOK_SUBCOMMAND}`, + ); + mkdirSync(dirname(settingsPath), { recursive: true }); + writeFileSync(settingsPath, JSON.stringify(settings, null, 2), "utf8"); + return settingsPath; + } catch { + // Never let a settings-write failure mask the underlying agent run. + return undefined; + } +} + export function runClaudeAgent( prompt: string, options: AgentRunOptions, @@ -327,9 +372,13 @@ export function runClaudeAgent( | SecurityEnforcementError | FsError > { + const settingsFilePath = writeProtectedPathSettings( + options.phaseFolderPath, + options.approvedProtectedPaths, + ); let args: string[]; try { - args = buildArgs(options, resumeSessionId); + args = buildArgs(options, resumeSessionId, settingsFilePath); } catch (err) { if (err instanceof SecurityEnforcementError) { return Effect.fail(err); diff --git a/src/infra/providers/protectedPathHookSettings.ts b/src/infra/providers/protectedPathHookSettings.ts new file mode 100644 index 00000000..c21a47cb --- /dev/null +++ b/src/infra/providers/protectedPathHookSettings.ts @@ -0,0 +1,48 @@ +import { PHAX_APPROVED_PATHS_ENV } from "../../schemas/claudeHookPayload.js"; + +export { PHAX_APPROVED_PATHS_ENV }; + +/** + * Claude Code settings object shape for a PreToolUse hook entry. + * This is a pure module — no I/O. + */ +export interface ClaudeHookSettings { + readonly env: Record; + readonly hooks: { + readonly PreToolUse: ReadonlyArray<{ + readonly matcher: string; + readonly hooks: ReadonlyArray<{ + readonly type: "command"; + readonly command: string; + }>; + }>; + }; +} + +/** + * Pure builder: given the absolute paths the operator has approved and the + * command phax should invoke for the hook, returns the Claude settings object + * that wires a PreToolUse hook scoped to Edit|Write|MultiEdit. + * + * Approved paths are passed to the hook process via the PHAX_APPROVED_PATHS + * env var (JSON-encoded) set in the settings `env` block — no arg-quoting + * pitfalls, no shell escaping required. + */ +export function buildProtectedPathHookSettings( + approvedAbsolutePaths: readonly string[], + hookCommand: string, +): ClaudeHookSettings { + return { + env: { + [PHAX_APPROVED_PATHS_ENV]: JSON.stringify(approvedAbsolutePaths), + }, + hooks: { + PreToolUse: [ + { + matcher: "Edit|Write|MultiEdit", + hooks: [{ type: "command", command: hookCommand }], + }, + ], + }, + }; +} diff --git a/src/ports/backend.ts b/src/ports/backend.ts index e632669b..70c7ec75 100644 --- a/src/ports/backend.ts +++ b/src/ports/backend.ts @@ -36,6 +36,14 @@ export interface AgentRunOptions { * (codex/vibe). Recorded in security.json regardless of provider. */ readonly agentCommands?: readonly string[] | undefined; + /** + * Absolute paths of declared protected files the operator has opted into + * allowing the agent to write (resolved from the phase's planned-file lists + * and security.filesystem.allowWriteProtected in phax.json). Only consumed + * by the claude provider (PreToolUse hook); recorded in security.json for + * all providers. Absent or empty means no protected-path grant. + */ + readonly approvedProtectedPaths?: readonly string[] | undefined; readonly outputJsonlPath?: string | undefined; readonly phaseFolderPath?: string | undefined; } diff --git a/src/schemas/claudeHookPayload.ts b/src/schemas/claudeHookPayload.ts new file mode 100644 index 00000000..2b49b8bf --- /dev/null +++ b/src/schemas/claudeHookPayload.ts @@ -0,0 +1,35 @@ +import { Either, Schema } from "effect"; + +/** + * Env var name that carries the JSON-encoded array of approved absolute paths + * from the Claude settings file into the hook command process. Defined here + * (schemas layer) so both cli/ and infra/ can import it without violating + * the cli→infra boundary guard. + */ +export const PHAX_APPROVED_PATHS_ENV = "PHAX_APPROVED_PATHS"; + +/** + * Subset of the Claude Code PreToolUse stdin payload that phax needs. + * Extra fields from the actual payload are tolerated and ignored. + */ +export const ClaudeHookPayloadSchema = Schema.Struct({ + tool_name: Schema.String, + tool_input: Schema.Struct({ + file_path: Schema.optional(Schema.String), + }), +}); + +export type ClaudeHookPayload = Schema.Schema.Type; + +export const decodeClaudeHookPayload = Schema.decodeUnknownEither(ClaudeHookPayloadSchema); + +export function parseClaudeHookPayload(raw: string): ClaudeHookPayload | undefined { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; + } + const result = decodeClaudeHookPayload(parsed); + return Either.isRight(result) ? result.right : undefined; +} diff --git a/src/schemas/securityConfig.ts b/src/schemas/securityConfig.ts index b9ec95ce..aee55b98 100644 --- a/src/schemas/securityConfig.ts +++ b/src/schemas/securityConfig.ts @@ -13,6 +13,7 @@ export const McpModeSchema = Schema.Literal( const FilesystemConfigSchema = Schema.Struct({ allowRead: Schema.optional(Schema.Array(Schema.NonEmptyString)), allowWrite: Schema.optional(Schema.Array(Schema.NonEmptyString)), + allowWriteProtected: Schema.optional(Schema.Array(Schema.NonEmptyString)), }); const NetworkConfigSchema = Schema.Struct({ @@ -41,6 +42,7 @@ export interface ResolvedSecurityConfig { readonly filesystem: { readonly allowRead: readonly string[]; readonly allowWrite: readonly string[]; + readonly allowWriteProtected: readonly string[]; }; readonly network: { readonly profile: NetworkProfile; @@ -61,6 +63,7 @@ export function resolveSecurityConfig( filesystem: { allowRead: raw?.filesystem?.allowRead ?? [], allowWrite: raw?.filesystem?.allowWrite ?? [], + allowWriteProtected: raw?.filesystem?.allowWriteProtected ?? [], }, network: { profile: raw?.network?.profile ?? "provider-only", diff --git a/src/schemas/securityPosture.ts b/src/schemas/securityPosture.ts index 31bf728f..a23cd5d6 100644 --- a/src/schemas/securityPosture.ts +++ b/src/schemas/securityPosture.ts @@ -36,6 +36,7 @@ export const SecurityPostureSchema = Schema.Struct({ reason: Schema.NonEmptyString, }), ), + approvedProtectedPaths: Schema.Array(Schema.String), }); export type SecurityPosture = Schema.Schema.Type; diff --git a/tests/e2e/gateExhaustionResume.test.ts b/tests/e2e/gateExhaustionResume.test.ts index bb26e649..d93e6c1f 100644 --- a/tests/e2e/gateExhaustionResume.test.ts +++ b/tests/e2e/gateExhaustionResume.test.ts @@ -94,7 +94,7 @@ describe.skipIf(!shouldRun)("E2E gate-exhaustion resume", () => { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/e2e/resetPhase.test.ts b/tests/e2e/resetPhase.test.ts index 9ced3a9a..dfda53b8 100644 --- a/tests/e2e/resetPhase.test.ts +++ b/tests/e2e/resetPhase.test.ts @@ -99,7 +99,7 @@ describe.skipIf(!shouldRun)("E2E reset-phase → resume fresh re-execution", () fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/e2e/semanticTrace.providers.test.ts b/tests/e2e/semanticTrace.providers.test.ts index c04730e3..9ff101f7 100644 --- a/tests/e2e/semanticTrace.providers.test.ts +++ b/tests/e2e/semanticTrace.providers.test.ts @@ -175,7 +175,7 @@ describe.skipIf(!shouldRun)("E2E semantic trace — per-provider snapshots", () security: { profile: testCase.securityMode, - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/e2e/semanticTrace.test.ts b/tests/e2e/semanticTrace.test.ts index 87441522..c67edab8 100644 --- a/tests/e2e/semanticTrace.test.ts +++ b/tests/e2e/semanticTrace.test.ts @@ -97,7 +97,7 @@ describe.skipIf(!shouldRun)("E2E semantic trace — happy-path snapshot", () => security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/integration/adjustPlanCommand.test.ts b/tests/integration/adjustPlanCommand.test.ts index e25321fa..b8b565ce 100644 --- a/tests/integration/adjustPlanCommand.test.ts +++ b/tests/integration/adjustPlanCommand.test.ts @@ -30,7 +30,7 @@ function makeBaseConfig(stateRoot: string) { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe" as const, - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only" as const, allowDomains: [] }, mcp: { mode: "disabled" as const, allow: [] }, agentCommands: [], diff --git a/tests/integration/claudeProtectedPathHook.test.ts b/tests/integration/claudeProtectedPathHook.test.ts new file mode 100644 index 00000000..01b24de7 --- /dev/null +++ b/tests/integration/claudeProtectedPathHook.test.ts @@ -0,0 +1,244 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { readFileSync, existsSync } from "node:fs"; +import { buildArgs, writeProtectedPathSettings } from "../../src/infra/providers/claudeCode.js"; +import { + buildProtectedPathHookSettings, + PHAX_APPROVED_PATHS_ENV, +} from "../../src/infra/providers/protectedPathHookSettings.js"; +import { decideProtectedPathApproval } from "../../src/domain/security/protectedPaths.js"; +import { parseClaudeHookPayload } from "../../src/schemas/claudeHookPayload.js"; +import type { AgentRunOptions } from "../../src/ports/backend.js"; +import type { SecurityPolicy } from "../../src/domain/security/types.js"; + +const securePolicy: SecurityPolicy = { + mode: "secure", + filesystem: { + allowRead: ["/tmp/work"], + allowWrite: ["/tmp/work"], + allowWriteProtected: [".claude/skills/"], + }, + network: { profile: "provider-only", allowDomains: ["api.anthropic.com"] }, + mcp: { mode: "disabled", allow: [] }, + failClosed: true, +}; + +const baseOptions = (security: SecurityPolicy): AgentRunOptions => ({ + provider: "claude-code", + model: "claude-sonnet-4-6", + effort: "high", + cwd: "/tmp/work", + security, + approvedProtectedPaths: [], +}); + +// ── Settings builder ────────────────────────────────────────────────────────── + +describe("buildProtectedPathHookSettings", () => { + it("produces a PreToolUse entry with Edit|Write|MultiEdit matcher", () => { + const settings = buildProtectedPathHookSettings( + ["/abs/work/.claude/skills/my-skill/SKILL.md"], + "phax __approve-protected-path", + ); + expect(settings.hooks.PreToolUse).toHaveLength(1); + expect(settings.hooks.PreToolUse[0]!.matcher).toBe("Edit|Write|MultiEdit"); + expect(settings.hooks.PreToolUse[0]!.hooks).toHaveLength(1); + expect(settings.hooks.PreToolUse[0]!.hooks[0]!.type).toBe("command"); + expect(settings.hooks.PreToolUse[0]!.hooks[0]!.command).toBe("phax __approve-protected-path"); + }); + + it("encodes approved paths as JSON in the PHAX_APPROVED_PATHS env var", () => { + const approved = ["/abs/work/.claude/skills/a.md", "/abs/work/.claude/skills/b.md"]; + const settings = buildProtectedPathHookSettings(approved, "phax __approve-protected-path"); + const encoded = settings.env[PHAX_APPROVED_PATHS_ENV]; + expect(encoded).toBeDefined(); + expect(JSON.parse(encoded!)).toEqual(approved); + }); + + it("produces an empty env array when no approved paths are provided", () => { + const settings = buildProtectedPathHookSettings([], "phax __approve-protected-path"); + const encoded = settings.env[PHAX_APPROVED_PATHS_ENV]; + expect(encoded).toBeDefined(); + expect(JSON.parse(encoded!)).toEqual([]); + }); +}); + +// ── buildArgs --settings flag ───────────────────────────────────────────────── + +describe("buildArgs — --settings flag", () => { + it("appends --settings when settingsFilePath is provided", () => { + const args = buildArgs(baseOptions(securePolicy), undefined, "/tmp/work/claude-approval.json"); + expect(args).toContain("--settings"); + const idx = args.indexOf("--settings"); + expect(args[idx + 1]).toBe("/tmp/work/claude-approval.json"); + }); + + it("omits --settings when settingsFilePath is absent", () => { + const args = buildArgs(baseOptions(securePolicy)); + expect(args).not.toContain("--settings"); + }); + + it("omits --settings when settingsFilePath is undefined", () => { + const args = buildArgs(baseOptions(securePolicy), undefined, undefined); + expect(args).not.toContain("--settings"); + }); +}); + +// ── writeProtectedPathSettings ──────────────────────────────────────────────── + +describe("writeProtectedPathSettings", () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), "phax-hook-test-")); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + it("writes the settings file and returns its absolute path when paths are non-empty", () => { + const approved = [join(tmpDir, ".claude/skills/my-skill/SKILL.md")]; + const result = writeProtectedPathSettings(tmpDir, approved); + expect(result).toBeDefined(); + expect(result!.endsWith("claude-protected-approval.settings.json")).toBe(true); + expect(existsSync(result!)).toBe(true); + const content = JSON.parse(readFileSync(result!, "utf8")) as unknown; + expect(content).toMatchObject({ + hooks: { + PreToolUse: [{ matcher: "Edit|Write|MultiEdit" }], + }, + }); + }); + + it("returns undefined when approvedProtectedPaths is empty", () => { + const result = writeProtectedPathSettings(tmpDir, []); + expect(result).toBeUndefined(); + }); + + it("returns undefined when approvedProtectedPaths is undefined", () => { + const result = writeProtectedPathSettings(tmpDir, undefined); + expect(result).toBeUndefined(); + }); + + it("returns undefined when phaseFolderPath is undefined", () => { + const result = writeProtectedPathSettings(undefined, ["/abs/path"]); + expect(result).toBeUndefined(); + }); +}); + +// ── Hook payload decode ─────────────────────────────────────────────────────── + +describe("parseClaudeHookPayload", () => { + it("decodes a valid PreToolUse payload", () => { + const payload = parseClaudeHookPayload( + JSON.stringify({ + tool_name: "Edit", + tool_input: { file_path: "/abs/path/SKILL.md" }, + }), + ); + expect(payload).toBeDefined(); + expect(payload!.tool_name).toBe("Edit"); + expect(payload!.tool_input.file_path).toBe("/abs/path/SKILL.md"); + }); + + it("tolerates and ignores extra fields in the payload", () => { + const payload = parseClaudeHookPayload( + JSON.stringify({ + tool_name: "Write", + tool_input: { file_path: "/abs/path/x.ts", extra_field: "ignored" }, + unknown_top_level: true, + }), + ); + expect(payload).toBeDefined(); + expect(payload!.tool_name).toBe("Write"); + }); + + it("returns undefined for non-JSON input", () => { + expect(parseClaudeHookPayload("not-json")).toBeUndefined(); + }); + + it("returns undefined when tool_name is missing", () => { + expect( + parseClaudeHookPayload(JSON.stringify({ tool_input: { file_path: "/x" } })), + ).toBeUndefined(); + }); +}); + +// ── Domain decision (hook logic) ────────────────────────────────────────────── + +describe("decideProtectedPathApproval via hook logic", () => { + const approved = ["/abs/work/.claude/skills/my-skill/SKILL.md"]; + + it("allows Edit on an approved absolute path", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths: approved, + toolName: "Edit", + filePath: "/abs/work/.claude/skills/my-skill/SKILL.md", + }), + ).toBe("allow"); + }); + + it("allows Write on an approved absolute path", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths: approved, + toolName: "Write", + filePath: "/abs/work/.claude/skills/my-skill/SKILL.md", + }), + ).toBe("allow"); + }); + + it("allows MultiEdit on an approved absolute path", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths: approved, + toolName: "MultiEdit", + filePath: "/abs/work/.claude/skills/my-skill/SKILL.md", + }), + ).toBe("allow"); + }); + + it("defers for a non-approved path", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths: approved, + toolName: "Edit", + filePath: "/abs/work/.claude/skills/other/SKILL.md", + }), + ).toBe("defer"); + }); + + it("defers for a non-edit tool (e.g. Bash)", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths: approved, + toolName: "Bash", + filePath: "/abs/work/.claude/skills/my-skill/SKILL.md", + }), + ).toBe("defer"); + }); + + it("defers when filePath is undefined", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths: approved, + toolName: "Edit", + filePath: undefined, + }), + ).toBe("defer"); + }); + + it("defers when approved list is empty", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths: [], + toolName: "Edit", + filePath: "/abs/work/.claude/skills/my-skill/SKILL.md", + }), + ).toBe("defer"); + }); +}); diff --git a/tests/integration/cliProgram.test.ts b/tests/integration/cliProgram.test.ts index 9753dd00..c9e7a180 100644 --- a/tests/integration/cliProgram.test.ts +++ b/tests/integration/cliProgram.test.ts @@ -39,7 +39,10 @@ describe("buildProgram", () => { it("exposes the expected top-level commands", () => { const program = buildProgram(); - const names = program.commands.map((c) => c.name()); + // Only count visible commands; internal subcommands (__ prefix, e.g. __approve-protected-path) + // are excluded from the public surface check. + const visibleCommands = program.commands.filter((c) => !c.name().startsWith("__")); + const names = visibleCommands.map((c) => c.name()); for (const name of TOP_LEVEL_COMMANDS) { expect(names, `expected top-level command '${name}'`).toContain(name); } diff --git a/tests/integration/enter.test.ts b/tests/integration/enter.test.ts index b23b49e7..f035f44f 100644 --- a/tests/integration/enter.test.ts +++ b/tests/integration/enter.test.ts @@ -119,7 +119,7 @@ describe("runEnter", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/integration/enterPhase.test.ts b/tests/integration/enterPhase.test.ts index fd2b4338..641d05d1 100644 --- a/tests/integration/enterPhase.test.ts +++ b/tests/integration/enterPhase.test.ts @@ -176,7 +176,7 @@ describe("runEnterPhase", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/integration/executePlan.test.ts b/tests/integration/executePlan.test.ts index c2bfb569..8005ceac 100644 --- a/tests/integration/executePlan.test.ts +++ b/tests/integration/executePlan.test.ts @@ -184,7 +184,7 @@ describe("executePlan — happy-path 2-phase run", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -347,7 +347,7 @@ describe("executePlan — happy-path 2-phase run", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -455,7 +455,7 @@ describe("executePlan — happy-path 2-phase run", () => { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -567,7 +567,7 @@ describe("executePlan — happy-path 2-phase run", () => { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -645,7 +645,7 @@ function makeStatusTestConfig(root: string): ResolvedConfig { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -1055,7 +1055,7 @@ describe("executePlan — resume from gates_exhausted", () => { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -1433,7 +1433,7 @@ function makePublishBaseConfig( fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -1713,7 +1713,7 @@ describe("executePlan — security preflight", () => { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -1824,7 +1824,7 @@ describe("executePlan — security preflight", () => { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: ["deno fmt"], diff --git a/tests/integration/perPhaseBranch.test.ts b/tests/integration/perPhaseBranch.test.ts index 64ce9542..8e0a3f11 100644 --- a/tests/integration/perPhaseBranch.test.ts +++ b/tests/integration/perPhaseBranch.test.ts @@ -167,7 +167,7 @@ describe("executePlan — per-phase branch regression", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -309,7 +309,7 @@ describe("executePlan — per-phase branch regression", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/plansOverlapCommand.test.ts b/tests/integration/plansOverlapCommand.test.ts index ce9f50ea..db57bd64 100644 --- a/tests/integration/plansOverlapCommand.test.ts +++ b/tests/integration/plansOverlapCommand.test.ts @@ -24,7 +24,7 @@ function makeBaseConfig(stateRoot: string) { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe" as const, - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only" as const, allowDomains: [] }, mcp: { mode: "disabled" as const, allow: [] }, agentCommands: [], diff --git a/tests/integration/plansOverlapLanded.test.ts b/tests/integration/plansOverlapLanded.test.ts index a702cb6c..28a8b617 100644 --- a/tests/integration/plansOverlapLanded.test.ts +++ b/tests/integration/plansOverlapLanded.test.ts @@ -25,7 +25,7 @@ function makeBaseConfig(stateRoot: string) { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe" as const, - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only" as const, allowDomains: [] }, mcp: { mode: "disabled" as const, allow: [] }, agentCommands: [], diff --git a/tests/integration/protectedPathApprovals.test.ts b/tests/integration/protectedPathApprovals.test.ts new file mode 100644 index 00000000..08c052cb --- /dev/null +++ b/tests/integration/protectedPathApprovals.test.ts @@ -0,0 +1,321 @@ +import { Effect, Either, Layer } from "effect"; +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { executePlan } from "../../src/app/executePlan.js"; +import { createRunFolder } from "../../src/app/runFolder.js"; +import { decodeShortName } from "../../src/domain/branded.js"; +import type { ClaudeSessionId } from "../../src/domain/branded.js"; +import { SecurityPreflightError } from "../../src/domain/errors.js"; +import { makeFakeBackend } from "../../src/infra/fakes/backend.js"; +import { makeFakeGit } from "../../src/infra/fakes/git.js"; +import { makeFakeGitHub } from "../../src/infra/fakes/github.js"; +import { makeFakeShell } from "../../src/infra/fakes/shell.js"; +import { NodeFileSystemLayer } from "../../src/infra/fs.js"; +import { NoopSystemTelemetryLayer } from "../../src/ports/systemTelemetry.js"; +import type { ResolvedConfig } from "../../src/schemas/phaxConfig.js"; +import { decodePhaxPlan } from "../../src/schemas/phaxPlan.js"; + +const HANDOFF_CONTENT = [ + "## What was delivered", + "Phase completed successfully.", + "## Key decisions and why", + "No major decisions.", + "## Exact locations (file paths and exported names)", + "No new exports.", + "## What the next phase needs to know", + "Ready to proceed.", +].join("\n"); + +const shortName = Either.getOrThrow(decodeShortName("my-run")); + +function makeMinimalConfig( + stateRoot: string, + overrides?: { + allowWriteProtected?: readonly string[]; + profile?: "secure" | "unsafe" | "isolated"; + }, +): ResolvedConfig { + return { + raw: { + version: 1, + project: { name: "test-project", type: "single-package" }, + state: { root: stateRoot }, + gateProfiles: { full: ["true"] }, + commands: { setup: ["true"], cleanup: ["true"] }, + }, + stateRoot, + namespace: "test-project", + repoRoot: stateRoot, + maxFixAttempts: 1, + extractPlanModel: "claude-haiku-4-5-20251001", + extractPlanEffort: "low" as const, + fileReconciliationMode: "report_only" as const, + security: { + profile: overrides?.profile ?? "unsafe", + filesystem: { + allowRead: [], + allowWrite: [], + allowWriteProtected: overrides?.allowWriteProtected ?? [], + }, + network: { profile: "provider-only", allowDomains: [] }, + mcp: { mode: "disabled", allow: [] }, + agentCommands: [], + }, + }; +} + +function makeLayers(stateRoot: string, sessionId: string) { + const phase01Worktree = join(stateRoot, "worktrees", "test-project.my-run", "phase-01"); + const fakeGit = makeFakeGit(); + fakeGit.impl.setRepoIsClean(true); + fakeGit.impl.enqueueWorktreeIsClean(phase01Worktree, false); + + const fakeShell = makeFakeShell(); + fakeShell.impl.setResponse("true", { exitCode: 0, stdout: "", stderr: "" }); + fakeShell.impl.setResponse("git rev-parse HEAD", { + exitCode: 0, + stdout: "deadbeef12345678\n", + stderr: "", + }); + fakeShell.impl.setResponse("git diff HEAD^ HEAD", { exitCode: 0, stdout: "", stderr: "" }); + + const fakeBackend = makeFakeBackend(); + fakeBackend.impl.addRunResponse({ + sessionId: sessionId as ClaudeSessionId, + outputPath: "", + finalText: "", + }); + fakeBackend.impl.addResumeResponse({ + sessionId: `${sessionId}-handoff` as ClaudeSessionId, + outputPath: "", + finalText: "", + }); + + return { + layers: Layer.mergeAll( + fakeGit.layer, + fakeShell.layer, + fakeBackend.layer, + makeFakeGitHub().layer, + NodeFileSystemLayer, + NoopSystemTelemetryLayer, + ), + fakeBackend, + phase01Worktree, + }; +} + +describe("executePlan — protected-path approvals", () => { + let stateRoot: string; + + beforeEach(async () => { + stateRoot = await mkdtemp(join(tmpdir(), "phax-protected-test-")); + }); + + afterEach(async () => { + await rm(stateRoot, { recursive: true, force: true }); + }); + + it("passes approvedProtectedPaths to runAgent when phase declares covered protected file", async () => { + const phase01Worktree = join(stateRoot, "worktrees", "test-project.my-run", "phase-01"); + await mkdir(join(phase01Worktree, ".phax-context"), { recursive: true }); + const { layers, fakeBackend } = makeLayers(stateRoot, "sess-01"); + // Pre-create worktree handoff so generatePhaseHandoff can find it. + const { writeFile } = await import("node:fs/promises"); + await writeFile(join(phase01Worktree, ".phax-context", "phase-handoff.md"), HANDOFF_CONTENT); + + const rawPlan = { + version: 1 as const, + run: { + shortName: "my-run", + title: "My Run", + branch: "ai/my-run", + requiredCommands: [], + }, + phases: [ + { + id: "phase-01", + title: "First Phase", + model: "claude-sonnet-4-6", + effort: "low" as const, + planMarkdownAnchor: "#phase-01", + plannedFilesToCreate: [".claude/skills/my-skill/SKILL.md"], + plannedFilesToEdit: [], + optionalFilesToEdit: [], + commit: { subject: "ai(phase-01): add skill", body: "Adds the skill." }, + }, + ], + } as const; + + const plan = Either.getOrThrow(decodePhaxPlan(rawPlan)); + const config = makeMinimalConfig(stateRoot, { + allowWriteProtected: [".claude/skills/"], + }); + + const { runPath, runId } = await Effect.runPromise( + createRunFolder(shortName, "# My Plan", plan, config).pipe(Effect.provide(layers)), + ); + + const result = await Effect.runPromise( + Effect.either( + executePlan({ + shortName, + namespace: "test-project", + plan, + planMd: "# My Plan", + config, + gateProfileId: "full", + allowDirty: false, + runPath, + runId, + startIndex: 0, + // Use secure mode so the policy carries allowWriteProtected from config + // and the approval is actually computed and passed through. + securityMode: "secure", + }).pipe(Effect.provide(layers)), + ), + ); + + expect(Either.isRight(result)).toBe(true); + expect(fakeBackend.impl.runCalls).toHaveLength(1); + const call = fakeBackend.impl.runCalls[0]!; + const approved = call.options.approvedProtectedPaths; + expect(approved).toBeDefined(); + expect(approved).toHaveLength(1); + // The approved path must be absolute and end with the repo-relative path. + expect(approved![0]).toContain(".claude/skills/my-skill/SKILL.md"); + expect(approved![0]!.startsWith("/")).toBe(true); + }); + + it("fails preflight with SecurityPreflightError when phase declares protected path not opted in", async () => { + const rawPlan = { + version: 1 as const, + run: { + shortName: "my-run", + title: "My Run", + branch: "ai/my-run", + requiredCommands: [], + }, + phases: [ + { + id: "phase-01", + title: "First Phase", + model: "claude-sonnet-4-6", + effort: "low" as const, + planMarkdownAnchor: "#phase-01", + plannedFilesToCreate: [".claude/skills/my-skill/SKILL.md"], + plannedFilesToEdit: [], + optionalFilesToEdit: [], + commit: { subject: "ai(phase-01): add skill", body: "Adds the skill." }, + }, + ], + } as const; + + const plan = Either.getOrThrow(decodePhaxPlan(rawPlan)); + // allowWriteProtected is empty — operator has NOT opted in. + const config = makeMinimalConfig(stateRoot, { allowWriteProtected: [] }); + + const phase01Worktree = join(stateRoot, "worktrees", "test-project.my-run", "phase-01"); + const { layers, fakeBackend } = makeLayers(stateRoot, "sess-01"); + await mkdir(join(phase01Worktree, ".phax-context"), { recursive: true }); + + const { runPath, runId } = await Effect.runPromise( + createRunFolder(shortName, "# My Plan", plan, config).pipe(Effect.provide(layers)), + ); + + const result = await Effect.runPromise( + Effect.either( + executePlan({ + shortName, + namespace: "test-project", + plan, + planMd: "# My Plan", + config, + gateProfileId: "full", + allowDirty: false, + runPath, + runId, + startIndex: 0, + // Use secure mode so the preflight check runs (it only runs in secure mode). + securityMode: "secure", + }).pipe(Effect.provide(layers)), + ), + ); + + expect(Either.isLeft(result)).toBe(true); + if (Either.isLeft(result)) { + expect(result.left).toBeInstanceOf(SecurityPreflightError); + const err = result.left as SecurityPreflightError; + expect(err.message).toContain("phase-01"); + expect(err.message).toContain("allowWriteProtected"); + expect(err.missing).toContain(".claude/skills/my-skill/SKILL.md"); + } + // Backend must never be called — preflight rejects before spawn. + expect(fakeBackend.impl.runCalls).toHaveLength(0); + }); + + it("passes empty approvedProtectedPaths when phase declares only non-protected files", async () => { + const phase01Worktree = join(stateRoot, "worktrees", "test-project.my-run", "phase-01"); + await mkdir(join(phase01Worktree, ".phax-context"), { recursive: true }); + const { writeFile } = await import("node:fs/promises"); + await writeFile(join(phase01Worktree, ".phax-context", "phase-handoff.md"), HANDOFF_CONTENT); + + const rawPlan = { + version: 1 as const, + run: { + shortName: "my-run", + title: "My Run", + branch: "ai/my-run", + requiredCommands: [], + }, + phases: [ + { + id: "phase-01", + title: "First Phase", + model: "claude-sonnet-4-6", + effort: "low" as const, + planMarkdownAnchor: "#phase-01", + plannedFilesToCreate: ["src/foo.ts"], + plannedFilesToEdit: ["src/bar.ts"], + optionalFilesToEdit: ["README.md"], + commit: { subject: "ai(phase-01): normal changes", body: "Normal phase." }, + }, + ], + } as const; + + const plan = Either.getOrThrow(decodePhaxPlan(rawPlan)); + // No allowWriteProtected needed — no protected paths declared. + const config = makeMinimalConfig(stateRoot, { allowWriteProtected: [] }); + const { layers, fakeBackend } = makeLayers(stateRoot, "sess-01"); + + const { runPath, runId } = await Effect.runPromise( + createRunFolder(shortName, "# My Plan", plan, config).pipe(Effect.provide(layers)), + ); + + const result = await Effect.runPromise( + Effect.either( + executePlan({ + shortName, + namespace: "test-project", + plan, + planMd: "# My Plan", + config, + gateProfileId: "full", + allowDirty: false, + runPath, + runId, + startIndex: 0, + }).pipe(Effect.provide(layers)), + ), + ); + + expect(Either.isRight(result)).toBe(true); + expect(fakeBackend.impl.runCalls).toHaveLength(1); + const call = fakeBackend.impl.runCalls[0]!; + // No protected paths → empty (or undefined) approvedProtectedPaths. + const approved = call.options.approvedProtectedPaths; + expect(approved === undefined || approved.length === 0).toBe(true); + }); +}); diff --git a/tests/integration/providerDispatcher.test.ts b/tests/integration/providerDispatcher.test.ts index 271f62e9..2f289193 100644 --- a/tests/integration/providerDispatcher.test.ts +++ b/tests/integration/providerDispatcher.test.ts @@ -12,7 +12,7 @@ const baseOptions: AgentRunOptions = { cwd: "/tmp", security: { mode: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "open", allowDomains: [] }, mcp: { mode: "provider-default", allow: [] }, failClosed: false, diff --git a/tests/integration/rateLimit.test.ts b/tests/integration/rateLimit.test.ts index e9df39a6..8eedf29e 100644 --- a/tests/integration/rateLimit.test.ts +++ b/tests/integration/rateLimit.test.ts @@ -85,7 +85,7 @@ function makeConfig(stateRoot: string): ResolvedConfig { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/reconciliation.test.ts b/tests/integration/reconciliation.test.ts index d8182028..5fe36a28 100644 --- a/tests/integration/reconciliation.test.ts +++ b/tests/integration/reconciliation.test.ts @@ -88,7 +88,7 @@ describe("reconcilePhaseFiles — lifecycle wiring", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -204,7 +204,7 @@ describe("reconcilePhaseFiles — lifecycle wiring", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/resume.test.ts b/tests/integration/resume.test.ts index 5629305a..3da8ae19 100644 --- a/tests/integration/resume.test.ts +++ b/tests/integration/resume.test.ts @@ -94,7 +94,7 @@ describe("executePlan — resume from startIndex: 1", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -272,7 +272,7 @@ describe("executePlan — resume from startIndex: 1", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/resumeFromCleanup.test.ts b/tests/integration/resumeFromCleanup.test.ts index 017a98e9..cbb94ef8 100644 --- a/tests/integration/resumeFromCleanup.test.ts +++ b/tests/integration/resumeFromCleanup.test.ts @@ -81,7 +81,7 @@ function makeConfig(stateRoot: string): ResolvedConfig { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/resumeFromCommit.test.ts b/tests/integration/resumeFromCommit.test.ts index 58da740e..0f493d08 100644 --- a/tests/integration/resumeFromCommit.test.ts +++ b/tests/integration/resumeFromCommit.test.ts @@ -81,7 +81,7 @@ function makeConfig(stateRoot: string): ResolvedConfig { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/resumeHandoff.test.ts b/tests/integration/resumeHandoff.test.ts index f9dd25af..56cb9d1c 100644 --- a/tests/integration/resumeHandoff.test.ts +++ b/tests/integration/resumeHandoff.test.ts @@ -81,7 +81,7 @@ function makeConfig(stateRoot: string): ResolvedConfig { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/reviewCodeCommand.test.ts b/tests/integration/reviewCodeCommand.test.ts index 0d0bc828..b3613946 100644 --- a/tests/integration/reviewCodeCommand.test.ts +++ b/tests/integration/reviewCodeCommand.test.ts @@ -25,7 +25,7 @@ function makeBaseConfig(stateRootOverride: string) { fileReconciliationMode: "report_only" as const, security: { profile: "unsafe" as const, - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only" as const, allowDomains: [] }, mcp: { mode: "disabled" as const, allow: [] }, agentCommands: [], diff --git a/tests/integration/reviewCompliance.test.ts b/tests/integration/reviewCompliance.test.ts index 77ab2001..d8c8e47d 100644 --- a/tests/integration/reviewCompliance.test.ts +++ b/tests/integration/reviewCompliance.test.ts @@ -78,7 +78,7 @@ const fakeSecurity = { mode: "secure" as const, config: { profile: "secure", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only" }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/reviewComplianceCommand.test.ts b/tests/integration/reviewComplianceCommand.test.ts index eff068f0..5eea1474 100644 --- a/tests/integration/reviewComplianceCommand.test.ts +++ b/tests/integration/reviewComplianceCommand.test.ts @@ -74,7 +74,7 @@ const fakeSecurity = { mode: "secure" as const, config: { profile: "secure", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only" }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/routing.test.ts b/tests/integration/routing.test.ts index 480b5681..221be684 100644 --- a/tests/integration/routing.test.ts +++ b/tests/integration/routing.test.ts @@ -126,7 +126,7 @@ describe("executePlan routing — mistral-vibe priority", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/runFolder.test.ts b/tests/integration/runFolder.test.ts index 2cf1b9c8..43bccb49 100644 --- a/tests/integration/runFolder.test.ts +++ b/tests/integration/runFolder.test.ts @@ -28,7 +28,7 @@ const resolvedConfig: ResolvedConfig = { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/integration/sessionInfo.test.ts b/tests/integration/sessionInfo.test.ts index 666b68f6..ef79f4e1 100644 --- a/tests/integration/sessionInfo.test.ts +++ b/tests/integration/sessionInfo.test.ts @@ -127,7 +127,7 @@ describe("runSessionInfo", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/integration/setupFailure.test.ts b/tests/integration/setupFailure.test.ts index dad0060c..339523a2 100644 --- a/tests/integration/setupFailure.test.ts +++ b/tests/integration/setupFailure.test.ts @@ -72,7 +72,7 @@ describe("executePlan — setup command failure", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -140,7 +140,7 @@ describe("executePlan — setup command failure", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -208,7 +208,7 @@ describe("executePlan — setup command failure", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/stateMachineContract.test.ts b/tests/integration/stateMachineContract.test.ts index 1c948057..70232e0d 100644 --- a/tests/integration/stateMachineContract.test.ts +++ b/tests/integration/stateMachineContract.test.ts @@ -81,7 +81,7 @@ describe("State Machine Contract", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -174,7 +174,7 @@ describe("State Machine Contract", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -269,7 +269,7 @@ describe("State Machine Contract", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/integration/telemetry/end-to-end.test.ts b/tests/integration/telemetry/end-to-end.test.ts index c2e88e73..d276e79d 100644 --- a/tests/integration/telemetry/end-to-end.test.ts +++ b/tests/integration/telemetry/end-to-end.test.ts @@ -81,7 +81,7 @@ describe("executePlan — semantic telemetry end-to-end", () => { security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/unit/dryRun.test.ts b/tests/unit/dryRun.test.ts index 63d733ca..eb88a2ab 100644 --- a/tests/unit/dryRun.test.ts +++ b/tests/unit/dryRun.test.ts @@ -39,7 +39,7 @@ const minimalConfig: ResolvedConfig = { }, security: { profile: "secure", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], diff --git a/tests/unit/gateProfile.test.ts b/tests/unit/gateProfile.test.ts index 5a971393..fccf565a 100644 --- a/tests/unit/gateProfile.test.ts +++ b/tests/unit/gateProfile.test.ts @@ -24,7 +24,7 @@ function makeConfig(overrides?: Partial): ResolvedConfig security: { profile: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only", allowDomains: [] }, mcp: { mode: "disabled", allow: [] }, }, diff --git a/tests/unit/protectedPaths.test.ts b/tests/unit/protectedPaths.test.ts new file mode 100644 index 00000000..6ba69b09 --- /dev/null +++ b/tests/unit/protectedPaths.test.ts @@ -0,0 +1,180 @@ +import { describe, expect, it } from "vitest"; +import { + CLAUDE_PROTECTED_PREFIXES, + decideProtectedPathApproval, + isProtectedPath, + resolveProtectedApprovals, +} from "../../src/domain/security/protectedPaths.js"; + +describe("CLAUDE_PROTECTED_PREFIXES", () => { + it("includes .claude/ but does not include .claude/worktrees/", () => { + expect(CLAUDE_PROTECTED_PREFIXES).toContain(".claude/"); + expect(CLAUDE_PROTECTED_PREFIXES).not.toContain(".claude/worktrees/"); + }); +}); + +describe("isProtectedPath", () => { + it("returns true for .claude/skills/x.md", () => { + expect(isProtectedPath(".claude/skills/x.md")).toBe(true); + }); + + it("returns false for .claude/worktrees/foo", () => { + expect(isProtectedPath(".claude/worktrees/foo")).toBe(false); + }); + + it("returns false for non-protected source paths", () => { + expect(isProtectedPath("src/x.ts")).toBe(false); + expect(isProtectedPath("docs/security/x.md")).toBe(false); + }); + + it("normalizes leading ./ and redundant segments", () => { + expect(isProtectedPath("./.claude/skills/y.md")).toBe(true); + expect(isProtectedPath(".claude/./skills/../skills/y.md")).toBe(true); + }); + + it("does not match bare '.claude' directory entry as protected file (boundary)", () => { + // `.claude` alone (without trailing path) is the configured directory + // root; treat it as protected so an edit attempt is recognized. + expect(isProtectedPath(".claude")).toBe(true); + }); + + it("rejects paths that escape the repo root", () => { + expect(isProtectedPath("../outside")).toBe(false); + }); +}); + +describe("resolveProtectedApprovals", () => { + const worktreeRoot = "/tmp/run/wt"; + + it("partitions protected paths against allowWriteProtected prefixes", () => { + const result = resolveProtectedApprovals({ + plannedPaths: [".claude/skills/a.md", ".claude/hooks/b.sh", "src/index.ts"], + allowWriteProtected: [".claude/skills/"], + worktreeRoot, + }); + + expect(result.approved).toEqual(["/tmp/run/wt/.claude/skills/a.md"]); + expect(result.uncovered).toEqual([".claude/hooks/b.sh"]); + }); + + it("returns every protected path as uncovered when allowWriteProtected is empty", () => { + const result = resolveProtectedApprovals({ + plannedPaths: [".claude/skills/a.md", ".claude/hooks/b.sh", "src/x.ts"], + allowWriteProtected: [], + worktreeRoot, + }); + + expect(result.approved).toEqual([]); + expect(result.uncovered).toEqual([".claude/skills/a.md", ".claude/hooks/b.sh"]); + }); + + it("ignores non-protected paths entirely", () => { + const result = resolveProtectedApprovals({ + plannedPaths: ["src/a.ts", "docs/b.md"], + allowWriteProtected: [".claude/skills/"], + worktreeRoot, + }); + + expect(result.approved).toEqual([]); + expect(result.uncovered).toEqual([]); + }); + + it("deduplicates while preserving input order", () => { + const result = resolveProtectedApprovals({ + plannedPaths: [".claude/skills/a.md", "./.claude/skills/a.md", ".claude/skills/b.md"], + allowWriteProtected: [".claude/skills/"], + worktreeRoot, + }); + + expect(result.approved).toEqual([ + "/tmp/run/wt/.claude/skills/a.md", + "/tmp/run/wt/.claude/skills/b.md", + ]); + }); + + it("treats a prefix without trailing slash the same as with one", () => { + const result = resolveProtectedApprovals({ + plannedPaths: [".claude/skills/a.md"], + allowWriteProtected: [".claude/skills"], + worktreeRoot, + }); + + expect(result.approved).toEqual(["/tmp/run/wt/.claude/skills/a.md"]); + }); +}); + +describe("decideProtectedPathApproval", () => { + const approvedAbsolutePaths = ["/tmp/run/wt/.claude/skills/a.md"]; + + it("allows exact match on Edit", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths, + toolName: "Edit", + filePath: "/tmp/run/wt/.claude/skills/a.md", + }), + ).toBe("allow"); + }); + + it("allows exact match on Write and MultiEdit", () => { + for (const toolName of ["Write", "MultiEdit"]) { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths, + toolName, + filePath: "/tmp/run/wt/.claude/skills/a.md", + }), + ).toBe("allow"); + } + }); + + it("defers on a non-matching path", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths, + toolName: "Edit", + filePath: "/tmp/run/wt/.claude/skills/b.md", + }), + ).toBe("defer"); + }); + + it("defers on a non-edit tool name", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths, + toolName: "Bash", + filePath: "/tmp/run/wt/.claude/skills/a.md", + }), + ).toBe("defer"); + }); + + it("defers when filePath is missing", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths, + toolName: "Edit", + filePath: undefined, + }), + ).toBe("defer"); + }); + + it("defers on relative filePath (must be absolute)", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths, + toolName: "Edit", + filePath: ".claude/skills/a.md", + }), + ).toBe("defer"); + }); + + it("normalizes redundant path segments before matching", () => { + expect( + decideProtectedPathApproval({ + approvedAbsolutePaths, + toolName: "Edit", + filePath: "/tmp/run/wt/./.claude/skills/a.md", + }), + ).toBe("allow"); + }); +}); diff --git a/tests/unit/providers/claudeCode.test.ts b/tests/unit/providers/claudeCode.test.ts index 36ab253e..b94c9e05 100644 --- a/tests/unit/providers/claudeCode.test.ts +++ b/tests/unit/providers/claudeCode.test.ts @@ -10,7 +10,7 @@ import type { SecurityPolicy } from "../../../src/domain/security/types.js"; const unsafePolicy: SecurityPolicy = { mode: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "open", allowDomains: [] }, mcp: { mode: "provider-default", allow: [] }, failClosed: false, @@ -21,6 +21,7 @@ const securePolicy: SecurityPolicy = { filesystem: { allowRead: ["/tmp/work", "/home/me/.phax"], allowWrite: ["/tmp/work", "/home/me/.phax"], + allowWriteProtected: [], }, network: { profile: "provider-only", allowDomains: ["api.anthropic.com"] }, mcp: { mode: "disabled", allow: [] }, @@ -245,7 +246,7 @@ describe("buildArgs — secure mode fail-closed", () => { it("throws SecurityEnforcementError when secure policy has no writable paths", () => { const impossiblePolicy: SecurityPolicy = { ...securePolicy, - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, }; try { buildArgs(baseOptions(impossiblePolicy)); diff --git a/tests/unit/providers/codexCli.test.ts b/tests/unit/providers/codexCli.test.ts index 3c576259..7b96de2d 100644 --- a/tests/unit/providers/codexCli.test.ts +++ b/tests/unit/providers/codexCli.test.ts @@ -23,7 +23,7 @@ const sampleLines = readFileSync(join(fixtureDir, "codex-exec-sample.jsonl"), "u const unsafePolicy: SecurityPolicy = { mode: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "open", allowDomains: [] }, mcp: { mode: "provider-default", allow: [] }, failClosed: false, @@ -34,6 +34,7 @@ const securePolicy: SecurityPolicy = { filesystem: { allowRead: ["/tmp/work", "/home/me/.phax"], allowWrite: ["/tmp/work", "/home/me/.phax"], + allowWriteProtected: [], }, network: { profile: "provider-only", allowDomains: ["api.openai.com"] }, mcp: { mode: "disabled", allow: [] }, @@ -161,7 +162,7 @@ describe("buildCodexArgs — secure mode fail-closed", () => { it("throws SecurityEnforcementError when secure policy has no writable paths", () => { const impossiblePolicy: SecurityPolicy = { ...securePolicy, - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, }; try { buildCodexArgs(baseEntry, baseOptions(impossiblePolicy)); diff --git a/tests/unit/providers/mistralVibe.test.ts b/tests/unit/providers/mistralVibe.test.ts index 9b0ac8f3..a497b9e0 100644 --- a/tests/unit/providers/mistralVibe.test.ts +++ b/tests/unit/providers/mistralVibe.test.ts @@ -20,7 +20,7 @@ const baseEntry = { const unsafePolicy: SecurityPolicy = { mode: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "open", allowDomains: [] }, mcp: { mode: "provider-default", allow: [] }, failClosed: false, @@ -31,6 +31,7 @@ const securePolicy: SecurityPolicy = { filesystem: { allowRead: ["/tmp/work", "/home/me/.phax"], allowWrite: ["/tmp/work", "/home/me/.phax"], + allowWriteProtected: [], }, network: { profile: "provider-only", allowDomains: ["api.mistral.ai"] }, mcp: { mode: "disabled", allow: [] }, @@ -128,7 +129,7 @@ describe("buildVibeArgs — secure mode fail-closed", () => { it("throws SecurityEnforcementError when secure policy has no writable paths", () => { const impossiblePolicy: SecurityPolicy = { ...securePolicy, - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, }; try { buildVibeArgs(baseEntry, "p", baseOptions(impossiblePolicy)); diff --git a/tests/unit/resolvePolicyProtected.test.ts b/tests/unit/resolvePolicyProtected.test.ts new file mode 100644 index 00000000..cbaf9272 --- /dev/null +++ b/tests/unit/resolvePolicyProtected.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "vitest"; +import { Schema } from "effect"; +import { resolveSecurityConfig, SecurityConfigSchema } from "../../src/schemas/securityConfig.js"; +import { resolveSecurityPolicy } from "../../src/domain/security/resolvePolicy.js"; +import type { ResolvedSecurityConfig } from "../../src/schemas/securityConfig.js"; + +const baseConfig: ResolvedSecurityConfig = { + profile: "secure", + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, + network: { profile: "provider-only" }, + mcp: { mode: "disabled", allow: [] }, + agentCommands: [], +}; + +describe("resolveSecurityConfig — allowWriteProtected", () => { + it("defaults to [] when filesystem is absent", () => { + const resolved = resolveSecurityConfig(undefined, "secure"); + expect(resolved.filesystem.allowWriteProtected).toEqual([]); + }); + + it("defaults to [] when filesystem.allowWriteProtected is absent", () => { + const resolved = resolveSecurityConfig({ filesystem: { allowRead: ["/foo"] } }, "secure"); + expect(resolved.filesystem.allowWriteProtected).toEqual([]); + }); + + it("passes a provided allowWriteProtected array through", () => { + const resolved = resolveSecurityConfig( + { filesystem: { allowWriteProtected: [".claude/skills/", ".claude/commands/"] } }, + "secure", + ); + expect(resolved.filesystem.allowWriteProtected).toEqual([ + ".claude/skills/", + ".claude/commands/", + ]); + }); + + it("decodes through SecurityConfigSchema without error", () => { + const raw = { + security: { + filesystem: { + allowWriteProtected: [".claude/skills/"], + }, + }, + }; + const decode = Schema.decodeUnknownSync( + Schema.Struct({ security: Schema.optional(SecurityConfigSchema) }), + ); + const result = decode(raw); + expect(result.security?.filesystem?.allowWriteProtected).toEqual([".claude/skills/"]); + }); +}); + +describe("resolveSecurityPolicy — allowWriteProtected in secure mode", () => { + it("carries allowWriteProtected from config into the policy", () => { + const config: ResolvedSecurityConfig = { + ...baseConfig, + filesystem: { ...baseConfig.filesystem, allowWriteProtected: [".claude/skills/"] }, + }; + const policy = resolveSecurityPolicy({ mode: "secure", worktreePath: "/repo/wt", config }); + expect(policy.filesystem.allowWriteProtected).toEqual([".claude/skills/"]); + }); + + it("defaults to [] when config has no allowWriteProtected", () => { + const policy = resolveSecurityPolicy({ + mode: "secure", + worktreePath: "/repo/wt", + config: baseConfig, + }); + expect(policy.filesystem.allowWriteProtected).toEqual([]); + }); +}); + +describe("resolveSecurityPolicy — allowWriteProtected in unsafe mode", () => { + it("is always [] in unsafe mode regardless of config", () => { + const config: ResolvedSecurityConfig = { + ...baseConfig, + filesystem: { ...baseConfig.filesystem, allowWriteProtected: [".claude/skills/"] }, + }; + const policy = resolveSecurityPolicy({ mode: "unsafe", worktreePath: "/repo/wt", config }); + expect(policy.filesystem.allowWriteProtected).toEqual([]); + }); +}); diff --git a/tests/unit/security/capabilities.test.ts b/tests/unit/security/capabilities.test.ts index d17b46d4..0ae14d46 100644 --- a/tests/unit/security/capabilities.test.ts +++ b/tests/unit/security/capabilities.test.ts @@ -24,7 +24,7 @@ void commandEnforcementSample; const securePolicy: SecurityPolicy = { mode: "secure", - filesystem: { allowRead: ["/repo"], allowWrite: ["/repo"] }, + filesystem: { allowRead: ["/repo"], allowWrite: ["/repo"], allowWriteProtected: [] }, network: { profile: "provider-only" }, mcp: { mode: "disabled", allow: [] }, failClosed: true, @@ -32,7 +32,7 @@ const securePolicy: SecurityPolicy = { const unsafePolicy: SecurityPolicy = { mode: "unsafe", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "open" }, mcp: { mode: "provider-default", allow: [] }, failClosed: false, diff --git a/tests/unit/security/posture.test.ts b/tests/unit/security/posture.test.ts index 71b2a109..798ad8af 100644 --- a/tests/unit/security/posture.test.ts +++ b/tests/unit/security/posture.test.ts @@ -26,6 +26,7 @@ const baseSecurePosture = { marks: [] as const, agentCommands: [] as const, providerSkippedForSecurity: [], + approvedProtectedPaths: [], }; const unsafePosture = { @@ -48,6 +49,7 @@ const unsafePosture = { marks: [] as const, agentCommands: [] as const, providerSkippedForSecurity: [], + approvedProtectedPaths: [], }; const downgradedVibePosture = { @@ -70,6 +72,7 @@ const downgradedVibePosture = { marks: ["partial-filesystem", "mcp-unenforced"] as const, agentCommands: [] as const, providerSkippedForSecurity: [], + approvedProtectedPaths: [], }; const withSkippedProviders = { @@ -94,6 +97,7 @@ const withSkippedProviders = { providerSkippedForSecurity: [ { provider: "mistral-vibe" as const, reason: "cannot satisfy strict secure mode" }, ], + approvedProtectedPaths: [], }; describe("SecurityPostureSchema", () => { diff --git a/tests/unit/security/resolvePolicy.test.ts b/tests/unit/security/resolvePolicy.test.ts index 6f78c522..3366c14b 100644 --- a/tests/unit/security/resolvePolicy.test.ts +++ b/tests/unit/security/resolvePolicy.test.ts @@ -4,7 +4,7 @@ import type { ResolvedSecurityConfig } from "../../../src/schemas/securityConfig const baseSecureConfig: ResolvedSecurityConfig = { profile: "secure", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only" }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -12,7 +12,7 @@ const baseSecureConfig: ResolvedSecurityConfig = { const devAllowlistConfig: ResolvedSecurityConfig = { profile: "secure", - filesystem: { allowRead: ["/extra/read"], allowWrite: ["/extra/write"] }, + filesystem: { allowRead: ["/extra/read"], allowWrite: ["/extra/write"], allowWriteProtected: [] }, network: { profile: "dev-allowlist" }, mcp: { mode: "allowlist", allow: ["my-mcp"] }, agentCommands: [], @@ -79,7 +79,7 @@ describe("resolveSecurityPolicy — secure mode, provider-only network", () => { worktreePath: "/home/user/.phax/worktrees/run/phase-01", config: { ...baseSecureConfig, - filesystem: { allowRead: [], allowWrite: ["/home/user/.phax"] }, + filesystem: { allowRead: [], allowWrite: ["/home/user/.phax"], allowWriteProtected: [] }, }, }); expect(policy.filesystem.allowWrite).toContain("/home/user/.phax"); @@ -145,7 +145,7 @@ describe("resolveSecurityPolicy — secure mode, provider-only network", () => { worktreePath: "/shared", config: { ...baseSecureConfig, - filesystem: { allowRead: [], allowWrite: ["/shared"] }, + filesystem: { allowRead: [], allowWrite: ["/shared"], allowWriteProtected: [] }, }, }); const writeCount = policy.filesystem.allowWrite.filter((p) => p === "/shared").length; diff --git a/tests/unit/security/resolveReviewPolicy.test.ts b/tests/unit/security/resolveReviewPolicy.test.ts index c63dd54d..e44393c5 100644 --- a/tests/unit/security/resolveReviewPolicy.test.ts +++ b/tests/unit/security/resolveReviewPolicy.test.ts @@ -5,7 +5,7 @@ import type { ResolvedSecurityConfig } from "../../../src/schemas/securityConfig const baseConfig: ResolvedSecurityConfig = { profile: "secure", - filesystem: { allowRead: [], allowWrite: [] }, + filesystem: { allowRead: [], allowWrite: [], allowWriteProtected: [] }, network: { profile: "provider-only" }, mcp: { mode: "disabled", allow: [] }, agentCommands: [], @@ -38,7 +38,7 @@ describe("resolveReviewSecurityPolicy — allowWrite is only .phax-context", () worktreePath, config: { ...baseConfig, - filesystem: { allowRead: [], allowWrite: ["/extra/path"] }, + filesystem: { allowRead: [], allowWrite: ["/extra/path"], allowWriteProtected: [] }, }, }); expect(policy.filesystem.allowWrite).not.toContain("/extra/path"); @@ -62,7 +62,7 @@ describe("resolveReviewSecurityPolicy — allowRead contains the worktree", () = worktreePath, config: { ...baseConfig, - filesystem: { allowRead: ["/extra/read"], allowWrite: [] }, + filesystem: { allowRead: ["/extra/read"], allowWrite: [], allowWriteProtected: [] }, }, }); expect(policy.filesystem.allowRead).toContain("/extra/read");