@SecurelyStored with GitHub Copilot

@SecurelyStored with GitHub Copilot

A Swift property wrapper written by AI using TDD

I was interested how well can AI write code for my home project, where I needed to store any Codable securely using Keychain Services.

Based on my previous experience on the blog, I was wondering how can I provide specific context to Copilot to best match my needs. After reading the article by Kedasha Kerr that was sent as part of the https://www.pointer.io newsletter (which I recommend the reader to subscribe to #notsponsored) I came to the conclusion that I might as well try to respect AI as a fellow developer and I would pair program with it using TDD.

I installed and authorised these two extensions:

You can find the finished code on my GitHub Gist.

I started from a unit test which would allow to read and write data using SecKeychainItem.

final class SecurelyStoredTests: XCTestCase {

    /// In order to check if the Apple Keychain is accessible
    /// and we have read and write permissions on the platform where we run these tests,
    /// this test creates a String which is stored securely in the Keychain
    /// than the data is retreived and asserted.
    func testKeychainIsAccessible() {

        let accessor = KeychainAccessor()
        let exampleKey = "MyTestKey"
        let exampleData = "Some test data"
        accessor.store(value: exampleData, forKey: exampleKey)

        let value: String = accessor.retrieveValue(forKey: exampleKey)

        XCTAssertEqual(exampleData, value)
    }
}

Then I asked Copilot chat to create the implementation for the test.

Sure, here's a basic implementation of the KeychainAccessor class in Swift using SecKeychainItem. This class has two methods: storeData and retrieveData. The storeData method stores a Codable object in the keychain, and the retrieveData method retrieves it.

import Foundation
import Security

class KeychainAccessor {
    private let service: String

    init(service: String) {
        self.service = service
    }

    func storeData<T: Codable>(_ object: T, forKey key: String) -> OSStatus {
        let encoder = JSONEncoder()
        guard let encoded = try? encoder.encode(object) else {
            return errSecParam
        }

        let query: [String: Any] = [kSecClass as String: kSecClassGenericPassword,
                                    kSecAttrService as String: service,
                                    kSecAttrAccount as String: key,
                                    kSecValueData as String: encoded]

        SecItemDelete(query as CFDictionary)

        return SecItemAdd(query as CFDictionary, nil)
    }

    func retrieveData<T: Codable>(forKey key: String) -> T? {
        let query: [String: Any] = [kSecClass as String: kSecClassGenericPassword,
                                    kSecAttrService as String: service,
                                    kSecAttrAccount as String: key,
                                    kSecReturnData as String: kCFBooleanTrue!,
                                    kSecMatchLimit as String: kSecMatchLimitOne]

        var dataTypeRef: AnyObject?
        let status: OSStatus = SecItemCopyMatching(query as CFDictionary, &dataTypeRef)

        if status == noErr {
            let data = dataTypeRef as! Data
            let decoder = JSONDecoder()
            return try? decoder.decode(T.self, from: data)
        } else {
            return nil
        }
    }
}

This class uses the SecItemAdd and SecItemCopyMatching functions from the Security framework to store and retrieve data. The storeData method first deletes any existing items with the same key, then adds the new item. The retrieveData method retrieves the item with the specified key. If the item does not exist, it returns nil.

Not bad, lets see if it compiles and passes the tests.

I've renamed the functions in my test based on the freshly generated implementation and asked Copilot to make the class and functions public in the implementation.

In Xcode I've run into the error ⚠️ Result of call to 'storeData(_:forKey:)' is unused, great opportunity for Copilot to fix!

I asked Copilot "Can you assert the storeData result with OSStatus success?".

It recommended the following:

let storeResult = accessor.storeData(exampleData, forKey: exampleKey)
XCTAssertEqual(storeResult, OSStatus.success, "Failed to store data in Keychain")

Exactly, thank you. I pressed 'Accept' and tried to run it in Xcode (or you could use terminal in VS Code).

Oh wait 🔥 Type 'OSStatus' (aka 'Int32') has no member 'success'. Here we go again...

Me: It looks like OSStatus.success doesn't exist. Can you fix it by asserting the successful OSStatus using the correct Int32 static value?

Copilot: To fix the issue, I would replace OSStatus.success with the correct Int32 static value for a successful status in the assertion.

XCTAssertEqual(storeResult, Int32(0), "Failed to store data in Keychain")

Et voilà! The test passes ✅

It would be useful, if we could make the Keychain now mockable and write a new test. This is the test I wrote as a start in TDD style while the implementation doesn't exist yet. This would open up the API to be able to pass in the SecItem functions or anything else.

func testKeychainAccessorCanStoreElsewhere() {

    var values = [String: Any]()

    let accessor = KeychainAccessor(
        service: "TestService",
        add: { query, _ in
            let service = (query as Dictionary)[kSecAttrService] as! String
            let key = (query as Dictionary)[kSecAttrAccount] as! String
            let data = (query as Dictionary)[kSecValueData]
            values[service+key] = data
            return errSecSuccess
        },
        delete: { query in
            let service = (query as Dictionary)[kSecAttrService] as! String
            let key = (query as Dictionary)[kSecAttrAccount] as! String
            values[service+key] = nil
            return errSecSuccess
        },
        copy: { query, dataTypeRef in
            let service = (query as Dictionary)[kSecAttrService] as! String
            let key = (query as Dictionary)[kSecAttrAccount] as! String
            if let value = values[service+key] as? AnyObject {
                dataTypeRef?.pointee = value
                return errSecSuccess
            } else {
                return errSecItemNotFound
            }
        })

    let exampleKey = "MyTestKey"
    let exampleData = "Some test data"
    let storeResult = accessor.storeData(exampleData, forKey: exampleKey)
    XCTAssertEqual(storeResult, Int32(0), "Failed to store data in Keychain")

    let value: String? = accessor.retrieveData(forKey: exampleKey)
    XCTAssertEqual(exampleData, value)
}

This wouldn't compile of course as I would need to update the implementation, so I asked Copilot to do some refactoring.

Me: Extract SecItemAdd function call into a stored closure property called add that is passed in during init.

Accept

Extract SecItemDelete function call into a stored closure property called add that is passed in during init.

Accept

Extract SecItemCopyMatching into private property that is passed in through init and name it copy.

Accept

Create a new convenience initializer that is passing SecItemAdd, SecItemDelete and SecItemCopyMatching to add, delete and copy in the public initializer.

Accept

Now func testKeychainAccessorCanStoreElsewhere() ✅ passes!

I looked at the two tests we have and decided to improve on the first test, where I forgot to cleanup the stored data from the Keychain. It could cause issues when we're re-running tests and the Simulator's keychain is not clean.

To provide the best context for Copilot I wrote the function signature than tapped the function and asked Copilot to implement it.

Me: Using the delete property can you implement this function?

public func deleteData(forKey key: String) -> OSStatus {
    /// Copilot's recommendation is commented out
    /// let query: [String: Any] = [kSecClass as String: kSecClassGenericPassword,
    ///                             kSecAttrService as String: service,
    ///                             kSecAttrAccount as String: key]

    /// return delete(query as CFDictionary)
}

It did implement it with the code commented out above. Then I would use this new function in my test asking Copilot to implement it by moving the cursor to the last empty line in the function.

Can you delete the exampleData to cleanup the test?

let deleteResult = accessor.deleteData(forKey: exampleKey)
XCTAssertEqual(deleteResult, Int32(0), "Failed to delete data from Keychain")

Accept

After a quick Google search, I remembered we have this fantastic website for OSStatus codes, a potential gap in Copilot or maybe I haven't asked specifically enough for the correct status code I would like.

Can you replace all Int32(0) calls with errSecSuccess in this file?

Accept

Okay well, that went pretty fast let's jump to the end game? I'll create a quick unit test:

Using KeychainAccessor can you write a Swift property wrapper named SecurelyStored that can wrap Codable types, it needs a KeychainAccessor passed in as a dependency.

@propertyWrapper
struct SecurelyStored<T: Codable> {
    private let key: String
    private let keychainAccessor: KeychainAccessor

    init(key: String, keychainAccessor: KeychainAccessor) {
        self.key = key
        self.keychainAccessor = keychainAccessor
    }

    var wrappedValue: T? {
        get {
            guard let data = keychainAccessor.getData(forKey: key) else {
                return nil
            }

            do {
                let decoder = JSONDecoder()
                return try decoder.decode(T.self, from: data)
            } catch {
                print("Failed to decode data from Keychain: \(error)")
                return nil
            }
        }
        set {
            guard let value = newValue else {
                keychainAccessor.removeData(forKey: key)
                return
            }

            do {
                let encoder = JSONEncoder()
                let data = try encoder.encode(value)
                keychainAccessor.setData(data, forKey: key)
            } catch {
                print("Failed to encode data for Keychain: \(error)")
            }
        }
    }
}

Let's count the compiler errors shall we? 😄

  1. Unknown attribute 'SecurelyStored' (that's my fault should have made it public)

  2. Value of type 'KeychainAccessor' has no member 'getData'

  3. Value of type 'KeychainAccessor' has no member 'removeData'

  4. Value of type 'KeychainAccessor' has no member 'setData'

  5. Generic parameter 'T' could not be inferred

  6. Result of call to 'deleteData(forKey:)' is unused

  7. Result of call to 'storeData(_:forKey:)' is unused

Okay we'll use correct names retrieveData, deleteData and storeData. What is this whole JSONDecoder and do-catch, we don't need those so I'll remove it?

@propertyWrapper
public struct SecurelyStored<T: Codable> {
    private let key: String
    private let keychainAccessor: KeychainAccessor

    public init(key: String, service: String) {
        self.init(key: key, accessor: KeychainAccessor(service: service))
    }

    public init(key: String, accessor: KeychainAccessor) {
        self.key = key
        self.keychainAccessor = accessor
    }

    public var wrappedValue: T? {
        get {
            keychainAccessor.retrieveData(forKey: key)
        }
        set {
            guard let value = newValue else {
                _ = keychainAccessor.deleteData(forKey: key)
                return
            }

            _ = keychainAccessor.storeData(value, forKey: key)
        }
    }
}

Once cleaned up we'll also need to test this new wrapper. I wrote this test myself.

func testSecurelyStoredAPIToken() {

    class MyViewModel {
        static let keychainService = "MyViewModelService"

        @SecurelyStored(key: "ApiToken", service: MyViewModel.keychainService)
        var apiToken: String?
    }

    let testee = MyViewModel()
    let exampleData = "Some test data"

    testee.apiToken = exampleData
    XCTAssertEqual(testee.apiToken, exampleData, "Failed to store data in Keychain")

    let accessor = KeychainAccessor(service: MyViewModel.keychainService)
    XCTAssertEqual(accessor.retrieveData(forKey: "ApiToken"), exampleData)

    testee.apiToken = nil
    XCTAssertEqual(testee.apiToken, nil, "Failed to delete data from Keychain")
}

Fascinating, func testSecurelyStoredAPIToken() ✅ passes.

How can we can push this exercise and Copilot further? As we ignored errors on the property wrapper implementation, because var wrappedValue can't throw I decided to write a test where we could be notified when an error happens.

func testSecurelyStoredAPITokenWithErrorHandling() {

    class MyViewModel {
        static let keychainService = "MyViewModelService"

        @SecurelyStored(key: "ApiToken", accessor: ConcernedKeychainAccessor(service: MyViewModel.keychainService))
        var apiToken: String?
    }

    let expectation = XCTNSNotificationExpectation(name: Notification.Name("KeychainAccessorError"))
    let testee = MyViewModel()

    // Whoops, we're trying to delete the item that doesn't exist,
    // we should get `errSecItemNotFound`
    testee.apiToken = nil

    wait(for: [expectation], timeout: 1.0)
}

I asked Copilot again to create the implementation for this new subclass.

Can you create a new subclass of KeychainAccessor named ConcernedKeychainAccessor that will override each function and post an NSNotification named KeychainAccessorError in case the OSStatus is not errSecSuccess?

class ConcernedKeychainAccessor: KeychainAccessor {
    override func save(key: String, value: String) {
        super.save(key: key, value: value)
        if status != errSecSuccess {
            NotificationCenter.default.post(name: NSNotification.Name("KeychainAccessorError"), object: nil)
        }
    }

    override func load(key: String) -> String? {
        let value = super.load(key: key)
        if status != errSecSuccess {
            NotificationCenter.default.post(name: NSNotification.Name("KeychainAccessorError"), object: nil)
        }
        return value
    }

    override func delete(key: String) {
        super.delete(key: key)
        if status != errSecSuccess {
            NotificationCenter.default.post(name: NSNotification.Name("KeychainAccessorError"), object: nil)
        }
    }
}

Again I've got wrongly named functions 🤔 What's up with the name of the functions? It looks like Copilot is guessing them? I'll clean it up.

class ConcernedKeychainAccessor: KeychainAccessor {
    override public func storeData<T: Codable>(_ object: T, forKey key: String) -> OSStatus {
        let status = super.storeData(object, forKey: key)
        if status != errSecSuccess {
            NotificationCenter.default.post(name: NSNotification.Name("KeychainAccessorError"), object: nil)
        }
        return status
    }

    override public func deleteData(forKey key: String) -> OSStatus {
        let status = super.deleteData(forKey: key)
        if status != errSecSuccess {
            NotificationCenter.default.post(name: NSNotification.Name("KeychainAccessorError"), object: nil)
        }
        return status
    }
}

and func testSecurelyStoredAPITokenWithErrorHandling() ✅ passes 🥳🎉

As I mentioned at the start you can find the final code on my GitHub Gist.

TL;DR learnings 😊

  1. Copilot chat is easy to use, but the Editor embedded helper where you can highlight or tap on class or function names to prompt it is very convenient.

  2. Copilot also doesn't generate code that compiles on the first prompt.

  3. Some of the compilation errors are very obvious as the implementation is referencing functions and calls with the wrong name. I can only assume Copilot is guessing the names instead of reading or copy pasting them in the code it generates, would be interesting to see how we can overcome this challenge.

  4. In case you could write the code yourself, it still feels faster to generate the code and then quickly modify it to make it compile.

  5. I haven't used the autocompletion feature yet, I would like to see how that works.

I hope you enjoyed this exercise and stay tuned for more learnings on how to get Copilot correctly use the function names and how to write code faster with AI suggested completion.