[front] Support fixed windows in the plan fair-use AWU limiter - #29617
[front] Support fixed windows in the plan fair-use AWU limiter#29617tdraier wants to merge 1 commit into
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
| workspaceId: string, | ||
| timeframe: FixedAwuCreditsTimeframeType | ||
| ): Promise<FixedWindowBounds> { | ||
| if (timeframe === "contract_billing_cycle") { |
There was a problem hiding this comment.
[GEN6] Prefer exhaustive switch + assertNever over if/else on union types
FixedAwuCreditsTimeframeType is a string union. Branching on it with if/else instead of an exhaustive switch + assertNever means TypeScript won't enforce that all future values are handled. Please use a switch so any new fixed timeframe added to the union is a compile error here.
| if (timeframe === "contract_billing_cycle") { | |
| switch (timeframe) { | |
| case "contract_billing_cycle": { | |
| const periodResult = | |
| await getCachedMetronomeCurrentBillingPeriod(workspaceId); | |
| if (periodResult.isOk() && periodResult.value) { | |
| const { cycleStart, cycleEnd } = periodResult.value; | |
| return { | |
| label: `cycle-${cycleStart.getTime()}`, | |
| windowEndMs: cycleEnd.getTime(), | |
| }; | |
| } | |
| return computeCalendarWindowBounds("calendar_month", new Date()); | |
| } | |
| default: | |
| return computeCalendarWindowBounds(timeframe, new Date()); | |
| } |
| key: string; | ||
| timeframe: MaxAwuCreditsTimeframeType; | ||
| }): Promise<Result<number, Error>> { | ||
| if (isRollingAwuCreditsTimeframeType(timeframe)) { |
There was a problem hiding this comment.
[GEN6] Prefer exhaustive switch + assertNever over if/else on union types
Branching on MaxAwuCreditsTimeframeType via isRollingAwuCreditsTimeframeType + implicit else provides no compile-time guarantee that all non-rolling values are accounted for. If a new timeframe is added that is neither rolling nor fixed, it silently falls into the fixed-window path. Please prefer an exhaustive switch with assertNever on the full union so TypeScript enforces exhaustiveness.
52a1d1d to
548d25d
Compare
8373076 to
d8dd543
Compare
548d25d to
4ebb8a1
Compare
d8dd543 to
936c6fb
Compare
4ebb8a1 to
8ac32a4
Compare
58fd074 to
07806a8
Compare
|
🟡 1 security issue detected below the blocking threshold. Reviewed everything up to 812c61a. The following issues were found:
Evidence: In
Security Overview
Detected Code Changes
|
| const boundsResult = await resolveFairUseFixedWindowBounds( | ||
| workspaceId, | ||
| timeframe | ||
| ); | ||
| if (boundsResult.isErr()) { | ||
| logger.error( | ||
| { workspaceId, timeframe, err: boundsResult.error }, | ||
| "[FairUse] Could not resolve fixed window; skipping usage record" | ||
| ); | ||
| return; | ||
| } | ||
| await addFixedWindowCount({ | ||
| key, | ||
| bounds: boundsResult.value, | ||
| incrementBy, | ||
| logger, | ||
| }); |
There was a problem hiding this comment.
Authorization: AWU rate-limit skip on billing period resolve failure (front/lib/api/assistant/rate_limits.ts) (Severity: MEDIUM)
The vulnerability allows unlimited messages if the billing period cannot be resolved, as the fixed-window AWU recording skips usage and the pre-check treats the error as non-blocking. This leads to open enforcement when getCachedMetronomeCurrentBillingPeriod fails, enabling an attacker to bypass fair-use limits by triggering messages while resolution fails, which causes usage not to be accumulated against the plan.
View details in ZeroPath
Automatic patch generation was not possible for this finding.
💬 Reply @ZeroPath false-positive because … or @ZeroPath accepted-risk because … to triage this finding, or ask it any question.
8ac32a4 to
b5d6616
Compare
07806a8 to
64a93f5
Compare
b5d6616 to
8b74e44
Compare
64a93f5 to
cb38acf
Compare
POC (may not merge). Extends the plan-level AWU-credits fair-use limiter to support calendar fixed windows in addition to rolling ones. Owns the rate-limiter window vocabulary this needs, kept out of the base fixed-window primitive since only fair-use consumes it: - types/rate_limiter.ts: ROLLING vs CALENDAR timeframe groupings + isRollingAwuCreditsTimeframeType. - computeCalendarWindowBounds (UTC day/ISO-week/month boundaries) and the RollingAwuCreditsTimeframeType narrowing of getTimeframeSecondsFromLiteral. - MAX_AWU_CREDITS_TIMEFRAMES gains the calendar_* variants; callers route rolling vs calendar via isRollingAwuCreditsTimeframeType. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8b74e44 to
9163a2e
Compare
cb38acf to
812c61a
Compare
Description
POC — may not merge. Explores extending the plan-level fair-use AWU limiter (
plan.limits.assistant.maxAwuCredits/maxAwuCreditsTimeframe) to support fixed-window (calendar) timeframes in addition to rolling ones, so a plan's fair-use cap can reset on a clock boundary.MAX_AWU_CREDITS_TIMEFRAMES= rolling +FIXED_AWU_CREDITS_TIMEFRAMES(calendar day/week/month). Thecontract_billing_cyclewindow is not offered for plans (it's specific to the per-user spend cap in [front] Rate-limiter backup for the per-user spend cap #29576).getFairUseAwuCreditsCount/recordFairUseAwuCreditsdispatch to the rolling limiter or the fixed-window (calendar) counter based on the timeframe; enforcement (conversation), recording (credit_cost), status (user_block) and reset use them.Stacked on #29575; sibling of the spend-cap PR #29576.
Tests
fair_use_window.test.ts— round-trip for a rolling timeframe and a calendar (fixed) timeframe via the dispatch helpers (real Redis).tsgo --noEmitclean onfrontandfront-api.Risk
Low-to-medium, but currently no workspace uses the fair-use limiter (every plan ships
maxAwuCredits = -1), so this touches a dormant path. Rolling timeframes keep their exact previous behavior; only the dispatch layer is new. No schema/data changes — safe to roll back.Deploy Plan
Merge #29575 first. No migration/config. To try it: in poke set a plan's
maxAwuCreditsTimeframetocalendar_day/calendar_week/calendar_monthand a non--1maxAwuCredits, then exercise the fair-use limit on a workspace on that plan. (Being a POC, likely stays open for evaluation rather than merging.)🤖 Generated with Claude Code