Merge branch 'feature/multi-key-support' into feature/multi-repo

passKit/Models/PasswordStore.swift

@@ -411,8 +411,7 @@ public class PasswordStore {
                 return [keyID]
             }
             if Defaults.isEnableGPGIDOn {
-                let encryptedDataPath = password.fileURL(in: storeURL)
+                return findGPGIDs(underPath: password.path)
-                return findGPGIDs(from: encryptedDataPath)
             }
             return []
         }()
@@ -430,6 +429,37 @@ public class PasswordStore {
         AppKeychain.shared.removeContent(for: SSHKey.PRIVATE.getKeychainKey())
         gitSSHPrivateKeyPassphrase = nil
     }
+
+    func findGPGIDs(underPath relativePath: String) -> [String] {
+        guard let gpgIDFileURL = findGPGIDFile(atPath: relativePath) else {
+            return []
+        }
+        let allKeysSeparatedByNewline = (try? String(contentsOf: gpgIDFileURL)) ?? ""
+        return allKeysSeparatedByNewline
+            .split(separator: "\n")
+            .map { String($0).trimmed }
+            .filter { !$0.isEmpty }
+    }
+
+    func findGPGIDFile(atPath relativePath: String) -> URL? {
+        // Walk up the directory hierarchy, but never escape the store root
+        let storeRoot = storeURL.absoluteURL.resolvingSymlinksInPath()
+        var current = storeRoot.appendingPathComponent(relativePath).resolvingSymlinksInPath()
+        
+        while current.path.hasPrefix(storeRoot.path) {
+            let candidate = current.appendingPathComponent(".gpg-id")
+            var isDir: ObjCBool = false
+            if FileManager.default.fileExists(atPath: candidate.path, isDirectory: &isDir), !isDir.boolValue {
+                return candidate.standardizedFileURL
+            }
+            let parent = current.deletingLastPathComponent().resolvingSymlinksInPath()
+            if parent.path == current.path {
+                break
+            }
+            current = parent
+        }
+        return nil
+    }
 }
 
 extension PasswordStore {
@@ -462,18 +492,3 @@ extension PasswordStore {
         return try gitRepository.commit(signature: gitSignatureForNow, message: message)
     }
 }
-
-func findGPGIDs(from url: URL) -> [String] {
-    var path = url
-    while !FileManager.default.fileExists(atPath: path.appendingPathComponent(".gpg-id").path),
-          path.path != "file:///" {
-        path = path.deletingLastPathComponent()
-    }
-    path = path.appendingPathComponent(".gpg-id")
-
-    let allKeysSeparatedByNewline = (try? String(contentsOf: path)) ?? ""
-    return allKeysSeparatedByNewline
-        .split(separator: "\n")
-        .map { String($0).trimmed }
-        .filter { !$0.isEmpty }
-}

passKitTests/Models/PasswordStoreTest.swift

@@ -332,12 +332,85 @@ final class PasswordStoreTest: XCTestCase {
         waitForExpectations(timeout: 1, handler: nil)
     }
 
-    // MARK: - .gpg-id support
+    // MARK: - Find .gpg-id
+
+    func testFindGPGIDFile() throws {
+        try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true)
+        XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data("under root".utf8)))
+
+        try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("foo/bar/baz"), withIntermediateDirectories: true)
+        XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path, contents: Data("under foo".utf8)))
+
+        try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/"), withIntermediateDirectories: true)
+        try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey"), withIntermediateDirectories: true)
+        XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey/.gpg-id").path, contents: Data("under hey".utf8)))
+
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "/")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "doesnt-exist")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/..")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
+
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path))
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path))
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar/baz")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path))
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/doesnt-exist")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path))
+
+        // there is a _drectory_ called .gpg-id in here
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir/.gpg-id")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir/.gpg-id/hey")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey/.gpg-id").path))
+
+        // "foo/bar/../../baz" resolves to "baz" which has no .gpg-id, so should find root's.
+        // Without path resolution, the walk ["foo","bar","..","..","baz"] → remove "baz" → remove ".." →
+        // "foo/bar/.." → remove ".." → "foo/bar" → finds foo/.gpg-id (wrong).
+        try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("baz"), withIntermediateDirectories: true)
+        XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar/../../baz")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
+    }
+
+    func testMissingGPGIDFile() throws {
+        XCTAssertFalse(FileManager.default.fileExists(atPath: localRepoURL.appendingPathComponent(".gpg-id").path))
+        try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("subdir"), withIntermediateDirectories: true)
+
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: ""))
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "subdir"))
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "missing"))
+    }
+
+    func testFindGPGIDFileStopsAtRoot() throws {
+        // Place a .gpg-id file ABOVE the store root, this should not be found
+        let parentDir = localRepoURL.deletingLastPathComponent()
+        let escapedGPGIDURL = parentDir.appendingPathComponent(".gpg-id")
+        XCTAssertTrue(FileManager.default.createFile(atPath: escapedGPGIDURL.path, contents: Data("ESCAPED_KEY".utf8)))
+        defer { try? FileManager.default.removeItem(at: escapedGPGIDURL) }
+
+        // Store has no .gpg-id at all
+        try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("sub/deep"), withIntermediateDirectories: true)
+        // Direct paths, should not find the escaped .gpg-id since it's outside the store root
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: ""))
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub"))
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep"))
+
+        // Path traversal attempts via ".."
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: ".."))
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "../.."))
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/../.."))
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep/../../.."))
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep/../../../../../etc"))
+
+        // Symlink escape: create a symlink inside the store pointing outside
+        let evilDir = parentDir.appendingPathComponent("evil")
+        try FileManager.default.createDirectory(at: evilDir, withIntermediateDirectories: true)
+        XCTAssertTrue(FileManager.default.createFile(atPath: evilDir.appendingPathComponent(".gpg-id").path, contents: Data("EVIL_KEY".utf8)))
+        defer { try? FileManager.default.removeItem(at: evilDir) }
+        try FileManager.default.createSymbolicLink(at: localRepoURL.appendingPathComponent("sub/escape"), withDestinationURL: evilDir)
+        // Following the symlink would find evil/.gpg-id — must not happen
+        XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/escape"))
+    }
+
+    // MARK: Parse .gpg-id
 
     func testReadGPGIDFile() throws {
         try cloneRepository(.withGPGID)
-        Defaults.isEnableGPGIDOn = true
-
         [
             ("", [RSA4096.longFingerprint]),
             ("family", [String(NISTP384.longFingerprint.suffix(16))]),
@@ -345,11 +418,36 @@ final class PasswordStoreTest: XCTestCase {
             ("shared", [RSA2048.longFingerprint, RSA4096.longFingerprint]),
             ("work", [RSA2048.longFingerprint]),
         ].forEach { path, expectedKeyIDs in
-            let foundKeyIDs = findGPGIDs(from: localRepoURL.appendingPathComponent(path))
+            let foundKeyIDs = passwordStore.findGPGIDs(underPath: path)
-            XCTAssertEqual(foundKeyIDs, expectedKeyIDs.map { $0.uppercased() })
+            XCTAssertEqual(foundKeyIDs, expectedKeyIDs.map { $0.uppercased() }, "Failed for path: \(path)")
         }
     }
 
+    func testReadEmptyGPGIDFile() throws {
+        try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true)
+
+        XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: nil))
+        XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), [])
+
+        XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data("  \n\t".utf8)))
+        XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), [])
+    }
+
+    func testReadGPGIDFileWithWhitespace() throws {
+        try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true)
+
+        XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: nil))
+        XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), [])
+
+        XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data("  \n\t".utf8)))
+        XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), [])
+
+        XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data("  \nbar  foo\n\tbaz\n  \n".utf8)))
+        XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), ["bar  foo", "baz"])
+    }
+
+    // MARK: Handle .gpg-id
+
     func testAddPasswordInRoot_WithSingleEntryInPGPIDFile_EncryptsWithThatKey() throws {
         let mockPGPInterface = setUpMockedPGPInterface()
         mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints)
@@ -436,6 +534,48 @@ final class PasswordStoreTest: XCTestCase {
         }
     }
 
+    func testGPGIDDisabledIgnoresGPGIDFile() throws {
+        try cloneRepository(.withGPGID)
+        try importSinglePGPKey() // Only import RSA4096, but not RSA2048
+        Defaults.isEnableGPGIDOn = false
+
+        // /work uses RSA2048, but we didn't import that one
+        let testPassword = Password(name: "test", path: "work/test.gpg", plainText: "testpassword")
+        // this would throw if isEnableGPGIDOn was true, since we are missing the key to encrypt it
+        _ = try passwordStore.add(password: testPassword)
+
+        // check that we can decrypt it with the key we have, which confirms that it was encrypted without using the .gpg-id file
+        let decryptedPassword = try decrypt(path: "work/test.gpg", keyID: RSA4096.longFingerprint)
+        XCTAssertEqual(decryptedPassword.plainText, "testpassword")
+
+        // we can't even decrypt it with RSA2048
+        try importMultiplePGPKeys()
+        XCTAssertThrowsError(try decrypt(path: "work/test.gpg", keyID: RSA2048.longFingerprint)) {
+            XCTAssertEqual($0 as? AppError, .keyExpiredOrIncompatible)
+        }
+    }
+
+    func testEncryptWithExplicitKeyID_OverridesGPGIDFile() throws {
+        continueAfterFailure = false // avoid index out of bounds error below
+
+        let mockPGPInterface = setUpMockedPGPInterface()
+        mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints)
+        try cloneRepository(.withGPGID)
+        Defaults.isEnableGPGIDOn = true
+
+        // Even though /personal would normally use RSA4096 from the root .gpg-id file, if we explicitly specify a key ID then that should be used instead
+        let testPassword1 = Password(name: "test1", path: "personal/test1.gpg", plainText: "testpassword1")
+        _ = try passwordStore.add(password: testPassword1)
+        let testPassword2 = Password(name: "test2", path: "personal/test2.gpg", plainText: "testpassword2")
+        _ = try passwordStore.add(password: testPassword2, keyID: RSA2048.longFingerprint)
+
+        XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls.count, 2)
+        XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[0].plainData, testPassword1.plainData)
+        XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[0].keyIDs, [RSA4096.longFingerprint].map { $0.uppercased() })
+        XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[1].plainData, testPassword2.plainData)
+        XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[1].keyIDs, [RSA2048.longFingerprint])
+    }
+
     // MARK: - Helpers
 
     private enum RemoteRepo {

plans/02-multi-recipient-encryption-plan.md

@@ -30,7 +30,7 @@ The codebase does **not** support encrypting to multiple public keys. Every laye
 
 ### 1. `findGPGID(from:) -> [String]`
 
-Split file contents by newline, trim each line, filter empty lines. Return array of key IDs. Callers that only need a single key (e.g. for decryption routing) can use `.first`.
+Split file contents by newline, trim each line, filter empty lines. Return array of key IDs.
 
 ### 2. `PGPInterface` protocol
 
@@ -60,10 +60,10 @@ When a store lists multiple key IDs in `.gpg-id`, the user needs the public keys
 
 ### Current state
 
-- The keychain holds exactly **one** `pgpPublicKey` blob and **one** `pgpPrivateKey` blob.
+- The keychain holds **one** `pgpPublicKey` blob and **one** `pgpPrivateKey` blob, but each can contain **multiple concatenated armored key blocks**. Both interface implementations parse all keys from these blobs.
-- The import UI (armor paste, URL, file picker) has one public key field + one private key field. Importing **replaces** the previous key pair entirely.
+- The import UI (armor paste, URL, file picker) has one public key field + one private key field. Importing **replaces** the set of keys entirely, there is no append mode for adding additional keys or managing existing keys.
-- Both `GopenPGPInterface` and `ObjectivePGPInterface` *can* parse multiple keys from a single armored blob (e.g. concatenated armor blocks). So if the user pastes multiple public keys into the single field, they would be parsed — but the encrypt path only uses one key, and the UI doesn't communicate this.
+- There is no UI for viewing which key IDs are loaded or for importing additional recipient-only public keys, nor for viewing the key metadata.
-- There is no UI for viewing which key IDs are loaded.
+- There is no UI for viewing or editing `.gpg-id` files, which are the source of truth for which keys are used for encryption.
 
 ### Key storage approach
 
@@ -142,20 +142,19 @@ This can be expensive for large directories. Show progress and allow cancellatio
 
 ## Implementation Order
 
-| Step | Description | Depends On |
+| Step | Description | Status | Depends On |
-|------|-------------|------------|
+|------|-------------|--------|------------|
-| 1 | `findGPGID` returns `[String]` + update callers | — |
+| 1 | `findGPGIDs` returns `[String]` + update callers | ✅ Done | — |
-| 2 | `PGPInterface` protocol change (`keyIDs: [String]?`) | — |
+| 2 | `PGPInterface` protocol change (`keyIDs: [String]`) | ✅ Done | — |
-| 3 | `GopenPGPInterface` multi-key encryption | Step 2 |
+| 3 | `GopenPGPInterface` multi-key encryption | ✅ Done | Step 2 |
-| 4 | `ObjectivePGPInterface` multi-key encryption | Step 2 |
+| 4 | `ObjectivePGPInterface` multi-key encryption | ✅ Done | Step 2 |
-| 5 | `PGPAgent` updated overloads | Steps 2-4 |
+| 5 | `PGPAgent` updated overloads | ✅ Done | Steps 2-4 |
-| 6 | `PasswordStore.encrypt()` uses `[String]` from `findGPGID` | Steps 1+5 |
+| 6 | `PasswordStore.encrypt()` uses `[String]` from `findGPGIDs` | ✅ Done | Steps 1+5 |
-| 7 | UI: import additional recipient public keys | Step 5 |
+| 7 | UI: import additional recipient public keys | Not started | Step 5 |
-| 8 | UI: view loaded key IDs | Step 5 |
+| 8 | UI: view loaded key IDs and metadata | Not started | Step 5 |
-| 9a | UI: view `.gpg-id` in password detail / folder view | Step 1 |
+| 9a | UI: view `.gpg-id` in password detail / folder view | Not started | Step 1 |
-| 9b | UI: edit `.gpg-id` | Step 9a |
+| 9b | UI: edit `.gpg-id` | Not started | Step 9a |
-| 10 | Re-encryption when `.gpg-id` changes | Steps 6+9b |
+| 10 | Re-encryption when `.gpg-id` changes | Not started | Steps 6+9b |
-| T | Tests (see testing section) | Steps 1-10 |
 
 ---