Skip to content

Add new resource limits#8

Merged
ScriptSmith merged 4 commits intomainfrom
limits
Mar 12, 2026
Merged

Add new resource limits#8
ScriptSmith merged 4 commits intomainfrom
limits

Conversation

@ScriptSmith
Copy link
Owner

No description provided.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 5, 2026

Greptile Summary

This PR significantly expands the resource limits system by adding 19 new configurable caps (teams, projects, service accounts, API keys, vector stores, files, conversations, prompts, SSO group mappings, domain verifications, and member counts). The best-effort enforcement model (documented in ResourceLimits) is appropriate, and the majority of the limit checks are well-implemented. One logic bug was found in the service-account branch of API key creation.

Key changes:

  • src/config/limits.rs: 19 new ResourceLimits fields, each with a #[serde(default)] backing function and a corresponding entry in Default::default().
  • All routes follow a consistent count → compare → create pattern, returning 409 Conflict when the limit is reached.
  • New count_by_team and count_by_owner query methods added to Postgres/SQLite implementations and their trait definitions.
  • api_v1_vector_stores_create_file_batch gains a new MAX_BATCH_SIZE = 500 guard in addition to the per-store file count check.

Issue found:

  • In src/routes/admin/api_keys.rs, the ServiceAccount owner branch calls count_by_org(sa.org_id), which is implemented as WHERE owner_type = 'organization' AND owner_id = ?. Service-account-owned API keys (owner_type = 'service_account') are never included in this count, so service accounts can create up to max_api_keys_per_org keys each, effectively bypassing the intended org-wide ceiling.

Confidence Score: 3/5

  • Safe to merge with the caveat that the service account API key org-limit is not correctly enforced.
  • The vast majority of the 28 changed files are correct and consistent. A single logic bug in the ServiceAccount branch of api_keys::create means service accounts can bypass the org-level API key limit, since count_by_org only counts org-scoped keys. This is a functional correctness issue but not a security vulnerability — API key creation still requires admin authentication.
  • src/routes/admin/api_keys.rs — ServiceAccount owner branch uses count_by_org which under-counts.

Important Files Changed

Filename Overview
src/config/limits.rs Adds 19 new resource limit fields with sensible defaults and a clear best-effort enforcement model comment. Clean, well-structured addition.
src/routes/admin/api_keys.rs Adds per-scope API key limit checks. The ServiceAccount branch contains a logic bug: count_by_org only counts org-scoped keys (owner_type='organization'), so service-account-owned keys are never counted against the org limit.
src/routes/api/vector_stores.rs Adds vector store and file-per-store limit checks. Batch vs single-file predicate inconsistency (already flagged in a prior thread). New MAX_BATCH_SIZE=500 guard is a good defensive addition.
src/routes/api/files.rs Adds per-owner file upload limit check. Files are hard-deleted so omitting deleted_at IS NULL is correct. Error wrapping follows the file's existing ApiError pattern.
src/routes/admin/projects.rs Adds per-org and per-team project count checks. Logic is straightforward and correct.
src/routes/admin/dynamic_providers.rs Adds per-scope provider limit checks for org/team/project/user. No issues found.
src/routes/admin/teams.rs Adds team count limit for org and member count limit for team. Both checks are straightforward.
src/routes/admin/users.rs Adds org member and project member limit checks. Clean and consistent with other limit implementations.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Route as Admin Route (api_keys::create)
    participant SASvc as ServiceAccountService
    participant KeySvc as ApiKeyService
    participant DB as Database

    Client->>Route: POST /admin/api_keys (owner: ServiceAccount)
    Route->>SASvc: get_by_id(service_account_id)
    SASvc->>DB: SELECT * FROM service_accounts WHERE id = ?
    DB-->>SASvc: sa { org_id }
    SASvc-->>Route: sa

    Route->>KeySvc: count_by_org(sa.org_id, false)
    KeySvc->>DB: SELECT COUNT(*) FROM api_keys\nWHERE owner_type='organization' AND owner_id=?
    Note over DB: ⚠️ Does NOT count\nowner_type='service_account' keys
    DB-->>KeySvc: count (org-scoped keys only)
    KeySvc-->>Route: count

    alt count >= max_api_keys_per_org
        Route-->>Client: 409 Conflict
    else count < max (SA keys uncounted → limit bypassable)
        Route->>KeySvc: create(owner: ServiceAccount)
        KeySvc->>DB: INSERT INTO api_keys (owner_type='service_account', ...)
        DB-->>KeySvc: new key
        KeySvc-->>Route: api_key
        Route-->>Client: 201 Created
    end
Loading

Comments Outside Diff (1)

  1. src/routes/admin/api_keys.rs, line 531-552 (link)

    Service account API key limit not enforced against SA-owned keys

    count_by_org is implemented as:

    SELECT COUNT(*) FROM api_keys WHERE owner_type = 'organization' AND owner_id = $1

    It counts only org-scoped keys (owner_type = 'organization'). Keys stored with owner_type = 'service_account' are never counted. As a result, the limit check here is comparing the number of org-scope keys against max_api_keys_per_org, completely ignoring how many service-account-owned keys already exist in the org.

    Concretely:

    • An org with 0 org-scoped keys and 499 service-account keys passes this check and can create another SA key — for every service account independently, up to max_api_keys_per_org each.
    • The "Organization has reached the maximum number of API keys" error fires only when the org-scoped count hits the ceiling, not when total org key volume does.

    If the intent is to apply an org-wide ceiling across all owner types, you need a query that aggregates across all scopes within the org (org + teams + projects + users + service accounts). If the intent is a per-service-account cap, a dedicated max_api_keys_per_service_account limit would be more precise.

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/routes/admin/api_keys.rs
Line: 531-552

Comment:
**Service account API key limit not enforced against SA-owned keys**

`count_by_org` is implemented as:

```sql
SELECT COUNT(*) FROM api_keys WHERE owner_type = 'organization' AND owner_id = $1
```

It counts only org-scoped keys (`owner_type = 'organization'`). Keys stored with `owner_type = 'service_account'` are never counted. As a result, the limit check here is comparing the number of *org-scope* keys against `max_api_keys_per_org`, completely ignoring how many service-account-owned keys already exist in the org.

Concretely:
- An org with 0 org-scoped keys and 499 service-account keys passes this check and can create another SA key — for every service account independently, up to `max_api_keys_per_org` each.
- The "Organization has reached the maximum number of API keys" error fires only when the org-scoped count hits the ceiling, not when total org key volume does.

If the intent is to apply an org-wide ceiling across all owner types, you need a query that aggregates across all scopes within the org (org + teams + projects + users + service accounts). If the intent is a per-service-account cap, a dedicated `max_api_keys_per_service_account` limit would be more precise.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: f0b6fdb

@ScriptSmith
Copy link
Owner Author

@greptile-apps

@ScriptSmith
Copy link
Owner Author

@greptile-apps

@ScriptSmith
Copy link
Owner Author

@greptile-apps

@ScriptSmith ScriptSmith merged commit c2b9f51 into main Mar 12, 2026
20 checks passed
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