max / makenotwork
13 files changed,
+480 insertions,
-26 deletions
| @@ -0,0 +1,16 @@ | |||
| 1 | + | { | |
| 2 | + | "db_name": "PostgreSQL", | |
| 3 | + | "query": "\n INSERT INTO oauth_granted_scopes (user_id, app_id, scopes)\n VALUES ($1, $2, $3)\n ON CONFLICT (user_id, app_id)\n DO UPDATE SET scopes = EXCLUDED.scopes, updated_at = NOW()\n ", | |
| 4 | + | "describe": { | |
| 5 | + | "columns": [], | |
| 6 | + | "parameters": { | |
| 7 | + | "Left": [ | |
| 8 | + | "Uuid", | |
| 9 | + | "Uuid", | |
| 10 | + | "Text" | |
| 11 | + | ] | |
| 12 | + | }, | |
| 13 | + | "nullable": [] | |
| 14 | + | }, | |
| 15 | + | "hash": "39dcb9ee5b63cc511d1bf2edd2ae2e42586acc4746f7bfcd64d87dcd49d6d455" | |
| 16 | + | } |
| @@ -0,0 +1,23 @@ | |||
| 1 | + | { | |
| 2 | + | "db_name": "PostgreSQL", | |
| 3 | + | "query": "SELECT scopes FROM oauth_granted_scopes WHERE user_id = $1 AND app_id = $2", | |
| 4 | + | "describe": { | |
| 5 | + | "columns": [ | |
| 6 | + | { | |
| 7 | + | "ordinal": 0, | |
| 8 | + | "name": "scopes", | |
| 9 | + | "type_info": "Text" | |
| 10 | + | } | |
| 11 | + | ], | |
| 12 | + | "parameters": { | |
| 13 | + | "Left": [ | |
| 14 | + | "Uuid", | |
| 15 | + | "Uuid" | |
| 16 | + | ] | |
| 17 | + | }, | |
| 18 | + | "nullable": [ | |
| 19 | + | false | |
| 20 | + | ] | |
| 21 | + | }, | |
| 22 | + | "hash": "cf99b70b83553f5c4709d9e79c10524aa3f320282e9ff2ec9010e04180173beb" | |
| 23 | + | } |
| @@ -0,0 +1,14 @@ | |||
| 1 | + | -- Per-(user, app) record of the OAuth scopes a user has interactively consented | |
| 2 | + | -- to for an app. The prompt=none silent re-auth path may only mint a code for | |
| 3 | + | -- scopes already present here; anything broader returns error=consent_required | |
| 4 | + | -- and forces a fresh interactive approval (ultra-fuzz Run 6 R6-Sec-L5). Today's | |
| 5 | + | -- scopes are read-only, so this is latent hardening that becomes load-bearing | |
| 6 | + | -- the moment a write scope ships. | |
| 7 | + | CREATE TABLE oauth_granted_scopes ( | |
| 8 | + | user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, | |
| 9 | + | app_id UUID NOT NULL REFERENCES sync_apps(id) ON DELETE CASCADE, | |
| 10 | + | scopes TEXT NOT NULL, | |
| 11 | + | granted_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | |
| 12 | + | updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | |
| 13 | + | PRIMARY KEY (user_id, app_id) | |
| 14 | + | ); |
| @@ -253,3 +253,53 @@ pub async fn is_registered_redirect_uri( | |||
| 253 | 253 | ||
| 254 | 254 | Ok(row.map(|r| r.0).unwrap_or(false)) | |
| 255 | 255 | } | |
| 256 | + | ||
| 257 | + | /// Scopes the user has already interactively consented to for this app. Empty | |
| 258 | + | /// when the pair has no row (so a first prompt=none with any non-empty scope is | |
| 259 | + | /// not a subset and must fall back to interactive consent). Run 6 R6-Sec-L5. | |
| 260 | + | #[tracing::instrument(skip_all)] | |
| 261 | + | pub async fn get_granted_scopes( | |
| 262 | + | pool: &PgPool, | |
| 263 | + | user_id: UserId, | |
| 264 | + | app_id: SyncAppId, | |
| 265 | + | ) -> Result<crate::oauth_scope::GrantedScopes> { | |
| 266 | + | let row = sqlx::query_scalar!( | |
| 267 | + | "SELECT scopes FROM oauth_granted_scopes WHERE user_id = $1 AND app_id = $2", | |
| 268 | + | user_id as UserId, | |
| 269 | + | app_id as SyncAppId, | |
| 270 | + | ) | |
| 271 | + | .fetch_optional(pool) | |
| 272 | + | .await?; | |
| 273 | + | Ok(row | |
| 274 | + | .map(|s| crate::oauth_scope::GrantedScopes::parse(&s)) | |
| 275 | + | .unwrap_or_default()) | |
| 276 | + | } | |
| 277 | + | ||
| 278 | + | /// Record an interactive consent: union the freshly-approved scope set into the | |
| 279 | + | /// user's standing grant for this app, so a later prompt=none re-auth can | |
| 280 | + | /// silently reuse what was approved (Run 6 R6-Sec-L5). | |
| 281 | + | #[tracing::instrument(skip_all)] | |
| 282 | + | pub async fn record_granted_scopes( | |
| 283 | + | pool: &PgPool, | |
| 284 | + | user_id: UserId, | |
| 285 | + | app_id: SyncAppId, | |
| 286 | + | scope: &crate::oauth_scope::GrantedScopes, | |
| 287 | + | ) -> Result<()> { | |
| 288 | + | let mut merged = get_granted_scopes(pool, user_id, app_id).await?; | |
| 289 | + | merged.union_with(scope); | |
| 290 | + | let scopes = merged.to_string(); | |
| 291 | + | sqlx::query!( | |
| 292 | + | r#" | |
| 293 | + | INSERT INTO oauth_granted_scopes (user_id, app_id, scopes) | |
| 294 | + | VALUES ($1, $2, $3) | |
| 295 | + | ON CONFLICT (user_id, app_id) | |
| 296 | + | DO UPDATE SET scopes = EXCLUDED.scopes, updated_at = NOW() | |
| 297 | + | "#, | |
| 298 | + | user_id as UserId, | |
| 299 | + | app_id as SyncAppId, | |
| 300 | + | scopes, | |
| 301 | + | ) | |
| 302 | + | .execute(pool) | |
| 303 | + | .await?; | |
| 304 | + | Ok(()) | |
| 305 | + | } |
| @@ -337,18 +337,21 @@ pub fn blame_file( | |||
| 337 | 337 | commit_oid: Oid, | |
| 338 | 338 | path: &str, | |
| 339 | 339 | ) -> Result<Vec<BlameLine>, GitError> { | |
| 340 | + | // Size/binary gate BEFORE the blame walk. `repo.blame_file` walks history per | |
| 341 | + | // line, so blaming a large file is expensive; `read_file` returns early for | |
| 342 | + | // binary and over-cap blobs without loading them, so checking it first avoids | |
| 343 | + | // the walk entirely on a file we won't render (ultra-fuzz Run 6 R6-Sec-L3). | |
| 344 | + | let file = super::read_file(repo, commit_oid, path)?; | |
| 345 | + | if file.is_binary || file.size > crate::constants::GIT_MAX_FILE_SIZE_BYTES as u64 { | |
| 346 | + | return Err(GitError::PathNotFound); | |
| 347 | + | } | |
| 348 | + | ||
| 340 | 349 | let mut opts = git2::BlameOptions::new(); | |
| 341 | 350 | opts.newest_commit(commit_oid); | |
| 342 | 351 | let blame = repo | |
| 343 | 352 | .blame_file(Path::new(path), Some(&mut opts)) | |
| 344 | 353 | .map_err(|_| GitError::PathNotFound)?; | |
| 345 | 354 | ||
| 346 | - | // Read the file content at this commit | |
| 347 | - | let file = super::read_file(repo, commit_oid, path)?; | |
| 348 | - | if file.is_binary { | |
| 349 | - | return Err(GitError::PathNotFound); | |
| 350 | - | } | |
| 351 | - | ||
| 352 | 355 | let content_lines: Vec<&str> = file.content.lines().collect(); | |
| 353 | 356 | let mut result = Vec::with_capacity(content_lines.len()); | |
| 354 | 357 |
| @@ -465,6 +465,36 @@ mod tests { | |||
| 465 | 465 | } | |
| 466 | 466 | ||
| 467 | 467 | #[test] | |
| 468 | + | fn blame_gates_oversize_file_before_walk() { | |
| 469 | + | // R6-Sec-L3: an over-cap file is rejected by the size gate before the | |
| 470 | + | // expensive blame walk; a small file still blames normally. | |
| 471 | + | let tmp = tempfile::TempDir::new().unwrap(); | |
| 472 | + | let bare_path = tmp.path().join("owner").join("blame.git"); | |
| 473 | + | std::fs::create_dir_all(&bare_path).unwrap(); | |
| 474 | + | let repo = Repository::init_bare(&bare_path).unwrap(); | |
| 475 | + | let sig = git2::Signature::now("Test", "test@example.com").unwrap(); | |
| 476 | + | ||
| 477 | + | let big = vec![b'a'; crate::constants::GIT_MAX_FILE_SIZE_BYTES + 1024]; | |
| 478 | + | let big_oid = repo.blob(&big).unwrap(); | |
| 479 | + | let small_oid = repo.blob(b"line one\nline two\n").unwrap(); | |
| 480 | + | let mut tb = repo.treebuilder(None).unwrap(); | |
| 481 | + | tb.insert("big.txt", big_oid, 0o100644).unwrap(); | |
| 482 | + | tb.insert("small.txt", small_oid, 0o100644).unwrap(); | |
| 483 | + | let tree_oid = tb.write().unwrap(); | |
| 484 | + | let tree = repo.find_tree(tree_oid).unwrap(); | |
| 485 | + | let commit_oid = repo | |
| 486 | + | .commit(Some("refs/heads/main"), &sig, &sig, "c", &tree, &[]) | |
| 487 | + | .unwrap(); | |
| 488 | + | ||
| 489 | + | assert!(matches!( | |
| 490 | + | blame_file(&repo, commit_oid, "big.txt"), | |
| 491 | + | Err(GitError::PathNotFound) | |
| 492 | + | )); | |
| 493 | + | let lines = blame_file(&repo, commit_oid, "small.txt").unwrap(); | |
| 494 | + | assert_eq!(lines.len(), 2); | |
| 495 | + | } | |
| 496 | + | ||
| 497 | + | #[test] | |
| 468 | 498 | fn resolve_ref_and_list_tree() { | |
| 469 | 499 | let (tmp, _) = make_test_repo(); | |
| 470 | 500 | let repo = open_repo(tmp.path(), "owner", "testrepo").unwrap(); |
| @@ -252,6 +252,15 @@ pub fn record_sanitizer_rejection(kind: &'static str) { | |||
| 252 | 252 | counter!("custom_pages_sanitizer_rejections_total", "kind" => kind).increment(1); | |
| 253 | 253 | } | |
| 254 | 254 | ||
| 255 | + | /// Increment the ClamAV fail-open counter: a trusted upload accepted Clean while | |
| 256 | + | /// clamd was transiently down and the health probe had not yet observed it (the | |
| 257 | + | /// one reduced-AV-coverage acceptance path). Scraped at `/metrics` and paired | |
| 258 | + | /// with a WAM ticket so the window is observable rather than log-only | |
| 259 | + | /// (ultra-fuzz Run 6 R6-Sec-L1, 3rd appearance). | |
| 260 | + | pub fn record_clamav_fail_open() { | |
| 261 | + | counter!("clamav_fail_open_total").increment(1); | |
| 262 | + | } | |
| 263 | + | ||
| 255 | 264 | /// Axum middleware that implements idempotency keys for POST endpoints. | |
| 256 | 265 | /// | |
| 257 | 266 | /// If the request includes an `Idempotency-Key` header and the user is |
| @@ -80,10 +80,17 @@ impl GrantedScopes { | |||
| 80 | 80 | ||
| 81 | 81 | /// True if every scope in `self` is also in `other`. The downgrade-only | |
| 82 | 82 | /// invariant for refresh: a refresh request may narrow but never widen the | |
| 83 | - | /// scope it was originally granted. | |
| 83 | + | /// scope it was originally granted. Also the prompt=none consent gate: a | |
| 84 | + | /// silent re-auth's requested scope must be a subset of what was consented. | |
| 84 | 85 | pub fn subset_of(&self, other: &GrantedScopes) -> bool { | |
| 85 | 86 | self.0.is_subset(&other.0) | |
| 86 | 87 | } | |
| 88 | + | ||
| 89 | + | /// Merge every scope from `other` into this set (set union). Used to fold a | |
| 90 | + | /// fresh interactive-consent grant into the user's standing grant for an app. | |
| 91 | + | pub fn union_with(&mut self, other: &GrantedScopes) { | |
| 92 | + | self.0.extend(other.0.iter().copied()); | |
| 93 | + | } | |
| 87 | 94 | } | |
| 88 | 95 | ||
| 89 | 96 | impl fmt::Display for GrantedScopes { | |
| @@ -148,4 +155,14 @@ mod tests { | |||
| 148 | 155 | assert!(GrantedScopes::parse("").is_empty()); | |
| 149 | 156 | assert!(GrantedScopes::parse(" ").is_empty()); | |
| 150 | 157 | } | |
| 158 | + | ||
| 159 | + | #[test] | |
| 160 | + | fn union_with_merges_and_dedups() { | |
| 161 | + | let mut a = GrantedScopes::parse("profile:read"); | |
| 162 | + | a.union_with(&GrantedScopes::parse("perks:read profile:read")); | |
| 163 | + | assert_eq!(a.to_string(), "profile:read perks:read"); | |
| 164 | + | // The consent-gate use: a narrower silent request is permitted by the union. | |
| 165 | + | assert!(GrantedScopes::parse("perks:read").subset_of(&a)); | |
| 166 | + | assert!(!GrantedScopes::parse("offline_access").subset_of(&a)); | |
| 167 | + | } | |
| 151 | 168 | } |
| @@ -245,16 +245,24 @@ pub(crate) async fn resolve_git_http_principal( | |||
| 245 | 245 | return Some(GitHttpPrincipal { user_id, token_push: None }); | |
| 246 | 246 | } | |
| 247 | 247 | let header = headers.get(axum::http::header::AUTHORIZATION)?.to_str().ok()?; | |
| 248 | - | let b64 = header.strip_prefix("Basic ")?; | |
| 248 | + | let token = parse_basic_auth_token(header)?; | |
| 249 | + | let hash = crate::crypto::git_token_hash(&token); | |
| 250 | + | let resolved = db::git_access_tokens::resolve_active(&state.db, &hash).await.ok()??; | |
| 251 | + | Some(GitHttpPrincipal { user_id: resolved.user_id, token_push: Some(resolved.can_push) }) | |
| 252 | + | } | |
| 253 | + | ||
| 254 | + | /// Extract the credential token from a `Basic` Authorization header value. | |
| 255 | + | /// Git sends `username:token` with the token in the password field, so the | |
| 256 | + | /// username half is ignored. Returns `None` for a non-`Basic` scheme, invalid | |
| 257 | + | /// base64, or non-UTF-8 bytes. A value with no colon is treated as the whole | |
| 258 | + | /// string being the token; an empty password yields `Some("")`, which hashes to | |
| 259 | + | /// a value that never matches an active token (so it resolves to unauthenticated). | |
| 260 | + | fn parse_basic_auth_token(header: &str) -> Option<String> { | |
| 249 | 261 | use base64::Engine; | |
| 262 | + | let b64 = header.strip_prefix("Basic ")?; | |
| 250 | 263 | let decoded = base64::engine::general_purpose::STANDARD.decode(b64).ok()?; | |
| 251 | 264 | let creds = String::from_utf8(decoded).ok()?; | |
| 252 | - | // "username:token" — git puts the token in the password field; the username | |
| 253 | - | // is irrelevant (the token alone identifies the user). | |
| 254 | - | let token = creds.split_once(':').map_or(creds.as_str(), |(_, t)| t); | |
| 255 | - | let hash = crate::crypto::git_token_hash(token); | |
| 256 | - | let resolved = db::git_access_tokens::resolve_active(&state.db, &hash).await.ok()??; | |
| 257 | - | Some(GitHttpPrincipal { user_id: resolved.user_id, token_push: Some(resolved.can_push) }) | |
| 265 | + | Some(creds.split_once(':').map_or(creds.clone(), |(_, t)| t.to_string())) | |
| 258 | 266 | } | |
| 259 | 267 | ||
| 260 | 268 | // ============================================================================ | |
| @@ -326,3 +334,45 @@ async fn fetch_linked_releases( | |||
| 326 | 334 | let project = Project::from_db(&db_project, db_items.len() as u32); | |
| 327 | 335 | (Some(project), release_items) | |
| 328 | 336 | } | |
| 337 | + | ||
| 338 | + | #[cfg(test)] | |
| 339 | + | mod tests { | |
| 340 | + | use super::parse_basic_auth_token; | |
| 341 | + | use base64::Engine; | |
| 342 | + | ||
| 343 | + | fn basic(creds: &str) -> String { | |
| 344 | + | format!("Basic {}", base64::engine::general_purpose::STANDARD.encode(creds)) | |
| 345 | + | } | |
| 346 | + | ||
| 347 | + | #[test] | |
| 348 | + | fn parses_token_from_password_field() { | |
| 349 | + | assert_eq!(parse_basic_auth_token(&basic("git:tok_abc")).as_deref(), Some("tok_abc")); | |
| 350 | + | } | |
| 351 | + | ||
| 352 | + | #[test] | |
| 353 | + | fn no_colon_treats_whole_value_as_token() { | |
| 354 | + | assert_eq!(parse_basic_auth_token(&basic("tok_only")).as_deref(), Some("tok_only")); | |
| 355 | + | } | |
| 356 | + | ||
| 357 | + | #[test] | |
| 358 | + | fn empty_password_yields_empty_token() { | |
| 359 | + | // Hashes to a value that never matches an active token -> unauthenticated. | |
| 360 | + | assert_eq!(parse_basic_auth_token(&basic("git:")).as_deref(), Some("")); | |
| 361 | + | } | |
| 362 | + | ||
| 363 | + | #[test] | |
| 364 | + | fn non_basic_scheme_rejected() { | |
| 365 | + | assert_eq!(parse_basic_auth_token("Bearer abc"), None); | |
| 366 | + | } | |
| 367 | + | ||
| 368 | + | #[test] | |
| 369 | + | fn invalid_base64_rejected() { | |
| 370 | + | assert_eq!(parse_basic_auth_token("Basic !!!not-base64!!!"), None); | |
| 371 | + | } | |
| 372 | + | ||
| 373 | + | #[test] | |
| 374 | + | fn non_utf8_bytes_rejected() { | |
| 375 | + | let b64 = base64::engine::general_purpose::STANDARD.encode([0xff, 0xfe, 0xfd]); | |
| 376 | + | assert_eq!(parse_basic_auth_token(&format!("Basic {b64}")), None); | |
| 377 | + | } | |
| 378 | + | } |
| @@ -330,6 +330,21 @@ async fn authorize_get( | |||
| 330 | 330 | let validated = session_user.as_ref().filter(|_| validated_session); | |
| 331 | 331 | return match validated { | |
| 332 | 332 | Some(user) => { | |
| 333 | + | // Silent re-auth may only mint a code for scopes the user has | |
| 334 | + | // ALREADY consented to for this app; anything broader requires | |
| 335 | + | // interactive approval (ultra-fuzz Run 6 R6-Sec-L5). Empty scope | |
| 336 | + | // (legacy sync client) is a subset of any grant, so it still | |
| 337 | + | // passes. The check reads consent purely from the DB, so a | |
| 338 | + | // client-supplied scope can never silently widen a grant. | |
| 339 | + | let granted = | |
| 340 | + | db::oauth::get_granted_scopes(&state.db, user.id, app.id).await?; | |
| 341 | + | if !scope.subset_of(&granted) { | |
| 342 | + | return Ok(redirect_with_error( | |
| 343 | + | redirect_uri, | |
| 344 | + | state_param, | |
| 345 | + | "consent_required", | |
| 346 | + | )); | |
| 347 | + | } | |
| 333 | 348 | issue_authorization_code( | |
| 334 | 349 | &state.db, | |
| 335 | 350 | app.id, | |
| @@ -540,6 +555,10 @@ async fn authorize_post( | |||
| 540 | 555 | // Empty scope = legacy sync client; non-empty opts into the userinfo flow. | |
| 541 | 556 | let scope = GrantedScopes::parse(&form.scope); | |
| 542 | 557 | ||
| 558 | + | // Record this interactive consent so a later prompt=none re-auth can silently | |
| 559 | + | // reuse the approved scopes (R6-Sec-L5). Union with any prior grant. | |
| 560 | + | db::oauth::record_granted_scopes(&state.db, user_id, app.id, &scope).await?; | |
| 561 | + | ||
| 543 | 562 | issue_authorization_code( | |
| 544 | 563 | &state.db, | |
| 545 | 564 | app.id, |
| @@ -66,6 +66,22 @@ fn detect_kind(magic: &[u8]) -> Option<ArchiveKind> { | |||
| 66 | 66 | } | |
| 67 | 67 | } | |
| 68 | 68 | ||
| 69 | + | /// Recognize containers we have no pure-Rust decompression-bomb checker for | |
| 70 | + | /// (7z, RAR). Returns the container label so the caller can reject them where | |
| 71 | + | /// they aren't a legitimate download payload, rather than passing them through | |
| 72 | + | /// on the ClamAV (FailOpen) layer alone (ultra-fuzz Run 6 R6-Sec-L2). | |
| 73 | + | fn detect_unsupported_container(magic: &[u8]) -> Option<&'static str> { | |
| 74 | + | if magic.starts_with(&[0x37, 0x7A, 0xBC, 0xAF, 0x27, 0x1C]) { | |
| 75 | + | Some("7z") | |
| 76 | + | } else if magic.starts_with(&[0x52, 0x61, 0x72, 0x21, 0x1A, 0x07]) { | |
| 77 | + | // RAR4 ("Rar!\x1a\x07\x00") and RAR5 ("Rar!\x1a\x07\x01\x00") share this | |
| 78 | + | // 6-byte prefix. | |
| 79 | + | Some("RAR") | |
| 80 | + | } else { | |
| 81 | + | None | |
| 82 | + | } | |
| 83 | + | } | |
| 84 | + | ||
| 69 | 85 | /// ZIP end-of-central-directory signature (`PK\x05\x06`). A valid ZIP always | |
| 70 | 86 | /// ends with this record (plus an optional trailing comment), even when bytes | |
| 71 | 87 | /// are prepended ahead of the first local header (self-extracting archives). | |
| @@ -176,6 +192,19 @@ pub fn inspect_archive( | |||
| 176 | 192 | file_type: FileType, | |
| 177 | 193 | yara_rules: Option<&yara_x::Rules>, | |
| 178 | 194 | ) -> (LayerResult, LayerResult) { | |
| 195 | + | // 7z / RAR have no pure-Rust bomb checker, so without this they would fall | |
| 196 | + | // through to detect_kind == None and rely on ClamAV (FailOpen) alone. Reject | |
| 197 | + | // them (FailClosed → held for review) anywhere they aren't a legitimate | |
| 198 | + | // download payload; Download keeps the ClamAV backstop (R6-Sec-L2). | |
| 199 | + | if let Some(container) = detect_unsupported_container(data) | |
| 200 | + | && file_type != FileType::Download | |
| 201 | + | { | |
| 202 | + | let detail = format!( | |
| 203 | + | "{container} archives are not accepted for this upload type (cannot be decompression-bomb inspected)" | |
| 204 | + | ); | |
| 205 | + | return (error(detail.clone()), nested(LayerVerdict::Skip, detail)); | |
| 206 | + | } | |
| 207 | + | ||
| 179 | 208 | // A cover-disguised archive: layer 1 (content_type) handles the type | |
| 180 | 209 | // mismatch, so the bomb walk is skipped — but the interior is still scanned | |
| 181 | 210 | // (the nested layer never gated on file type). `bomb` off, `content` on. | |
| @@ -910,6 +939,30 @@ mod tests { | |||
| 910 | 939 | assert_eq!(result.verdict, LayerVerdict::Skip); | |
| 911 | 940 | } | |
| 912 | 941 | ||
| 942 | + | // -- 7z / RAR: no pure-Rust bomb checker (R6-Sec-L2) -- | |
| 943 | + | ||
| 944 | + | #[test] | |
| 945 | + | fn sevenzip_rejected_for_non_download() { | |
| 946 | + | let data = [0x37, 0x7A, 0xBC, 0xAF, 0x27, 0x1C, 0, 0, 0, 0]; | |
| 947 | + | let result = check_archive_safety(&data, FileType::Cover); | |
| 948 | + | assert_eq!(result.verdict, LayerVerdict::Error); | |
| 949 | + | } | |
| 950 | + | ||
| 951 | + | #[test] | |
| 952 | + | fn sevenzip_allowed_for_download() { | |
| 953 | + | // Download keeps the ClamAV backstop; the in-process layer doesn't reject. | |
| 954 | + | let data = [0x37, 0x7A, 0xBC, 0xAF, 0x27, 0x1C, 0, 0, 0, 0]; | |
| 955 | + | let result = check_archive_safety(&data, FileType::Download); | |
| 956 | + | assert_ne!(result.verdict, LayerVerdict::Error); | |
| 957 | + | } | |
| 958 | + | ||
| 959 | + | #[test] | |
| 960 | + | fn rar_rejected_for_non_download() { | |
| 961 | + | let data = [0x52, 0x61, 0x72, 0x21, 0x1A, 0x07, 0x00, 0, 0, 0]; | |
| 962 | + | let result = check_archive_safety(&data, FileType::MediaImage); | |
| 963 | + | assert_eq!(result.verdict, LayerVerdict::Error); | |
| 964 | + | } | |
| 965 | + | ||
| 913 | 966 | #[test] | |
| 914 | 967 | fn cover_zip_skipped() { | |
| 915 | 968 | // A ZIP file claimed as cover should be skipped (layer 1 handles type mismatch) |
| @@ -444,24 +444,55 @@ async fn run_pipeline_and_decide( | |||
| 444 | 444 | // path where a file is accepted on reduced AV coverage without a hold — so | |
| 445 | 445 | // an operator must be able to see it rather than have it pass silently | |
| 446 | 446 | // (ultra-fuzz Run 4 M-Sec4). | |
| 447 | - | if status == FileScanStatus::Clean && is_trusted && !clamav_down { | |
| 448 | - | let clamav_errored = result | |
| 449 | - | .layers | |
| 450 | - | .iter() | |
| 451 | - | .any(|l| l.layer == "clamav" && l.verdict == LayerVerdict::Error); | |
| 452 | - | if clamav_errored { | |
| 453 | - | tracing::warn!( | |
| 454 | - | s3_key = %job.s3_key, | |
| 455 | - | user_id = %job.user_id, | |
| 456 | - | "clamav layer errored while the health probe still reports clamd up; \ | |
| 457 | - | trusted upload passed on reduced AV coverage (fail-open window)" | |
| 447 | + | if clamav_fail_open_occurred(status, is_trusted, clamav_down, &result.layers) { | |
| 448 | + | tracing::warn!( | |
| 449 | + | s3_key = %job.s3_key, | |
| 450 | + | user_id = %job.user_id, | |
| 451 | + | "clamav layer errored while the health probe still reports clamd up; \ | |
| 452 | + | trusted upload passed on reduced AV coverage (fail-open window)" | |
| 453 | + | ); | |
| 454 | + | // Metric (scraped) + WAM ticket (active alert): close R6-Sec-L1's repeated | |
| 455 | + | // "log-only" finding so the reduced-coverage window is operationally visible. | |
| 456 | + | crate::metrics::record_clamav_fail_open(); | |
| 457 | + | if let Some(wam) = ctx.wam.clone() { | |
| 458 | + | let body = format!( | |
| 459 | + | "A trusted upload passed Clean while the clamav layer errored and the \ | |
| 460 | + | health probe had not yet observed clamd down (fail-open window).\n\n\ | |
| 461 | + | s3_key: {}\nuser_id: {}", | |
| 462 | + | job.s3_key, job.user_id | |
| 458 | 463 | ); | |
| 464 | + | wam.create_ticket( | |
| 465 | + | "ClamAV fail-open: trusted upload accepted on reduced AV coverage", | |
| 466 | + | Some(&body), | |
| 467 | + | "medium", | |
| 468 | + | "clamav-fail-open", | |
| 469 | + | Some(&job.s3_key), | |
| 470 | + | ) | |
| 471 | + | .await; | |
| 459 | 472 | } | |
| 460 | 473 | } | |
| 461 | 474 | ||
| 462 | 475 | Ok(status) | |
| 463 | 476 | } | |
| 464 | 477 | ||
| 478 | + | /// The one fail-open acceptance path: a trusted upload resolved `Clean` while its | |
| 479 | + | /// clamav layer errored but the health probe had not yet observed clamd down. | |
| 480 | + | /// Extracted as a pure predicate so the condition is unit-tested rather than | |
| 481 | + | /// living only inline (ultra-fuzz Run 6 R6-Sec-L1). | |
| 482 | + | fn clamav_fail_open_occurred( | |
| 483 | + | status: FileScanStatus, | |
| 484 | + | is_trusted: bool, | |
| 485 | + | clamav_down: bool, | |
| 486 | + | layers: &[crate::scanning::LayerResult], | |
| 487 | + | ) -> bool { | |
| 488 | + | status == FileScanStatus::Clean | |
| 489 | + | && is_trusted | |
| 490 | + | && !clamav_down | |
| 491 | + | && layers | |
| 492 | + | .iter() | |
| 493 | + | .any(|l| l.layer == "clamav" && l.verdict == LayerVerdict::Error) | |
| 494 | + | } | |
| 495 | + | ||
| 465 | 496 | /// Update the per-entity `scan_status` column for the kinds that have one. | |
| 466 | 497 | /// `ItemImage` / `ProjectImage` / `GalleryImage` / `ContentInsertion` don't | |
| 467 | 498 | /// carry their own column — the worker still scanned the file and recorded | |
| @@ -535,6 +566,45 @@ mod tests { | |||
| 535 | 566 | assert_eq!(resolve_pass_status(CLEAN, true, true, &layers), FileScanStatus::Clean); | |
| 536 | 567 | } | |
| 537 | 568 | ||
| 569 | + | // ── clamav_fail_open_occurred (R6-Sec-L1 observability predicate) ── | |
| 570 | + | ||
| 571 | + | #[test] | |
| 572 | + | fn fail_open_detected_for_trusted_errored_while_probe_up() { | |
| 573 | + | let layers = [layer("clamav", LayerVerdict::Error)]; | |
| 574 | + | assert!(clamav_fail_open_occurred(CLEAN, true, false, &layers)); | |
| 575 | + | } | |
| 576 | + | ||
| 577 | + | #[test] | |
| 578 | + | fn fail_open_not_detected_for_untrusted_upload() { | |
| 579 | + | // Untrusted uploads are held on a clamav error, never fail-open. | |
| 580 | + | let layers = [layer("clamav", LayerVerdict::Error)]; | |
| 581 | + | assert!(!clamav_fail_open_occurred(CLEAN, false, false, &layers)); | |
| 582 | + | } | |
| 583 | + | ||
| 584 | + | #[test] | |
| 585 | + | fn fail_open_not_detected_when_probe_already_down() { | |
| 586 | + | // resolve_pass_status already held this case; it is not a silent window. | |
| 587 | + | let layers = [layer("clamav", LayerVerdict::Error)]; | |
| 588 | + | assert!(!clamav_fail_open_occurred(CLEAN, true, true, &layers)); | |
| 589 | + | } | |
| 590 | + | ||
| 591 | + | #[test] | |
| 592 | + | fn fail_open_not_detected_when_clamav_passed() { | |
| 593 | + | let layers = [layer("clamav", LayerVerdict::Pass)]; | |
| 594 | + | assert!(!clamav_fail_open_occurred(CLEAN, true, false, &layers)); | |
| 595 | + | } | |
| 596 | + | ||
| 597 | + | #[test] | |
| 598 | + | fn fail_open_not_detected_when_pipeline_not_clean() { | |
| 599 | + | let layers = [layer("clamav", LayerVerdict::Error)]; | |
| 600 | + | assert!(!clamav_fail_open_occurred( | |
| 601 | + | FileScanStatus::HeldForReview, | |
| 602 | + | true, | |
| 603 | + | false, | |
| 604 | + | &layers | |
| 605 | + | )); | |
| 606 | + | } | |
| 607 | + | ||
| 538 | 608 | #[test] | |
| 539 | 609 | fn trusted_passes_on_transient_error_while_probe_healthy() { | |
| 540 | 610 | // Single transient clamav error but the probe still reports healthy — |
| @@ -700,13 +700,41 @@ async fn oauth_prompt_none_logged_in_issues_code() { | |||
| 700 | 700 | // Signup leaves a validated session (tracking id set by track_session). | |
| 701 | 701 | let (_app_id, client_id) = create_sync_app(&h.db, user_id).await; | |
| 702 | 702 | let (_verifier, challenge) = generate_pkce(); | |
| 703 | + | let redirect_uri = "http://127.0.0.1:9999/callback"; | |
| 703 | 704 | ||
| 705 | + | // Silent re-auth only issues a code for scopes already consented to | |
| 706 | + | // (R6-Sec-L5), so interactively authorize the scope first — the real RP flow | |
| 707 | + | // (a user logs in with MNW once, then the RP refreshes silently). | |
| 708 | + | let resp = h | |
| 709 | + | .client | |
| 710 | + | .get(&format!( | |
| 711 | + | "/oauth/authorize?response_type=code&client_id={}&redirect_uri={}&state=pn0&code_challenge={}&code_challenge_method=S256&scope={}", | |
| 712 | + | urlencoding::encode(&client_id), | |
| 713 | + | urlencoding::encode(redirect_uri), | |
| 714 | + | challenge, | |
| 715 | + | urlencoding::encode("profile:read perks:read"), | |
| 716 | + | )) | |
| 717 | + | .await; | |
| 718 | + | assert_eq!(resp.status.as_u16(), 200, "authorize page: {}", resp.text); | |
| 719 | + | let csrf = h.client.csrf_token().expect("csrf").to_string(); | |
| 720 | + | let body = format!( | |
| 721 | + | "client_id={}&redirect_uri={}&state=pn0&code_challenge={}&code_challenge_method=S256&scope={}&_csrf={}", | |
| 722 | + | urlencoding::encode(&client_id), | |
| 723 | + | urlencoding::encode(redirect_uri), | |
| 724 | + | challenge, | |
| 725 | + | urlencoding::encode("profile:read perks:read"), | |
| 726 | + | urlencoding::encode(&csrf), | |
| 727 | + | ); | |
| 728 | + | let resp = h.client.post_form("/oauth/authorize", &body).await; | |
| 729 | + | assert!(resp.status.is_redirection(), "consent POST: {} {}", resp.status, resp.text); | |
| 730 | + | ||
| 731 | + | // Now prompt=none silently issues a code for the consented scope. | |
| 704 | 732 | let resp = h | |
| 705 | 733 | .client | |
| 706 | 734 | .get(&format!( | |
| 707 | 735 | "/oauth/authorize?response_type=code&client_id={}&redirect_uri={}&state=pn&code_challenge={}&code_challenge_method=S256&scope={}&prompt=none", | |
| 708 | 736 | urlencoding::encode(&client_id), | |
| 709 | - | urlencoding::encode("http://127.0.0.1:9999/callback"), | |
| 737 | + | urlencoding::encode(redirect_uri), | |
| 710 | 738 | challenge, | |
| 711 | 739 | urlencoding::encode("profile:read perks:read"), | |
| 712 | 740 | )) | |
| @@ -757,3 +785,75 @@ async fn oauth_discovery_metadata() { | |||
| 757 | 785 | let scopes = meta.get("scopes_supported").and_then(|s| s.as_array()).expect("scopes"); | |
| 758 | 786 | assert!(scopes.iter().any(|s| s == "offline_access")); | |
| 759 | 787 | } | |
| 788 | + | ||
| 789 | + | /// R6-Sec-L5: the prompt=none silent re-auth path may only mint a code for | |
| 790 | + | /// scopes already interactively consented to. A subset request is issued | |
| 791 | + | /// silently; a wider request returns error=consent_required. | |
| 792 | + | #[tokio::test] | |
| 793 | + | async fn oauth_prompt_none_gated_by_prior_consent() { | |
| 794 | + | let mut h = TestHarness::new().await; | |
| 795 | + | let user_id = h.signup("oauthconsent", "oc@example.com", "Password1!").await; | |
| 796 | + | let (_app_id, client_id) = create_sync_app(&h.db, user_id).await; | |
| 797 | + | ||
| 798 | + | // A validated site session is required for the silent path. | |
| 799 | + | h.client.post_form("/logout", "").await; | |
| 800 | + | h.login("oauthconsent", "Password1!").await; | |
| 801 | + | ||
| 802 | + | let (_verifier, challenge) = generate_pkce(); | |
| 803 | + | let redirect_uri = "http://127.0.0.1:9999/callback"; | |
| 804 | + | ||
| 805 | + | // Interactive consent for `profile:read perks:read`. | |
| 806 | + | let resp = h | |
| 807 | + | .client | |
| 808 | + | .get(&format!( | |
| 809 | + | "/oauth/authorize?response_type=code&client_id={}&redirect_uri={}&state=s1&code_challenge={}&code_challenge_method=S256&scope={}", | |
| 810 | + | urlencoding::encode(&client_id), | |
| 811 | + | urlencoding::encode(redirect_uri), | |
| 812 | + | challenge, | |
| 813 | + | urlencoding::encode("profile:read perks:read"), | |
| 814 | + | )) | |
| 815 | + | .await; | |
| 816 | + | assert_eq!(resp.status.as_u16(), 200, "authorize page: {}", resp.text); | |
| 817 | + | let csrf = h.client.csrf_token().expect("csrf").to_string(); | |
| 818 | + | let body = format!( | |
| 819 | + | "client_id={}&redirect_uri={}&state=s1&code_challenge={}&code_challenge_method=S256&scope={}&_csrf={}", | |
| 820 | + | urlencoding::encode(&client_id), | |
| 821 | + | urlencoding::encode(redirect_uri), | |
| 822 | + | challenge, | |
| 823 | + | urlencoding::encode("profile:read perks:read"), | |
| 824 | + | urlencoding::encode(&csrf), | |
| 825 | + | ); | |
| 826 | + | let resp = h.client.post_form("/oauth/authorize", &body).await; | |
| 827 | + | assert!(resp.status.is_redirection(), "consent POST: {} {}", resp.status, resp.text); | |
| 828 | + | ||
| 829 | + | // prompt=none with a SUBSET scope -> silent code issued. | |
| 830 | + | let resp = h | |
| 831 | + | .client | |
| 832 | + | .get(&format!( | |
| 833 | + | "/oauth/authorize?response_type=code&client_id={}&redirect_uri={}&state=s2&code_challenge={}&code_challenge_method=S256&scope={}&prompt=none", | |
| 834 | + | urlencoding::encode(&client_id), | |
| 835 | + | urlencoding::encode(redirect_uri), | |
| 836 | + | challenge, | |
| 837 | + | urlencoding::encode("profile:read"), | |
| 838 | + | )) | |
| 839 | + | .await; | |
| 840 | + | assert!(resp.status.is_redirection(), "silent subset should redirect: {} {}", resp.status, resp.text); | |
| 841 | + | let loc = resp.header("location").expect("location"); | |
| 842 | + | assert!(loc.contains("code="), "silent subset should carry a code: {loc}"); | |
| 843 | + | assert!(!loc.contains("error="), "silent subset should not error: {loc}"); | |
| 844 | + | ||
| 845 | + | // prompt=none with a WIDER scope (offline_access never consented) -> consent_required. | |
| 846 | + | let resp = h | |
| 847 | + | .client | |
| 848 | + | .get(&format!( | |
| 849 | + | "/oauth/authorize?response_type=code&client_id={}&redirect_uri={}&state=s3&code_challenge={}&code_challenge_method=S256&scope={}&prompt=none", | |
| 850 | + | urlencoding::encode(&client_id), | |
| 851 | + | urlencoding::encode(redirect_uri), | |
| 852 | + | challenge, | |
| 853 | + | urlencoding::encode("profile:read perks:read offline_access"), | |
| 854 | + | )) | |
| 855 | + | .await; | |
| 856 | + | assert!(resp.status.is_redirection(), "silent wider should redirect: {} {}", resp.status, resp.text); | |
| 857 | + | let loc = resp.header("location").expect("location"); | |
| 858 | + | assert!(loc.contains("error=consent_required"), "wider scope must require consent: {loc}"); | |
| 859 | + | } |