No request/spec size limits

[pimlock]

Summary

CreateSandbox and the inference route create/update handlers accept the full request body without enforcing any maximum size or length on spec fields, provider lists, policy blob, or route strings. A client could send very large payloads and stress the store, memory, or serialization path, contributing to DoS or resource exhaustion.

Source Code

• crates/navigator-server/src/grpc.rs: create_sandbox (lines 62-151) takes CreateSandboxRequest, extracts the spec, validates that spec and policy exist and that provider names exist in the store, but does not check length or size.
• crates/navigator-server/src/inference.rs: the create/update route handlers validate route spec fields for non-empty trimmed strings in validate_route_spec but do not impose maximum lengths on routing_hint, base_url, api_key, model_id, or on the overall message size.

---

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

[johntmyers]

🔒 security-review-agent

Security Review

Determination: Legitimate concern

Summary

The gRPC endpoints for CreateSandbox, CreateInferenceRoute, and UpdateInferenceRoute accept unbounded string fields and nested message payloads without enforcing application-level size or length limits. While tonic imposes a default 4 MB max decoding message size and the server requires mTLS authentication (reducing the attack surface to authenticated clients only), there are no per-field length checks on spec fields such as providers, policy, routing_hint, base_url, api_key, model_id, environment maps, or template structures like pod_template / resources / volume_claim_templates.

Severity Assessment

• Impact: Low-to-medium. An authenticated attacker could send maximally-sized payloads (up to 4 MB per request) to exhaust server memory, database storage, or serialization overhead over time. The SandboxSpec includes arbitrarily nested google.protobuf.Struct fields (resources, pod_template, volume_claim_templates) and unbounded map<string,string> fields that could amplify storage and processing cost.
• Exploitability: Low. The server enforces mTLS (tls.rs:24-25), meaning only clients with valid certificates signed by the cluster CA can connect. This limits the attack surface to trusted, authenticated callers. Additionally, tonic's built-in 4 MB message size limit (max_decoding_message_size defaults in the generated proto code at navigator.v1.rs:685) provides a baseline cap, though this is not explicitly configured and relies on the library default.
• Affected components:
  • crates/navigator-server/src/grpc.rs:72-170 — create_sandbox accepts spec with unbounded providers list, policy blob, environment maps, and template structs
  • crates/navigator-server/src/grpc.rs:487-500 — create_provider has no field length validation
  • crates/navigator-server/src/inference.rs:60-101 — create_inference_route validates non-empty fields but no max lengths
  • crates/navigator-server/src/inference.rs:103-142 — update_inference_route same issue
  • crates/navigator-server/src/multiplex.rs:47-48 — NavigatorServer::new() and InferenceServer::new() are created without explicit max_decoding_message_size() configuration

Attack Scenario

1. An authenticated client (with valid mTLS cert) sends repeated CreateSandbox requests with maximally-padded spec fields — e.g., a 4 MB policy blob, thousands of provider names, or deeply nested pod_template Struct values
2. Each request is persisted to the SQLite store via put_message without size checks, accumulating storage
3. Over sustained requests, this could exhaust database storage, increase memory pressure during serialization/deserialization, and degrade server responsiveness for legitimate clients
4. Similarly, CreateInferenceRoute could be used to store many routes with near-max-size api_key or base_url strings

Remediation Plan

1. Explicitly configure tonic message size limits — call .max_decoding_message_size(N) on NavigatorServer and InferenceServer in multiplex.rs:47-48 with a conservative limit (e.g., 1 MB) rather than relying on the implicit 4 MB default
2. Add application-level field validation in create_sandbox and the inference route handlers:
   • Cap spec.providers list length (e.g., max 32 entries)
   • Cap individual string field lengths (name, routing_hint, base_url, api_key, model_id) to reasonable bounds (e.g., 1024 bytes)
   • Cap environment map size (entries and key/value lengths)
   • Validate or cap the serialized size of SandboxTemplate fields (resources, pod_template, volume_claim_templates)
   • Cap policy blob serialized size
3. Add rate limiting at the connection or request level for mutating RPCs as a defense-in-depth measure

Additional Notes

• The mTLS requirement is a strong mitigating factor — this is not exploitable by unauthenticated external attackers. The risk is limited to compromised or malicious authenticated clients within the cluster trust boundary.
• Tonic's default 4 MB limit already provides a baseline defense, but it is not documented or intentional in this codebase, so it should be made explicit.
• The push_sandbox_logs streaming endpoint already has a per-batch cap of 100 lines (grpc.rs:1153), showing the pattern is established elsewhere in the codebase.

[johntmyers]

🏗️ build-plan

Implementation Plan

Issue type: fix
Complexity: Low
Confidence: High

Summary

Add application-level field validation (max lengths, list sizes, map sizes) to create_sandbox and create_provider handlers, and explicitly configure tonic's max_decoding_message_size on both gRPC servers in the multiplex layer. This hardens the server against resource-exhaustion attacks from authenticated clients sending oversized payloads. Inference route validation is excluded — issue #133 is removing the entire inference route CRUD system.

Scope

• crates/navigator-server/src/multiplex.rs: Set explicit max_decoding_message_size(1MB) on both NavigatorServer and InferenceServer instead of relying on the implicit 4 MB tonic default.
• crates/navigator-server/src/grpc.rs: Add a validate_sandbox_spec function that enforces max lengths on name, log_level, environment map (entry count + key/value lengths), providers list (max 32), and caps on serialized size of SandboxTemplate Struct fields. Call it in create_sandbox before persisting. Add field length checks to create_provider_record for name, type, credentials map, and config map.

Implementation Steps

1. Configure explicit tonic message size limits — In multiplex.rs, chain .max_decoding_message_size(1_048_576) (1 MB) on both NavigatorServer::new(...) and InferenceServer::new(...).
2. Add validate_sandbox_spec in grpc.rs — Create a validation function that checks:
   • request.name length ≤ 253 chars (Kubernetes name limit)
   • spec.providers length ≤ 32
   • spec.log_level length ≤ 32
   • spec.environment map: ≤ 128 entries, keys ≤ 256 bytes, values ≤ 8192 bytes
   • spec.template string fields (image, runtime_class_name, agent_socket) ≤ 1024 bytes
   • spec.template map fields (labels, annotations, environment) ≤ 128 entries each
   • spec.template Struct fields (resources, pod_template, volume_claim_templates) serialized size ≤ 65536 bytes each
   • Policy serialized size ≤ 262144 bytes (256 KB)
3. Wire validate_sandbox_spec into create_sandbox — Call the new validation function right after extracting the spec, before the provider-existence loop.
4. Add field length checks to create_provider_record — Validate name ≤ 253, type ≤ 64, credentials map ≤ 32 entries with keys ≤ 256 and values ≤ 8192, config map ≤ 64 entries with same key/value limits.
5. Add unit tests for all new validation — Test boundary conditions (at limit, over limit) for each validated field in grpc.rs::tests.
6. Verify existing integration tests still pass — Run cargo test -p navigator-server to confirm no regressions.

Test Plan

• Unit tests: Add tests in crates/navigator-server/src/grpc.rs (in the existing mod tests block) for validate_sandbox_spec and the new provider field limits.
• Integration tests: Existing tests in crates/navigator-server/tests/ should continue to pass unchanged (they use small payloads).
• E2E tests: N/A — unit tests provide sufficient coverage for input validation.

Risks & Open Questions

• Limit values are best-guess — The specific limits are reasonable defaults but may need tuning based on real usage patterns. They should be documented as constants for easy adjustment.
• Struct size validation — Checking serialized size of google.protobuf.Struct fields requires encoding them, which adds a small CPU cost per request. Acceptable since sandbox creation is not a hot path.
• No rate limiting in scope — Rate limiting is recommended as defense-in-depth but excluded to keep scope focused. Can be a follow-up issue.
• Backward compatibility — The 1 MB message limit is 4x smaller than the current implicit 4 MB default. Existing clients sending legitimately large payloads (unlikely) could be rejected.
• Inference routes excluded — Issue Simplify inference routing: introduce inference.local and remove implicit catch-all #133 is rearchitecting the entire inference system (removing route CRUD, introducing inference.local). Adding field validation to APIs that are about to be deleted is wasted effort.

Documentation Impact

• None expected — internal server hardening. Limits documented as code-level constants.

---

Revision 2 — removed inference route validation (excluded per #133 rearchitecture)
Revision 1 — initial plan

[johntmyers]

🏗️ build-from-issue-agent

Implementation Complete

PR: #145

What was built

Added application-level field validation to create_sandbox and create_provider gRPC handlers with 14 named constants, and configured explicit 1 MB tonic max_decoding_message_size on both gRPC servers. Inference routes excluded per #133 rearchitecture.

Tests

• Unit: 21 tests added covering boundary conditions for all validated fields

Docs updated

• None needed — internal server hardening

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