Skip to content

docs: add rate-limiting guidance to CLAUDE.md#228

Merged
an9xyz merged 1 commit into
Mininglamp-OSS:mainfrom
dmwork-org:docs/rate-limiting-guidance
Jun 2, 2026
Merged

docs: add rate-limiting guidance to CLAUDE.md#228
an9xyz merged 1 commit into
Mininglamp-OSS:mainfrom
dmwork-org:docs/rate-limiting-guidance

Conversation

@an9xyz
Copy link
Copy Markdown
Contributor

@an9xyz an9xyz commented Jun 2, 2026

Summary

Adds a Rate Limiting section to CLAUDE.md so contributors mount the shared rate-limit middleware instead of hand-rolling Redis counters for request-frequency limiting.

What's documented

  • The three shared middlewares in pkg/wkhttp/ratelimit.go and their X-RateLimit-Scope values:
    • RateLimitMiddleware → scope ip (global per-IP DDoS floor, already mounted in main.go)
    • StrictIPRateLimitMiddleware → scope strict:{tag} (per-IP, unauthenticated sensitive endpoints)
    • SharedUIDRateLimiter → scope uid (per-login-user, default for authenticated endpoints)
  • All three set X-RateLimit-* headers, return i18n rate.limited, and are fail-open on Redis errors.
  • Mount-order requirement: SharedUIDRateLimiter must come after AuthMiddleware, otherwise it cannot read the uid and silently fails open.
  • Exception: per-resource cooldowns keyed by a business identity (sms_rate_limit:, email_rate_limit:, OIDC bind attempt caps) may keep their hand-written Redis counters — the IP/UID buckets cannot express that semantic.
  • Test note: routes behind UID limiting must reset ratelimit:uid:* in setup (CleanAllTables does not clear it).

Context

Docs-only; no code behavior change. Motivated by finding that POST /v1/app_bot/apply (modules/app_bot) hand-rolls a fixed-window Redis counter instead of using the shared middleware.

Test plan

  • N/A — documentation only

@an9xyz an9xyz requested a review from a team as a code owner June 2, 2026 10:01
@github-actions github-actions Bot added the size/S PR size: S label Jun 2, 2026
Copy link
Copy Markdown
Contributor

@lml2468 lml2468 left a comment

Choose a reason for hiding this comment

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

Summary

Docs-only: adds rate-limiting guidance to CLAUDE.md. Clear, accurate, well-structured. No blocking issues.

Covers the three-layer middleware stack (global IP / strict IP per-endpoint / shared UID), correct mounting order (after AuthMiddleware), exception policy for business-identity cooldowns, and test bucket cleanup. The checklist addition reinforces the "never hand-roll" rule.

APPROVED.

Copy link
Copy Markdown
Contributor

@Jerry-Xin Jerry-Xin left a comment

Choose a reason for hiding this comment

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

✅ APPROVE

Clean documentation addition — rate-limiting guidance for CLAUDE.md.

  • Three-layer middleware taxonomy is accurate and well-structured
  • SharedUIDRateLimiter mount-after-auth caveat is important and clearly called out
  • Exception clause for per-resource cooldowns (SMS/email/bind) is correctly scoped
  • Test setup reminder (resetUIDRateLimit) prevents CI flakes
  • Architecture section cross-reference added

No issues found.

Document the shared rate-limit middleware (ip / strict:{tag} / uid scopes
in pkg/wkhttp) and require it over hand-rolled Redis counters for generic
request-frequency limiting. Note the per-resource cooldown exception
(sms/email/OIDC bind) and the AuthMiddleware mount-order requirement.
@an9xyz an9xyz force-pushed the docs/rate-limiting-guidance branch from b2d7183 to b2803eb Compare June 2, 2026 10:05
Copy link
Copy Markdown
Contributor

@lml2468 lml2468 left a comment

Choose a reason for hiding this comment

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

Re-review (b2803eb)

Pure rebase onto main (includes merged #225 + #226). CLAUDE.md content unchanged. APPROVED.

Copy link
Copy Markdown
Contributor

@Jerry-Xin Jerry-Xin left a comment

Choose a reason for hiding this comment

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

✅ Re-Review: APPROVE

Delta is a rebase incorporating merged #225/#226 into main. The CLAUDE.md rate-limiting documentation is unchanged from the previous review. No new issues.

@an9xyz an9xyz merged commit 965f98b into Mininglamp-OSS:main Jun 2, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants