Unbounded list limit

[pimlock]

Summary

The list RPCs (ListSandboxes, ListProviders, ListInferenceRoutes) take limit and offset from the client. When limit is 0, the code uses a default (e.g. 100); otherwise it uses the client-provided value as-is. There is no upper bound. A client can send limit = 4294967295 (u32::MAX) and cause the server to load and return a very large number of records, stressing memory and the store and potentially causing DoS.

Source Code

• crates/navigator-server/src/grpc.rs: in list_sandboxes (lines 359-368), limit is either request.limit or 100, then passed to store.list(). Same pattern in the list providers handler (lines 473-481).
• crates/navigator-server/src/inference.rs: in list_inference_routes (lines 243-253), limit is set the same way.

---

Originally by @drew on 2026-02-19T08:59:54.892-08:00

[johntmyers]

🔒 security-review-agent

Security Review

Determination: Legitimate concern

Summary

The list RPCs (ListSandboxes, ListProviders, ListInferenceRoutes, ListPolicyRevisions) accept a client-provided limit parameter with no upper bound. When limit is non-zero, the raw value is passed directly to SQL LIMIT clauses, allowing a client to request up to 4,294,967,295 rows in a single call, potentially causing memory exhaustion and denial of service.

Severity Assessment

• Impact: Medium — an attacker can cause server memory pressure and potential OOM, degrading or crashing the service for all users.
• Exploitability: Trivial — requires only a single gRPC call with limit set to a large value. No authentication bypass is needed; any client with gRPC access can trigger this. There is no rate limiting or auth middleware on these endpoints.
• Affected components:
  • crates/navigator-server/src/grpc.rs — list_sandboxes (line 405), list_providers (line 519), list_policies (line 995), list_provider_records (line 1830)
  • crates/navigator-server/src/inference.rs — list_inference_routes (line 168)
  • crates/navigator-server/src/persistence/sqlite.rs — list() (line 153)
  • crates/navigator-server/src/persistence/postgres.rs — list() (line 130)

Attack Scenario

1. Attacker sends a gRPC ListSandboxes (or any list RPC) request with limit = 4294967295.
2. The server passes this value directly as the SQL LIMIT, causing the database to attempt to return all matching rows.
3. The server loads all results into memory (fetch_all), serializes them into a gRPC response, and attempts to send the massive payload.
4. This causes excessive memory allocation on the server, potentially leading to OOM kills and service disruption for all clients.

Remediation Plan

1. Define a server-side maximum limit constant (e.g., const MAX_PAGE_SIZE: u32 = 1000;) and clamp the client-provided limit using limit.min(MAX_PAGE_SIZE) in all list handlers.
2. Apply this cap consistently across all four affected list RPCs and the list_provider_records helper function.
3. Consider adding the maximum allowed page size to the API documentation / proto comments so clients are aware of the constraint.

Additional Notes

• The Vec::with_capacity(records.len()) calls are safe since they use the actual result count, not the raw limit — the primary concern is the unbounded SQL query and response size.
• While the actual number of records in the database may be small today, this is a defense-in-depth issue: as the system grows or if the store is shared, the risk increases.
• There is currently no rate limiting or authentication middleware on these gRPC endpoints, which makes exploitation even more straightforward.

[johntmyers]

🏗️ build-plan

Implementation Plan

Issue type: fix
Complexity: Low
Confidence: High

Summary

All list RPCs pass the client-provided limit directly to SQL queries with no upper bound, allowing a single request to load an unbounded number of rows and cause memory exhaustion. The fix introduces a shared MAX_PAGE_SIZE constant and clamps the limit in every list handler before it reaches the persistence layer.

Scope

• crates/navigator-server/src/grpc.rs: Clamp limit to MAX_PAGE_SIZE in list_sandboxes (line 405), list_providers (line 519), list_sandbox_policies (line 995), and list_provider_records (line 1830). Define the MAX_PAGE_SIZE constant and a helper function clamp_limit(raw: u32, default: u32) -> u32 at module level.
• crates/navigator-server/src/inference.rs: Clamp limit to MAX_PAGE_SIZE in list_inference_routes (line 168). Import the constant/helper from grpc or a shared location.

Implementation Steps

1. Add a pub(crate) const MAX_PAGE_SIZE: u32 = 1000; and a pub(crate) fn clamp_limit(raw: u32, default: u32, max: u32) -> u32 helper in crates/navigator-server/src/grpc.rs (or a shared module). The helper returns default when raw == 0, otherwise raw.min(max).
2. Replace the inline if request.limit == 0 { 100 } else { request.limit } pattern in list_sandboxes (line 405-408) with clamp_limit(request.limit, 100, MAX_PAGE_SIZE).
3. Replace the same pattern in list_providers (line 519-523) with clamp_limit(request.limit, 100, MAX_PAGE_SIZE).
4. Replace the same pattern in list_sandbox_policies (line 995) with clamp_limit(req.limit, 50, MAX_PAGE_SIZE) (preserving the existing default of 50).
5. Replace the same pattern in list_inference_routes in inference.rs (line 168-171) with clamp_limit(request.limit, 100, MAX_PAGE_SIZE), importing the constant and helper.
6. Add unit tests for clamp_limit: verify 0 returns default, a value within range passes through, and a value exceeding MAX_PAGE_SIZE is capped.
7. Run mise run pre-commit to verify everything compiles and passes.

Test Plan

• Unit tests: Add tests for clamp_limit in the existing mod tests block in crates/navigator-server/src/grpc.rs — cover zero input (returns default), normal input (pass-through), and over-limit input (clamped to max).
• Integration tests: The existing sqlite_list_paging test in crates/navigator-server/src/persistence/tests.rs already validates list paging at the store level; no new integration tests needed since the clamp is applied at the handler layer above the store.
• E2E tests: N/A

Risks & Open Questions

• The MAX_PAGE_SIZE of 1000 is a reasonable default but could be made configurable via server config in the future. For this fix, a compile-time constant is sufficient.
• No proto/API changes are needed — the server silently clamps the limit. Clients requesting more than 1000 will simply receive at most 1000 results per page. This is standard pagination behavior and not a breaking change.

Documentation Impact

• None expected. The proto comments for limit fields could optionally be updated to document the server-side maximum, but that is a follow-up concern.

---

Revision 1 — initial plan

[johntmyers]

🏗️ build-from-issue-agent

Implementation Complete

PR: #140

What was built

Added a MAX_PAGE_SIZE constant (1000) and clamp_limit helper function to cap client-provided limit values in all list RPCs (ListSandboxes, ListProviders, ListSandboxPolicies, ListInferenceRoutes), preventing unbounded SQL queries and potential memory exhaustion.

Tests

• Unit: 3 tests added (clamp_limit_zero_returns_default, clamp_limit_within_range_passes_through, clamp_limit_exceeding_max_is_capped)

Docs updated

• None needed

The issue will auto-close when the PR is merged.