Files
dtourolleandClaude Opus 5 830b4b452f
CI / fmt, clippy, test (pull_request) Successful in 5m29s
CI / advisories and licences (pull_request) Successful in 36s
CI / static musl binary (pull_request) Successful in 7m14s
fix: drop a redundant borrow clippy now rejects
error: redundant reference in `assert_eq!` argument
      --> tests/injection.rs:633:13
      = note: `-D clippy::useless-borrows-in-formatting` implied by `-D warnings`

No code changed to cause this. ci.yml installs whatever rustup calls
stable and the tree pins no toolchain, so the gate moved under the
repository: `useless_borrows_in_formatting` fires on a line that has been
there since the injection tests were written, and `RUSTFLAGS: -D
warnings` turns it into a hard failure.

The borrow was never doing anything -- `{:?}` formats a String and a
&String identically -- so this is the lint being right, not a workaround
for it.

Verified against the toolchain CI actually resolved to (1.97.1, not the
1.85 in Cargo.toml's rust-version), all four gates the workflow runs:
fmt clean, clippy --all-targets --all-features clean with no other lint,
208 tests passing, and cargo deny reporting advisories/bans/licenses/
sources ok.

Worth separating from this fix: the failure was invisible until it
happened because the toolchain floats. Nothing here pins it, so the next
stable release can turn this repository red without a commit. A
rust-toolchain.toml would close that, at the cost of not seeing new lints
until it is bumped -- a policy call, not a bug fix, so it is not in here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-03 20:53:49 +02:00

637 lines
24 KiB
Rust

//! Injection resistance — SQL, JSON and header.
//!
//! These are regression tests for properties the design already provides, kept
//! separate from `api.rs` because their purpose is different: `api.rs` asserts the
//! spec's behaviour, this asserts that hostile input cannot escape its layer.
//!
//! Two distinct defences are at work, and it is worth being precise about which
//! applies where, because they fail differently:
//!
//! 1. **Parameterised queries** (§7, §8). Every value reaches SQLite through
//! `params![]`; the only `format!`-built SQL interpolates compile-time
//! constants (a column list and a status literal). So a value carrying SQL
//! syntax is bound as *data* and simply matches nothing.
//! 2. **Closed-vocabulary validation** (§5a, §6 stage 2). Identifiers are
//! regex-constrained and free text is restricted to a closed character class,
//! so most injection strings are rejected before they reach the database.
//!
//! Defence 1 is what actually prevents injection; defence 2 means an attacker
//! usually cannot even reach it. Testing both matters: if validation were ever
//! loosened, these tests should still pass on the strength of parameterisation
//! alone.
use std::sync::Arc;
use axum::body::Body;
use axum::http::{Request, StatusCode};
use http_body_util::BodyExt;
use jray_server::app;
use jray_server::config::Config;
use jray_server::db::Db;
use jray_server::ratelimit::RateLimiter;
use jray_server::state::AppState;
use jray_server::tmdb::TmdbClient;
use serde_json::{json, Value};
use tower::ServiceExt;
/// Payloads spanning the usual SQL-injection shapes: boolean tautology, statement
/// termination, stacked statements, UNION exfiltration, comment truncation, and
/// string-concatenation exfiltration.
const SQL_PAYLOADS: &[&str] = &[
"1' OR '1'='1",
"1'; DROP TABLE manifests;--",
"1 UNION SELECT token_hash FROM contributors",
"' OR 1=1--",
"1'||(SELECT token_hash FROM contributors)||'",
"1)) OR 1=1 --",
"'; UPDATE manifests SET status='listed' WHERE 1=1;--",
"1/**/UNION/**/SELECT/**/1",
"x' AND (SELECT COUNT(*) FROM sqlite_master)>0 --",
"\"; DELETE FROM scenes; --",
];
struct TestServer {
router: axum::Router,
db: Db,
_dir: TempDir,
}
struct TempDir(std::path::PathBuf);
impl TempDir {
fn new(tag: &str) -> Self {
let mut p = std::env::temp_dir();
p.push(format!("jray-inj-{}-{}", tag, unique()));
std::fs::create_dir_all(&p).expect("creating temp dir");
Self(p)
}
fn db_path(&self) -> String {
self.0.join("test.db").to_string_lossy().into_owned()
}
}
impl Drop for TempDir {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.0);
}
}
fn unique() -> String {
use std::sync::atomic::{AtomicU64, Ordering};
static N: AtomicU64 = AtomicU64::new(0);
let t = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_nanos())
.unwrap_or(0);
format!("{t}-{}", N.fetch_add(1, Ordering::Relaxed))
}
impl TestServer {
fn new(tag: &str) -> Self {
let dir = TempDir::new(tag);
let db = Db::open(&dir.db_path()).expect("opening database");
let config = Arc::new(Config {
bind: "127.0.0.1:0".into(),
db_path: dir.db_path(),
tmdb_api_key: None,
tmdb_base_url: "http://127.0.0.1:1".into(),
trusted_proxies: Vec::new(),
server_id: "test.example".into(),
publish_peer_directory: true,
contact: Some("admin@test.example".into()),
request_timeout: std::time::Duration::from_secs(30),
job_batch: 8,
job_poll_interval: std::time::Duration::from_secs(3600),
});
let state = AppState {
db: db.clone(),
config: config.clone(),
limiter: Arc::new(RateLimiter::new()),
tmdb: Arc::new(TmdbClient::new(config.tmdb_base_url.clone(), None)),
};
Self { router: app::router(state), db, _dir: dir }
}
async fn send(&self, req: Request<Body>) -> (StatusCode, Value) {
let resp = self.router.clone().oneshot(req).await.expect("router call");
let status = resp.status();
let bytes = resp.into_body().collect().await.expect("body").to_bytes();
let body = if bytes.is_empty() {
Value::Null
} else {
serde_json::from_slice(&bytes)
.unwrap_or(Value::String(String::from_utf8_lossy(&bytes).into_owned()))
};
(status, body)
}
async fn get(&self, uri: &str) -> (StatusCode, Value) {
self.send(Request::builder().uri(uri).body(Body::empty()).unwrap()).await
}
async fn post(&self, uri: &str, token: Option<&str>, body: &Value) -> (StatusCode, Value) {
let mut b =
Request::builder().method("POST").uri(uri).header("content-type", "application/json");
if let Some(t) = token {
b = b.header("authorization", format!("Bearer {t}"));
}
self.send(b.body(Body::from(body.to_string())).unwrap()).await
}
async fn token(&self) -> String {
let (_, body) = self.post("/api/v1/tokens", None, &json!({})).await;
body["token"].as_str().expect("token").to_string()
}
/// Confirms the schema is intact and the expected row counts hold.
///
/// A successful injection would most likely drop a table or delete rows, so
/// this is the assertion that actually matters after each payload.
async fn assert_schema_intact(&self) {
let tables: Vec<String> = self
.db
.read(|conn| {
let mut stmt = conn
.prepare("SELECT name FROM sqlite_master WHERE type='table' ORDER BY name")?;
let rows = stmt
.query_map([], |r| r.get::<_, String>(0))?
.collect::<rusqlite::Result<Vec<_>>>()?;
Ok(rows)
})
.await
.expect("listing tables");
for expected in [
"contributors",
"jobs",
"manifest_actors",
"manifests",
"people",
"reports",
"scenes",
"titles",
"tmdb_cache",
] {
assert!(
tables.iter().any(|t| t == expected),
"table {expected} is missing — an injection may have dropped it. tables: {tables:?}"
);
}
}
}
fn urlencode(s: &str) -> String {
let mut out = String::new();
for b in s.bytes() {
match b {
b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
out.push(b as char)
}
_ => out.push_str(&format!("%{b:02X}")),
}
}
out
}
// ---------------------------------------------------------------------------
// SQL injection — query parameters
// ---------------------------------------------------------------------------
#[tokio::test]
async fn sql_payloads_in_query_parameters_are_inert() {
let s = TestServer::new("query");
for payload in SQL_PAYLOADS {
let enc = urlencode(payload);
for uri in [
format!("/api/v1/manifests/exists?tmdb_id={enc}"),
format!("/api/v1/manifests/exists?imdb_id={enc}"),
format!("/api/v1/manifests/movie?tmdb_id={enc}"),
format!("/api/v1/manifests/movie?imdb_id={enc}"),
format!("/api/v1/manifests/episode?series_tmdb_id={enc}&season=1&episode=1"),
format!("/api/v1/manifests/series/{enc}"),
format!("/api/v1/manifests/exists?tmdb_id=1&runtime_sec={enc}"),
] {
let (status, body) = s.get(&uri).await;
// The payload is bound as data, so it matches nothing. What must never
// happen is a 5xx, which would mean SQLite saw it as syntax.
assert!(
status.is_success()
|| status == StatusCode::NOT_FOUND
|| status == StatusCode::BAD_REQUEST,
"payload {payload:?} on {uri} produced {status} — expected data-not-found, \
not a server error. body: {body}"
);
}
}
s.assert_schema_intact().await;
}
#[tokio::test]
async fn sql_payloads_in_path_parameters_are_inert() {
let s = TestServer::new("path");
for payload in SQL_PAYLOADS {
let enc = urlencode(payload);
for uri in [
format!("/api/v1/manifests/{enc}"),
format!("/api/v1/manifests/{enc}/status"),
format!("/api/v1/manifests/series/{enc}"),
] {
let (status, body) = s.get(&uri).await;
assert!(
!status.is_server_error(),
"payload {payload:?} on {uri} produced {status}: {body}"
);
}
}
s.assert_schema_intact().await;
}
// ---------------------------------------------------------------------------
// SQL injection — JSON body fields
// ---------------------------------------------------------------------------
#[tokio::test]
async fn sql_payloads_in_identifier_fields_are_rejected() {
// §6 stage 2 regex-constrains every identifier, so these never even reach the
// query layer. The response must be a clean 400 naming the field.
let s = TestServer::new("body-ids");
let token = s.token().await;
for payload in SQL_PAYLOADS {
let manifest = json!({
"jmanifest_version": 2,
"identity": { "type": "movie", "tmdb_id": payload },
"cut": { "runtime_sec": 100.0 },
"actors": [ { "tmdb_id": "884", "scenes": [{"start":1.0,"end":2.0}] } ]
});
let (status, body) = s.post("/api/v1/manifests", Some(&token), &manifest).await;
assert_eq!(
status,
StatusCode::BAD_REQUEST,
"payload {payload:?} should be rejected by validation: {body}"
);
}
s.assert_schema_intact().await;
}
#[tokio::test]
async fn sql_payloads_in_free_text_fields_are_rejected() {
// The two free-text fields (§5a) are the only place arbitrary strings could
// arrive. The closed character class excludes quotes, semicolons and digits,
// which is what makes SQL syntax unrepresentable there.
let s = TestServer::new("body-text");
let token = s.token().await;
for payload in SQL_PAYLOADS {
for manifest in [
json!({
"jmanifest_version": 2,
"identity": { "type": "movie", "tmdb_id": "504172", "title": payload },
"cut": { "runtime_sec": 100.0 },
"actors": [ { "tmdb_id": "884", "scenes": [{"start":1.0,"end":2.0}] } ]
}),
json!({
"jmanifest_version": 2,
"identity": { "type": "movie", "tmdb_id": "504172" },
"cut": { "runtime_sec": 100.0 },
"actors": [ { "name": payload, "tmdb_id": "884", "scenes": [{"start":1.0,"end":2.0}] } ]
}),
] {
let (status, body) = s.post("/api/v1/manifests", Some(&token), &manifest).await;
assert_eq!(
status,
StatusCode::BAD_REQUEST,
"free-text payload {payload:?} should be rejected: {body}"
);
}
}
s.assert_schema_intact().await;
}
#[tokio::test]
async fn sql_payloads_in_a_report_note_cannot_escape() {
// `note` is the one field that accepts relatively free text (control
// characters stripped, length capped) because only the operator reads it. It
// reaches the database, so it is the strongest test of parameterisation:
// validation is *not* filtering SQL syntax here.
let s = TestServer::new("report-note");
let token = s.token().await;
let (_, body) = s
.post(
"/api/v1/manifests",
Some(&token),
&json!({
"jmanifest_version": 2,
"identity": { "type": "movie", "tmdb_id": "504172" },
"cut": { "runtime_sec": 100.0 },
"actors": [ { "tmdb_id": "884", "scenes": [{"start":1.0,"end":2.0}] } ]
}),
)
.await;
let id = body["manifest_id"].as_str().expect("manifest id").to_string();
for payload in SQL_PAYLOADS {
let (status, body) = s
.post(
&format!("/api/v1/manifests/{id}/report"),
None,
&json!({ "reason": "spam", "note": payload }),
)
.await;
assert!(
status.is_success() || status == StatusCode::TOO_MANY_REQUESTS,
"note payload {payload:?} produced {status}: {body}"
);
if status.is_success() {
s.assert_schema_intact().await;
}
}
// The notes were stored verbatim as *data* — proving they were bound, not
// executed. Verified by reading them back out.
let stored: i64 =
s.db.read(|conn| Ok(conn.query_row("SELECT COUNT(*) FROM reports", [], |r| r.get(0))?))
.await
.expect("counting reports");
assert!(stored > 0, "reports should have been stored as inert data");
}
#[tokio::test]
async fn sql_payloads_in_a_bearer_token_are_inert() {
// The token is hashed before it reaches any query, but a payload arriving via
// a header must still not produce a 5xx.
let s = TestServer::new("token-inj");
for payload in SQL_PAYLOADS {
let req = Request::builder()
.method("POST")
.uri("/api/v1/manifests")
.header("content-type", "application/json")
.header("authorization", format!("Bearer {payload}"))
.body(Body::from(
json!({
"jmanifest_version": 2,
"identity": { "type": "movie", "tmdb_id": "504172" },
"cut": { "runtime_sec": 100.0 },
"actors": [ { "tmdb_id": "884", "scenes": [{"start":1.0,"end":2.0}] } ]
})
.to_string(),
))
.unwrap();
let (status, body) = s.send(req).await;
assert_eq!(
status,
StatusCode::UNAUTHORIZED,
"token payload {payload:?} produced {status}: {body}"
);
}
s.assert_schema_intact().await;
}
#[tokio::test]
async fn sql_payloads_in_the_batch_exists_body_are_inert() {
let s = TestServer::new("batch-inj");
let items: Vec<Value> = SQL_PAYLOADS.iter().map(|p| json!({ "tmdb_id": p })).collect();
let (status, body) = s.post("/api/v1/manifests/exists", None, &json!({ "items": items })).await;
assert_eq!(status, StatusCode::OK, "{body}");
// Each malformed item degrades to "absent" rather than erroring the batch.
for result in body["results"].as_array().expect("results") {
assert_eq!(result["exists"], false);
}
s.assert_schema_intact().await;
}
// ---------------------------------------------------------------------------
// JSON injection / parser abuse
// ---------------------------------------------------------------------------
#[tokio::test]
async fn json_structure_abuse_is_rejected_cleanly() {
// A parser handed hostile structure must fail with 400/413, never 5xx and
// never a hang (§6 stage 1: "a parser handed an unbounded body is a
// denial-of-service primitive").
let s = TestServer::new("json-abuse");
let token = s.token().await;
let cases: Vec<(&str, String)> = vec![
("deep nesting", format!("{}{}", "[".repeat(20_000), "]".repeat(20_000))),
("unterminated", "{\"identity\": {\"type\": \"movie\"".to_string()),
("duplicate keys", r#"{"jmanifest_version":2,"jmanifest_version":2}"#.to_string()),
("null bytes", "{\"jmanifest_version\":\u{0}1}".to_string()),
("huge number", format!("{{\"jmanifest_version\":{}}}", "9".repeat(5000))),
("nan literal", r#"{"jmanifest_version":2,"cut":{"runtime_sec":NaN}}"#.to_string()),
("bare array", "[1,2,3]".to_string()),
("bare string", "\"just a string\"".to_string()),
("empty body", String::new()),
(
"prototype-style key",
r#"{"__proto__":{"admin":true},"jmanifest_version":2}"#.to_string(),
),
];
for (label, body) in cases {
let req = Request::builder()
.method("POST")
.uri("/api/v1/manifests")
.header("content-type", "application/json")
.header("authorization", format!("Bearer {token}"))
.body(Body::from(body))
.unwrap();
let (status, resp) = s.send(req).await;
assert!(
status == StatusCode::BAD_REQUEST || status == StatusCode::PAYLOAD_TOO_LARGE,
"{label} produced {status}, expected a clean rejection: {resp}"
);
}
s.assert_schema_intact().await;
}
#[tokio::test]
async fn non_finite_scene_times_are_rejected() {
// §6 explicitly rejects NaN/Infinity. They cannot arrive as JSON literals, but
// they can arrive as overflowing decimals, which parse to f64 infinity.
let s = TestServer::new("nonfinite");
let token = s.token().await;
// Sent as raw JSON text rather than via `json!`, because rustc refuses an
// out-of-range float literal — and the point is to make the *server's* parser
// handle it, which is the real attack path.
let raw = r#"{"jmanifest_version":2,
"identity":{"type":"movie","tmdb_id":"504172"},
"cut":{"runtime_sec":100.0},
"actors":[{"tmdb_id":"884","scenes":[[1.0,1e400]]}]}"#;
let req = Request::builder()
.method("POST")
.uri("/api/v1/manifests")
.header("content-type", "application/json")
.header("authorization", format!("Bearer {token}"))
.body(Body::from(raw))
.unwrap();
let (status, body) = s.send(req).await;
assert_eq!(status, StatusCode::BAD_REQUEST, "{body}");
// Likewise an overflowing runtime.
let raw = r#"{"jmanifest_version":2,
"identity":{"type":"movie","tmdb_id":"504172"},
"cut":{"runtime_sec":1e400},
"actors":[{"tmdb_id":"884","scenes":[{"start":1.0,"end":2.0}]}]}"#;
let req = Request::builder()
.method("POST")
.uri("/api/v1/manifests")
.header("content-type", "application/json")
.header("authorization", format!("Bearer {token}"))
.body(Body::from(raw))
.unwrap();
let (status, body) = s.send(req).await;
assert_eq!(status, StatusCode::BAD_REQUEST, "{body}");
}
// ---------------------------------------------------------------------------
// Header injection
// ---------------------------------------------------------------------------
#[tokio::test]
async fn crlf_in_a_header_value_cannot_split_the_response() {
// A CRLF-carrying header value must not appear in the response as new headers.
// `http` rejects such values at construction, so this asserts the invariant
// holds at the boundary rather than relying on our own escaping.
let bad = "1.2.3.4\r\nX-Injected: yes";
assert!(
axum::http::HeaderValue::from_str(bad).is_err(),
"the http crate must refuse CRLF in header values"
);
// And a percent-encoded variant reaching a handler stays inert data.
let s = TestServer::new("crlf");
let (status, _) = s.get("/api/v1/manifests/exists?tmdb_id=1%0D%0AX-Injected:%20yes").await;
assert!(!status.is_server_error());
s.assert_schema_intact().await;
}
#[tokio::test]
async fn oversized_headers_do_not_take_the_server_down() {
let s = TestServer::new("big-header");
let big = "a".repeat(100_000);
let req = Request::builder()
.uri("/api/v1/manifests/exists?tmdb_id=1")
.header("x-filler", big)
.body(Body::empty())
.unwrap();
let (status, _) = s.send(req).await;
assert!(!status.is_server_error(), "got {status}");
}
// ---------------------------------------------------------------------------
// Path traversal
// ---------------------------------------------------------------------------
#[tokio::test]
async fn path_traversal_attempts_reach_no_filesystem() {
// The server serves no files at all, so traversal has nowhere to go. Asserted
// anyway, because the manifest id is a path segment.
let s = TestServer::new("traversal");
for probe in [
"..%2F..%2F..%2Fetc%2Fpasswd",
"....%2F%2F....%2F%2Fetc%2Fpasswd",
"%2e%2e%2f%2e%2e%2fetc%2fshadow",
"..%5C..%5Cwindows%5Csystem32",
"%00/etc/passwd",
] {
let (status, body) = s.get(&format!("/api/v1/manifests/{probe}")).await;
assert!(
status == StatusCode::NOT_FOUND || status == StatusCode::BAD_REQUEST,
"probe {probe} produced {status}: {body}"
);
// Nothing that looks like file content should ever come back.
let text = body.to_string();
assert!(!text.contains("root:"), "probe {probe} returned passwd-like content");
}
}
// ---------------------------------------------------------------------------
// Unicode and encoding tricks against the §5a character class
// ---------------------------------------------------------------------------
#[tokio::test]
async fn unicode_tricks_cannot_smuggle_text_past_the_character_class() {
// §5a's class is checked *after* NFC normalisation, so decomposed and
// compatibility forms must not provide a way in. Fullwidth digits are the
// sharpest case: NFKC would fold them to ASCII digits, but NFC does not, and
// they are `Nd` (not a letter), so the class rejects them either way.
let s = TestServer::new("unicode");
let token = s.token().await;
for payload in [
"Actor 123", // fullwidth letters and digits
"Steve\u{FEFF}Buscemi", // zero-width no-break space
"Ste\u{0301}ve\u{202E}", // combining acute plus bidi override
"𝐒𝐭𝐞𝐯𝐞", // mathematical bold (compatibility form)
"Steve\u{2028}Buscemi", // line separator
"\u{1F600} Actor", // emoji
"Actor\u{00A0}Name\u{0000}", // nbsp plus NUL
] {
let (status, body) = s
.post(
"/api/v1/manifests",
Some(&token),
&json!({
"jmanifest_version": 2,
"identity": { "type": "movie", "tmdb_id": "504172" },
"cut": { "runtime_sec": 100.0 },
"actors": [ { "name": payload, "tmdb_id": "884", "scenes": [{"start":1.0,"end":2.0}] } ]
}),
)
.await;
assert_eq!(
status,
StatusCode::BAD_REQUEST,
"unicode payload {payload:?} should be rejected: {body}"
);
}
}
#[tokio::test]
async fn the_audio_signature_field_cannot_carry_arbitrary_bytes() {
// §3: a variable-length blob would be a payload channel — "precisely what §5a
// closes". Length is fixed and every byte is structurally constrained.
let s = TestServer::new("audio-sig");
let token = s.token().await;
for sig in [
"v1:aGVsbG8gd29ybGQ=", // too short to be a signature
&format!("v1:{}", "/".repeat(4000)), // high bit set throughout
&format!("v1:{}", "A".repeat(100_000)), // oversized
"not-base64-at-all", // missing version prefix
&"A".repeat(1720), // unprefixed
] {
let (status, body) = s
.post(
"/api/v1/manifests",
Some(&token),
&json!({
"jmanifest_version": 2,
"identity": { "type": "movie", "tmdb_id": "504172" },
"cut": { "runtime_sec": 6420.5, "audio_signature": sig },
"actors": [ { "tmdb_id": "884", "scenes": [{"start":1.0,"end":2.0}] } ]
}),
)
.await;
assert_eq!(
status,
StatusCode::BAD_REQUEST,
"signature {:?} should be rejected: {body}",
sig.chars().take(40).collect::<String>()
);
}
}