[craftzdog]

Hi @ospfranco !

I recently came across your library, and it looks great for storing access keys for my app.
I found that it doesn't support service or accessGroup for iOS yet, so I added them. It allows apps to access the keychain items from share extensions.
Here is a description my agent wrote:

Add service and accessGroup options (iOS)

Adds optional service and accessGroup params to set, get and del. With them, a Keychain item can be shared with an app extension (for example a share extension) through a Keychain access group or an app group.

const scope = { service: 'com.example.app', accessGroup: 'group.com.example.app' };
set({ key: 'token', value: 'secret', ...scope });
get({ key: 'token', ...scope });
del({ key: 'token', ...scope });

Changes

• iOS (ios/bindings.mm): get_base_entry_dict now takes an ItemScope.
  • service sets kSecAttrService and defaults to the bundle identifier, as before.
  • accessGroup sets kSecAttrAccessGroup only when given.
  • The scope is applied to set (including its delete-before-add step), get and del.
  • Empty strings are treated as unset.
  • Non-string values are an error: set returns { error }, and get/del throw.
• Android: both options are ignored, the same way accessibility is, because items are always private to the app.
• Types: added an ItemScope interface and used it in the set/get/del params and the native param types.
• README: new "iOS Service and Access Group" section. It documents that when accessGroup is omitted, get, del and the replace step in set match the key in every access group the app belongs to, the same as the Keychain API itself.
• Hardening: stringToNSString now builds from the byte length, so an embedded NUL no longer truncates the value. A nil bundle identifier no longer crashes.

Backward compatibility

If you pass neither option, nothing changes: same service, default access group, and existing items are still readable.

Tests

New cases in example/src/tests/s2.ts:

• iOS:
  • set/get with a custom service.
  • Isolation between services.
  • The default service is the bundle ID.
  • Empty strings are treated as unset.
  • An unentitled access group returns [op-s2] Missing entitlement.
  • Non-string values are rejected.
• Android: both options are ignored.

All tests pass on an iOS 26.5 simulator and an Android API 36 emulator, both Release builds.

Note

on the iOS 27 simulator, the example app crashes at launch in _UIApplicationEvaluateRuntimeIssueForNoSceneLifecycleAdoption, because it hasn't adopted the UIScene lifecycle. This is unrelated to this PR.

---

Feel free to discard the PR and implement it in your own way 🙏

[ospfranco]

Hi Takuya, I didn't get the time today to review it myself, but an AI review found the following:

---

I found 7 issues in this PR. The first three are correctness bugs in the iOS keychain scoping, and all three can silently delete or expose items. The rest are inconsistencies and cleanup.

Correctness

1. The query matches every service when the service can't be resolved (ios/bindings.mm:194). If bundleIdentifier is nil (test host, some app extensions) or service fails UTF-8 conversion, the query is built without kSecAttrService. A set({key:'token'}) then deletes every item named 'token' in all services, and get returns whichever copy the keychain finds first. The old code crashed here; this should return an error instead.

2. An access group that fails conversion is ignored silently (ios/bindings.mm:202). If stringToNSString(accessGroup) returns nil for a non-empty value, the access-group filter is skipped. del and the replace step in set then act on every access group the app belongs to. Same fix: return an error.

3. A set without accessGroup deletes copies shared with other targets (ios/bindings.mm:333). The delete-before-add step runs without an access-group filter. If an app stored token in group.x for its share extension, a later plain set({key:'token'}) deletes that shared copy and writes the new one to the default group. The README documents this, but it is still silent data loss. Using SecItemUpdate, and falling back to SecItemAdd on errSecItemNotFound, would fix it at the root.

Inconsistencies

4. get/del throw where set returns an error (ios/bindings.mm:389). For an invalid service/accessGroup, get and del throw a JSError while set returns { error }. Code that follows the documented const { value, error } = get(...) pattern gets an uncaught exception. GetErrors in src/index.tsx has no entry for this case either.

5. Android ignores these fields (README.md:136). The README and SetErrors say a non-string service/accessGroup is an error, but android/bindings.cpp never reads them. service: 123 returns an error on iOS and succeeds on Android. Either validate on Android too or document these fields as iOS-only.

6. ItemScope is not exported (src/index.tsx:139). The README tells users to reuse one scope object across set/get/del, but they can't import the type for it.

Cleanup

7. Each optional field is looked up twice (ios/bindings.mm:49). readOptionalString calls hasProperty and then getProperty. getProperty alone already returns undefined for a missing field, so the first call can go.

I'd block on 1–3 before merging, since each one can delete keychain data silently. 4 and 5 are worth fixing in this PR too.

---

I will take a more in-depth look tomorrow, but you can review these and see if they are valid or not?

[craftzdog]

Sorry to bug you on your trip and thanks for checking!

---

1–3: No more unscoped keychain queries (1254aa6)

• 1 & 2: get_base_entry_dict now returns an error instead of leaving out kSecAttrService or kSecAttrAccessGroup when it can't resolve them (a nil bundle identifier, or a value that fails UTF-8 conversion).
• 3: I went a different way than SecItemUpdate. An update without kSecAttrAccessGroup also matches every group, so it would overwrite the shared copies rather than delete them. It also can't easily switch an item's biometric access control, which delete-then-add handles today.
  • Instead, an omitted accessGroup now means the app's default access group, for set, get and del alike. That's the group SecItemAdd already writes to, so items written by earlier versions are still found, with the caveat below.
  • The default group is found once, by adding a throwaway probe item and reading its kSecAttrAccessGroup back. The result is cached. If the probe fails (e.g. before the first unlock), the call returns an error and the next call retries; it never falls back to an unscoped query.
  • The example app has a single access group, so CI can't cover the cross-group case. I'm verifying it in an app that has two.

Open question on 3: compatibility for existing apps

Pinning to the default group has two side effects for apps that don't pass accessGroup. Neither exists on main or in the first commit of this PR.

1. When the default group changes, existing items are no longer found. The default group is the first one in the app's access group list. Adding a keychain-access-groups entitlement changes it (adding an app group doesn't, since app groups come last). Items written before stay in the old group, and calls without accessGroup stop finding them. That hits exactly the apps adopting this feature, unless they migrate the items.
2. Reads can fail before the first unlock after a reboot. The probe item uses kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, so the probe fails until the first unlock. Until then, every call without accessGroup returns an error, including a get of an item stored with ALWAYS / ALWAYS_THIS_DEVICE_ONLY that main could read. This follows from the Keychain rules; I haven't reproduced it on a device. It recovers after the first unlock, since only successful probes are cached.

Options I see:

• For 2: create the probe item with kSecAttrAccessibleAlwaysThisDeviceOnly, so the probe works before the first unlock. It's deprecated, but still works.
• For 1:
  • (a) Keep it and document it as a migration step: read with the old group, write with the new one.
  • (b) Pin the default group only where it prevents data loss: the replace step in set, and del. get without accessGroup would search every group again, as on main. Nothing gets deleted by accident, but get goes back to returning whichever copy the Keychain finds first.

I lean towards (b) plus the fix for 2, but I'd like your take before changing it.

4–5: Error handling (d6f57a8)

• 4: I aligned these with how get and del already handle a missing key: invalid params throw, and runtime failures return { error }.
  • So a non-string service or accessGroup throws from get and del, while a scope that can't be resolved returns { error }.
  • set keeps returning { error } for everything, as it already did.
  • del now returns an object on both platforms, so the README's const { error } = del(...) example works. Before, it returned undefined.
  • GetErrors and the new DelErrors list the errors they can return.
  • Happy to switch to returning { error } for invalid params too if you prefer, but then the existing key checks should probably change as well, perhaps in a separate PR.
• 5: Android now type-checks service and accessGroup with the same messages as iOS. Valid values are still ignored there, and the README says so.

6–7: Cleanup (aa43a12)

• 6: ItemScope is exported.
• 7: readOptionalString uses getProperty alone.

The example app's test suite passes on the iOS 26.5 simulator and the Android API 36 emulator.

[ospfranco]

Hi, I took a quick look. Some thoughts:

• The wording/example on the README makes it confusing to show that 'service' is optional and is now auto discovered to match the bundle id. Some clarification and a more explicit example would be good. Also, might break for existing users?
• I don't know if auto discovering the group is desired... I would assume one should always want explicit group permission when saving sensitive data. Not 100% sure about this though, but I always go for explicit APIs as implicit internal behavior is hard to debug if something goes wrong. If the group is there I always now I'm writing a value that can be read/written by the group. Therefore I would get rid of implicit group discovery.
• Same goes for the auto discovering of the service, but here I'm just worried if it will break compatibility with the existing users of the library. Not sure what is the default behavior is service is missing, it should default to the bundle id no?
• I agree with the error throwing about the params. Everything should be returned in the error field instead of throwing exceptions, including the param errors. This was an oversight on my part. Can you take care of it in a following PR and will release it in a single go.

[craftzdog]

• README: Ok, updated the section. Added comments to the example code, and clarified the notes.
• Access group discovery: Agreed. Developers should be aware of the access group when using it. When accessGroup is omitted, no access group is set on the query, same as before this PR. Also mentioned it in the README. No breaking changes for the existing apps.
• service: It isn't discovered. When omitted, it defaults to the bundle identifier, which is exactly the value op-s2 has always hardcoded. So nothing changes for existing users who never pass either option; their items are still found.
• Error handling: ok!