-
-
Notifications
You must be signed in to change notification settings - Fork 285
feat(connections): resolve connection passwords from file, env, or command (#1254) #1478
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
7a1e6c6
2013836
f43176c
eb2de91
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 |
|---|---|---|
|
|
@@ -479,6 +479,9 @@ enum DatabaseDriverFactory { | |
| return try await resolveIAMPassword(for: connection, fields: fields) | ||
| } | ||
| if let override { return override } | ||
| if let passwordSource = connection.passwordSource { | ||
| return try await PasswordSourceResolver.resolve(passwordSource) | ||
|
Comment on lines
+482
to
+483
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.
This resolves Useful? React with 👍 / 👎. |
||
| } | ||
| if connection.usePgpass { | ||
| let pgpassHost = connection.additionalFields["pgpassOriginalHost"] ?? connection.host | ||
| let pgpassPort = connection.additionalFields["pgpassOriginalPort"] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,6 +113,8 @@ struct SyncRecordMapper { | |
| // the sync schema in the future, apply path contraction to its snapshot. | ||
| // cloudflareTunnelMode is also NOT synced: it is device-local runtime | ||
| // config and its service-token secrets live in the Keychain. | ||
| // passwordSource is also NOT synced: its file path, env var, or command | ||
| // is device-local and may not exist or resolve on another Mac. | ||
|
Comment on lines
+116
to
+117
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.
When a Mac already has a connection with a local Useful? React with 👍 / 👎. |
||
| do { | ||
| let sshData = try encoder.encode(Self.makePortable(connection.sshConfig)) | ||
| record["sshConfigJson"] = sshData as CKRecordValue | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,237 @@ | ||
| // | ||
| // PasswordSourceResolver.swift | ||
| // TablePro | ||
| // | ||
|
|
||
| import Foundation | ||
| import os | ||
|
|
||
| /// Resolves a connection password from an external source declared in connections.json. | ||
| /// File and command sources require a non-sandboxed build; TablePro ships with the hardened | ||
| /// runtime and no App Sandbox, so spawning a process and reading arbitrary files is allowed. | ||
| enum PasswordSourceResolver { | ||
| private static let logger = Logger(subsystem: "com.TablePro", category: "PasswordSourceResolver") | ||
|
|
||
| private static let commandTimeoutSeconds: UInt64 = 30 | ||
| private static let maxOutputBytes = 1_048_576 | ||
|
|
||
| enum ResolutionError: LocalizedError { | ||
| case fileNotFound(path: String) | ||
| case fileUnreadable(path: String) | ||
| case environmentVariableNotSet(name: String) | ||
| case commandFailed(exitCode: Int32, stderr: String) | ||
| case commandTimedOut | ||
| case outputTooLarge | ||
| case emptyPassword | ||
|
|
||
| var errorDescription: String? { | ||
| switch self { | ||
| case let .fileNotFound(path): | ||
| return String(format: String(localized: "Password file not found: %@"), path) | ||
| case let .fileUnreadable(path): | ||
| return String(format: String(localized: "Could not read password file: %@"), path) | ||
| case let .environmentVariableNotSet(name): | ||
| return String( | ||
| format: String(localized: """ | ||
| Environment variable %@ is not set in TablePro's environment. \ | ||
| Apps launched from the Dock do not inherit shell exports. Launch TablePro \ | ||
| from a terminal, or set the variable with launchctl setenv. | ||
| """), | ||
| name | ||
| ) | ||
| case let .commandFailed(exitCode, stderr): | ||
| let message = stderr.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| if message.isEmpty { | ||
| return String(format: String(localized: "Password command failed with exit code %d"), exitCode) | ||
| } | ||
| return String(format: String(localized: "Password command failed (exit %d): %@"), exitCode, message) | ||
| case .commandTimedOut: | ||
| return String(localized: "Password command timed out after 30 seconds") | ||
| case .outputTooLarge: | ||
| return String(localized: "Password command produced too much output") | ||
| case .emptyPassword: | ||
| return String(localized: "The password source produced an empty password") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| static func resolve(_ source: PasswordSource) async throws -> String { | ||
| switch source { | ||
| case let .file(path): | ||
| return try resolveFile(path: path) | ||
| case let .env(variable): | ||
| return try resolveEnvironment(variable: variable) | ||
| case let .command(shell): | ||
| return try await resolveCommand(shell: shell, timeoutSeconds: commandTimeoutSeconds) | ||
| } | ||
| } | ||
|
|
||
| private static func resolveFile(path: String) throws -> String { | ||
| let expandedPath = (path as NSString).expandingTildeInPath | ||
| guard FileManager.default.fileExists(atPath: expandedPath) else { | ||
| throw ResolutionError.fileNotFound(path: expandedPath) | ||
| } | ||
| warnIfPermissionsInsecure(path: expandedPath) | ||
| guard let contents = try? String(contentsOfFile: expandedPath, encoding: .utf8) else { | ||
| throw ResolutionError.fileUnreadable(path: expandedPath) | ||
| } | ||
| return try nonEmpty(contents.trimmingCharacters(in: .whitespacesAndNewlines)) | ||
|
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.
For file-based password sources, trimming Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| private static func resolveEnvironment(variable: String) throws -> String { | ||
| guard let value = ProcessInfo.processInfo.environment[variable] else { | ||
| throw ResolutionError.environmentVariableNotSet(name: variable) | ||
| } | ||
| return try nonEmpty(value.trimmingCharacters(in: .whitespacesAndNewlines)) | ||
| } | ||
|
|
||
| static func resolveCommand(shell: String, timeoutSeconds: UInt64) async throws -> String { | ||
| let output = try await Task.detached(priority: .userInitiated) { () throws -> String in | ||
| let process = Process() | ||
| process.executableURL = URL(fileURLWithPath: "/bin/bash") | ||
| process.arguments = ["-c", shell] | ||
| process.environment = augmentedEnvironment() | ||
| process.standardInput = FileHandle.nullDevice | ||
|
|
||
| let stdoutPipe = Pipe() | ||
| let stderrPipe = Pipe() | ||
| process.standardOutput = stdoutPipe | ||
| process.standardError = stderrPipe | ||
|
|
||
| let stdoutCollector = PipeDataCollector(maxBytes: maxOutputBytes) | ||
| let stderrCollector = PipeDataCollector(maxBytes: maxOutputBytes) | ||
| stdoutPipe.fileHandleForReading.readabilityHandler = { handle in | ||
| let chunk = handle.availableData | ||
| guard !chunk.isEmpty else { return } | ||
| stdoutCollector.append(chunk) | ||
| if stdoutCollector.overflowed, process.isRunning { | ||
| process.terminate() | ||
| } | ||
| } | ||
| stderrPipe.fileHandleForReading.readabilityHandler = { handle in | ||
| let chunk = handle.availableData | ||
| if !chunk.isEmpty { stderrCollector.append(chunk) } | ||
| } | ||
|
|
||
| try process.run() | ||
|
|
||
| let didTimeout = AtomicFlag() | ||
| let timeoutTask = Task.detached { | ||
| try await Task.sleep(nanoseconds: timeoutSeconds * 1_000_000_000) | ||
| if process.isRunning { | ||
| didTimeout.set() | ||
| process.terminate() | ||
| } | ||
|
Comment on lines
+121
to
+124
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.
For a password command that traps or ignores SIGTERM, this timeout path sets the flag and calls Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| process.waitUntilExit() | ||
| timeoutTask.cancel() | ||
|
|
||
| stdoutPipe.fileHandleForReading.readabilityHandler = nil | ||
| stderrPipe.fileHandleForReading.readabilityHandler = nil | ||
|
|
||
| if stdoutCollector.overflowed { | ||
| throw ResolutionError.outputTooLarge | ||
| } | ||
| if didTimeout.isSet { | ||
| throw ResolutionError.commandTimedOut | ||
| } | ||
| if process.terminationStatus != 0 { | ||
| throw ResolutionError.commandFailed( | ||
| exitCode: process.terminationStatus, | ||
| stderr: stderrCollector.string | ||
| ) | ||
| } | ||
| return stdoutCollector.string | ||
| }.value | ||
|
|
||
| guard !output.contains("\0") else { | ||
| throw ResolutionError.emptyPassword | ||
| } | ||
| return try nonEmpty(output.trimmingCharacters(in: .whitespacesAndNewlines)) | ||
| } | ||
|
|
||
| private static func augmentedEnvironment() -> [String: String] { | ||
| var environment = ProcessInfo.processInfo.environment | ||
| let toolPaths = ["/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin", "/usr/sbin", "/sbin"] | ||
| var pathComponents = (environment["PATH"] ?? "").split(separator: ":").map(String.init) | ||
| for toolPath in toolPaths where !pathComponents.contains(toolPath) { | ||
| pathComponents.append(toolPath) | ||
| } | ||
| environment["PATH"] = pathComponents.joined(separator: ":") | ||
| return environment | ||
| } | ||
|
|
||
| private static func warnIfPermissionsInsecure(path: String) { | ||
| guard let attributes = try? FileManager.default.attributesOfItem(atPath: path), | ||
| let permissions = attributes[.posixPermissions] as? Int else { | ||
| return | ||
| } | ||
| if permissions & 0o077 != 0 { | ||
| logger.warning("Password file is group or world accessible; restrict it with chmod 600") | ||
| } | ||
| } | ||
|
|
||
| private static func nonEmpty(_ password: String) throws -> String { | ||
| guard !password.isEmpty else { | ||
| throw ResolutionError.emptyPassword | ||
| } | ||
| return password | ||
| } | ||
| } | ||
|
|
||
| private final class PipeDataCollector: @unchecked Sendable { | ||
| private let lock = NSLock() | ||
| private let maxBytes: Int | ||
| private var data = Data() | ||
| private var didOverflow = false | ||
|
|
||
| init(maxBytes: Int) { | ||
| self.maxBytes = maxBytes | ||
| } | ||
|
|
||
| func append(_ chunk: Data) { | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
| let remaining = maxBytes - data.count | ||
| guard remaining > 0 else { | ||
| didOverflow = true | ||
| return | ||
| } | ||
| if chunk.count > remaining { | ||
| data.append(chunk.prefix(remaining)) | ||
| didOverflow = true | ||
| } else { | ||
| data.append(chunk) | ||
| } | ||
| } | ||
|
|
||
| var overflowed: Bool { | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
| return didOverflow | ||
| } | ||
|
|
||
| var string: String { | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
| return String(data: data, encoding: .utf8) ?? "" | ||
| } | ||
| } | ||
|
|
||
| private final class AtomicFlag: @unchecked Sendable { | ||
| private let lock = NSLock() | ||
| private var value = false | ||
|
|
||
| func set() { | ||
| lock.lock() | ||
| value = true | ||
| lock.unlock() | ||
| } | ||
|
|
||
| var isSet: Bool { | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
| return value | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -334,6 +334,7 @@ struct DatabaseConnection: Identifiable, Hashable { | |
| var localOnly: Bool = false | ||
| var isSample: Bool = false | ||
| var isFavorite: Bool = false | ||
| var passwordSource: PasswordSource? | ||
|
|
||
| var mongoAuthSource: String? { | ||
| get { additionalFields["mongoAuthSource"]?.nilIfEmpty } | ||
|
|
@@ -430,6 +431,7 @@ struct DatabaseConnection: Identifiable, Hashable { | |
| localOnly: Bool = false, | ||
| isSample: Bool = false, | ||
| isFavorite: Bool = false, | ||
| passwordSource: PasswordSource? = nil, | ||
|
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.
Because this new initializer argument defaults to Useful? React with 👍 / 👎. |
||
| additionalFields: [String: String]? = nil | ||
| ) { | ||
| self.id = id | ||
|
|
@@ -472,6 +474,7 @@ struct DatabaseConnection: Identifiable, Hashable { | |
| self.localOnly = localOnly | ||
| self.isSample = isSample | ||
| self.isFavorite = isFavorite | ||
| self.passwordSource = passwordSource | ||
| if let additionalFields { | ||
| self.additionalFields = additionalFields | ||
| } else { | ||
|
|
@@ -520,6 +523,7 @@ extension DatabaseConnection: Codable { | |
| case sshConfig, sslConfig, color, tagId, groupId, sshProfileId | ||
| case sshTunnelMode, cloudflareTunnelMode, safeModeLevel, aiPolicy, aiRules, aiAlwaysAllowedTools, externalAccess, additionalFields | ||
| case redisDatabase, startupCommands, sortOrder, localOnly, isSample, isFavorite | ||
| case passwordSource | ||
| } | ||
|
|
||
| init(from decoder: Decoder) throws { | ||
|
|
@@ -549,6 +553,7 @@ extension DatabaseConnection: Codable { | |
| localOnly = try container.decodeIfPresent(Bool.self, forKey: .localOnly) ?? false | ||
| isSample = try container.decodeIfPresent(Bool.self, forKey: .isSample) ?? false | ||
| isFavorite = try container.decodeIfPresent(Bool.self, forKey: .isFavorite) ?? false | ||
| passwordSource = PasswordSource.resilientlyDecoded(from: container, forKey: .passwordSource) | ||
| cloudflareTunnelMode = try container.decodeIfPresent(CloudflareTunnelMode.self, forKey: .cloudflareTunnelMode) ?? .disabled | ||
|
|
||
| // Migrate from legacy fields if sshTunnelMode is not present | ||
|
|
@@ -600,6 +605,7 @@ extension DatabaseConnection: Codable { | |
| try container.encode(localOnly, forKey: .localOnly) | ||
| try container.encode(isSample, forKey: .isSample) | ||
| try container.encode(isFavorite, forKey: .isFavorite) | ||
| try container.encodeIfPresent(passwordSource, forKey: .passwordSource) | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
When a connection uses SSH or Cloudflare tunneling,
DatabaseManager.tunneledConnection(from:localPort:)rebuilds aDatabaseConnectionfor127.0.0.1beforecreateDrivercalls this password lookup, but that rebuilt connection does not carrypasswordSource. In that environment this new branch is skipped and the driver falls back to Keychain/empty password, so a connection that works directly withpasswordSourcefails as soon as a tunnel is enabled; propagate the source onto the tunneled connection before resolving the password.Useful? React with 👍 / 👎.