fix(frontend): stop environment switches from decrypting with stale keys

frontend/app/[team]/apps/[app]/environments/[environment]/[[...path]]/page.tsx

@@ -677,10 +677,40 @@ export default function EnvironmentPath({
     [deleteFolder, params.environment, secretPath]
   )
 
+  // wrappedSeed identifies which environment's keys are currently derived.
+  // GetSecrets polls every 5s (see the useQuery below), so `data` gets a new
+  // object reference on every poll tick even when nothing changed; keying off
+  // wrappedSeed rather than `data` itself means an unrelated poll refresh of
+  // the same environment does not re-trigger key derivation or clear envKeys.
+  const derivedForSeedRef = useRef<string | null>(null)
+
   useEffect(() => {
-    const initEnvKeys = async () => {
-      const wrappedSeed = data.environmentKeys[0].wrappedSeed
+    if (!data || !keyring) return
+
+    // Optional chaining rather than a bare index: this read used to sit inside
+    // the async function below, where an empty environmentKeys would surface
+    // as a rejected promise. Hoisting it up here to compare against the ref
+    // would otherwise turn that same case into a synchronous throw from the
+    // effect body, which takes the page down via the error boundary. Keeping
+    // the old failure mode rather than changing it as a side effect.
+    const wrappedSeed = data.environmentKeys[0]?.wrappedSeed
+    if (!wrappedSeed) return
+    if (derivedForSeedRef.current === wrappedSeed) return
+
+    // Switching environments (e.g. via the environment tabs) keeps this page
+    // mounted and only changes `data`, so a slower-resolving key derivation
+    // for an environment the user has since navigated away from must not be
+    // allowed to land after a newer one; `ignore` covers that regardless of
+    // resolution order. Clearing envKeys here is a display nicety, not the
+    // race guard itself: it stops the previous environment's already-decrypted
+    // secrets from staying on screen while this one's keys are still deriving.
+    // The decryptSecrets effect below does its own check against
+    // derivedForSeedRef, so it never runs against a mismatched envKeys/data
+    // pair even if it fires before this line's update is visible to it.
+    let ignore = false
+    setEnvKeys(null)
 
+    const initEnvKeys = async () => {
       const userKxKeys = {
         publicKey: await getUserKxPublicKey(keyring!.publicKey),
         privateKey: await getUserKxPrivateKey(keyring!.privateKey),
@@ -694,18 +724,36 @@ export default function EnvironmentPath({
       )
       const { publicKey, privateKey } = await envKeyring(seed)
 
-      setEnvKeys({
-        publicKey,
-        privateKey,
-        salt,
-      })
+      if (!ignore) {
+        derivedForSeedRef.current = wrappedSeed
+        setEnvKeys({
+          publicKey,
+          privateKey,
+          salt,
+        })
+      }
     }
 
-    if (data && keyring) initEnvKeys()
+    initEnvKeys()
+
+    return () => {
+      ignore = true
+    }
   }, [data, keyring])
 
   useEffect(() => {
-    if (data && envKeys) {
+    // This is the actual guard against decrypting one environment's secrets
+    // with another environment's keys. envKeys can be one render behind data
+    // changing, since the effect above derives it asynchronously, so this
+    // effect must not trust that envKeys already matches data just because
+    // both are non-null; it checks against derivedForSeedRef directly instead
+    // of relying on the ordering of the two effects.
+    const currentWrappedSeed = data?.environmentKeys[0]?.wrappedSeed
+    const envKeysAreCurrent =
+      currentWrappedSeed !== undefined && derivedForSeedRef.current === currentWrappedSeed
+
+    if (data && envKeys && envKeysAreCurrent) {
+      let ignore = false
       setDecrypting(true)
       const decryptSecrets = async () => {
         const decryptedStaticSecrets = await Promise.all(
@@ -811,13 +859,28 @@ export default function EnvironmentPath({
         return { decryptedStaticSecrets, decryptedDynamicSecrets }
       }
 
-      decryptSecrets().then((decryptedSecrets) => {
-        setServerSecrets(decryptedSecrets.decryptedStaticSecrets)
-        setClientSecrets(decryptedSecrets.decryptedStaticSecrets)
-        setDynamicSecrets(decryptedSecrets.decryptedDynamicSecrets)
-        setDecrypting(false)
-        setSecretsLoaded(true)
-      })
+      decryptSecrets()
+        .then((decryptedSecrets) => {
+          if (ignore) return
+          setServerSecrets(decryptedSecrets.decryptedStaticSecrets)
+          setClientSecrets(decryptedSecrets.decryptedStaticSecrets)
+          setDynamicSecrets(decryptedSecrets.decryptedDynamicSecrets)
+          setDecrypting(false)
+          setSecretsLoaded(true)
+        })
+        .catch((error) => {
+          // A decrypt call rejects if envKeys ever gets paired with data from
+          // a different environment (see the guard in the effect above). This
+          // used to be an unhandled rejection that left `decrypting` stuck at
+          // true, so the page never recovered from the race without a reload.
+          if (ignore) return
+          console.error('Failed to decrypt secrets:', error)
+          setDecrypting(false)
+        })
+
+      return () => {
+        ignore = true
+      }
     }
   }, [envKeys, data])
 

frontend/tests/utils/crypto/environmentKeyRace.test.ts

@@ -0,0 +1,52 @@
+/**
+ * @jest-environment node
+ */
+
+/*
+  👆
+  overrides: testEnvironment: 'jsdom' in jest.config.js
+  to fix: ReferenceError: TextDecoder is not defined
+*/
+
+/*
+  Regression test for the environment-switch race described in the PR that
+  added this file. It does not mount the page component (that needs Apollo,
+  Next navigation and the keyring context, none of which are set up in this
+  suite); instead it proves the property the fix in
+  app/[team]/apps/[app]/environments/[environment]/[[...path]]/page.tsx
+  relies on, using the real crypto primitives: decryptAsymmetric rejects,
+  rather than silently returning garbage, when the keypair does not match the
+  ciphertext's session.
+
+  That is why the original unguarded effect (pairing one environment's `data`
+  with another environment's `envKeys` while the correct keys were still
+  being derived) surfaced as a promise rejection, and why the missing
+  `.catch()` on that call turned it into an unhandled rejection that left
+  `decrypting` stuck at `true` with no way to recover short of a reload.
+*/
+
+import { decryptAsymmetric, encryptAsymmetric, randomKeyPair } from '@/utils/crypto'
+
+const toHexKeyPair = (keyPair: { publicKey: Uint8Array; privateKey: Uint8Array }) => ({
+  publicKey: Buffer.from(keyPair.publicKey).toString('hex'),
+  privateKey: Buffer.from(keyPair.privateKey).toString('hex'),
+})
+
+describe("Environment key race (decrypting one environment's secrets with another's keys)", () => {
+  test('decrypting with a mismatched keypair rejects rather than returning garbage', async () => {
+    const envA = toHexKeyPair(await randomKeyPair())
+    const envB = toHexKeyPair(await randomKeyPair())
+
+    const ciphertext = await encryptAsymmetric('super-secret-value', envA.publicKey)
+
+    // This is the exact operation the page performs when envKeys still holds
+    // environment B's keys while data has already updated to environment A's
+    // secrets (or vice versa): decrypting A's ciphertext with B's keypair.
+    await expect(decryptAsymmetric(ciphertext, envB.privateKey, envB.publicKey)).rejects.toBeDefined()
+
+    // Decrypting with the matching keypair still works, so the rejection
+    // above is specifically about the key mismatch, not a broken fixture.
+    const decrypted = await decryptAsymmetric(ciphertext, envA.privateKey, envA.publicKey)
+    expect(decrypted).toBe('super-secret-value')
+  })
+})