Built a secure encrypted storage layer for sensitive mobile data

SECURITY.md

@@ -153,15 +153,42 @@ externally connected wallet.
 
 Wallet-linked state and authentication state are never persisted in plaintext AsyncStorage.
 The `wallet-storage`, `session-storage`, `sync-storage`, and
-`guildpass:reconciliation:v1` Zustand slices use
+`guildpass:reconciliation:v1` Zustand slices, push tokens, and cached
+attestation revocation registries use
 `expo-secure-store`, backed by the iOS Keychain and Android Keystore, with the
 iOS accessibility level set to `WHEN_UNLOCKED_THIS_DEVICE_ONLY`. This includes:
 
 - the connected wallet address and connection status;
 - the connector kind (manual entry, WalletConnect, or another provider); and
 - session tokens, expiry timestamps, and their associated wallet address;
 - wallet-scoped sync metadata and reconciliation sequence numbers; and
-- cached wallet role attestations, issuer verification keys, and their indexes.
+- cached wallet role attestations, issuer verification keys, and their indexes;
+- push-notification delivery tokens and cached issuer revocation registries.
+
+Secure persistence errors are surfaced to callers after any stale AsyncStorage
+copy has been removed. Feature code must not treat a failed secure write as a
+successful save.
+
+### Approved storage practices
+
+Classify a value before persisting it:
+
+- **Sensitive:** authentication/session state, provider tokens, wallet-linked
+  identifiers, signed credentials or attestations, access decisions, sync
+  metadata, and security settings. Use `migratingSecureStorage` from
+  `src/lib/storage` for small state. Large offline datasets must use the
+  AES-GCM encrypted persister with its device-bound key.
+- **Non-sensitive:** presentation-only preferences that cannot identify a user
+  or affect authentication or authorization. These may use `asyncStorage` from
+  `src/lib/storage`.
+- **Ephemeral:** private keys, seed phrases, raw signing material, one-time
+  challenges, and transient access scans. Keep these in provider/native custody
+  or memory; do not persist them in application storage.
+
+Feature modules must not import `AsyncStorage` or `expo-secure-store` directly.
+Native storage access belongs in the centralized storage/encrypted-persister
+modules. New legacy key families must also be registered in the startup
+migration and covered by a plaintext-removal test.
 
 ### Upgrade migration
 

docs/threat-model.md

@@ -44,9 +44,9 @@ The hardening layer consists of two controls:
 
 | Asset                                  | Sensitivity  | Storage                                       |
 | -------------------------------------- | ------------ | --------------------------------------------- |
-| Wallet address / identifier            | Medium       | AsyncStorage (public data)                    |
+| Wallet address / identifier            | Medium       | `expo-secure-store`                           |
 | Access-control decisions (QR payloads) | High         | In-memory only                                |
-| Signed attestations / proofs           | High         | Transient; not persisted                      |
+| Signed attestations / proofs           | High         | `expo-secure-store` when cached for offline use |
 | Future: embedded private keys          | **Critical** | Planned: `expo-secure-store` / Secure Enclave |
 | Session / auth tokens                  | High         | `expo-secure-store`                           |
 

src/lib/mutationQueue.ts

@@ -94,6 +94,9 @@ class MutationQueueManager {
 
       const envelope = JSON.parse(stored) as Partial<EncryptedQueueEnvelope>;
       if (envelope.v !== ENVELOPE_MAGIC || !envelope.n || !envelope.t || !envelope.c) {
+        // Older releases could leave queue payloads as plaintext JSON. Never
+        // hydrate or retain an unrecognised value containing mutation payloads.
+        await AsyncStorage.removeItem(MUTATION_QUEUE_STORAGE_KEY);
         this.queueCache = [];
         this.notifyListeners();
         return this.queueCache;
@@ -123,6 +126,9 @@ class MutationQueueManager {
       this.queueCache = Array.isArray(decrypted) ? decrypted : [];
     } catch (e) {
       console.error("[MutationQueue] Error loading queue", e);
+      // A malformed legacy/corrupt entry may contain plaintext. Encrypted
+      // envelopes are authenticated, so removing either form is fail-closed.
+      await AsyncStorage.removeItem(MUTATION_QUEUE_STORAGE_KEY).catch(() => {});
       this.queueCache = [];
     }
 

src/lib/storage/index.ts

@@ -26,14 +26,18 @@ const SENSITIVE_LEGACY_KEYS = new Set([
   "wallet-storage",
   "session-storage",
   "sync-storage",
+  "biometric-storage",
   "guildpass:reconciliation:v1",
+  "guildpass:push-notifications:v1",
   "guildpass:issuer-keys-index",
+  "guildpass:attestation-key-registry-index",
 ]);
 
 const SENSITIVE_LEGACY_KEY_PREFIXES = [
   "guildpass:attestations:",
   "guildpass:attestation-index",
   "guildpass:issuer-keys:",
+  "guildpass:attestation-key-registry:",
 ];
 
 function getLegacyEncodedSecureStorageKey(name: string): string {
@@ -330,13 +334,16 @@ export const migratingSecureStorage: StateStorage = {
   },
 
   setItem: async (name: string, value: string): Promise<void> => {
+    let writeError: unknown;
     try {
       await rawSecureStorage.setItem(name, value);
     } catch (error) {
       console.error(`Error writing sensitive data to SecureStore: ${name}`, error);
+      writeError = error;
     }
     // Never retain a fallback or stale plaintext copy.
     await clearLegacyOrFailClosed(name);
+    if (writeError) throw writeError;
   },
 
   removeItem: async (name: string): Promise<void> => {

tests/mutationQueue.test.ts

@@ -32,6 +32,26 @@ describe("MutationQueue", () => {
     expect(queue).toHaveLength(0);
   });
 
+  it("fails closed and removes a legacy plaintext mutation queue", async () => {
+    vi.mocked(AsyncStorage.getItem).mockResolvedValueOnce(
+      JSON.stringify([
+        {
+          id: "legacy",
+          type: MutationType.UPDATE_PROFILE,
+          payload: { email: "sensitive@example.com" },
+          createdAt: 1,
+          status: "PENDING",
+          retryCount: 0,
+        },
+      ]),
+    );
+
+    const queue = await mutationQueue.getQueue();
+
+    expect(queue).toEqual([]);
+    expect(AsyncStorage.removeItem).toHaveBeenCalledWith("GUILDPASS_MUTATION_QUEUE");
+  });
+
   it("dequeues successfully", async () => {
     const item = await mutationQueue.enqueue(MutationType.UPDATE_PROFILE, { name: "Test" });
     let queue = await mutationQueue.getQueue();

tests/storage.test.ts

@@ -346,6 +346,15 @@ describe("Persistence and Rehydration", () => {
       ],
       ["guildpass:issuer-keys:guild-1", JSON.stringify({ issuerAddress: walletAddress })],
       ["guildpass:issuer-keys-index", JSON.stringify(["guild-1"])],
+      [
+        "guildpass:push-notifications:v1",
+        JSON.stringify({ state: { pushToken: "ExponentPushToken[legacy-secret]" } }),
+      ],
+      [
+        "guildpass:attestation-key-registry:guild-1",
+        JSON.stringify({ guildId: "guild-1", revokedAddresses: [walletAddress] }),
+      ],
+      ["guildpass:attestation-key-registry-index", JSON.stringify(["guild-1"])],
     ]);
     const asyncGet = vi.mocked(AsyncStorage.getItem);
     const asyncRemove = vi.mocked(AsyncStorage.removeItem);
@@ -380,7 +389,7 @@ describe("Persistence and Rehydration", () => {
       const report = await migrateLegacySensitiveStorage();
 
       expect(report.failedKeys).toEqual([]);
-      expect(report.migratedKeys).toHaveLength(8);
+      expect(report.migratedKeys).toHaveLength(11);
       expect(legacyEntries.size).toBe(0);
       for (const [key] of vi.mocked(SecureStore.setItemAsync).mock.calls) {
         expect(key).toMatch(/^[\w.-]+$/);
@@ -403,6 +412,22 @@ describe("Persistence and Rehydration", () => {
     }
   });
 
+  it("reports secure write failures after removing any plaintext fallback", async () => {
+    const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined);
+    vi.mocked(SecureStore.getItemAsync).mockResolvedValueOnce(null);
+    vi.mocked(SecureStore.setItemAsync).mockRejectedValueOnce(new Error("Keystore unavailable"));
+    vi.mocked(AsyncStorage.getItem).mockResolvedValueOnce(null);
+
+    try {
+      await expect(
+        migratingSecureStorage.setItem("session-storage", "sensitive-session"),
+      ).rejects.toThrow("Keystore unavailable");
+      expect(AsyncStorage.removeItem).toHaveBeenCalledWith("session-storage");
+    } finally {
+      consoleError.mockRestore();
+    }
+  });
+
   it("does not hydrate legacy data when AsyncStorage cleanup cannot be verified", async () => {
     const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined);
     const asyncGet = vi.mocked(AsyncStorage.getItem);