diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index f11c1d6..bcaa5cf 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -11,6 +11,7 @@ export type WsRequest = type: "permission_response"; request_id: string; approved: boolean; + always_allow: boolean; } | { type: "ping" }; @@ -469,8 +470,17 @@ export class ChatWebSocket { this.send({ type: "cancel" }); } - sendPermissionResponse(requestId: string, approved: boolean) { - this.send({ type: "permission_response", request_id: requestId, approved }); + sendPermissionResponse( + requestId: string, + approved: boolean, + alwaysAllow = false, + ) { + this.send({ + type: "permission_response", + request_id: requestId, + approved, + always_allow: alwaysAllow, + }); } close() { diff --git a/frontend/src/components/Chat.tsx b/frontend/src/components/Chat.tsx index 962fe55..dfe39e9 100644 --- a/frontend/src/components/Chat.tsx +++ b/frontend/src/components/Chat.tsx @@ -578,10 +578,14 @@ export function Chat({ projectPath, onCloseProject }: ChatProps) { } }; - const handlePermissionResponse = (approved: boolean) => { + const handlePermissionResponse = (approved: boolean, alwaysAllow = false) => { const current = permissionQueue[0]; if (!current) return; - wsRef.current?.sendPermissionResponse(current.requestId, approved); + wsRef.current?.sendPermissionResponse( + current.requestId, + approved, + alwaysAllow, + ); setPermissionQueue((prev) => prev.slice(1)); }; @@ -1095,6 +1099,21 @@ export function Chat({ projectPath, onCloseProject }: ChatProps) { > Approve + diff --git a/frontend/src/types.ts b/frontend/src/types.ts index b5df52c..19a1918 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -53,6 +53,7 @@ export type WsRequest = type: "permission_response"; request_id: string; approved: boolean; + always_allow: boolean; }; export type WsResponse = diff --git a/server/src/http/context.rs b/server/src/http/context.rs index 9e2726a..370f3ed 100644 --- a/server/src/http/context.rs +++ b/server/src/http/context.rs @@ -7,6 +7,18 @@ use poem::http::StatusCode; use std::sync::Arc; use tokio::sync::{broadcast, mpsc, oneshot}; +/// The user's decision when responding to a permission dialog. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PermissionDecision { + /// One-time denial. + Deny, + /// One-time approval. + Approve, + /// Approve and persist the rule to `.claude/settings.json` so Claude Code's + /// built-in permission system handles future checks without prompting. + AlwaysAllow, +} + /// A permission request forwarded from the MCP `prompt_permission` tool to the /// active WebSocket session. The MCP handler blocks on `response_tx` until the /// user approves or denies via the frontend dialog. @@ -14,7 +26,7 @@ pub struct PermissionForward { pub request_id: String, pub tool_name: String, pub tool_input: serde_json::Value, - pub response_tx: oneshot::Sender, + pub response_tx: oneshot::Sender, } #[derive(Clone)] @@ -82,4 +94,13 @@ mod tests { let err = bad_request(String::new()); assert_eq!(err.status(), StatusCode::BAD_REQUEST); } + + #[test] + fn permission_decision_equality() { + assert_eq!(PermissionDecision::Deny, PermissionDecision::Deny); + assert_eq!(PermissionDecision::Approve, PermissionDecision::Approve); + assert_eq!(PermissionDecision::AlwaysAllow, PermissionDecision::AlwaysAllow); + assert_ne!(PermissionDecision::Deny, PermissionDecision::Approve); + assert_ne!(PermissionDecision::Approve, PermissionDecision::AlwaysAllow); + } } diff --git a/server/src/http/mcp.rs b/server/src/http/mcp.rs index c15eca7..1566340 100644 --- a/server/src/http/mcp.rs +++ b/server/src/http/mcp.rs @@ -1793,6 +1793,93 @@ fn tool_get_server_logs(args: &Value) -> Result { Ok(all_lines[start..].join("\n")) } +/// Generate a Claude Code permission rule string for the given tool name and input. +/// +/// - `Edit` / `Write` / `Read` / `Grep` / `Glob` etc. → just the tool name +/// - `Bash` → `Bash(first_word *)` derived from the `command` field in `tool_input` +/// - `mcp__*` → the full tool name (e.g. `mcp__story-kit__create_story`) +fn generate_permission_rule(tool_name: &str, tool_input: &Value) -> String { + if tool_name == "Bash" { + // Extract command from tool_input.command and use first word as prefix + let command_str = tool_input + .get("command") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let first_word = command_str.split_whitespace().next().unwrap_or("unknown"); + format!("Bash({first_word} *)") + } else { + // For Edit, Write, Read, Glob, Grep, MCP tools, etc. — use the tool name directly + tool_name.to_string() + } +} + +/// Add a permission rule to `.claude/settings.json` in the project root. +/// Does nothing if the rule already exists. Creates the file if missing. +fn add_permission_rule(project_root: &std::path::Path, rule: &str) -> Result<(), String> { + let claude_dir = project_root.join(".claude"); + fs::create_dir_all(&claude_dir) + .map_err(|e| format!("Failed to create .claude/ directory: {e}"))?; + + let settings_path = claude_dir.join("settings.json"); + let mut settings: Value = if settings_path.exists() { + let content = fs::read_to_string(&settings_path) + .map_err(|e| format!("Failed to read settings.json: {e}"))?; + serde_json::from_str(&content) + .map_err(|e| format!("Failed to parse settings.json: {e}"))? + } else { + json!({ "permissions": { "allow": [] } }) + }; + + let allow_arr = settings + .pointer_mut("/permissions/allow") + .and_then(|v| v.as_array_mut()); + + let allow = match allow_arr { + Some(arr) => arr, + None => { + // Ensure the structure exists + settings + .as_object_mut() + .unwrap() + .entry("permissions") + .or_insert(json!({ "allow": [] })); + settings + .pointer_mut("/permissions/allow") + .unwrap() + .as_array_mut() + .unwrap() + } + }; + + // Check for duplicates — exact string match + let rule_value = Value::String(rule.to_string()); + if allow.contains(&rule_value) { + return Ok(()); + } + + // Also check for wildcard coverage: if "mcp__story-kit__*" exists, don't add + // a more specific "mcp__story-kit__create_story". + let dominated = allow.iter().any(|existing| { + if let Some(pat) = existing.as_str() + && let Some(prefix) = pat.strip_suffix('*') + { + return rule.starts_with(prefix); + } + false + }); + if dominated { + return Ok(()); + } + + allow.push(rule_value); + + let pretty = + serde_json::to_string_pretty(&settings).map_err(|e| format!("Failed to serialize: {e}"))?; + fs::write(&settings_path, pretty) + .map_err(|e| format!("Failed to write settings.json: {e}"))?; + Ok(()) +} + /// MCP tool called by Claude Code via `--permission-prompt-tool`. /// /// Forwards the permission request through the shared channel to the active @@ -1821,7 +1908,9 @@ async fn tool_prompt_permission(args: &Value, ctx: &AppContext) -> Result Result } // Deny: { behavior: "deny", message: string } @@ -3382,7 +3483,7 @@ stage = "coder" tokio::spawn(async move { let mut rx = perm_rx.lock().await; if let Some(forward) = rx.recv().await { - let _ = forward.response_tx.send(true); + let _ = forward.response_tx.send(crate::http::context::PermissionDecision::Approve); } }); @@ -3414,7 +3515,7 @@ stage = "coder" tokio::spawn(async move { let mut rx = perm_rx.lock().await; if let Some(forward) = rx.recv().await { - let _ = forward.response_tx.send(false); + let _ = forward.response_tx.send(crate::http::context::PermissionDecision::Deny); } }); @@ -3508,4 +3609,134 @@ stage = "coder" let pct = read_coverage_percent_from_json(tmp.path()); assert!(pct.is_none()); } + + // ── Permission rule generation tests ───────────────────────── + + #[test] + fn generate_rule_for_edit_tool() { + let rule = generate_permission_rule("Edit", &json!({})); + assert_eq!(rule, "Edit"); + } + + #[test] + fn generate_rule_for_write_tool() { + let rule = generate_permission_rule("Write", &json!({})); + assert_eq!(rule, "Write"); + } + + #[test] + fn generate_rule_for_bash_git() { + let rule = + generate_permission_rule("Bash", &json!({"command": "git status"})); + assert_eq!(rule, "Bash(git *)"); + } + + #[test] + fn generate_rule_for_bash_cargo() { + let rule = + generate_permission_rule("Bash", &json!({"command": "cargo test --all"})); + assert_eq!(rule, "Bash(cargo *)"); + } + + #[test] + fn generate_rule_for_bash_empty_command() { + let rule = generate_permission_rule("Bash", &json!({})); + assert_eq!(rule, "Bash(unknown *)"); + } + + #[test] + fn generate_rule_for_mcp_tool() { + let rule = generate_permission_rule( + "mcp__story-kit__create_story", + &json!({"name": "foo"}), + ); + assert_eq!(rule, "mcp__story-kit__create_story"); + } + + // ── Settings.json writing tests ────────────────────────────── + + #[test] + fn add_rule_creates_settings_file_when_missing() { + let tmp = tempfile::tempdir().unwrap(); + add_permission_rule(tmp.path(), "Edit").unwrap(); + + let content = fs::read_to_string(tmp.path().join(".claude/settings.json")).unwrap(); + let settings: Value = serde_json::from_str(&content).unwrap(); + let allow = settings["permissions"]["allow"].as_array().unwrap(); + assert!(allow.contains(&json!("Edit"))); + } + + #[test] + fn add_rule_does_not_duplicate_existing() { + let tmp = tempfile::tempdir().unwrap(); + add_permission_rule(tmp.path(), "Edit").unwrap(); + add_permission_rule(tmp.path(), "Edit").unwrap(); + + let content = fs::read_to_string(tmp.path().join(".claude/settings.json")).unwrap(); + let settings: Value = serde_json::from_str(&content).unwrap(); + let allow = settings["permissions"]["allow"].as_array().unwrap(); + let count = allow.iter().filter(|v| v == &&json!("Edit")).count(); + assert_eq!(count, 1); + } + + #[test] + fn add_rule_skips_when_wildcard_already_covers() { + let tmp = tempfile::tempdir().unwrap(); + let claude_dir = tmp.path().join(".claude"); + fs::create_dir_all(&claude_dir).unwrap(); + fs::write( + claude_dir.join("settings.json"), + r#"{"permissions":{"allow":["mcp__story-kit__*"]}}"#, + ) + .unwrap(); + + add_permission_rule(tmp.path(), "mcp__story-kit__create_story").unwrap(); + + let content = fs::read_to_string(claude_dir.join("settings.json")).unwrap(); + let settings: Value = serde_json::from_str(&content).unwrap(); + let allow = settings["permissions"]["allow"].as_array().unwrap(); + assert_eq!(allow.len(), 1); + assert_eq!(allow[0], "mcp__story-kit__*"); + } + + #[test] + fn add_rule_appends_to_existing_rules() { + let tmp = tempfile::tempdir().unwrap(); + let claude_dir = tmp.path().join(".claude"); + fs::create_dir_all(&claude_dir).unwrap(); + fs::write( + claude_dir.join("settings.json"), + r#"{"permissions":{"allow":["Edit"]}}"#, + ) + .unwrap(); + + add_permission_rule(tmp.path(), "Write").unwrap(); + + let content = fs::read_to_string(claude_dir.join("settings.json")).unwrap(); + let settings: Value = serde_json::from_str(&content).unwrap(); + let allow = settings["permissions"]["allow"].as_array().unwrap(); + assert_eq!(allow.len(), 2); + assert!(allow.contains(&json!("Edit"))); + assert!(allow.contains(&json!("Write"))); + } + + #[test] + fn add_rule_preserves_other_settings_fields() { + let tmp = tempfile::tempdir().unwrap(); + let claude_dir = tmp.path().join(".claude"); + fs::create_dir_all(&claude_dir).unwrap(); + fs::write( + claude_dir.join("settings.json"), + r#"{"permissions":{"allow":["Edit"]},"enabledMcpjsonServers":["story-kit"]}"#, + ) + .unwrap(); + + add_permission_rule(tmp.path(), "Write").unwrap(); + + let content = fs::read_to_string(claude_dir.join("settings.json")).unwrap(); + let settings: Value = serde_json::from_str(&content).unwrap(); + let servers = settings["enabledMcpjsonServers"].as_array().unwrap(); + assert_eq!(servers.len(), 1); + assert_eq!(servers[0], "story-kit"); + } } diff --git a/server/src/http/ws.rs b/server/src/http/ws.rs index 755fb09..07f212d 100644 --- a/server/src/http/ws.rs +++ b/server/src/http/ws.rs @@ -1,4 +1,4 @@ -use crate::http::context::AppContext; +use crate::http::context::{AppContext, PermissionDecision}; use crate::http::workflow::{PipelineState, load_pipeline_state}; use crate::io::onboarding; use crate::io::watcher::WatcherEvent; @@ -29,6 +29,8 @@ enum WsRequest { PermissionResponse { request_id: String, approved: bool, + #[serde(default)] + always_allow: bool, }, /// Heartbeat ping from the client. The server responds with `Pong` so the /// client can detect stale (half-closed) connections. @@ -250,7 +252,7 @@ pub async fn ws_handler(ws: WebSocket, ctx: Data<&Arc>) -> impl poem // Permission requests arrive from the MCP `prompt_permission` tool via // `ctx.perm_rx` and are forwarded to the client as `PermissionRequest`. // When the client responds, we resolve the corresponding oneshot. - let mut pending_perms: HashMap> = HashMap::new(); + let mut pending_perms: HashMap> = HashMap::new(); loop { // Outer loop: wait for the next WebSocket message. @@ -324,9 +326,16 @@ pub async fn ws_handler(ws: WebSocket, ctx: Data<&Arc>) -> impl poem // (permission responses and cancellations). Some(Ok(WsMessage::Text(inner_text))) = stream.next() => { match serde_json::from_str::(&inner_text) { - Ok(WsRequest::PermissionResponse { request_id, approved }) => { + Ok(WsRequest::PermissionResponse { request_id, approved, always_allow }) => { if let Some(resp_tx) = pending_perms.remove(&request_id) { - let _ = resp_tx.send(approved); + let decision = if always_allow { + PermissionDecision::AlwaysAllow + } else if approved { + PermissionDecision::Approve + } else { + PermissionDecision::Deny + }; + let _ = resp_tx.send(decision); } } Ok(WsRequest::Cancel) => { @@ -457,9 +466,11 @@ mod tests { WsRequest::PermissionResponse { request_id, approved, + always_allow, } => { assert_eq!(request_id, "req-42"); assert!(approved); + assert!(!always_allow); } _ => panic!("expected PermissionResponse variant"), } @@ -477,9 +488,34 @@ mod tests { WsRequest::PermissionResponse { request_id, approved, + always_allow, } => { assert_eq!(request_id, "req-99"); assert!(!approved); + assert!(!always_allow); + } + _ => panic!("expected PermissionResponse variant"), + } + } + + #[test] + fn deserialize_permission_response_always_allow() { + let json = r#"{ + "type": "permission_response", + "request_id": "req-100", + "approved": true, + "always_allow": true + }"#; + let req: WsRequest = serde_json::from_str(json).unwrap(); + match req { + WsRequest::PermissionResponse { + request_id, + approved, + always_allow, + } => { + assert_eq!(request_id, "req-100"); + assert!(approved); + assert!(always_allow); } _ => panic!("expected PermissionResponse variant"), }