fix: add rate limiting to download counter to prevent inflation#40
Closed
orlyjamie wants to merge 1 commit intoopenclaw:mainfrom
Closed
fix: add rate limiting to download counter to prevent inflation#40orlyjamie wants to merge 1 commit intoopenclaw:mainfrom
orlyjamie wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Downloads can be trivially inflated by spamming the endpoint with spoofed X-Forwarded-For headers. This PR adds defense-in-depth measures: - Rate limit: 5 downloads per skill per IP per hour - Only trust cf-connecting-ip header (x-forwarded-for is spoofable) - Still serve the file even if rate limited, just don't count it NOTE: Download counts are fundamentally ungameable as trust metrics because they're anonymous. Even with rate limiting, attackers can use proxies/VPNs. Consider de-emphasizing downloads in the UI in favor of stars and installs which require authenticated sessions. Related: httpApiV1.ts:getClientIp() also trusts x-forwarded-for and should be audited for similar issues.
Contributor
|
@orlyjamie is attempting to deploy a commit to the Amantus Machina Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Author
Collaborator
|
Superseded by #278, which is merged on (merge commit ).\n\nThat PR implemented the download-inflation mitigation with tests and docs (hourly unique identity counting using API-token user id fallback to IP). Closing this one to avoid duplicate/conflicting changes. |
Collaborator
|
Superseded by #278, which is merged on That PR implemented the download-inflation mitigation with tests and docs (hourly unique identity counting using API-token user id fallback to IP). Closing this one to avoid duplicate/conflicting changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
The Vulnerability
The download endpoint at convex/downloads.ts:56 incremented download counts with no rate limiting. An
attacker could inflate any skill's download count by:
Important Note
Download counts are fundamentally gameable as trust metrics because downloads are anonymous. Even with
rate limiting, attackers can use proxies/VPNs/Tor.
Recommendation: De-emphasize downloads in the UI. Stars and installs are better trust signals because they
require authenticated sessions.
Reference: https://x.com/theonejvo/status/2015892980851474595