feat(mac): persist device identity in keychain - #6
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Protocol displayName validation can diverge cross-platform (Swift grapheme count vs JS UTF-16 length) and the Keychain-backed provider isn’t covered by tests.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new macOS SwiftPM identity module that persists a device P-256 private key in Keychain, derives a protocol-compatible public identity envelope (including tcid1_ deviceId), and produces DER-encoded ECDSA signatures over transcripts.
Changes:
- Introduces
TouchCodeIdentitytarget withDeviceIdentityStorefor load-or-create and signing using a Keychain-backed P-256 key. - Implements shared-protocol deviceId derivation (domain-separated SHA-256 over X9.63 public key) and base64url encoding.
- Adds deterministic unit tests covering derivation, concurrency behavior, tamper detection, and failure/validation scenarios via in-memory key providers.
File summaries
| File | Description |
|---|---|
| apps/mac-app/Sources/TouchCodeIdentity/DeviceIdentityStore.swift | New identity store + derivation utilities + Keychain key provider for persistent P-256 identity and DER signing. |
| apps/mac-app/Tests/TouchCodeIdentityTests/DeviceIdentityStoreTests.swift | New tests for deviceId golden vector, stability across calls, concurrency, signature verification, and failure modes. |
| apps/mac-app/Package.swift | Registers the new TouchCodeIdentity target, wires it into the mac executable, and adds a dedicated test target. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+139
to
+142
| let normalizedName = displayName.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard !normalizedName.isEmpty, normalizedName.count <= 128 else { | ||
| throw DeviceIdentityError.invalidDisplayName | ||
| } |
Comment on lines
+69
to
+77
| func loadPrivateKey() throws -> SecKey? { | ||
| var result: CFTypeRef? | ||
| let status = SecItemCopyMatching([ | ||
| kSecClass as String: kSecClassKey, | ||
| kSecAttrApplicationTag as String: applicationTag, | ||
| kSecAttrKeyClass as String: kSecAttrKeyClassPrivate, | ||
| kSecReturnRef as String: true, | ||
| kSecMatchLimit as String: kSecMatchLimitOne, | ||
| ] as CFDictionary, &result) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the first Mac-side Phase 2 identity foundation: one persistent P-256 private key in Keychain, a protocol-compatible public identity envelope, and DER ECDSA transcript signing. This slice does not wire identity into Bridge routes or pairing UI.
Changes
TouchCodeIdentitySwiftPM target used by the Mac executable.tcid1_device ID using the shared protocol domain separator.Validation
DEVELOPER_DIR=/Applications/Xcode-beta.app/Contents/Developer swift test --scratch-path /tmp/touchcode-mac-device-identity.v64cbr— 7/7 passed.DEVELOPER_DIR=/Applications/Xcode-beta.app/Contents/Developer swift build --scratch-path /tmp/touchcode-mac-device-identity.v64cbr— passed.DEVELOPER_DIR=/Applications/Xcode-beta.app/Contents/Developer ./script/build_and_run.sh verify— passed; existing--build-system nativedeprecation warning only.git diff --check origin/main...HEAD— passed.API Changes
Adds the internal Swift
TouchCodeIdentitytarget withDeviceIdentity,DeviceIdentityStore,DeviceIdentityDerivation, andDeviceIdentityError. No Bridge HTTP or shared TypeScript protocol shape changes.Risks / unresolved issues
.buildpath in this FileProvider checkout acquired Finder attributes that broke test-bundle codesign; tests passed with an isolated/tmpscratch path.Implementation complete: yes
Self-review complete: yes
Known remaining risks: production Keychain and end-to-end pairing are not yet verified.
Delivered branch SHA:
f99bf344fe39961efb447401ab9dbdd8d3021d41