Skip to content

fix(api): add community write abuse admission - #82

Merged
sayed710 merged 4 commits into
mainfrom
claude/community-abuse-admission
Oct 1, 2026
Merged

sayed710 merged 4 commits into
mainfrom
claude/community-abuse-admission

Conversation

@sayed710

@sayed710 sayed710 commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

What this closes

Fable+Astra audit findings P1-35 (social writes) and P1-36 (team/forum writes). This is the "Social / messaging / teams / forums … no abuse budget" row of docs/audits/FABLE_ASTRA_FULL_AUDIT.md.

Reverified on origin/main 3a10e3c:

  • Already metered, with atomic per-user + per-IP admission: seek creation, conversation open and message send.
  • Not metered: follow, friend request, team creation, public join, join request, thread creation and reply. A declined friend request could also be re-sent without limit.
  • GraphQL is read-only, and the realtime gateway carries only game commands. Neither is a bypass.

Backend only. No packages/web, ADR or docs/audits/GEMINI_* changes, so no overlap with #81 or #80.

Endpoint classification

Route Classification Admission
POST /v1/social/follows/:playerId Outbound initiation (edge in the target's public followers list) socialInitiation
POST /v1/social/friend-requests Outbound initiation (unsolicited request in the target's inbox) socialInitiation
DELETE /v1/social/follows/:playerId Clean-up of own edge unmetered
POST /v1/social/friend-requests/:id/respond Accept / decline / cancel a request someone's budget already paid for unmetered
POST /v1/social/blocks/:playerId Safety (also tears down follows and friendship) unmetered
DELETE /v1/social/blocks/:playerId Safety reversal (private list; re-following afterwards is metered) unmetered
POST /v1/messages/conversations, …/:id/messages Already metered; unchanged existing
POST /v1/teams Creation (listed team, claimed unique slug) teamCreation
PATCH /v1/teams/:id Governance of an existing row unmetered
POST /v1/teams/:id/members Outbound initiation (appears in another team's public member list) teamJoin
POST /v1/teams/:id/join-requests Outbound initiation (item in another team's admin queue) teamJoin
DELETE /v1/teams/:id/members/:playerId Leave / remove (clean-up, moderation) unmetered
PATCH /v1/teams/:id/members/:playerId Role change (governance) unmetered
POST /v1/teams/:id/transfer-ownership Governance unmetered
POST /v1/teams/:id/join-requests/:reqId/respond Admin answering a request the requester paid for unmetered
DELETE /v1/teams/:id/join-requests/:reqId Requester withdrawing own request unmetered
POST /v1/teams/:id/forum/threads Creation (thread + opening post) forumThreadCreation
PATCH / DELETE /v1/teams/:id/forum/threads/:threadId Edit, lock, pin, tombstone (moderation) unmetered
POST /v1/teams/:id/forum/threads/:threadId/posts Creation (reply) forumPostCreation
PATCH / DELETE /v1/forum/posts/:postId Author edit, tombstone unmetered

The unmetered list is pinned by a structural test. Blocking an abuser, declining a request, leaving a team or deleting a post must never fail because a creation budget ran out.

Policy categories and rationale

Category Per account Per IP Shape reused
socialInitiation 20 / 5 min 200 / 5 min conversationCreation: reaching out to another player
teamCreation 5 / h 50 / h the hourly register / webauthnRegister shape for rarely repeated durable creations
teamJoin 20 / 5 min 200 / 5 min conversationCreation: reaching out to a team
forumThreadCreation 20 / 5 min 200 / 5 min conversationCreation: opening a thread
forumPostCreation 30 / min 300 / min messageSend: a reply
friendRequestRepeat (per sender → recipient pair) 3 / 24 h n/a count from emailVerificationRequest.perUser (repeated sends to one inbox); a day-long window because the recipient bears the cost
  • No new numbers were invented. Each category copies an existing shape.
  • Follow shares a budget with friend request, and public join with join request. Each pair is one kind of outbound act, so sharing stops alternating between two routes to double the rate.
  • Thread and reply are separate, so a long discussion cannot use up the ability to start a thread.
  • A friend request also charges the pair bucket in the same atomic admission. It was added after Qodo's exact-head review of f31385b: without it, the account budget alone let one sender re-send a declined request to the same player about 5,760 times a day. The key is friend-request:pair:${sender}:${recipient-lowercased}, so only the sender can charge it. The recipient can still ask the sender, and others can still ask the recipient; tests prove both.
  • Follow charges even when the edge already exists. This bounds repeated get-or-create calls exactly as conversation open does.

NAT fairness and target lockout

  • Every IP allowance is ten account budgets, the margin seekCreation, conversationCreation and messageSend already use. Ten players behind one NAT can each use their full budget.
  • Keys are <category>:user:${actorId} and <category>:ip:${ctx.ip ?? 'unknown'}. The one exception is the sender-first friend-request pair key. No key lets anyone but the caller spend it, so strangers aiming writes at a victim spend only their own budgets.
  • A structural test pins the key shape on all ten authenticated-write routes, including seek and messaging: caller, then address, then (friend requests only) sender → recipient.
  • Adding a per-team or per-recipient bucket would let one member or stranger lock everyone else out, so the design deliberately has none.

Admission semantics

  • One atomic call. Each route makes exactly one admit([user, ip]), the existing multi-bucket port, so a refusal charges neither bucket.
  • Nothing written on refusal. Admission runs after cheap validation and before any repository call. A refusal is a 429 with the standard Retry-After, and nothing is written.
  • Cheap validation first. Self-relations in any UUID case, malformed slugs, and blank names, titles and bodies are refused before charging. They go through the domain's own validators, with unchanged error contracts. This also closes an upper-cased self-follow that main did not reject as 422.
  • Fails closed. A limiter fault is a 500 with nothing written.
  • Disabled switch preserved. rateLimit.enabled = false still bypasses admission.
  • Authorization unchanged. No existence lookup was added before admission.
  • Config validation. validateMessagingRateLimits became validateWriteRateLimits over all eight authenticated-write categories. A missing, non-integer, non-positive or over-32-bit budget, or a malformed policy shape, fails startup.
  • OpenAPI. The seven routes document 429 with Retry-After. Follow and friend request also document their existing 404. openapi.json is regenerated.

Evidence

Failing before: the final test files, run against main's routes.ts and config.ts.

Suite Failing on main Failure reason
community-admission.test.ts 14 of 17 200/201 where 429 was expected; 8 of 8 concurrent posts admitted; 200 where a limiter fault should be 500; no 429 in OpenAPI; self-relation charged or accepted
rate-limit-structure.test.ts 4 of 9 the new structural checks
config.test.ts 2 of 21 the new default and validation checks

The 3 community tests that pass on main pin invariants that already held there: the victim's budgets are untouched, a disabled limiter charges nothing, and malformed input is rejected.

Falsification: 23 disposable mutations of the final code, all caught. Sources were restored byte-identical from a disk backup after each.

  1. Remove the admission.
  2. Key the account bucket by the target.
  3. Key the IP bucket by the caller.
  4. Split into two sequential admissions.
  5. Drop an IP bucket.
  6. Key the join bucket by the team.
  7. Move admission after the write.
  8. Remove the thread admission.
  9. Meter block.
  10. Ignore enabled.
  11. Stop validating a category.
  12. Charge before the self check.
  13. Compare self case-sensitively.
  14. Drop the slug pre-check.
  15. Drop the post-body pre-check.
  16. Drop the friend-request pair bucket.
  17. Keep the recipient's case in the pair key.
  18. Key the pair by the recipient alone.
  19. Key the pair by the sender alone.
  20. Stop validating socialInitiation (validator's final shape).
  21. Stop validating friendRequestRepeat.
  22. Key the join bucket by the team (final placement).
  23. Move the join-request admission after the write.

Local validation:

Check Result
Build, lint pass
Every check:* guard (8) pass
API unit 1143 / 1143, 0 skipped
Hermetic workspaces 19 workspaces, 3,609 tests, 0 skipped
API PostgreSQL suite (fresh pgvector PostgreSQL 16) 65 / 65, 0 skipped

The PostgreSQL suite includes a new two-replica test: shared budgets, refusal before write, and one-winner races for posts and for join vs. join request.

Reviews:

  • Delegated read-only reviews: architecture, security and test quality ran on Gemini 3.8 Flash High, and concurrency on Claude Sonnet 4.6. Every valid finding is fixed: pre-charge validation, the join race, Retry-After/404 docs, and five test-strength gaps.
  • Independent read-only review (Gemini 3.8 Flash High) before the first push: CLEAN. On f31385b it reported the same re-send gap as Qodo (fixed), a missing openapi.json (a false positive: the file was only left out of the review patch), and one consistency point (fixed: both join routes now admit inside their try like the other five).

Not addressed here (pre-existing or out of scope)

  • POST /v1/studies/:id/collaborators is unmetered. It is study sharing, not P1-35/36, and is worth a follow-up.
  • The shared unknown IP bucket when a proxy drops X-Forwarded-For is a deployment concern shared by every metered route.
  • editPost takes the team advisory lock in PostgreSQL. Unmetered edits can contend with it, but deletion stays deliberately unmetered.
  • PgRateLimiter measures every bucket before rolling back a refusal. This is deliberate, so Retry-After is the longest wait, and it is unchanged.
  • Distributed Sybil abuse (many accounts on many addresses) is beyond per-account and per-IP limits by design.
  • No rematch, challenges, notifications, moderation UI or other product changes.

PROJECT_STATE

Appended as M15 Increment 79, the next number after main's latest (78); an open PR does not reserve an increment. Prior history is unchanged and append-only. Both this PR and #81 edit the _Last updated_ line, so whichever merges second renumbers its own entry and reconciles that line.

Merge decision is the owner's.

Summary by CodeRabbit

  • New Features
    • Added rate limits for following players, sending friend requests, creating teams and forum threads, joining teams, and posting in forums. Limits can apply per account, IP address, or friend-request pair.
    • Requests that exceed a limit receive a 429 response with a Retry-After header. Invalid requests are checked before they count toward limits.
  • Documentation
    • Updated API documentation to describe rate-limit responses and missing-player errors for social actions.

Close audit P1-35 (social writes) and P1-36 (team/forum writes): follow,
friend request, team creation, public join, join request, thread creation
and reply had no abuse budget, and a declined friend request could be
re-sent without limit.

Each of the seven creation routes now makes one atomic admission of a
per-account and a per-IP bucket, after cheap validation and before the
repository is touched. Keys name only the caller and the caller's
address, never the target player or team, so writes aimed at a player
cannot spend that player's budget. Five categories reuse existing
shapes with the 10-account shared-NAT margin: socialInitiation (20/5min,
200/5min), teamCreation (5/h, 50/h), teamJoin (20/5min, 200/5min),
forumThreadCreation (20/5min, 200/5min) and forumPostCreation (30/min,
300/min).

Self-relations (in any UUID case), malformed slugs and blank names,
titles and bodies are rejected by the domain's own validators before
charging, with unchanged error contracts. Safety, clean-up and
governance routes (unfollow, respond, block, unblock, leave/remove,
role, transfer, join-request respond/cancel, thread/post edit/delete)
stay unmetered, pinned by a structural test.

The messaging budget validator becomes validateWriteRateLimits over all
eight authenticated-write categories. The seven routes document 429 with
Retry-After (follow and friend request also document 404), and
openapi.json is regenerated.
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0692b862-f6a3-484d-b047-4df291ea3146

📥 Commits

Reviewing files that changed from the base of the PR and between 3a10e3c and 805081e.

📒 Files selected for processing (8)
  • docs/PROJECT_STATE.md
  • packages/api/openapi.json
  • packages/api/src/config.ts
  • packages/api/src/routes.ts
  • packages/api/test/community-admission.integration.test.ts
  • packages/api/test/community-admission.test.ts
  • packages/api/test/config.test.ts
  • packages/api/test/rate-limit-structure.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The API applies per-user and per-IP rate limits to social, team, and forum write routes. Friend-request creation also has a sender-recipient pair limit. Selected input validation and self-relation checks occur before admission. The API documents rate-limit responses, and tests cover configuration, route behavior, concurrency, and shared admission across replicas.

Changes

Community write admission

Layer / File(s) Summary
Admission policies and configuration
packages/api/src/config.ts, packages/api/test/config.test.ts
Adds budgets for social initiation, team creation and joining, forum thread and post creation, and repeated friend requests. Configuration resolution validates the dimensions and limits.
Community route admission and response contracts
packages/api/src/routes.ts, packages/api/openapi.json
Applies admission to social, team, and forum write routes. Selected input validation and self-relation checks run before admission. OpenAPI documents 429 responses and 404 responses for nonexistent social targets.
Admission validation and handover
packages/api/test/community-admission.test.ts, packages/api/test/community-admission.integration.test.ts, packages/api/test/rate-limit-structure.test.ts, docs/PROJECT_STATE.md
Tests cover configured budgets, validation order, refused and concurrent writes, unmetered actions, limiter failures, and shared admission across API replicas. The project handover records the increment and validation results.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant CommunityRoute
  participant RateLimiter
  participant Repository
  Client->>CommunityRoute: Submit community write
  CommunityRoute->>RateLimiter: Check configured admission buckets
  RateLimiter-->>CommunityRoute: Admit or reject
  CommunityRoute->>Repository: Persist admitted write
  CommunityRoute-->>Client: Return result or rate-limit response
Loading

Suggested reviewers: edwardnewgate710, griffinshanks

Merge Risk: ⚪ Minimal · up to 80508

The change adds community write limits with documented refusals and coverage for rejected writes and concurrent requests. No concrete merge-blocking issue remains; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 80508

The change adds abuse controls without a demonstrated new security weakness. Remaining uncertainty concerns deployment-wide enforcement and recovery after interrupted writes, rather than a verified bypass.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The controls constrain authenticated initiation into follower lists, friend-request inboxes, team membership queues, and forums. Account buckets belong to the caller, not the target player, team, or thread. IP budgets are shared by callers resolving to the same address, while the repeat-friend-request budget distinguishes sender and recipient.

Trust Boundaries and Controls

  • observed — Admission uses authenticated actor identity and centralized proxy-aware IP resolution rather than request-supplied account identity. With limiting enabled, refusals become 429 responses and limiter exceptions propagate to the router's error path before persistence. A missing resolved address uses the inherited shared unknown bucket.

Resilience and Maintainability Implications

  • observed — The reused PostgreSQL limiter charges multi-bucket admissions all-or-nothing, orders locks consistently, bounds lock and statement waits, and destroys connections whose rollback cannot establish a clean state. Expired windows reset on admission, with opportunistic deletion of long-expired rows. These mechanisms support fail-closed recovery for the newly metered writes.

Hardening Proposals

  • proposed — Verify that every serving replica uses the new enabled policies and that proxy-hop configuration matches deployment reality. Treat mixed-version rollout or rollback as incomplete enforcement of the new controls, not as a verified vulnerability introduced by this PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 6 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the main change: adding community write abuse admission to the API.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 6 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@sayed710

Copy link
Copy Markdown
Owner Author

/review

@sayed710

Copy link
Copy Markdown
Owner Author

@greptileai review

@sayed710

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add abuse admission to social, team, and forum writes

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Limit seven previously unmetered community creation routes using atomic caller and address
 budgets.
• Reject invalid requests before admission while keeping safety, clean-up, and governance actions
 unmetered.
• Cover refusals, concurrent writes, configuration, and API responses with tests and updated OpenAPI
 documentation.
Diagram

graph TD
  Caller["Authenticated caller"] --> Validate["Route validation"] --> Admit["Atomic admission"] --> Repo["Community repositories"]
  Admit --> User[("Caller bucket")]
  Admit --> IP[("Address bucket")]
  Admit -- "429 refusal" --> Caller
Loading
High-Level Assessment

Reusing the existing atomic admission port and separate caller/address buckets fits the established seek and messaging policy. A new limiter or target-keyed budgets would add complexity or let activity aimed at a target exhaust someone else's allowance.

Files changed (8) +1169 / -9

Bug fix (2) +142 / -6
config.tsDefine and validate community write budgets +77/-4

Define and validate community write budgets

• Adds five caller-and-address rate-limit categories. Extends startup validation to all eight authenticated-write categories.

packages/api/src/config.ts

routes.tsAdmit seven community writes before persistence +65/-2

Admit seven community writes before persistence

• Adds atomic caller-and-address admission to follows, friend requests, team creation and joins, threads, and replies. Runs domain validation first where needed and documents rate-limit responses.

packages/api/src/routes.ts

Tests (4) +859 / -2
community-admission.integration.test.tsVerify admission across PostgreSQL-backed replicas +130/-0

Verify admission across PostgreSQL-backed replicas

• Tests shared budgets, refusal-before-write behavior, and single-winner races across two API instances.

packages/api/test/community-admission.integration.test.ts

community-admission.test.tsExercise community admission behavior +575/-0

Exercise community admission behavior

• Covers caller and address limits, shared categories, validation, target isolation, concurrent requests, limiter faults, disabled limiting, safety actions, and OpenAPI responses.

packages/api/test/community-admission.test.ts

config.test.tsTest write-budget defaults and startup validation +68/-1

Test write-budget defaults and startup validation

• Checks community defaults and rejects missing, malformed, or out-of-range policies across authenticated-write categories.

packages/api/test/config.test.ts

rate-limit-structure.test.tsGuard admission placement and key ownership +86/-1

Guard admission placement and key ownership

• Requires one two-bucket admission before repository access, caller-only and address-only keys, and no admission on safety, clean-up, or governance routes.

packages/api/test/rate-limit-structure.test.ts

Documentation (2) +168 / -1
PROJECT_STATE.mdRecord the community admission milestone +15/-1

Record the community admission milestone

• Documents the audit findings, route classifications, budget choices, tests, and validation results.

docs/PROJECT_STATE.md

openapi.jsonPublish community rate-limit responses +153/-0

Publish community rate-limit responses

• Adds 429 responses with Retry-After headers to seven creation endpoints and documents existing 404 responses for the social routes.

packages/api/openapi.json

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@qodo-code-review

qodo-code-review Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Informational

1. A declined sender can keep re-requesting one person ✓ Resolved
Description
The friend-request admission keys only on the caller's account and IP
(social-initiate:user:${actorId} / social-initiate:ip:…), with no pair-scoped counter and no
cooldown after a decline. At the default of 20 requests per 5 minutes, one account can re-send a
declined request to the same recipient up to about 5,760 times a day, and the recipient's only way
to stop it is to block the sender.
Code

packages/api/src/routes.ts[R2959-2963]

+        assertDistinct(actorId.toLowerCase(), addresseeId.toLowerCase());
+        await admit([
+          { key: `social-initiate:user:${actorId}`, limit: config.rateLimit.socialInitiation.perUser },
+          { key: `social-initiate:ip:${ctx.ip ?? 'unknown'}`, limit: config.rateLimit.socialInitiation.perIp },
+        ]);
Relevance

●● Moderate

Security gap is plausible, but pair-scoped throttling conflicts with the PR's deliberate caller-only
admission policy.

PR-#63
PR-#74

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The admission keys contain only the caller's ID and IP. The PR's own loop test shows re-sends stop
only when the sender's overall budget runs out. At the default settings that budget refills every 5
minutes, so repeat requests to one recipient are limited only by the sender's total budget.

packages/api/src/routes.ts[2959-2964]
packages/api/src/config.ts[291-294]
packages/api/test/community-admission.test.ts[104-128]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR says the unlimited decline-then-resend loop is closed, but a sender's overall socialInitiation budget is the only limit. At the default of 20 per 5 minutes, one account can re-send a declined request to the same recipient thousands of times a day, and each one reaches the recipient's inbox.

## Fix Focus Areas
- packages/api/src/routes.ts[2959-2963]
- packages/api/src/config.ts[291-294]

## Recommended Fix
Add a cooldown per sender–recipient pair after a decline. Enforce it in the social domain, for example with `sendFriendRequest` returning 409 while the same pair has a recent decline. Alternatively, add a second admission key scoped to that sender and recipient, charged only to the sender. Either way, no key may let someone spend another player's budget. Add a test that re-sending after a decline is refused within the cooldown.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⏭️ Skipped: The latest push only corrects milestone numbering and related historical documentation text in docs/PROJECT_STATE.md, with no behavioral or runtime change.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 805081e

Results up to commit f31385b 🧠 Deep


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Informational
1. A declined sender can keep re-requesting one person ✓ Resolved
Description
The friend-request admission keys only on the caller's account and IP
(social-initiate:user:${actorId} / social-initiate:ip:…), with no pair-scoped counter and no
cooldown after a decline. At the default of 20 requests per 5 minutes, one account can re-send a
declined request to the same recipient up to about 5,760 times a day, and the recipient's only way
to stop it is to block the sender.
Code

packages/api/src/routes.ts[R2959-2963]

+        assertDistinct(actorId.toLowerCase(), addresseeId.toLowerCase());
+        await admit([
+          { key: `social-initiate:user:${actorId}`, limit: config.rateLimit.socialInitiation.perUser },
+          { key: `social-initiate:ip:${ctx.ip ?? 'unknown'}`, limit: config.rateLimit.socialInitiation.perIp },
+        ]);
Relevance

●● Moderate

Security gap is plausible, but pair-scoped throttling conflicts with the PR's deliberate caller-only
admission policy.

PR-#63
PR-#74

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The admission keys contain only the caller's ID and IP. The PR's own loop test shows re-sends stop
only when the sender's overall budget runs out. At the default settings that budget refills every 5
minutes, so repeat requests to one recipient are limited only by the sender's total budget.

packages/api/src/routes.ts[2959-2964]
packages/api/src/config.ts[291-294]
packages/api/test/community-admission.test.ts[104-128]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR says the unlimited decline-then-resend loop is closed, but a sender's overall socialInitiation budget is the only limit. At the default of 20 per 5 minutes, one account can re-send a declined request to the same recipient thousands of times a day, and each one reaches the recipient's inbox.

## Fix Focus Areas
- packages/api/src/routes.ts[2959-2963]
- packages/api/src/config.ts[291-294]

## Recommended Fix
Add a cooldown per sender–recipient pair after a decline. Enforce it in the social domain, for example with `sendFriendRequest` returning 409 while the same pair has a recent decline. Alternatively, add a second admission key scoped to that sender and recipient, charged only to the sender. Either way, no key may let someone spend another player's budget. Add a test that re-sending after a decline is refused within the cooldown.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread packages/api/src/routes.ts
@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Critical risk] Adds rate limiting to community write operations across the API.

The PR appears safe to merge from a code-behavior standpoint, but its project-state numbering should be coordinated with PR #81.

Findings

  1. P2 Duplicate increment number ▶
Fix with agent prompt
### Issue 1
docs/PROJECT_STATE.md:4562
This entry is now labeled Increment 79, but the text removed from this PR records that the separate PR #81 already claims 79. If both PRs merge with that number, references to “Increment 79” in the project handover will be ambiguous. Keep this entry at 80 or coordinate the numbering with #81.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds atomic account-and-IP admission to seven community write routes, a sender–recipient repeat budget for friend requests, configuration validation, OpenAPI responses, and tests. The latest change renumbers its project-state entry to an increment already claimed by another PR.

Reviews (3) · Last reviewed commit: "docs: renumber the community admission e..."

The account and address budgets alone still let one sender re-send a
declined friend request to the same player about 5,760 times a day
(exact-head Qodo finding). A friend request now also charges
friendRequestRepeat.perPair (3 per 24 h), keyed
friend-request:pair:<sender>:<recipient>, in the same atomic admission.
The key names the sender first, so only the sender's own requests
charge it: the recipient can still ask the sender, and others can still
ask the recipient. The recipient is lower-cased so re-casing the UUID
cannot reset the pair.

The write-budget validator now carries each category's dimensions, so
the new single-dimension policy is validated like the others. Both join
routes admit inside their try block like the other five routes.
@sayed710

Copy link
Copy Markdown
Owner Author

/review

@sayed710

Copy link
Copy Markdown
Owner Author

@greptileai review

@sayed710

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 19 minutes.

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 7bd2cf6

An open PR does not reserve an increment; main's latest is 78.
@sayed710

sayed710 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

/review

@sayed710

sayed710 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

@greptileai review

@sayed710

sayed710 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 805081e

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

Comment thread docs/PROJECT_STATE.md
@sayed710
sayed710 merged commit 0639043 into main Oct 1, 2026
11 checks passed
@sayed710
sayed710 deleted the claude/community-abuse-admission branch October 1, 2026 04:05
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