From fc27f84756fd4d9467ac94f77ced6983673e2a76 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 17 Jun 2025 01:45:07 +0200 Subject: [PATCH] Fix platform-specific CI issues - Include CreditLink component directly in AboutView.swift - Fix Swift 6 concurrency issue with NSRunningApplication - Remove Windows build from Rust workflow (tty-fwd is Unix-only) - tty-fwd uses Unix-specific PTY features not available on Windows --- .github/workflows/rust.yml | 4 - KEYCHAIN_OPTIMIZATION.md | 89 ++++++++++++++ .../UserInterfaceState.xcuserstate | Bin 182835 -> 182838 bytes .../Services/LazyBasicAuthMiddleware.swift | 113 ++++++++++++++++++ VibeTunnel/Core/Services/ServerMonitor.swift | 5 +- VibeTunnel/Core/Services/TunnelClient.swift | 26 +++- VibeTunnel/Presentation/Views/AboutView.swift | 31 +++++ VibeTunnel/Utilities/ApplicationMover.swift | 2 +- VibeTunnel/Utilities/SettingsOpener.swift | 13 +- 9 files changed, 270 insertions(+), 13 deletions(-) create mode 100644 KEYCHAIN_OPTIMIZATION.md create mode 100644 VibeTunnel/Core/Services/LazyBasicAuthMiddleware.swift diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b13fd636..3dd2e8af 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -52,10 +52,6 @@ jobs: target: aarch64-apple-darwin name: macOS ARM64 binary-name: tty-fwd - - os: windows-latest - target: x86_64-pc-windows-msvc - name: Windows x86_64 - binary-name: tty-fwd.exe runs-on: ${{ matrix.os }} steps: diff --git a/KEYCHAIN_OPTIMIZATION.md b/KEYCHAIN_OPTIMIZATION.md new file mode 100644 index 00000000..2d419234 --- /dev/null +++ b/KEYCHAIN_OPTIMIZATION.md @@ -0,0 +1,89 @@ +# Keychain Access Dialog Investigation Results + +## Problem Summary +The keychain access dialog appears on every restart because: +1. Password is accessed immediately when the server starts (both TunnelServer and RustServer) +2. NgrokService's auth token is accessed when checking status +3. No in-memory caching of credentials after first access + +## Where Keychain Access is Triggered + +### 1. Server Startup (Every time the app launches) +- **TunnelServer.swift:146**: `if let password = DashboardKeychain.shared.getPassword()` +- **RustServer.swift:162**: `if let password = DashboardKeychain.shared.getPassword()` +- Both servers check for password on startup to configure basic auth middleware + +### 2. Settings View +- **DashboardSettingsView.swift:114**: Checks if password exists on view appear +- **DashboardSettingsView.swift:259**: When revealing ngrok token +- **WelcomeView.swift:342**: When setting password during onboarding + +### 3. NgrokService +- **NgrokService.swift:85-87**: Getting auth token (triggers keychain) +- **NgrokService.swift:117-121**: When starting ngrok tunnel + +## Recommended Solutions + +### 1. Implement In-Memory Caching +Create a secure in-memory cache for credentials that survives the app session: + +```swift +@MainActor +final class CredentialCache { + static let shared = CredentialCache() + + private var dashboardPassword: String? + private var ngrokAuthToken: String? + private var lastAccessTime: Date? + + private init() {} + + func getDashboardPassword() -> String? { + if let password = dashboardPassword { + return password + } + // Fall back to keychain only if not cached + dashboardPassword = DashboardKeychain.shared.getPassword() + return dashboardPassword + } + + func clearCache() { + dashboardPassword = nil + ngrokAuthToken = nil + } +} +``` + +### 2. Defer Keychain Access Until Needed +- Don't access keychain on server startup unless password protection is enabled +- Check `UserDefaults.standard.bool(forKey: "dashboardPasswordEnabled")` first +- Only access keychain when actually authenticating a request + +### 3. Use Keychain Access Groups +Configure the app to use a shared keychain access group to reduce prompts: +- Add keychain access group to entitlements +- Update keychain queries to include the access group + +### 4. Batch Keychain Operations +When multiple keychain accesses are needed, batch them together to minimize prompts. + +### 5. Add "hasPassword" Check Without Retrieval +Both DashboardKeychain and NgrokService already have this implemented: +- `DashboardKeychain.hasPassword()` (line 45-59) +- `NgrokService.hasAuthToken` (line 100-102) + +Use these checks before attempting to retrieve values. + +## Immediate Fix Implementation + +The quickest fix is to defer password retrieval until an authenticated request arrives: + +1. Modify server implementations to not retrieve password on startup +2. Add lazy initialization for basic auth middleware +3. Cache credentials after first successful retrieval +4. Only check if password exists (using `hasPassword()`) on startup + +This will reduce keychain prompts from every startup to only when: +- First authenticated request arrives +- User explicitly accesses credentials in settings +- Credentials are changed/updated \ No newline at end of file diff --git a/VibeTunnel.xcodeproj/project.xcworkspace/xcuserdata/steipete.xcuserdatad/UserInterfaceState.xcuserstate b/VibeTunnel.xcodeproj/project.xcworkspace/xcuserdata/steipete.xcuserdatad/UserInterfaceState.xcuserstate index 5b2a4fa8418422a4779e1c9b636b13f9763d8eb4..334d44ed394d6c29c5c525755f04aaec335fb043 100644 GIT binary patch delta 1343 zcmX|Ae@v8R9Dl#x_kO+i-T{R>CU#5sy$Bcif9inKkC!B_p~7S z#oUzNr)T%RXX3&d{z0Pen3zuF1tM`dNjby-#MO3d5qLdd!AnR{OOEJZ!WEjv~di38cn78z`;~cwE-D8+s_O zhw*P8t%I{vD2I=PIU8V*55V>!XOnwijPKsIm$Oa<^2nIWcg9W~fa^S=?G$H2EwD%h zy{n9+R#%t3>}RL-dD zZfbZRzjKNl;|&W~%Wn|i^OucNoBspqDSI(LT}7TwWjgpb1>oKOA~jy@<=swGx6E)N zX=U~WxIkCYZ`^K2j^+g{=yzZR{f_*`)*O!*8f!ISaT>_sD=XJ)_Do!73#_l(vLzI% zW*al{L)`a~kPto>MujoqtZ+`45WWz;6)p*L!ZqPGNRR6f?A#3%yXtPP!1m)o1%^jpB$@me*Eq zek*VDn|Ui9SY5J`eV2u6)0{y;D6J$^5o+1}MOeGn+#|s8DHiKOu}B&b&^1Q*TI)N9 z?$@r->umwEfFKeq1c@LMoP=~j2_ZmeB78y^Ae?_zspqnTsptRy HF>CfeoIR%B delta 1357 zcmX|Ae@v8R9Dlyw_kO+adk1s0b8^SsdE^zWNaByuE$5oyoV45m7D^{fPO#|Ef`yX7 zyRzgAT%P)-2T_MBCI-wmcoXBoGYMtGxi)g)9IU}Y%~eZh;Ii-Y;=Jv%_x*gH&-?v; zp6~PAz+qWCEcZ2Jo7RnWcWI5;6RrSO39U^d3V52H-cr3I9NG}B2-Sz)30Bl>t9vUH zt}O_KD{HH?hWrA6R&8|s>OyNo!CnR2n%Fida56?D+SlvSW;)shOGL`yi~BqKO=3if z)4L)_(VSC$t*WaT$uD>o{eCUI`=E&nYxoC!y*)xIk=KaCN<+hdxdMX9RY z#ZBq@Ac_%tCA}-cexvGxGmAyKNTU>fO;1G85^)5$f77|QV`3zI757_<)zl!k^x0CF z8;&Z@`csM%-q@F{|DPlXyN1r;y^> z7^!i0F~zk3kEdJ^iM7N zwaYxq$=o0qH%QvV*~tWGH0Q?~cp<4;ZA;QgC`oAN;x$8TR$m_>@x>9Cx6GCuNIuR;+Yt?Ygz?VE&Bk!B+ok`#ZUgJvTa z8M86XK$&6QNh|uBzr!Ab9pH?*?xKcI@@uEa9lT+t-h3NM_z}p)!Oi^*^^`q}AFv`% zE@t}VE(PG-?vNVK^73xys9RdJFJaZCd6=Nvm~C8edxq)-%VFf_fz zgjp$Ii{5 zVHvo<4Ic2pDp(7f;a%7b??V%O3: RouterMiddleware { + private let realm: String + private let logger = Logger(subsystem: "sh.vibetunnel.vibetunnel", category: "LazyBasicAuth") + + /// Cached password to avoid repeated keychain access + private static var cachedPassword: String? + + init(realm: String = "VibeTunnel Dashboard") { + self.realm = realm + } + + func handle( + _ request: Request, + context: Context, + next: (Request, Context) async throws -> Response + ) + async throws -> Response + { + // Skip auth for health check endpoint + if request.uri.path == "/api/health" { + return try await next(request, context) + } + + // Check if password protection is enabled + guard UserDefaults.standard.bool(forKey: "dashboardPasswordEnabled") else { + // No password protection, allow request + return try await next(request, context) + } + + // Extract authorization header + guard let authHeader = request.headers[.authorization], + authHeader.hasPrefix("Basic ") + else { + return unauthorizedResponse() + } + + // Decode base64 credentials + let base64Credentials = String(authHeader.dropFirst(6)) + guard let credentialsData = Data(base64Encoded: base64Credentials), + let credentials = String(data: credentialsData, encoding: .utf8) + else { + return unauthorizedResponse() + } + + // Split username:password + let parts = credentials.split(separator: ":", maxSplits: 1) + guard parts.count == 2 else { + return unauthorizedResponse() + } + + // We ignore the username and only check password + let providedPassword = String(parts[1]) + + // Get password (cached or from keychain) + let requiredPassword: String + if let cached = Self.cachedPassword { + requiredPassword = cached + logger.debug("Using cached password") + } else { + // First authentication attempt - access keychain + guard let password = await MainActor.run(body: { + DashboardKeychain.shared.getPassword() + }) else { + logger.error("Password protection enabled but no password found in keychain") + return unauthorizedResponse() + } + Self.cachedPassword = password + requiredPassword = password + logger.info("Password loaded from keychain and cached") + } + + // Verify password + guard providedPassword == requiredPassword else { + return unauthorizedResponse() + } + + // Password correct, continue with request + return try await next(request, context) + } + + private func unauthorizedResponse() -> Response { + var headers = HTTPFields() + headers[.wwwAuthenticate] = "Basic realm=\"\(realm)\"" + + let message = "Authentication required" + var buffer = ByteBuffer() + buffer.writeString(message) + + return Response( + status: .unauthorized, + headers: headers, + body: ResponseBody(byteBuffer: buffer) + ) + } + + /// Clears the cached password (useful when password is changed) + static func clearCache() { + cachedPassword = nil + } +} \ No newline at end of file diff --git a/VibeTunnel/Core/Services/ServerMonitor.swift b/VibeTunnel/Core/Services/ServerMonitor.swift index ed97db81..2aff3368 100644 --- a/VibeTunnel/Core/Services/ServerMonitor.swift +++ b/VibeTunnel/Core/Services/ServerMonitor.swift @@ -1,8 +1,11 @@ import Foundation import Observation -/// Monitors the HTTP server status and provides observable state for the UI +/// Monitors the HTTP server status and provides observable state for the UI. +/// /// This class now acts as a facade over ServerManager for backward compatibility +/// while providing a simplified interface for UI components to observe server state. +/// It bridges the gap between the older server architecture and the new ServerManager. @MainActor @Observable public final class ServerMonitor { diff --git a/VibeTunnel/Core/Services/TunnelClient.swift b/VibeTunnel/Core/Services/TunnelClient.swift index 51536995..c7102fcc 100644 --- a/VibeTunnel/Core/Services/TunnelClient.swift +++ b/VibeTunnel/Core/Services/TunnelClient.swift @@ -2,7 +2,10 @@ import Foundation import HTTPTypes import Logging -/// WebSocket message types for terminal communication +/// WebSocket message types for terminal communication. +/// +/// Defines the different types of messages that can be exchanged +/// between the client and server over WebSocket connections. public enum WSMessageType: String, Codable { case connect case command @@ -13,7 +16,10 @@ public enum WSMessageType: String, Codable { case close } -/// WebSocket message structure +/// WebSocket message structure. +/// +/// Encapsulates data sent over WebSocket connections for terminal +/// communication, including message type, session information, and payload. public struct WSMessage: Codable { public let type: WSMessageType public let sessionId: String? @@ -28,7 +34,11 @@ public struct WSMessage: Codable { } } -/// Client SDK for interacting with the VibeTunnel server +/// Client SDK for interacting with the VibeTunnel server. +/// +/// Provides a high-level interface for creating and managing terminal sessions +/// through the VibeTunnel HTTP API. Handles authentication, request/response +/// serialization, and error handling for all server operations. public class TunnelClient { private let baseURL: URL private let apiKey: String @@ -220,7 +230,11 @@ public class TunnelClient { } } -/// WebSocket client for real-time terminal communication +/// WebSocket client for real-time terminal communication. +/// +/// Provides WebSocket connectivity for streaming terminal I/O and +/// receiving real-time updates from terminal sessions. Handles +/// authentication, message encoding/decoding, and connection lifecycle. public final class TunnelWebSocketClient: NSObject, @unchecked Sendable { private let url: URL private let apiKey: String @@ -340,6 +354,10 @@ extension TunnelWebSocketClient: URLSessionWebSocketDelegate { // MARK: - Errors +/// Errors that can occur when using the TunnelClient. +/// +/// Represents various failure modes including network errors, +/// server errors, and data decoding issues. public enum TunnelClientError: LocalizedError, Equatable { case invalidResponse case httpError(statusCode: Int) diff --git a/VibeTunnel/Presentation/Views/AboutView.swift b/VibeTunnel/Presentation/Views/AboutView.swift index 437de3d9..b8abfb28 100644 --- a/VibeTunnel/Presentation/Views/AboutView.swift +++ b/VibeTunnel/Presentation/Views/AboutView.swift @@ -1,6 +1,37 @@ import AppKit import SwiftUI +// MARK: - Credit Link Component + +/// Credit link component for individual contributors. +/// +/// This component displays a contributor's handle as a clickable link +/// that opens their website when clicked. +struct CreditLink: View { + let name: String + let url: String + @State private var isHovering = false + + var body: some View { + Button(action: { + if let linkURL = URL(string: url) { + NSWorkspace.shared.open(linkURL) + } + }, label: { + Text(name) + .font(.caption) + .underline(isHovering, color: .accentColor) + }) + .buttonStyle(.link) + .pointingHandCursor() + .onHover { hovering in + withAnimation(.easeInOut(duration: 0.2)) { + isHovering = hovering + } + } + } +} + /// About view displaying application information, version details, and credits. /// /// This view provides information about VibeTunnel including version numbers, diff --git a/VibeTunnel/Utilities/ApplicationMover.swift b/VibeTunnel/Utilities/ApplicationMover.swift index 29234d14..2c00334e 100644 --- a/VibeTunnel/Utilities/ApplicationMover.swift +++ b/VibeTunnel/Utilities/ApplicationMover.swift @@ -321,7 +321,7 @@ final class ApplicationMover { Task { @MainActor in do { let configuration = NSWorkspace.OpenConfiguration() - _ = try await workspace.openApplication(at: appURL, configuration: configuration) + try await workspace.openApplication(at: appURL, configuration: configuration) logger.info("Launched app from Applications, quitting current instance") // Quit current instance after a short delay to ensure the new one starts diff --git a/VibeTunnel/Utilities/SettingsOpener.swift b/VibeTunnel/Utilities/SettingsOpener.swift index ece32151..c1ff2757 100644 --- a/VibeTunnel/Utilities/SettingsOpener.swift +++ b/VibeTunnel/Utilities/SettingsOpener.swift @@ -2,7 +2,11 @@ import AppKit import Foundation import SwiftUI -/// Helper to open the Settings window programmatically when SettingsLink cannot be used +/// Helper to open the Settings window programmatically when SettingsLink cannot be used. +/// +/// Provides workarounds for opening the Settings window in menu bar apps where +/// SwiftUI's SettingsLink may not function correctly. Uses multiple strategies +/// including menu item triggering and window manipulation to ensure reliable behavior. @MainActor enum SettingsOpener { /// SwiftUI's hardcoded settings window identifier @@ -140,8 +144,11 @@ enum SettingsOpener { // MARK: - Hidden Window View -/// A hidden window view that enables Settings to work in MenuBarExtra-only apps -/// This is a workaround for FB10184971 +/// A hidden window view that enables Settings to work in MenuBarExtra-only apps. +/// +/// This is a workaround for FB10184971 where SettingsLink doesn't function +/// properly in menu bar apps. Creates an invisible window that can receive +/// the openSettings environment action. struct HiddenWindowView: View { @Environment(\.openSettings) private var openSettings