diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AcpmuxRemotePersonKeys.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AcpmuxRemotePersonKeys.swift index d2e91ffd5c34..ff2c09ccc7b6 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AcpmuxRemotePersonKeys.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AcpmuxRemotePersonKeys.swift @@ -23,6 +23,20 @@ public nonisolated final class AcpmuxSecretKey: @unchecked Sendable { // crash-a } } + /// The key of 64 lowercase hex digits (tests: a key acpmux's own tests use too). + init?(hex: String) { + // Parsed before the buffer exists, so a bad digit fails with nothing to free. + let hexDigits = Set("0123456789abcdef".utf8) + let digits = Array(hex.utf8) + guard digits.count == Self.length * 2, digits.allSatisfy(hexDigits.contains) else { return nil } + let parsed = (0.. String? { + guard let canonical = Self.canonicalJSON(answers) else { return nil } + var digest = [UInt8](repeating: 0, count: Int(CC_SHA256_DIGEST_LENGTH)) + let text = Array(canonical.utf8) + text.withUnsafeBytes { _ = CC_SHA256($0.baseAddress, CC_LONG(text.count), &digest) } + let hex = digest.map { String(format: "%02x", $0) }.joined() + return mac(domain: "acpmux-person-decision-answers-v1", parts: [instance, session, permission, option, nonce, hex]) + } + + /// The bytes acpmux hashes (`approvers::canonical_json`): object keys in UTF-8 byte order, no + /// whitespace, and only `"`, `\` and control characters escaped (`\b \f \n \r \t`, else + /// `\u00xx`). `value` is what `JSONSerialization` parsed. + static func canonicalJSON(_ value: Any) -> String? { + switch value { + case is NSNull: + return "null" + case let string as String: + return canonicalString(string) + case let number as NSNumber: + if CFGetTypeID(number) == CFBooleanGetTypeID() { return number.boolValue ? "true" : "false" } + if CFNumberIsFloatType(number) { + let double = number.doubleValue + guard double.rounded() == double, abs(double) < 1e15 else { return nil } + return String(Int64(double)) + } + return number.stringValue + case let array as [Any]: + var parts: [String] = [] + for item in array { + guard let part = canonicalJSON(item) else { return nil } + parts.append(part) + } + return "[" + parts.joined(separator: ",") + "]" + case let object as [String: Any]: + var parts: [String] = [] + for key in object.keys.sorted(by: { Array($0.utf8).lexicographicallyPrecedes(Array($1.utf8)) }) { + guard let item = object[key], let part = canonicalJSON(item) else { return nil } + parts.append(canonicalString(key) + ":" + part) + } + return "{" + parts.joined(separator: ",") + "}" + default: + return nil + } + } + + private static func canonicalString(_ string: String) -> String { + var out = "\"" + for scalar in string.unicodeScalars { + switch scalar { + case "\"": out += "\\\"" + case "\\": out += "\\\\" + case "\u{8}": out += "\\b" + case "\u{c}": out += "\\f" + case "\n": out += "\\n" + case "\r": out += "\\r" + case "\t": out += "\\t" + case _ where scalar.value < 0x20: out += String(format: "\\u%04x", scalar.value) + default: out.unicodeScalars.append(scalar) + } + } + return out + "\"" + } +} diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneGestureIntent.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneGestureIntent.swift index 22652c6693fa..51068ed61493 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneGestureIntent.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneGestureIntent.swift @@ -97,10 +97,11 @@ public nonisolated struct AgentPaneGesturePick: Equatable, Sendable { self.params = (acpmux?["promptId"] as? String).map { ["promptId": .string($0)] } return } - // A permission answer's pick names its session too (the ticket binds the prompt). + // A permission answer's pick names its session too (the ticket binds the prompt). A + // question's `answers` are no pick: the answers rule bounds them and the proof covers them. if method == "_acpmux/permission_respond" { guard let session = params["sessionId"] as? String, let permission = params["permissionId"] as? String, - let option = params["optionId"] as? String, params["answers"] == nil else { + let option = params["optionId"] as? String else { self.params = nil return } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneTransport+RelayedAllow.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneTransport+RelayedAllow.swift index b8d776d561dd..41f68b7d2a0a 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneTransport+RelayedAllow.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneTransport+RelayedAllow.swift @@ -26,8 +26,26 @@ extension AgentPaneTransport { Self.logger.info("agent pane transport relayed allow without the person's ticket: sent without a proof") return } + // A question's answers: the proof covers them too, else that daemon refuses the answer. + if let answers = Self.answers(box) { + guard let proof = key.decisionProof(instance: context.instance, session: session, permission: permission, + option: option, nonce: context.nonce, answers: answers) else { + Self.logger.info("agent pane transport relayed answer without a canonical form: sent without a proof") + return + } + box.setDecision(nonce: context.nonce, proof: proof) + return + } let proof = key.decisionProof(instance: context.instance, session: session, permission: permission, option: option, nonce: context.nonce) box.setDecision(nonce: context.nonce, proof: proof) } + + /// The frame's `answers` (a question's), when it has any that are not null. + private static func answers(_ box: FrameBox) -> Any? { + guard let text = box.encoded(id: nil), + let object = (try? JSONSerialization.jsonObject(with: Data(text.utf8))) as? [String: Any], + let answers = (object["params"] as? [String: Any])?["answers"], !(answers is NSNull) else { return nil } + return answers + } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentSessionRemoteClient.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentSessionRemoteClient.swift index a30780f629f1..3bdb3624c8c6 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentSessionRemoteClient.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentSessionRemoteClient.swift @@ -15,9 +15,10 @@ public nonisolated protocol AgentSessionRemoteClient: Sendable { /// Sends one text prompt; answers `{prompt_id, turn_id, queued}` once acpmux recorded it. func prompt(id: String, text: String) async throws -> Data func cancel() async throws - /// Answers a permission request the daemon announced on this attachment, with the person's - /// decision proof of a relayed allow when the pane made one (cx-0q7o; acpmux decides). - func permission(id: String, option: String, decisionNonce: String?, personDecision: String?) async throws + /// Answers a permission request the daemon announced on this attachment, with a question's + /// `answers` (JSON) and the person's decision proof of a relayed allow when the pane made one + /// (cx-0q7o; acpmux decides, and an answer's proof covers its answers). + func permission(id: String, option: String, answers: Data?, decisionNonce: String?, personDecision: String?) async throws func detach() async /// Ends the attachment and its daemon connection. func close() async diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/RemoteAcpmuxWire.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/RemoteAcpmuxWire.swift index 5a8d71ff68b5..bd74cebd799d 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/RemoteAcpmuxWire.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/RemoteAcpmuxWire.swift @@ -145,8 +145,13 @@ public nonisolated final class RemoteAcpmuxWire: AcpmuxPaneWire { // The decision proof the host added (never the page): acpmux on that machine decides. let decision = (params["_meta"] as? [String: Any])?["acpmux"] as? [String: Any] let nonce = decision?["decisionNonce"] as? String, proof = decision?["personDecision"] as? String + // A question's answers go along unchanged: that machine's acpmux checks them and the + // proof over them (dropped, the answer was refused there). + let answers = params["answers"].flatMap { $0 is NSNull ? nil : $0 } + .flatMap { try? JSONSerialization.data(withJSONObject: $0, options: [.fragmentsAllowed]) } run(id) { client in - try await client.permission(id: permission, option: option, decisionNonce: nonce, personDecision: proof) + try await client.permission(id: permission, option: option, answers: answers, decisionNonce: nonce, + personDecision: proof) return Data("{}".utf8) } default: diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/AgentTabRemoteHost.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/AgentTabRemoteHost.swift index 2146f2727168..7b17e6f2ea54 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/AgentTabRemoteHost.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/AgentTabRemoteHost.swift @@ -140,9 +140,10 @@ nonisolated struct AgentTabRemoteClient: AgentSessionRemoteClient { func cancel() async throws { _ = try await mapped { try await client.cancel(); return Data() } } - func permission(id: String, option: String, decisionNonce: String?, personDecision: String?) async throws { + func permission(id: String, option: String, answers: Data?, decisionNonce: String?, personDecision: String?) async throws { _ = try await mapped { - try await client.permission(id: id, option: option, decisionNonce: decisionNonce, personDecision: personDecision) + try await client.permission(id: id, option: option, answers: answers, decisionNonce: decisionNonce, + personDecision: personDecision) return Data() } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/AgentSession/AgentSessionAttachClient.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/AgentSession/AgentSessionAttachClient.swift index 816451e64bd4..d9755e0169ec 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/AgentSession/AgentSessionAttachClient.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/AgentSession/AgentSessionAttachClient.swift @@ -155,15 +155,17 @@ public actor AgentSessionAttachClient { _ = try await call("agent-session-cancel", [:]) } - /// `agent-session-permission`, with the person's decision proof of a relayed allow when there - /// is one (cx-0q7o): the daemon passes it to acpmux unchanged, and acpmux decides. - public func permission(id: String, option: String, decisionNonce: String? = nil, personDecision: String? = nil) async throws { + /// `agent-session-permission`, with a question's `answers` (a JSON object) and the person's + /// decision proof of a relayed allow when there is one (cx-0q7o): the daemon passes both to + /// acpmux unchanged, and acpmux decides (an answer's proof covers its answers). + public func permission(id: String, option: String, answers: Data? = nil, decisionNonce: String? = nil, + personDecision: String? = nil) async throws { var params: [String: any Sendable] = ["permission_id": id, "option_id": option] if let decisionNonce, let personDecision { params["decision_nonce"] = decisionNonce params["person_decision"] = personDecision } - _ = try await call("agent-session-permission", params) + _ = try await call("agent-session-permission", params, answers: answers) } public func detach() async { @@ -200,10 +202,14 @@ public actor AgentSessionAttachClient { return params } - /// One verb on the tab; the result's `data` as JSON, or the daemon's refusal. - private func call(_ cmd: String, _ params: [String: any Sendable]) async throws -> Data { + /// One verb on the tab; the result's `data` as JSON, or the daemon's refusal. `answers` (a + /// question's, as JSON) goes in the request as it is. + private func call(_ cmd: String, _ params: [String: any Sendable], answers: Data? = nil) async throws -> Data { let transport = try await liveTransport() - var object = params + var object: [String: Any] = params + if let answers { + object["answers"] = try JSONSerialization.jsonObject(with: answers, options: [.fragmentsAllowed]) + } object["cmd"] = cmd object["surface"] = surface // Integers stay integers (the daemon reads u64 seqs and surface ids). diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneRelayedAllowTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneRelayedAllowTests.swift index a9d705ac5d8a..80ba2295e998 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneRelayedAllowTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneRelayedAllowTests.swift @@ -189,4 +189,59 @@ struct AgentPaneRelayedAllowTests { } } } + + static func pendingQuestion() -> String { + #"{"jsonrpc":"2.0","method":"_acpmux/permission_pending","params":{"sessionId":"s","permissionId":"p1","decisionNonce":"\#(nonce)","personInstance":"\#(instance)","request":{"options":[{"optionId":"yes","kind":"allow_once"},{"optionId":"no","kind":"reject_once"}],"toolCall":{"_meta":{"acpmux":{"question":{"items":[{"id":"q1","prompt":"Which one?"}]}}}}}}}"# + } + + /// Leo 2026-10-21: an agent's question answered in the pane for a chat on another machine was + /// refused there as "accepted only from the cmux app". The answer's proof covers its answers + /// (acpmux `decision_answers_proof`), so that machine counts it as the person's. + @Test func anAnswerToAnotherMachinesQuestionCarriesTheProofOverItsAnswers() async throws { + let machine = "machine-\(UUID().uuidString)" + let key = try #require(AcpmuxSecretKey()) + AcpmuxRemotePersonKeys.shared.bind(machine: machine, instance: Self.instance, key: key) + defer { AcpmuxRemotePersonKeys.shared.drop(machine: machine) } + let approvals = AgentPaneRemoteApprovals(machineID: machine, label: "host") { nil } + let wire = RemoteWire() + let route = AgentPaneRemoteRoute(machine: "host", approvals: approvals) { wire } + let inbox = PageInbox() + let transport = AgentPaneTransport() + transport.deliver = { event, done in inbox.deliver(event.frames); done() } + let id = try await transport.open(.remote(route)) + _ = await transport.send(connection: id, frames: [#"{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}"#]) + _ = await inbox.frame(containing: "\"origin\":\"remote\"") + transport.sessions.add("s") + wire.push(Self.pendingQuestion()) + _ = await inbox.frame(containing: "permission_pending") + + transport.gestures.record(click: true) + let ticket = try #require(transport.reserveGesture(try #require(Self.intent(option: "yes")))) + let answer = #"{"jsonrpc":"2.0","id":7,"method":"_acpmux/permission_respond","params":{"sessionId":"s","permissionId":"p1","optionId":"yes","answers":{"Which one?":"B"},"_meta":{"cmuxGesture":"\#(ticket)"}}}"# + let error = await transport.send(connection: id, frames: [answer]) + #expect(error == nil, "\(String(describing: error))") + let sent = await wire.sent(containing: "permission_respond") + let object = try JSONSerialization.jsonObject(with: Data(sent.utf8)) as? [String: Any] + let params = try #require(object?["params"] as? [String: Any]) + let acpmux = (params["_meta"] as? [String: Any])?["acpmux"] as? [String: Any] + #expect(acpmux?["decisionNonce"] as? String == Self.nonce) + let expected = key.decisionProof(instance: Self.instance, session: "s", permission: "p1", option: "yes", nonce: Self.nonce, + answers: ["Which one?": "B"]) + #expect(expected != nil) + #expect(acpmux?["personDecision"] as? String == expected, "the answer went without the proof over its answers: \(sent)") + #expect(acpmux?["personDecision"] as? String + != key.decisionProof(instance: Self.instance, session: "s", permission: "p1", option: "yes", nonce: Self.nonce)) + } + + /// The same bytes acpmux checks (`the_answers_decision_proof_matches_the_apps_vector` in + /// cmux-tui/crates/acpmux/tests/person_allow.rs): key order by UTF-8 bytes, acpmux's escapes. + @Test func theAnswersProofMatchesAcpmuxsVector() throws { + let key = try #require(AcpmuxSecretKey(hex: "5e1f0c2a9b8d7e6f5a4b3c2d1e0f9a8b7c6d5e4f3a2b1c0d9e8f7a6b5c4d3e2f")) + let answers = try JSONSerialization.jsonObject( + with: Data(#"{"Which one?":"B","Pick a color\n(one)":["red","blue"],"é":"\u0001"}"#.utf8)) + #expect(AcpmuxSecretKey.canonicalJSON(answers) == "{\"Pick a color\\n(one)\":[\"red\",\"blue\"],\"Which one?\":\"B\",\"é\":\"\\u0001\"}") + #expect(key.decisionProof(instance: "0123456789abcdef", session: "sess-1", permission: "perm-1", option: "allow_once", + nonce: "nonce-1", answers: answers) + == "3b11c2af201d95772c106a4c8463a54b94e615f85e88eac0700f7d657c24f0e9") + } } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/RemoteAcpmuxObserverTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/RemoteAcpmuxObserverTests.swift index 841b5e46bc14..f95acaaebf57 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/RemoteAcpmuxObserverTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/RemoteAcpmuxObserverTests.swift @@ -27,7 +27,7 @@ import Testing func cancel() async throws { calls.withLock { $0.append("cancel") } } - func permission(id: String, option: String, decisionNonce: String?, personDecision: String?) async throws { + func permission(id: String, option: String, answers: Data?, decisionNonce: String?, personDecision: String?) async throws { calls.withLock { $0.append("permission") } } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/RemoteAcpmuxWireAnswerTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/RemoteAcpmuxWireAnswerTests.swift new file mode 100644 index 000000000000..a1e80bf1955e --- /dev/null +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/RemoteAcpmuxWireAnswerTests.swift @@ -0,0 +1,108 @@ +import Foundation +import CmuxNextCompat +import Testing +@testable import CmuxNextAgentPane + +/// An agent's question answered in the pane for a chat on another machine goes out through the +/// real ``RemoteAcpmuxWire``, which turns the page's `_acpmux/permission_respond` into the owning +/// session daemon's `agent-session-permission`. The answer must reach that daemon with its answers +/// and the proof over them: before, the wire dropped the answers and that machine's acpmux checked +/// the answers proof against a plain allow and refused it (Codex review of #23869, Leo 2026-10-21). +@Suite(.serialized) +@MainActor +struct RemoteAcpmuxWireAnswerTests { + typealias Relayed = AgentPaneRelayedAllowTests + + /// One answer as the wire hands it to the daemon client. + struct Answer: Sendable { + var option: String + var answers: Data? + var nonce: String? + var proof: String? + } + + /// The owning session daemon's side of the chat: attaches at once, pushes what the test sends, + /// and records each permission answer the wire makes. + nonisolated final class DaemonClient: AgentSessionRemoteClient { + private let handler = Mutex<(@Sendable (AgentSessionRemoteEvent) -> Void)?>(nil) + private let answered = Mutex<(answers: [Answer], waiters: [CheckedContinuation])>(([], [])) + let isReadOnly = false + + func attach(_ page: AgentSessionPage, events handler: @escaping @Sendable (AgentSessionRemoteEvent) -> Void) async throws -> Data { + self.handler.withLock { $0 = handler } + return Data(#"{"session":{"sessionId":"s"},"events":[],"hasMore":false,"lastSeq":0}"#.utf8) + } + + func events(_ page: AgentSessionPage) async throws -> Data { Data(#"{"events":[],"hasMore":false,"lastSeq":0}"#.utf8) } + func prompt(id: String, text: String) async throws -> Data { Data("{}".utf8) } + func cancel() async throws {} + + func permission(id: String, option: String, answers: Data?, decisionNonce: String?, personDecision: String?) async throws { + record(Answer(option: option, answers: answers, nonce: decisionNonce, proof: personDecision)) + } + + func detach() async {} + func close() async {} + + func push(_ event: AgentSessionRemoteEvent) { handler.withLock { $0 }?(event) } + + private func record(_ answer: Answer) { + let waiters = answered.withLock { state -> [CheckedContinuation] in + state.answers.append(answer) + defer { state.waiters = [] } + return state.waiters + } + for waiter in waiters { waiter.resume(returning: answer) } + } + + /// The first answer the wire made. + func answer() async -> Answer { + await withCheckedContinuation { waiter in + let found = answered.withLock { state -> Answer? in + if let first = state.answers.first { return first } + state.waiters.append(waiter) + return nil + } + if let found { waiter.resume(returning: found) } + } + } + } + + @Test func anAnswerThroughTheRemoteWireReachesTheDaemonWithItsAnswersAndTheirProof() async throws { + let machine = "machine-\(UUID().uuidString)" + let key = try #require(AcpmuxSecretKey()) + AcpmuxRemotePersonKeys.shared.bind(machine: machine, instance: Relayed.instance, key: key) + defer { AcpmuxRemotePersonKeys.shared.drop(machine: machine) } + let approvals = AgentPaneRemoteApprovals(machineID: machine, label: "host") { nil } + let client = DaemonClient() + let wire = RemoteAcpmuxWire(client: client, session: "s") + let route = AgentPaneRemoteRoute(machine: "host", approvals: approvals) { wire } + let inbox = Relayed.PageInbox() + let transport = AgentPaneTransport() + transport.deliver = { event, done in inbox.deliver(event.frames); done() } + let id = try await transport.open(.remote(route)) + _ = await transport.send(connection: id, frames: [#"{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}"#]) + _ = await inbox.frame(containing: "\"origin\":\"remote\"") + transport.sessions.add("s") + _ = await transport.send(connection: id, frames: [#"{"jsonrpc":"2.0","id":1,"method":"_acpmux/attach","params":{"sessionId":"s"}}"#]) + _ = await inbox.frame(containing: "\"lastSeq\"") + let pending = try JSONSerialization.jsonObject(with: Data(Relayed.pendingQuestion().utf8)) as? [String: Any] + let request = try JSONSerialization.data(withJSONObject: try #require(pending?["params"])) + client.push(.permission(request)) + _ = await inbox.frame(containing: "permission_pending") + + transport.gestures.record(click: true) + let ticket = try #require(transport.reserveGesture(try #require(Relayed.intent(option: "yes")))) + let frame = #"{"jsonrpc":"2.0","id":7,"method":"_acpmux/permission_respond","params":{"sessionId":"s","permissionId":"p1","optionId":"yes","answers":{"Which one?":"B"},"_meta":{"cmuxGesture":"\#(ticket)"}}}"# + let error = await transport.send(connection: id, frames: [frame]) + #expect(error == nil, "\(String(describing: error))") + + let answer = await client.answer() + #expect(answer.option == "yes") + #expect(answer.nonce == Relayed.nonce) + #expect(answer.proof == key.decisionProof(instance: Relayed.instance, session: "s", permission: "p1", option: "yes", + nonce: Relayed.nonce, answers: ["Which one?": "B"])) + let answers = try #require(answer.answers, "the wire dropped the answers, so that machine refuses the answer") + #expect(try JSONSerialization.jsonObject(with: answers) as? [String: String] == ["Which one?": "B"]) + } +} diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/RemoteChildSessionsTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/RemoteChildSessionsTests.swift index b8d82aaddd89..d11576e5a226 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/RemoteChildSessionsTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/RemoteChildSessionsTests.swift @@ -27,7 +27,7 @@ import Testing func events(_ page: AgentSessionPage) async throws -> Data { Data("{}".utf8) } func prompt(id: String, text: String) async throws -> Data { Data("{}".utf8) } func cancel() async throws {} - func permission(id: String, option: String, decisionNonce: String?, personDecision: String?) async throws {} + func permission(id: String, option: String, answers: Data?, decisionNonce: String?, personDecision: String?) async throws {} func detach() async {} func close() async {} func children() async throws -> Data { RemoteAcpmuxWire.json(["sessions": listed]) } diff --git a/cmux-tui/crates/acpmux/src/hub/permissions.rs b/cmux-tui/crates/acpmux/src/hub/permissions.rs index c9a4e7620cc1..35451a23a6d5 100644 --- a/cmux-tui/crates/acpmux/src/hub/permissions.rs +++ b/cmux-tui/crates/acpmux/src/hub/permissions.rs @@ -706,13 +706,13 @@ impl Hub { if (allows || answered_by.is_some()) && !ancestor_answers { let proved = match (&option_id, answerer.decision) { (Some(o), Some((nonce, proof))) => { - answers.is_none() - && cmux_local_auth::tokens_match(nonce, &p.decision_nonce) + cmux_local_auth::tokens_match(nonce, &p.decision_nonce) && self.person.decision_proves( &session.id, permission_id, o, nonce, + answers.as_ref(), proof, ) } diff --git a/cmux-tui/crates/acpmux/src/hub/person.rs b/cmux-tui/crates/acpmux/src/hub/person.rs index 7dcbb04e052f..440d3313cd1e 100644 --- a/cmux-tui/crates/acpmux/src/hub/person.rs +++ b/cmux-tui/crates/acpmux/src/hub/person.rs @@ -49,8 +49,8 @@ //! bounded store of spent nonces ([`SPENT_LIMIT`] entries, [`SPENT_TTL`]) refuses it again. A //! prompt the next daemon adopts gets a new nonce under the new run (`permission_reissued`, also //! a `permission_pending`). A restarted daemon has a new instance, so an old proof proves -//! nothing. A proof never covers question answers, a group's allow, a policy, a rule or a mode: -//! those widen and stay the person's own. A keyed daemon is not dumpable (`daemon.rs`), so an +//! nothing. A question's answers count only under [`decision_answers_proof`]. A proof never +//! covers a group's allow, a policy, a rule or a mode: those widen and stay the person's own. A keyed daemon is not dumpable (`daemon.rs`), so an //! agent it starts cannot read the key from its memory. //! - Through a peer (slice 2a, 2b): a daemon that only mirrors a peer's session cannot check a //! proof made with the peer's key, so it forwards every answer for that session (allow, deny, @@ -509,13 +509,15 @@ impl PersonGate { permission: &str, option: &str, nonce: &str, + answers: Option<&Value>, proof: &str, ) -> bool { let key = self.key.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); let instance = self.instance(); - let Some(expected) = - key.and_then(|k| decision_proof(&k, instance, session, permission, option, nonce)) - else { + let Some(expected) = key.and_then(|k| match answers.filter(|a| !a.is_null()) { + None => decision_proof(&k, instance, session, permission, option, nonce), + Some(a) => decision_answers_proof(&k, instance, session, permission, option, nonce, a), + }) else { return false; }; cmux_local_auth::tokens_match(proof, &expected) && !self.spent(nonce) @@ -654,6 +656,8 @@ pub fn person_proof(key: &str, transport: &str, nonce: &str, connection: &str) - ))) } +mod decision_answers; +pub use decision_answers::decision_answers_proof; /// The bytes a decision proof covers. fn decision_message(parts: [&str; 5]) -> Vec { let mut message = b"acpmux-person-decision-v1".to_vec(); @@ -1010,8 +1014,7 @@ impl Hub { } /// Whether a relayed allow carries the person's decision proof for the - /// pending prompt it answers (module docs, cx-0q7o). Question answers - /// are never covered by a proof. + /// pending prompt it answers (module docs, cx-0q7o), over its answers too. fn person_decided(&self, params: &Value, option: &Value) -> bool { let meta = params.pointer("/_meta/acpmux"); let field = |f: &str| meta.and_then(|m| m.get(f)).and_then(Value::as_str); @@ -1020,9 +1023,6 @@ impl Hub { else { return false; }; - if params.get("answers").is_some_and(|v| !v.is_null()) { - return false; - } let Some(s) = crate::server::session_key(params).ok().and_then(|k| self.resolve(k).ok()) else { return false; @@ -1034,7 +1034,7 @@ impl Hub { p.decision_nonce.clone() }; cmux_local_auth::tokens_match(nonce, &pending_nonce) - && self.person.decision_proves(&s.id, pid, option, nonce, proof) + && self.person.decision_proves(&s.id, pid, option, nonce, params.get("answers"), proof) } /// Whether `option` is a reject the pending permission offered. Unknown diff --git a/cmux-tui/crates/acpmux/src/hub/person/decision_answers.rs b/cmux-tui/crates/acpmux/src/hub/person/decision_answers.rs new file mode 100644 index 000000000000..4a756e39ab96 --- /dev/null +++ b/cmux-tui/crates/acpmux/src/hub/person/decision_answers.rs @@ -0,0 +1,31 @@ +//! The person's decision proof for an answer to a question (an agent's AskUserQuestion) relayed +//! from another machine's app: the allow's proof ([`super::decision_proof`]) also covers the +//! answers, so a relay cannot swap them. The app's pane on the Mac signs it for a chat on an ssh +//! peer (`AgentPaneTransport+RelayedAllow.swift`); before it, every such answer was refused as +//! not the person's (Leo, 2026-10-21). + +use serde_json::Value; + +/// Lowercase hex HMAC-SHA256(key, "acpmux-person-decision-answers-v1" 0 instance 0 session 0 +/// permission 0 option 0 nonce 0 answers digest), the digest being the sha256 of the canonical +/// answers JSON as a phone's signed answer has it (`approvers::answers_digest`). None when `key` +/// is not a person key. +pub fn decision_answers_proof( + key: &str, + instance: &str, + session: &str, + permission: &str, + option: &str, + nonce: &str, + answers: &Value, +) -> Option { + use cmux_local_auth::frontend_proof::{hex, hmac_sha256, unhex}; + let key = unhex::<32>(key).filter(|_| super::valid_key(key))?; + let digest = crate::hub::approvers::answers_digest(Some(answers)); + let mut message = b"acpmux-person-decision-answers-v1".to_vec(); + for part in [instance, session, permission, option, nonce, digest.as_str()] { + message.push(0); + message.extend_from_slice(part.as_bytes()); + } + Some(hex(&hmac_sha256(&key, &message))) +} diff --git a/cmux-tui/crates/acpmux/tests/person_allow.rs b/cmux-tui/crates/acpmux/tests/person_allow.rs index 14e9dd12ec77..bb7913d9652d 100644 --- a/cmux-tui/crates/acpmux/tests/person_allow.rs +++ b/cmux-tui/crates/acpmux/tests/person_allow.rs @@ -1368,3 +1368,76 @@ async fn a_phones_signed_answer_covers_exactly_its_answers() { phone.ok("_acpmux/permission_respond", answer).await; assert_eq!(turn_said(&mut phone, rid).await, r#"chose allow_once {"Which one?": "B"}"#); } + +/// The decision proof of a relayed answer to a question (cmux NEXT's pane, a chat on another +/// machine): as [`decision_proof`] under "acpmux-person-decision-answers-v1", with the +/// sha256 of the canonical answers JSON (`approvers::answers_digest`) as a sixth part. +fn decision_answers_proof(key: &str, parts: [&str; 5], canonical_answers: &str) -> String { + use cmux_local_auth::frontend_proof::{hex, hmac_sha256, unhex}; + use sha2::Digest; + let key = unhex::<32>(key).unwrap(); + let digest = hex_of(&sha2::Sha256::digest(canonical_answers.as_bytes())); + let mut message = b"acpmux-person-decision-answers-v1".to_vec(); + for part in parts.into_iter().chain([digest.as_str()]) { + message.push(0); + message.extend_from_slice(part.as_bytes()); + } + hex(&hmac_sha256(&key, &message)) +} + +/// Leo 2026-10-21: answering an agent's question in the app's pane, for a chat on an ssh peer, +/// was refused as "accepted only from the cmux app". A relayed answer counts with the person's +/// decision proof over exactly its answers; a proof that covers no answers, or other answers, +/// counts nothing and the question stays pending. +#[tokio::test] +async fn a_relayed_answer_counts_with_the_persons_proof_over_exactly_its_answers() { + let d = Daemon::start("decans", Some(KEY)); + let mut relay = Rpc::connect(&d.socket).await; + let id = new_session(&mut relay, &d).await; + let rid = relay.send("session/prompt", prompt(&id, "question: Which one?")).await; + let pending = relay.note("_acpmux/permission_pending", |_| true).await; + let pid = pending["permissionId"].as_str().unwrap().to_owned(); + let nonce = pending["decisionNonce"].as_str().unwrap().to_owned(); + let instance = pending["personInstance"].as_str().unwrap().to_owned(); + let answered = |proof: &str, answers: Value| { + let mut frame = decided(&id, &pid, "allow_once", &nonce, proof); + frame["answers"] = answers; + frame + }; + let parts = [instance.as_str(), id.as_str(), pid.as_str(), "allow_once", nonce.as_str()]; + + let covers_none = decision_proof(KEY, &instance, &id, &pid, "allow_once", &nonce); + let r = relay + .call("_acpmux/permission_respond", answered(&covers_none, json!({"Which one?": "B"}))) + .await; + assert_eq!(reason(&r), REASON, "{r}"); + let for_b = decision_answers_proof(KEY, parts, r#"{"Which one?":"B"}"#); + let r = relay + .call("_acpmux/permission_respond", answered(&for_b, json!({"Which one?": "A"}))) + .await; + assert_eq!(reason(&r), REASON, "{r}"); + assert_eq!(pending_ids(&mut relay, &id).await, vec![pid.clone()]); + + relay.ok("_acpmux/permission_respond", answered(&for_b, json!({"Which one?": "B"}))).await; + assert_eq!(turn_said(&mut relay, rid).await, r#"chose allow_once {"Which one?": "B"}"#); +} + +/// The answers proof's bytes, as the Mac app computes them too +/// (`AcpmuxPersonKeyAnswersProofTests`): key order by bytes, escapes as `approvers::canonical_json`. +#[test] +fn the_answers_decision_proof_matches_the_apps_vector() { + let answers = json!({"Which one?": "B", "Pick a color\n(one)": ["red", "blue"], "é": "\u{1}"}); + let proof = acpmux::hub::person::decision_answers_proof( + KEY, + "0123456789abcdef", + "sess-1", + "perm-1", + "allow_once", + "nonce-1", + &answers, + ); + assert_eq!( + proof.as_deref(), + Some("3b11c2af201d95772c106a4c8463a54b94e615f85e88eac0700f7d657c24f0e9") + ); +} diff --git a/cmux-tui/crates/cmux-tui-core/src/server/agent_session_answers_tests.rs b/cmux-tui/crates/cmux-tui-core/src/server/agent_session_answers_tests.rs new file mode 100644 index 000000000000..6e7520b2fc4a --- /dev/null +++ b/cmux-tui/crates/cmux-tui-core/src/server/agent_session_answers_tests.rs @@ -0,0 +1,43 @@ +/// A question's answers ride with the permission answer (`answers`, a JSON +/// object), to acpmux unchanged beside the person's decision proof, which on +/// that machine covers them (acpmux `decision_answers_proof`). Before, the +/// verb refused the field, so an answer from the Mac app's pane for a chat on +/// this machine never reached acpmux with its answers and was refused there +/// (#23869). Answers that are not an object, or too large, are a bad request. +#[test] +fn a_question_answer_carries_its_answers_and_proof_to_acpmux_unchanged() { + let mut fake = FakeAcpmux::start("answers"); + let mux = mux_with(&fake, "asa-answers"); + let surface = agent_tab(&mux, Some(SESSION)); + let mut client = Client::connect(&mux, "answers"); + assert_eq!(attach(&mut client, surface)["ok"], json!(true)); + let _ = fake.frame(); + let _ = client.event(WAIT).expect("the early record"); + fake.notify( + "_acpmux/permission_pending", + json!({"sessionId":SESSION,"permissionId":"perm_q","decisionNonce":"n", + "personInstance":"i","options":[{"optionId":"allow"}]}), + ); + let _ = client.event(WAIT).expect("the question is shown to the user"); + let nonce = "ab".repeat(32); + let proof = "cd".repeat(32); + let answer = |answers: Value| { + json!({"cmd":"agent-session-permission","surface":surface,"permission_id":"perm_q", + "option_id":"allow","answers":answers,"decision_nonce":nonce,"person_decision":proof}) + }; + assert_code(&client.request(answer(json!(["B"]))), "agent_session.bad_request"); + // Over 64 KiB, the bound of a prompt too. + let huge = "x".repeat(64 * 1024); + assert_code(&client.request(answer(json!({"Which one?": huge}))), "agent_session.bad_request"); + fake.no_frame(); + let reply = client.request(answer(json!({"Which one?":"B","Pick":{"answers":["red"]}}))); + assert_eq!(reply["ok"], json!(true), "{reply}"); + let sent = fake.frame(); + assert_eq!(sent["method"], json!("_acpmux/permission_respond")); + assert_eq!( + sent["params"], + json!({"sessionId":SESSION,"permissionId":"perm_q","optionId":"allow", + "answers":{"Which one?":"B","Pick":{"answers":["red"]}}, + "_meta":{"acpmux":{"decisionNonce":nonce,"personDecision":proof}}}) + ); +} diff --git a/cmux-tui/crates/cmux-tui-core/src/server/agent_session_attach.rs b/cmux-tui/crates/cmux-tui-core/src/server/agent_session_attach.rs index 4b3138755bc1..873ce1ffcdd9 100644 --- a/cmux-tui/crates/cmux-tui-core/src/server/agent_session_attach.rs +++ b/cmux-tui/crates/cmux-tui-core/src/server/agent_session_attach.rs @@ -44,6 +44,9 @@ pub(crate) const MAX_ATTACHMENTS_TOTAL: usize = 64; /// Records per attach or events page. pub(crate) const MAX_PAGE: u64 = 500; pub(crate) const MAX_PROMPT_BYTES: usize = 64 * 1024; +/// The most bytes of a question's `answers` (as JSON) one permission answer +/// carries; acpmux bounds each item itself. +const MAX_ANSWERS_BYTES: usize = 64 * 1024; /// Permission ids remembered per attachment (announced, not yet answered). const MAX_PERMISSIONS: usize = 64; /// Record kinds or categories one page may ask for: the agent pane asks for all @@ -288,6 +291,10 @@ struct PermissionParams { surface: SurfaceId, permission_id: String, option_id: String, + /// A question's answers (a JSON object, acpmux `hub/questions.rs`): + /// passed to acpmux unchanged; the decision proof covers them. + #[serde(default)] + answers: Option, /// The person's decision proof for this answer (acpmux `hub/person.rs`, /// cx-0q7o): passed to acpmux unchanged, and acpmux decides. #[serde(default)] @@ -781,6 +788,7 @@ fn call(attachment: &Attachment, command: AgentSessionCommand) -> Result Result 256 { return Err(Refusal::BadRequest); } + let answers = answers.filter(|a| !a.is_null()); + if answers + .as_ref() + .is_some_and(|a| !a.is_object() || a.to_string().len() > MAX_ANSWERS_BYTES) + { + return Err(Refusal::BadRequest); + } if !decision_nonce.as_deref().is_none_or(is_hex64) || !person_decision.as_deref().is_none_or(is_hex64) || answered_by @@ -802,6 +817,9 @@ fn call(attachment: &Attachment, command: AgentSessionCommand) -> Result("transport.gesture", { intent: { diff --git a/webviews/src/agent-session/acpmux/questionAnswerTicket.test.ts b/webviews/src/agent-session/acpmux/questionAnswerTicket.test.ts new file mode 100644 index 000000000000..c2f741dca707 --- /dev/null +++ b/webviews/src/agent-session/acpmux/questionAnswerTicket.test.ts @@ -0,0 +1,79 @@ +// A question's answer goes with the person's gesture ticket for its pick, as an option pick does: +// the host signs an answer for another machine's agent daemon only with that ticket (cx-0q7o), so +// an answer sent without one was refused there and the question card failed with "approve this +// on the Mac app". +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { JSDOM } from "jsdom"; + +const dom = new JSDOM("", { url: "http://localhost/" }); +const scope = globalThis as Record; +const saved = { + window: "window" in scope, + document: "document" in scope, + values: { window: scope.window, document: scope.document }, +}; +const gestures: unknown[] = []; + +beforeAll(() => { + scope.window = dom.window; + scope.document = dom.window.document; + // The host's message handler: a ticket for every transport.gesture, nothing for the rest. + (dom.window as unknown as Record).webkit = { + messageHandlers: { + agentSession: { + postMessage: (message: { method: string; params: { intent?: unknown } }) => { + if (message.method !== "transport.gesture") return { ok: true, value: {} }; + gestures.push(message.params.intent); + return { ok: true, value: { ticket: "ticket-1" } }; + }, + }, + }, + }; +}); +afterAll(() => { + for (const key of ["window", "document"] as const) + if (saved[key]) scope[key] = saved.values[key]; + else delete scope[key]; +}); + +test("a question's answer carries the person's ticket for its session, prompt and option", async () => { + const { AcpmuxDirectClient } = await import("./direct"); + const { MockAcpmuxSocket, mockHost } = await import("./mock"); + const sent: { method: string; params: Record }[] = []; + class RecordingHost extends MockAcpmuxSocket { + constructor() { + super(() => Promise.resolve()); + const host = this as unknown as { answer: (method: string, params: Record) => Promise }; + const answer = host.answer.bind(this); + host.answer = (method, params) => { + sent.push({ method, params }); + return answer(method, params); + }; + } + } + const client = await AcpmuxDirectClient.connect( + mockHost, + () => undefined, + undefined, + () => new RecordingHost() as unknown as WebSocket, + ); + try { + await client.permission("perm-1", "allow_once", { "Which one?": "B" }, "sess-1"); + expect(gestures).toEqual([ + { + method: "_acpmux/permission_respond", + params: { sessionId: "sess-1", permissionId: "perm-1", optionId: "allow_once" }, + }, + ]); + const respond = sent.find((frame) => frame.method === "_acpmux/permission_respond"); + expect(respond?.params).toEqual({ + sessionId: "sess-1", + permissionId: "perm-1", + optionId: "allow_once", + answers: { "Which one?": "B" }, + _meta: { cmuxGesture: "ticket-1" }, + }); + } finally { + client.close?.(); + } +});