Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ These are the privacy and compatibility invariants of the project. Reintroducing

**Exception (off by default):** when `TRUSTED_WHITELIST_ENABLED=true` AND the embedded whitelist is non-empty, `/api/register` MAY return a new `403 Forbidden` with one of two distinct bodies — `{"success":false,"message":"Mostro instance pubkey required"}` (missing field) or `{"success":false,"message":"Mostro instance not trusted"}` (untrusted value). The flag defaults to `false` precisely so the byte-identical fixture set continues to hold for clients that pre-date the feature; only flip it after the mobile rollout.

**Exception (always on):** `/api/register` and `/api/unregister` are wrapped by `register_ip_rate_limit_mw` and MAY return `429 Too Many Requests` with the shared `rate_limited_response` body — `{"success":false,"message":"rate limited"}` plus a `Retry-After` header in whole seconds, `.max(1)` — or `500 Internal Server Error` with `{"success":false,"message":"internal error"}` when the per-IP key cannot be extracted (fail-closed, same rule as `/api/notify`). Both bodies keep the `success, message` field order. The `200` and `400` bodies are unchanged, so the pre-1.1 fixture set still holds for every request that is not rate-limited.

4. **Token store is in-memory only.** No persistence to disk for `trade_pubkey -> device_token`. UnifiedPush endpoints are the only on-disk state (atomic JSON write to `data/unifiedpush_endpoints.json`).

5. **Logs never carry raw pubkeys.** Every log site that touches a `trade_pubkey` goes through `crate::utils::log_pubkey::log_pubkey(salt, pubkey)`. The salt is a 32-byte random value generated once per process and never persisted.
Expand Down
15 changes: 11 additions & 4 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,16 +227,23 @@ curl -i -X POST http://localhost:8080/api/notify \
| 200 | `/api/health`, `/api/info`, `/api/status`, `/api/register`, `/api/unregister` |
| 202 | `/api/notify` on parse-valid input |
| 400 | Malformed body, invalid `trade_pubkey`, invalid `platform`, empty `token` |
| 429 | `/api/notify` only — per-IP or per-pubkey rate limit |
| 500 | `/api/notify` only — fail-closed when the per-IP key cannot be extracted |
| 429 | `/api/register`, `/api/unregister`, `/api/notify` rate limits |
| 500 | Rate-limited endpoints fail closed when the per-IP key cannot be extracted |

## Rate limiting

Only `/api/notify` is rate-limited. Limits are per-IP and per-`trade_pubkey`; both must allow the request to pass. Defaults:
`/api/register` and `/api/unregister` share a per-IP limit to protect the
in-memory token store from registration churn:

- Per-IP: `120/min`, burst `100`

`/api/notify` has separate per-IP and per-`trade_pubkey` limits; both must allow the request to pass. Defaults:

- Per-pubkey: `30/min`, burst `10` (env `NOTIFY_RATE_PER_PUBKEY_PER_MIN`)
- Per-IP: `120/min`, burst `30` (env `NOTIFY_RATE_PER_IP_PER_MIN`)

The other endpoints are intentionally not rate-limited at the HTTP layer; capacity at the edge is governed by `fly.toml` `hard_limit = 25`.
`/api/health`, `/api/info`, and `/api/status` are intentionally not
rate-limited at the HTTP layer; capacity at the edge is governed by
`fly.toml` `hard_limit = 25`.

See [configuration.md](./configuration.md) for the full rate-limit knob list and the `NOTIFY_TRUST_PROXY_HEADERS` flag governing trust of `Fly-Client-IP` / `X-Forwarded-For`.
2 changes: 1 addition & 1 deletion docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ src/

### HTTP server (`actix-web`)

Five always-on endpoints (`/api/health`, `/api/info`, `/api/status`, `/api/register`, `/api/unregister`) plus the rate-limited `/api/notify` resource. The `/api/notify` resource is the only endpoint wrapped by middleware: `request_id_mw` (outermost) and `per_ip_rate_limit_mw`.
Five always-on endpoints (`/api/health`, `/api/info`, `/api/status`, `/api/register`, `/api/unregister`) plus `/api/notify`. `/api/register` and `/api/unregister` share a per-IP middleware to bound token-store churn. `/api/notify` has its own middleware stack: `request_id_mw` (outermost) and `per_ip_rate_limit_mw`.

### Nostr listener (`nostr-sdk`)

Expand Down
11 changes: 9 additions & 2 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,11 @@ To turn the filter on/off without rebuilding, flip
| `TOKEN_TTL_HOURS` | `48` | Tokens older than this are evicted by the cleanup task |
| `CLEANUP_INTERVAL_HOURS` | `1` | How often the cleanup task runs |

## `/api/notify` rate limiter
## HTTP rate limiters

The dual-keyed rate limiter is documented in detail in [architecture.md](./architecture.md). Defaults are tuned for the Fly.io single-machine deployment.
The `/api/notify` dual-keyed rate limiter is documented in detail in
[architecture.md](./architecture.md). Defaults are tuned for the Fly.io
single-machine deployment.

| Variable | Default | Description |
|-----------------------------------------------|----------|------------------------------------------------------------------------------------------------------|
Expand All @@ -99,6 +101,11 @@ The dual-keyed rate limiter is documented in detail in [architecture.md](./archi

Setting either of `NOTIFY_RATE_PER_PUBKEY_PER_MIN` or `NOTIFY_RATE_PER_IP_PER_MIN` to `0` causes startup to fail with a chained error message; both must be greater than zero.

`/api/register` and `/api/unregister` also share a fixed per-IP limiter:
`120/min`, burst `100`. It is intentionally separate from `/api/notify` so
registration churn cannot consume the notify per-IP bucket, and notify traffic
cannot disable client re-registration.

## Legacy / reserved

| Variable | Default | Description |
Expand Down
66 changes: 66 additions & 0 deletions src/api/rate_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ pub type PerPubkeyLimiter = DefaultKeyedRateLimiter<String>;
/// Per-IP keyed rate limiter type alias (D-20).
pub type PerIpLimiter = DefaultKeyedRateLimiter<IpAddr>;

/// Separate per-IP limiter for /api/register and /api/unregister.
///
/// Kept as a newtype so it can live next to the /api/notify per-IP limiter
/// in Actix app_data without type collision.
#[derive(Clone)]
pub struct RegisterIpLimiter(pub Arc<PerIpLimiter>);

/// Newtype injected into `web::Data` to gate trust of `Fly-Client-IP` and
/// `X-Forwarded-For`. When `false`, both proxy headers are ignored and the
/// per-IP limiter keys exclusively on `req.peer_addr()`. Default is `false`
Expand All @@ -38,6 +45,11 @@ pub const PUBKEY_BURST: u32 = 10;
/// Per-IP burst (D-02). Not env-overridable in this phase per D-29.
pub const IP_BURST: u32 = 30;

/// Register/unregister per-IP limiter. Higher than /api/notify because normal
/// clients can re-register on app/session churn, but still caps store spam.
pub const REGISTER_IP_RATE_PER_MIN: u32 = 120;
pub const REGISTER_IP_BURST: u32 = 100;

/// Cleanup interval default in seconds (D-16). Override via NOTIFY_RATE_LIMIT_CLEANUP_INTERVAL_SECS.
/// Defaults are duplicated in `Config::from_env` so this constant is currently
/// unused at runtime; kept as the canonical reference for D-16.
Expand Down Expand Up @@ -165,6 +177,60 @@ pub async fn per_ip_rate_limit_mw(
}
}

/// Per-IP middleware for /api/register and /api/unregister.
pub async fn register_ip_rate_limit_mw(
req: ServiceRequest,
next: Next<impl MessageBody + 'static>,
) -> Result<ServiceResponse<BoxBody>, Error> {
let limiter = req
.app_data::<web::Data<RegisterIpLimiter>>()
.map(|d| d.0.clone());

let limiter = match limiter {
Some(l) => l,
None => {
warn!("register per-ip rate-limit middleware: limiter not in app_data (wiring bug)");
let resp = HttpResponse::InternalServerError().json(json!({
"success": false,
"message": "internal error"
}));
return Ok(req.into_response(resp).map_into_boxed_body());
}
};

let trust_proxy_headers = req
.app_data::<web::Data<TrustProxyHeaders>>()
.map(|d| d.0)
.unwrap_or(false);

let ip = match extract_client_ip(&req, trust_proxy_headers) {
Some(ip) => ip,
None => {
let resp = HttpResponse::InternalServerError().json(json!({
"success": false,
"message": "internal error"
}));
return Ok(req.into_response(resp).map_into_boxed_body());
}
};

match limiter.check_key(&ip) {
Ok(()) => {
let res = next.call(req).await?;
Ok(res.map_into_boxed_body())
}
Err(not_until) => {
let retry_after_secs = not_until
.wait_time_from(DefaultClock::default().now())
.as_secs()
.max(1);
let resp = rate_limited_response(retry_after_secs);
log::debug!("register per-ip 429 retry_after={}s", retry_after_secs);
Ok(req.into_response(resp).map_into_boxed_body())
}
}
}

/// Soft-cap branch of the cleanup task, extracted as a sync helper so the
/// LIMIT-06 warn-emission path is unit-testable without spawning a real
/// `tokio::time::interval` loop.
Expand Down
Loading
Loading