diff --git a/CHANGELOG.md b/CHANGELOG.md index 8259f93ae..222ba1dac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Docs: https://docs.clawd.bot - **BREAKING:** Control UI now rejects insecure HTTP without device identity by default. Use HTTPS (Tailscale Serve) or set `gateway.controlUi.allowInsecureAuth: true` to allow token-only auth. https://docs.clawd.bot/web/control-ui#insecure-http ### Fixes +- Nodes/macOS: prompt on allowlist miss for node exec approvals, persist allowlist decisions, and flatten node invoke errors. (#1394) Thanks @ngutman. - Gateway: keep auto bind loopback-first and add explicit tailnet binding to avoid Tailscale taking over local UI. (#1380) - Embedded runner: persist injected history images so attachments aren’t reloaded each turn. (#1374) Thanks @Nicell. - Nodes tool: include agent/node/gateway context in tool failure logs to speed approval debugging. diff --git a/apps/macos/Sources/Clawdbot/ExecApprovals.swift b/apps/macos/Sources/Clawdbot/ExecApprovals.swift index 15a0eeb71..122176592 100644 --- a/apps/macos/Sources/Clawdbot/ExecApprovals.swift +++ b/apps/macos/Sources/Clawdbot/ExecApprovals.swift @@ -554,6 +554,30 @@ enum ExecCommandFormatter { } } +enum ExecApprovalHelpers { + static func parseDecision(_ raw: String?) -> ExecApprovalDecision? { + let trimmed = raw?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + guard !trimmed.isEmpty else { return nil } + return ExecApprovalDecision(rawValue: trimmed) + } + + static func requiresAsk( + ask: ExecAsk, + security: ExecSecurity, + allowlistMatch: ExecAllowlistEntry?, + skillAllow: Bool) -> Bool + { + if ask == .always { return true } + if ask == .onMiss, security == .allowlist, allowlistMatch == nil, !skillAllow { return true } + return false + } + + static func allowlistPattern(command: [String], resolution: ExecCommandResolution?) -> String? { + let pattern = resolution?.resolvedPath ?? resolution?.rawExecutable ?? command.first ?? "" + return pattern.isEmpty ? nil : pattern + } +} + enum ExecAllowlistMatcher { static func match(entries: [ExecAllowlistEntry], resolution: ExecCommandResolution?) -> ExecAllowlistEntry? { guard let resolution, !entries.isEmpty else { return nil } diff --git a/apps/macos/Sources/Clawdbot/ExecApprovalsSocket.swift b/apps/macos/Sources/Clawdbot/ExecApprovalsSocket.swift index 268d155a0..dcbc0a9bb 100644 --- a/apps/macos/Sources/Clawdbot/ExecApprovalsSocket.swift +++ b/apps/macos/Sources/Clawdbot/ExecApprovalsSocket.swift @@ -314,7 +314,7 @@ private enum ExecHostExecutor { } var approvedByAsk = approvalDecision != nil - if self.requiresAsk( + if ExecApprovalHelpers.requiresAsk( ask: context.ask, security: context.security, allowlistMatch: context.allowlistMatch, @@ -417,36 +417,20 @@ private enum ExecHostExecutor { skillAllow: skillAllow) } - private static func requiresAsk( - ask: ExecAsk, - security: ExecSecurity, - allowlistMatch: ExecAllowlistEntry?, - skillAllow: Bool) -> Bool - { - if ask == .always { return true } - if ask == .onMiss, security == .allowlist, allowlistMatch == nil, !skillAllow { return true } - return false - } - private static func persistAllowlistEntry( decision: ExecApprovalDecision?, context: ExecApprovalContext) { guard decision == .allowAlways, context.security == .allowlist else { return } - guard let pattern = self.allowlistPattern(command: context.command, resolution: context.resolution) else { + guard let pattern = ExecApprovalHelpers.allowlistPattern( + command: context.command, + resolution: context.resolution) + else { return } ExecApprovalsStore.addAllowlistEntry(agentId: context.trimmedAgent, pattern: pattern) } - private static func allowlistPattern( - command: [String], - resolution: ExecCommandResolution?) -> String? - { - let pattern = resolution?.resolvedPath ?? resolution?.rawExecutable ?? command.first ?? "" - return pattern.isEmpty ? nil : pattern - } - private static func ensureScreenRecordingAccess(_ needsScreenRecording: Bool?) async -> ExecHostResponse? { guard needsScreenRecording == true else { return nil } let authorized = await PermissionManager diff --git a/apps/macos/Sources/Clawdbot/NodeMode/MacNodeRuntime.swift b/apps/macos/Sources/Clawdbot/NodeMode/MacNodeRuntime.swift index ae8d87136..8c6f3f776 100644 --- a/apps/macos/Sources/Clawdbot/NodeMode/MacNodeRuntime.swift +++ b/apps/macos/Sources/Clawdbot/NodeMode/MacNodeRuntime.swift @@ -480,13 +480,13 @@ actor MacNodeRuntime { message: "SYSTEM_RUN_DISABLED: security=deny") } - let requiresAsk: Bool = { - if ask == .always { return true } - if ask == .onMiss, security == .allowlist, allowlistMatch == nil, !skillAllow { return true } - return false - }() + let requiresAsk = ExecApprovalHelpers.requiresAsk( + ask: ask, + security: security, + allowlistMatch: allowlistMatch, + skillAllow: skillAllow) - let decisionFromParams = Self.parseApprovalDecision(params.approvalDecision) + let decisionFromParams = ExecApprovalHelpers.parseDecision(params.approvalDecision) var approvedByAsk = params.approved == true || decisionFromParams != nil var persistAllowlist = decisionFromParams == .allowAlways if decisionFromParams == .deny { @@ -536,14 +536,10 @@ actor MacNodeRuntime { approvedByAsk = true } } - if persistAllowlist, security == .allowlist { - let pattern = resolution?.resolvedPath - ?? resolution?.rawExecutable - ?? command.first - ?? "" - if !pattern.isEmpty { - ExecApprovalsStore.addAllowlistEntry(agentId: agentId, pattern: pattern) - } + if persistAllowlist, security == .allowlist, + let pattern = ExecApprovalHelpers.allowlistPattern(command: command, resolution: resolution) + { + ExecApprovalsStore.addAllowlistEntry(agentId: agentId, pattern: pattern) } if security == .allowlist, allowlistMatch == nil, !skillAllow, !approvedByAsk { @@ -807,12 +803,6 @@ extension MacNodeRuntime { UserDefaults.standard.object(forKey: cameraEnabledKey) as? Bool ?? false } - private static func parseApprovalDecision(_ raw: String?) -> ExecApprovalDecision? { - let trimmed = raw?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" - guard !trimmed.isEmpty else { return nil } - return ExecApprovalDecision(rawValue: trimmed) - } - private static let blockedEnvKeys: Set = [ "PATH", "NODE_OPTIONS", diff --git a/apps/macos/Tests/ClawdbotIPCTests/ExecApprovalHelpersTests.swift b/apps/macos/Tests/ClawdbotIPCTests/ExecApprovalHelpersTests.swift new file mode 100644 index 000000000..a1774d49f --- /dev/null +++ b/apps/macos/Tests/ClawdbotIPCTests/ExecApprovalHelpersTests.swift @@ -0,0 +1,60 @@ +import Foundation +import Testing +@testable import Clawdbot + +@Suite struct ExecApprovalHelpersTests { + @Test func parseDecisionTrimsAndRejectsInvalid() { + #expect(ExecApprovalHelpers.parseDecision("allow-once") == .allowOnce) + #expect(ExecApprovalHelpers.parseDecision(" allow-always ") == .allowAlways) + #expect(ExecApprovalHelpers.parseDecision("deny") == .deny) + #expect(ExecApprovalHelpers.parseDecision("") == nil) + #expect(ExecApprovalHelpers.parseDecision("nope") == nil) + } + + @Test func allowlistPatternPrefersResolution() { + let resolved = ExecCommandResolution( + rawExecutable: "rg", + resolvedPath: "/opt/homebrew/bin/rg", + executableName: "rg", + cwd: nil) + #expect(ExecApprovalHelpers.allowlistPattern(command: ["rg"], resolution: resolved) == resolved.resolvedPath) + + let rawOnly = ExecCommandResolution( + rawExecutable: "rg", + resolvedPath: nil, + executableName: "rg", + cwd: nil) + #expect(ExecApprovalHelpers.allowlistPattern(command: ["rg"], resolution: rawOnly) == "rg") + #expect(ExecApprovalHelpers.allowlistPattern(command: ["rg"], resolution: nil) == "rg") + #expect(ExecApprovalHelpers.allowlistPattern(command: [], resolution: nil) == nil) + } + + @Test func requiresAskMatchesPolicy() { + let entry = ExecAllowlistEntry(pattern: "/bin/ls", lastUsedAt: nil, lastUsedCommand: nil, lastResolvedPath: nil) + #expect(ExecApprovalHelpers.requiresAsk( + ask: .always, + security: .deny, + allowlistMatch: nil, + skillAllow: false)) + #expect(ExecApprovalHelpers.requiresAsk( + ask: .onMiss, + security: .allowlist, + allowlistMatch: nil, + skillAllow: false)) + #expect(!ExecApprovalHelpers.requiresAsk( + ask: .onMiss, + security: .allowlist, + allowlistMatch: entry, + skillAllow: false)) + #expect(!ExecApprovalHelpers.requiresAsk( + ask: .onMiss, + security: .allowlist, + allowlistMatch: nil, + skillAllow: true)) + #expect(!ExecApprovalHelpers.requiresAsk( + ask: .off, + security: .allowlist, + allowlistMatch: nil, + skillAllow: false)) + } +}