[Feature] Add parameter validation for API endpoints
- Dominant language
- Python
- Stars
- 485
- Forks
- 81
- Avg merge
- 16h 12m
- Merged PRs (30d)
- 8
Description
## Add parameter validation for API endpoints
### Problem
Several API endpoints in the admin service accept string parameters (e.g., `sandbox_id`, `image`) without validating that they are non-empty or non-whitespace. When clients send
empty or whitespace-only strings, the request propagates deeper into the system before eventually failing with unclear or misleading errors — making debugging difficult and
polluting server-side error metrics.
For example:
- `GET /is_alive?sandbox_id=` proceeds to query sandbox status with an empty ID
- `POST /start` with `{"image": " "}` attempts to start a sandbox with a whitespace-only image name
- `POST /stop` with an empty `sandbox_id` body reaches the sandbox manager before failing
### Expected Behavior
API endpoints should validate required string parameters at the boundary and immediately return a clear `BadRequestRockError` when they are `None`, empty, or whitespace-only —
before any business logic executes.
### Scope
The following endpoints/parameters need validation:
**`sandbox_api.py`**
- `POST /start` — `image`
- `POST /start_async` — `image`
- `GET /is_alive` — `sandbox_id`
- `GET /get_sandbox_statistics` — `sandbox_id`
- `GET /get_status` — `sandbox_id`
- `POST /upload` — `sandbox_id`
- `POST /stop` — `sandbox_id`
- `POST /commit` — `sandbox_id`, `image_tag`
**`sandbox_proxy_api.py`**
- `GET /is_alive` — `sandbox_id`
- `POST /upload` — `sandbox_id`
**`gem/api.py`**
- `POST /step` — `sandbox_id`
- `POST /reset` — `sandbox_id`
- `POST /close` — `sandbox_id`
**`SandboxProxyService`**
- `get_service_status()` — `sandbox_id`
- `_update_expire_time()` — `sandbox_id`
### Additional Improvements
- **Metrics**: Distinguish client errors (`BadRequestRockError`) from server errors in the metrics decorator. Client validation failures should increment `request.client_error`
instead of `request.failure` to avoid alerting on user input mistakes.
### Acceptance Criteria
- [ ] A reusable `validate_required_str(value, param_name)` utility exists in `rock/common/validation.py`
- [ ] All listed endpoints call validation before executing business logic
- [ ] `BadRequestRockError` is raised with a message like `"{param_name} is required and must be a non-empty string"`
- [ ] Metrics decorator records `request.client_error` for `BadRequestRockError`, not `request.failure`
- [ ] Unit tests cover the validation utility, endpoint-level validation, and metrics classification
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with sandbox_api.py, sandbox_proxy_api.py, gem/api.py, SandboxProxyService, and the metrics decorator. Review existing error handling and tests, then trace each listed parameter to confirm validation occurs before business logic. Done means rock/common/validation.py has the reusable utility, all listed endpoints and service methods validate inputs, metrics classify BadRequestRockError as client errors, and unit tests cover each requirement.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 64/100