refactor(frontend): harden log decryption against unhandled rejections

[omlahore]

What happened

components/logs/SecretLogs.tsx, in the LogRow sub-component:

const envKeyPair = envKeys.find((envKey) => envKey.envId === event.environment.id)
const { publicKey, privateKey } = envKeyPair!.keys

envKeyPair can be undefined: Array.prototype.find returns that whenever no entry matches. This isn't hypothetical here, a log entry can reference an environment that no longer has a corresponding key locally, e.g. audit history retained for an environment that's since been deleted, or one the current viewer has lost access to.

The ! silences TypeScript but not the runtime. Dereferencing undefined!.keys throws, and it throws inside an async function (decryptSecretEvent) that's called with no .catch() and no surrounding try/catch:

if (log && envKeys.length > 0) decryptSecretEvent()

That's an unhandled promise rejection, the same shape as the bug fixed in #970 (different trigger, same underlying pattern: an async decrypt call with nowhere for its rejection to go).

The fix

• Replace the assertion with an explicit if (!envKeyPair) guard that logs and returns, rather than crashing the row.
• Wrap the decrypt call itself in try/catch, so a genuine decrypt failure for one log entry (corrupted ciphertext, key rotation mid-flight, whatever) doesn't hit the same unhandled-rejection shape either.

A dead end I want to be upfront about

I initially also added envKeys to this effect's dependency array, since it's read inside but wasn't listed, and I couldn't immediately see why that was safe. Running eslint surfaced its own answer:

React Hook useEffect has an unnecessary dependency: 'envKeys'. Outer scope values like 'envKeys' aren't valid dependencies because mutating them doesn't re-render the component

LogRow is defined inside SecretLogs's render body, so when envKeys (the parent's state) updates, SecretLogs re-renders, which recreates LogRow as a fresh closure and resets this effect regardless of its dependency array. So a stale envKeys snapshot from before it finished loading doesn't get stuck, this effect gets another chance once the parent re-renders. I'd suspected the opposite (that a row could stay stuck showing its skeleton forever if key derivation resolved after this effect's one shot), and eslint's own reasoning is what told me that wasn't right. I've left a comment plus the disable directive so this doesn't get "corrected" back into a lint warning later without the context.

I'm flagging this in case I'm still missing something about how the two components interact. I did not change this behaviour, only documented it, since I could not find a case where it's actually wrong once I understood why eslint was silent on it.

Testing

tsc --noEmit and eslint components/logs/SecretLogs.tsx are both clean on the changed file. I didn't add a test for this one: the fix is a straightforward null-guard plus a try/catch, and reproducing the envKeys lookup miss faithfully would need mocking the same GraphQL/Apollo/keyring context stack as the page component, which felt disproportionate for a change this size, same reasoning as the test scope note in #970.

[omlahore]

Reviewing my own reasoning here, and the justification I gave for this is wrong. Correcting it rather than leaving it to a reviewer.

I claimed a log entry can reference an environment with no matching entry in envKeys, e.g. one the viewer lost access to. I went and read resolve_secret_logs in backend/backend/schema.py and that isn't right. It derives the environments to fetch logs for from the same EnvironmentKey filter that populates environmentKeys:

env_keys_filter = {"environment__app": app, "user": org_member, "deleted_at": None}
env_ids = list(EnvironmentKey.objects.filter(**env_keys_filter).values_list("environment_id", flat=True).distinct())
if not env_ids:
    return SecretLogsResponseType(logs=[], count=0)

So logs are scoped to exactly the environments the user holds a key for. The two lists are consistent by construction, and loadMore's updateQuery explicitly carries environmentKeys: prev.environmentKeys through pagination, so that can't desync them either. I couldn't construct a case where the .find() actually misses.

That leaves this PR as defence-in-depth rather than a live bug fix, which is a materially weaker claim than the one in the description. Two things I'd still argue for keeping it:

• envKeyPair!.keys throws inside an async function with no .catch(), so if the backend/frontend invariant is ever broken (a resolver change, a new caller of this component passing different data), the failure mode is an unhandled rejection and a row stuck on its skeleton, not something legible.
• The try/catch around decryptAsymmetric is independently useful: that call can reject on genuinely corrupt ciphertext regardless of the key lookup, and right now nothing catches it.

Happy for you to close this if you'd rather not carry a guard for an invariant the backend already enforces. I'd rather flag that myself than have it merged on a premise I've since disproved. If you do want it, I can retitle it to something like "harden log decryption against unhandled rejections" so the commit history doesn't claim a bug that wasn't there.

[omlahore]

Retitled from fix(frontend): guard against a missing environment key in log decryption to reflect what this actually is, since nobody had picked between the two options I offered above and I'd rather the title not claim a bug I disproved myself.

It's hardening, not a bug fix: an unhandled rejection becomes a legible failure, and decryptAsymmetric gets a catch it didn't have. Still happy for you to close it if you'd rather not carry a guard for an invariant the backend already enforces.