Verify keyring deletes actually removed the credential (Refs #3011)

packages/storage/src/secure-store/default-keyring-adapter.ts

@@ -28,6 +28,8 @@ import type { StorageLogger } from '../types/logger.js';
 import { NullStorageLoggerImpl } from '../types/logger.js';
 import { isRuntimeReplaced } from './runtime-identity.js';
 import { assertRuntimeNotReplaced } from './runtime-replaced-errors.js';
+import { verifyKeyringDelete } from './keyring-delete-verification.js';
+import { SecureStoreError } from './secure-store-errors.js';
 import type { KeyringAdapter } from './secure-store.js';
 
 export type FindCredentialsFunction = (
@@ -242,7 +244,45 @@ export async function createDefaultKeyringAdapter(): Promise<KeyringAdapter | nu
       },
       deletePassword: async (service: string, account: string) => {
         const entry = new kr.AsyncEntry(service, account);
-        return entry.deleteCredential();
+        const deleted = await entry.deleteCredential();
+        // Always probe, regardless of the native boolean. On macOS the delete
+        // status is destroyed below the binding (security-framework discards
+        // the OSStatus, apple-native-keyring-store returns Ok unconditionally,
+        // and the binding maps that to true), so a true result is no guarantee
+        // the credential is gone. Only the read-back can confirm absence.
+        const outcome = await verifyKeyringDelete(() => entry.getPassword());
+        if (outcome === 'still-present') {
+          // Diagnostics only — service/account names, never the read-back value.
+          _keyringLogger.debug(
+            () =>
+              `[keyring] credential remains after deletion: service='${service}' account='${account}'`,
+          );
+          // Where this rejection currently surfaces:
+          //   - MCP KeychainTokenStorage.deleteCredentials() calls this
+          //     adapter directly and propagates it. Observable today.
+          //   - SecureStore.deleteLocked() still wraps this call in a bare
+          //     `catch {}` and discards it. NOT observable through
+          //     SecureStore.delete() yet; the in-flight PR for issue #1985
+          //     replaces that catch with classification and a rethrow of
+          //     anything that is not NOT_FOUND. secure-store.ts is owned by
+          //     that PR, so it is deliberately not modified here.
+          //
+          // The message is fixed and interpolation-free for when that lands:
+          // classifyError() re-derives the code from message text and ignores
+          // SecureStoreError.code, so the message must avoid every trigger
+          // substring ("not found", "locked", "denied", "permission",
+          // "timeout", "timed out"). validateKey() permits a key literally
+          // named "not found" — interpolating one would re-classify this as
+          // NOT_FOUND and get it swallowed, silently defeating the throw.
+          throw new SecureStoreError(
+            'Credential remains after keyring deletion',
+            'UNAVAILABLE',
+            'Retry the delete; if it persists, inspect the OS keyring entry for this service and account and remove it manually.',
+          );
+        }
+        // Absent: return the original native boolean unchanged. true means "a
+        // credential was deleted"; false means "there was nothing to delete".
+        return deleted;
       },
     };
     const withFindCreds = withFindCredentials(adapter, findCredentialsFn);

packages/storage/src/secure-store/keyring-delete-verification.ts

@@ -0,0 +1,59 @@
+/**
+ * @license
+ * Copyright 2026 Vybestack LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * Keyring delete verification.
+ *
+ * Reads back the credential after a delete to check whether it is actually
+ * gone. This is necessary on every platform because `@napi-rs/keyring`'s
+ * `deleteCredential()` erases backend errors into a boolean: on Linux/Windows
+ * failures collapse to `false`, and on macOS the OSStatus is discarded at
+ * three layers below the binding so failures collapse to `true`. Absence is
+ * decided by `=== null` only — the empty string is a present credential.
+ *
+ * A rejecting read-back propagates, because absence cannot be confirmed and
+ * claiming the secret is gone would be the exact failure this module exists
+ * to detect.
+ *
+ * Known limitation: this detects a credential that remains *readable* after a
+ * delete (the macOS silent-failure case, ACL denials, partial backend
+ * failure). It does NOT detect a fully locked store (the read erases to
+ * `null`) or `Ambiguous` collisions (the binding erases the ambiguous read
+ * error to `null`, so the probe reports absent for a credential that still
+ * exists). Completeness requires the upstream fix; see
+ * project-plans/issue3011/plan.md for the full matrix.
+ *
+ * @plan PLAN-20260804-ISSUE3011
+ * @requirement R1
+ */
+
+/**
+ * Discriminated outcome of a post-delete read-back probe.
+ *
+ * - `absent` — read-back is `null`; the credential is gone.
+ * - `still-present` — read-back is a value (including the empty string); the
+ *   delete did not take effect.
+ *
+ * @plan PLAN-20260804-ISSUE3011
+ * @requirement R1
+ */
+export type KeyringDeleteOutcome = 'absent' | 'still-present';
+
+/**
+ * Reads back the credential after a delete and classifies whether it is gone.
+ *
+ * Takes a thunk rather than a `KeyringAdapter` because the adapter is still
+ * under construction inside the factory when this runs.
+ *
+ * @plan PLAN-20260804-ISSUE3011
+ * @requirement R1
+ */
+export async function verifyKeyringDelete(
+  readBack: () => Promise<string | null>,
+): Promise<KeyringDeleteOutcome> {
+  const value = await readBack();
+  return value === null ? 'absent' : 'still-present';
+}