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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 55 additions & 88 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,15 @@
//
// SECURITY: The cookie_secret is auto-generated on first run and persisted
// to settings.toml. It is never left at a well-known default value.
// FIX[CRITICAL-1]: removed hardcoded default secret; see generate_settings_file_if_missing().

use once_cell::sync::Lazy;
use rand_core::{OsRng, RngCore};
use serde::Deserialize;
use std::env;
use std::path::PathBuf;

/// Absolute path to the directory the running binary lives in.
fn binary_dir() -> PathBuf {
// FIX[MEDIUM-2]: log a warning when fallback is used so operators
// are aware that data may land in an unexpected location.
match std::env::current_exe()
.ok()
.and_then(|p| p.parent().map(|p| p.to_path_buf()))
Expand All @@ -38,8 +36,8 @@ fn binary_dir() -> PathBuf {
}

fn settings_file_path() -> PathBuf {
// Store settings.toml in rustrustchan-data/ alongside the database.
// rustrustchan-data/ is created by run_server before CONFIG is first accessed,
// Store settings.toml in rustchan-data/ alongside the database.
// rustchan-data/ is created by run_server before CONFIG is first accessed,
// so this directory always exists by the time settings are read.
let data_dir = binary_dir().join("rustchan-data");
data_dir.join("settings.toml")
Expand All @@ -50,12 +48,10 @@ fn settings_file_path() -> PathBuf {
#[derive(Deserialize, Default)]
struct SettingsFile {
forum_name: Option<String>,
/// Home page subtitle shown below the site name. Displayed on the index
/// page. Can also be changed later via the admin panel.
/// Home page subtitle shown below the site name.
site_subtitle: Option<String>,
/// Default theme served to first-time visitors before they pick one.
/// Valid values: terminal, aero, dorfic, fluorogrid, neoncubicle, chanclassic
/// (empty string or "terminal" = default dark terminal theme).
default_theme: Option<String>,
port: Option<u16>,
max_image_size_mb: Option<u32>,
Expand Down Expand Up @@ -83,18 +79,16 @@ fn load_settings_file() -> SettingsFile {
/// Create settings.toml with defaults if it does not exist yet.
/// Call this once at startup (before CONFIG is accessed for the first time).
///
/// FIX[CRITICAL-1]: A cryptographically random cookie_secret is generated on
/// first run and written to settings.toml. Subsequent runs load it from the
/// file. The server never operates with a known/default secret.
/// A cryptographically random cookie_secret is generated on first run and
/// written to settings.toml. Subsequent runs load it from the file.
/// The server never operates with a known/default secret.
pub fn generate_settings_file_if_missing() {
let path = settings_file_path();
if path.exists() {
return;
}

// Generate a random 64-hex-char secret (32 bytes of entropy).
// This runs before CONFIG is initialised, so we call OsRng directly.
use rand_core::{OsRng, RngCore};
let mut secret_bytes = [0u8; 32];
OsRng.fill_bytes(&mut secret_bytes);
let secret = hex::encode(secret_bytes);
Expand Down Expand Up @@ -170,7 +164,7 @@ pub struct Config {
/// Initial default theme slug (seeds the DB on first run).
/// Valid: terminal, aero, dorfic, fluorogrid, neoncubicle, chanclassic
pub initial_default_theme: String,
#[allow(dead_code)]
#[allow(dead_code)] // read by CLI subcommands and printed at startup
pub port: u16,
pub max_image_size: usize, // bytes
pub max_video_size: usize, // bytes
Expand All @@ -187,11 +181,11 @@ pub struct Config {
pub database_path: String,
pub upload_dir: String,
pub thumb_size: u32,
#[allow(dead_code)]
#[allow(dead_code)] // used as default when creating boards via CLI/admin
pub default_bump_limit: u32,
#[allow(dead_code)]
#[allow(dead_code)] // used as default when creating boards via CLI/admin
pub max_threads_per_board: u32,
/// Maximum GET requests per IP per rate_limit_window (CRIT-3: DoS via catalog/search).
/// Maximum GET requests per IP per rate_limit_window.
pub rate_limit_gets: u32,
pub rate_limit_window: u64,
pub cookie_secret: String,
Expand Down Expand Up @@ -224,20 +218,21 @@ impl Config {
"CHAN_DEFAULT_THEME",
s.default_theme.as_deref().unwrap_or("terminal"),
);
let port = env_u16("CHAN_PORT", s.port.unwrap_or(8080));
let max_image_mb = env_u32("CHAN_MAX_IMAGE_MB", s.max_image_size_mb.unwrap_or(8));
let max_video_mb = env_u32("CHAN_MAX_VIDEO_MB", s.max_video_size_mb.unwrap_or(50));
let max_audio_mb = env_u32("CHAN_MAX_AUDIO_MB", s.max_audio_size_mb.unwrap_or(150));

let host = env_str("CHAN_HOST", "0.0.0.0");
let bind_addr = env_str("CHAN_BIND", &format!("{host}:{port}"));
let port: u16 = env_parse("CHAN_PORT", s.port.unwrap_or(8080));
let max_image_mb: u32 = env_parse("CHAN_MAX_IMAGE_MB", s.max_image_size_mb.unwrap_or(8));
let max_video_mb: u32 = env_parse("CHAN_MAX_VIDEO_MB", s.max_video_size_mb.unwrap_or(50));
let max_audio_mb: u32 = env_parse("CHAN_MAX_AUDIO_MB", s.max_audio_size_mb.unwrap_or(150));

let bind_addr = env_str(
"CHAN_BIND",
&format!("{}:{}", env_str("CHAN_HOST", "0.0.0.0"), port),
);

let behind_proxy = env_bool("CHAN_BEHIND_PROXY", false);

// FIX[CRITICAL-1]: Resolve cookie_secret from env > settings.toml.
// If neither is set, emit a loud warning. The generate_settings_file_if_missing()
// call at startup ensures settings.toml always has a generated secret,
// so this fallback should only be reached in abnormal circumstances.
// Resolve cookie_secret from env > settings.toml.
// generate_settings_file_if_missing() ensures settings.toml always has
// a generated secret, so this fallback should only fire in abnormal cases.
let cookie_secret = if let Ok(v) = env::var("CHAN_COOKIE_SECRET") {
v
} else if let Some(v) = s.cookie_secret {
Expand All @@ -248,10 +243,10 @@ impl Config {
IP hashing is using an empty secret. Run the server once to auto-generate, \
or set CHAN_COOKIE_SECRET."
);
// Emit a random in-memory secret so each restart invalidates hashes
// Random in-memory secret so each restart invalidates hashes
// (better than a known empty string, worse than a persisted one).
let mut b = [0u8; 32];
rand_core::OsRng.fill_bytes(&mut b);
OsRng.fill_bytes(&mut b);
hex::encode(b)
};

Expand All @@ -270,16 +265,16 @@ impl Config {
bind_addr,
database_path: env_str("CHAN_DB", &default_db),
upload_dir: env_str("CHAN_UPLOADS", &default_uploads),
thumb_size: env_u32("CHAN_THUMB_SIZE", 250),
default_bump_limit: env_u32("CHAN_BUMP_LIMIT", 500),
max_threads_per_board: env_u32("CHAN_MAX_THREADS", 150),
rate_limit_gets: env_u32("CHAN_RATE_GETS", 60),
rate_limit_window: env_u64("CHAN_RATE_WINDOW", 60),
thumb_size: env_parse("CHAN_THUMB_SIZE", 250),
default_bump_limit: env_parse("CHAN_BUMP_LIMIT", 500),
max_threads_per_board: env_parse("CHAN_MAX_THREADS", 150),
rate_limit_gets: env_parse("CHAN_RATE_GETS", 60),
rate_limit_window: env_parse("CHAN_RATE_WINDOW", 60),
cookie_secret,
session_duration: env_i64("CHAN_SESSION_SECS", 8 * 3600),
session_duration: env_parse("CHAN_SESSION_SECS", 8 * 3600),
behind_proxy,
https_cookies: env_bool("CHAN_HTTPS_COOKIES", behind_proxy),
wal_checkpoint_interval: env_u64(
wal_checkpoint_interval: env_parse(
"CHAN_WAL_CHECKPOINT_SECS",
s.wal_checkpoint_interval_secs.unwrap_or(3600),
),
Expand Down Expand Up @@ -327,16 +322,13 @@ impl Config {
);
}

// Port 0 is technically valid (OS assigns one) but almost certainly a
// misconfiguration in a server context.
if self.port == 0 {
anyhow::bail!("CONFIG ERROR: port must not be 0.");
}

// Verify the upload directory is writable.
let upload_path = std::path::Path::new(&self.upload_dir);
if upload_path.exists() {
// Try writing a probe file to verify write permission.
let probe = upload_path.join(".write_probe");
if std::fs::write(&probe, b"").is_err() {
anyhow::bail!(
Expand All @@ -351,8 +343,7 @@ impl Config {
}
}

// ─── Import needed for OsRng in cookie_secret fallback ───────────────────────
use rand_core::RngCore as _;
// ─── Cookie secret rotation check ────────────────────────────────────────────

/// Check whether the cookie_secret has changed since the last run by comparing
/// a SHA-256 hash stored in the DB against the currently loaded secret.
Expand All @@ -379,64 +370,40 @@ pub fn check_cookie_secret_rotation(conn: &rusqlite::Connection) {
)
.ok();

match stored {
None => {
// First run — store the hash silently.
let _ = conn.execute(
"INSERT INTO site_settings (key, value) VALUES (?1, ?2)
ON CONFLICT(key) DO UPDATE SET value = excluded.value",
rusqlite::params![KEY, current_hash],
);
}
Some(ref h) if h == &current_hash => {
// Secret unchanged — nothing to do.
}
Some(_) => {
// Secret has rotated.
tracing::warn!(
"SECURITY WARNING: cookie_secret has changed since the last run. \
All IP-based bans are now invalid because all IP hashes have changed. \
If this was unintentional, restore the previous cookie_secret from \
settings.toml. If intentional, consider running: \
DELETE FROM bans; DELETE FROM ban_appeals;"
);
// Update the stored hash so subsequent restarts with the same secret are silent.
let _ = conn.execute(
"INSERT INTO site_settings (key, value) VALUES (?1, ?2)
ON CONFLICT(key) DO UPDATE SET value = excluded.value",
rusqlite::params![KEY, current_hash],
);
if let Some(ref h) = stored {
if h == &current_hash {
return; // Secret unchanged — nothing to do.
}
tracing::warn!(
"SECURITY WARNING: cookie_secret has changed since the last run. \
All IP-based bans are now invalid because all IP hashes have changed. \
If this was unintentional, restore the previous cookie_secret from \
settings.toml. If intentional, consider running: \
DELETE FROM bans; DELETE FROM ban_appeals;"
);
}

// First run (None) or rotated secret (Some) — store the current hash.
let _ = conn.execute(
"INSERT INTO site_settings (key, value) VALUES (?1, ?2)
ON CONFLICT(key) DO UPDATE SET value = excluded.value",
rusqlite::params![KEY, current_hash],
);
}

// ─── Env helpers ──────────────────────────────────────────────────────────────

fn env_str(key: &str, default: &str) -> String {
env::var(key).unwrap_or_else(|_| default.to_string())
}
fn env_u16(key: &str, default: u16) -> u16 {
env::var(key)
.ok()
.and_then(|v| v.parse().ok())
.unwrap_or(default)
}
fn env_u32(key: &str, default: u32) -> u32 {
env::var(key)
.ok()
.and_then(|v| v.parse().ok())
.unwrap_or(default)
}
fn env_u64(key: &str, default: u64) -> u64 {
env::var(key)
.ok()
.and_then(|v| v.parse().ok())
.unwrap_or(default)
}
fn env_i64(key: &str, default: i64) -> i64 {

fn env_parse<T: std::str::FromStr>(key: &str, default: T) -> T {
env::var(key)
.ok()
.and_then(|v| v.parse().ok())
.unwrap_or(default)
}

fn env_bool(key: &str, default: bool) -> bool {
env::var(key)
.map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
Expand Down
15 changes: 14 additions & 1 deletion src/db/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,18 @@ pub fn purge_expired_sessions(conn: &rusqlite::Connection) -> Result<usize> {
pub fn is_banned(conn: &rusqlite::Connection, ip_hash: &str) -> Result<Option<String>> {
let now = chrono::Utc::now().timestamp();
// A ban with NULL expires_at is permanent.
//
// FIX[BAN-ORDER]: Previously there was no ORDER BY, so LIMIT 1 returned
// whichever row the query planner happened to visit first via the index.
// With multiple active bans (e.g. a timed ban and a permanent ban) the
// reason shown to the user was non-deterministic across restarts/VACUUM.
// We now order by expires_at DESC NULLS FIRST so a permanent ban (NULL)
// always surfaces first, and among timed bans the latest-expiring one wins.
let result: Option<Option<String>> = conn
.query_row(
"SELECT reason FROM bans WHERE ip_hash = ?1
AND (expires_at IS NULL OR expires_at > ?2)
ORDER BY expires_at DESC NULLS FIRST
LIMIT 1",
params![ip_hash, now],
|r| r.get(0),
Expand Down Expand Up @@ -393,13 +401,18 @@ pub fn dismiss_ban_appeal(conn: &rusqlite::Connection, appeal_id: i64) -> Result
}

/// Dismiss appeal AND lift the ban for this ip_hash.
///
/// FIX[AUDIT]: Previously both dismiss_ban_appeal and accept_ban_appeal set
/// status='dismissed', making them indistinguishable in the moderation history.
/// Accepted appeals now correctly set status='accepted' so the audit trail
/// accurately reflects whether an appeal was denied or granted.
pub fn accept_ban_appeal(conn: &rusqlite::Connection, appeal_id: i64, ip_hash: &str) -> Result<()> {
// Both updates must succeed together.
let tx = conn
.unchecked_transaction()
.context("Failed to begin accept-appeal transaction")?;
tx.execute(
"UPDATE ban_appeals SET status='dismissed' WHERE id=?1",
"UPDATE ban_appeals SET status='accepted' WHERE id=?1",
params![appeal_id],
)?;
tx.execute("DELETE FROM bans WHERE ip_hash=?1", params![ip_hash])?;
Expand Down
33 changes: 28 additions & 5 deletions src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,21 @@ fn create_schema(conn: &rusqlite::Connection) -> Result<()> {
conn.query_row("SELECT version FROM schema_version", [], |r| r.get(0))?;

// Each entry is (introduced_at_version, sql).
// SQLite returns SQLITE_ERROR (code 1) for "duplicate column" — we allow
// that specific error so re-running against an already-upgraded DB is safe.
// ALTER TABLE … ADD COLUMN returns SQLITE_ERROR (code 1) with the message
// "duplicate column name: X" when the column already exists — this happens
// when the binary is restarted against a DB that was already migrated.
// CREATE INDEX … IF NOT EXISTS is already idempotent and never errors.
//
// FIX[MIGRATION]: The previous guard caught ALL ErrorCode::Unknown errors,
// which maps to the generic SQLITE_ERROR (code 1). That code is also
// returned for SQL syntax errors, wrong number of columns, etc. A typo
// in migration SQL (e.g. "ADD COULMN") would be silently swallowed,
// marked as applied in schema_version, and the column would never exist.
//
// We now additionally inspect the error message string to confirm the
// error is specifically "duplicate column name" before treating it as
// idempotent. Any other SQLITE_ERROR is propagated so the operator sees
// it immediately rather than discovering a missing column at runtime.
let migrations: &[(i64, &str)] = &[
(1, "ALTER TABLE boards ADD COLUMN allow_video INTEGER NOT NULL DEFAULT 1"),
(2, "ALTER TABLE boards ADD COLUMN allow_tripcodes INTEGER NOT NULL DEFAULT 1"),
Expand Down Expand Up @@ -371,10 +384,20 @@ fn create_schema(conn: &rusqlite::Connection) -> Result<()> {
tracing::debug!("Applied migration v{}", version);
highest_applied = version;
}
Err(rusqlite::Error::SqliteFailure(ref e, _))
if e.code == rusqlite::ErrorCode::Unknown =>
Err(rusqlite::Error::SqliteFailure(ref e, ref msg))
if e.code == rusqlite::ErrorCode::Unknown
&& msg
.as_deref()
.map(|m| {
m.contains("duplicate column name") || m.contains("already exists")
})
.unwrap_or(false) =>
{
// SQLITE_ERROR with "duplicate column name" — idempotent, skip.
// Idempotent: column already added or index already exists.
// Only reached for ALTER TABLE … ADD COLUMN (duplicate column)
// and CREATE INDEX (already exists). All other SQLITE_ERROR
// values (syntax errors, wrong column counts, etc.) are NOT
// caught here and will propagate as real failures.
tracing::debug!(
"Migration v{} already applied (idempotent), skipping",
version
Expand Down
Loading