From df2850214009732eb9bb0c64c324e4dcd91c2b1f Mon Sep 17 00:00:00 2001 From: Takuya Matsuyama Date: Wed, 14 Oct 2026 10:46:41 +0900 Subject: [PATCH 1/5] feat(ios): Add service and accessGroup options --- README.md | 26 +++++++ example/src/tests/s2.ts | 146 ++++++++++++++++++++++++++++++++++++++++ ios/bindings.mm | 92 +++++++++++++++++++++---- src/index.tsx | 42 +++++++++--- 4 files changed, 284 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 2fce348..69d9857 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,32 @@ The resulting Android authenticator combinations are: If you pass `withBiometrics: true` without a `biometricPrompt` object at all, the prompt falls back to the defaults listed above. +### iOS Service and Access Group + +On iOS every item is identified by its `key` plus a keychain service and access group. Pass `service` and/or `accessGroup` to `set`, `get` and `del` to control them, e.g. to share an item with an app extension: + +```ts +const scope = { + service: 'com.example.app', + accessGroup: 'group.com.example.app', +}; + +set({ key: 'token', value: 'secret', ...scope }); +const { value } = get({ key: 'token', ...scope }); +del({ key: 'token', ...scope }); +``` + +| Option | Explanation | Default | +| ------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------- | +| `service` | The keychain service (`kSecAttrService`). | The app's bundle identifier | +| `accessGroup` | The keychain access group (`kSecAttrAccessGroup`). The app (and any extension reading the item) must list it in its `keychain-access-groups` entitlement, or use an app group ID (`group.*`) it has. Otherwise the call fails with `[op-s2] Missing entitlement`. | The app's default group | + +- Empty strings are treated as unset. +- Use the same `service` and `accessGroup` for every operation on an item. An item written without them is not found by a `get` with a different `service`. +- Omitting `accessGroup` follows the Keychain API: `set` writes to the app's default access group, but `get`, `del` and the replace step of `set` match the key in **every** access group the app belongs to. So a `set` or `del` without `accessGroup` also removes a copy shared through another group, and a `get` without it returns whichever copy it finds first. Pass `accessGroup` whenever an item may exist in more than one group. +- A non-string `service` or `accessGroup` is an error rather than silently using the default scope. +- Both options are ignored on Android, where items are always private to the app. + ## Secure Enclave The library already uses the [Secure Enclave](https://support.apple.com/en-gb/guide/security/sec59b0b31ff/web) to encrypt the data before saving it on the keychain. However, it is not possible to store data directly on the Secure Enclave as it is meant to store the private part of asymmetrical keys, meaning data saved there cannot be retrieved to user space. diff --git a/example/src/tests/s2.ts b/example/src/tests/s2.ts index 7f930bc..c0ca652 100644 --- a/example/src/tests/s2.ts +++ b/example/src/tests/s2.ts @@ -1,5 +1,10 @@ import { del, get, set, ACCESSIBILITY } from '@op-engineering/op-s2'; import { describe, expect, it } from '@op-engineering/op-test'; +import { Platform } from 'react-native'; + +// PRODUCT_BUNDLE_IDENTIFIER of the example app, which op-s2 uses as the +// default keychain service on iOS. +const EXAMPLE_BUNDLE_ID = 'com.op.s2'; // Toggle to `true` when running on a simulator/emulator (where biometrics are // not enrolled and the prompt short-circuits with "Biometrics not available"). @@ -188,6 +193,147 @@ describe('accessibility option (iOS)', () => { }); }); +if (Platform.OS === 'ios') { + describe('service / accessGroup options (iOS)', () => { + it('set/get with a custom service', () => { + const key = 'key_service'; + + const { error: setError } = set({ + key, + value: 'myServiceValue', + service: 'svcA', + }); + expect(setError).toBe(undefined); + + const { value, error } = get({ key, service: 'svcA' }); + expect(error).toBe(undefined); + expect(value).toEqual('myServiceValue'); + + del({ key, service: 'svcA' }); + }); + + it('items in different services do not collide', () => { + const key = 'key_service_isolation'; + + set({ key, value: 'valueA', service: 'svcA' }); + set({ key, value: 'valueB', service: 'svcB' }); + + expect(get({ key, service: 'svcA' }).value).toEqual('valueA'); + expect(get({ key, service: 'svcB' }).value).toEqual('valueB'); + + const { value, error } = get({ key }); + expect(value).toBe(undefined); + expect(error).toEqual('[op-s2] Item not found'); + + del({ key, service: 'svcA' }); + expect(get({ key, service: 'svcA' }).error).toEqual( + '[op-s2] Item not found' + ); + expect(get({ key, service: 'svcB' }).value).toEqual('valueB'); + + del({ key, service: 'svcB' }); + }); + + it('defaults the service to the bundle identifier', () => { + const key = 'key_service_default'; + + set({ key, value: 'myDefaultValue' }); + + const { value, error } = get({ key, service: EXAMPLE_BUNDLE_ID }); + expect(error).toBe(undefined); + expect(value).toEqual('myDefaultValue'); + + del({ key, service: EXAMPLE_BUNDLE_ID }); + expect(get({ key }).error).toEqual('[op-s2] Item not found'); + }); + + it('treats empty service and accessGroup as unset', () => { + const key = 'key_service_empty'; + + const { error: setError } = set({ + key, + value: 'myEmptyScopeValue', + service: '', + accessGroup: '', + }); + expect(setError).toBe(undefined); + + const { value, error } = get({ key }); + expect(error).toBe(undefined); + expect(value).toEqual('myEmptyScopeValue'); + + del({ key, service: '', accessGroup: '' }); + expect(get({ key }).error).toEqual('[op-s2] Item not found'); + }); + + it('fails with an access group the app is not entitled to', () => { + const key = 'key_access_group_unentitled'; + + const { error } = set({ + key, + value: 'myValue', + accessGroup: 'group.op.s2.not-entitled', + }); + expect(error).toEqual('[op-s2] Missing entitlement'); + + // Clean up in case the write unexpectedly succeeded on this setup. + del({ key, accessGroup: 'group.op.s2.not-entitled' }); + }); + + it('rejects a non-string service or accessGroup', () => { + const key = 'key_service_invalid'; + + const { error: serviceError } = set({ + key, + value: 'myValue', + // @ts-ignore + service: 123, + }); + expect(serviceError).toEqual('service property is not a string'); + + const { error: groupError } = set({ + key, + value: 'myValue', + // @ts-ignore + accessGroup: 123, + }); + expect(groupError).toEqual('accessGroup property is not a string'); + + let getError: unknown; + try { + // @ts-ignore + get({ key, service: 123 }); + } catch (e) { + getError = e; + } + expect(String(getError)).toContain('service property is not a string'); + + expect(get({ key }).error).toEqual('[op-s2] Item not found'); + }); + }); +} else { + describe('service / accessGroup options (Android)', () => { + it('ignores service and accessGroup', () => { + const key = 'key_service_android'; + + const { error: setError } = set({ + key, + value: 'myAndroidValue', + service: 'svcA', + accessGroup: 'group.op.s2.not-entitled', + }); + expect(setError).toBe(undefined); + + const { value, error } = get({ key }); + expect(error).toBe(undefined); + expect(value).toEqual('myAndroidValue'); + + del({ key, service: 'svcB' }); + expect(get({ key }).error).toEqual('[op-s2] Item not found'); + }); + }); +} + if (RUN_BIOMETRIC_TESTS) { describe('biometricPrompt options', () => { it('set/get with only title/subtitle (defaults: no device credential, strong biometric)', () => { diff --git a/ios/bindings.mm b/ios/bindings.mm index 4f8f2c3..d00b02b 100644 --- a/ios/bindings.mm +++ b/ios/bindings.mm @@ -15,6 +15,15 @@ bool allowBiometricWeak; }; +// Identifies which keychain item an operation targets beyond its key. +// Empty strings mean "unset": the service falls back to the bundle identifier +// and no access group is set, which matches the behavior before these options +// existed. +struct ItemScope { + std::string service; + std::string accessGroup; +}; + typedef enum { kBiometricsStateAvailable, kBiometricsStateNotAvailable, @@ -28,8 +37,22 @@ static void zeroizeString(std::string &str) { } } +// Uses the byte length rather than the C string, so an embedded NUL does not +// truncate the value. Returns nil only for invalid UTF-8. static NSString *stringToNSString(const std::string &str) { - return [NSString stringWithUTF8String:str.c_str()]; + return [[NSString alloc] initWithBytes:str.data() length:str.size() encoding:NSUTF8StringEncoding]; +} + +// Reads an optional string property. Returns false when the property is +// present but neither a string nor null/undefined; `out` is left untouched +// unless a string is found. +static bool readOptionalString(jsi::Runtime &rt, const jsi::Object &obj, const char *name, std::string &out) { + if (!obj.hasProperty(rt, name)) return true; + jsi::Value v = obj.getProperty(rt, name); + if (v.isNull() || v.isUndefined()) return true; + if (!v.isString()) return false; + out = v.asString(rt).utf8(rt); + return true; } static BiometricPromptArgs extractBiometricPromptArgs(jsi::Runtime &rt, const jsi::Object ¶ms) { @@ -50,10 +73,8 @@ static BiometricPromptArgs extractBiometricPromptArgs(jsi::Runtime &rt, const js // empty string the same as a missing property so we always fall back // to a safe default. auto getString = [&](const char *name, const std::string &fallback, bool allowEmpty = true) -> std::string { - if (!prompt.hasProperty(rt, name)) return fallback; - jsi::Value v = prompt.getProperty(rt, name); - if (!v.isString()) return fallback; - std::string s = v.asString(rt).utf8(rt); + std::string s = fallback; + if (!readOptionalString(rt, prompt, name, s)) return fallback; if (!allowEmpty && s.empty()) return fallback; return s; }; @@ -76,6 +97,19 @@ static BiometricPromptArgs extractBiometricPromptArgs(jsi::Runtime &rt, const js return args; } +// Reads `service` and `accessGroup` into `scope`. Returns an error message +// when either is present with a non-string value, so a typo can't silently +// fall back to the default scope; returns an empty string otherwise. +static std::string extractItemScope(jsi::Runtime &rt, const jsi::Object ¶ms, ItemScope &scope) { + if (!readOptionalString(rt, params, "service", scope.service)) { + return "service property is not a string"; + } + if (!readOptionalString(rt, params, "accessGroup", scope.accessGroup)) { + return "accessGroup property is not a string"; + } + return ""; +} + // MARK: - LAContext Factory static LAContext *createLAContext(const BiometricPromptArgs &args) { LAContext *context = [[LAContext alloc] init]; @@ -146,14 +180,29 @@ SecAccessControlRef createBioSecAccessControl(bool allowDeviceCredential, bool a nil); } -NSMutableDictionary *get_base_entry_dict(std::string key) { +NSMutableDictionary *get_base_entry_dict(const std::string &key, const ItemScope &scope) { NSMutableDictionary *queryDictionary = [[NSMutableDictionary alloc] init]; [queryDictionary setObject:(__bridge id)kSecClassGenericPassword forKey:(__bridge id)kSecClass]; NSData *encodedIdentifier = [NSData dataWithBytes:key.data() length:key.length()]; [queryDictionary setObject:encodedIdentifier forKey:(__bridge id)kSecAttrGeneric]; [queryDictionary setObject:encodedIdentifier forKey:(__bridge id)kSecAttrAccount]; - [queryDictionary setObject:[[NSBundle mainBundle] bundleIdentifier] forKey:(__bridge id)kSecAttrService]; + + // The bundle identifier can be nil in some test hosts; leave the service + // unset then rather than crash on a nil dictionary value. + NSString *service = scope.service.empty() + ? [[NSBundle mainBundle] bundleIdentifier] + : stringToNSString(scope.service); + if (service != nil) { + [queryDictionary setObject:service forKey:(__bridge id)kSecAttrService]; + } + + // Without an access group, queries (including the delete in `set`) match + // every access group the app belongs to, as with the Keychain API itself. + NSString *accessGroup = scope.accessGroup.empty() ? nil : stringToNSString(scope.accessGroup); + if (accessGroup != nil) { + [queryDictionary setObject:accessGroup forKey:(__bridge id)kSecAttrAccessGroup]; + } return queryDictionary; } @@ -176,8 +225,8 @@ CFStringRef getAccessibilityValue(int accessibility) { // authentication UI — otherwise deleting an entry that was previously stored // with biometric protection would pop the Face ID / passcode prompt even // though the caller explicitly opted out of biometrics. -void _delete(std::string &key, bool withBiometrics) { - NSMutableDictionary *dict = get_base_entry_dict(key); +void _delete(const std::string &key, const ItemScope &scope, bool withBiometrics) { + NSMutableDictionary *dict = get_base_entry_dict(key, scope); [dict setObject:(__bridge id)kSecAttrSynchronizableAny forKey:(__bridge id)kSecAttrSynchronizable]; if (!withBiometrics) { [dict setObject:(__bridge id)kSecUseAuthenticationUIFail forKey:(__bridge id)kSecUseAuthenticationUI]; @@ -250,6 +299,13 @@ void install(jsi::Runtime &rt, std::shared_ptr jsCallInvoker } std::string key = params.getProperty(rt, "key").asString(rt).utf8(rt); + ItemScope scope; + std::string scopeError = extractItemScope(rt, params, scope); + if (!scopeError.empty()) { + res.setProperty(rt, "error", jsi::String::createFromUtf8(rt, scopeError)); + return res; + } + std::string val = params.getProperty(rt, "value").asString(rt).utf8(rt); CFStringRef accessibility = kSecAttrAccessibleAfterFirstUnlock; @@ -274,9 +330,9 @@ void install(jsi::Runtime &rt, std::shared_ptr jsCallInvoker } // Delete prior entries (both local and synced) to prevent errSecDuplicateItem - _delete(key, withBiometrics); + _delete(key, scope, withBiometrics); - NSMutableDictionary *dict = get_base_entry_dict(key); + NSMutableDictionary *dict = get_base_entry_dict(key, scope); if (withBiometrics) { SecAccessControlRef accessControl = createBioSecAccessControl(promptArgs.allowDeviceCredential, promptArgs.allowBiometricWeak); @@ -327,8 +383,13 @@ void install(jsi::Runtime &rt, std::shared_ptr jsCallInvoker } BiometricPromptArgs promptArgs = extractBiometricPromptArgs(rt, params); + ItemScope scope; + std::string scopeError = extractItemScope(rt, params, scope); + if (!scopeError.empty()) { + throw jsi::JSError(rt, scopeError); + } - NSMutableDictionary *dict = get_base_entry_dict(key); + NSMutableDictionary *dict = get_base_entry_dict(key, scope); [dict setObject:(__bridge id)kSecMatchLimitOne forKey:(__bridge id)kSecMatchLimit]; [dict setObject:(__bridge id)kCFBooleanTrue forKey:(__bridge id)kSecReturnData]; @@ -395,7 +456,12 @@ void install(jsi::Runtime &rt, std::shared_ptr jsCallInvoker withBiometrics = params.getProperty(rt, "withBiometrics").asBool(); } - _delete(key, withBiometrics); + ItemScope scope; + std::string scopeError = extractItemScope(rt, params, scope); + if (!scopeError.empty()) { + throw jsi::JSError(rt, scopeError); + } + _delete(key, scope, withBiometrics); return {}; }); diff --git a/src/index.tsx b/src/index.tsx index 384a04a..694ceb5 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -123,6 +123,8 @@ type SetErrors = | 'Key property is missing' | 'Value property is missing' | 'Value property is not a string' + | 'service property is not a string' + | 'accessGroup property is not a string' | '[op-s2] Could not set value, duplicate item' | SharedOperationErrors; @@ -134,7 +136,27 @@ type GetErrors = | 'Biometrics not available' | SharedOperationErrors; -interface SetParams { +interface ItemScope { + /** + * iOS only: the keychain service (`kSecAttrService`) the item is stored + * under. Defaults to the app's bundle identifier. Use the same value for + * `set`, `get` and `del` of an item. + */ + service?: string; + /** + * iOS only: the keychain access group (`kSecAttrAccessGroup`) the item is + * stored in, e.g. to share it with an app extension. The app must have the + * group in its `keychain-access-groups` or app group entitlement, otherwise + * the operation fails with `[op-s2] Missing entitlement`. + * + * When omitted, `set` writes to the app's default access group, but `get`, + * `del` and the replace step of `set` match the key in every access group + * the app belongs to. + */ + accessGroup?: string; +} + +interface SetParams extends ItemScope { key: string; value: string; /** @@ -153,7 +175,7 @@ interface SetParams { biometricPrompt?: BiometricPromptOptions; } -interface GetParams { +interface GetParams extends ItemScope { key: string; accessibility?: ACCESSIBILITY; withBiometrics?: boolean; @@ -163,11 +185,13 @@ interface GetParams { interface OPS2 { set: (params: SetParams) => { error?: SetErrors }; get: (params: GetParams) => { value: string | undefined; error?: GetErrors }; - del: (params: { - key: string; - withBiometrics?: boolean; - biometricPrompt?: BiometricPromptOptions; - }) => void; + del: ( + params: ItemScope & { + key: string; + withBiometrics?: boolean; + biometricPrompt?: BiometricPromptOptions; + } + ) => void; } // Map the string-based ACCESSIBILITY enum to the integer codes the iOS @@ -193,7 +217,7 @@ function normalizeAccessibility( return mapped ?? 1; // default to kSecAttrAccessibleAfterFirstUnlock } -type NativeSetParams = { +type NativeSetParams = ItemScope & { key: string; value: string; accessibility?: number; @@ -201,7 +225,7 @@ type NativeSetParams = { biometricPrompt?: BiometricPromptOptions; }; -type NativeGetParams = { +type NativeGetParams = ItemScope & { key: string; accessibility?: number; withBiometrics?: boolean; From 1254aa6c0071e65c7c65593c0b2ab9753293f93a Mon Sep 17 00:00:00 2001 From: Takuya Matsuyama Date: Wed, 21 Oct 2026 11:51:55 +0900 Subject: [PATCH 2/5] fix(ios): Never run an unscoped keychain query PR #24 review flagged three cases where a query could match items beyond the intended one: a nil bundle identifier or unconvertible service left kSecAttrService out, an unconvertible accessGroup silently dropped the group filter, and an omitted accessGroup matched copies in every group. Fail when a scope attribute can't be resolved, and pin an omitted accessGroup to the app's default group, found once with a probe item. Note: inkdrop://note/s2IGKjCd --- README.md | 3 +- ios/bindings.mm | 115 +++++++++++++++++++++++++++++++++++++----------- src/index.tsx | 6 +-- 3 files changed, 94 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 69d9857..19cd27b 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,8 @@ del({ key: 'token', ...scope }); - Empty strings are treated as unset. - Use the same `service` and `accessGroup` for every operation on an item. An item written without them is not found by a `get` with a different `service`. -- Omitting `accessGroup` follows the Keychain API: `set` writes to the app's default access group, but `get`, `del` and the replace step of `set` match the key in **every** access group the app belongs to. So a `set` or `del` without `accessGroup` also removes a copy shared through another group, and a `get` without it returns whichever copy it finds first. Pass `accessGroup` whenever an item may exist in more than one group. +- Omitting `accessGroup` means the app's default access group (the first group in its access group list) for `set`, `get` and `del` alike. Unlike a raw Keychain query without an access group, it never reads, replaces or deletes a copy of the item stored in another group. +- If the default `service` or access group can't be determined (e.g. a nil bundle identifier, or a Keychain error before the first unlock), the operation fails rather than falling back to an unscoped query: `set` and `get` return an `error`, and `del` throws. - A non-string `service` or `accessGroup` is an error rather than silently using the default scope. - Both options are ignored on Android, where items are always private to the app. diff --git a/ios/bindings.mm b/ios/bindings.mm index d00b02b..948bb20 100644 --- a/ios/bindings.mm +++ b/ios/bindings.mm @@ -3,6 +3,7 @@ #import #import #import +#include namespace ops2 { namespace jsi = facebook::jsi; @@ -180,30 +181,78 @@ SecAccessControlRef createBioSecAccessControl(bool allowDeviceCredential, bool a nil); } -NSMutableDictionary *get_base_entry_dict(const std::string &key, const ItemScope &scope) { - NSMutableDictionary *queryDictionary = [[NSMutableDictionary alloc] init]; - [queryDictionary setObject:(__bridge id)kSecClassGenericPassword forKey:(__bridge id)kSecClass]; +std::string getSecurityErrorMessage(OSStatus status); - NSData *encodedIdentifier = [NSData dataWithBytes:key.data() length:key.length()]; - [queryDictionary setObject:encodedIdentifier forKey:(__bridge id)kSecAttrGeneric]; - [queryDictionary setObject:encodedIdentifier forKey:(__bridge id)kSecAttrAccount]; +static OSStatus getDefaultAccessGroup(NSString **out) { + static std::mutex mutex; + static NSString *cached = nil; + std::lock_guard lock(mutex); + if (cached != nil) { + *out = cached; + return noErr; + } + + NSDictionary *probe = @{ + (__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword, + (__bridge id)kSecAttrService: @"op-s2.default-access-group-probe", + (__bridge id)kSecAttrAccount: @"probe", + }; + // The probe's own queries can match every group: its service is private + // to op-s2. Clear a probe left behind by an interrupted earlier run. + SecItemDelete((__bridge CFDictionaryRef)probe); + + NSMutableDictionary *add = [probe mutableCopy]; + [add setObject:(__bridge id)kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly forKey:(__bridge id)kSecAttrAccessible]; + [add setObject:(__bridge id)kCFBooleanTrue forKey:(__bridge id)kSecReturnAttributes]; + CFTypeRef result = NULL; + OSStatus status = SecItemAdd((__bridge CFDictionaryRef)add, &result); + if (status != noErr) return status; + + NSDictionary *attributes = (__bridge_transfer NSDictionary *)result; + SecItemDelete((__bridge CFDictionaryRef)probe); + + NSString *group = [attributes objectForKey:(__bridge id)kSecAttrAccessGroup]; + if (group == nil) return errSecItemNotFound; + cached = group; + *out = cached; + return noErr; +} - // The bundle identifier can be nil in some test hosts; leave the service - // unset then rather than crash on a nil dictionary value. +NSMutableDictionary *get_base_entry_dict(const std::string &key, const ItemScope &scope, std::string &error) { NSString *service = scope.service.empty() ? [[NSBundle mainBundle] bundleIdentifier] : stringToNSString(scope.service); - if (service != nil) { - [queryDictionary setObject:service forKey:(__bridge id)kSecAttrService]; + if (service == nil) { + error = scope.service.empty() + ? "[op-s2] Could not determine the default service, pass service explicitly" + : "service is not valid UTF-8"; + return nil; } - // Without an access group, queries (including the delete in `set`) match - // every access group the app belongs to, as with the Keychain API itself. - NSString *accessGroup = scope.accessGroup.empty() ? nil : stringToNSString(scope.accessGroup); - if (accessGroup != nil) { - [queryDictionary setObject:accessGroup forKey:(__bridge id)kSecAttrAccessGroup]; + NSString *accessGroup = nil; + if (scope.accessGroup.empty()) { + OSStatus status = getDefaultAccessGroup(&accessGroup); + if (status != noErr) { + error = "[op-s2] Could not determine the default access group. " + getSecurityErrorMessage(status); + return nil; + } + } else { + accessGroup = stringToNSString(scope.accessGroup); + if (accessGroup == nil) { + error = "accessGroup is not valid UTF-8"; + return nil; + } } + NSMutableDictionary *queryDictionary = [[NSMutableDictionary alloc] init]; + [queryDictionary setObject:(__bridge id)kSecClassGenericPassword forKey:(__bridge id)kSecClass]; + + NSData *encodedIdentifier = [NSData dataWithBytes:key.data() length:key.length()]; + [queryDictionary setObject:encodedIdentifier forKey:(__bridge id)kSecAttrGeneric]; + [queryDictionary setObject:encodedIdentifier forKey:(__bridge id)kSecAttrAccount]; + [queryDictionary setObject:service forKey:(__bridge id)kSecAttrService]; + [queryDictionary setObject:accessGroup forKey:(__bridge id)kSecAttrAccessGroup]; + return queryDictionary; } @@ -225,13 +274,17 @@ CFStringRef getAccessibilityValue(int accessibility) { // authentication UI — otherwise deleting an entry that was previously stored // with biometric protection would pop the Face ID / passcode prompt even // though the caller explicitly opted out of biometrics. -void _delete(const std::string &key, const ItemScope &scope, bool withBiometrics) { - NSMutableDictionary *dict = get_base_entry_dict(key, scope); +// Returns an error message when the scope can't be resolved. +std::string _delete(const std::string &key, const ItemScope &scope, bool withBiometrics) { + std::string error; + NSMutableDictionary *dict = get_base_entry_dict(key, scope, error); + if (dict == nil) return error; [dict setObject:(__bridge id)kSecAttrSynchronizableAny forKey:(__bridge id)kSecAttrSynchronizable]; if (!withBiometrics) { [dict setObject:(__bridge id)kSecUseAuthenticationUIFail forKey:(__bridge id)kSecUseAuthenticationUI]; } SecItemDelete((__bridge CFDictionaryRef)dict); + return ""; } // Thread-safe dispatch for Keychain APIs that present authentication UI @@ -330,9 +383,12 @@ void install(jsi::Runtime &rt, std::shared_ptr jsCallInvoker } // Delete prior entries (both local and synced) to prevent errSecDuplicateItem - _delete(key, scope, withBiometrics); - - NSMutableDictionary *dict = get_base_entry_dict(key, scope); + scopeError = _delete(key, scope, withBiometrics); + NSMutableDictionary *dict = scopeError.empty() ? get_base_entry_dict(key, scope, scopeError) : nil; + if (dict == nil) { + res.setProperty(rt, "error", jsi::String::createFromUtf8(rt, scopeError)); + return res; + } if (withBiometrics) { SecAccessControlRef accessControl = createBioSecAccessControl(promptArgs.allowDeviceCredential, promptArgs.allowBiometricWeak); @@ -389,13 +445,17 @@ void install(jsi::Runtime &rt, std::shared_ptr jsCallInvoker throw jsi::JSError(rt, scopeError); } - NSMutableDictionary *dict = get_base_entry_dict(key, scope); + auto res = jsi::Object(rt); + + NSMutableDictionary *dict = get_base_entry_dict(key, scope, scopeError); + if (dict == nil) { + res.setProperty(rt, "error", jsi::String::createFromUtf8(rt, scopeError)); + return res; + } [dict setObject:(__bridge id)kSecMatchLimitOne forKey:(__bridge id)kSecMatchLimit]; [dict setObject:(__bridge id)kCFBooleanTrue forKey:(__bridge id)kSecReturnData]; - auto res = jsi::Object(rt); - if (withBiometrics) { BiometricsState biometricsState = getBiometricsState(promptArgs.allowDeviceCredential); @@ -450,18 +510,21 @@ void install(jsi::Runtime &rt, std::shared_ptr jsCallInvoker } std::string key = params.getProperty(rt, "key").asString(rt).utf8(rt); - + bool withBiometrics = false; if (params.hasProperty(rt, "withBiometrics")) { withBiometrics = params.getProperty(rt, "withBiometrics").asBool(); } - + ItemScope scope; std::string scopeError = extractItemScope(rt, params, scope); if (!scopeError.empty()) { throw jsi::JSError(rt, scopeError); } - _delete(key, scope, withBiometrics); + scopeError = _delete(key, scope, withBiometrics); + if (!scopeError.empty()) { + throw jsi::JSError(rt, scopeError); + } return {}; }); diff --git a/src/index.tsx b/src/index.tsx index 694ceb5..77f7eda 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -149,9 +149,9 @@ interface ItemScope { * group in its `keychain-access-groups` or app group entitlement, otherwise * the operation fails with `[op-s2] Missing entitlement`. * - * When omitted, `set` writes to the app's default access group, but `get`, - * `del` and the replace step of `set` match the key in every access group - * the app belongs to. + * When omitted, every operation uses the app's default access group (the + * first group in its access group list), so it never touches a copy of the + * item shared through another group. */ accessGroup?: string; } From d6f57a811cdea022728d105c7d726b6d122576f2 Mon Sep 17 00:00:00 2001 From: Takuya Matsuyama Date: Wed, 21 Oct 2026 16:49:09 +0900 Subject: [PATCH 3/5] fix(api): Report or throw consistently for scope errors PR #24 review flagged that get and del threw where set returned an error. Invalid params (a non-string service or accessGroup) now throw from get and del, the same way a missing key does, and set returns { error }. Runtime failures, such as a scope that can't be resolved, return { error } from all three, and del now returns an object on both platforms. Android type-checks service and accessGroup too, so a value rejected on iOS is no longer silently accepted there. Note: inkdrop://note/s2IGKjCd --- README.md | 6 +-- android/bindings.cpp | 25 ++++++++++++- example/src/tests/s2.ts | 83 ++++++++++++++++++++++++++--------------- ios/bindings.mm | 6 ++- src/index.tsx | 12 +++++- 5 files changed, 94 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 19cd27b..af437c5 100644 --- a/README.md +++ b/README.md @@ -133,9 +133,9 @@ del({ key: 'token', ...scope }); - Empty strings are treated as unset. - Use the same `service` and `accessGroup` for every operation on an item. An item written without them is not found by a `get` with a different `service`. - Omitting `accessGroup` means the app's default access group (the first group in its access group list) for `set`, `get` and `del` alike. Unlike a raw Keychain query without an access group, it never reads, replaces or deletes a copy of the item stored in another group. -- If the default `service` or access group can't be determined (e.g. a nil bundle identifier, or a Keychain error before the first unlock), the operation fails rather than falling back to an unscoped query: `set` and `get` return an `error`, and `del` throws. -- A non-string `service` or `accessGroup` is an error rather than silently using the default scope. -- Both options are ignored on Android, where items are always private to the app. +- If the default `service` or access group can't be determined (e.g. a nil bundle identifier, or a Keychain error before the first unlock), the operation returns an `error` rather than falling back to an unscoped query. +- A non-string `service` or `accessGroup` is rejected rather than silently using the default scope, on both platforms: `set` returns an `error`, and `get` and `del` throw, as they do for a missing `key`. +- Otherwise both options are ignored on Android, where items are always private to the app. ## Secure Enclave diff --git a/android/bindings.cpp b/android/bindings.cpp index ba5d341..56ee0cb 100644 --- a/android/bindings.cpp +++ b/android/bindings.cpp @@ -52,6 +52,16 @@ static BiometricPromptOptions extractBiometricPromptOptions(jsi::Runtime &rt, co return options; } +static const char *validateItemScope(jsi::Runtime &rt, const jsi::Object ¶ms) { + auto isInvalid = [&](const char *name) { + jsi::Value v = params.getProperty(rt, name); + return !v.isNull() && !v.isUndefined() && !v.isString(); + }; + if (isInvalid("service")) return "service property is not a string"; + if (isInvalid("accessGroup")) return "accessGroup property is not a string"; + return nullptr; +} + void install( jsi::Runtime &rt, std::function setFn, @@ -92,6 +102,11 @@ void install( return res; } + if (const char *scopeError = validateItemScope(rt, params)) { + res.setProperty(rt, "error", scopeError); + return res; + } + std::string key = params.getProperty(rt, "key").asString(rt).utf8(rt); std::string val = params.getProperty(rt, "value").asString(rt).utf8(rt); @@ -140,6 +155,10 @@ void install( BiometricPromptOptions promptOptions = extractBiometricPromptOptions(rt, params); + if (const char *scopeError = validateItemScope(rt, params)) { + throw jsi::JSError(rt, scopeError); + } + auto res = jsi::Object(rt); try { @@ -185,9 +204,13 @@ void install( BiometricPromptOptions promptOptions = extractBiometricPromptOptions(rt, params); + if (const char *scopeError = validateItemScope(rt, params)) { + throw jsi::JSError(rt, scopeError); + } + _del(key.c_str(), withBiometrics, promptOptions); - return {}; + return jsi::Object(rt); }); jsi::Object module = jsi::Object(rt); diff --git a/example/src/tests/s2.ts b/example/src/tests/s2.ts index c0ca652..270ac97 100644 --- a/example/src/tests/s2.ts +++ b/example/src/tests/s2.ts @@ -193,6 +193,58 @@ describe('accessibility option (iOS)', () => { }); }); +describe('service / accessGroup validation', () => { + it('rejects a non-string service or accessGroup', () => { + const key = 'key_service_invalid'; + + const { error: serviceError } = set({ + key, + value: 'myValue', + // @ts-ignore + service: 123, + }); + expect(serviceError).toEqual('service property is not a string'); + + const { error: groupError } = set({ + key, + value: 'myValue', + // @ts-ignore + accessGroup: 123, + }); + expect(groupError).toEqual('accessGroup property is not a string'); + + // Like a missing key, an invalid param makes get/del throw. + let getError: unknown; + try { + // @ts-ignore + get({ key, service: 123 }); + } catch (e) { + getError = e; + } + expect(String(getError)).toContain('service property is not a string'); + + let delError: unknown; + try { + // @ts-ignore + del({ key, accessGroup: 123 }); + } catch (e) { + delError = e; + } + expect(String(delError)).toContain('accessGroup property is not a string'); + + expect(get({ key }).error).toEqual('[op-s2] Item not found'); + }); + + it('returns an object without error from a valid del', () => { + const key = 'key_del_result'; + + set({ key, value: 'myValue' }); + const { error } = del({ key }); + expect(error).toBe(undefined); + expect(get({ key }).error).toEqual('[op-s2] Item not found'); + }); +}); + if (Platform.OS === 'ios') { describe('service / accessGroup options (iOS)', () => { it('set/get with a custom service', () => { @@ -279,37 +331,6 @@ if (Platform.OS === 'ios') { // Clean up in case the write unexpectedly succeeded on this setup. del({ key, accessGroup: 'group.op.s2.not-entitled' }); }); - - it('rejects a non-string service or accessGroup', () => { - const key = 'key_service_invalid'; - - const { error: serviceError } = set({ - key, - value: 'myValue', - // @ts-ignore - service: 123, - }); - expect(serviceError).toEqual('service property is not a string'); - - const { error: groupError } = set({ - key, - value: 'myValue', - // @ts-ignore - accessGroup: 123, - }); - expect(groupError).toEqual('accessGroup property is not a string'); - - let getError: unknown; - try { - // @ts-ignore - get({ key, service: 123 }); - } catch (e) { - getError = e; - } - expect(String(getError)).toContain('service property is not a string'); - - expect(get({ key }).error).toEqual('[op-s2] Item not found'); - }); }); } else { describe('service / accessGroup options (Android)', () => { diff --git a/ios/bindings.mm b/ios/bindings.mm index 948bb20..3998a3f 100644 --- a/ios/bindings.mm +++ b/ios/bindings.mm @@ -521,11 +521,13 @@ void install(jsi::Runtime &rt, std::shared_ptr jsCallInvoker if (!scopeError.empty()) { throw jsi::JSError(rt, scopeError); } + + auto res = jsi::Object(rt); scopeError = _delete(key, scope, withBiometrics); if (!scopeError.empty()) { - throw jsi::JSError(rt, scopeError); + res.setProperty(rt, "error", jsi::String::createFromUtf8(rt, scopeError)); } - return {}; + return res; }); jsi::Object module = jsi::Object(rt); diff --git a/src/index.tsx b/src/index.tsx index 77f7eda..9ca0bf7 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -117,6 +117,12 @@ type SharedOperationErrors = | `[op-s2] Security error code: ${string}. Look up code error at https://www.osstatus.com/` | `op-s2 could not set value, error code: ${string}`; +type ScopeErrors = + | 'service is not valid UTF-8' + | 'accessGroup is not valid UTF-8' + | '[op-s2] Could not determine the default service, pass service explicitly' + | `[op-s2] Could not determine the default access group. ${string}`; + type SetErrors = | 'Params object is missing' | 'Params is not an object' @@ -126,6 +132,7 @@ type SetErrors = | 'service property is not a string' | 'accessGroup property is not a string' | '[op-s2] Could not set value, duplicate item' + | ScopeErrors | SharedOperationErrors; type GetErrors = @@ -134,8 +141,11 @@ type GetErrors = | 'key property is missing' | '[op-s2] Item not found' | 'Biometrics not available' + | ScopeErrors | SharedOperationErrors; +type DelErrors = ScopeErrors; + interface ItemScope { /** * iOS only: the keychain service (`kSecAttrService`) the item is stored @@ -191,7 +201,7 @@ interface OPS2 { withBiometrics?: boolean; biometricPrompt?: BiometricPromptOptions; } - ) => void; + ) => { error?: DelErrors }; } // Map the string-based ACCESSIBILITY enum to the integer codes the iOS From aa43a12d45e4758217cb6f57b4ca278a962e0c24 Mon Sep 17 00:00:00 2001 From: Takuya Matsuyama Date: Wed, 21 Oct 2026 16:51:44 +0900 Subject: [PATCH 4/5] refactor(api): Export ItemScope and drop redundant property check ItemScope is exported so callers can type the shared scope object they pass to set, get and del. readOptionalString no longer calls hasProperty first, since getProperty already yields undefined for a missing property. Note: inkdrop://note/s2IGKjCd --- ios/bindings.mm | 1 - src/index.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ios/bindings.mm b/ios/bindings.mm index 3998a3f..6e4d7b1 100644 --- a/ios/bindings.mm +++ b/ios/bindings.mm @@ -48,7 +48,6 @@ static void zeroizeString(std::string &str) { // present but neither a string nor null/undefined; `out` is left untouched // unless a string is found. static bool readOptionalString(jsi::Runtime &rt, const jsi::Object &obj, const char *name, std::string &out) { - if (!obj.hasProperty(rt, name)) return true; jsi::Value v = obj.getProperty(rt, name); if (v.isNull() || v.isUndefined()) return true; if (!v.isString()) return false; diff --git a/src/index.tsx b/src/index.tsx index 9ca0bf7..d3e974b 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -146,7 +146,7 @@ type GetErrors = type DelErrors = ScopeErrors; -interface ItemScope { +export interface ItemScope { /** * iOS only: the keychain service (`kSecAttrService`) the item is stored * under. Defaults to the app's bundle identifier. Use the same value for From 3baa872b9edfd4f118470a12a9e7774fea4d3254 Mon Sep 17 00:00:00 2001 From: Takuya Matsuyama Date: Fri, 23 Oct 2026 12:07:23 +0900 Subject: [PATCH 5/5] refactor(ios): Drop implicit default access group discovery Review asked for explicit access groups: an omitted accessGroup now leaves kSecAttrAccessGroup out of the query, as before, and the probe item that discovered the default group is removed. The discovered group could change when entitlements changed, hiding items saved earlier, and the probe failed before the first unlock. Invalid or unconvertible service and accessGroup values still return an error. Note: inkdrop://note/s2IGKjCd --- README.md | 27 ++++++++++++++------- ios/bindings.mm | 62 +++++++------------------------------------------ src/index.tsx | 8 +++---- 3 files changed, 30 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index af437c5..f963ee5 100644 --- a/README.md +++ b/README.md @@ -112,11 +112,23 @@ If you pass `withBiometrics: true` without a `biometricPrompt` object at all, th ### iOS Service and Access Group -On iOS every item is identified by its `key` plus a keychain service and access group. Pass `service` and/or `accessGroup` to `set`, `get` and `del` to control them, e.g. to share an item with an app extension: +On iOS, an item is identified by its `key`, a keychain service and an access group. `service` and `accessGroup` are both optional. If omitted, the service is the app's bundle identifier and no access group is set. + +| Option | Explanation | Default | +| ------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------- | +| `service` | The keychain service (`kSecAttrService`). | The app's bundle identifier | +| `accessGroup` | The keychain access group (`kSecAttrAccessGroup`). The app (and any extension reading the item) must list it in its `keychain-access-groups` entitlement, or use an app group ID (`group.*`) it has. Otherwise the call fails with `[op-s2] Missing entitlement`. | None (see below) | + +#### Sharing an item with an app extension + +Pass both options explicitly, with the same values in the app and in the extension: ```ts const scope = { + // The extension has its own bundle identifier, so the default service + // would differ between the two. Pin it to one value. service: 'com.example.app', + // An app group (or keychain access group) both targets are entitled to. accessGroup: 'group.com.example.app', }; @@ -125,17 +137,16 @@ const { value } = get({ key: 'token', ...scope }); del({ key: 'token', ...scope }); ``` -| Option | Explanation | Default | -| ------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------- | -| `service` | The keychain service (`kSecAttrService`). | The app's bundle identifier | -| `accessGroup` | The keychain access group (`kSecAttrAccessGroup`). The app (and any extension reading the item) must list it in its `keychain-access-groups` entitlement, or use an app group ID (`group.*`) it has. Otherwise the call fails with `[op-s2] Missing entitlement`. | The app's default group | +#### Notes - Empty strings are treated as unset. -- Use the same `service` and `accessGroup` for every operation on an item. An item written without them is not found by a `get` with a different `service`. -- Omitting `accessGroup` means the app's default access group (the first group in its access group list) for `set`, `get` and `del` alike. Unlike a raw Keychain query without an access group, it never reads, replaces or deletes a copy of the item stored in another group. -- If the default `service` or access group can't be determined (e.g. a nil bundle identifier, or a Keychain error before the first unlock), the operation returns an `error` rather than falling back to an unscoped query. +- If you pass `service` or `accessGroup` for an item, pass the same values in every `set`, `get` and `del` call for it: + - With a different `service`, the item isn't found. + - Without `accessGroup`, the call isn't limited to one group. A `set` or `del` then also deletes the copy stored in your shared group, and a `get` may return a copy from another group. +- If the default `service` can't be determined (a nil bundle identifier), or a given `service` or `accessGroup` can't be converted, the operation returns an `error` rather than running a query that matches every service or group. - A non-string `service` or `accessGroup` is rejected rather than silently using the default scope, on both platforms: `set` returns an `error`, and `get` and `del` throw, as they do for a missing `key`. - Otherwise both options are ignored on Android, where items are always private to the app. +- If you've never passed either option, upgrading changes nothing: items saved by earlier versions of op-s2 are still found. ## Secure Enclave diff --git a/ios/bindings.mm b/ios/bindings.mm index 6e4d7b1..08a4c97 100644 --- a/ios/bindings.mm +++ b/ios/bindings.mm @@ -3,7 +3,6 @@ #import #import #import -#include namespace ops2 { namespace jsi = facebook::jsi; @@ -18,8 +17,7 @@ // Identifies which keychain item an operation targets beyond its key. // Empty strings mean "unset": the service falls back to the bundle identifier -// and no access group is set, which matches the behavior before these options -// existed. +// and no access group is set. struct ItemScope { std::string service; std::string accessGroup; @@ -180,43 +178,6 @@ SecAccessControlRef createBioSecAccessControl(bool allowDeviceCredential, bool a nil); } -std::string getSecurityErrorMessage(OSStatus status); - -static OSStatus getDefaultAccessGroup(NSString **out) { - static std::mutex mutex; - static NSString *cached = nil; - std::lock_guard lock(mutex); - if (cached != nil) { - *out = cached; - return noErr; - } - - NSDictionary *probe = @{ - (__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword, - (__bridge id)kSecAttrService: @"op-s2.default-access-group-probe", - (__bridge id)kSecAttrAccount: @"probe", - }; - // The probe's own queries can match every group: its service is private - // to op-s2. Clear a probe left behind by an interrupted earlier run. - SecItemDelete((__bridge CFDictionaryRef)probe); - - NSMutableDictionary *add = [probe mutableCopy]; - [add setObject:(__bridge id)kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly forKey:(__bridge id)kSecAttrAccessible]; - [add setObject:(__bridge id)kCFBooleanTrue forKey:(__bridge id)kSecReturnAttributes]; - CFTypeRef result = NULL; - OSStatus status = SecItemAdd((__bridge CFDictionaryRef)add, &result); - if (status != noErr) return status; - - NSDictionary *attributes = (__bridge_transfer NSDictionary *)result; - SecItemDelete((__bridge CFDictionaryRef)probe); - - NSString *group = [attributes objectForKey:(__bridge id)kSecAttrAccessGroup]; - if (group == nil) return errSecItemNotFound; - cached = group; - *out = cached; - return noErr; -} - NSMutableDictionary *get_base_entry_dict(const std::string &key, const ItemScope &scope, std::string &error) { NSString *service = scope.service.empty() ? [[NSBundle mainBundle] bundleIdentifier] @@ -228,19 +189,10 @@ static OSStatus getDefaultAccessGroup(NSString **out) { return nil; } - NSString *accessGroup = nil; - if (scope.accessGroup.empty()) { - OSStatus status = getDefaultAccessGroup(&accessGroup); - if (status != noErr) { - error = "[op-s2] Could not determine the default access group. " + getSecurityErrorMessage(status); - return nil; - } - } else { - accessGroup = stringToNSString(scope.accessGroup); - if (accessGroup == nil) { - error = "accessGroup is not valid UTF-8"; - return nil; - } + NSString *accessGroup = scope.accessGroup.empty() ? nil : stringToNSString(scope.accessGroup); + if (!scope.accessGroup.empty() && accessGroup == nil) { + error = "accessGroup is not valid UTF-8"; + return nil; } NSMutableDictionary *queryDictionary = [[NSMutableDictionary alloc] init]; @@ -250,7 +202,9 @@ static OSStatus getDefaultAccessGroup(NSString **out) { [queryDictionary setObject:encodedIdentifier forKey:(__bridge id)kSecAttrGeneric]; [queryDictionary setObject:encodedIdentifier forKey:(__bridge id)kSecAttrAccount]; [queryDictionary setObject:service forKey:(__bridge id)kSecAttrService]; - [queryDictionary setObject:accessGroup forKey:(__bridge id)kSecAttrAccessGroup]; + if (accessGroup != nil) { + [queryDictionary setObject:accessGroup forKey:(__bridge id)kSecAttrAccessGroup]; + } return queryDictionary; } diff --git a/src/index.tsx b/src/index.tsx index d3e974b..1c6bd34 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -120,8 +120,7 @@ type SharedOperationErrors = type ScopeErrors = | 'service is not valid UTF-8' | 'accessGroup is not valid UTF-8' - | '[op-s2] Could not determine the default service, pass service explicitly' - | `[op-s2] Could not determine the default access group. ${string}`; + | '[op-s2] Could not determine the default service, pass service explicitly'; type SetErrors = | 'Params object is missing' @@ -159,9 +158,8 @@ export interface ItemScope { * group in its `keychain-access-groups` or app group entitlement, otherwise * the operation fails with `[op-s2] Missing entitlement`. * - * When omitted, every operation uses the app's default access group (the - * first group in its access group list), so it never touches a copy of the - * item shared through another group. + * When omitted, no access group is set: `set` writes to the app's default + * access group. */ accessGroup?: string; }