openclaw/clawhub
1
0
Fork 0
mirror of https://github.com/openclaw/clawhub.git synced 2026-07-21 19:25:25 -04:00
Code Issues 183 Packages Projects Releases 1 Wiki Activity

[PR #42] [CLOSED] fix: centralize rate limiting and CF IP #228

New Issue
Closed
opened 2026-02-15 17:16:23 -05:00 by yindo · 0 comments
yindo commented 2026-02-15 17:16:23 -05:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/openclaw/clawhub/pull/42
Author: @regenrek
Created: 1/26/2026
Status: ❌ Closed

Base: main ← Head: fix/rate-limit-helper-cf-ip

📝 Commits (2)

  • 36fdfe2 fix: centralize rate limiting and CF IP
  • 20837c2 fix: allow opt-in forwarded IP headers

📊 Changes

3 files changed (+149 additions, -89 deletions)

View changed files

📝 convex/httpApiV1.ts (+2 -89)
➕ convex/lib/httpRateLimit.test.ts (+35 -0)
➕ convex/lib/httpRateLimit.ts (+112 -0)

📄 Description

summary

  • Centralize rate limit helpers and enforce CF-only IP parsing.
  • Add opt-in forwarded-header support for non-Cloudflare deployments.

motivation

  • Remove duplicated logic, block spoofing by default, and avoid breaking non-CF setups.

what's included

  • Shared rate-limit helper in convex/lib/httpRateLimit.ts.
  • convex/httpApiV1.ts updated to use helper.
  • Optional forwarded IP support gated by TRUST_FORWARDED_IPS.
  • Tests for CF-only and opt-in behavior.

what's not included

  • Download endpoint changes or dedupe logic.
  • UI changes.

tests

  • bun run test
  • bun run lint

affected files

  • convex/httpApiV1.ts
  • convex/lib/httpRateLimit.ts
  • convex/lib/httpRateLimit.test.ts

prompt

# ClawdHub Security Hardening

## Goal & Success Criteria

- Block download inflation: rate limit + per‑IP/day dedupe on ZIP downloads.
- IP spoofing fixed: trust only cf-connecting-ip.
- Files tab shows full file viewer + warnings for dangerous commands.
- Each PR has Conventional Commits, full test suite runs, PR opened from regenrek fork.

## Non‑goals / Out of Scope

- Replace download stats with installs.
- Auth‑gated downloads or paid access.
- Deep static analysis beyond warning heuristics.

## Assumptions

- Rate limits: new “download” tier tighter than “read”.
- IP trust: CF‑only, no fallback.
- PRs live on regenrek fork, not upstream.

## Proposed Solution

- Single canonical rate‑limit/IP module (Convex best‑practice: no duplicated logic).
- Dedupe table keyed by hashed IP + skill + day bucket; increment only once.
- UI file viewer loads via existing getFileText; warnings from regex scan.

### Alternatives Considered

- Reuse “read” limits in convex/httpApiV1.ts:634 — too permissive.
- Fallback to x-forwarded-for — violates CF‑only requirement.
- Store raw IPs — privacy risk.

## System Design

- Increment flow: new mutation recordDownload handles dedupe + stats increment atomically.
- Cleanup: cron job prunes dedupe rows older than N days.
- UI: SkillDetailPage Files tab extracted to SkillFilesPanel with viewer + warnings.

## Interfaces & Data Contracts

- recordDownload mutation args: { skillId: Id<'skills'>, ipHash?: string, dayStart: number }.
- downloadDedupes schema: { skillId, ipHash, dayStart, createdAt }.
- Rate limit config includes download: { ip, key } in shared helper.

## Execution Details

PR 1 — IP + Rate‑Limit Helper (fix/refactor)

- Extract applyRateLimit/getClientIp from convex/httpApiV1.ts:634 into convex/lib/httpRateLimit.ts.
- Update convex/httpApiV1.ts to use shared helper.
- Enforce CF‑only IP parsing.

## Testing & Quality

- Full suite per PR: bun run test and bun run lint.
- Add unit tests for dedupe/rate limit in convex/downloads.test.ts.
- Extend handler tests to cover CF‑only IP logic.

## Risks & Mitigations

- Missing CF header → “unknown” key. Mitigate by documenting requirement.
- Dedupe table growth → cron pruning.
- Viewer perf → relies on existing 200KB cap.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/openclaw/clawhub/pull/42 **Author:** [@regenrek](https://github.com/regenrek) **Created:** 1/26/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/rate-limit-helper-cf-ip` --- ### 📝 Commits (2) - [`36fdfe2`](https://github.com/openclaw/clawhub/commit/36fdfe2f806d56b48b69337642947ec9944cf478) fix: centralize rate limiting and CF IP - [`20837c2`](https://github.com/openclaw/clawhub/commit/20837c22df616832432b59f9c51a388076c7d0c0) fix: allow opt-in forwarded IP headers ### 📊 Changes **3 files changed** (+149 additions, -89 deletions) <details> <summary>View changed files</summary> 📝 `convex/httpApiV1.ts` (+2 -89) ➕ `convex/lib/httpRateLimit.test.ts` (+35 -0) ➕ `convex/lib/httpRateLimit.ts` (+112 -0) </details> ### 📄 Description ### summary - Centralize rate limit helpers and enforce CF-only IP parsing. - Add opt-in forwarded-header support for non-Cloudflare deployments. ### motivation - Remove duplicated logic, block spoofing by default, and avoid breaking non-CF setups. ### what's included - Shared rate-limit helper in `convex/lib/httpRateLimit.ts`. - `convex/httpApiV1.ts` updated to use helper. - Optional forwarded IP support gated by `TRUST_FORWARDED_IPS`. - Tests for CF-only and opt-in behavior. ### what's not included - Download endpoint changes or dedupe logic. - UI changes. ### tests - `bun run test` - `bun run lint` ### affected files - `convex/httpApiV1.ts` - `convex/lib/httpRateLimit.ts` - `convex/lib/httpRateLimit.test.ts` ### prompt ``` # ClawdHub Security Hardening ## Goal & Success Criteria - Block download inflation: rate limit + per‑IP/day dedupe on ZIP downloads. - IP spoofing fixed: trust only cf-connecting-ip. - Files tab shows full file viewer + warnings for dangerous commands. - Each PR has Conventional Commits, full test suite runs, PR opened from regenrek fork. ## Non‑goals / Out of Scope - Replace download stats with installs. - Auth‑gated downloads or paid access. - Deep static analysis beyond warning heuristics. ## Assumptions - Rate limits: new “download” tier tighter than “read”. - IP trust: CF‑only, no fallback. - PRs live on regenrek fork, not upstream. ## Proposed Solution - Single canonical rate‑limit/IP module (Convex best‑practice: no duplicated logic). - Dedupe table keyed by hashed IP + skill + day bucket; increment only once. - UI file viewer loads via existing getFileText; warnings from regex scan. ### Alternatives Considered - Reuse “read” limits in convex/httpApiV1.ts:634 — too permissive. - Fallback to x-forwarded-for — violates CF‑only requirement. - Store raw IPs — privacy risk. ## System Design - Increment flow: new mutation recordDownload handles dedupe + stats increment atomically. - Cleanup: cron job prunes dedupe rows older than N days. - UI: SkillDetailPage Files tab extracted to SkillFilesPanel with viewer + warnings. ## Interfaces & Data Contracts - recordDownload mutation args: { skillId: Id<'skills'>, ipHash?: string, dayStart: number }. - downloadDedupes schema: { skillId, ipHash, dayStart, createdAt }. - Rate limit config includes download: { ip, key } in shared helper. ## Execution Details PR 1 — IP + Rate‑Limit Helper (fix/refactor) - Extract applyRateLimit/getClientIp from convex/httpApiV1.ts:634 into convex/lib/httpRateLimit.ts. - Update convex/httpApiV1.ts to use shared helper. - Enforce CF‑only IP parsing. ## Testing & Quality - Full suite per PR: bun run test and bun run lint. - Add unit tests for dedupe/rate limit in convex/downloads.test.ts. - Extend handler tests to cover CF‑only IP logic. ## Risks & Mitigations - Missing CF header → “unknown” key. Mitigate by documenting requirement. - Dedupe table growth → cron pruning. - Viewer perf → relies on existing 200KB cap. ``` --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
yindo added the pull-request label 2026-02-15 17:16:23 -05:00
yindo closed this issue 2026-02-15 17:16:23 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
pull-request
Mirrored from GitHub Pull Request
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: openclaw/clawhub#228
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?