Skip to content

[MOO-2391] Improve SessionCookieStore#53

Closed
MxKevinBeqo wants to merge 5 commits into
mx/11.12.xfrom
moo/MOO-2391-improve-enc-storage
Closed

[MOO-2391] Improve SessionCookieStore#53
MxKevinBeqo wants to merge 5 commits into
mx/11.12.xfrom
moo/MOO-2391-improve-enc-storage

Conversation

@MxKevinBeqo

Copy link
Copy Markdown
Contributor

Problem

Some users on iOS were seeing a system "Enter password" dialog appear over the splash screen during app startup. The root cause was a combination of two issues:

  1. Oversized keychain item. SessionCookieStore serialised all no-expiry session cookies into a single keychain item. For users with many or large session cookies the blob could exceed iOS's reasonable-size threshold (~400 KB), producing an item that required user interaction to decrypt.

What changed

ios/Modules/NativeCookieModule/SessionCookieStore.swift

Chunked keychain storage (set / get)

Instead of writing the entire cookie blob as a single keychain item, blobs larger than 64 KB are now split into sequential 64 KB items. Blobs that fit within 64 KB continue to use the existing single-item format, preserving full backward compatibility with items written by older builds.

A commit marker item (_chunkcount) is written last, after every chunk has been stored successfully. Its presence is the guarantee that all chunks were committed. If the process is killed mid-write the marker is absent, and the incomplete set is discarded cleanly on the next read.

If any chunk write fails, all chunks already written are deleted before returning, so restore() never reassembles a partial cookie set.

On read, the code checks for the commit marker first. If it is present the chunks are reassembled in order. If it is absent the code falls back to reading the legacy bare key, so items written by older builds are transparently consumed on first launch after upgrade.

There is no longer any ceiling that can cause persistence to fail due to blob size.

Self-healing reads (get)

All keychain read queries now carry kSecUseAuthenticationUIFail. This causes the keychain to return errSecInteractionNotAllowed rather than showing an authentication dialog if a stored item would require user interaction.

On errSecInteractionNotAllowed the item is deleted and nil is returned, breaking any retry loop.

Any other unreadable state (including deserialization failure of a corrupt or legacy blob) also deletes the item and returns nil. This guarantees that a bad item is cleaned up during restore() on the first launch after the fix is deployed; the user loses the bridged session once and re-authenticates once.

clear()

Updated to remove both the legacy single-item key and all chunk keys, so a clearAll() call from JavaScript leaves no orphaned chunk items in the keychain.

persist(completion:) — DEBUG only

A #if DEBUG-only overload of persist that accepts a completion closure. The public persist() signature is unchanged. The overload exists solely to let the test bridge resolve its Promise after the keychain write finishes rather than racing against it.

Covered scenarios:

  • Small-blob round-trip: persist and restore cookies whose serialised size is below the chunk threshold; verify no commit marker is created; verify the keychain is empty after restore.
  • Large-blob round-trip: persist and restore cookies whose serialised size exceeds 64 KB; verify the commit marker is present with a count greater than 1; verify the marker and all chunk items are gone after restore.
  • Format transitions: overwriting a chunked write with a single-item write leaves no orphaned chunk keys, and vice versa.
  • clearAll: removes cookies and any chunk marker after both small and large persists; does not throw on an empty store.

Security notes

The five new methods added to NativeCookieModule are protected by #if DEBUG at every layer — Swift, Objective-C bridge, and they are exported under a clearly named NativeCookieTestHelpers TypeScript object. They are not reachable from a production (Release configuration) binary. The CI harness build uses -configuration Debug, so they are available during test runs.

@MxKevinBeqo MxKevinBeqo self-assigned this Jul 9, 2026
@MxKevinBeqo MxKevinBeqo added the enhancement New feature or request label Jul 9, 2026
@MxKevinBeqo MxKevinBeqo changed the base branch from main to mx/11.12.x July 9, 2026 13:33
@MxKevinBeqo MxKevinBeqo force-pushed the moo/MOO-2391-improve-enc-storage branch from dccc1e6 to 69c6314 Compare July 9, 2026 13:39
@MxKevinBeqo

Copy link
Copy Markdown
Contributor Author

Closing as not required for 11.12.

@MxKevinBeqo MxKevinBeqo closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant