fix: resolve ReferenceError in hasEncryptionKey service - #2405
fix: resolve ReferenceError in hasEncryptionKey service#2405CodeBySayak wants to merge 2 commits into
Conversation
Signed-off-by: Chandra Keshav Mishra <chandrakeshavmishra@gmail.com>
WalkthroughThe PR adjusts biometric cancellation error handling in the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
machines/store.ts (1)
365-379:⚠️ Potential issue | 🟠 MajorAdd return statements to prevent event cascade after cancellation and error handling.
The control flow in the
hasEncryptionKeyservice callback (lines 365–379) allows execution to fall through after dispatchingBIOMETRIC_CANCELLED, causing the machine to receive conflicting events in rapid succession:
- Line 367 calls
callback(model.events.BIOMETRIC_CANCELLED())but does not return, so execution continues to line 369 (sendErrorEvent), polluting telemetry for a user-initiated cancel.- Lines 371–377 then dispatch either
KEY_INVALIDATE_ERRORorSTORE_ERRORfor the same cancellation event.- Line 379 calls
callback(model.events.READY())outside the try/catch, so it executes after any error path as well.This causes the state machine to receive
BIOMETRIC_CANCELLED→STORE_ERROR/KEY_INVALIDATE_ERROR→READYin quick succession. SinceBIOMETRIC_CANCELLEDtargetscheckFreshInstall(lines 305–308) whileREADYtargetsready(line 135), the final state becomes unpredictable.Add return statements after
BIOMETRIC_CANCELLEDdispatch and after the error handling block to short-circuit execution and preventREADYfrom firing on error paths.🛠 Proposed fix
} catch (e) { if (e instanceof BiometricCancellationError) { callback(model.events.BIOMETRIC_CANCELLED()); + return; } sendErrorEvent(getErrorEventData('ENCRYPTION', '', e)); if (e.message.includes(keyinvalidatedString)) { await clear(); callback(model.events.KEY_INVALIDATE_ERROR()); sendUpdate(); } else { callback(model.events.STORE_ERROR(e)); } + return; } callback(model.events.READY());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@machines/store.ts` around lines 365 - 379, In the hasEncryptionKey service callback, stop fall-through after biometric cancellation and after handling errors: when catching BiometricCancellationError and calling callback(model.events.BIOMETRIC_CANCELLED()) insert an immediate return to avoid subsequent sendErrorEvent and STORE_ERROR/KEY_INVALIDATE_ERROR dispatches, and ensure you also return after the error-handling branch (after sending sendErrorEvent and dispatching either model.events.KEY_INVALIDATE_ERROR() or model.events.STORE_ERROR(e)) so execution does not continue to callback(model.events.READY()); this keeps the machine from receiving BIOMETRIC_CANCELLED → STORE_ERROR/KEY_INVALIDATE_ERROR → READY in quick succession.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@machines/store.ts`:
- Around line 365-379: In the hasEncryptionKey service callback, stop
fall-through after biometric cancellation and after handling errors: when
catching BiometricCancellationError and calling
callback(model.events.BIOMETRIC_CANCELLED()) insert an immediate return to avoid
subsequent sendErrorEvent and STORE_ERROR/KEY_INVALIDATE_ERROR dispatches, and
ensure you also return after the error-handling branch (after sending
sendErrorEvent and dispatching either model.events.KEY_INVALIDATE_ERROR() or
model.events.STORE_ERROR(e)) so execution does not continue to
callback(model.events.READY()); this keeps the machine from receiving
BIOMETRIC_CANCELLED → STORE_ERROR/KEY_INVALIDATE_ERROR → READY in quick
succession.
|
@CodeBySayak can you raise PR for develop branch and sign off the commit too? |
3 participants
Summary
This PR fixes a
ReferenceErrorin thehasEncryptionKeyservice withinmachines/store.ts. The code was attempting to accessevent.requesterwhen handling aBiometricCancellationError, but theeventobject is not defined within the scope of this callback service.Changes
event.requesterin thehasEncryptionKeycallback.model.events.BIOMETRIC_CANCELLED()without arguments to safely handle biometric cancellation.Impact
This fix prevents an application crash during the startup/initialization flow. Specifically, it addresses the scenario where a user cancels a biometric authentication prompt while the app is checking for an encryption key. Previously, this cancellation would trigger a
ReferenceError: event is not defined, leading to an unstable app state.Testing
BIOMETRIC_CANCELLEDevent model supports an optional requester.ReferenceErrorin the async callback scope.closes #2404
Summary by CodeRabbit