Skip to content

Make relay rate limiter fail open on missing rule and skip when unconfigured - #8773

Merged
azooz2003-bit merged 2 commits into
mainfrom
feat-relay-failopen
Jul 23, 2026
Merged

azooz2003-bit merged 2 commits into
mainfrom
feat-relay-failopen

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

enforceRelayRateLimit in web/services/relay/http.ts (used by /api/relay/token and /api/relay/preferences) hard-fails on every non-success firewall outcome: an unset CMUX_RELAY_TOKEN_RATE_LIMIT_ID fails with rate_limit_not_configured and a deleted Vercel firewall rule (not-found) fails as rate_limit_unavailable, both returning 503 to every authenticated request. Since the Vercel rate-limit rules were removed, every device's relay policy fetch has been failing (policyUnavailable in the client diag rings), macOS hosts loop on endpointStarting -> relayPolicyRefreshFailed -> endpointFailed and never come up, and phones cannot discover or connect to Macs.

#8714 fixed this exact class in web/services/iroh/routeHandler.ts but did not cover this second limiter.

Fix

  • Unset/blank rule id: skip rate limiting entirely (operator wants no limits).
  • Rule check returns not-found (rule deleted): warn and fail open, mirroring 8714.
  • Unchanged: rateLimited/blocked still 429 with retry-after; thrown or unexpected check outcomes still fail closed with 503.

Commits

Two-commit regression structure: commit 1 adds the failing tests (red), commit 2 the fix (green).

Verification

  • bun test tests/relay-token-route.test.ts tests/relay-preferences-route.test.ts tests/iroh-route-handler.test.ts: 29 pass.
  • bun run typecheck clean.
  • Live evidence of the outage: cmux NIGHTLY host diag ring shows the continuous relayPolicyRefreshFailed(policyUnavailable) loop while CMUX_RELAY_TOKEN_RATE_LIMIT_ID points at a deleted rule.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fail open in the relay rate limiter when the Vercel rule is missing or unconfigured to stop 503s on /api/relay/token and /api/relay/preferences and restore device connectivity. Matches the existing behavior in services/iroh/routeHandler.ts.

  • Bug Fixes
    • Unset CMUX_RELAY_TOKEN_RATE_LIMIT_ID: skip rate limiting.
    • Vercel check returns not-found: warn and proceed (no limit).
    • Still return 429 for rateLimited and blocked; unexpected errors still 503.
    • Added tests for the skip and fail-open cases.

Written for commit 69c8cb3. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Relay requests now proceed when no rate-limit rule is configured.
    • Requests also proceed when a configured rate-limit rule cannot be found.
  • Bug Fixes

    • Rate-limit outages continue to return an appropriate service-unavailable response.
    • Active rate limits still block requests with a 429 response.

azooz2003-bit and others added 2 commits July 23, 2026 16:05
…e is missing

The relay token route 503s every request when CMUX_RELAY_TOKEN_RATE_LIMIT_ID
is unset or its Vercel firewall rule was deleted (not-found), taking every
device off the relay network. These tests encode the intended behavior:
no configured rule means no rate limiting, and a deleted rule fails open.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…figured

enforceRelayRateLimit (used by /api/relay/token and /api/relay/preferences)
treated every firewall-check outcome except success as fatal: an unset rule id
env failed with rate_limit_not_configured and a deleted Vercel rule (not-found)
failed as rate_limit_unavailable, both returning 503 for every authenticated
request. With the Vercel rate-limit rules removed, every device's relay policy
fetch failed with policyUnavailable, hosts never started their iroh endpoints,
and phones could not discover or connect to Macs.

Mirror the not-found fail-open that PR 8714 applied to services/iroh/
routeHandler.ts: an unset rule id now skips rate limiting entirely, and a
not-found rule logs a warning and proceeds. Real limits (429), blocked
requests, and genuine check failures (thrown/unexpected status, still 503)
keep their behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 23, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e9655400-7692-41c1-827d-1745b29ef4be

📥 Commits

Reviewing files that changed from the base of the PR and between d2d0ec4 and 69c8cb3.

📒 Files selected for processing (2)
  • web/services/relay/http.ts
  • web/tests/relay-token-route.test.ts

📝 Walkthrough

Walkthrough

Relay token rate limiting now skips missing configuration and fails open when a configured rule is absent, while preserving blocked-request and rate-limit outage responses through updated route tests.

Changes

Relay rate-limit behavior

Layer / File(s) Summary
Rate-limit enforcement behavior
web/services/relay/http.ts
Missing rule IDs skip rate limiting, and deleted rules are treated as unlimited; other rate-limit errors still fail.
Relay token route coverage
web/tests/relay-token-route.test.ts
Tests cover blocked requests returning 429, rate-limit failures returning 503, and fail-open behavior returning 200.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • manaflow-ai/cmux#8771: Updates the same relay rate-limit enforcement and relay-token test behavior for missing or deleted rule IDs.
🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main fix: relay rate limiting now fails open when the rule is missing or unconfigured.
Description check ✅ Passed The description is detailed and covers the problem, fix, and verification, though it omits some template sections like review trigger and checklist.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The PR diff touches only TypeScript files; no Swift files were introduced or modified, so no Swift actor-isolation regression is present.
Cmux Swift Blocking Runtime ✅ Passed PASS: The diff only touches web/services/relay/http.ts (TypeScript); no Swift files or runtime-blocking primitives were introduced or expanded.
Cmux Browser Automation Off-Main ✅ Passed PR only changes relay rate-limit HTTP logic; no browser.* socket routing, WebKit/AppKit waits, or worker-router policy changes are present.
Cmux Expensive Synchronous Load ✅ Passed PR only changes web relay TS/tests; no Swift files or agent-history loaders are touched, so the Swift load rule is not applicable.
Cmux Cache Substitution Correctness ✅ Passed Diff only changes relay rate-limit fail-open behavior; no authoritative-read-to-cache substitution or persistence/snapshot path is present.
Cmux No Hacky Sleeps ✅ Passed Touched TS files add fail-open rate-limit logic and tests only; no sleep, timer, polling, or wall-clock wait primitives appear in the diff.
Cmux Algorithmic Complexity ✅ Passed The diff only changes relay-rate-limit control flow plus tests; it adds no collection scans, rescans, or repeated sorting/filtering in production paths.
Cmux Swift Concurrency ✅ Passed Diff only changes web/services/relay/http.ts; no Swift files or concurrency patterns were added, so the Swift-concurrency check is not applicable.
Cmux Swift @Concurrent ✅ Passed PR diff only changes web/services/relay/http.ts; no Swift files or @concurrent-sensitive code were touched.
Cmux Swift Package Boundaries ✅ Passed No Swift files changed; the diff only touches web/services/relay/http.ts and web/tests/relay-token-route.test.ts, so the Swift package-boundary rule is not applicable.
Cmux Swiftpm Lockfiles ✅ Passed Only web/services/relay/http.ts changed; no SwiftPM, Xcode project, .gitignore, workflow, or dependency files were modified.
Cmux Swift Logging ✅ Passed No Swift files changed in the commit; the only new log is in web/TypeScript, so the Swift logging rule is not applicable.
Cmux User-Facing Error Privacy ✅ Passed New user-visible text is a generic warning only; API responses stay on existing generic codes, with no vendor/env names or raw upstream messages added.
Cmux Full Internationalization ✅ Passed Only backend rate-limit logic/tests changed; no web/messages, locale catalog, or user-facing copy was added. New text is a developer log and protocol tokens.
Cmux Swiftui State Layout ✅ Passed Only web/services/relay/http.ts changed; no SwiftUI views or state/layout patterns were introduced, so the SwiftUI rule is not applicable.
Cmux Architecture Rethink ✅ Passed This PR only changes web/services/relay/http.ts (TypeScript); no Swift files or Swift architecture patterns are present, so the Swift rethink rule is not applicable.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR only changes web/services/relay/http.ts; no Swift window/panel/controller/WindowGroup code is touched, so the auxiliary-window shortcut rule is not applicable.
Cmux Source Artifacts ✅ Passed Only changed path is hand-written source (web/services/relay/http.ts); no artifact, temp, cache, build, or log paths were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The patch only changes web/services/relay/http.ts; no Swift file or Sources/ production code was touched, so the seam rule isn’t applicable.
Cmux No Ambient Global State ✅ Passed PASS: PR diff since merge-base only touches web/* TypeScript files; no Swift production code or ambient global state changes are present.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-relay-failopen

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a relay outage caused by enforceRelayRateLimit hard-failing on two benign infrastructure states: an unset rule-id env var and a deleted Vercel firewall rule. The fix mirrors the pattern already applied to the iroh route handler in PR #8714.

  • Skip when unconfigured: an empty/unset ruleId now returns Effect.void instead of failing with rate_limit_not_configured, treating a blank env var as "operator wants no rate limit."
  • Fail open on not-found: a not-found response from the firewall check (Vercel 404 for a deleted rule) now logs a warning and passes rather than 503-ing every authenticated request.
  • Unchanged fail-closed paths: thrown check errors and unknown error strings still return 503; rateLimited: true and blocked still return 429.

Confidence Score: 4/5

Safe to merge — the fail-open changes are intentional, well-scoped to the two benign infrastructure states, and both behaviors are directly tested.

The logic change is correct and the two new test cases plus the augmented blocked case verify the exact failure modes described. The only gaps are minor: the return type still advertises RelayConfigurationError even though no code path can produce it, and the new console.warn uses a free-form string rather than the structured key pattern the rest of the file follows. Neither affects runtime behavior.

web/services/relay/http.ts — return type annotation and log format are the only items worth a second look.

Important Files Changed

Filename Overview
web/services/relay/http.ts Fixes enforceRelayRateLimit to fail open on missing rule (skip) and not-found rule (warn + pass); RelayConfigurationError is now unreachable in the return type and a new warn uses an unstructured format inconsistent with the rest of the file.
web/tests/relay-token-route.test.ts Adds two new standalone tests covering the skip-when-unconfigured and fail-open-when-not-found behaviors, and augments the existing rate-limit test with a blocked case; test coverage is correct and well-structured.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[enforceRelayRateLimit called] --> B{isVercel?}
    B -- No --> C[Effect.void — pass]
    B -- Yes --> D{ruleId set and non-empty?}
    D -- "No — NEW: skip, not 503" --> C
    D -- Yes --> E[call check ruleId]
    E -- throws --> F[RelayRateLimitError rate_limit_unavailable → 503]
    E -- ok --> G{rateLimited or error=blocked?}
    G -- Yes --> H[RelayRateLimitError rate_limited → 429]
    G -- No --> I{error = not-found?}
    I -- "Yes — NEW: fail open, not 503" --> J[console.warn + Effect.void — pass]
    I -- No --> K{any other error?}
    K -- Yes --> F
    K -- No --> C
Loading

Comments Outside Diff (1)

  1. web/services/relay/http.ts, line 35 (link)

    P2 The RelayConfigurationError is now unreachable — no code path in enforceRelayRateLimit emits it after this change. The declared error channel is more permissive than the actual behavior, which can mislead callers doing typed Effect error handling. Narrowing the type to match reality is a low-risk cleanup.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "Make relay rate limiter fail open on mis..." | Re-trigger Greptile

// means the operator deleted the limit, so treat it as "no limit" and
// fail open rather than 503-ing every request. Genuine unavailability
// (a thrown check or an unexpected status) still fails closed below.
console.warn("relay rate-limit rule not found; failing open");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 The new warning uses a bare string, while every other operational event in this file uses a structured key + payload (e.g. console.error("relay.policy.unavailable", tag)). Using a consistent key makes the event filterable in log aggregation.

Suggested change
console.warn("relay rate-limit rule not found; failing open");
console.warn("relay.rate_limit.rule_not_found", { ruleId });

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

1 participant