Skip to content

Conversation

@sirtimid
Copy link
Contributor

@sirtimid sirtimid commented Jan 26, 2026

Closes #661

Summary

  • Add sliding window rate limiting to protect against flooding attacks
  • Message rate limiting: 100 messages/second per peer (configurable via maxMessagesPerSecond)
  • Connection attempt rate limiting: 10 attempts/minute per peer (configurable via maxConnectionAttemptsPerMinute)

Implementation Details

  • New SlidingWindowRateLimiter class with automatic pruning of old events
  • Rate checks integrated into sendRemoteMessage before sending
  • Connection rate checks before dialing new connections
  • Throws ResourceLimitError when limits are exceeded
  • Rate limiter state cleaned up when peers become stale or transport stops

Test plan

  • All 1665 tests pass
  • Lint passes
  • New test file with 28 tests for rate limiter

🤖 Generated with Claude Code


Note

Adds configurable sliding-window rate limiting to remote comms and propagates limit errors.

  • Introduces SlidingWindowRateLimiter with defaults in constants and factories for message/connection limits
  • Extends initTransport to enforce messageRate before send and connectionRate before dialing; cleans limiter state on stale-peer cleanup/stop; new RemoteCommsOptions (maxMessagesPerSecond, maxConnectionAttemptsPerMinute)
  • Reconnection flow checks connection rate pre-dial and treats ResourceLimitError as retryable; logs and continues after backoff
  • Expands ResourceLimitError schema to include messageRate and connectionRate
  • Adds comprehensive unit tests for rate limiter and reconnection behavior; updates e2e tests/helpers to pass custom rate options

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

sirtimid and others added 2 commits January 26, 2026 21:50
Add sliding window rate limiting to protect against flooding attacks:
- Message rate limiting: 100 messages/second per peer (configurable)
- Connection attempt rate limiting: 10 attempts/minute per peer (configurable)

Implementation:
- Add SlidingWindowRateLimiter class with automatic pruning
- Add maxMessagesPerSecond and maxConnectionAttemptsPerMinute options
- Integrate rate checks in sendRemoteMessage before sending
- Integrate connection rate checks before dialing new connections
- Clean up rate limiter state when peers become stale or transport stops

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive test coverage for SlidingWindowRateLimiter:
- Basic limit checking (wouldExceedLimit)
- Event recording and pruning
- checkAndRecord with error handling
- getCurrentCount with window expiration
- clearKey and clear methods
- pruneStale for cleanup
- Sliding window behavior with real timing

Also test factory functions and constants.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sirtimid sirtimid requested a review from a team as a code owner January 26, 2026 19:51
- Add 'messageRate' and 'connectionRate' to ResourceLimitError limitType
- Update rate limiter to use correct limit type enum values
- Update tests to match new limit types

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move message rate recording to after successful write instead of before
  send attempt. This prevents failed sends from consuming rate quota.
- Add connection rate limiting to automatic reconnection attempts via
  checkConnectionRateLimit dependency in reconnection lifecycle.
- Handle ResourceLimitError gracefully during reconnection by continuing
  the loop after backoff instead of giving up on the peer.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
sirtimid and others added 3 commits January 26, 2026 23:12
- Add remoteCommsOptions parameter to setupAliceAndBob helper
- Configure higher maxMessagesPerSecond for queue limit test to
  ensure rate limiting doesn't interfere with queue limit testing

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…rror

- Call getCurrentCount once and reuse the value for both message and data
- Use DEFAULT_MESSAGE_RATE_WINDOW_MS constant instead of hardcoded 1000

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move DEFAULT_MESSAGE_RATE_LIMIT, DEFAULT_MESSAGE_RATE_WINDOW_MS,
DEFAULT_CONNECTION_RATE_LIMIT, and DEFAULT_CONNECTION_RATE_WINDOW_MS
to constants.ts for consistency with other default constants.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jan 26, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 88.49%
⬆️ +0.05%
5822 / 6579
🔵 Statements 88.38%
⬆️ +0.06%
5919 / 6697
🔵 Functions 87.35%
⬆️ +0.12%
1520 / 1740
🔵 Branches 85.02%
⬆️ +0.07%
2095 / 2464
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/kernel-errors/src/errors/ResourceLimitError.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/ocap-kernel/src/remotes/types.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/ocap-kernel/src/remotes/platform/constants.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/ocap-kernel/src/remotes/platform/rate-limiter.ts 100% 92.85% 100% 100%
packages/ocap-kernel/src/remotes/platform/reconnection-lifecycle.ts 87.09%
⬆️ +0.89%
87.09%
⬆️ +0.89%
80%
🟰 ±0%
87.09%
⬆️ +0.89%
105-109, 129-130, 209-210
packages/ocap-kernel/src/remotes/platform/transport.ts 86.62%
⬇️ -1.57%
80.59%
⬆️ +0.59%
80.95%
⬆️ +0.95%
86.62%
⬇️ -1.57%
103, 122-131, 163, 197-215, 238, 382, 394, 446, 457
Generated in workflow #3350 for commit cf0fab5 by the Vitest Coverage Report Action

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.

Revert to using checkAndRecord() for message rate limiting instead of
separate check and record calls. The separated approach had a TOCTOU
race where concurrent sends could all pass the check before any recorded,
bypassing the rate limit.

Yes, failed sends now consume quota, but this is necessary for security -
recording after send would allow attackers to make unlimited concurrent
attempts that bypass the rate limit.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.

Remote comms: Basic Rate Limiting

2 participants