diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 1c968a8499fd..86bae3bc157c 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -25,6 +25,9 @@ reviews: - path: "**/*.{ts,tsx,js,jsx,mjs,cjs,sh,zsh}" instructions: | Apply `.github/review-bot-rules/runtime-no-hacky-sleeps.md` during review. For production runtime, script, and build changes, flag fixed sleeps, timers, delayed dispatch, polling, or wall-clock waits used as synchronization. Pass for tests, pure presentation timing, dedicated cancellation-aware retry/timeout abstractions with tests, and existing delay code not worsened. + - path: "**/*.{swift,ts,tsx,js,jsx,mjs,cjs}" + instructions: | + Apply `.github/review-bot-rules/user-facing-errors.md` during review. For production user-facing errors, alerts, command output, API error bodies, and recovery copy, flag implementation leaks such as upstream vendor names, internal provider names, environment variables, database or migration details, raw upstream messages, internal billing ids, or unredacted payloads. pre_merge_checks: custom_checks: @@ -56,6 +59,10 @@ reviews: mode: error instructions: | For production Swift changes, fail when the diff violates `.github/review-bot-rules/swift-logging.md`: `print`, `debugPrint`, `dump`, or `NSLog` in app/runtime code; ad hoc file/stdout logging for diagnostics; MainActor-coupled file-scoped Logger constants; or logs that expose secrets or personal data. Do not require new logs for new code paths; only check logging that the diff adds or materially changes. Pass for CLI output, tests, debug-only logs, and explicitly sanitized provider diagnostics. + - name: "cmux user-facing error privacy" + mode: error + instructions: | + For production changes, fail when the diff violates `.github/review-bot-rules/user-facing-errors.md`: user-facing errors, alerts, command output, API error bodies, or recovery copy must not expose upstream vendor names, internal provider names, provider-specific flags, templates, snapshots, manifests, environment variables, database or migration details, raw upstream messages, billing item ids, billing customer ids, unrelated team ids, credentials, tokens, headers, private keys, refresh tokens, session ids, or unredacted payload dumps. Pass for tests, docs, operational runbooks, developer-only comments, safe generic terms like billing/team/Cloud VM service, and explicitly advanced help text for user-configured settings. - name: "cmux SwiftUI state layout" mode: error instructions: | diff --git a/.github/review-bot-rules/README.md b/.github/review-bot-rules/README.md index 537709ba0fdc..e82154fae151 100644 --- a/.github/review-bot-rules/README.md +++ b/.github/review-bot-rules/README.md @@ -18,5 +18,6 @@ Current rules: - `swift-file-package-boundaries.md` - `swift-logging.md` - `swiftui-state-layout.md` +- `user-facing-errors.md` Open source repository note: review bots should apply the configuration from the base branch. A PR that edits these rules should not be able to weaken its own review. diff --git a/.github/review-bot-rules/user-facing-errors.md b/.github/review-bot-rules/user-facing-errors.md new file mode 100644 index 000000000000..6c4f18479cc0 --- /dev/null +++ b/.github/review-bot-rules/user-facing-errors.md @@ -0,0 +1,23 @@ +# User-Facing Error Messages + +Flag production changes that add or materially change user-facing errors, alerts, command output, API error bodies, or recovery copy when they expose implementation details. + +Fail when user-facing text includes: + +- Upstream vendor or service names unless the user explicitly configured that vendor in the product UI. +- Internal provider names, provider-specific flags, templates, snapshots, manifests, environment variable names, database or migration details. +- Raw upstream error messages, stack traces, request ids from third-party systems, billing item ids, billing customer ids, or team ids unless the user supplied that exact id in the request. +- Secret material, credentials, tokens, headers, private keys, refresh tokens, session ids, or unredacted payload dumps. + +Expected shape: + +- State what happened in cmux/product terms. +- Give one or two concrete next actions the user can take. +- Put only safe, minimal diagnostics in `details`. +- Keep provider, billing, database, and auth implementation details in sanitized logs or internal telemetry, not in user-visible text. + +Allowed cases: + +- Developer-only comments, tests, docs, and operational runbooks that are not shown to end users. +- Existing public CLI flags or config keys in help text when the user asked for advanced configuration help. +- Generic terms such as "billing", "team", "Cloud VM service", or "Cloud VM state". diff --git a/.greptile/rules.md b/.greptile/rules.md index 64dcdd0eca83..1c42456f7065 100644 --- a/.greptile/rules.md +++ b/.greptile/rules.md @@ -15,3 +15,12 @@ Review production Swift and runtime changes for: - Production logging that bypasses unified logging or leaks sensitive data. - SwiftUI state and layout patterns that cause stale state, broad invalidation, or render-time mutation. - Architectural fixes that patch symptoms while leaving bad state representable. +- User-facing errors, alerts, command output, API error bodies, and recovery copy that expose implementation details. + +## User-Facing Error Messages + +For production user-facing errors, alerts, command output, API error bodies, and recovery copy, do not expose implementation details. + +Flag copy that includes upstream vendor or service names, internal provider names, provider-specific flags, templates, snapshots, manifests, environment variable names, database or migration details, raw upstream error messages, stack traces, request ids from third-party systems unless the user supplied that exact id, billing item ids, billing customer ids, team ids not supplied by the user, credentials, tokens, headers, private keys, refresh tokens, session ids, or unredacted payload dumps. + +Error copy should say what happened in cmux terms, provide concrete user actionables, and keep only safe minimal diagnostics in `details`. Provider, billing, database, and auth implementation details belong in sanitized logs or internal telemetry. diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 6bd7088647d5..6fdb2fc42fa4 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -1813,12 +1813,111 @@ final class SocketClient { if let error = response["error"] as? [String: Any] { let code = (error["code"] as? String) ?? "error" let message = (error["message"] as? String) ?? "Unknown v2 error" - throw CLIError(message: "\(code): \(message)") + let action = error["action"] as? String + let reason = error["reason"] as? String + throw CLIError( + message: formatV2Error( + code: code, + message: message, + action: action, + reason: reason, + details: safeV2Details(error["details"]) + ) + ) } throw CLIError(message: "v2 request failed") } + private func formatV2Error( + code: String, + message: String, + action: String? = nil, + reason: String? = nil, + details: String? = nil + ) -> String { + let header: String + if code == "vm_error" { + header = message + } else if message.contains("\n") { + header = "\(code):\n\(message)" + } else { + header = "\(code): \(message)" + } + var sections = [header] + if let reason = trimmedNonEmptyV2Text(reason) { + sections.append("Reason:\n\(indentV2ErrorLines(reason))") + } + if let action = trimmedNonEmptyV2Text(action) { + sections.append("What to do:\n\(indentV2ErrorLines(action))") + } + if let details = trimmedNonEmptyV2Text(details) { + sections.append("Details:\n\(indentV2ErrorLines(details))") + } + return sections.joined(separator: "\n\n") + } + + private func safeV2Details(_ value: Any?) -> String? { + guard let value else { return nil } + if let string = value as? String { + return trimmedNonEmptyV2Text(string) + } + if let dictionary = value as? [String: Any] { + let allowedKeys = Set([ + "amount", + "code", + "duration", + "durationMs", + "field", + "idempotencyKeySet", + "imageRequested", + "limit", + "operation", + "retryable", + "status", + "type", + "vmId", + ]) + let lines = dictionary.keys.sorted().compactMap { key -> String? in + guard allowedKeys.contains(key), let value = dictionary[key], !(value is NSNull) else { return nil } + return "\(key): \(safeV2DetailValue(value))" + } + return lines.isEmpty ? nil : lines.joined(separator: "\n") + } + return nil + } + + private func safeV2DetailValue(_ value: Any) -> String { + if let string = value as? String { + return string.replacingOccurrences(of: "\n", with: "\\n") + .replacingOccurrences(of: "\r", with: "\\r") + } + if let number = value as? NSNumber { + if CFGetTypeID(number) == CFBooleanGetTypeID() { + return number.boolValue ? "true" : "false" + } + return "\(number)" + } + if value is [String: Any] || value is [Any] { + return "available" + } + return String(describing: value) + .replacingOccurrences(of: "\n", with: "\\n") + .replacingOccurrences(of: "\r", with: "\\r") + } + + private func trimmedNonEmptyV2Text(_ value: String?) -> String? { + let trimmed = value?.trimmingCharacters(in: .whitespacesAndNewlines) + return trimmed?.isEmpty == false ? trimmed : nil + } + + private func indentV2ErrorLines(_ value: String) -> String { + value + .split(separator: "\n", omittingEmptySubsequences: false) + .map { " \($0)" } + .joined(separator: "\n") + } + func streamV2( method: String, params: [String: Any] = [:], @@ -1961,11 +2060,20 @@ struct CMUXCLI { } let normalized = trimmed.lowercased() guard normalized == "e2b" || normalized == "freestyle" else { - throw CLIError(message: "vm new: unsupported provider '\(trimmed)'. Expected e2b or freestyle.") + throw CLIError(message: """ + vm new: unsupported Cloud VM service override. + + Try: + cmux vm new + """) } return normalized } + private static func isFlagToken(_ value: String) -> Bool { value.hasPrefix("-") && value != "-" } + + private static func isUnknownFlagToken(_ value: String, allowedShortFlags: Set = []) -> Bool { isFlagToken(value) && !allowedShortFlags.contains(value) } + private static func vmCreateIdempotencyStoreURL() -> URL { FileManager.default.homeDirectoryForCurrentUser .appendingPathComponent(".cmuxterm", isDirectory: true) @@ -2709,15 +2817,33 @@ struct CMUXCLI { let (providerOpt, rem1) = parseOption(rem0, name: "--provider") let detach = hasFlag(rem1, name: "--detach") || hasFlag(rem1, name: "-d") let remaining = rem1.filter { $0 != "--detach" && $0 != "-d" } - if let unknown = remaining.first(where: { $0.hasPrefix("--") }) { - throw CLIError(message: "vm new: unknown flag '\(unknown)'. Known flags: --image, --provider, --detach/-d") + if let unknown = remaining.first(where: { Self.isUnknownFlagToken($0, allowedShortFlags: ["-d"]) }) { + throw CLIError(message: """ + vm new: unknown flag '\(unknown)'. + + Known flags: + --image + --provider + --detach, -d + + Try: + cmux vm new + """) } // Stray positional args (e.g. a typo like `cmux vm new myvm`) previously fell // through and still provisioned a VM. That silently costs the user money and // hides the typo. Reject them explicitly. - if let extra = remaining.first(where: { !$0.hasPrefix("--") && $0 != "-d" }) { + if let extra = remaining.first(where: { !Self.isFlagToken($0) }) { throw CLIError( - message: "vm new: unexpected argument '\(extra)'. vm new takes no positional args; use --image / --provider / --detach." + message: """ + vm new: unexpected argument '\(extra)'. + + `cmux vm new` does not take a VM name or positional arguments. + + Try: + cmux vm new + cmux vm new --detach + """ ) } let normalizedProvider = try Self.normalizedVMProvider(providerOpt) @@ -2768,7 +2894,12 @@ struct CMUXCLI { case "shell", "attach": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux \(command) shell ") + throw CLIError(message: """ + Usage: cmux \(command) shell + + Find an id: + cmux vm ls + """) } let shortId = String(vmId.prefix(8)) try vmOpenShell( @@ -2781,7 +2912,12 @@ struct CMUXCLI { case "rm", "destroy", "delete": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux vm rm ") + throw CLIError(message: """ + Usage: cmux vm rm + + Find an id: + cmux vm ls + """) } _ = try client.sendV2(method: "vm.destroy", params: ["id": vmId], responseTimeout: 60) if jsonOutput { @@ -2792,7 +2928,12 @@ struct CMUXCLI { case "ssh": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux \(command) ssh ") + throw CLIError(message: """ + Usage: cmux \(command) ssh + + Find an id: + cmux vm ls + """) } let shortId = String(vmId.prefix(8)) try vmOpenShell( @@ -2805,7 +2946,12 @@ struct CMUXCLI { case "ssh-info": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux \(command) ssh-info ") + throw CLIError(message: """ + Usage: cmux \(command) ssh-info + + Find an id: + cmux vm ls + """) } try printVMSSHInfo(id: vmId, command: command, client: client, jsonOutput: jsonOutput) @@ -2814,7 +2960,13 @@ struct CMUXCLI { case "exec": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux vm exec -- ") + throw CLIError(message: """ + Usage: cmux vm exec -- + + Examples: + cmux vm ls + cmux vm exec -- pwd + """) } var commandArgsForVM: [String] = Array(rest.dropFirst()) // Consume a leading "--" separator if present. @@ -2822,7 +2974,12 @@ struct CMUXCLI { commandArgsForVM.removeFirst() } guard !commandArgsForVM.isEmpty else { - throw CLIError(message: "Usage: cmux vm exec -- ") + throw CLIError(message: """ + Usage: cmux vm exec -- + + Example: + cmux vm exec \(vmId) -- uname -a + """) } // Shell-quote each argv element before joining. Plain-space join previously // dropped quoting so `cmux vm exec -- printf '%s\n' "a b"` reached the @@ -2856,7 +3013,15 @@ struct CMUXCLI { } default: - throw CLIError(message: "Usage: cmux \(command) [args...]") + throw CLIError(message: """ + Usage: cmux \(command) [args...] + + Common commands: + cmux vm ls + cmux vm new + cmux vm ssh + cmux vm rm + """) } case "rpc": @@ -6274,7 +6439,16 @@ struct CMUXCLI { let endpoint = try parseVMPtyWebSocketEndpoint(response) guard endpoint.daemon != nil else { throw CLIError( - message: "vm.attach_info returned a WebSocket PTY without daemon/proxy support. Rebuild the cloud VM image or snapshot with the current cmuxd-remote." + message: """ + This Cloud VM image does not support interactive attach in this cmux build. + + What to do: + Update cmux, then create a fresh VM with `cmux vm new`. + If this keeps happening, contact support with the VM id. + + Details: + Interactive attach is not available for this VM image. + """ ) } try runVMPtyWebSocketWorkspace( @@ -6319,19 +6493,55 @@ struct CMUXCLI { let cred = response["credential"] as? [String: Any], let kind = cred["kind"] as? String else { - throw CLIError(message: "vm.attach_info returned malformed SSH payload: \(response)") + throw CLIError(message: """ + cmux could not read the attach information for this Cloud VM. + + What to do: + Retry `cmux vm ssh `. + If it keeps failing, recreate the VM with `cmux vm new` and share the details below. + + Details: + Cloud VM attach details were incomplete. + """) } guard kind == "password" else { if kind == "authorizedKey" { throw CLIError( - message: "authorizedKey credentials aren't supported by `cmux vm shell` yet; received from server." + message: """ + This Cloud VM does not support interactive SSH attach in this cmux build. + + What to do: + Update cmux and retry. + If this keeps happening, contact support with the VM id. + + Details: + Interactive SSH attach is unavailable for this VM. + """ ) } - throw CLIError(message: "vm.attach_info returned unknown credential kind: \(kind)") + throw CLIError(message: """ + cmux could not use the attach information for this Cloud VM. + + What to do: + Retry `cmux vm ssh `. + If it keeps failing, recreate the VM with `cmux vm new`. + + Details: + Interactive SSH attach is unavailable for this VM. + """) } guard let token = cred["value"] as? String, !token.isEmpty else { - throw CLIError(message: "vm.attach_info password credential missing `value`") + throw CLIError(message: """ + cmux could not open an interactive SSH session for this Cloud VM. + + What to do: + Retry `cmux vm ssh `. + If it keeps failing, recreate the VM with `cmux vm new`. + + Details: + Cloud VM attach details were incomplete. + """) } // Freestyle gateway has a fresh host key per session and we re-mint per attach, @@ -6387,16 +6597,18 @@ struct CMUXCLI { print(" username: \(username)") print(" password: \(credValue)") } else { - let kindDescription = credKind.isEmpty || credKind == "?" ? "unknown" : credKind - print("credential kind \"\(kindDescription)\" not yet supported by `cmux \(command) ssh-info`; raw response:") - print(jsonString(response)) + print("This Cloud VM does not support `cmux \(command) ssh-info` in this cmux build.") + print("") + print("What to do:") + print(" Update cmux and retry.") + print(" If this keeps happening, contact support with the VM id.") } } private func runVMSSHAttach(commandArgs: [String], client: SocketClient) throws { let (vmIDOpt, remaining) = parseOption(commandArgs, name: "--id") - if let unknown = remaining.first(where: { $0.hasPrefix("--") }) { - throw CLIError(message: "vm ssh-attach: unknown flag '\(unknown)'") + if let unknown = remaining.first(where: { Self.isFlagToken($0) }) { + throw CLIError(message: "vm ssh-attach: unknown flag '\(unknown)'. Use `cmux vm ssh-attach --id `.") } guard remaining.isEmpty else { throw CLIError(message: "Usage: cmux vm ssh-attach --id ") @@ -6417,7 +6629,7 @@ struct CMUXCLI { ) let sshArguments = buildSSHCommandArguments(options) guard let launchPath = sshArguments.first else { - throw CLIError(message: "vm ssh-attach: failed to construct ssh command") + throw CLIError(message: "vm ssh-attach could not construct an ssh command. Retry `cmux vm ssh ` from a normal cmux shell.") } client.close() try execInteractiveProgram( @@ -6438,7 +6650,16 @@ struct CMUXCLI { guard let url = response["url"] as? String, let token = response["token"] as? String, let sessionId = response["session_id"] as? String else { - throw CLIError(message: "vm.attach_info websocket endpoint missing url/token/session_id: \(response)") + throw CLIError(message: """ + cmux could not read the attach information for this Cloud VM. + + What to do: + Retry `cmux vm ssh `. + If it keeps failing, recreate the VM with `cmux vm new`. + + Details: + Cloud VM attach details were incomplete. + """) } let headers = parseHeaders(response["headers"]) let expiresAtUnix = (response["expires_at_unix"] as? Int64) @@ -6623,8 +6844,8 @@ struct CMUXCLI { private func runVMPtyAttach(commandArgs: [String], client: SocketClient) throws { let (vmIDOpt, remaining) = parseOption(commandArgs, name: "--id") - if let unknown = remaining.first(where: { $0.hasPrefix("--") }) { - throw CLIError(message: "vm-pty-attach: unknown flag '\(unknown)'") + if let unknown = remaining.first(where: { Self.isFlagToken($0) }) { + throw CLIError(message: "vm-pty-attach: unknown flag '\(unknown)'. Use `cmux vm-pty-attach --id `.") } guard remaining.isEmpty else { throw CLIError(message: "Usage: cmux vm-pty-attach --id ") @@ -9126,7 +9347,7 @@ struct CMUXCLI { Subcommands: ls List your cloud VMs. - new [--image