Skip to content

fix: reject inbound messages on expired sessions (WAPI-1130)#72

Open
chakra-guy wants to merge 2 commits intocyfrin/wapi-1121from
cyfrin/wapi-1130
Open

fix: reject inbound messages on expired sessions (WAPI-1130)#72
chakra-guy wants to merge 2 commits intocyfrin/wapi-1121from
cyfrin/wapi-1130

Conversation

@chakra-guy
Copy link
Collaborator

@chakra-guy chakra-guy commented Feb 25, 2026

Summary

  • Adds session expiry check to the inbound message handler in BaseClient, closing a gap where only outbound messages were checked for expiry
  • Expired inbound messages are silently discarded with an error event emitted and disconnect triggered
  • Consolidates the duplicate isSessionExpired / checkSessionExpiry methods into a single checkSessionExpiry that returns a boolean, reused by both the inbound and outbound paths

Background

The Cyfrin audit identified that while sendMessage() checks for session expiry before sending, the transport.on("message", ...) handler did not perform any expiry check. This meant a message arriving on an expired session would still be decrypted and processed.

Changes

  • packages/core/src/base-client.ts: Unified checkSessionExpiry() method (returns boolean, handles disconnect + error emission). Used in both the inbound message handler (early return) and sendMessage (throws after check).
  • packages/core/src/base-client.integration.test.ts: New test case for expired session message rejection
  • apps/integration-tests/src/end-to-end.integration.test.ts: New E2E test verifying messages sent to an expired receiver are dropped and trigger SESSION_EXPIRED

Test plan

  • yarn build passes
  • yarn test:unit passes (63/63 tests)
  • yarn lint passes (no new warnings)
  • Integration tests pass (8/8 tests, including new expired session test)

Note

Medium Risk
Touches core session lifecycle behavior for all inbound messages; could introduce unexpected disconnects or error emissions if expiry timing/clock skew is mishandled, though changes are small and covered by new tests.

Overview
Inbound message handling now enforces session expiry. BaseClient checks expiry on every transport message event and, if expired, triggers disconnect cleanup and emits a SESSION_EXPIRED error instead of decrypting/processing the payload.

checkSessionExpiry() is refactored to return a boolean (expired vs not) and is reused by both inbound handling and sendMessage() (which now throws when the check reports expiry). New integration and E2E tests cover dropping inbound messages to an expired receiver, and the core changelog is updated.

Written by Cursor Bugbot for commit 5c38e3b. This will update automatically on new commits. Configure here.

Previously, only outbound messages checked for session expiry. Inbound
messages arriving on an expired session were still decrypted and processed.
Now, the inbound message handler checks session expiry before decryption
and silently discards messages on expired sessions, emitting an error event
and triggering a disconnect.
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Move the SESSION_EXPIRED error emit out of checkSessionExpiry() and
into the inbound message handler, so the outbound path (sendMessage)
only signals expiry via the thrown exception - not both emit and throw.
Copy link

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants