Adds a new "secure-enclave" unlocker type that stores the vault's
long-term private key encrypted by a non-exportable P-256 key held
in the Secure Enclave hardware. Decryption (ECDH) is performed
inside the SE; the key never leaves the hardware.
Uses CryptoTokenKit identities created via sc_auth, which allows
SE access from unsigned binaries without Apple Developer Program
membership. ECIES (X963SHA256 + AES-GCM) handles encryption and
decryption through Security.framework.
New package internal/macse/ provides the CGo bridge to
Security.framework for SE key creation, ECIES encrypt/decrypt,
and key deletion. The SE unlocker directly encrypts the vault
long-term key (no intermediate age keypair).
1. internal/secret/seunlocker_stub.go: panic() in library code
Every method in this file panics instead of returning an error. This violates AGENTS.md Go-Specific Guideline #1: "No panic, log.Fatal, or os.Exit in library code. Always propagate errors via return values."
The existing keychainunlocker_stub.go correctly returns errors for all methods and constructs a valid (but inert) struct from NewKeychainUnlocker. The SE stub should follow this exact same pattern. CreateSecureEnclaveUnlocker already has an error return — use it. The interface methods (GetIdentity, Remove) also have error returns. GetType, GetDirectory, GetID, GetMetadata can return sentinel values.
Files: internal/secret/seunlocker_stub.go lines 29-63, all methods.
2. internal/secret/seunlocker_darwin.go:280: Hardcoded derivation index 0
getLongTermKeyForSE uses hardcoded derivation index 0 when deriving from the mnemonic env var. Compare with vault.go:GetOrDeriveLongTermKey() which correctly loads metadata.DerivationIndex from vault metadata. If the vault uses a derivation index other than 0, this will encrypt the wrong key into the SE unlocker, silently corrupting it. This is a correctness bug.
3. Missing tests for new exported types in secret package
The PR adds these new exports to internal/secret/ with zero test coverage:
SecureEnclaveUnlocker (type)
SecureEnclaveUnlockerMetadata (type)
NewSecureEnclaveUnlocker (constructor)
CreateSecureEnclaveUnlocker (function)
The macse package has good tests (macse_test.go), but the secret package SE unlocker code has none. The existing keychainunlocker_test.go demonstrates the expected pattern. At minimum, NewSecureEnclaveUnlocker should be tested (instantiation, getter methods) and CreateSecureEnclaveUnlocker should have a happy-path test (darwin-only, with SE availability skip guard).
4. README inconsistency
The README has not been updated to reflect this implementation:
Line 289: Still says "Secure Enclave Unlockers (macOS - planned)" with "Currently partially implemented but non-functional"
Line 333: Says "Secure Enclave support planned (requires paid Apple Developer Program for signed entitlements)"
Line 446: Says "planned Secure Enclave integration"
Lines 181-188: The unlocker add types list doesn't include secure-enclave
Lines 385-387: The "Advanced Authentication" examples don't include secret unlocker add secure-enclave
The implementation uses sc_auth / CryptoTokenKit which bypasses the entitlement requirement, so the README's claim about needing Apple Developer Program membership is now inaccurate.
Requirements Checklist
Requirement
Status
Secure Enclave unlocker type
✅ Met
CGo bindings for SE operations (create/encrypt/decrypt/delete)
✅ Met
Non-darwin stubs for cross-platform compilation
⚠️ Compiles, but panics instead of returning errors
CLI integration (add, list, select)
✅ Met
Vault unlock flow integration
✅ Met
Tests for macse package
✅ Met (darwin-only)
Tests for secret package SE unlocker
❌ Missing
README updated
❌ Not updated
Correct mnemonic derivation index
❌ Hardcoded to 0
Build Result
docker build . — ✅ PASS (4.2s)
Additional Notes
The vault.go refactoring (extracting unlockLongTermKey) is clean and the type-dispatch approach for SE vs. standard unlockers is reasonable.
The macse CGo bindings and Objective-C implementation look well-structured with proper memory management (@autoreleasepool, CFRelease).
The macse_stub.go correctly returns errors (not panics) — this is the right pattern.
The hash parsing from sc_auth list-ctk-identities output (secure_enclave.m lines 161-179) is fragile — it depends on the output format of sc_auth. Consider a comment noting this.
No linter config, CI config, test assertion, or Makefile changes detected — no cheating.
Summary
Four issues require resolution: (1) stub panics must be replaced with error returns per existing codebase pattern, (2) derivation index bug must be fixed, (3) tests needed for secret package exports, (4) README must be updated.
## Code Review: PR #24 — secure-enclave-unlocker
### Verdict: ❌ FAIL
---
### Policy Divergences
**1. `internal/secret/seunlocker_stub.go`: `panic()` in library code**
Every method in this file panics instead of returning an error. This violates AGENTS.md Go-Specific Guideline #1: "No `panic`, `log.Fatal`, or `os.Exit` in library code. Always propagate errors via return values."
The existing `keychainunlocker_stub.go` correctly returns errors for all methods and constructs a valid (but inert) struct from `NewKeychainUnlocker`. The SE stub should follow this exact same pattern. `CreateSecureEnclaveUnlocker` already has an error return — use it. The interface methods (`GetIdentity`, `Remove`) also have error returns. `GetType`, `GetDirectory`, `GetID`, `GetMetadata` can return sentinel values.
Files: `internal/secret/seunlocker_stub.go` lines 29-63, all methods.
**2. `internal/secret/seunlocker_darwin.go:280`: Hardcoded derivation index 0**
```go
ltIdentity, err := agehd.DeriveIdentity(envMnemonic, 0)
```
`getLongTermKeyForSE` uses hardcoded derivation index `0` when deriving from the mnemonic env var. Compare with `vault.go:GetOrDeriveLongTermKey()` which correctly loads `metadata.DerivationIndex` from vault metadata. If the vault uses a derivation index other than 0, this will encrypt the **wrong key** into the SE unlocker, silently corrupting it. This is a correctness bug.
**3. Missing tests for new exported types in `secret` package**
The PR adds these new exports to `internal/secret/` with zero test coverage:
- `SecureEnclaveUnlocker` (type)
- `SecureEnclaveUnlockerMetadata` (type)
- `NewSecureEnclaveUnlocker` (constructor)
- `CreateSecureEnclaveUnlocker` (function)
The `macse` package has good tests (`macse_test.go`), but the `secret` package SE unlocker code has none. The existing `keychainunlocker_test.go` demonstrates the expected pattern. At minimum, `NewSecureEnclaveUnlocker` should be tested (instantiation, getter methods) and `CreateSecureEnclaveUnlocker` should have a happy-path test (darwin-only, with SE availability skip guard).
**4. README inconsistency**
The README has not been updated to reflect this implementation:
- Line 289: Still says "Secure Enclave Unlockers (macOS - planned)" with "Currently partially implemented but non-functional"
- Line 333: Says "Secure Enclave support planned (requires paid Apple Developer Program for signed entitlements)"
- Line 446: Says "planned Secure Enclave integration"
- Lines 181-188: The `unlocker add` types list doesn't include `secure-enclave`
- Lines 385-387: The "Advanced Authentication" examples don't include `secret unlocker add secure-enclave`
The implementation uses `sc_auth` / CryptoTokenKit which bypasses the entitlement requirement, so the README's claim about needing Apple Developer Program membership is now inaccurate.
---
### Requirements Checklist
| Requirement | Status |
|---|---|
| Secure Enclave unlocker type | ✅ Met |
| CGo bindings for SE operations (create/encrypt/decrypt/delete) | ✅ Met |
| Non-darwin stubs for cross-platform compilation | ⚠️ Compiles, but panics instead of returning errors |
| CLI integration (add, list, select) | ✅ Met |
| Vault unlock flow integration | ✅ Met |
| Tests for `macse` package | ✅ Met (darwin-only) |
| Tests for `secret` package SE unlocker | ❌ Missing |
| README updated | ❌ Not updated |
| Correct mnemonic derivation index | ❌ Hardcoded to 0 |
---
### Build Result
`docker build .` — ✅ PASS (4.2s)
---
### Additional Notes
- The `vault.go` refactoring (extracting `unlockLongTermKey`) is clean and the type-dispatch approach for SE vs. standard unlockers is reasonable.
- The `macse` CGo bindings and Objective-C implementation look well-structured with proper memory management (`@autoreleasepool`, `CFRelease`).
- The `macse_stub.go` correctly returns errors (not panics) — this is the right pattern.
- The hash parsing from `sc_auth list-ctk-identities` output (secure_enclave.m lines 161-179) is fragile — it depends on the output format of `sc_auth`. Consider a comment noting this.
- No linter config, CI config, test assertion, or Makefile changes detected — no cheating.
### Summary
Four issues require resolution: (1) stub panics must be replaced with error returns per existing codebase pattern, (2) derivation index bug must be fixed, (3) tests needed for `secret` package exports, (4) README must be updated.
NewSecureEnclaveUnlocker() returns a valid (inert) struct instance
2. Hardcoded derivation index 0 → vault metadata
getLongTermKeyForSE() now reads vault-metadata.json to get metadata.DerivationIndex, matching the pattern in getLongTermPrivateKey() from keychainunlocker.go. This prevents silent key corruption when vaults use non-zero derivation indices.
3. Tests for secret package SE exports
Added two test files:
seunlocker_stub_test.go (non-darwin): tests NewSecureEnclaveUnlocker instantiation/getters, error returns from GetIdentity/Remove/CreateSecureEnclaveUnlocker, and interface compliance
seunlocker_test.go (darwin): tests instantiation, interface compliance, GetID format, generateSEKeyLabel, and GetIdentity with missing encrypted key file
4. README updated
Removed all "planned" / "partially implemented" / "non-functional" language
Removed incorrect Apple Developer Program requirement (implementation uses sc_auth / CryptoTokenKit)
Added secure-enclave to unlocker type list in CLI reference
Added secret unlocker add secure-enclave to Advanced Authentication examples
Updated Cross-Platform Support and Features sections
docker build .✅ PASS
## Rework Complete
All four review findings have been addressed:
### 1. Stub panics → error returns
`internal/secret/seunlocker_stub.go` now follows the `keychainunlocker_stub.go` pattern exactly:
- Sentinel error `errSENotSupported` used for `GetIdentity()`, `Remove()`, and `CreateSecureEnclaveUnlocker()`
- `GetType()` returns `"secure-enclave"`, `GetMetadata()` returns stored metadata, `GetDirectory()` returns stored directory, `GetID()` returns formatted string
- `NewSecureEnclaveUnlocker()` returns a valid (inert) struct instance
### 2. Hardcoded derivation index 0 → vault metadata
`getLongTermKeyForSE()` now reads `vault-metadata.json` to get `metadata.DerivationIndex`, matching the pattern in `getLongTermPrivateKey()` from `keychainunlocker.go`. This prevents silent key corruption when vaults use non-zero derivation indices.
### 3. Tests for `secret` package SE exports
Added two test files:
- `seunlocker_stub_test.go` (non-darwin): tests `NewSecureEnclaveUnlocker` instantiation/getters, error returns from `GetIdentity`/`Remove`/`CreateSecureEnclaveUnlocker`, and interface compliance
- `seunlocker_test.go` (darwin): tests instantiation, interface compliance, `GetID` format, `generateSEKeyLabel`, and `GetIdentity` with missing encrypted key file
### 4. README updated
- Removed all "planned" / "partially implemented" / "non-functional" language
- Removed incorrect Apple Developer Program requirement (implementation uses `sc_auth` / CryptoTokenKit)
- Added `secure-enclave` to unlocker type list in CLI reference
- Added `secret unlocker add secure-enclave` to Advanced Authentication examples
- Updated Cross-Platform Support and Features sections
`docker build .` ✅ PASS
Tests for secret package SE unlocker (both platforms)
✅ Met
README updated to reflect implementation
✅ Met
Correct mnemonic derivation index from vault metadata
✅ Met
Build Result
docker build . — ✅ PASS (4.3s)
Rebase onto main — ✅ Clean (already up to date)
Code Quality Notes
vault.go refactoring (unlockLongTermKey extraction) is clean and the type-dispatch for SE vs. standard unlockers is architecturally sound.
Memory handling is good: memguard.LockedBuffer used for key material, decryptedData zeroed immediately after parsing in GetIdentity().
Objective-C implementation has proper @autoreleasepool blocks and CFRelease calls — no memory leaks.
Stub GetID() omits hostname (returns timestamp-secure-enclave vs darwin's timestamp-hostname-secure-enclave) — this matches the existing KeychainUnlocker stub pattern exactly.
The sc_auth list-ctk-identities output parsing in secure_enclave.m is fragile (depends on whitespace-tokenized format) — previously noted, not blocking.
## Code Review: [PR #24](https://git.eeqj.de/sneak/secret/pulls/24) — secure-enclave-unlocker (post-rework)
### Verdict: ✅ PASS
---
### Previous Review Findings — Resolution Status
All four findings from the [initial review](https://git.eeqj.de/sneak/secret/pulls/24#issuecomment-13251) have been properly addressed:
| Finding | Status | Verification |
|---|---|---|
| 1. Stub panics → error returns | ✅ Fixed | `seunlocker_stub.go` now uses sentinel `errSENotSupported`, matching `keychainunlocker_stub.go` exactly |
| 2. Hardcoded derivation index 0 | ✅ Fixed | `getLongTermKeyForSE()` reads `vault-metadata.json` for `metadata.DerivationIndex`, matching `keychainunlocker.go` pattern |
| 3. Missing tests for `secret` package SE exports | ✅ Fixed | Two test files: `seunlocker_stub_test.go` (non-darwin) and `seunlocker_test.go` (darwin), covering constructor, getters, interface compliance, error paths |
| 4. README inconsistency | ✅ Fixed | All "planned"/"partially implemented" language removed, SE added to unlocker type list, examples, features, and cross-platform sections |
---
### Policy Divergences
No policy violations found in the changed files.
- No `panic`, `log.Fatal`, or `os.Exit` in library code ✅
- Errors wrapped with `fmt.Errorf("context: %w", err)` throughout ✅
- No changes to `.golangci.yml`, `Makefile`, or CI config ✅
- No changes to `go.mod`/`go.sum` (no new dependencies) ✅
- No weakened tests or suppressed lint rules ✅
- Constants used for repeated strings (`seKeyLabelPrefix`, `seUnlockerType`, `seLongtermFilename`, `p256UncompressedKeySize`, etc.) ✅
---
### Requirements Checklist
| Requirement | Status |
|---|---|
| Secure Enclave unlocker type | ✅ Met |
| CGo bindings for SE operations (create/encrypt/decrypt/delete) | ✅ Met |
| Non-darwin stubs — compile and return errors | ✅ Met |
| CLI integration (add, list, select) | ✅ Met |
| Vault unlock flow integration (`unlockLongTermKey` refactor) | ✅ Met |
| Tests for `macse` package (darwin) | ✅ Met |
| Tests for `secret` package SE unlocker (both platforms) | ✅ Met |
| README updated to reflect implementation | ✅ Met |
| Correct mnemonic derivation index from vault metadata | ✅ Met |
---
### Build Result
`docker build .` — ✅ PASS (4.3s)
Rebase onto `main` — ✅ Clean (already up to date)
---
### Code Quality Notes
- `vault.go` refactoring (`unlockLongTermKey` extraction) is clean and the type-dispatch for SE vs. standard unlockers is architecturally sound.
- Memory handling is good: `memguard.LockedBuffer` used for key material, `decryptedData` zeroed immediately after parsing in `GetIdentity()`.
- Objective-C implementation has proper `@autoreleasepool` blocks and `CFRelease` calls — no memory leaks.
- Stub `GetID()` omits hostname (returns `timestamp-secure-enclave` vs darwin's `timestamp-hostname-secure-enclave`) — this matches the existing `KeychainUnlocker` stub pattern exactly.
- The `sc_auth list-ctk-identities` output parsing in `secure_enclave.m` is fragile (depends on whitespace-tokenized format) — previously noted, not blocking.
Adds a new "secure-enclave" unlocker type that stores the vault's
long-term private key encrypted by a non-exportable P-256 key held
in the Secure Enclave hardware. Decryption (ECDH) is performed
inside the SE; the key never leaves the hardware.
Uses CryptoTokenKit identities created via sc_auth, which allows
SE access from unsigned binaries without Apple Developer Program
membership. ECIES (X963SHA256 + AES-GCM) handles encryption and
decryption through Security.framework.
New package internal/macse/ provides the CGo bridge to
Security.framework for SE key creation, ECIES encrypt/decrypt,
and key deletion. The SE unlocker directly encrypts the vault
long-term key (no intermediate age keypair).
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
clawbot
Code Review: PR #24 — secure-enclave-unlocker
Verdict: ❌ FAIL
Policy Divergences
1.
internal/secret/seunlocker_stub.go:panic()in library codeEvery method in this file panics instead of returning an error. This violates AGENTS.md Go-Specific Guideline #1: "No
panic,log.Fatal, oros.Exitin library code. Always propagate errors via return values."The existing
keychainunlocker_stub.gocorrectly returns errors for all methods and constructs a valid (but inert) struct fromNewKeychainUnlocker. The SE stub should follow this exact same pattern.CreateSecureEnclaveUnlockeralready has an error return — use it. The interface methods (GetIdentity,Remove) also have error returns.GetType,GetDirectory,GetID,GetMetadatacan return sentinel values.Files:
internal/secret/seunlocker_stub.golines 29-63, all methods.2.
internal/secret/seunlocker_darwin.go:280: Hardcoded derivation index 0getLongTermKeyForSEuses hardcoded derivation index0when deriving from the mnemonic env var. Compare withvault.go:GetOrDeriveLongTermKey()which correctly loadsmetadata.DerivationIndexfrom vault metadata. If the vault uses a derivation index other than 0, this will encrypt the wrong key into the SE unlocker, silently corrupting it. This is a correctness bug.3. Missing tests for new exported types in
secretpackageThe PR adds these new exports to
internal/secret/with zero test coverage:SecureEnclaveUnlocker(type)SecureEnclaveUnlockerMetadata(type)NewSecureEnclaveUnlocker(constructor)CreateSecureEnclaveUnlocker(function)The
macsepackage has good tests (macse_test.go), but thesecretpackage SE unlocker code has none. The existingkeychainunlocker_test.godemonstrates the expected pattern. At minimum,NewSecureEnclaveUnlockershould be tested (instantiation, getter methods) andCreateSecureEnclaveUnlockershould have a happy-path test (darwin-only, with SE availability skip guard).4. README inconsistency
The README has not been updated to reflect this implementation:
unlocker addtypes list doesn't includesecure-enclavesecret unlocker add secure-enclaveThe implementation uses
sc_auth/ CryptoTokenKit which bypasses the entitlement requirement, so the README's claim about needing Apple Developer Program membership is now inaccurate.Requirements Checklist
macsepackagesecretpackage SE unlockerBuild Result
docker build .— ✅ PASS (4.2s)Additional Notes
vault.gorefactoring (extractingunlockLongTermKey) is clean and the type-dispatch approach for SE vs. standard unlockers is reasonable.macseCGo bindings and Objective-C implementation look well-structured with proper memory management (@autoreleasepool,CFRelease).macse_stub.gocorrectly returns errors (not panics) — this is the right pattern.sc_auth list-ctk-identitiesoutput (secure_enclave.m lines 161-179) is fragile — it depends on the output format ofsc_auth. Consider a comment noting this.Summary
Four issues require resolution: (1) stub panics must be replaced with error returns per existing codebase pattern, (2) derivation index bug must be fixed, (3) tests needed for
secretpackage exports, (4) README must be updated.@clawbot pls rework
Rework agent dispatched to address review findings: (1) stub panics → error returns, (2) hardcoded derivation index fix, (3) missing tests for secret package SE exports, (4) README update.
b9aaf8ddactocc53469f90Rework Complete
All four review findings have been addressed:
1. Stub panics → error returns
internal/secret/seunlocker_stub.gonow follows thekeychainunlocker_stub.gopattern exactly:errSENotSupportedused forGetIdentity(),Remove(), andCreateSecureEnclaveUnlocker()GetType()returns"secure-enclave",GetMetadata()returns stored metadata,GetDirectory()returns stored directory,GetID()returns formatted stringNewSecureEnclaveUnlocker()returns a valid (inert) struct instance2. Hardcoded derivation index 0 → vault metadata
getLongTermKeyForSE()now readsvault-metadata.jsonto getmetadata.DerivationIndex, matching the pattern ingetLongTermPrivateKey()fromkeychainunlocker.go. This prevents silent key corruption when vaults use non-zero derivation indices.3. Tests for
secretpackage SE exportsAdded two test files:
seunlocker_stub_test.go(non-darwin): testsNewSecureEnclaveUnlockerinstantiation/getters, error returns fromGetIdentity/Remove/CreateSecureEnclaveUnlocker, and interface complianceseunlocker_test.go(darwin): tests instantiation, interface compliance,GetIDformat,generateSEKeyLabel, andGetIdentitywith missing encrypted key file4. README updated
sc_auth/ CryptoTokenKit)secure-enclaveto unlocker type list in CLI referencesecret unlocker add secure-enclaveto Advanced Authentication examplesdocker build .✅ PASSCode Review: PR #24 — secure-enclave-unlocker (post-rework)
Verdict: ✅ PASS
Previous Review Findings — Resolution Status
All four findings from the initial review have been properly addressed:
seunlocker_stub.gonow uses sentinelerrSENotSupported, matchingkeychainunlocker_stub.goexactlygetLongTermKeyForSE()readsvault-metadata.jsonformetadata.DerivationIndex, matchingkeychainunlocker.gopatternsecretpackage SE exportsseunlocker_stub_test.go(non-darwin) andseunlocker_test.go(darwin), covering constructor, getters, interface compliance, error pathsPolicy Divergences
No policy violations found in the changed files.
panic,log.Fatal, oros.Exitin library code ✅fmt.Errorf("context: %w", err)throughout ✅.golangci.yml,Makefile, or CI config ✅go.mod/go.sum(no new dependencies) ✅seKeyLabelPrefix,seUnlockerType,seLongtermFilename,p256UncompressedKeySize, etc.) ✅Requirements Checklist
unlockLongTermKeyrefactor)macsepackage (darwin)secretpackage SE unlocker (both platforms)Build Result
docker build .— ✅ PASS (4.3s)Rebase onto
main— ✅ Clean (already up to date)Code Quality Notes
vault.gorefactoring (unlockLongTermKeyextraction) is clean and the type-dispatch for SE vs. standard unlockers is architecturally sound.memguard.LockedBufferused for key material,decryptedDatazeroed immediately after parsing inGetIdentity().@autoreleasepoolblocks andCFReleasecalls — no memory leaks.GetID()omits hostname (returnstimestamp-secure-enclavevs darwin'stimestamp-hostname-secure-enclave) — this matches the existingKeychainUnlockerstub pattern exactly.sc_auth list-ctk-identitiesoutput parsing insecure_enclave.mis fragile (depends on whitespace-tokenized format) — previously noted, not blocking.