mirror of
https://github.com/Such-Software/smirk-backend-core.git
synced 2026-10-17 15:31:38 +02:00
jwandClaude Opus 5
0a6ae18f80
Fixes every confirmed backend finding from the v0.3.0 pre-ship review. Identity (the two that corrupt accounts): - nostr_register creates a row with pubkey_hash NULL, so that wallet's later /auth/extension call matched nothing, fell through the rotation path (which no client ever satisfies, since nothing sends rotation_signature) and minted a SECOND account. A fingerprint match on a NULL-pubkey_hash row now adopts the submitted pubkey_hash onto that row, but only when the row's on-file BTC key equals the submitted one, so a bare fingerprint (which check_restore discloses, and is therefore not a secret) still cannot claim an account. - The fingerprint-ownership check no longer short-circuits on returning_by_pubkey. It was skipped for returning users, so a fingerprint owned by another row got backfilled, violated the UNIQUE index, and 500'd that account on every subsequent login. - The rotation proof is verified over a distinct, instance- and target-bound message. It shared the login message, so a captured login signature was a replayable rotation proof against another instance. Abuse control: - The rate limiters key on the real client IP, honouring X-Forwarded-For only from a trusted proxy. Behind the TLS terminator every request looked like 127.0.0.1, so the whole internet shared one bucket and ~3 req/s 429'd every wallet unlock globally. - A PoW solution is single-use instead of replayable for the challenge TTL. - Delegated /verify is opt-in and rate-limited; it was an unauthenticated amplifier into Electrum, LWS and a full-genesis grin scan. - A grin scan with no start_height gets the same depth policy and PoW price as one that passes a height, instead of an unbounded free scan. Retention and logging: - The retention sweep is implemented AND scheduled. RETENTION_* was advertised and inert, which made the privacy policy's retention promise untrue. Zero means keep forever, and each statement is batched so a backlog drains over ticks rather than holding row locks. - login_events.ip_hash is no longer written, and legacy values are scrubbed: nothing ever read it, so it was retention without a purpose. - Tracing spans log user_id only, not the raw npub and username. - secret-scan.sh matches the coverage its comment claims. Also fixes the test suite, which did not compile on this commit: NewSession gained an id inc64cd9band two call sites were never updated. mint_session now binds one session_id for both the token and the row, since a different id there would silently recreate the bugc64cd9bfixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
92 lines
2.9 KiB
Rust
92 lines
2.9 KiB
Rust
//! L1 integration: the per-IP rate limiter actually throttles a burst, and it
|
|
//! keys on the client IP the trusted-proxy policy resolves.
|
|
//!
|
|
//! The strict tier wraps the unauthenticated auth/website surface. The governor
|
|
//! runs before the handler, so an unauthenticated burst to a strict-tier route
|
|
//! still counts — past the burst size it must return 429. (All requests share the
|
|
//! injected loopback peer, so they hit one bucket.)
|
|
|
|
mod common;
|
|
|
|
use axum::http::StatusCode;
|
|
use ipnetwork::IpNetwork;
|
|
use std::net::IpAddr;
|
|
|
|
#[tokio::test]
|
|
async fn strict_tier_throttles_a_burst() {
|
|
let app = require_app!();
|
|
let mut saw_429 = false;
|
|
for _ in 0..40 {
|
|
let (status, _) = app.request("GET", "/api/v1/auth/me", None, None).await;
|
|
if status == StatusCode::TOO_MANY_REQUESTS {
|
|
saw_429 = true;
|
|
break;
|
|
}
|
|
}
|
|
assert!(
|
|
saw_429,
|
|
"a rapid burst to a strict-tier endpoint should be rate-limited (429)"
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn spoofed_forwarded_for_from_an_untrusted_peer_shares_one_bucket() {
|
|
let app = require_app!();
|
|
// Default config trusts no proxies, so the loopback peer the harness injects
|
|
// is untrusted and its X-Forwarded-For must be ignored. If the limiter keyed
|
|
// on the header, every request below would mint a fresh bucket and nothing
|
|
// would ever throttle.
|
|
let mut saw_429 = false;
|
|
for i in 0..40 {
|
|
let spoofed = format!("203.0.113.{i}");
|
|
let (status, _, _) = app
|
|
.request_full(
|
|
"GET",
|
|
"/api/v1/auth/me",
|
|
&[("x-forwarded-for", spoofed.as_str())],
|
|
None,
|
|
)
|
|
.await;
|
|
if status == StatusCode::TOO_MANY_REQUESTS {
|
|
saw_429 = true;
|
|
break;
|
|
}
|
|
}
|
|
assert!(
|
|
saw_429,
|
|
"a spoofed X-Forwarded-For from an untrusted peer must not buy its own bucket"
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn trusted_proxy_gets_a_bucket_per_forwarded_client() {
|
|
// With the peer inside `trusted_proxies`, X-Forwarded-For IS the client, so
|
|
// distinct clients behind the proxy must not share (and exhaust) one bucket:
|
|
// that is the whole point of keying on the resolved client IP.
|
|
let maybe_app = common::try_app_with(|cfg| {
|
|
cfg.trusted_proxies = vec![IpNetwork::from(IpAddr::from([127, 0, 0, 1]))];
|
|
})
|
|
.await;
|
|
let Some(app) = maybe_app else {
|
|
eprintln!("skipping: TEST_DATABASE_URL not set");
|
|
return;
|
|
};
|
|
|
|
for i in 0..40 {
|
|
let client = format!("198.51.100.{i}");
|
|
let (status, _, _) = app
|
|
.request_full(
|
|
"GET",
|
|
"/api/v1/auth/me",
|
|
&[("x-forwarded-for", client.as_str())],
|
|
None,
|
|
)
|
|
.await;
|
|
assert_ne!(
|
|
status,
|
|
StatusCode::TOO_MANY_REQUESTS,
|
|
"one request per forwarded client must not trip the limiter ({client})"
|
|
);
|
|
}
|
|
}
|