fix(cloud): bound pending login sessions and require enabled cloud#75
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
The two new start_login tests built a full daemon AppState via the process-global data-dir override, racing other parallel tests on the shared asset index. Each test now builds state with an explicit, owned temp data dir, so no global state is shared.
hyperb1iss
force-pushed
the
codex/fix-unbounded-cloud-login-sessions
branch
from
0719de1 to
cb0cb9a
Compare
* test(cloud): re-enable cloud in regressed login start/poll tests PR #75 added a gate that rejects `POST /cloud/login/start` with 409 when cloud is disabled. Two login tests built their `AppState` through `cloud_test_state`, which hard-codes `cloud.enabled = false`, so after #75 they hit that gate. The start test asserted 201 and got 409; the poll test then panicked on the `login_id` missing from the 409 error body. The `cloud` feature carries no CI coverage, so the regression reached main unnoticed. Point both tests at `cloud_test_state_with_cloud` with cloud enabled and drop the now-unused `cloud_test_state` wrapper. Co-Authored-By: Nova (Claude Opus 4.7) <noreply@anthropic.com> * style(cloud): use larger Duration units in cloud socket Clippy's `duration_suboptimal_units` flags `Duration::from_secs` calls whose values land exactly on a larger unit. `cloud_socket.rs` built a 3600-second heartbeat ceiling and a 60-second reconnect cap that read more clearly as one hour and one minute. Switch to `Duration::from_hours(1)` and `Duration::from_mins(1)`, and update the matching `cloud_socket_tests.rs` assertions. The durations are unchanged. These lints had accumulated because the `cloud` feature has no CI lint pass. Co-Authored-By: Nova (Claude Opus 4.7) <noreply@anthropic.com> --------- Co-authored-by: Nova (Claude Opus 4.7) <noreply@anthropic.com>
1 participant
Motivation
DeviceAuthorizationSessions in an unbounded in-memory map and never pruned them unless a client polled, enabling unbounded memory growth and an availability DoS.start_logindid not check whether cloud was enabled in the daemon config, so the endpoint could be used on deployments where cloud should be disabled.Description
MAX_PENDING_CLOUD_LOGIN_SESSIONS = 128and admission checking incrates/hypercolor-daemon/src/api/cloud.rsto refuse new starts when the pending-session limit is reached.cloud.enabledforstart_loginand return409 Conflictwhen cloud is disabled.429 Rate Limitedwhen the pending limit is reached, and insert the new session only after these checks.crates/hypercolor-daemon/tests/cloud_api_tests.rsnamedcloud_login_start_rejects_when_cloud_disabledandcloud_login_start_rejects_when_pending_session_limit_reachedto cover the new behaviors.Testing
cargo fmt --all, which completed successfully.cloud_login_start_rejects_when_cloud_disabledandcloud_login_start_rejects_when_pending_session_limit_reachedwere added to thecloudfeature test suite.cargo test -p hypercolor-daemon --features cloud ...) but a full test build did not finish within the execution window due to long compilation times; no unexpected failures were observed during edit/compile iterations.Codex Task