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

[omlahore]

What happened

Switching environments (the environment tabs use a plain <Link>, so the page stays mounted and only data changes) can leave the page stuck on "Decrypting..." until reloaded.

app/[team]/apps/[app]/environments/[environment]/[[...path]]/page.tsx has two effects:

1. Derives envKeys asynchronously from data.environmentKeys[0].wrappedSeed (several awaited decrypt calls).
2. Once data and envKeys are both set, decrypts every secret in data.secrets with envKeys.

The second effect fires on the same render where data changes to the new environment, before the first effect's async derivation has any chance to resolve. At that point envKeys still holds the previous environment's keys. It decrypts the new environment's ciphertext with the old environment's keypair, which decryptAsymmetric rejects (verified in the added test, it does not silently return garbage).

That rejection went to decryptSecrets().then(...) with no .catch(). An unhandled rejection, so setDecrypting(false) was never reached and decrypting stayed true.

There's a second, narrower case: switching to a third environment before the second one's key derivation resolves could let it land after the third's, pairing envKeys with the wrong data even once the promise settles.

The fix

• Track which environment's wrappedSeed the current envKeys were derived from (derivedForSeedRef), and skip re-deriving when it's unchanged. This matters because GetSecrets polls every 5s (pollInterval: unsavedChanges ? 0 : 5000), so keying key derivation directly off data would re-derive and flash envKeys to null on every idle poll tick, not just on real environment switches.
• The deriving effect gets a standard ignore flag so a slower-resolving derivation for an environment the user has since left can't land after a newer one, regardless of resolution order.
• The decrypting effect independently checks envKeys against that same tracked seed before running, rather than assuming the deriving effect's setEnvKeys(null) is visible to it in the same pass. I didn't want to build correctness on an assumption about React's effect-batching order between two separate effects that I couldn't verify without the app running end to end, so this effect verifies the pairing itself either way.
• Added the missing .catch(), so a genuine decrypt failure surfaces to the console and clears decrypting instead of hanging forever.

Testing

tests/utils/crypto/environmentKeyRace.test.ts uses the real crypto primitives (randomKeyPair, encryptAsymmetric, decryptAsymmetric) to prove the underlying failure mode: decrypting with a mismatched keypair rejects rather than returning garbage, which is why the race above surfaces as a stuck page rather than silently wrong data.

I didn't mount the page component itself. It needs Apollo's MockedProvider, Next navigation, and the keyring context, none of which are set up in this test suite, and building that harness felt disproportionate to the fix. The existing tests in tests/utils/crypto/ follow the same pattern of testing the crypto layer directly rather than mounting pages, so I matched that.

tsc --noEmit and eslint are clean on both changed files (eslint reports one pre-existing warning at line 489 unrelated to this change, confirmed present on main before my edit too).

Note for reviewers

Five other open PRs touch this same file (#968, #956, #935, #592, #474), none of them near these two effects as far as I could tell, but flagging it since a merge conflict is likely depending on ordering. Happy to rebase.

[omlahore]

Self-review caught a regression in my own patch, now pushed as a follow-up commit.

Hoisting the wrappedSeed read out of initEnvKeys so it could be compared against the ref quietly changed what an empty environmentKeys array does. It used to sit inside the async function, so environmentKeys[0].wrappedSeed on an empty array became a rejected promise. Moved into the effect body it became a synchronous throw, which unmounts the page through the error boundary instead.

Every other call site in the codebase indexes environmentKeys[0] without a guard, so I don't think this array is ever actually empty in practice. But changing a failure mode as a side effect of an unrelated race fix isn't something I want in this PR, so it now reads with optional chaining and bails:

const wrappedSeed = data.environmentKeys[0]?.wrappedSeed
if (!wrappedSeed) return

The wrappedSalt read further down still uses a bare index, but it's still inside the async function where it always was, so its behaviour is unchanged by this PR and I've left it alone.

[omlahore]

@rohan-chaturvedi all four of these are parked at CI/CD Pipeline: action_required rather than waiting on a review: #970, #971, #972 and #973. The pipeline has never run on any of them, so the only green marks are third-party apps and they do not mean the code was tested.

If someone can approve the runs I will act on whatever they turn up. Separately I am closing #971 myself, the premise I gave for it did not survive my own recheck.

[rohan-chaturvedi]

@rohan-chaturvedi all four of these are parked at CI/CD Pipeline: action_required rather than waiting on a review: #970, #971, #972 and #973. The pipeline has never run on any of them, so the only green marks are third-party apps and they do not mean the code was tested.

If someone can approve the runs I will act on whatever they turn up. Separately I am closing #971 myself, the premise I gave for it did not survive my own recheck.

Sorry I haven't gotten the chance to review these yet. I should hopefully have some bandwidth in the coming week. The CI runs just run the test suites and builds, all of which can be run locally as well. Maybe rebase the branches against main, and I'll take a look at them as soon as I can.

[omlahore]

Rebased all three on main, they were 29 commits behind: #970, #972, #973. Clean, no conflicts.

Ran the CI test jobs locally against each rebased head. yarn test for the frontend ones: #973 is 14 suites and 358 tests green, #970 is 15 and 359, the extra suite being the environmentKeyRace regression test that comes with it. pytest tests/ for the backend: #972 is 1269 passed.

[omlahore]

Rebased all three onto main, they sit on c27c290 now.

Since CI has never run on any of them, I ran the frontend suite locally: #970 is 18 suites / 386 tests passing, #973 is 17 / 385. #972 is backend only, four files under backend/, and I did not have a Postgres instance to run pytest tests/ against, so that one has had a syntax check and nothing more.

One note on #973. The account page commit in main also touched HistoryDialog.tsx, but it added the deleted-actor branch in the render path while my change is in the key-unwrap path above it, so the rebase was clean and the two do not interact.

[omlahore]

Applied the suggestion. The ref is cleared alongside setEnvKeys(null) now, so it only ever holds a seed while real keys exist and the return to A re-derives.

Comments trimmed in both the page and the test file.

Frontend suite passes locally: 18 suites, 386 tests. Rebased on main.

[omlahore]

The red X is the image build, not the code. docker/login-action gets Secret source: None on fork PRs and fails with "Username and password required" before any build step runs. Both test jobs pass: frontend 1m59s, backend 58s.

[omlahore]

The red X is the image build, not the code. docker/login-action gets Secret source: None on fork PRs and fails with "Username and password required" before any build step runs. Both test jobs pass: frontend 1m59s, backend 58s.

[omlahore]

Both points are in.

The ref reset landed in 4ca41e1: derivedForSeedRef.current = null now sits directly above setEnvKeys(null), so the ref only ever holds a seed while real keys exist and the A to B to A case re-derives instead of early-returning on a stale match.

Comments are trimmed in a3b882a. The two effect comments were four and six lines and the decrypt catch was another four, all of them restating what the code shows. Each is down to the part that is not obvious from reading it: why the ref is cleared alongside envKeys, why envKeys being non-null is not proof it belongs to the current environment, and what the catch prevents. Same treatment on the block comment in the race test.

Note the CI red is not from this branch. Every job fails at ##[error]Username and password required in the registry login step, which fork PRs cannot pass because they do not get repository secrets. The same four jobs are red on all six of my open PRs here for that reason. npx jest tests/utils/crypto/environmentKeyRace.test.ts passes locally and prettier is clean on both files.