-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Found during deep code review of server/github.rs and config.rs
1. Webhook signature bypass (CRITICAL, github.rs:253-269)
```rust
if let Some(ref secret) = config.github.webhook_secret {
if !secret.is_empty() { // Only validates when non-empty
// ... verify signature
}
}
```
If `webhook_secret` is `Some("")` (e.g., from `github_webhook_secret: ""` in YAML config), signature verification is entirely skipped. Any HTTP client can trigger arbitrary PR reviews.
The env var path normalizes empty strings to `None`, but the YAML path does not.
2. Config Debug/Serialize exposes all secrets (config.rs:213)
```rust
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Config {
pub api_key: Option,
```
`Config`, `GitHubConfig`, and `VaultConfig` all derive `Debug` and `Serialize` without `#[serde(skip_serializing)]` on secret fields. `{:?}` formatting prints API keys, GitHub tokens, webhook secrets, and Vault tokens to logs. `save_config_async` writes them all to disk in plaintext YAML.
3. No authentication on REST API
`start_review`, `get_status`, `list_events`, and all other API endpoints have no authentication. The server is reachable from the internet when exposed via the GitHub webhook integration.
4. No rate limiting
No rate limiting middleware on any endpoint. The review semaphore limits concurrency but the queue is unbounded — an attacker can accumulate unlimited pending requests in memory.
Acceptance
- Empty webhook secret treated same as missing (no bypass)
- Secret fields use `#[serde(skip_serializing)]` and custom `Debug` impl
- API endpoints require authentication token
- Rate limiting on webhook and review endpoints
🤖 Generated with Claude Code