Conversation
Greptile SummaryThis PR systematically adds configurable resource limits across the platform by introducing 15 new fields in Key findings:
The majority of the changes are mechanical and correct — adding config fields and count guards consistently across routes. However, the SSO config logic bug creates a semantic inconsistency that will silently override admin intent when Confidence Score: 3/5
Sequence DiagramsequenceDiagram
participant Client
participant Route as Route Handler
participant Config as ResourceLimits (config)
participant Service
participant DB as Database (read pool)
Client->>Route: POST /resource (create request)
Route->>Route: Authenticate & authorize
Route->>Config: Read max_<resource>_per_<scope>
alt max == 0 (unlimited)
Route->>Route: Skip limit check (proceed)
Note over Route: ⚠️ Exception: SSO configs fall into<br/>else-if and still enforce 1-per-org
else max > 0
Route->>Service: count_by_<scope>(id)
Service->>DB: SELECT COUNT(*) … WHERE … AND deleted_at IS NULL
DB-->>Service: current_count
Service-->>Route: current_count
alt current_count >= max
Route-->>Client: 409 Conflict (limit reached)
else current_count < max
Note over Route,DB: ⚠️ TOCTOU window — concurrent requests<br/>can both pass this check
Route->>Service: create(input)
Service->>DB: INSERT …
DB-->>Service: new record
Service-->>Route: created resource
Route-->>Client: 201 Created
end
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Check per-scope API key limits | ||
| let limits = &state.config.limits.resource_limits; | ||
| match &input.owner { | ||
| crate::models::ApiKeyOwner::Organization { org_id } => { | ||
| let max = limits.max_api_keys_per_org; | ||
| if max > 0 { | ||
| let count = services.api_keys.count_by_org(*org_id, false).await?; | ||
| if count >= max as i64 { | ||
| return Err(AdminError::Conflict(format!( | ||
| "Organization has reached the maximum number of API keys ({max})" | ||
| ))); | ||
| } | ||
| } | ||
| } | ||
| crate::models::ApiKeyOwner::Team { team_id } => { | ||
| let max = limits.max_api_keys_per_team; | ||
| if max > 0 { | ||
| let count = services.api_keys.count_by_team(*team_id, false).await?; | ||
| if count >= max as i64 { | ||
| return Err(AdminError::Conflict(format!( | ||
| "Team has reached the maximum number of API keys ({max})" | ||
| ))); | ||
| } | ||
| } | ||
| } | ||
| crate::models::ApiKeyOwner::Project { project_id } => { | ||
| let max = limits.max_api_keys_per_project; | ||
| if max > 0 { | ||
| let count = services | ||
| .api_keys | ||
| .count_by_project(*project_id, false) | ||
| .await?; | ||
| if count >= max as i64 { | ||
| return Err(AdminError::Conflict(format!( | ||
| "Project has reached the maximum number of API keys ({max})" | ||
| ))); | ||
| } | ||
| } | ||
| } | ||
| crate::models::ApiKeyOwner::User { user_id } => { | ||
| let max = limits.max_api_keys_per_user; | ||
| if max > 0 { | ||
| let count = services.api_keys.count_by_user(*user_id, false).await?; | ||
| if count >= max as i64 { | ||
| return Err(AdminError::Conflict(format!( | ||
| "User has reached the maximum number of API keys ({max})" | ||
| ))); | ||
| } | ||
| } | ||
| } | ||
| crate::models::ApiKeyOwner::ServiceAccount { .. } => {} | ||
| } |
There was a problem hiding this comment.
All limit checks throughout this PR follow a count() → compare() → create() pattern without transactional isolation. Under concurrent load, two or more requests can both read the same count (e.g., max - 1), both pass the limit check, and both proceed to create, resulting in the final count exceeding the configured limit.
This TOCTOU (time-of-check / time-of-use) race affects every limit-checked route in this PR: teams, projects, service_accounts, conversations, prompts, vector_stores, API keys, dynamic providers, SSO configs, domain verifications, and SSO group mappings.
To clarify intent, add a comment near the limit-checking code (here and in other routes) documenting whether limits are:
- Best-effort (allow occasional over-limit when concurrent requests race), or
- Strict (require
SELECT … FOR UPDATEor serializable transactions to prevent races).
If best-effort is acceptable, document it. If strict enforcement is required, consider wrapping count + insert in a serializable transaction or advisory lock per owner.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/routes/admin/api_keys.rs
Line: 322-373
Comment:
All limit checks throughout this PR follow a `count() → compare() → create()` pattern without transactional isolation. Under concurrent load, two or more requests can both read the same count (e.g., `max - 1`), both pass the limit check, and both proceed to create, resulting in the final count exceeding the configured limit.
This TOCTOU (time-of-check / time-of-use) race affects every limit-checked route in this PR: teams, projects, service_accounts, conversations, prompts, vector_stores, API keys, dynamic providers, SSO configs, domain verifications, and SSO group mappings.
To clarify intent, add a comment near the limit-checking code (here and in other routes) documenting whether limits are:
- **Best-effort** (allow occasional over-limit when concurrent requests race), or
- **Strict** (require `SELECT … FOR UPDATE` or serializable transactions to prevent races).
If best-effort is acceptable, document it. If strict enforcement is required, consider wrapping count + insert in a serializable transaction or advisory lock per owner.
How can I resolve this? If you propose a fix, please make it concise.1 participant
No description provided.