[devin-ai-integration]

Summary

Automated first-pass security fix for SEC-1592. Requires human security review before merge.

The default token storage (createDefaultStorage in src/main/storage.ts) enforces its plaintext policy only on the write path: encrypt() uses safeStorage when available and throws EncryptionUnavailableError unless allowPlaintext is set. The read path did not mirror this — decrypt() accepted any plain:-prefixed value unconditionally:

if (stored.startsWith(PLAINTEXT_PREFIX)) {
  return stored.slice(PLAINTEXT_PREFIX.length); // accepted even when encryption is available
}

Because the electron-store backing file (authkit-session.json) is an ordinary user-writable JSON file, a local process that can write that file — but cannot read/produce keychain enc: values (e.g. on macOS, where Keychain ACLs gate safeStorage) — could inject a value the SDK would never write, forging the persisted session, the PKCE sealing secret (cookiePassword), and pending verifiers → session fixation / login-CSRF.

The fix makes decrypt() honor the same policy as encrypt():

if (stored.startsWith(PLAINTEXT_PREFIX)) {
  if (!allowPlaintext) return null; // reject values the SDK would never have written
  return stored.slice(PLAINTEXT_PREFIX.length);
}

plain: values remain honored when allowPlaintext is enabled (the documented dev-only path), so the keyless-Linux fallback keeps working.

Validation

• Added regression tests in storage.spec.ts covering injected plain: session, cookiePassword, and pending-verifier values (rejected when encryption is available; still honored under allowPlaintext).
• Attacker-perspective reproduction (out-of-tree, real electron-store file + built dist): a plain:-injected file yields a forged session/cookiePassword/verifier on the vulnerable revision, and is rejected (getSession() → null) on this fixed revision. Negative control: an attacker-authored enc: value is rejected either way (they cannot forge keychain ciphertext), confirming the plain: branch is specifically what granted injection.
• pnpm test, pnpm run typecheck, pnpm run lint, and oxfmt --check pass on the changed files.

Full triage verdict and evidence are on the Linear ticket.

Link to Devin session: https://app.devin.ai/sessions/ee54943321d64f5ab14a295bd6bf7d6a

[devin-ai-integration]

Original prompt from Linear User

Please work on ticket "AuthKit Electron default storage accepts unauthenticated plain: values on read, letting a local file writer forge sessions, the PKCE sealing key, and pending verifiers (session fixation / login-CSRF)" (SEC-1592)

@playbook:playbook-b588614117c7477a9b9729928385384f

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[linear-code]

SEC-1592 AuthKit Electron default storage accepts unauthenticated `plain:` values on read, letting a local file writer forge sessions, the PKCE sealing key, and pending verifiers (session fixation / login-CSRF)

Summary

The SDK's default token storage (createDefaultStorage in src/main/storage.ts) encrypts all values it writes using Electron's safeStorage (OS keychain) and, by policy, refuses to persist plaintext unless the development-only allowPlaintext flag is set. However, the read path does not enforce this policy: the decrypt() function unconditionally accepts any stored value carrying the plain: marker prefix, even when OS encryption is available and plaintext storage is disabled.

Because the backing store is an ordinary user-writable JSON file (e.g., ~/Library/Application Support/<App>/authkit-session.json), any code that can write that file — without any ability to read or produce keychain-encrypted values — can inject a complete forged authentication state: the persisted session (tokens + user), the PKCE state-sealing secret (cookiePassword), and pending OAuth verifiers.

The practical impact is session fixation: an attacker can silently bind the victim's app to an attacker-controlled WorkOS account, so the victim operates unknowingly in the attacker's context. The marginal attacker gain from this vulnerability is most significant on macOS, where Keychain ACLs genuinely block same-user processes from accessing enc: values. On Windows (DPAPI) and Linux (Secret Service), same-user malware can typically also decrypt safeStorage-backed values directly, so the plain: bypass provides less incremental advantage.

Affected Code

Write path enforces policy (encrypt(), src/main/storage.ts:119-127)

function encrypt(value: string): string {
  if (ss.isEncryptionAvailable()) {
    return ENCRYPTED_PREFIX + ss.encryptString(value).toString('base64');
  }
  if (allowPlaintext) {
    return PLAINTEXT_PREFIX + value;
  }
  throw new EncryptionUnavailableError();
}

Read path does NOT enforce policy (decrypt(), src/main/storage.ts:130-148)

function decrypt(stored: unknown): string | null {
  if (typeof stored !== 'string') {
    return null;
  }
  try {
    if (stored.startsWith(ENCRYPTED_PREFIX)) {
      const buf = Buffer.from(stored.slice(ENCRYPTED_PREFIX.length), 'base64');
      return ss.decryptString(buf);
    }
    if (stored.startsWith(PLAINTEXT_PREFIX)) {
      // BUG: No check of allowPlaintext or isEncryptionAvailable().
      // A value the SDK would NEVER have written is accepted when read.
      return stored.slice(PLAINTEXT_PREFIX.length);
    }
    return null;
  } catch {
    return null;
  }
}

Attack Flow

Variant 1: Direct Session Injection

1. The electron-store JSON file at ~/Library/Application Support/<App>/authkit-session.json is writable by any same-user process. Only the values are keychain-encrypted.

2. The attacker obtains valid tokens for the same WorkOS client by self-registering via the public OAuth flow (the clientId ships in the app binary; no apiKey or secret is required).

3. The attacker writes the forged session as a plaintext-prefixed value into the store file:

   {
     "session": "plain:{\"accessToken\":\"eyJ...\",\"refreshToken\":\"rt_...\",\"user\":{\"id\":\"user_ATTACKER\",\"email\":\"attacker@evil.example\"}}"
   }

4. On the next getUser() call, getSession() invokes decrypt(), which hits the plain: branch and returns the attacker's session.

5. core.validateAndRefresh() in session-manager.ts:110 verifies the JWT signature against the app's JWKS. Because the tokens are genuine (signed by WorkOS for this client), validation passes.

6. The app now operates under the attacker's identity. All getAccessToken() calls return attacker-account tokens, and useAuth() in the renderer shows the attacker's user data.

Variant 2: Login CSRF via PKCE State Injection

1. The attacker writes "cookiePassword": "plain:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" into the store file, overriding the keychain-encrypted sealing secret.
2. getOrCreateCookiePassword() reads the existing value via decrypt(), finds the plain: value, and returns it — using a sealing secret now known to the attacker.
3. The attacker also plants a pendingVerifiers entry sealed under the known password.
4. The attacker mints an authorization code by running the public-client authorize flow with their own credentials and the matching PKCE challenge.
5. The attacker triggers the app's deep-link handler: open 'workos-auth://callback?code=<code>&state=<sealed>'.
6. completeCallback() in session-manager.ts:210-247 finds the planted verifier, verifyCallbackState unseals it with the attacker-known cookiePassword, and authenticateWithCode succeeds. The victim's app completes a sign-in into the attacker's account.

Root Cause

Missing policy/integrity enforcement on the storage read path. The decrypt() function trusts an unauthenticated plain:/enc: marker prefix chosen by whoever last wrote the file. The allowPlaintext flag and isEncryptionAvailable() are enforced only in encrypt(), so a value the SDK would never have written (plaintext when encryption is available and plaintext is disallowed) is still accepted when read back.

More fundamentally, at-rest auth state is confidentiality-protected but not integrity-bound — nothing authenticates that a stored value was produced by this app's keychain-backed write path.

Preconditions

1. Local file write access as the victim OS user. The attacker must be able to write the app's electron-store JSON file (or have offline disk/backup access). This is weaker than full Keychain access — on macOS specifically, safeStorage items are Keychain-ACL'd to the app, so a same-user utility process can write this file yet cannot decrypt or produce enc: values. This finding exploits exactly the gap between file-write capability and Keychain-read capability.
2. Valid tokens for the same WorkOS client. Trivially obtainable since the SDK operates as a public OAuth client — the clientId ships in the binary, so the attacker can self-register.
3. Default configuration. The vulnerable code path is the default: createDefaultStorage() with allowPlaintext unset (false) and OS encryption available. No opt-in or misconfiguration required.

Impact

• Session fixation: The victim's app silently operates against an attacker-controlled account. Documents, configuration, uploads, and backend writes the app performs with getAccessToken() are stored in the attacker's tenant.
• Login CSRF: The attacker can force a complete sign-in into their account through the SDK's own PKCE state defense by replacing the sealing secret and pending verifiers.
• Scope: Affects every application using @workos/authkit-electron with the default storage. The marginal gain is sharpest on macOS where Keychain ACLs genuinely block same-user processes from other apps.
• Severity consideration: Requires local same-user file write access, which limits the attack surface. On platforms other than macOS, a same-user attacker can typically also decrypt enc: values, reducing the incremental advantage of the plain: bypass.

Recommended Fix

The decrypt() function should mirror the write-path policy: when allowPlaintext is false and/or isEncryptionAvailable() returns true, plain: prefixed values should be rejected (treated as absent/null). This ensures that a value the SDK would never have written is also never accepted on read:

function decrypt(stored: unknown): string | null {
  if (typeof stored !== 'string') return null;
  try {
    if (stored.startsWith(ENCRYPTED_PREFIX)) {
      const buf = Buffer.from(stored.slice(ENCRYPTED_PREFIX.length), 'base64');
      return ss.decryptString(buf);
    }
    if (stored.startsWith(PLAINTEXT_PREFIX)) {
      // Enforce the same policy as encrypt():
      if (!allowPlaintext) return null;
      return stored.slice(PLAINTEXT_PREFIX.length);
    }
    return null;
  } catch {
    return null;
  }
}

For defense-in-depth, consider adding integrity protection (e.g., HMAC with a keychain-derived key) to detect file tampering.

---

Source: prodsec-ai (scanner)
Finding ID: 77135498-7fd4-499a-b6ee-7927bc128ec2
Finding URL: https://prodsec-ai.workos.tools/findings/77135498-7fd4-499a-b6ee-7927bc128ec2
Repository: workos/authkit-electron
Location: src/main/storage.ts:130
Scan model: claude-mythos-5
Scanner severity: medium
Static validator verdict: confirmed

Review in Linear · Agent session

[greptile-apps]

Greptile Summary

This PR closes a security gap (SEC-1592) where the decrypt() path in createDefaultStorage accepted plain:-prefixed values unconditionally, even when allowPlaintext was false and OS-level encryption was available. A local process with write access to the electron-store JSON file (but no keychain access) could inject forged plain: session, cookiePassword, or pending-verifier values that the SDK would never have written itself.

• src/main/storage.ts: Adds a !allowPlaintext guard inside the plain: branch of decrypt(), mirroring the existing write-path guard in encrypt(). The plaintext fallback is still honored when allowPlaintext: true (documented dev/Linux-keyringless path).
• src/main/__tests__/storage.spec.ts: Adds five regression tests covering injected plain: session, cookiePassword, and pending-verifier rejection, plus two positive-case tests for allowPlaintext: true with and without encryption available.
• .github/workflows/ci.yml and README.md: Minor CI flag correction and Markdown table whitespace alignment.

Confidence Score: 5/5

Safe to merge — the change is a minimal, well-targeted guard that makes the read path consistent with the already-correct write path, with no regressions to the legitimate allowPlaintext fallback.

The fix is a single two-line guard in decrypt() that directly mirrors the existing encrypt() policy. The logic is straightforward, all three affected values (session, cookiePassword, pending verifiers) are covered by regression tests, and the positive case (allowPlaintext: true with both encryption states) is also exercised. No existing behavior for the non-default path is broken.

No files require special attention — src/main/storage.ts carries the only functional change and its logic is simple and correct.

Important Files Changed

Filename	Overview
src/main/storage.ts	Core security fix: adds !allowPlaintext guard in decrypt() to reject injected plain: values when encryption is available, mirroring the existing write-path policy in encrypt().
src/main/tests/storage.spec.ts	Adds five regression tests for the read-path plaintext policy: three injection-rejection tests (session, cookiePassword, pending verifiers) and two positive-case tests for allowPlaintext: true with encryption unavailable and available.
src/main/tests/session-manager.spec.ts	Cosmetic-only change: reformats a long toHaveBeenCalledWith assertion onto a single line for consistency.
.github/workflows/ci.yml	Removes spurious -- separator before --pack in the attw CI step; no functional impact on correct shells but is a cleaner invocation.
README.md	Markdown table column-width alignment only; no content changes.

Sequence Diagram

sequenceDiagram
    participant Attacker as Local Attacker
    participant File as authkit-session.json
    participant SDK as decrypt() in storage.ts
    participant App as Application

    Note over Attacker,App: Attack vector (BEFORE fix)
    Attacker->>File: "Write plain:{session JSON}"
    App->>File: store.get("session")
    File-->>SDK: "plain:eyJATTACKER..."
    SDK->>SDK: startsWith("plain:") true
    SDK-->>App: Return forged session

    Note over Attacker,App: After fix (allowPlaintext=false, default)
    Attacker->>File: "Write plain:{session JSON}"
    App->>File: store.get("session")
    File-->>SDK: "plain:eyJATTACKER..."
    SDK->>SDK: startsWith("plain:") true
    SDK->>SDK: !allowPlaintext true, return null
    SDK-->>App: null (treated as absent)

    Note over Attacker,App: Legitimate path (allowPlaintext=true)
    App->>SDK: encrypt(session)
    SDK->>SDK: isEncryptionAvailable() false, allowPlaintext true
    SDK->>File: "Write plain:{session JSON}"
    App->>File: store.get("session")
    File-->>SDK: "plain:{session JSON}"
    SDK->>SDK: !allowPlaintext false, return value
    SDK-->>App: Valid session

Loading

_{Reviews (4): Last reviewed commit: "Fix attw CI step arg passing" | Re-trigger Greptile}