From e29b0c12ab256996bc465d48f2bf08385dd47532 Mon Sep 17 00:00:00 2001 From: csd113 Date: Fri, 26 Jun 2026 18:57:06 -0700 Subject: [PATCH 1/4] Document RustChan 1.3.0 release notes --- CHANGELOG.md | 56 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c43ff3..3d2c0dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,15 +4,65 @@ All notable changes to RustChan will be documented in this file. ## RustChan 1.3.0 +### Added + +- Added a guided first-run setup wizard at `/setup` with public/private/local presets, CSRF protection, review-before-write confirmation, initial admin and board creation, Tor/proxy/Secure-cookie options, CAPTCHA and activity-badge toggles, media limits, and backup defaults. +- Added admin maintenance controls to reopen or close the setup wizard later without replacing existing admin credentials. +- Added per-board PDF upload limits, setup/admin controls for enabling PDF uploads, and upload-form hints that show PDF caps separately from image, video, audio, and generic file limits. +- Added a clean `1.3.0` database baseline. Fresh databases now install the baseline directly, matching in-development databases are stamped as `1.3.0`, and partial or unknown schemas fail closed with diagnostics. +- Added schema health checks to database/admin status paths, including structural verification, SQLite quick checks, foreign-key checks, and `rustchan-cli admin db-status`. +- Added admin dashboard summaries for setup state, schema/database health, backups, storage, Tor, dependencies, background jobs, reports, and live/total thread counts. +- Added a footer copy control for the active Tor onion address. + +### Improved + +- Hardened upload auto-compression so processed images and videos are re-statted and revalidated before a post is finalized, with better cleanup for failed compression, poll validation failures, stale staged files, thumbnails, and partial database rows. +- Improved browser-side auto-compression reliability for extension-only or blank-MIME uploads, Chromium/WebKit video capture timeouts, submit-time guards, retry behavior, and inline compression status messages. +- Improved media validation for PDFs, ADTS AAC, WebM audio, MKV/Matroska video, small FLAC/audio files, invalid thumbnail payloads, empty upload controls, zero-byte named uploads, and cross-board media deduplication. +- Returned semantic post-upload error statuses for JS, no-JS, and XHR submissions, including `413`, `415`, and `422` where appropriate. +- Improved admin and public UI accessibility with stronger labels, IDs, ARIA attributes, inert/hidden modal states, focus behavior, report and confirmation dialogs, catalog menus, NSFW dialog behavior, theme picker behavior, media controls, and mobile form hints. +- Improved Tor/onion operation with Arti `0.43.0`, safer onion admin origin handling, clearer bootstrap/service logs, more accurate hidden-service key path reporting, and stabilized Tor restore tests. +- Improved observability defaults and documentation so `/healthz` stays public and minimal, `/readyz` hides operational details unless `public_readiness_details = true`, and `/metrics` returns `404` unless `public_metrics_enabled = true`. +- Improved upload, thread, admin, and setup tests around size limits, schema state, CSRF handling, setup reopen/close behavior, pending media refresh, and upload error classification. + +### Fixed + +- Fixed non-image media regressions that affected small supported audio files, valid `.mkv` uploads, `audio/webm` persistence, and audio/video MIME preservation. +- Fixed malformed ADTS AAC acceptance so invalid AAC is rejected before storage or background media jobs are created. +- Fixed PDF limit edge cases so files exactly at the configured cap are accepted while cap-plus-one uploads are rejected. +- Fixed no-JS and pending-media regressions: Firefox no-JS theme changes persist through safe local redirects, and thread update controls expose a stable `data-action="fetch-updates"` hook for pending-media refreshes. +- Fixed YouTube embed thumbnail sizing. +- Fixed legacy `1.3.0` schema drift handling on startup for in-development databases. +- Fixed unintended manual textarea resize handles on RustChan forms. +- Updated admin login button copy. + +### Security + +- Required CSRF validation for theme changes and board backup actions. +- Made detailed readiness data and unauthenticated Prometheus metrics opt-in to avoid exposing operational internals on internet or onion deployments by default. +- Kept setup password handling from echoing raw admin passwords during review by using a short-lived pending hash token. +- Pinned `rustls-webpki` to a patched release for RUSTSEC-2026-0049 and refreshed the dependency lockfile. + +### Documentation + +- Updated `README.md` and `SETUP.md` for RustChan `1.3.0`, the new schema baseline, `db-status`, PDF upload limits, setup behavior, and observability endpoint defaults. +- Added a static ChanNet security audit record for future follow-up. +- Refreshed release documentation and current-version references. + +### Internal + +- Updated Rust dependencies, including Arti/Tor crates, `sha3`, `zip`, `toml`, `tokio`, `axum`, `rustls`, and related lockfile entries. +- Renamed the saved-backup implementation module for clearer ownership. +- Cleaned focused Clippy warnings and kept strict lint coverage passing during the release cycle. + +## RustChan 1.2.2 + - Replaced browser proof-of-work posting CAPTCHA with server-generated image CAPTCHA challenges. - Improved secure-cookie handling across HTTP, HTTPS, trusted-proxy HTTPS, admin sessions, board access cookies, CSRF cookies, and owned-post cookies. - Added no-JS fallbacks and accessibility polish for posting, reporting, catalog actions, board preferences, moderation controls, and own-post edit/delete flows. - Hardened upload handling for empty file controls, zero-byte named uploads, empty thumbnail payloads, invalid media, and cross-board media deduplication. -- Tightened post upload validation so rejected media now returns semantic HTTP statuses for JS and no-JS submissions, exact-limit PDFs are accepted while cap + 1 PDFs are rejected, and malformed ADTS AAC is rejected before storage or background job creation. -- Fixed late-cycle no-JS and pending-media regressions: Firefox no-JS theme changes persist through safe local redirects, and thread update controls expose a stable `data-action="fetch-updates"` hook for pending-media refreshes. - Improved activity badge cache behavior, especially on mobile WebKit and browser back/forward navigation. - Hardened settings validation so invalid config values fail closed instead of silently falling back. -- Squashed the pre-release internal database migration ladder into a clean `1.3.0` baseline schema. Fresh databases now install that baseline directly, structurally matching in-development databases are stamped as schema version `1.3.0`, and partial or unknown schemas fail closed with diagnostics instead of blind migration attempts. - Polished responsive layout, long-content wrapping, modal focus behavior, ESC handling, touch targets, and light-theme error contrast. - Updated backup UI metadata handling and dynamic split-part options. - Refreshed README screenshots and release documentation. From f8814fe0e10385cafa1fbfdcad1004d989e34870 Mon Sep 17 00:00:00 2001 From: csd113 Date: Fri, 26 Jun 2026 22:36:29 -0700 Subject: [PATCH 2/4] fix(security): address release-blocker audit findings Scope board thread updates to their board and preserve Tor stream-token precedence. Harden ChanNet route/key handling and private secret, config, and backup file modes. Recompute body_html during restore and keep full-restore temp DB files private with cleanup guard coverage. Validation passed: git diff --check && cargo fmt --all --check && cargo clippy --workspace --all-targets --all-features -- -D warnings -D clippy::all -D clippy::pedantic -D clippy::nursery -D clippy::cargo && cargo test --workspace --all-features. --- src/chan_net/mod.rs | 177 +++++++- src/config.rs | 198 +++++++-- src/db/posts.rs | 22 +- src/detect.rs | 18 + src/handlers/admin/backup.rs | 48 ++- src/handlers/admin/backup/archive.rs | 32 +- src/handlers/admin/backup/create.rs | 152 ++++--- src/handlers/admin/backup/downloads.rs | 8 +- src/handlers/admin/backup/restore_full.rs | 473 +++++++++++++++++++++- src/handlers/admin/backup/saved_backup.rs | 6 +- src/handlers/board/tests.rs | 41 ++ src/handlers/thread.rs | 9 +- src/middleware/ip.rs | 107 +++-- src/server/server.rs | 3 + 14 files changed, 1126 insertions(+), 168 deletions(-) diff --git a/src/chan_net/mod.rs b/src/chan_net/mod.rs index b2afbe8..b171d26 100644 --- a/src/chan_net/mod.rs +++ b/src/chan_net/mod.rs @@ -32,6 +32,7 @@ use axum::{ Json, Router, }; use serde_json::json; +use std::sync::Arc; // ── ChanError ───────────────────────────────────────────────────────────────── // @@ -125,18 +126,20 @@ async fn json_body_limit_error(req: axum::http::Request, next: // ─── ChanNet API key middleware ─────────────────────────────────────────────── /// Middleware that enforces the pre-shared `X-ChanNet-Key` header on sensitive -/// `ChanNet` endpoints (/chan/refresh and /chan/poll). +/// `ChanNet` endpoints. /// -/// These endpoints were previously unauthenticated. Any process -/// that could reach the `ChanNet` bind address could trigger a full DB snapshot -/// push (refresh) or pull-and-import from a remote node (poll) with no -/// credentials. The API key is configured via `CHAN_NET_API_KEY` / settings.toml. +/// Any process that can reach the `ChanNet` bind address can otherwise trigger +/// snapshot export/import or gateway commands with no credentials. The API key +/// is configured via `CHAN_NET_API_KEY` / settings.toml. /// /// If `chan_net_api_key` is empty the request is rejected with 403 Forbidden /// (the feature is intentionally disabled rather than wide open). -async fn verify_chan_api_key(req: axum::extract::Request, next: Next) -> Response { +async fn verify_chan_api_key( + axum::extract::State(expected): axum::extract::State>, + req: axum::extract::Request, + next: Next, +) -> Response { use subtle::ConstantTimeEq as _; - let expected = &crate::config::CONFIG.chan_net_api_key; if expected.is_empty() { // API key not configured — refuse the request to prevent accidental // exposure when an operator forgets to set the key. @@ -167,9 +170,21 @@ async fn verify_chan_api_key(req: axum::extract::Request, next: Next) -> Respons /// being rejected as 413 before validation runs. The config default is /// `128 * 1024`. Do NOT set it below `34_000` bytes. pub fn chan_router(state: AppState) -> Router { - Router::new() - // ── Status ────────────────────────────────────────────────────────── - .route("/chan/status", get(status::chan_status)) + chan_router_with_auth( + state, + Arc::from(CONFIG.chan_net_api_key.as_str()), + CONFIG.chan_net_command_max_body, + CONFIG.chan_net_max_body, + ) +} + +fn chan_router_with_auth( + state: AppState, + api_key: Arc, + command_max_body: usize, + import_max_body: usize, +) -> Router { + let protected_routes = Router::new() // ── RustWave gateway — raw JSON in, ZIP data package out ───────────── // // The json_body_limit_error middleware is applied *outside* the @@ -178,23 +193,143 @@ pub fn chan_router(state: AppState) -> Router { .route( "/chan/command", post(command::chan_command) - .layer(DefaultBodyLimit::max(CONFIG.chan_net_command_max_body)) + .layer(DefaultBodyLimit::max(command_max_body)) .layer(middleware::from_fn(json_body_limit_error)), ) // ── Federation sync — ZIP in, ZIP out ──────────────────────────────── .route("/chan/export", post(export::chan_export)) .route( "/chan/import", - post(import::chan_import).layer(DefaultBodyLimit::max(CONFIG.chan_net_max_body)), - ) - // /chan/refresh and /chan/poll now require X-ChanNet-Key. - .route( - "/chan/refresh", - post(refresh::chan_refresh).layer(middleware::from_fn(verify_chan_api_key)), - ) - .route( - "/chan/poll", - post(poll::chan_poll).layer(middleware::from_fn(verify_chan_api_key)), + post(import::chan_import).layer(DefaultBodyLimit::max(import_max_body)), ) + .route("/chan/refresh", post(refresh::chan_refresh)) + .route("/chan/poll", post(poll::chan_poll)) + .route_layer(middleware::from_fn_with_state(api_key, verify_chan_api_key)); + + Router::new() + // ── Status ────────────────────────────────────────────────────────── + .route("/chan/status", get(status::chan_status)) + .merge(protected_routes) .with_state(state) } + +#[cfg(test)] +mod tests { + use super::chan_router_with_auth; + use axum::{ + body::Body, + http::{header, Request, StatusCode}, + }; + use std::sync::Arc; + use tower::ServiceExt as _; + + const TEST_KEY: &str = "0123456789abcdef0123456789abcdef"; + + fn chan_test_state() -> crate::middleware::AppState { + let mut state = crate::test_support::app_state(); + state.chan_ledger = Some(Arc::new(parking_lot::Mutex::new( + crate::chan_net::ledger::TxLedger::default(), + ))); + state + } + + fn chan_test_router(state: crate::middleware::AppState) -> axum::Router { + chan_router_with_auth(state, Arc::from(TEST_KEY), 128 * 1024, 10 * 1024 * 1024) + } + + #[tokio::test] + async fn protected_chan_routes_reject_missing_and_wrong_key() { + let router = chan_test_router(chan_test_state()); + let cases = [ + ("/chan/command", Body::from(r#"{"type":"full_export"}"#)), + ("/chan/export", Body::empty()), + ("/chan/import", Body::from(Vec::::new())), + ]; + + for (path, body) in cases { + let missing = router + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri(path) + .header(header::CONTENT_TYPE, "application/json") + .body(body) + .expect("missing-key request"), + ) + .await + .expect("missing-key response"); + assert_eq!(missing.status(), StatusCode::UNAUTHORIZED, "{path}"); + + let wrong = router + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri(path) + .header("X-ChanNet-Key", "wrong-key") + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(r#"{"type":"full_export"}"#)) + .expect("wrong-key request"), + ) + .await + .expect("wrong-key response"); + assert_eq!(wrong.status(), StatusCode::UNAUTHORIZED, "{path}"); + } + } + + #[tokio::test] + async fn protected_chan_routes_accept_correct_key() { + let state = chan_test_state(); + let snapshot = { + let conn = state.db.get().expect("db connection"); + crate::chan_net::snapshot::build_snapshot(&conn) + .expect("snapshot") + .0 + }; + let router = chan_test_router(state); + + let command = router + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri("/chan/command") + .header("X-ChanNet-Key", TEST_KEY) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(r#"{"type":"full_export"}"#)) + .expect("command request"), + ) + .await + .expect("command response"); + assert_eq!(command.status(), StatusCode::OK); + + let export = router + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri("/chan/export") + .header("X-ChanNet-Key", TEST_KEY) + .body(Body::empty()) + .expect("export request"), + ) + .await + .expect("export response"); + assert_eq!(export.status(), StatusCode::OK); + + let import = router + .oneshot( + Request::builder() + .method("POST") + .uri("/chan/import") + .header("X-ChanNet-Key", TEST_KEY) + .header(header::CONTENT_TYPE, "application/zip") + .body(Body::from(snapshot)) + .expect("import request"), + ) + .await + .expect("import response"); + assert_eq!(import.status(), StatusCode::OK); + } +} diff --git a/src/config.rs b/src/config.rs index 4b87790..809e5bb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -121,6 +121,82 @@ pub fn runtime_banner_dir() -> PathBuf { runtime_dir().join("banner") } +/// Create a private directory and repair its mode on Unix. +/// +/// On non-Unix platforms this preserves the existing platform behavior. +/// +/// # Errors +/// Returns an error if the directory cannot be created or its mode cannot be set. +pub fn ensure_private_dir(path: &Path) -> anyhow::Result<()> { + std::fs::create_dir_all(path)?; + restrict_private_dir_permissions(path) +} + +/// Repair a secret-bearing directory mode on Unix. +/// +/// # Errors +/// Returns an error if the mode cannot be changed. +pub fn restrict_private_dir_permissions(path: &Path) -> anyhow::Result<()> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700))?; + } + + #[cfg(not(unix))] + let _ = path; + + Ok(()) +} + +/// Repair a secret-bearing file mode on Unix. +/// +/// # Errors +/// Returns an error if the mode cannot be changed. +pub fn restrict_private_file_permissions(path: &Path) -> anyhow::Result<()> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; + } + + #[cfg(not(unix))] + let _ = path; + + Ok(()) +} + +/// Write a secret-bearing file and repair its mode on Unix. +/// +/// # Errors +/// Returns an error if the parent directory cannot be created, the file cannot +/// be written, or its mode cannot be set. +pub fn write_private_file(path: &Path, contents: impl AsRef<[u8]>) -> anyhow::Result<()> { + if let Some(parent) = path.parent() { + ensure_private_dir(parent)?; + } + + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + + let mut file = std::fs::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .mode(0o600) + .open(path)?; + file.write_all(contents.as_ref())?; + file.sync_all()?; + } + + #[cfg(not(unix))] + std::fs::write(path, contents)?; + + restrict_private_file_permissions(path)?; + Ok(()) +} + type RuntimeDirMigration = (&'static str, fn() -> PathBuf); const RUNTIME_LAYOUT_MIGRATIONS: &[RuntimeDirMigration] = &[ @@ -156,7 +232,7 @@ fn migrate_dir_if_present(old_path: &Path, new_path: &Path) -> anyhow::Result<() return Ok(()); } if let Some(parent) = new_path.parent() { - std::fs::create_dir_all(parent)?; + ensure_private_dir(parent)?; } std::fs::rename(old_path, new_path)?; Ok(()) @@ -169,7 +245,7 @@ fn migrate_dir_if_present(old_path: &Path, new_path: &Path) -> anyhow::Result<() /// created or removed, or if an old and new path conflict by type. pub fn migrate_runtime_layout_if_needed() -> anyhow::Result<()> { let data_dir = data_dir(); - std::fs::create_dir_all(&data_dir)?; + ensure_private_dir(&data_dir)?; for dir in [ runtime_dir(), runtime_tmp_dir(), @@ -180,7 +256,7 @@ pub fn migrate_runtime_layout_if_needed() -> anyhow::Result<()> { full_backups_dir(), board_backups_dir(), ] { - std::fs::create_dir_all(dir)?; + ensure_private_dir(&dir)?; } for &(legacy_name, destination) in RUNTIME_LAYOUT_MIGRATIONS { @@ -346,6 +422,16 @@ fn parse_settings_file_str(raw: &str) -> Result { toml::from_str(raw) } +fn bind_addr_is_loopback(bind_addr: &str) -> bool { + if let Ok(addr) = bind_addr.parse::() { + return addr.ip().is_loopback(); + } + + bind_addr + .strip_prefix("localhost:") + .is_some_and(|port| port.parse::().is_ok()) +} + /// Create settings.toml with defaults if it does not exist yet. /// Call this once at startup (before CONFIG is accessed for the first time). /// @@ -355,6 +441,12 @@ fn parse_settings_file_str(raw: &str) -> Result { pub fn generate_settings_file_if_missing() { let path = settings_file_path(); if path.exists() { + if let Err(error) = restrict_private_file_permissions(&path) { + let _ = writeln!( + std::io::stderr().lock(), + "Warning: could not repair settings.toml permissions: {error}" + ); + } return; } // Generate a random 64-hex-char secret (32 bytes of entropy). @@ -365,7 +457,7 @@ pub fn generate_settings_file_if_missing() { ); let secret = hex::encode(secret_bytes); let content = template::settings_template(&secret); - match std::fs::write(&path, content) { + match write_private_file(&path, content) { Ok(()) => { let _ = writeln!( std::io::stdout().lock(), @@ -1059,27 +1151,9 @@ impl Config { // into bootstrap as a cryptic internal error — invisible at startup. if self.enable_tor_support { for dir in [runtime_tor_state_dir(), runtime_tor_cache_dir()] { - std::fs::create_dir_all(&dir).map_err(|e| { + ensure_private_dir(&dir).map_err(|e| { anyhow::anyhow!("CONFIG ERROR: cannot create Tor dir {}: {e}", dir.display()) })?; - // Arti requires runtime/tor/state/ to have permissions 0700 (no group - // or other read access) for its key material. create_dir_all - // respects the process umask, typically yielding 0755, which - // Arti rejects with "problem with filesystem permissions". - // Explicitly set 0700 on Unix so Arti accepts the directory. - // runtime/tor/cache/ holds no sensitive data and is left at normal - // permissions, but we restrict it too for defence-in-depth. - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt as _; - let perms = std::fs::Permissions::from_mode(0o700); - std::fs::set_permissions(&dir, perms).map_err(|e| { - anyhow::anyhow!( - "CONFIG ERROR: cannot set permissions on Tor dir {}: {e}", - dir.display() - ) - })?; - } let probe = dir.join(".write_probe"); std::fs::write(&probe, b"").map_err(|_error| { anyhow::anyhow!( @@ -1132,6 +1206,21 @@ impl Config { Ok(()) } + /// Validate `ChanNet` listener settings when the `ChanNet` service is enabled. + /// + /// # Errors + /// Returns an error if `ChanNet` would bind to a non-loopback address without + /// a configured pre-shared key. + pub fn validate_chan_net_listener(&self) -> anyhow::Result<()> { + if self.chan_net_api_key.is_empty() && !bind_addr_is_loopback(&self.chan_net_bind) { + anyhow::bail!( + "CONFIG ERROR: --chan-net with chan_net_bind '{}' requires chan_net_api_key when binding outside loopback.", + self.chan_net_bind + ); + } + Ok(()) + } + #[must_use] pub fn bind_addr_with_port(&self, port: u16) -> String { bind_addr_for_port(&self.bind_addr, port) @@ -1248,6 +1337,8 @@ fn update_settings_file_entries_result( tmp.persist(&path) .map_err(|error| error.error) .with_context(|| format!("could not atomically replace {}", path.display()))?; + restrict_private_file_permissions(&path) + .with_context(|| format!("could not repair permissions on {}", path.display()))?; Ok(()) } @@ -2002,6 +2093,67 @@ port = 8080 config.validate().expect("32-char key is accepted"); } + #[test] + fn validate_chan_net_listener_requires_key_for_non_loopback_bind() { + let mut config = valid_config(); + config.chan_net_api_key.clear(); + config.chan_net_bind = "0.0.0.0:7070".to_owned(); + + let error = config + .validate_chan_net_listener() + .expect_err("non-loopback bind without key should fail") + .to_string(); + assert_eq!( + error, + "CONFIG ERROR: --chan-net with chan_net_bind '0.0.0.0:7070' requires chan_net_api_key when binding outside loopback." + ); + + config.chan_net_bind = "127.0.0.1:7070".to_owned(); + config + .validate_chan_net_listener() + .expect("loopback bind may disable endpoints"); + + config.chan_net_bind = "localhost:7070".to_owned(); + config + .validate_chan_net_listener() + .expect("localhost bind may disable endpoints"); + + config.chan_net_bind = "0.0.0.0:7070".to_owned(); + config.chan_net_api_key = "x".repeat(32); + config + .validate_chan_net_listener() + .expect("non-loopback bind with key is accepted"); + } + + #[cfg(unix)] + #[test] + fn private_file_helpers_set_owner_only_modes() { + use std::os::unix::fs::PermissionsExt as _; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let private_dir = temp_dir.path().join("private"); + super::ensure_private_dir(&private_dir).expect("private dir"); + assert_eq!( + std::fs::metadata(&private_dir) + .expect("dir metadata") + .permissions() + .mode() + & 0o777, + 0o700 + ); + + let private_file = private_dir.join("settings.toml"); + super::write_private_file(&private_file, b"secret").expect("private file"); + assert_eq!( + std::fs::metadata(&private_file) + .expect("file metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + } + #[test] fn validate_rejects_tor_only_without_tor_support() { let mut config = valid_config(); diff --git a/src/db/posts.rs b/src/db/posts.rs index f95de12..4ac063c 100644 --- a/src/db/posts.rs +++ b/src/db/posts.rs @@ -149,18 +149,22 @@ pub fn get_posts_for_thread(conn: &rusqlite::Connection, thread_id: i64) -> Resu /// Returns an error if the database operation fails. pub fn get_new_posts_since( conn: &rusqlite::Connection, + board_id: i64, thread_id: i64, since_id: i64, max_results: i64, ) -> Result> { let mut stmt = conn.prepare_cached(&format!( "SELECT {POST_SELECT_COLUMNS} - FROM posts WHERE thread_id = ?1 AND id > ?2 + FROM posts WHERE board_id = ?1 AND thread_id = ?2 AND id > ?3 ORDER BY id ASC - LIMIT ?3" + LIMIT ?4" ))?; let posts = stmt - .query_map(params![thread_id, since_id, max_results], map_post)? + .query_map( + params![board_id, thread_id, since_id, max_results], + map_post, + )? .collect::>>()?; Ok(posts) } @@ -171,6 +175,7 @@ pub fn get_new_posts_since( /// Returns an error if the database operation fails. pub fn get_posts_by_ids_in_thread( conn: &rusqlite::Connection, + board_id: i64, thread_id: i64, post_ids: &[i64], ) -> Result> { @@ -181,18 +186,21 @@ pub fn get_posts_by_ids_in_thread( let placeholders = post_ids .iter() .enumerate() - .map(|(index, _)| format!("?{}", index + 2)) + .map(|(index, _)| format!("?{}", index + 3)) .collect::>() .join(", "); let sql = format!( "SELECT {POST_SELECT_COLUMNS} FROM posts - WHERE thread_id = ?1 AND id IN ({placeholders}) + WHERE board_id = ?1 AND thread_id = ?2 AND id IN ({placeholders}) ORDER BY created_at ASC, id ASC" ); let mut stmt = conn.prepare_cached(&sql)?; - let params = - rusqlite::params_from_iter(std::iter::once(thread_id).chain(post_ids.iter().copied())); + let params = rusqlite::params_from_iter( + [board_id, thread_id] + .into_iter() + .chain(post_ids.iter().copied()), + ); let posts = stmt .query_map(params, map_post)? .collect::>>()?; diff --git a/src/detect.rs b/src/detect.rs index 411ed8e..a9c0615 100644 --- a/src/detect.rs +++ b/src/detect.rs @@ -1,5 +1,6 @@ // Startup detection for optional external tools. +use std::net::SocketAddr; use std::path::Path; use std::process::{Command, Stdio}; use std::sync::Arc; @@ -339,6 +340,23 @@ use tor_hsservice::{config::OnionServiceConfigBuilder, handle_rend_requests, HsI // The token is random per-stream so it cannot track users across reconnections. pub static TOR_STREAM_TOKENS: LazyLock>> = LazyLock::new(DashMap::new); +#[must_use] +pub fn tor_stream_token_identity( + peer: Option, + enable_tor_support: bool, +) -> Option { + if !enable_tor_support { + return None; + } + let addr = peer?; + if !addr.ip().is_loopback() { + return None; + } + TOR_STREAM_TOKENS + .get(&addr.port()) + .map(|token| token.value().to_string()) +} + /// Removes a port→token entry from `TOR_STREAM_TOKENS` when dropped. struct TokenGuard(u16); impl Drop for TokenGuard { diff --git a/src/handlers/admin/backup.rs b/src/handlers/admin/backup.rs index 9afc994..a98f953 100644 --- a/src/handlers/admin/backup.rs +++ b/src/handlers/admin/backup.rs @@ -171,6 +171,12 @@ pub async fn admin_backup(State(state): State, jar: CookieJar) -> Resu conn.execute_batch(&format!("VACUUM INTO '{temp_db_str}'")) .map_err(|e| AppError::Internal(anyhow::anyhow!("VACUUM INTO failed: {e}")))?; + crate::config::restrict_private_file_permissions(&temp_db).map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Set private permissions on {}: {error}", + temp_db.display() + )) + })?; // Count files for progress bar before compressing. progress.reset(crate::middleware::backup_phase::COUNT_FILES); @@ -588,7 +594,7 @@ pub async fn board_backup( let download_token = new_session_id(); write_temp_board_download_token(&temp_name, &download_token)?; let download_dir = temp_board_download_dir(); - std::fs::create_dir_all(&download_dir).map_err(|error| { + crate::config::ensure_private_dir(&download_dir).map_err(|error| { AppError::Internal(anyhow::anyhow!( "Create temp board download dir {}: {error}", download_dir.display() @@ -601,6 +607,12 @@ pub async fn board_backup( final_path.display() )) })?; + crate::config::restrict_private_file_permissions(&final_path).map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Set private permissions on {}: {error}", + final_path.display() + )) + })?; temp_zip_guard.disarm(); return Ok(Redirect::to(&format!( "/admin/backup/download/temp-board/{temp_name}?cleanup=1&token={download_token}" @@ -1215,6 +1227,19 @@ mod tests { let download_path = temp_board_download_dir().join(download_filename); assert!(download_path.exists()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + + assert_eq!( + std::fs::metadata(&download_path) + .expect("temp zip metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + } assert!( temp_board_download_token_path(download_filename).exists(), "download token should be written before the download" @@ -1462,6 +1487,27 @@ mod tests { let _ = std::fs::remove_file(&token_path); write_temp_board_download_token(filename, token).expect("write token"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + + assert_eq!( + std::fs::metadata(&token_path) + .expect("token metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + assert_eq!( + std::fs::metadata(temp_board_download_dir()) + .expect("download dir metadata") + .permissions() + .mode() + & 0o777, + 0o700 + ); + } assert!(consume_temp_board_download_token(filename, token).expect("consume token")); assert!(!consume_temp_board_download_token(filename, token).expect("token removed")); } diff --git a/src/handlers/admin/backup/archive.rs b/src/handlers/admin/backup/archive.rs index 8426328..59f3421 100644 --- a/src/handlers/admin/backup/archive.rs +++ b/src/handlers/admin/backup/archive.rs @@ -73,6 +73,12 @@ pub(super) fn extract_sqlite_db_from_full_backup_archive Result<(PathBuf, String)> { prune_stale_temp_board_downloads(); - std::fs::create_dir_all(temp_board_download_dir()).map_err(|error| { + crate::config::ensure_private_dir(&temp_board_download_dir()).map_err(|error| { AppError::Internal(anyhow::anyhow!("Create temp board backup dir: {error}")) })?; @@ -615,6 +639,12 @@ pub(super) fn create_temp_board_backup_from_full_backup_path( let _ = std::fs::remove_file(&tmp_path); AppError::Internal(anyhow::anyhow!("Rename extracted board backup: {error}")) })?; + crate::config::restrict_private_file_permissions(&final_path).map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Set private permissions on {}: {error}", + final_path.display() + )) + })?; Ok((final_path, filename)) } diff --git a/src/handlers/admin/backup/create.rs b/src/handlers/admin/backup/create.rs index 2c73631..2aefe02 100644 --- a/src/handlers/admin/backup/create.rs +++ b/src/handlers/admin/backup/create.rs @@ -78,12 +78,8 @@ pub(crate) fn create_full_backup_to_server( let root_dir = saved_backup::create_backup_root(&backup_id)?; let db_dir = root_dir.join("db"); let config_dir = root_dir.join("config"); - std::fs::create_dir_all(&db_dir).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create {}: {error}", db_dir.display())) - })?; - std::fs::create_dir_all(&config_dir).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create {}: {error}", config_dir.display())) - })?; + ensure_backup_dir(&db_dir)?; + ensure_backup_dir(&config_dir)?; progress.reset(crate::middleware::backup_phase::SNAPSHOT_DB); log_backup_phase(crate::middleware::backup_phase::SNAPSHOT_DB); @@ -94,6 +90,7 @@ pub(crate) fn create_full_backup_to_server( .replace('\'', "''"); conn.execute_batch(&format!("VACUUM INTO '{db_snapshot_str}'")) .map_err(|error| AppError::Internal(anyhow::anyhow!("VACUUM INTO: {error}")))?; + restrict_backup_file(&db_snapshot_path)?; let db_snapshot_size = std::fs::metadata(&db_snapshot_path) .map(|metadata| metadata.len()) .map_err(|error| AppError::Internal(anyhow::anyhow!("Stat DB snapshot: {error}")))?; @@ -298,18 +295,9 @@ pub(crate) fn create_pre_maintenance_backup_to_server( let db_dir = root_dir.join("db"); let config_dir = root_dir.join("config"); let maintenance_dir = root_dir.join("maintenance"); - std::fs::create_dir_all(&db_dir).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create {}: {error}", db_dir.display())) - })?; - std::fs::create_dir_all(&config_dir).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create {}: {error}", config_dir.display())) - })?; - std::fs::create_dir_all(&maintenance_dir).map_err(|error| { - AppError::Internal(anyhow::anyhow!( - "Create {}: {error}", - maintenance_dir.display() - )) - })?; + ensure_backup_dir(&db_dir)?; + ensure_backup_dir(&config_dir)?; + ensure_backup_dir(&maintenance_dir)?; progress.reset(crate::middleware::backup_phase::SNAPSHOT_DB); log_backup_phase(crate::middleware::backup_phase::SNAPSHOT_DB); @@ -321,6 +309,7 @@ pub(crate) fn create_pre_maintenance_backup_to_server( .replace('\'', "''"); conn.execute_batch(&format!("VACUUM INTO '{db_snapshot_str}'")) .map_err(|error| AppError::Internal(anyhow::anyhow!("VACUUM INTO: {error}")))?; + restrict_backup_file(&db_snapshot_path)?; let db_snapshot_size = std::fs::metadata(&db_snapshot_path) .map(|metadata| metadata.len()) .map_err(|error| AppError::Internal(anyhow::anyhow!("Stat DB snapshot: {error}")))?; @@ -376,12 +365,7 @@ pub(crate) fn create_pre_maintenance_backup_to_server( ); let integrity_path = maintenance_dir.join("pre-integrity-check.txt"); - std::fs::write(&integrity_path, pre_integrity.as_bytes()).map_err(|error| { - AppError::Internal(anyhow::anyhow!( - "Write {}: {error}", - integrity_path.display() - )) - })?; + write_backup_bytes(&integrity_path, pre_integrity.as_bytes())?; push_v4_file_entry( &mut files, "maintenance/pre-integrity-check.txt".to_owned(), @@ -393,12 +377,7 @@ pub(crate) fn create_pre_maintenance_backup_to_server( ); let foreign_key_path = maintenance_dir.join("pre-foreign-key-check.txt"); - std::fs::write(&foreign_key_path, pre_foreign_key.as_bytes()).map_err(|error| { - AppError::Internal(anyhow::anyhow!( - "Write {}: {error}", - foreign_key_path.display() - )) - })?; + write_backup_bytes(&foreign_key_path, pre_foreign_key.as_bytes())?; push_v4_file_entry( &mut files, "maintenance/pre-foreign-key-check.txt".to_owned(), @@ -431,9 +410,7 @@ pub(crate) fn create_pre_maintenance_backup_to_server( sql }; let schema_path = maintenance_dir.join("pre-schema.sql"); - std::fs::write(&schema_path, schema_dump.as_bytes()).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Write {}: {error}", schema_path.display())) - })?; + write_backup_bytes(&schema_path, schema_dump.as_bytes())?; push_v4_file_entry( &mut files, "maintenance/pre-schema.sql".to_owned(), @@ -562,12 +539,35 @@ fn count_required_private_files(dir: &Path, missing_message: &str) -> Result Result<()> { + crate::config::ensure_private_dir(path) + .map_err(|error| AppError::Internal(anyhow::anyhow!("Create {}: {error}", path.display()))) +} + +fn restrict_backup_file(path: &Path) -> Result<()> { + crate::config::restrict_private_file_permissions(path).map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Set private permissions on {}: {error}", + path.display() + )) + }) +} + +fn write_backup_bytes(path: &Path, bytes: &[u8]) -> Result<()> { + crate::config::write_private_file(path, bytes) + .map_err(|error| AppError::Internal(anyhow::anyhow!("Write {}: {error}", path.display()))) +} + fn write_jsonl_file(path: &Path, rows: &[T]) -> Result<(u64, String)> { use sha2::Digest as _; + if let Some(parent) = path.parent() { + ensure_backup_dir(parent)?; + } let mut file = std::fs::File::create(path).map_err(|error| { AppError::Internal(anyhow::anyhow!("Create {}: {error}", path.display())) })?; + restrict_backup_file(path)?; let mut hasher = sha2::Sha256::new(); let mut written = 0u64; for row in rows { @@ -587,9 +587,7 @@ fn write_pretty_json_file(path: &Path, value: &T) -> Result let bytes = serde_json::to_vec_pretty(value).map_err(|error| { AppError::Internal(anyhow::anyhow!("Serialize {}: {error}", path.display())) })?; - std::fs::write(path, &bytes).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Write {}: {error}", path.display())) - })?; + write_backup_bytes(path, &bytes)?; Ok(( u64::try_from(bytes.len()).unwrap_or(u64::MAX), saved_backup::sha256_hex_for_bytes(&bytes), @@ -609,13 +607,12 @@ fn relative_path_string(path: &Path, root: &Path) -> Result { fn copy_regular_file_to_backup(source: &Path, destination: &Path) -> Result<(u64, String)> { if let Some(parent) = destination.parent() { - std::fs::create_dir_all(parent).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create {}: {error}", parent.display())) - })?; + ensure_backup_dir(parent)?; } let mut output = std::fs::File::create(destination).map_err(|error| { AppError::Internal(anyhow::anyhow!("Create {}: {error}", destination.display())) })?; + restrict_backup_file(destination)?; saved_backup::copy_file_and_hash(source, &mut output) } @@ -807,9 +804,7 @@ pub async fn create_board_backup( super::prune_stale_temp_board_downloads(); super::temp_board_download_dir() }; - std::fs::create_dir_all(&backup_dir).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create board backup dir: {error}")) - })?; + ensure_backup_dir(&backup_dir)?; let ts = super::local_backup_timestamp_label(); let filename = super::unique_backup_filename( &backup_dir, @@ -856,6 +851,7 @@ pub async fn create_board_backup( let _ = std::fs::remove_file(&tmp_path); AppError::Internal(anyhow::anyhow!("Rename board backup: {error}")) })?; + restrict_backup_file(&final_path)?; let size = std::fs::metadata(&final_path) .map(|metadata| metadata.len()) @@ -1282,10 +1278,13 @@ pub(super) fn write_board_backup_archive( where F: FnMut(&mut zip::ZipWriter>) -> Result<()>, { - let out_file = std::io::BufWriter::new( - std::fs::File::create(output_path) - .map_err(|error| AppError::Internal(anyhow::anyhow!("Create zip tmp: {error}")))?, - ); + if let Some(parent) = output_path.parent() { + ensure_backup_dir(parent)?; + } + let file = std::fs::File::create(output_path) + .map_err(|error| AppError::Internal(anyhow::anyhow!("Create zip tmp: {error}")))?; + restrict_backup_file(output_path)?; + let out_file = std::io::BufWriter::new(file); let mut zip = zip::ZipWriter::new(out_file); let opts = zip::write::SimpleFileOptions::default() .compression_method(zip::CompressionMethod::Deflated); @@ -1447,9 +1446,7 @@ fn materialize_split_zip_parts( target_part_size: u64, ) -> Result<()> { let parts_dir = root_dir.join(saved_backup::PARTS_DIR_NAME); - std::fs::create_dir_all(&parts_dir).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create {}: {error}", parts_dir.display())) - })?; + ensure_backup_dir(&parts_dir)?; let planned_parts = plan_split_zip_parts(&manifest.files, target_part_size); let total_parts = u32::try_from(planned_parts.len()).unwrap_or(u32::MAX); let mut part_infos = Vec::with_capacity(planned_parts.len()); @@ -1460,15 +1457,14 @@ fn materialize_split_zip_parts( let part_path = root_dir.join(&part_filename); let tmp_path = root_dir.join(format!("{part_filename}.tmp")); if let Some(parent) = tmp_path.parent() { - std::fs::create_dir_all(parent).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create {}: {error}", parent.display())) - })?; + ensure_backup_dir(parent)?; } let write_result = (|| -> Result<()> { let output = std::fs::File::create(&tmp_path).map_err(|error| { AppError::Internal(anyhow::anyhow!("Create {}: {error}", tmp_path.display())) })?; + restrict_backup_file(&tmp_path)?; let mut zip = zip::ZipWriter::new(std::io::BufWriter::new(output)); for file_index in &planned.files { let entry = manifest.files.get(*file_index).ok_or_else(|| { @@ -1498,9 +1494,15 @@ fn materialize_split_zip_parts( let writer = zip.finish().map_err(|error| { AppError::Internal(anyhow::anyhow!("Finalize {}: {error}", tmp_path.display())) })?; - writer.into_inner().map_err(|error| { - AppError::Internal(anyhow::anyhow!("Flush {}: {error}", tmp_path.display())) - })?; + writer + .into_inner() + .map_err(|error| { + AppError::Internal(anyhow::anyhow!("Flush {}: {error}", tmp_path.display())) + })? + .sync_all() + .map_err(|error| { + AppError::Internal(anyhow::anyhow!("Sync {}: {error}", tmp_path.display())) + })?; Ok(()) })(); if let Err(error) = write_result { @@ -1514,6 +1516,7 @@ fn materialize_split_zip_parts( part_path.display() )) })?; + restrict_backup_file(&part_path)?; let part_size = std::fs::metadata(&part_path) .map(|metadata| metadata.len()) @@ -1674,9 +1677,7 @@ fn write_board_exports_to_v4_dir( let board_root = destination_root .join("boards") .join(&manifest.board.short_name); - std::fs::create_dir_all(&board_root).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Create {}: {error}", board_root.display())) - })?; + ensure_backup_dir(&board_root)?; let board_json_path = board_root.join("board.json"); let (board_json_size, board_json_sha) = write_pretty_json_file(&board_json_path, manifest)?; @@ -1778,7 +1779,10 @@ fn finalize_v4_backup_root( #[cfg(test)] mod tests { - use super::{build_full_backup_manifest, count_required_private_files, FullBackupCreateForm}; + use super::{ + build_full_backup_manifest, copy_regular_file_to_backup, count_required_private_files, + FullBackupCreateForm, + }; use crate::handlers::admin::backup::common::{ resolve_tor_hidden_service_keys_availability, verify_full_backup_zip, TorHiddenServiceKeysAvailability, FULL_BACKUP_MANIFEST_NAME, @@ -1853,6 +1857,36 @@ mod tests { assert_eq!(&body[..], b"false"); } + #[cfg(unix)] + #[test] + fn backup_secret_file_copy_uses_private_mode() { + use std::os::unix::fs::PermissionsExt as _; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let source = temp_dir.path().join("settings.toml"); + let destination = temp_dir.path().join("backup/config/settings.toml"); + std::fs::write(&source, "cookie_secret = \"secret\"").expect("source"); + + copy_regular_file_to_backup(&source, &destination).expect("copy backup file"); + + assert_eq!( + std::fs::metadata(&destination) + .expect("destination metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + assert_eq!( + std::fs::metadata(destination.parent().expect("destination parent")) + .expect("parent metadata") + .permissions() + .mode() + & 0o777, + 0o700 + ); + } + fn write_test_full_backup_zip(zip_path: &std::path::Path, include_tor_keys: bool) { let temp_dir = tempfile::tempdir().expect("tempdir"); let uploads_dir = temp_dir.path().join("uploads"); diff --git a/src/handlers/admin/backup/downloads.rs b/src/handlers/admin/backup/downloads.rs index 8b17108..d85245b 100644 --- a/src/handlers/admin/backup/downloads.rs +++ b/src/handlers/admin/backup/downloads.rs @@ -8,12 +8,12 @@ pub(super) fn temp_board_download_token_path(filename: &str) -> PathBuf { } pub fn write_temp_board_download_token(filename: &str, token: &str) -> Result<()> { - std::fs::create_dir_all(temp_board_download_dir()).map_err(|error| { + crate::config::ensure_private_dir(&temp_board_download_dir()).map_err(|error| { AppError::Internal(anyhow::anyhow!("Create temp board backup dir: {error}")) })?; - std::fs::write(temp_board_download_token_path(filename), token).map_err(|error| { - AppError::Internal(anyhow::anyhow!("Write temp board download token: {error}")) - })?; + crate::config::write_private_file(&temp_board_download_token_path(filename), token).map_err( + |error| AppError::Internal(anyhow::anyhow!("Write temp board download token: {error}")), + )?; Ok(()) } diff --git a/src/handlers/admin/backup/restore_full.rs b/src/handlers/admin/backup/restore_full.rs index 8db09af..4b327d9 100644 --- a/src/handlers/admin/backup/restore_full.rs +++ b/src/handlers/admin/backup/restore_full.rs @@ -163,12 +163,211 @@ fn validate_full_restore_db_trust_boundary(conn: &rusqlite::Connection) -> Resul Ok(()) } +fn recompute_restored_post_body_html(conn: &rusqlite::Connection) -> Result<()> { + let posts = { + let mut stmt = conn + .prepare("SELECT id, body FROM posts") + .map_err(|error| { + AppError::BadRequest(format!("Restored database is invalid: {error}")) + })?; + let rows = stmt + .query_map([], |row| { + Ok((row.get::<_, i64>(0)?, row.get::<_, String>(1)?)) + }) + .map_err(|error| { + AppError::BadRequest(format!("Restored database is invalid: {error}")) + })?; + rows.collect::>>() + .map_err(|error| { + AppError::BadRequest(format!( + "Restored database has an invalid post body row: {error}" + )) + })? + }; + + let mut update = conn + .prepare("UPDATE posts SET body_html = ?1 WHERE id = ?2") + .map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Prepare restored body_html update: {error}" + )) + })?; + for (post_id, body) in posts { + let body_html = render_restored_body_html(&body); + update + .execute(params![body_html, post_id]) + .map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Update restored body_html for post {post_id}: {error}" + )) + })?; + } + Ok(()) +} + fn scrub_full_restore_runtime_state(conn: &rusqlite::Connection) -> Result<()> { conn.execute("DELETE FROM admin_sessions", []) .map_err(|error| AppError::Internal(anyhow::anyhow!("Clear restored sessions: {error}")))?; Ok(()) } +struct FullRestoreTempPaths { + temp_db: PathBuf, + db_snapshot: PathBuf, +} + +#[cfg(test)] +thread_local! { + static FULL_RESTORE_RUNTIME_TMP_ROOT_FOR_TEST: std::cell::RefCell> = + const { std::cell::RefCell::new(None) }; +} + +#[cfg(test)] +fn full_restore_runtime_tmp_dir() -> PathBuf { + FULL_RESTORE_RUNTIME_TMP_ROOT_FOR_TEST.with(|root| { + if let Some(path) = root.borrow().as_ref() { + path.clone() + } else { + crate::config::runtime_tmp_dir() + } + }) +} + +#[cfg(not(test))] +fn full_restore_runtime_tmp_dir() -> PathBuf { + crate::config::runtime_tmp_dir() +} + +struct FullRestoreTempCleanupGuard { + db_paths: [PathBuf; 2], +} + +impl FullRestoreTempCleanupGuard { + fn new(paths: &FullRestoreTempPaths) -> Self { + Self { + db_paths: [paths.temp_db.clone(), paths.db_snapshot.clone()], + } + } +} + +impl Drop for FullRestoreTempCleanupGuard { + fn drop(&mut self) { + for path in &self.db_paths { + remove_sqlite_db_and_sidecars(path); + } + } +} + +fn sqlite_db_and_sidecar_paths(path: &Path) -> Vec { + let mut paths = vec![path.to_path_buf()]; + let Some(file_name) = path.file_name() else { + return paths; + }; + + for suffix in ["-wal", "-shm", "-journal"] { + let mut sidecar_name = file_name.to_os_string(); + sidecar_name.push(suffix); + paths.push(path.with_file_name(sidecar_name)); + } + paths +} + +fn remove_sqlite_db_and_sidecars(path: &Path) { + for candidate in sqlite_db_and_sidecar_paths(path) { + if let Err(error) = std::fs::remove_file(&candidate) { + if error.kind() != std::io::ErrorKind::NotFound { + warn!( + path = %candidate.display(), + %error, + "Failed to remove full restore temporary SQLite file" + ); + } + } + } +} + +fn prepare_full_restore_temp_paths(tmp_id: &str) -> Result { + prepare_full_restore_temp_paths_in(&full_restore_runtime_tmp_dir(), tmp_id) +} + +fn prepare_full_restore_temp_paths_in( + runtime_tmp_root: &Path, + tmp_id: &str, +) -> Result { + let temp_dir = runtime_tmp_root.join("full-restore"); + crate::config::ensure_private_dir(&temp_dir).map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Create private full restore temp dir {}: {error}", + temp_dir.display() + )) + })?; + Ok(FullRestoreTempPaths { + temp_db: temp_dir.join(format!("chan_restore_{tmp_id}.db")), + db_snapshot: temp_dir.join(format!("chan_restore_live_before_{tmp_id}.db")), + }) +} + +fn restrict_restore_temp_file(path: &Path) -> Result<()> { + crate::config::restrict_private_file_permissions(path).map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Set private permissions on {}: {error}", + path.display() + )) + }) +} + +fn create_private_restore_temp_db(path: &Path) -> Result { + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + + let file = std::fs::OpenOptions::new() + .create_new(true) + .write(true) + .mode(0o600) + .open(path) + .map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Create private temp DB {}: {error}", + path.display() + )) + })?; + restrict_restore_temp_file(path)?; + Ok(file) + } + + #[cfg(not(unix))] + { + let file = std::fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(path) + .map_err(|error| { + AppError::Internal(anyhow::anyhow!( + "Create private temp DB {}: {error}", + path.display() + )) + })?; + restrict_restore_temp_file(path)?; + Ok(file) + } +} + +fn create_live_restore_snapshot( + live_conn: &rusqlite::Connection, + db_snapshot: &Path, +) -> Result<()> { + let db_snapshot_str = db_snapshot + .to_str() + .ok_or_else(|| AppError::Internal(anyhow::anyhow!("Snapshot path is non-UTF-8")))? + .replace('\'', "''"); + live_conn + .execute_batch(&format!("VACUUM INTO '{db_snapshot_str}'")) + .map_err(|error| AppError::Internal(anyhow::anyhow!("Snapshot live DB: {error}")))?; + restrict_restore_temp_file(db_snapshot)?; + Ok(()) +} + // The signature mirrors the data passed between layers, so a wrapper would add more noise than clarity. #[expect(clippy::too_many_arguments, clippy::too_many_lines)] pub(super) fn execute_full_restore( @@ -200,9 +399,13 @@ pub(super) fn execute_full_restore( super::common::TorHiddenServiceKeysAvailability::Available(dir) => Some(dir), }; - let temp_dir = std::env::temp_dir(); let tmp_id = uuid::Uuid::new_v4().simple().to_string(); - let temp_db = temp_dir.join(format!("chan_restore_{tmp_id}.db")); + let temp_paths = prepare_full_restore_temp_paths(&tmp_id)?; + let _temp_file_cleanup = FullRestoreTempCleanupGuard::new(&temp_paths); + let FullRestoreTempPaths { + temp_db, + db_snapshot, + } = temp_paths; let upload_root = PathBuf::from(upload_dir); let staged_upload_root = create_staging_dir(&upload_root, "restore-stage")?; let live_global_favicon_dir = crate::favicon::global_backup_source_dir(); @@ -272,7 +475,6 @@ pub(super) fn execute_full_restore( }, ) }); - let db_snapshot = temp_dir.join(format!("chan_restore_live_before_{tmp_id}.db")); let mut db_extracted = false; for index in 0..archive.len() { @@ -283,8 +485,7 @@ pub(super) fn execute_full_restore( validate_restore_safe_entry_name(&name)?; if name == "chan.db" { - let mut out = std::fs::File::create(&temp_db) - .map_err(|error| AppError::Internal(anyhow::anyhow!("Create temp DB: {error}")))?; + let mut out = create_private_restore_temp_db(&temp_db)?; copy_limited_with_total_budget( &mut entry, &mut out, @@ -294,6 +495,7 @@ pub(super) fn execute_full_restore( "Full restore archive", ) .map_err(|error| AppError::Internal(anyhow::anyhow!("Write temp DB: {error}")))?; + restrict_restore_temp_file(&temp_db)?; let mut header = [0u8; 16]; { @@ -450,13 +652,7 @@ pub(super) fn execute_full_restore( ))); } - let db_snapshot_str = db_snapshot - .to_str() - .ok_or_else(|| AppError::Internal(anyhow::anyhow!("Snapshot path is non-UTF-8")))? - .replace('\'', "''"); - live_conn - .execute_batch(&format!("VACUUM INTO '{db_snapshot_str}'")) - .map_err(|error| AppError::Internal(anyhow::anyhow!("Snapshot live DB: {error}")))?; + create_live_restore_snapshot(live_conn, &db_snapshot)?; if banner_extracted { canonicalize_restored_banner_dir(&staged_global_banner_dir)?; @@ -549,6 +745,7 @@ pub(super) fn execute_full_restore( )) })?; validate_full_restore_db_trust_boundary(&src)?; + recompute_restored_post_body_html(&src)?; scrub_full_restore_runtime_state(&src)?; db::rebuild_pending_fs_ops_for_restore(&src)?; db::insert_pending_fs_op(&src, &pending_restore_op)?; @@ -939,6 +1136,25 @@ mod tests { } } + struct FullRestoreRuntimeTmpRootOverride { + previous: Option, + } + + impl FullRestoreRuntimeTmpRootOverride { + fn new(runtime_tmp_root: &std::path::Path) -> Self { + let previous = super::FULL_RESTORE_RUNTIME_TMP_ROOT_FOR_TEST + .with(|root| root.replace(Some(runtime_tmp_root.to_path_buf()))); + Self { previous } + } + } + + impl Drop for FullRestoreRuntimeTmpRootOverride { + fn drop(&mut self) { + super::FULL_RESTORE_RUNTIME_TMP_ROOT_FOR_TEST + .with(|root| root.replace(self.previous.take())); + } + } + fn create_snapshot_db() -> std::path::PathBuf { let temp_dir = tempfile::tempdir().expect("tempdir"); let db_path = temp_dir.path().join("snapshot.db"); @@ -951,6 +1167,126 @@ mod tests { temp_dir.keep().join("snapshot.db") } + fn write_sqlite_sidecars(path: &std::path::Path) { + for sidecar in super::sqlite_db_and_sidecar_paths(path).into_iter().skip(1) { + std::fs::write(sidecar, b"sqlite sidecar").expect("write SQLite sidecar"); + } + } + + fn assert_sqlite_db_and_sidecars_absent(path: &std::path::Path) { + for candidate in super::sqlite_db_and_sidecar_paths(path) { + let display = candidate.display(); + assert!( + !candidate.exists(), + "temporary SQLite file should be removed: {display}" + ); + } + } + + fn assert_full_restore_temp_dir_empty(runtime_tmp_root: &std::path::Path) { + let full_restore_dir = runtime_tmp_root.join("full-restore"); + if !full_restore_dir.exists() { + return; + } + + let mut leftovers = Vec::new(); + for entry in std::fs::read_dir(&full_restore_dir).expect("read full restore temp dir") { + leftovers.push(entry.expect("full restore temp dir entry").path()); + } + let display = leftovers + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join(", "); + assert!( + leftovers.is_empty(), + "full restore temp dir should be empty, found: {display}" + ); + } + + #[cfg(unix)] + fn unix_mode(path: &std::path::Path) -> u32 { + use std::os::unix::fs::PermissionsExt as _; + + std::fs::metadata(path) + .expect("metadata") + .permissions() + .mode() + & 0o777 + } + + #[cfg(unix)] + #[test] + fn full_restore_temp_db_files_are_private() { + let runtime_tmp_root = tempfile::tempdir().expect("tempdir"); + let tmp_id = format!("test_{}", uuid::Uuid::new_v4().simple()); + let paths = super::prepare_full_restore_temp_paths_in(runtime_tmp_root.path(), &tmp_id) + .expect("prepare temp paths"); + let temp_parent = paths.temp_db.parent().expect("temp db parent"); + + assert_eq!(unix_mode(temp_parent), 0o700); + + { + let mut file = + super::create_private_restore_temp_db(&paths.temp_db).expect("create temp db"); + file.write_all(b"uploaded database bytes") + .expect("write temp db"); + } + super::restrict_restore_temp_file(&paths.temp_db).expect("restrict copied temp db"); + assert_eq!(unix_mode(&paths.temp_db), 0o600); + + let pool = crate::db::init_test_pool().expect("test pool"); + let conn = pool.get().expect("db conn"); + super::create_live_restore_snapshot(&conn, &paths.db_snapshot) + .expect("create live snapshot"); + assert_eq!(unix_mode(&paths.db_snapshot), 0o600); + } + + #[test] + fn full_restore_failure_after_extracted_db_creation_removes_temp_db_and_sidecars() { + let runtime_tmp_root = tempfile::tempdir().expect("tempdir"); + let tmp_id = format!("test_{}", uuid::Uuid::new_v4().simple()); + let paths = super::prepare_full_restore_temp_paths_in(runtime_tmp_root.path(), &tmp_id) + .expect("prepare temp paths"); + + { + let _cleanup = super::FullRestoreTempCleanupGuard::new(&paths); + let mut file = + super::create_private_restore_temp_db(&paths.temp_db).expect("create temp db"); + file.write_all(b"uploaded database bytes") + .expect("write temp db"); + write_sqlite_sidecars(&paths.temp_db); + } + + assert_sqlite_db_and_sidecars_absent(&paths.temp_db); + } + + #[test] + fn full_restore_failure_after_snapshot_creation_removes_snapshot_and_sidecars() { + let runtime_tmp_root = tempfile::tempdir().expect("tempdir"); + let tmp_id = format!("test_{}", uuid::Uuid::new_v4().simple()); + let paths = super::prepare_full_restore_temp_paths_in(runtime_tmp_root.path(), &tmp_id) + .expect("prepare temp paths"); + + { + let _cleanup = super::FullRestoreTempCleanupGuard::new(&paths); + let mut file = + super::create_private_restore_temp_db(&paths.temp_db).expect("create temp db"); + file.write_all(b"uploaded database bytes") + .expect("write temp db"); + write_sqlite_sidecars(&paths.temp_db); + + let pool = crate::db::init_test_pool().expect("test pool"); + let conn = pool.get().expect("db conn"); + super::create_live_restore_snapshot(&conn, &paths.db_snapshot) + .expect("create live snapshot"); + write_sqlite_sidecars(&paths.db_snapshot); + } + + assert_sqlite_db_and_sidecars_absent(&paths.temp_db); + assert_sqlite_db_and_sidecars_absent(&paths.db_snapshot); + } + fn write_full_backup_zip( zip_path: &std::path::Path, backup_tor_keys: Option<&[(&str, &str)]>, @@ -1056,6 +1392,38 @@ mod tests { (fresh_sid, session_ids) } + #[test] + fn full_restore_success_removes_restore_temp_db_files_after_completion() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let runtime_tmp_root = temp_dir.path().join("runtime-tmp"); + let _runtime_tmp_root_override = FullRestoreRuntimeTmpRootOverride::new(&runtime_tmp_root); + let zip_path = temp_dir.path().join("backup.zip"); + write_full_backup_zip(&zip_path, None, false); + + let pool = crate::db::init_test_pool().expect("test pool"); + let mut live_conn = pool.get().expect("db conn"); + let file = std::fs::File::open(&zip_path).expect("open zip"); + let mut archive = zip::ZipArchive::new(file).expect("zip archive"); + let upload_dir = temp_dir.path().join("uploads"); + std::fs::create_dir_all(&upload_dir).expect("create uploads"); + + execute_full_restore( + &mut live_conn, + 1, + upload_dir.to_str().expect("upload dir"), + None, + false, + &mut archive, + "Test restore", + "Test restore completed", + "Test restore", + "Test restore", + ) + .expect("restore should succeed"); + + assert_full_restore_temp_dir_empty(&runtime_tmp_root); + } + fn read_tree(root: &std::path::Path) -> BTreeMap { fn visit( root: &std::path::Path, @@ -1235,6 +1603,87 @@ mod tests { assert_eq!(session_ids, vec![fresh_sid]); } + #[test] + fn full_restore_recomputes_untrusted_body_html() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let db_path = create_snapshot_db(); + let post_id = { + let conn = rusqlite::Connection::open(&db_path).expect("open snapshot"); + let board_id = conn + .query_row( + "SELECT id FROM boards WHERE short_name = 'tech'", + [], + |row| row.get::<_, i64>(0), + ) + .expect("board id"); + let post = crate::db::NewPost { + thread_id: 0, + board_id, + name: "anon".to_owned(), + tripcode: None, + subject: Some("subject".to_owned()), + body: "plain restored body".to_owned(), + body_html: "".to_owned(), + ip_hash: None, + file_path: None, + file_name: None, + file_size: None, + thumb_path: None, + mime_type: None, + media_type: None, + audio_file_path: None, + audio_file_name: None, + audio_file_size: None, + audio_mime_type: None, + deletion_token: "token".to_owned(), + is_op: true, + }; + crate::db::create_thread_with_optional_poll( + &conn, + board_id, + Some("subject"), + &post, + "", + None, + None, + ) + .expect("create restored thread") + .1 + }; + let zip_path = temp_dir.path().join("backup.zip"); + write_full_backup_zip_from_db(&zip_path, &db_path, None, false); + + let pool = crate::db::init_test_pool().expect("test pool"); + let mut live_conn = pool.get().expect("db conn"); + let file = std::fs::File::open(&zip_path).expect("open zip"); + let mut archive = zip::ZipArchive::new(file).expect("zip archive"); + let upload_dir = temp_dir.path().join("uploads"); + std::fs::create_dir_all(&upload_dir).expect("create uploads"); + execute_full_restore( + &mut live_conn, + 1, + upload_dir.to_str().expect("upload dir"), + None, + false, + &mut archive, + "Test restore", + "Test restore completed", + "Test restore", + "Test restore", + ) + .expect("restore should succeed"); + + let body_html: String = live_conn + .query_row( + "SELECT body_html FROM posts WHERE id = ?1", + rusqlite::params![post_id], + |row| row.get(0), + ) + .expect("restored body_html"); + assert!(body_html.contains("plain restored body")); + assert!(!body_html.contains(" String pub(crate) fn create_backup_root(backup_id: &str) -> Result { let root = backups_root_dir().join(backup_id); - std::fs::create_dir_all(&root).map_err(|error| { + crate::config::ensure_private_dir(&root).map_err(|error| { AppError::Internal(anyhow::anyhow!( "Create backup directory {}: {error}", root.display() @@ -349,14 +349,14 @@ pub(crate) fn write_json_pretty(path: &Path, value: &T) -> Result< let bytes = serde_json::to_vec_pretty(value).map_err(|error| { AppError::Internal(anyhow::anyhow!("Serialize {}: {error}", path.display())) })?; - std::fs::write(path, bytes).map_err(|error| { + crate::config::write_private_file(path, &bytes).map_err(|error| { AppError::Internal(anyhow::anyhow!("Write {}: {error}", path.display())) })?; Ok(()) } pub(crate) fn write_text(path: &Path, text: &str) -> Result<()> { - std::fs::write(path, text.as_bytes()).map_err(|error| { + crate::config::write_private_file(path, text.as_bytes()).map_err(|error| { AppError::Internal(anyhow::anyhow!("Write {}: {error}", path.display())) })?; Ok(()) diff --git a/src/handlers/board/tests.rs b/src/handlers/board/tests.rs index dabb683..159e890 100644 --- a/src/handlers/board/tests.rs +++ b/src/handlers/board/tests.rs @@ -2326,6 +2326,47 @@ async fn password_protected_board_does_not_leak_homepage_new_activity_badge() { assert!(!body.contains("board-card-activity-badge")); } +#[tokio::test] +async fn thread_updates_rejects_thread_id_from_other_board() { + let state = crate::test_support::app_state(); + let (_public_board_id, _public_thread_id) = seed_board_with_thread(&state, "pub", "public op"); + let (secret_board_id, secret_thread_id) = + seed_board_with_thread(&state, "secret", "protected op"); + { + let conn = state.db.get().expect("db connection"); + let password_hash = + crate::utils::crypto::hash_password("swordfish").expect("hash password"); + conn.execute( + "UPDATE boards SET access_mode = ?1, access_password_hash = ?2 WHERE id = ?3", + rusqlite::params!["view_password", password_hash, secret_board_id], + ) + .expect("protect secret board"); + } + create_reply_on_thread( + &state, + secret_board_id, + secret_thread_id, + "protected reply should not leak", + ); + let router = activity_router(state); + + let response = router + .oneshot( + Request::builder() + .method("GET") + .uri(format!("/pub/thread/{secret_thread_id}/updates?since=0")) + .body(Body::empty()) + .expect("request"), + ) + .await + .expect("response"); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + let body = response_body_string(response).await; + assert!(!body.contains("protected op")); + assert!(!body.contains("protected reply should not leak")); +} + #[tokio::test] async fn new_activity_pages_keep_private_no_store_cache_headers() { let state = crate::test_support::app_state(); diff --git a/src/handlers/thread.rs b/src/handlers/thread.rs index e7764fc..c881e5c 100644 --- a/src/handlers/thread.rs +++ b/src/handlers/thread.rs @@ -1341,6 +1341,11 @@ pub async fn thread_updates( .ok_or_else(|| crate::error::AppError::NotFound("Board not found.".into()))?; let thread = db::get_thread(&conn, thread_id)? .ok_or_else(|| crate::error::AppError::NotFound("Thread not found.".into()))?; + if thread.board_id != board.id { + return Err(crate::error::AppError::NotFound( + "Thread not found in this board.".into(), + )); + } let thread_badges_enabled = db::get_thread_new_reply_badges_enabled(&conn); let homepage_thread_badges_enabled = db::get_homepage_new_thread_badges_enabled(&conn); let homepage_reply_badges_enabled = db::get_homepage_new_reply_badges_enabled(&conn); @@ -1357,7 +1362,7 @@ pub async fn thread_updates( // Fetch posts newer than `since`, ordered oldest-first so they // render in the correct chronological order when appended. - let posts = db::get_new_posts_since(&conn, thread_id, since, 100)?; + let posts = db::get_new_posts_since(&conn, board.id, thread_id, since, 100)?; let last_id = posts.iter().map(|p| p.id).max().unwrap_or(since); let count = posts.len(); @@ -1388,7 +1393,7 @@ pub async fn thread_updates( } let refreshed_posts = - db::get_posts_by_ids_in_thread(&conn, thread_id, &refresh_post_ids)? + db::get_posts_by_ids_in_thread(&conn, board.id, thread_id, &refresh_post_ids)? .into_iter() .map(|post| RefreshedPostPayload { id: post.id, diff --git a/src/middleware/ip.rs b/src/middleware/ip.rs index fd24601..a06e234 100644 --- a/src/middleware/ip.rs +++ b/src/middleware/ip.rs @@ -47,11 +47,13 @@ pub(crate) fn forwarded_proto_is_https( }) } -fn forwarded_ip_from_headers( +fn forwarded_ip_from_headers_with( headers: &axum::http::HeaderMap, peer: Option, + behind_proxy: bool, + trusted_proxy_cidrs: &[String], ) -> Option { - if !CONFIG.behind_proxy || !trusted_proxy_peer(peer) { + if !behind_proxy || !trusted_proxy_peer_with(peer, trusted_proxy_cidrs) { return None; } @@ -71,27 +73,39 @@ fn forwarded_ip_from_headers( .map(str::to_owned) } +fn resolved_client_ip( + headers: &axum::http::HeaderMap, + peer: Option, + behind_proxy: bool, + trusted_proxy_cidrs: &[String], + enable_tor_support: bool, +) -> String { + if let Some(token) = crate::detect::tor_stream_token_identity(peer, enable_tor_support) { + return token; + } + + if let Some(ip) = + forwarded_ip_from_headers_with(headers, peer, behind_proxy, trusted_proxy_cidrs) + { + return ip; + } + + peer.map_or_else(|| "unknown".to_owned(), |addr| addr.ip().to_string()) +} + pub fn extract_ip(req: &Request) -> String { let peer = req .extensions() .get::>() .map(|connect_info| connect_info.0); - if let Some(ip) = forwarded_ip_from_headers(req.headers(), peer) { - return ip; - } - - if CONFIG.enable_tor_support { - if let Some(addr) = peer { - if addr.ip().is_loopback() { - if let Some(token) = crate::detect::TOR_STREAM_TOKENS.get(&addr.port()) { - return token.value().to_string(); - } - } - } - } - - peer.map_or_else(|| "unknown".to_owned(), |addr| addr.ip().to_string()) + resolved_client_ip( + req.headers(), + peer, + CONFIG.behind_proxy, + &CONFIG.trusted_proxy_cidrs, + CONFIG.enable_tor_support, + ) } pub struct ClientIp(pub String); @@ -111,31 +125,22 @@ where .get::>() .map(|connect_info| connect_info.0); - if let Some(ip) = forwarded_ip_from_headers(&parts.headers, peer) { - return Ok(Self(ip)); - } - - if CONFIG.enable_tor_support { - if let Some(addr) = peer { - if addr.ip().is_loopback() { - if let Some(token) = crate::detect::TOR_STREAM_TOKENS.get(&addr.port()) { - return Ok(Self(token.value().to_string())); - } - } - } - } - - Ok(Self(peer.map_or_else( - || "unknown".to_owned(), - |addr| addr.ip().to_string(), + Ok(Self(resolved_client_ip( + &parts.headers, + peer, + CONFIG.behind_proxy, + &CONFIG.trusted_proxy_cidrs, + CONFIG.enable_tor_support, ))) } } #[cfg(test)] mod tests { - use super::{forwarded_client_ip, trusted_proxy_peer_with}; + use super::{forwarded_client_ip, resolved_client_ip, trusted_proxy_peer_with}; + use axum::http::{HeaderMap, HeaderValue}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; + use std::sync::Arc; #[test] fn forwarded_ip_prefers_leftmost_hop() { @@ -201,4 +206,36 @@ mod tests { &trusted )); } + + #[test] + fn tor_stream_token_precedes_spoofed_forwarded_headers_for_loopback_peer() { + let peer = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 49_152); + crate::detect::TOR_STREAM_TOKENS.insert(peer.port(), Arc::from("tor:test-stream")); + + let mut headers = HeaderMap::new(); + headers.insert("x-real-ip", HeaderValue::from_static("198.51.100.10")); + headers.insert( + "x-forwarded-for", + HeaderValue::from_static("203.0.113.7, 127.0.0.1"), + ); + let trusted = vec!["127.0.0.1/32".to_owned(), "::1/128".to_owned()]; + + let resolved = resolved_client_ip(&headers, Some(peer), true, &trusted, true); + + crate::detect::TOR_STREAM_TOKENS.remove(&peer.port()); + assert_eq!(resolved, "tor:test-stream"); + } + + #[test] + fn forwarded_headers_still_apply_for_non_tor_trusted_proxy_peer() { + let peer = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 49_153); + let mut headers = HeaderMap::new(); + headers.insert("x-real-ip", HeaderValue::from_static("198.51.100.10")); + let trusted = vec!["127.0.0.1/32".to_owned(), "::1/128".to_owned()]; + + assert_eq!( + resolved_client_ip(&headers, Some(peer), true, &trusted, true), + "198.51.100.10" + ); + } } diff --git a/src/server/server.rs b/src/server/server.rs index 79a19cc..5f2d514 100644 --- a/src/server/server.rs +++ b/src/server/server.rs @@ -123,6 +123,9 @@ pub async fn run_server(port_override: Option, chan_net: bool) -> anyhow::R // Validate critical configuration values immediately — fail fast with a // clear error rather than discovering misconfiguration at runtime (#8). CONFIG.validate()?; + if chan_net { + CONFIG.validate_chan_net_listener()?; + } let data_dir = super::parent_dir_or_current(std::path::Path::new(&CONFIG.database_path)); From f6712080e03de7c295d248728711ec646496ea71 Mon Sep 17 00:00:00 2001 From: csd113 Date: Fri, 26 Jun 2026 22:37:02 -0700 Subject: [PATCH 3/4] fix(security): address medium-risk hardening findings Store generic uploads as .bin and recheck locked or archived thread state inside reply transactions. Require both an admin session and a one-time token for temp-board downloads, including replay and token-without-session rejection coverage. Escape ban appeal IP prefixes in moderation templates. Validation passed: git diff --check && cargo fmt --all --check && cargo clippy --workspace --all-targets --all-features -- -D warnings -D clippy::all -D clippy::pedantic -D clippy::nursery -D clippy::cargo && cargo test --workspace --all-features. --- src/db/threads.rs | 132 ++++++++++++++++++++++++- src/handlers/admin/backup.rs | 74 ++++++++++++++ src/handlers/admin/backup/downloads.rs | 59 +++++------ src/handlers/posting.rs | 10 ++ src/templates/admin/moderation.rs | 47 ++++++++- src/utils/files/storage.rs | 45 +++++---- 6 files changed, 308 insertions(+), 59 deletions(-) diff --git a/src/db/threads.rs b/src/db/threads.rs index d950886..827d8f1 100644 --- a/src/db/threads.rs +++ b/src/db/threads.rs @@ -285,19 +285,43 @@ pub fn create_reply_with_thread_update( .context("Failed to begin create_reply_with_thread_update transaction")?; let result: Result = (|| { + let flags = conn + .query_row( + "SELECT locked, archived + FROM threads + WHERE id = ?1 AND board_id = ?2", + params![post.thread_id, post.board_id], + |row| Ok((row.get::<_, i32>(0)? != 0, row.get::<_, i32>(1)? != 0)), + ) + .optional()?; + let Some((locked, archived)) = flags else { + anyhow::bail!( + "Thread id {} not found while creating reply", + post.thread_id + ); + }; + if locked { + anyhow::bail!("This thread is locked."); + } + if archived { + anyhow::bail!("This thread is archived."); + } + let post_id = super::posts::create_post_inner(conn, post)?; let updated = if should_bump { conn.execute( "UPDATE threads SET bumped_at = unixepoch(), reply_count = reply_count + 1 - WHERE id = ?1", - params![post.thread_id], + WHERE id = ?1 AND board_id = ?2 AND locked = 0 AND archived = 0", + params![post.thread_id, post.board_id], )? } else { conn.execute( - "UPDATE threads SET reply_count = reply_count + 1 WHERE id = ?1", - params![post.thread_id], + "UPDATE threads + SET reply_count = reply_count + 1 + WHERE id = ?1 AND board_id = ?2 AND locked = 0 AND archived = 0", + params![post.thread_id, post.board_id], )? }; if updated == 0 { @@ -819,6 +843,62 @@ mod tests { thread_id } + fn plain_reply(board_id: i64, thread_id: i64) -> NewPost { + NewPost { + thread_id, + board_id, + name: "anon".to_owned(), + tripcode: None, + subject: None, + body: "reply".to_owned(), + body_html: "reply".to_owned(), + ip_hash: None, + file_path: None, + file_name: None, + file_size: None, + thumb_path: None, + mime_type: None, + media_type: None, + audio_file_path: None, + audio_file_name: None, + audio_file_size: None, + audio_mime_type: None, + deletion_token: "token".to_owned(), + is_op: false, + } + } + + fn pending_upload_op(id: &str) -> crate::pending_fs::PendingFsOpInsert { + crate::pending_fs::PendingFsOpInsert { + id: id.to_owned(), + kind: crate::pending_fs::UPLOAD_FINALIZE_KIND, + payload_json: "{}".to_owned(), + } + } + + fn thread_reply_count(conn: &Connection, thread_id: i64) -> i64 { + conn.query_row( + "SELECT reply_count FROM threads WHERE id = ?1", + params![thread_id], + |row| row.get(0), + ) + .expect("thread reply count") + } + + fn post_count(conn: &Connection, thread_id: i64) -> i64 { + conn.query_row( + "SELECT COUNT(*) FROM posts WHERE thread_id = ?1", + params![thread_id], + |row| row.get(0), + ) + .expect("post count") + } + + fn pending_fs_op_count(conn: &Connection) -> i64 { + conn.query_row("SELECT COUNT(*) FROM pending_fs_ops", [], |row| row.get(0)) + .expect("pending fs op count") + } + #[test] fn prune_old_threads_commits_even_when_no_files_are_safe() { let conn = test_conn(); @@ -929,6 +1009,50 @@ mod tests { assert!(!thumb_dir.join("reply.webp").exists()); } + #[test] + fn create_reply_rejects_locked_thread_without_mutations() { + let conn = test_conn(); + let board_id = create_board(&conn, "lock", "Lock", "", false).expect("create board"); + let thread_id = create_plain_thread(&conn, board_id, "locked thread"); + conn.execute( + "UPDATE threads SET locked = 1 WHERE id = ?1", + params![thread_id], + ) + .expect("lock thread"); + let reply = plain_reply(board_id, thread_id); + let pending_op = pending_upload_op("locked-reply-upload"); + + let error = create_reply_with_thread_update(&conn, &reply, "", true, Some(&pending_op)) + .expect_err("locked thread should reject reply"); + + assert!(error.to_string().contains("This thread is locked.")); + assert_eq!(post_count(&conn, thread_id), 1); + assert_eq!(thread_reply_count(&conn, thread_id), 0); + assert_eq!(pending_fs_op_count(&conn), 0); + } + + #[test] + fn create_reply_rejects_archived_thread_without_mutations() { + let conn = test_conn(); + let board_id = create_board(&conn, "arch", "Archive", "", false).expect("create board"); + let thread_id = create_plain_thread(&conn, board_id, "archived thread"); + conn.execute( + "UPDATE threads SET archived = 1 WHERE id = ?1", + params![thread_id], + ) + .expect("archive thread"); + let reply = plain_reply(board_id, thread_id); + let pending_op = pending_upload_op("archived-reply-upload"); + + let error = create_reply_with_thread_update(&conn, &reply, "", true, Some(&pending_op)) + .expect_err("archived thread should reject reply"); + + assert!(error.to_string().contains("This thread is archived.")); + assert_eq!(post_count(&conn, thread_id), 1); + assert_eq!(thread_reply_count(&conn, thread_id), 0); + assert_eq!(pending_fs_op_count(&conn), 0); + } + #[test] fn delete_thread_returns_not_found_on_retry() { let conn = test_conn(); diff --git a/src/handlers/admin/backup.rs b/src/handlers/admin/backup.rs index a98f953..7473dd4 100644 --- a/src/handlers/admin/backup.rs +++ b/src/handlers/admin/backup.rs @@ -1246,12 +1246,17 @@ mod tests { ); let download_response = app + .clone() .oneshot( Request::builder() .method("GET") .uri(format!( "/admin/backup/download/temp-board/{download_filename}?cleanup=1&token={token}" )) + .header( + header::COOKIE, + "csrf_token=csrf123; chan_admin_session=session123", + ) .body(Body::empty()) .expect("request"), ) @@ -1279,6 +1284,75 @@ mod tests { !temp_board_download_token_path(download_filename).exists(), "temp-board download token should be consumed" ); + + let replay_response = app + .oneshot( + Request::builder() + .method("GET") + .uri(format!( + "/admin/backup/download/temp-board/{download_filename}?cleanup=1&token={token}" + )) + .header( + header::COOKIE, + "csrf_token=csrf123; chan_admin_session=session123", + ) + .body(Body::empty()) + .expect("request"), + ) + .await + .expect("replay response"); + + assert_eq!(replay_response.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn temp_board_download_rejects_token_without_admin_session() { + let state = crate::test_support::app_state(); + let filename = unique_zip_name("temp-token-only"); + let token = "token-only-test"; + let download_path = temp_board_download_dir().join(&filename); + let token_path = temp_board_download_token_path(&filename); + let _download_cleanup = PathCleanup(download_path.clone()); + let _token_cleanup = PathCleanup(token_path.clone()); + crate::config::write_private_file(&download_path, b"temporary board backup") + .expect("write temp download"); + write_temp_board_download_token(&filename, token).expect("write token"); + let kind_segment: String = ['{', 'k', 'i', 'n', 'd', '}'].into_iter().collect(); + let filename_segment: String = ['{', 'f', 'i', 'l', 'e', 'n', 'a', 'm', 'e', '}'] + .into_iter() + .collect(); + let app = Router::new() + .route( + &[ + "/admin/backup/download/", + &kind_segment, + "/", + &filename_segment, + ] + .concat(), + get(super::download_backup), + ) + .with_state(state); + + let response = app + .oneshot( + Request::builder() + .method("GET") + .uri(format!( + "/admin/backup/download/temp-board/{filename}?cleanup=1&token={token}" + )) + .body(Body::empty()) + .expect("request"), + ) + .await + .expect("download response"); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert!(download_path.exists()); + assert!( + token_path.exists(), + "unauthenticated token attempts must not consume the one-time token" + ); } #[test] diff --git a/src/handlers/admin/backup/downloads.rs b/src/handlers/admin/backup/downloads.rs index d85245b..9feccd5 100644 --- a/src/handlers/admin/backup/downloads.rs +++ b/src/handlers/admin/backup/downloads.rs @@ -147,41 +147,34 @@ pub async fn download_backup( sanitize_backup_zip_filename(&filename)? }; - match kind.as_str() { - "temp-board" => { - prune_stale_temp_board_downloads(); - if let Some(token) = query.token.as_deref() { - if !consume_temp_board_download_token(&safe_filename, token)? { - return Err(AppError::Forbidden( - "Invalid or expired download token.".into(), - )); - } - } else { - tokio::task::spawn_blocking({ - let pool = state.db.clone(); - move || -> Result<()> { - let conn = pool.get()?; - super::require_admin_session_sid(&conn, session_id.as_deref())?; - Ok(()) - } - }) - .await - .map_err(|e| AppError::Internal(anyhow::anyhow!(e)))??; - } + let requires_temp_token = match kind.as_str() { + "temp-board" => true, + "full" | "board" => false, + _ => return Err(AppError::BadRequest("Unknown backup kind.".into())), + }; + + tokio::task::spawn_blocking({ + let pool = state.db.clone(); + move || -> Result<()> { + let conn = pool.get()?; + super::require_admin_session_sid(&conn, session_id.as_deref())?; + Ok(()) } - "full" | "board" => { - tokio::task::spawn_blocking({ - let pool = state.db.clone(); - move || -> Result<()> { - let conn = pool.get()?; - super::require_admin_session_sid(&conn, session_id.as_deref())?; - Ok(()) - } - }) - .await - .map_err(|e| AppError::Internal(anyhow::anyhow!(e)))??; + }) + .await + .map_err(|e| AppError::Internal(anyhow::anyhow!(e)))??; + + if requires_temp_token { + prune_stale_temp_board_downloads(); + let token = query + .token + .as_deref() + .ok_or_else(|| AppError::Forbidden("Invalid or expired download token.".into()))?; + if !consume_temp_board_download_token(&safe_filename, token)? { + return Err(AppError::Forbidden( + "Invalid or expired download token.".into(), + )); } - _ => return Err(AppError::BadRequest("Unknown backup kind.".into())), } if let Some(part_name) = query.part.as_deref() { diff --git a/src/handlers/posting.rs b/src/handlers/posting.rs index e420baf..6089b6c 100644 --- a/src/handlers/posting.rs +++ b/src/handlers/posting.rs @@ -439,6 +439,9 @@ pub fn submit_post( if thread.locked { return Err(AppError::Forbidden("This thread is locked.".into())); } + if thread.archived { + return Err(AppError::Forbidden("This thread is archived.".into())); + } Some((*thread_id, *sage, thread.reply_count)) } @@ -641,7 +644,14 @@ pub fn submit_post( ) { Ok(post_id) => post_id, Err(error) => { + let stale_state_error = error.to_string(); uploads.rollback_new_files(conn, &upload_dir)?; + if stale_state_error.contains("This thread is locked.") { + return Err(AppError::Forbidden("This thread is locked.".into())); + } + if stale_state_error.contains("This thread is archived.") { + return Err(AppError::Forbidden("This thread is archived.".into())); + } return Err(error.into()); } }; diff --git a/src/templates/admin/moderation.rs b/src/templates/admin/moderation.rs index 1f30eef..9f049f1 100644 --- a/src/templates/admin/moderation.rs +++ b/src/templates/admin/moderation.rs @@ -23,7 +23,7 @@ pub(super) fn render(view: &AdminPanelViewModel<'_>) -> String { render_admin_moderation_section( view.csrf_token, &render_report_rows(view), - &render_appeal_rows(view), + &render_appeal_rows(view.csrf_token, view.moderation.appeals), &render_ban_rows(view), &render_filter_rows(view), &report_badge, @@ -139,17 +139,18 @@ fn render_report_rows(view: &AdminPanelViewModel<'_>) -> String { report_rows } -fn render_appeal_rows(view: &AdminPanelViewModel<'_>) -> String { +fn render_appeal_rows(csrf_token: &str, appeals: &[crate::models::BanAppeal]) -> String { let mut appeal_rows = String::new(); - if view.moderation.appeals.is_empty() { + if appeals.is_empty() { appeal_rows.push_str( r#"no open appeals"#, ); } - for a in view.moderation.appeals { + for a in appeals { let reason = escape_html(a.reason.trim()); let age = fmt_ts(a.created_at); let ip_short = a.ip_hash.get(..16).unwrap_or(&a.ip_hash); + let ip_short = escape_html(ip_short); let _ = write!( appeal_rows, r#" @@ -174,7 +175,7 @@ fn render_appeal_rows(view: &AdminPanelViewModel<'_>) -> String { ip_short = ip_short, reason = reason, age = escape_html(&age), - csrf = escape_html(view.csrf_token), + csrf = escape_html(csrf_token), aid = a.id, ip_hash = escape_html(&a.ip_hash) ); @@ -322,3 +323,39 @@ fn render_admin_moderation_section( moderation_summary_counter = escape_html(moderation_summary_counter), ) } + +#[cfg(test)] +mod tests { + use super::render_appeal_rows; + use crate::models::BanAppeal; + + #[test] + fn appeal_ip_prefix_is_escaped_for_html_like_and_multibyte_input() { + let script_tag = ["<", "script", ">"].concat(); + let script_close = [""].concat(); + let multibyte_attack = format!("éééé{script_tag}alert(1){script_close}"); + let appeals = [ + BanAppeal { + id: 1, + ip_hash: "".to_owned(), + reason: "html-like".to_owned(), + status: "open".to_owned(), + created_at: 1, + }, + BanAppeal { + id: 2, + ip_hash: multibyte_attack, + reason: "multibyte".to_owned(), + status: "open".to_owned(), + created_at: 2, + }, + ]; + + let rows = render_appeal_rows("csrf", &appeals); + + assert!(rows.contains("<img src=x oner")); + assert!(!rows.contains(" Result { - let ext = arbitrary_file_ext(options.original_filename); - let filename = format!("{file_id}.{ext}"); + let filename = format!("{file_id}.bin"); let file_path_abs = plan.dest_dir.join(&filename); crate::utils::fs_security::canonical_parent_for_new_child( Path::new(options.boards_dir), @@ -550,20 +549,6 @@ const fn media_label(media_type: crate::models::MediaType) -> &'static str { } } -fn arbitrary_file_ext(original_filename: &str) -> String { - std::path::Path::new(original_filename) - .extension() - .and_then(|ext| ext.to_str()) - .map(|ext| { - ext.chars() - .filter(char::is_ascii_alphanumeric) - .take(16) - .collect::() - }) - .filter(|ext| !ext.is_empty()) - .map_or_else(|| "bin".to_owned(), |ext| ext.to_ascii_lowercase()) -} - fn validate_decodable_image(input_path: &Path, mime_type: &str) -> Result<()> { let Some(format) = mime_to_image_format(mime_type) else { return Ok(()); @@ -1597,13 +1582,39 @@ trailer << /Root 1 0 R >> assert_eq!(uploaded.media_type, crate::models::MediaType::Other); assert!(std::path::Path::new(&uploaded.file_path) .extension() - .is_some_and(|ext| ext.eq_ignore_ascii_case("txt"))); + .is_some_and(|ext| ext.eq_ignore_ascii_case("bin"))); let stored = std::fs::read(tempdir.path().join(&uploaded.file_path)).expect("read stored upload"); assert_eq!(stored, contents); } + #[test] + fn arbitrary_upload_uses_bin_extension_even_with_inline_media_names() { + let tempdir = tempfile::tempdir().expect("tempdir"); + for original_name in ["renamed.pdf", "renamed.png"] { + let input = tempfile::Builder::new() + .tempfile_in(tempdir.path()) + .expect("temp file"); + let contents = b"arbitrary non-media bytes"; + std::fs::write(input.path(), contents).expect("write arbitrary upload"); + + let uploaded = save_upload_from_path( + input.path(), + contents, + contents.len(), + &arbitrary_upload_options(tempdir.path(), original_name), + ) + .expect("save arbitrary upload"); + + assert_eq!(uploaded.mime_type, "application/octet-stream"); + assert_eq!(uploaded.media_type, crate::models::MediaType::Other); + assert!(std::path::Path::new(&uploaded.file_path) + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("bin"))); + } + } + #[test] fn video_upload_saves_with_svg_placeholder_when_ffmpeg_is_missing() { let tempdir = tempfile::tempdir().expect("tempdir"); From 5736ed161a51a5ed430e955e21471887a773e634 Mon Sep 17 00:00:00 2001 From: csd113 Date: Sat, 27 Jun 2026 13:12:05 -0700 Subject: [PATCH 4/4] Fix thread ETags for poll state changes --- src/handlers/render.rs | 111 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-) diff --git a/src/handlers/render.rs b/src/handlers/render.rs index f19cc1d..005df3f 100644 --- a/src/handlers/render.rs +++ b/src/handlers/render.rs @@ -44,6 +44,9 @@ pub fn thread_page_etag_signature(data: &ThreadPageData) -> String { for post in &data.posts { update_post_signature(&mut hasher, post); } + if let Some(poll) = &data.poll { + update_poll_signature(&mut hasher, poll); + } hex::encode(hasher.finalize()) } @@ -75,6 +78,27 @@ fn update_post_signature(hasher: &mut Sha256, post: &crate::models::Post) { update_sig_field(hasher, post.media_processing_error.as_deref().unwrap_or("")); } +fn update_poll_signature(hasher: &mut Sha256, poll_data: &PollData) { + update_sig_field(hasher, &poll_data.poll.id.to_string()); + update_sig_field(hasher, &poll_data.poll.thread_id.to_string()); + update_sig_field(hasher, &poll_data.poll.question); + update_sig_field(hasher, &poll_data.poll.expires_at.to_string()); + update_sig_field(hasher, &poll_data.poll.created_at.to_string()); + update_sig_field(hasher, &poll_data.total_votes.to_string()); + update_sig_field( + hasher, + &poll_data.user_voted_option.unwrap_or_default().to_string(), + ); + update_sig_field(hasher, if poll_data.is_expired { "1" } else { "0" }); + for option in &poll_data.options { + update_sig_field(hasher, &option.id.to_string()); + update_sig_field(hasher, &option.poll_id.to_string()); + update_sig_field(hasher, &option.text); + update_sig_field(hasher, &option.position.to_string()); + update_sig_field(hasher, &option.vote_count.to_string()); + } +} + pub fn load_board_page_data( conn: &rusqlite::Connection, board_short: &str, @@ -216,7 +240,9 @@ mod tests { use super::{ board_page_etag_signature, thread_page_etag_signature, BoardPageData, ThreadPageData, }; - use crate::models::{Board, Pagination, Post, Thread, ThreadSummary}; + use crate::models::{ + Board, Pagination, Poll, PollData, PollOption, Post, Thread, ThreadSummary, + }; fn sample_board() -> Board { Board { @@ -284,6 +310,37 @@ mod tests { } } + fn sample_poll_data(user_voted_option: Option, vote_counts: [i64; 2]) -> PollData { + PollData { + poll: Poll { + id: 7, + thread_id: 42, + question: "poll?".into(), + expires_at: 2000, + created_at: 100, + }, + options: vec![ + PollOption { + id: 11, + poll_id: 7, + text: "yes".into(), + position: 0, + vote_count: vote_counts[0], + }, + PollOption { + id: 12, + poll_id: 7, + text: "no".into(), + position: 1, + vote_count: vote_counts[1], + }, + ], + total_votes: vote_counts[0] + vote_counts[1], + user_voted_option, + is_expired: false, + } + } + #[test] fn thread_page_etag_changes_when_reply_is_removed() { let board = sample_board(); @@ -347,6 +404,58 @@ mod tests { ); } + #[test] + fn thread_page_etag_changes_when_poll_vote_count_changes() { + let board = sample_board(); + let before = ThreadPageData { + board: board.clone(), + thread: sample_thread(0), + posts: vec![sample_post(1)], + poll: Some(sample_poll_data(None, [0, 0])), + is_admin: false, + owned_post_controls: std::collections::BTreeMap::new(), + }; + let after = ThreadPageData { + board, + thread: sample_thread(0), + posts: vec![sample_post(1)], + poll: Some(sample_poll_data(Some(11), [1, 0])), + is_admin: false, + owned_post_controls: std::collections::BTreeMap::new(), + }; + + assert_ne!( + thread_page_etag_signature(&before), + thread_page_etag_signature(&after) + ); + } + + #[test] + fn thread_page_etag_changes_when_poll_viewer_vote_state_changes() { + let board = sample_board(); + let before = ThreadPageData { + board: board.clone(), + thread: sample_thread(0), + posts: vec![sample_post(1)], + poll: Some(sample_poll_data(None, [1, 0])), + is_admin: false, + owned_post_controls: std::collections::BTreeMap::new(), + }; + let after = ThreadPageData { + board, + thread: sample_thread(0), + posts: vec![sample_post(1)], + poll: Some(sample_poll_data(Some(11), [1, 0])), + is_admin: false, + owned_post_controls: std::collections::BTreeMap::new(), + }; + + assert_ne!( + thread_page_etag_signature(&before), + thread_page_etag_signature(&after) + ); + } + #[test] fn board_page_etag_changes_when_reply_preview_changes() { let board = sample_board();