-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Extract dogfood feedback sink from TerminalController into CmuxDogfoodFeedbackSink #6168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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"), | ||
| ] | ||
| ), | ||
| ] | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -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/<ISO8601>_<shortid>/` (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. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+71
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The doc says "the cheap per-field character caps run on the calling actor," but in Swift 6, calling a Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// - 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 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+140
to
+151
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| "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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 3259
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 91
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 4514
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 2612
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 7274
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 1013
Enforce non-negative invariants in
DogfoodFeedbackLimits.init.DogfoodFeedbackService.submitconsumes these values directly inString.prefix(limits.max*Chars)(lines 93–95) anddirectories.dropLast(keep)(line 211), both of which require non-negative arguments and will crash with a precondition failure if negative limits are passed. Add guards to validate at construction time.Suggested fix
public init( maxTextChars: Int, maxTerminalChars: Int, maxBuildStampChars: Int, maxBlobBase64Chars: Int, maxBlobBytes: Int, maxRetainedBundles: Int ) { + precondition(maxTextChars >= 0, "maxTextChars must be >= 0") + precondition(maxTerminalChars >= 0, "maxTerminalChars must be >= 0") + precondition(maxBuildStampChars >= 0, "maxBuildStampChars must be >= 0") + precondition(maxBlobBase64Chars >= 0, "maxBlobBase64Chars must be >= 0") + precondition(maxBlobBytes >= 0, "maxBlobBytes must be >= 0") + precondition(maxRetainedBundles >= 0, "maxRetainedBundles must be >= 0") self.maxTextChars = maxTextChars self.maxTerminalChars = maxTerminalChars self.maxBuildStampChars = maxBuildStampChars self.maxBlobBase64Chars = maxBlobBase64Chars self.maxBlobBytes = maxBlobBytes self.maxRetainedBundles = maxRetainedBundles }🤖 Prompt for AI Agents