From 33c7c4110d715db8a83145020ddb54e7d55a55e4 Mon Sep 17 00:00:00 2001 From: Aziz Albahar Date: Mon, 15 Jun 2026 12:31:02 -0700 Subject: [PATCH] Extract dogfood feedback sink from TerminalController into CmuxDogfoodFeedbackSink Moves the privileged Mac<->phone dogfood feedback persistence domain out of the TerminalController god file into a new leaf package, CmuxDogfoodFeedbackSink (zero deps, mirrors CmuxMobileSupport's manifest shape). What moved: - The field/blob size caps and bundle-retention cap (now DogfoodFeedbackLimits, a Sendable value type; .default carries the exact prior constants). - The privileged-domain email gate (isPrivilegedFeedbackEmail, now a static on DogfoodFeedbackService). - The validate + off-main decode + bundle write + prune logic (now DogfoodFeedbackService.submit, a nonisolated Sendable Service). - The write outcome type (now DogfoodFeedbackOutcome, one case per RPC response). Seams: FileManager (via an @Sendable provider, since FileManager isn't Sendable and the writer runs on a detached task), the cache root URL, and the clock are constructor-injected with production defaults, so tests drive it against a temp dir with a fixed timestamp. App side: v2MobileDogfoodFeedbackSubmit is now a thin forward. It still resolves the authenticated email via the main-actor MobileHostService.shared, reads the four wire fields via v2RawString, calls service.submit(...), and maps the outcome to V2CallResult. The service re-enforces the @manaflow.ai gate at the boundary. Byte-identical: same caps, same base64-char-cap-before-decode and byte-cap ordering, same Task.detached(.utility) off-main structure, same bundle dir naming (ISO8601 with colons -> '-', 8-char lowercase uuid), same 0700/0600 perms, same bundle.json schema/keys/sorted-keys/pretty, same lexicographic prune keeping newest 50, and the same RPC error codes/messages and .ok payload keys. Tests: 7 behavior tests (privilege gate, unauthorized-before-IO, base64 char cap, decoded byte cap, bundle write + manifest + perms, field capping, prune) with injected temp dir + fixed clock; swift test green. Removed ~147 lines from TerminalController.swift (14785 -> 14638); budget ratcheted. Wired the package into cmux.xcodeproj (5 pbxproj entries mirroring CmuxFeedback) and the app target import. NOTE: full app build validated by CI. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 2 +- .../CmuxDogfoodFeedbackSink/Package.swift | 36 +++ .../DogfoodFeedbackLimits.swift | 61 +++++ .../DogfoodFeedbackOutcome.swift | 22 ++ .../DogfoodFeedbackService.swift | 215 ++++++++++++++++++ .../DogfoodFeedbackSubmission.swift | 37 +++ .../DogfoodFeedbackServiceTests.swift | 165 ++++++++++++++ Sources/TerminalController.swift | 211 +++-------------- cmux.xcodeproj/project.pbxproj | 12 + 9 files changed, 581 insertions(+), 180 deletions(-) create mode 100644 Packages/CmuxDogfoodFeedbackSink/Package.swift create mode 100644 Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackLimits.swift create mode 100644 Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackOutcome.swift create mode 100644 Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackService.swift create mode 100644 Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackSubmission.swift create mode 100644 Packages/CmuxDogfoodFeedbackSink/Tests/CmuxDogfoodFeedbackSinkTests/DogfoodFeedbackServiceTests.swift diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index abe5cbe6275d..4c4fcb6cca25 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -4,7 +4,7 @@ 34202 CLI/cmux.swift 17778 Sources/AppDelegate.swift 16674 Sources/ContentView.swift -14785 Sources/TerminalController.swift +14638 Sources/TerminalController.swift 13358 Sources/Panels/BrowserPanel.swift 12361 Sources/Workspace.swift 12093 Sources/GhosttyTerminalView.swift diff --git a/Packages/CmuxDogfoodFeedbackSink/Package.swift b/Packages/CmuxDogfoodFeedbackSink/Package.swift new file mode 100644 index 000000000000..46d7bbea5861 --- /dev/null +++ b/Packages/CmuxDogfoodFeedbackSink/Package.swift @@ -0,0 +1,36 @@ +// swift-tools-version: 6.0 + +import PackageDescription + +let package = Package( + name: "CmuxDogfoodFeedbackSink", + platforms: [ + .iOS(.v18), + .macOS(.v14), + ], + products: [ + .library( + name: "CmuxDogfoodFeedbackSink", + targets: ["CmuxDogfoodFeedbackSink"] + ), + ], + targets: [ + .target( + name: "CmuxDogfoodFeedbackSink", + swiftSettings: [ + .swiftLanguageMode(.v6), + .enableUpcomingFeature("ExistentialAny"), + .enableUpcomingFeature("InternalImportsByDefault"), + ] + ), + .testTarget( + name: "CmuxDogfoodFeedbackSinkTests", + dependencies: ["CmuxDogfoodFeedbackSink"], + swiftSettings: [ + .swiftLanguageMode(.v6), + .enableUpcomingFeature("ExistentialAny"), + .enableUpcomingFeature("InternalImportsByDefault"), + ] + ), + ] +) diff --git a/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackLimits.swift b/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackLimits.swift new file mode 100644 index 000000000000..764be6421cf4 --- /dev/null +++ b/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackLimits.swift @@ -0,0 +1,61 @@ +import Foundation + +/// Hard caps for the agent feedback sink. The only intended caller is a +/// paired phone, but a malformed or hostile request must not be able to +/// allocate huge buffers, block the Mac UI, or grow the cache without bound. +/// Strings are capped by character count before any large allocation; the +/// base64 blob is rejected outright past its cap (so it is never decoded), +/// and a decoded blob past the byte cap is dropped. +public struct DogfoodFeedbackLimits: Sendable, Equatable { + /// Maximum number of characters retained from the free-form `text` field. + public var maxTextChars: Int + /// Maximum number of characters retained from the captured `terminal_text`. + public var maxTerminalChars: Int + /// Maximum number of characters retained from the `build_stamp` field. + public var maxBuildStampChars: Int + /// Maximum length of the base64-encoded diagnostic blob string. A request + /// past this is rejected without ever decoding the blob into a `Data`. + public var maxBlobBase64Chars: Int + /// Maximum size in bytes of the decoded diagnostic blob. A decoded blob + /// past this is dropped. + public var maxBlobBytes: Int + /// Keep at most this many bundle directories; older ones are pruned after + /// each write so a retrying client can't grow the cache without bound. + public var maxRetainedBundles: Int + + /// Create an explicit set of feedback sink caps. + /// - Parameters: + /// - maxTextChars: cap on the free-form `text` field. + /// - maxTerminalChars: cap on the captured `terminal_text` field. + /// - maxBuildStampChars: cap on the `build_stamp` field. + /// - maxBlobBase64Chars: cap on the base64 blob string length. + /// - maxBlobBytes: cap on the decoded blob size. + /// - maxRetainedBundles: how many bundle directories to retain. + public init( + maxTextChars: Int, + maxTerminalChars: Int, + maxBuildStampChars: Int, + maxBlobBase64Chars: Int, + maxBlobBytes: Int, + maxRetainedBundles: Int + ) { + self.maxTextChars = maxTextChars + self.maxTerminalChars = maxTerminalChars + self.maxBuildStampChars = maxBuildStampChars + self.maxBlobBase64Chars = maxBlobBase64Chars + self.maxBlobBytes = maxBlobBytes + self.maxRetainedBundles = maxRetainedBundles + } + + /// The production caps used by the macOS host feedback sink. These match + /// the values that previously lived as static constants on the host's RPC + /// router, byte for byte. + public static let `default` = DogfoodFeedbackLimits( + maxTextChars: 16_384, + maxTerminalChars: 262_144, + maxBuildStampChars: 512, + maxBlobBase64Chars: 8_388_608, // ~6 MiB decoded + maxBlobBytes: 6_291_456, // 6 MiB + maxRetainedBundles: 50 + ) +} diff --git a/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackOutcome.swift b/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackOutcome.swift new file mode 100644 index 000000000000..9ca8d9df90ba --- /dev/null +++ b/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackOutcome.swift @@ -0,0 +1,22 @@ +import Foundation + +/// The result of attempting to persist a dogfood feedback submission. Every +/// case maps one-to-one onto the RPC response the host returns to the phone, so +/// the host router can translate without re-deriving any error text or codes. +public enum DogfoodFeedbackOutcome: Sendable, Equatable { + /// The caller's account is not in the privileged feedback domain. Maps to an + /// `unauthorized` RPC error. + case unauthorized + + /// A field exceeded its size cap (or the decoded blob exceeded the byte + /// cap). Maps to an `invalid_params` RPC error carrying `reason`. + case invalidParams(reason: String) + + /// The bundle directory or files could not be created on disk. Maps to an + /// `internal_error` RPC error. + case internalError + + /// The bundle was written. Carries the absolute bundle directory path and + /// the number of bytes written to `diagnostic.log`. + case written(bundlePath: String, diagnosticLogBytes: Int) +} diff --git a/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackService.swift b/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackService.swift new file mode 100644 index 000000000000..fe8f7de18707 --- /dev/null +++ b/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackService.swift @@ -0,0 +1,215 @@ +public import Foundation + +/// Privileged agent feedback sink (the Mac to phone feedback loop). +/// +/// Validates and persists a ``DogfoodFeedbackSubmission`` from a paired phone: +/// it caps each text field by character count *before* any large allocation, +/// rejects an oversized base64 blob without ever decoding it, decodes and +/// size-checks the blob, then writes a self-contained bundle directory under +/// `~/.cache/cmux-dogfood-feedback/_/` (a `bundle.json` +/// manifest plus the decoded `diagnostic.log`) and prunes old bundles. +/// +/// The service is `nonisolated`: it holds no UI state, the cheap field caps run +/// on the caller's actor, and the decode plus synchronous filesystem I/O run on +/// a detached utility task so a multi-MiB payload can never stall the caller's +/// actor (the Mac UI). `FileManager`, the cache root, and the clock are injected +/// so tests can drive it against a temp directory with a fixed timestamp. +public struct DogfoodFeedbackService: Sendable { + private let limits: DogfoodFeedbackLimits + private let fileManagerProvider: @Sendable () -> FileManager + private let cacheRoot: URL + private let now: @Sendable () -> Date + + /// Create a feedback sink. + /// + /// `FileManager` is supplied through a provider closure rather than stored + /// directly because `FileManager` is not `Sendable` and the writer runs on a + /// detached task; the provider returns a fresh handle on the writer's + /// executor. + /// - Parameters: + /// - limits: the size and retention caps. Defaults to + /// ``DogfoodFeedbackLimits/default``. + /// - fileManagerProvider: returns the file manager used for all I/O. + /// Defaults to `FileManager.default`. + /// - cacheRoot: the directory bundles are written under. Defaults to + /// `~/.cache/cmux-dogfood-feedback`. + /// - now: the clock used to timestamp bundle names and manifests. Defaults + /// to the current date. + public init( + limits: DogfoodFeedbackLimits = .default, + fileManagerProvider: @escaping @Sendable () -> FileManager = { .default }, + cacheRoot: URL? = nil, + now: @escaping @Sendable () -> Date = { Date.now } + ) { + self.limits = limits + self.fileManagerProvider = fileManagerProvider + self.cacheRoot = cacheRoot ?? FileManager.default.homeDirectoryForCurrentUser + .appendingPathComponent(".cache", isDirectory: true) + .appendingPathComponent("cmux-dogfood-feedback", isDirectory: true) + self.now = now + } + + /// The privileged feedback domain. Mirrors `isManaflowEmail` in + /// `CmuxMobileShellModel` (the phone's routing source of truth) but is + /// replicated here so the macOS app target need not link that mobile + /// package just for this one suffix check. Trims and lowercases before + /// matching so stored casing or padding does not bypass the gate. + /// - Parameter email: the caller's authenticated account email, if any. + /// - Returns: `true` when `email` is in the privileged `@manaflow.ai` domain. + public static func isPrivilegedFeedbackEmail(_ email: String?) -> Bool { + guard let email else { return false } + let normalized = email.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + return normalized.hasSuffix("@manaflow.ai") + } + + /// Validate and persist a feedback submission, returning the outcome to map + /// onto an RPC response. + /// + /// The privilege check and the cheap per-field character caps run on the + /// calling actor; an oversized base64 blob is rejected here without + /// decoding. The decode plus filesystem writes run on a detached utility + /// task so a large payload never blocks the caller. + /// - Parameters: + /// - submission: the raw, un-capped wire fields. + /// - authenticatedEmail: the caller's authenticated account email, used to + /// enforce the privileged-domain gate at the trust boundary. + /// - Returns: the ``DogfoodFeedbackOutcome`` describing success or the + /// precise failure. + public func submit( + _ submission: DogfoodFeedbackSubmission, + authenticatedEmail: String? + ) async -> DogfoodFeedbackOutcome { + // Privilege check at the trust boundary: the privileged agent feedback + // sink is restricted to the @manaflow.ai domain; a crafted request from + // any other account is rejected here regardless of which route the phone + // UI chose. + guard Self.isPrivilegedFeedbackEmail(authenticatedEmail) else { + return .unauthorized + } + + // Cheap caller-actor validation first: cap each field by character count + // before allocating anything large, and reject an oversized base64 blob + // outright so it is never decoded into a giant Data. + let text = String(submission.text.prefix(limits.maxTextChars)) + let terminalText = String(submission.terminalText.prefix(limits.maxTerminalChars)) + let buildStamp = String(submission.buildStamp.prefix(limits.maxBuildStampChars)) + let diagnosticBlobBase64 = submission.diagnosticBlobBase64 + guard diagnosticBlobBase64.count <= limits.maxBlobBase64Chars else { + return .invalidParams(reason: "diagnostic_blob_base64 exceeds size limit") + } + + let maxBlobBytes = limits.maxBlobBytes + let fileManagerProvider = fileManagerProvider + let cacheRoot = cacheRoot + let maxRetainedBundles = limits.maxRetainedBundles + let now = now + // Off-caller-actor: decode the blob and write the bundle. A + // `Task.detached` keeps the (potentially multi-MiB) decode plus + // synchronous file I/O off the caller's actor so it never stalls the Mac + // UI. Returns a Sendable outcome. + return await Task.detached(priority: .utility) { () -> DogfoodFeedbackOutcome in + let decoded = Data(base64Encoded: diagnosticBlobBase64) ?? Data() + guard decoded.count <= maxBlobBytes else { + return .invalidParams(reason: "diagnostic blob exceeds size limit") + } + return Self.writeBundle( + text: text, + terminalText: terminalText, + buildStamp: buildStamp, + diagnosticData: decoded, + cacheRoot: cacheRoot, + fileManager: fileManagerProvider(), + maxRetainedBundles: maxRetainedBundles, + now: now + ) + }.value + } + + /// Persist a validated feedback bundle to disk. Runs off the caller's actor + /// (called from the detached task), so its synchronous file I/O never blocks + /// the Mac UI. All text inputs are already size-capped by the caller. + private static func writeBundle( + text: String, + terminalText: String, + buildStamp: String, + diagnosticData: Data, + cacheRoot: URL, + fileManager: FileManager, + maxRetainedBundles: Int, + now: @Sendable () -> Date + ) -> DogfoodFeedbackOutcome { + let root = cacheRoot + + let formatter = ISO8601DateFormatter() + formatter.formatOptions = [.withInternetDateTime] + // Colons are legal in HFS+/APFS but awkward in shell globs; swap for `-` + // so the directory name is paste-safe. + let timestamp = formatter.string(from: now()).replacingOccurrences(of: ":", with: "-") + let shortID = String(UUID().uuidString.prefix(8)).lowercased() + let bundleDir = root.appendingPathComponent("\(timestamp)_\(shortID)", isDirectory: true) + + do { + // The bundle holds visible terminal text and debug logs, which can + // contain credentials or other private data. Create the root and + // bundle dirs owner-only (0700) so no other local user can traverse + // into them, and chmod the written files to 0600. The dir is created + // 0700 first, so even the brief window before the file chmod is not + // world-readable through a traversable parent. + let dirAttributes: [FileAttributeKey: Any] = [.posixPermissions: 0o700] + try fileManager.createDirectory( + at: root, + withIntermediateDirectories: true, + attributes: dirAttributes + ) + try fileManager.createDirectory( + at: bundleDir, + withIntermediateDirectories: true, + attributes: dirAttributes + ) + let diagnosticURL = bundleDir.appendingPathComponent("diagnostic.log") + try diagnosticData.write(to: diagnosticURL) + try fileManager.setAttributes([.posixPermissions: 0o600], ofItemAtPath: diagnosticURL.path) + let manifest: [String: Any] = [ + "schema": "cmux.dogfood.feedback.v1", + "received_at": formatter.string(from: now()), + "text": text, + "terminal_text": terminalText, + "build_stamp": buildStamp, + "diagnostic_log_file": "diagnostic.log", + "diagnostic_log_bytes": diagnosticData.count, + ] + let manifestData = try JSONSerialization.data( + withJSONObject: manifest, + options: [.prettyPrinted, .sortedKeys] + ) + let manifestURL = bundleDir.appendingPathComponent("bundle.json") + try manifestData.write(to: manifestURL) + try fileManager.setAttributes([.posixPermissions: 0o600], ofItemAtPath: manifestURL.path) + } catch { + return .internalError + } + + pruneBundles(root: root, keep: maxRetainedBundles, fileManager: fileManager) + return .written(bundlePath: bundleDir.path, diagnosticLogBytes: diagnosticData.count) + } + + /// Keep only the newest `keep` bundle directories under `root`, deleting the + /// rest. The directory names start with an ISO8601 timestamp, so a + /// lexicographic sort is chronological. Best-effort: a failure to enumerate + /// or remove is ignored (it only affects cleanup, not the just-written + /// bundle). Runs off the caller's actor with its writer. + private static func pruneBundles(root: URL, keep: Int, fileManager: FileManager) { + guard let entries = try? fileManager.contentsOfDirectory( + at: root, + includingPropertiesForKeys: [.isDirectoryKey], + options: [.skipsHiddenFiles] + ) else { return } + let directories = entries + .filter { (try? $0.resourceValues(forKeys: [.isDirectoryKey]).isDirectory) == true } + .sorted { $0.lastPathComponent < $1.lastPathComponent } + guard directories.count > keep else { return } + for stale in directories.dropLast(keep) { + try? fileManager.removeItem(at: stale) + } + } +} diff --git a/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackSubmission.swift b/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackSubmission.swift new file mode 100644 index 000000000000..ebf88bc5abab --- /dev/null +++ b/Packages/CmuxDogfoodFeedbackSink/Sources/CmuxDogfoodFeedbackSink/DogfoodFeedbackSubmission.swift @@ -0,0 +1,37 @@ +import Foundation + +/// The raw, un-capped fields of a dogfood feedback submission as decoded from +/// the wire. The caller (the macOS host RPC router) extracts these from the +/// inbound RPC params verbatim; the ``DogfoodFeedbackService`` is responsible +/// for capping, validating, and persisting them. Keeping the type a plain +/// `Sendable` value lets the whole submission cross into the detached writer +/// task without copying logic. +public struct DogfoodFeedbackSubmission: Sendable, Equatable { + /// The free-form feedback text (`text` on the wire), un-capped. + public var text: String + /// The captured visible terminal text (`terminal_text` on the wire), un-capped. + public var terminalText: String + /// The build identifier the phone reported (`build_stamp` on the wire), un-capped. + public var buildStamp: String + /// The base64-encoded diagnostic blob (`diagnostic_blob_base64` on the + /// wire), un-decoded. + public var diagnosticBlobBase64: String + + /// Create a raw submission from the four wire fields. + /// - Parameters: + /// - text: the free-form feedback text. + /// - terminalText: the captured visible terminal text. + /// - buildStamp: the reported build identifier. + /// - diagnosticBlobBase64: the base64-encoded diagnostic blob. + public init( + text: String, + terminalText: String, + buildStamp: String, + diagnosticBlobBase64: String + ) { + self.text = text + self.terminalText = terminalText + self.buildStamp = buildStamp + self.diagnosticBlobBase64 = diagnosticBlobBase64 + } +} diff --git a/Packages/CmuxDogfoodFeedbackSink/Tests/CmuxDogfoodFeedbackSinkTests/DogfoodFeedbackServiceTests.swift b/Packages/CmuxDogfoodFeedbackSink/Tests/CmuxDogfoodFeedbackSinkTests/DogfoodFeedbackServiceTests.swift new file mode 100644 index 000000000000..e197474b5c53 --- /dev/null +++ b/Packages/CmuxDogfoodFeedbackSink/Tests/CmuxDogfoodFeedbackSinkTests/DogfoodFeedbackServiceTests.swift @@ -0,0 +1,165 @@ +import Foundation +import Testing + +@testable import CmuxDogfoodFeedbackSink + +@Suite("DogfoodFeedbackService") +struct DogfoodFeedbackServiceTests { + private func makeService( + limits: DogfoodFeedbackLimits = .default, + now: @escaping @Sendable () -> Date = { Date(timeIntervalSince1970: 1_700_000_000) } + ) -> (DogfoodFeedbackService, URL) { + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-dogfood-test-\(UUID().uuidString)", isDirectory: true) + let service = DogfoodFeedbackService(limits: limits, cacheRoot: root, now: now) + return (service, root) + } + + @Test("privileged domain gate trims, lowercases, and matches the suffix") + func privilegeGate() { + #expect(DogfoodFeedbackService.isPrivilegedFeedbackEmail("a@manaflow.ai")) + #expect(DogfoodFeedbackService.isPrivilegedFeedbackEmail(" A@Manaflow.AI \n")) + #expect(!DogfoodFeedbackService.isPrivilegedFeedbackEmail("a@example.com")) + #expect(!DogfoodFeedbackService.isPrivilegedFeedbackEmail(nil)) + #expect(!DogfoodFeedbackService.isPrivilegedFeedbackEmail("manaflow.ai@evil.com")) + } + + @Test("non-privileged caller is rejected before any I/O") + func unauthorized() async throws { + let (service, root) = makeService() + let outcome = await service.submit( + DogfoodFeedbackSubmission(text: "hi", terminalText: "", buildStamp: "", diagnosticBlobBase64: ""), + authenticatedEmail: "nope@example.com" + ) + #expect(outcome == .unauthorized) + #expect(!FileManager.default.fileExists(atPath: root.path)) + } + + @Test("oversized base64 string is rejected without decoding") + func base64CharCapRejected() async throws { + let limits = DogfoodFeedbackLimits( + maxTextChars: 16, maxTerminalChars: 16, maxBuildStampChars: 16, + maxBlobBase64Chars: 4, maxBlobBytes: 1024, maxRetainedBundles: 5 + ) + let (service, _) = makeService(limits: limits) + let outcome = await service.submit( + DogfoodFeedbackSubmission(text: "", terminalText: "", buildStamp: "", diagnosticBlobBase64: "AAAAAAAA"), + authenticatedEmail: "a@manaflow.ai" + ) + #expect(outcome == .invalidParams(reason: "diagnostic_blob_base64 exceeds size limit")) + } + + @Test("decoded blob over the byte cap is dropped") + func blobByteCapRejected() async throws { + let limits = DogfoodFeedbackLimits( + maxTextChars: 16, maxTerminalChars: 16, maxBuildStampChars: 16, + maxBlobBase64Chars: 1_000_000, maxBlobBytes: 4, maxRetainedBundles: 5 + ) + let (service, _) = makeService(limits: limits) + let blob = Data(repeating: 0xAB, count: 32).base64EncodedString() + let outcome = await service.submit( + DogfoodFeedbackSubmission(text: "", terminalText: "", buildStamp: "", diagnosticBlobBase64: blob), + authenticatedEmail: "a@manaflow.ai" + ) + #expect(outcome == .invalidParams(reason: "diagnostic blob exceeds size limit")) + } + + @Test("a valid submission writes a bundle with a manifest and decoded log") + func writesBundle() async throws { + let (service, root) = makeService() + let payload = Data("hello-diagnostic".utf8) + let outcome = await service.submit( + DogfoodFeedbackSubmission( + text: "bug report", + terminalText: "$ echo hi", + buildStamp: "DEV abc", + diagnosticBlobBase64: payload.base64EncodedString() + ), + authenticatedEmail: "dev@manaflow.ai" + ) + guard case let .written(bundlePath, bytes) = outcome else { + Issue.record("expected written, got \(outcome)") + return + } + #expect(bytes == payload.count) + let bundleDir = URL(fileURLWithPath: bundlePath) + let logURL = bundleDir.appendingPathComponent("diagnostic.log") + let manifestURL = bundleDir.appendingPathComponent("bundle.json") + #expect(try Data(contentsOf: logURL) == payload) + let manifest = try JSONSerialization.jsonObject(with: Data(contentsOf: manifestURL)) as? [String: Any] + #expect(manifest?["schema"] as? String == "cmux.dogfood.feedback.v1") + #expect(manifest?["text"] as? String == "bug report") + #expect(manifest?["terminal_text"] as? String == "$ echo hi") + #expect(manifest?["build_stamp"] as? String == "DEV abc") + #expect(manifest?["diagnostic_log_file"] as? String == "diagnostic.log") + #expect(manifest?["diagnostic_log_bytes"] as? Int == payload.count) + // Owner-only permissions on the bundle directory and files. + let dirPerms = try FileManager.default.attributesOfItem(atPath: bundleDir.path)[.posixPermissions] as? Int + let logPerms = try FileManager.default.attributesOfItem(atPath: logURL.path)[.posixPermissions] as? Int + #expect(dirPerms == 0o700) + #expect(logPerms == 0o600) + try? FileManager.default.removeItem(at: root) + } + + @Test("text fields are capped before persisting") + func capsFields() async throws { + let limits = DogfoodFeedbackLimits( + maxTextChars: 3, maxTerminalChars: 2, maxBuildStampChars: 1, + maxBlobBase64Chars: 1_000_000, maxBlobBytes: 1_000_000, maxRetainedBundles: 5 + ) + let (service, root) = makeService(limits: limits) + let outcome = await service.submit( + DogfoodFeedbackSubmission( + text: "abcdef", + terminalText: "xyz", + buildStamp: "ZZZ", + diagnosticBlobBase64: Data("d".utf8).base64EncodedString() + ), + authenticatedEmail: "dev@manaflow.ai" + ) + guard case let .written(bundlePath, _) = outcome else { + Issue.record("expected written, got \(outcome)") + return + } + let manifestURL = URL(fileURLWithPath: bundlePath).appendingPathComponent("bundle.json") + let manifest = try JSONSerialization.jsonObject(with: Data(contentsOf: manifestURL)) as? [String: Any] + #expect(manifest?["text"] as? String == "abc") + #expect(manifest?["terminal_text"] as? String == "xy") + #expect(manifest?["build_stamp"] as? String == "Z") + try? FileManager.default.removeItem(at: root) + } + + @Test("pruning keeps only the newest N bundle directories") + func prunesOldBundles() async throws { + let limits = DogfoodFeedbackLimits( + maxTextChars: 16, maxTerminalChars: 16, maxBuildStampChars: 16, + maxBlobBase64Chars: 1_000_000, maxBlobBytes: 1_000_000, maxRetainedBundles: 2 + ) + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-dogfood-prune-\(UUID().uuidString)", isDirectory: true) + // Distinct, monotonically increasing timestamps so the lexicographic + // sort is chronological and deterministic across writes. + var tick = 1_700_000_000.0 + let payload = Data("x".utf8).base64EncodedString() + for _ in 0..<5 { + let captured = tick + let service = DogfoodFeedbackService( + limits: limits, + cacheRoot: root, + now: { Date(timeIntervalSince1970: captured) } + ) + _ = await service.submit( + DogfoodFeedbackSubmission(text: "", terminalText: "", buildStamp: "", diagnosticBlobBase64: payload), + authenticatedEmail: "dev@manaflow.ai" + ) + tick += 60 + } + let remaining = try FileManager.default.contentsOfDirectory( + at: root, + includingPropertiesForKeys: nil, + options: [.skipsHiddenFiles] + ) + #expect(remaining.count == 2) + try? FileManager.default.removeItem(at: root) + } +} diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index a320051ca727..292fbdb1b260 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -15,6 +15,7 @@ import CmuxSocketControl import CmuxSwiftRenderUI import Carbon.HIToolbox import CMUXMobileCore +import CmuxDogfoodFeedbackSink import CMUXWorkstream import Foundation import Bonsplit @@ -13568,107 +13569,54 @@ class TerminalController { return mobileHostResult(result) } - /// Hard caps for the agent feedback sink. The only intended caller is a - /// paired phone, but a malformed or hostile request must not be able to - /// allocate huge buffers, block the Mac UI, or grow the cache without bound. - /// Strings are capped by character count before any large allocation; the - /// base64 blob is rejected outright past its cap (so it is never decoded), - /// and a decoded blob past the byte cap is dropped. - nonisolated private static let dogfoodFeedbackMaxTextChars = 16_384 - nonisolated private static let dogfoodFeedbackMaxTerminalChars = 262_144 - nonisolated private static let dogfoodFeedbackMaxBuildStampChars = 512 - nonisolated private static let dogfoodFeedbackMaxBlobBase64Chars = 8_388_608 // ~6 MiB decoded - nonisolated private static let dogfoodFeedbackMaxBlobBytes = 6_291_456 // 6 MiB - /// Keep at most this many bundle directories; older ones are pruned after - /// each write so a retrying client can't grow the cache without bound. - nonisolated private static let dogfoodFeedbackMaxRetainedBundles = 50 - - /// The privileged feedback domain. Mirrors `isManaflowEmail` in - /// `CmuxMobileShellModel` (the phone's routing source of truth) but is - /// replicated here so the macOS app target need not link that mobile - /// package just for this one suffix check. Trims + lowercases before - /// matching so stored casing/padding does not bypass the gate. - nonisolated static func isPrivilegedFeedbackEmail(_ email: String?) -> Bool { - guard let email else { return false } - let normalized = email.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() - return normalized.hasSuffix("@manaflow.ai") - } - /// Privileged agent feedback sink (the Mac↔phone feedback loop). /// - /// Decodes `{ text, terminal_text, build_stamp, diagnostic_blob_base64 }`, - /// writes a self-contained bundle directory under - /// `~/.cache/cmux-dogfood-feedback/_/` (a `bundle.json` - /// manifest plus the decoded `diagnostic.log`), and returns the bundle path. + /// Reads `{ text, terminal_text, build_stamp, diagnostic_blob_base64 }` off + /// the wire and hands them to ``DogfoodFeedbackService`` (in the + /// `CmuxDogfoodFeedbackSink` package), which caps the fields, rejects an + /// oversized base64 blob without decoding, and writes a self-contained + /// bundle directory under `~/.cache/cmux-dogfood-feedback/_/` + /// (a `bundle.json` manifest plus the decoded `diagnostic.log`) off the main + /// actor. This method owns only the trust-boundary privilege check and the + /// wire mapping; the validation, allocation caps, and filesystem I/O live in + /// the service. + /// /// It is protected by the same-account Stack-auth authorization the rest of /// the mobile data plane enforces, so it never accepts an unauthenticated /// caller. The phone only ever routes here for `@manaflow.ai` users on an /// active connection, so this exists in Release builds too (the team can /// dogfood beta/prod), and only a Mac that runs the watcher acts on it. - /// - /// Field sizes are capped on the main actor *before* any large allocation, - /// invalid/oversized base64 is rejected without decoding, and the decode + - /// filesystem writes run off the main actor so a large payload cannot block - /// the Mac UI. private func v2MobileDogfoodFeedbackSubmit(params: [String: Any]) async -> V2CallResult { // Privilege check at the trust boundary: the mobile data plane only // accepts same-account connections, so the caller is this Mac's own Stack - // account. The privileged agent feedback sink is restricted to the - // @manaflow.ai domain; a crafted RPC from any other account is rejected - // here regardless of which route the phone UI chose. (The phone also - // gates the route on `@manaflow.ai` + `dogfood.v1`, but the Mac is the - // real boundary.) + // account. The service re-enforces the @manaflow.ai gate, but we resolve + // the authenticated email here because it requires the main-actor + // `MobileHostService`. (The phone also gates the route on `@manaflow.ai` + // + `dogfood.v1`, but the Mac is the real boundary.) let localEmail = await MobileHostService.shared.currentAuthenticatedLocalUserEmail() - guard Self.isPrivilegedFeedbackEmail(localEmail) else { - return .err( - code: "unauthorized", - message: "Feedback agent sink is restricted to privileged accounts", - data: nil - ) - } - - // Cheap main-actor validation first: cap each field by character count - // before allocating anything large, and reject an oversized base64 blob - // outright so it is never decoded into a giant Data. - let text = String((v2RawString(params, "text") ?? "").prefix(Self.dogfoodFeedbackMaxTextChars)) - let terminalText = String((v2RawString(params, "terminal_text") ?? "").prefix(Self.dogfoodFeedbackMaxTerminalChars)) - let buildStamp = String((v2RawString(params, "build_stamp") ?? "").prefix(Self.dogfoodFeedbackMaxBuildStampChars)) - let diagnosticBlobBase64 = v2RawString(params, "diagnostic_blob_base64") ?? "" - guard diagnosticBlobBase64.count <= Self.dogfoodFeedbackMaxBlobBase64Chars else { - return .err( - code: "invalid_params", - message: "diagnostic_blob_base64 exceeds size limit", - data: nil - ) - } - - let maxBlobBytes = Self.dogfoodFeedbackMaxBlobBytes - // Off-main: decode the blob and write the bundle. A `Task.detached` - // keeps the (potentially multi-MiB) decode + synchronous file I/O off the - // main actor so it never stalls the Mac UI. Returns a Sendable result. - let outcome = await Task.detached(priority: .utility) { () -> DogfoodFeedbackWriteOutcome in - let decoded = Data(base64Encoded: diagnosticBlobBase64) ?? Data() - guard decoded.count <= maxBlobBytes else { - return .rejected(reason: "diagnostic blob exceeds size limit") - } - return Self.writeDogfoodFeedbackBundle( - text: text, - terminalText: terminalText, - buildStamp: buildStamp, - diagnosticData: decoded - ) - }.value - + let submission = DogfoodFeedbackSubmission( + text: v2RawString(params, "text") ?? "", + terminalText: v2RawString(params, "terminal_text") ?? "", + buildStamp: v2RawString(params, "build_stamp") ?? "", + diagnosticBlobBase64: v2RawString(params, "diagnostic_blob_base64") ?? "" + ) + let outcome = await DogfoodFeedbackService().submit(submission, authenticatedEmail: localEmail) switch outcome { - case let .written(bundlePath, byteCount): + case let .written(bundlePath, diagnosticLogBytes): return .ok([ "ok": true, "bundle_path": bundlePath, - "diagnostic_log_bytes": byteCount, + "diagnostic_log_bytes": diagnosticLogBytes, ]) - case let .rejected(reason): + case .unauthorized: + return .err( + code: "unauthorized", + message: "Feedback agent sink is restricted to privileged accounts", + data: nil + ) + case let .invalidParams(reason): return .err(code: "invalid_params", message: reason, data: nil) - case .failed: + case .internalError: return .err( code: "internal_error", message: "Failed to persist dogfood feedback bundle", @@ -13677,101 +13625,6 @@ class TerminalController { } } - /// The result of writing a dogfood feedback bundle off the main actor. - private enum DogfoodFeedbackWriteOutcome: Sendable { - case written(bundlePath: String, byteCount: Int) - case rejected(reason: String) - case failed - } - - /// Persist a validated dogfood feedback bundle to disk. Runs off the main - /// actor (called from a detached task), so its synchronous file I/O never - /// blocks the Mac UI. All inputs are already size-capped by the caller. - nonisolated private static func writeDogfoodFeedbackBundle( - text: String, - terminalText: String, - buildStamp: String, - diagnosticData: Data - ) -> DogfoodFeedbackWriteOutcome { - let fileManager = FileManager.default - let root = fileManager.homeDirectoryForCurrentUser - .appendingPathComponent(".cache", isDirectory: true) - .appendingPathComponent("cmux-dogfood-feedback", isDirectory: true) - - let formatter = ISO8601DateFormatter() - formatter.formatOptions = [.withInternetDateTime] - // Colons are legal in HFS+/APFS but awkward in shell globs; swap for `-` - // so the directory name is paste-safe. - let timestamp = formatter.string(from: Date()).replacingOccurrences(of: ":", with: "-") - let shortID = String(UUID().uuidString.prefix(8)).lowercased() - let bundleDir = root.appendingPathComponent("\(timestamp)_\(shortID)", isDirectory: true) - - do { - // The bundle holds visible terminal text and debug logs, which can - // contain credentials or other private data. Create the root and - // bundle dirs owner-only (0700) so no other local user can traverse - // into them, and chmod the written files to 0600. The dir is created - // 0700 first, so even the brief window before the file chmod is not - // world-readable through a traversable parent. - let dirAttributes: [FileAttributeKey: Any] = [.posixPermissions: 0o700] - try fileManager.createDirectory( - at: root, - withIntermediateDirectories: true, - attributes: dirAttributes - ) - try fileManager.createDirectory( - at: bundleDir, - withIntermediateDirectories: true, - attributes: dirAttributes - ) - let diagnosticURL = bundleDir.appendingPathComponent("diagnostic.log") - try diagnosticData.write(to: diagnosticURL) - try fileManager.setAttributes([.posixPermissions: 0o600], ofItemAtPath: diagnosticURL.path) - let manifest: [String: Any] = [ - "schema": "cmux.dogfood.feedback.v1", - "received_at": formatter.string(from: Date()), - "text": text, - "terminal_text": terminalText, - "build_stamp": buildStamp, - "diagnostic_log_file": "diagnostic.log", - "diagnostic_log_bytes": diagnosticData.count, - ] - let manifestData = try JSONSerialization.data( - withJSONObject: manifest, - options: [.prettyPrinted, .sortedKeys] - ) - let manifestURL = bundleDir.appendingPathComponent("bundle.json") - try manifestData.write(to: manifestURL) - try fileManager.setAttributes([.posixPermissions: 0o600], ofItemAtPath: manifestURL.path) - } catch { - return .failed - } - - pruneDogfoodFeedbackBundles(root: root, keep: dogfoodFeedbackMaxRetainedBundles) - return .written(bundlePath: bundleDir.path, byteCount: diagnosticData.count) - } - - /// Keep only the newest `keep` bundle directories under `root`, deleting the - /// rest. The directory names start with an ISO8601 timestamp, so a - /// lexicographic sort is chronological. Best-effort: a failure to enumerate - /// or remove is ignored (it only affects cleanup, not the just-written - /// bundle). Runs off the main actor with its writer. - nonisolated private static func pruneDogfoodFeedbackBundles(root: URL, keep: Int) { - let fileManager = FileManager.default - guard let entries = try? fileManager.contentsOfDirectory( - at: root, - includingPropertiesForKeys: [.isDirectoryKey], - options: [.skipsHiddenFiles] - ) else { return } - let directories = entries - .filter { (try? $0.resourceValues(forKeys: [.isDirectoryKey]).isDirectory) == true } - .sorted { $0.lastPathComponent < $1.lastPathComponent } - guard directories.count > keep else { return } - for stale in directories.dropLast(keep) { - try? fileManager.removeItem(at: stale) - } - } - /// Mobile-gated wrapper over ``v2WorkspaceAction(params:)``. private func v2MobileWorkspaceAction(params: [String: Any]) -> V2CallResult { let rawAction = v2RawString(params, "action") diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index e6bcb1aa93c1..5041083ec651 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -214,6 +214,7 @@ CC0DE00000000000000000A3 /* CmuxCore in Frameworks */ = {isa = PBXBuildFile; productRef = CC0DE00000000000000000A2 /* CmuxCore */; }; CC0DE00000000000000000A4 /* CmuxCore in Frameworks */ = {isa = PBXBuildFile; productRef = CC0DE00000000000000000A2 /* CmuxCore */; }; D1320AA0D1320AA0D1320AA2 /* CmuxDockTilePlugin.plugin in Copy Dock Tile Plugin */ = {isa = PBXBuildFile; fileRef = D1320AA0D1320AA0D1320AA5 /* CmuxDockTilePlugin.plugin */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; + DF60000000000000000000F3 /* CmuxDogfoodFeedbackSink in Frameworks */ = {isa = PBXBuildFile; productRef = DF60000000000000000000F2 /* CmuxDogfoodFeedbackSink */; }; E7E000000000000000000005 /* CmuxEventBus.swift in Sources */ = {isa = PBXBuildFile; fileRef = E7E000000000000000000006 /* CmuxEventBus.swift */; }; E7E000000000000000000003 /* CmuxEventBusTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E7E000000000000000000004 /* CmuxEventBusTests.swift */; }; E7E00000000000000000000D /* CmuxEventLogWriter.swift in Sources */ = {isa = PBXBuildFile; fileRef = E7E00000000000000000000E /* CmuxEventLogWriter.swift */; }; @@ -1775,6 +1776,7 @@ 5E55300000000000000000C3 /* CmuxCommandPaletteUI in Frameworks */, C750100000000000000000A3 /* CmuxControlSocket in Frameworks */, CC0DE00000000000000000A3 /* CmuxCore in Frameworks */, + DF60000000000000000000F3 /* CmuxDogfoodFeedbackSink in Frameworks */, C0DE46000000000000000001 /* CMUXExtensionHostSupport in Frameworks */, C0DE45000000000000000001 /* CmuxExtensionKit in Frameworks */, C0DE49000000000000000001 /* CmuxExtensionSidebarExamples in Frameworks */, @@ -3039,6 +3041,7 @@ C750100000000000000000A1 /* XCLocalSwiftPackageReference "CmuxControlSocket" */, C9A2B00000000000000000B1 /* XCLocalSwiftPackageReference "CmuxAppKitSupportUI" */, C9A2C00000000000000000C1 /* XCLocalSwiftPackageReference "CmuxFeedback" */, + DF60000000000000000000F1 /* XCLocalSwiftPackageReference "CmuxDogfoodFeedbackSink" */, C9A2D00000000000000000D1 /* XCLocalSwiftPackageReference "CmuxFeedbackUI" */, C750200000000000000000A1 /* XCLocalSwiftPackageReference "CmuxTerminalCore" */, C750300000000000000000A1 /* XCLocalSwiftPackageReference "CmuxTerminalEngine" */, @@ -4595,6 +4598,10 @@ isa = XCLocalSwiftPackageReference; relativePath = Packages/CmuxFeedback; }; + DF60000000000000000000F1 /* XCLocalSwiftPackageReference "CmuxDogfoodFeedbackSink" */ = { + isa = XCLocalSwiftPackageReference; + relativePath = Packages/CmuxDogfoodFeedbackSink; + }; C9A2D00000000000000000D1 /* XCLocalSwiftPackageReference "CmuxFeedbackUI" */ = { isa = XCLocalSwiftPackageReference; relativePath = Packages/CmuxFeedbackUI; @@ -4874,6 +4881,11 @@ package = C9A2C00000000000000000C1 /* XCLocalSwiftPackageReference "CmuxFeedback" */; productName = CmuxFeedback; }; + DF60000000000000000000F2 /* CmuxDogfoodFeedbackSink */ = { + isa = XCSwiftPackageProductDependency; + package = DF60000000000000000000F1 /* XCLocalSwiftPackageReference "CmuxDogfoodFeedbackSink" */; + productName = CmuxDogfoodFeedbackSink; + }; C9A2D00000000000000000D2 /* CmuxFeedbackUI */ = { isa = XCSwiftPackageProductDependency; package = C9A2D00000000000000000D1 /* XCLocalSwiftPackageReference "CmuxFeedbackUI" */;