dovenest

Server: close review findings (session invalidation, setup race, share revokes)

A password change now drops that user's sessions, in the same transaction
as the hash write. An admin resetting a compromised account left whoever
held the old cookie signed in.

First-boot setup is atomic: create_admin inserts with a WHERE NOT EXISTS
guard and returns Option<User>, so two concurrent setups cannot both win.
The handler's user_count check stays as an optimization that avoids a
wasted Argon2 hash.

Shares name a path, not a file identity, so a delete, rename or move now
revokes every share on that path and below it. Without this, a new item
landing on the freed name inherited the old link's audience. The revoke
is best-effort: the filesystem change has already succeeded, so a database
error is logged rather than turned into a misleading 500. Changes made
outside the API are still invisible; inode pinning would catch them but
breaks across a restore from backup.

SIGTERM and Ctrl-C now shut down gracefully, so a container restart lets
in-flight uploads and archive downloads finish.

Also:
- P_Q/P_SCOPE/P_ROOT/P_PATH/P_FORMAT were declared shared but unused by
  the server, which matched on serde field names. serde(rename) takes a
  literal, so tests now build the query string from the constants and run
  the real extractor over it.
- DELETE returns a typed DeleteResp instead of an ad-hoc json! value.
- Dropped update_user_password, set_user_admin, set_user_active and
  set_user_roots: no production callers, only their own tests. Those tests
  now go through update_user, the path the admin API uses.
- The ADMIN_USERS doc comment sat on SEARCH.
- download_revalidates_with_last_modified failed about one run in three.
  Its "no validator within two seconds" assertion raced Env setup, which
  runs Argon2. The fixture's mtime is now pinned before the request.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

AuthorKonata <konata@posteo.jp>

Date2026-09-12 07:52:54 UTC

Commit496c1c54b237889eda913863331a54e91d7914f9

Parent4a6f5cc

13 files changed, 469 insertions(+), 107 deletions(-)

▾MCargo.lock

+11@ 496c1c5 @ master

@@ -2368,6 +2368,16 @@ version = "2.0.1"

source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba"

[[package]]

name = "signal-hook-registry"

version = "1.4.8"

source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b"

dependencies = [

"errno",

"libc",

]

[[package]]

name = "simd-adler32"

version = "0.3.10"

@@ -2623,6 +2633,7 @@ dependencies = [

"libc",

"mio",

"pin-project-lite",

"signal-hook-registry",

"socket2",

"tokio-macros",

"windows-sys",

▾Mapi-types/src/lib.rs

+9-1@ 496c1c5 @ master

@@ -20,10 +20,10 @@ pub const FILES: &str = "/api/files";

pub const SHARES: &str = "/api/shares";

/// Public share resolve (no login): `{SHARE}/{token}`.

pub const SHARE: &str = "/api/share";

/// Admin user management: `{ADMIN_USERS}` and `{ADMIN_USERS}/{id}`.

/// `GET /api/search` — name and/or content search, streamed as SSE.

pub const SEARCH: &str = "/api/search";

/// Admin user management: `{ADMIN_USERS}` and `{ADMIN_USERS}/{id}`.

pub const ADMIN_USERS: &str = "/api/admin/users";

pub const ADMIN_SETTINGS: &str = "/api/admin/settings";

/// Pseudo root id every signed-in admin has on the files API: the whole

@@ -347,6 +347,14 @@ pub struct OkResp {

pub ok: bool,

}

/// DELETE `{FILES}/...`: whether the removed item was a folder (the client

/// reports "folder deleted" vs "file deleted").

#[derive(Serialize, Deserialize)]

pub struct DeleteResp {

pub ok: bool,

pub is_dir: bool,

}

/// Upload success: `{"ok": true, "uploaded": n}`.

#[derive(Serialize, Deserialize)]

pub struct UploadResp {

▾Mserver/Cargo.toml

+9-1@ 496c1c5 @ master

@@ -24,7 +24,15 @@ rusqlite = { version = "0.37", features = ["bundled"] }

serde = { version = "1", features = ["derive"] }

serde_json = "1"

thiserror = "2"

tokio = { version = "1", features = ["rt-multi-thread", "macros", "fs", "io-util", "sync", "time"] }

tokio = { version = "1", features = [

"rt-multi-thread",

"macros",

"fs",

"io-util",

"sync",

"time",

"signal",

] }

tar = "0.4"

flate2 = "1"

zstd = "0.13"

▾Mserver/src/api/auth.rs

+16-6@ 496c1c5 @ master

@@ -147,6 +147,14 @@ pub async fn update_profile(

Ok(Json(me_for(&state, &user, roots).await?))

}

fn already_set_up() -> ApiError {

ApiError::localized(

StatusCode::CONFLICT,

"server is already set up",

"err_already_set_up",

)

}

/// POST /api/auth/setup — create the first admin account.

/// Only available while no users exist.

pub async fn setup(

@@ -156,16 +164,18 @@ pub async fn setup(

let name = body.name.trim();

validate_account_name(name)?;

validate_password(&body.password)?;

// A cheap pre-check: it keeps a POST to an already-configured server from

// paying for an Argon2 hash. `create_admin` re-checks atomically.

if state.db.user_count().await? > 0 {

return Err(ApiError::localized(

StatusCode::CONFLICT,

"server is already set up",

"err_already_set_up",

));

return Err(already_set_up());

}

let pass_hash = hash_password(&body.password).await?;

let user = state.db.create_admin(name, &pass_hash).await?;

// `None` = another setup request won the race between the check above and

// this insert.

let Some(user) = state.db.create_admin(name, &pass_hash).await? else {

return Err(already_set_up());

};

let token = auth::random_token();

state.db.create_session(user.id, &token).await?;

▾Mserver/src/api/common.rs

+9@ 496c1c5 @ master

@@ -179,6 +179,15 @@ pub(crate) fn display_name(state: &AppState, rel: &str) -> String {

.unwrap_or_else(|| rel.to_string())

}

/// A resolved absolute path re-expressed relative to the server root — the

/// form `shares.target` is stored in, so share lookups and share revokes both

/// speak the same spelling of a path.

pub(crate) fn target_rel(state: &AppState, abs: &std::path::Path) -> String {

abs.strip_prefix(&state.root)

.map(|p| p.to_string_lossy().into_owned())

.unwrap_or_else(|_| ".".to_string())

}

pub(crate) fn validate_account_name(name: &str) -> Result<(), ApiError> {

let n = name.trim();

if n.is_empty() || n.len() > 64 {

▾Mserver/src/api/files.rs

+74-7@ 496c1c5 @ master

@@ -24,12 +24,14 @@ use tokio::io::AsyncWriteExt;

use tokio::sync::mpsc;

use tokio_stream::wrappers::ReceiverStream;

use crate::api::common::AuthUser;

use crate::api::common::{AuthUser, target_rel};

use crate::archive::{self, ArchiveFormat};

use crate::db::{RootRow, ShareRow};

use crate::error::{ApiError, AppState};

use crate::fs::{self, FsError};

use api_types::{FilesResp, Mutation, OkResp, Op, P_ACTION, P_OVERWRITE, SaveResp, UploadResp};

use api_types::{

DeleteResp, FilesResp, Mutation, OkResp, Op, P_ACTION, P_OVERWRITE, SaveResp, UploadResp,

};

/// Upper bound for the in-memory text endpoint (preview, later editor).

pub(super) const MAX_TEXT_BYTES: u64 = 2 * 1024 * 1024;

@@ -41,6 +43,10 @@ pub(super) const MAX_TEXT_BYTES: u64 = 2 * 1024 * 1024;

/// Query params for `GET /api/files/{root_id}/{*path}`. Without `action` the

/// route lists the directory; `?action=download|preview|content` serve the

/// item itself.

///

/// The field names are the shared [`P_ACTION`] / [`P_FORMAT`] constants.

/// `#[serde(rename)]` only takes a literal, so that link cannot be written

/// here; `tests::query_fields_are_the_shared_constants` pins it instead.

#[derive(Deserialize, Default)]

pub struct FileQuery {

#[serde(default)]

@@ -864,7 +870,7 @@ async fn mutation(

req_rel,

body.overwrite,

);

tokio::task::spawn_blocking(move || {

let vacated = tokio::task::spawn_blocking(move || {

fs::rename_item(&server_root, &root_rel, &rel, &new_name, overwrite)

})

.await

@@ -875,6 +881,7 @@ async fn mutation(

"err_internal",

)

})??;

revoke_shares_at(&state, &vacated).await;

Ok(Json(OkResp { ok: true }))

}

Op::Move | Op::Copy => {

@@ -903,11 +910,14 @@ async fn mutation(

req_rel,

body.overwrite,

);

tokio::task::spawn_blocking(move || {

let vacated = tokio::task::spawn_blocking(move || {

if op_is_move {

fs::move_item(&server_root, &src_rel, &rel, &dst_rel, &dst_path, overwrite)

.map(Some)

} else {

// A copy frees no path, so it revokes nothing.

fs::copy_item(&server_root, &src_rel, &rel, &dst_rel, &dst_path, overwrite)

.map(|()| None)

}

})

.await

@@ -918,6 +928,9 @@ async fn mutation(

"err_internal",

)

})??;

if let Some(vacated) = vacated {

revoke_shares_at(&state, &vacated).await;

}

Ok(Json(OkResp { ok: true }))

}

@@ -931,7 +944,7 @@ pub async fn delete(

State(state): State<Arc<AppState>>,

auth: AuthUser,

path: AxumPath<(i64, String)>,

) -> Result<Json<serde_json::Value>, ApiError> {

) -> Result<Json<DeleteResp>, ApiError> {

let (root_id, req_rel) = path.0;

let root = require_rw_root(&auth.roots, root_id)?;

if req_rel.trim().is_empty() {

@@ -942,7 +955,7 @@ pub async fn delete(

));

}

let (server_root, root_rel, rel) = (state.root.clone(), root.path.clone(), req_rel);

let is_dir =

let (is_dir, gone) =

tokio::task::spawn_blocking(move || fs::remove_item(&server_root, &root_rel, &rel))

.await

.map_err(|_| {

@@ -952,7 +965,8 @@ pub async fn delete(

"err_internal",

)

})??;

Ok(Json(serde_json::json!({ "ok": true, "is_dir": is_dir })))

revoke_shares_at(&state, &gone).await;

Ok(Json(DeleteResp { ok: true, is_dir }))

}

// ---------------------------------------------------------------------------

@@ -1241,6 +1255,33 @@ fn validate_rel_path(name: &str) -> Result<(), ApiError> {

// Helpers

// ---------------------------------------------------------------------------

/// Drop every share that named `abs` or anything under it.

///

/// Called after a delete, a rename, or a move: each one frees a path, and a

/// share stores a path, not a file identity. Without this, a *new* item that

/// later lands on the freed path would inherit the old link's audience.

///

/// Only covers changes made through this API. A file moved out from under the

/// server (over SSH, say) leaves its shares in place, still pointing at a

/// path. Closing that needs inode pinning, which breaks across a restore from

/// backup, so it is deliberately not done.

///

/// Best-effort: the file operation has already succeeded by the time this

/// runs, so a database error must not turn it into a 500. The client would

/// read that as "the delete failed" and retry, and the retry would 404. The

/// failure is logged at `error` instead, and leaves a share pointing at a

/// path that no longer holds what it did.

async fn revoke_shares_at(state: &AppState, abs: &std::path::Path) {

let target = target_rel(state, abs);

match state.db.revoke_shares_at(&target).await {

Ok(0) => {}

Ok(n) => tracing::info!(target = %target, revoked = n, "shares revoked: path is gone"),

Err(e) => {

tracing::error!(error = %e, target = %target, "could not revoke shares on a freed path")

}

fn find_root(roots: &[RootRow], root_id: i64) -> Result<&RootRow, ApiError> {

roots.iter().find(|r| r.id == root_id).ok_or_else(|| {

ApiError::localized(

@@ -1262,3 +1303,29 @@ fn require_rw_root(roots: &[RootRow], root_id: i64) -> Result<&RootRow, ApiError

}

Ok(root)

}

#[cfg(test)]

mod tests {

use super::*;

use api_types::{ACTION_DOWNLOAD, P_FORMAT};

use axum::http::Uri;

/// A rename of `P_ACTION` or `P_FORMAT` without the matching field

/// rename would silently stop the server from reading the parameter the

/// client sends. This builds the query string from the constants and

/// runs the real extractor over it.

#[test]

fn query_fields_are_the_shared_constants() {

let uri: Uri = format!("/f/1/a.txt?{P_ACTION}={ACTION_DOWNLOAD}&{P_FORMAT}=zip")

.parse()

.unwrap();

let q: FileQuery = AxumQuery::try_from_uri(&uri).unwrap().0;

assert_eq!(q.action.as_deref(), Some(ACTION_DOWNLOAD));

assert_eq!(q.format.as_deref(), Some("zip"));

// The same constants drive the hand-rolled readers on the POST path.

assert_eq!(action_param(&uri).as_deref(), Some(ACTION_DOWNLOAD));

let uri: Uri = format!("/f/1/a.txt?{P_OVERWRITE}=true").parse().unwrap();

assert!(parse_overwrite(&uri));

}

▾Mserver/src/api/search.rs

+22@ 496c1c5 @ master

@@ -70,6 +70,10 @@ const SEARCH_MAX_MATCH_EVENTS: usize = 50_000;

/// Matched lines emitted per file (mirrors the client's render cap).

const SEARCH_MAX_LINES_PER_FILE: usize = 500;

/// The field names are the shared [`api_types::P_Q`] / [`api_types::P_SCOPE`]

/// / [`api_types::P_ROOT`] / [`api_types::P_PATH`] constants. `#[serde(rename)]`

/// only takes a literal, so that link cannot be written here;

/// `tests::query_fields_are_the_shared_constants` pins it instead.

#[derive(Debug, Deserialize)]

pub(super) struct SearchQuery {

q: Option<String>,

@@ -441,6 +445,24 @@ fn truncate_line(line: &[u8]) -> String {

mod tests {

use super::*;

/// A rename of one of the `P_*` constants without the matching field

/// rename would silently stop the server from reading the parameter the

/// client sends. This builds the query string from the constants and

/// runs the real extractor over it.

#[test]

fn query_fields_are_the_shared_constants() {

use api_types::{P_PATH, P_Q, P_ROOT, P_SCOPE};

let uri: axum::http::Uri =

format!("/api/search?{P_Q}=invoice&{P_SCOPE}=both&{P_ROOT}=7&{P_PATH}=docs")

.parse()

.unwrap();

let q: SearchQuery = AxumQuery::try_from_uri(&uri).unwrap().0;

assert_eq!(q.q.as_deref(), Some("invoice"));

assert_eq!(q.scope.as_deref(), Some("both"));

assert_eq!(q.root, Some(7));

assert_eq!(q.path.as_deref(), Some("docs"));

}

#[test]

fn name_match_needs_every_word() {

let words = vec!["invoice".to_string(), "2025".to_string()];

▾Mserver/src/api/shares.rs

+2-5@ 496c1c5 @ master

@@ -16,7 +16,7 @@ use axum::Json;

use axum::extract::{Path as AxumPath, State};

use axum::http::StatusCode;

use crate::api::common::{SessionUser, display_name};

use crate::api::common::{SessionUser, display_name, target_rel};

use crate::auth;

use crate::db::ShareRow;

use crate::error::{ApiError, AppState};

@@ -112,10 +112,7 @@ pub async fn create(

)

})??;

let target = abs

.strip_prefix(&state.root)

.map(|p| p.to_string_lossy().into_owned())

.unwrap_or_else(|_| ".".to_string());

let target = target_rel(&state, &abs);

let is_file = abs.is_file();

let token = auth::share_token();

▾Mserver/src/db.rs

+173-65@ 496c1c5 @ master

@@ -175,29 +175,36 @@ impl Db {

}

/// Create the first admin account with the whole root visible (read-write).

/// Only valid while no users exist (enforced by the caller).

pub async fn create_admin(&self, name: &str, pass_hash: &str) -> DbResult<User> {

///

/// `None` means a user already existed. The `WHERE NOT EXISTS` guard runs

/// inside the same transaction as the insert, so two concurrent first-boot

/// setups cannot both win; a caller's earlier `user_count` check is only

/// an optimization, not the guarantee.

pub async fn create_admin(&self, name: &str, pass_hash: &str) -> DbResult<Option<User>> {

let mut c = self.0.lock().await;

let tx = c.transaction()?;

tx.execute(

let inserted = tx.execute(

"INSERT INTO users (name, pass_hash, is_admin, created_at)

VALUES (?1, ?2, 1, ?3)",

SELECT ?1, ?2, 1, ?3 WHERE NOT EXISTS (SELECT 1 FROM users)",

params![name, pass_hash, now()],

)?;

if inserted == 0 {

return Ok(None); // dropping `tx` rolls back

}

let user_id = tx.last_insert_rowid();

tx.execute(

"INSERT INTO user_roots (user_id, path, mode) VALUES (?1, '.', 'rw')",

params![user_id],

)?;

tx.commit()?;

Ok(User {

Ok(Some(User {

id: user_id,

name: name.to_string(),

is_admin: true,

active: true,

single_click: false,

language: None,

})

}))

}

pub async fn verify_password(&self, name: &str, password: &str) -> DbResult<Option<User>> {

@@ -421,33 +428,6 @@ impl Db {

})

}

pub async fn update_user_password(&self, id: i64, pass_hash: &str) -> DbResult<()> {

let c = self.0.lock().await;

c.execute(

"UPDATE users SET pass_hash = ?1 WHERE id = ?2",

params![pass_hash, id],

)?;

Ok(())

}

pub async fn set_user_admin(&self, id: i64, is_admin: bool) -> DbResult<()> {

let c = self.0.lock().await;

c.execute(

"UPDATE users SET is_admin = ?1 WHERE id = ?2",

params![is_admin as i64, id],

)?;

Ok(())

}

pub async fn set_user_active(&self, id: i64, active: bool) -> DbResult<()> {

let c = self.0.lock().await;

c.execute(

"UPDATE users SET active = ?1 WHERE id = ?2",

params![active as i64, id],

)?;

Ok(())

}

pub async fn set_user_single_click(&self, id: i64, single_click: bool) -> DbResult<()> {

let c = self.0.lock().await;

c.execute(

@@ -479,10 +459,7 @@ impl Db {

let mut c = self.0.lock().await;

let tx = c.transaction()?;

if let Some(h) = pass_hash {

tx.execute(

"UPDATE users SET pass_hash = ?1 WHERE id = ?2",

params![h, id],

)?;

set_password(&tx, id, h)?;

}

if let Some(a) = is_admin {

tx.execute(

@@ -514,21 +491,6 @@ impl Db {

Ok(c.execute("DELETE FROM users WHERE id = ?1", [id])? > 0)

}

/// Replace a user's roots with the given (path, mode) pairs.

pub async fn set_user_roots(&self, user_id: i64, roots: &[(String, Mode)]) -> DbResult<()> {

let mut c = self.0.lock().await;

let tx = c.transaction()?;

tx.execute("DELETE FROM user_roots WHERE user_id = ?1", [user_id])?;

for (path, mode) in roots {

tx.execute(

"INSERT INTO user_roots (user_id, path, mode) VALUES (?1, ?2, ?3)",

params![user_id, path, SqlMode(*mode)],

)?;

}

tx.commit()?;

Ok(())

}

// ---------- shares ----------

pub async fn create_share(

@@ -584,6 +546,25 @@ impl Db {

rows.collect()

}

/// Revoke every share on `target` or on anything beneath it. Returns how

/// many were dropped.

///

/// Called when a path stops meaning what it meant: the item was deleted,

/// renamed, or moved away. A share names a path, and a path is not a

/// stable identity, so leaving the row behind would let a *new* item that

/// later takes the freed path inherit the old link's audience.

///

/// `substr` rather than `LIKE`: a target containing `%` or `_` would make

/// a `LIKE` pattern over-match and revoke unrelated shares.

pub async fn revoke_shares_at(&self, target: &str) -> DbResult<usize> {

let c = self.0.lock().await;

c.execute(

"DELETE FROM shares

WHERE target = ?1 OR substr(target, 1, length(?1) + 1) = ?1 || '/'",

[target],

)

}

/// Delete one of `creator_id`'s shares. `false` means no row matched.

pub async fn delete_share(&self, id: i64, creator_id: i64) -> DbResult<bool> {

let c = self.0.lock().await;

@@ -624,6 +605,22 @@ impl Db {

}

/// Write a new password hash and drop every session that was opened with the

/// old one.

///

/// The two belong together: a password is changed because the old one is

/// suspect (an admin resetting a compromised account), and a session that

/// survives the reset leaves whoever holds it signed in. Takes the

/// transaction so the caller can bundle it with its other edits.

fn set_password(tx: &rusqlite::Transaction<'_>, id: i64, pass_hash: &str) -> DbResult<()> {

tx.execute(

"UPDATE users SET pass_hash = ?1 WHERE id = ?2",

params![pass_hash, id],

)?;

tx.execute("DELETE FROM sessions WHERE user_id = ?1", [id])?;

Ok(())

}

/// Column order matched by the four `users` SELECTs above.

fn map_user(r: &rusqlite::Row) -> DbResult<User> {

Ok(User {

@@ -712,10 +709,16 @@ mod tests {

Db::open_in_memory().await.unwrap()

}

/// `update_user` is the only way production edits these fields, so the

/// tests exercise that path rather than per-field helpers.

async fn edit(db: &Db, id: i64, pass: Option<&str>, admin: Option<bool>, active: Option<bool>) {

db.update_user(id, pass, admin, active, None).await.unwrap();

}

async fn db_with_admin() -> (Db, User) {

let db = mem().await;

let hash = crate::auth::hash_password("admin1234").unwrap();

let admin = db.create_admin("admin", &hash).await.unwrap();

let admin = db.create_admin("admin", &hash).await.unwrap().unwrap();

(db, admin)

}

@@ -793,14 +796,14 @@ mod tests {

.is_some()

);

// Disabled users cannot verify.

db.set_user_active(admin.id, false).await.unwrap();

edit(&db, admin.id, None, None, Some(false)).await;

assert!(

db.verify_password("admin", "admin1234")

.await

.unwrap()

.is_none()

);

db.set_user_active(admin.id, true).await.unwrap();

edit(&db, admin.id, None, None, Some(true)).await;

assert!(

db.verify_password("admin", "admin1234")

.await

@@ -809,6 +812,75 @@ mod tests {

);

}

#[tokio::test]

async fn setup_is_won_by_exactly_one_caller() {

let db = mem().await;

let hash = crate::auth::hash_password("admin1234").unwrap();

assert!(db.create_admin("first", &hash).await.unwrap().is_some());

// The guard lives in the insert, so a different name loses too.

assert!(db.create_admin("second", &hash).await.unwrap().is_none());

assert_eq!(db.user_count().await.unwrap(), 1);

// The loser rolled back cleanly: no orphaned root row.

let first = db.find_user_by_name("first").await.unwrap().unwrap();

assert_eq!(db.user_roots(first.id).await.unwrap().len(), 1);

}

#[tokio::test]

async fn changing_a_password_drops_that_users_sessions() {

let (db, admin) = db_with_admin().await;

let h = crate::auth::hash_password("bobpass1").unwrap();

let bob = db.create_user("bob", &h, false, &[]).await.unwrap();

db.create_session(admin.id, "admin-tok").await.unwrap();

db.create_session(bob.id, "bob-tok-1").await.unwrap();

db.create_session(bob.id, "bob-tok-2").await.unwrap();

let new_h = crate::auth::hash_password("bobpass2").unwrap();

edit(&db, bob.id, Some(&new_h), None, None).await;

assert!(

db.session_user_with_roots("bob-tok-1")

.await

.unwrap()

.is_none()

);

assert!(

db.session_user_with_roots("bob-tok-2")

.await

.unwrap()

.is_none()

);

// Only the reset user is signed out.

assert!(

db.session_user_with_roots("admin-tok")

.await

.unwrap()

.is_some()

);

// The admin-edit path bundles the same rule into its transaction.

db.create_session(bob.id, "bob-tok-3").await.unwrap();

let h3 = crate::auth::hash_password("bobpass3").unwrap();

db.update_user(bob.id, Some(&h3), None, None, None)

.await

.unwrap();

assert!(

db.session_user_with_roots("bob-tok-3")

.await

.unwrap()

.is_none()

);

// An edit that leaves the password alone keeps the session.

db.create_session(bob.id, "bob-tok-4").await.unwrap();

db.update_user(bob.id, None, Some(true), None, None)

.await

.unwrap();

assert!(

db.session_user_with_roots("bob-tok-4")

.await

.unwrap()

.is_some()

);

}

#[tokio::test]

async fn sessions_lifecycle() {

let (db, admin) = db_with_admin().await;

@@ -822,9 +894,9 @@ mod tests {

let (u, _) = db.session_user_with_roots("tok1").await.unwrap().unwrap();

assert_eq!(u.id, admin.id);

// Disabling the user invalidates existing sessions.

db.set_user_active(admin.id, false).await.unwrap();

edit(&db, admin.id, None, None, Some(false)).await;

assert!(db.session_user_with_roots("tok1").await.unwrap().is_none());

db.set_user_active(admin.id, true).await.unwrap();

edit(&db, admin.id, None, None, Some(true)).await;

assert!(db.session_user_with_roots("tok1").await.unwrap().is_some());

db.delete_session("tok1").await.unwrap();

assert!(db.session_user_with_roots("tok1").await.unwrap().is_none());

@@ -861,18 +933,26 @@ mod tests {

// Root replacement semantics.

let roots = db.user_roots(bob.id).await.unwrap();

assert_eq!(roots.len(), 1);

db.set_user_roots(bob.id, &[(".".into(), Mode::Ro), ("docs".into(), Mode::Rw)])

.await

.unwrap();

db.update_user(

bob.id,

None,

Some(&[(".".into(), Mode::Ro), ("docs".into(), Mode::Rw)]),

)

.await

.unwrap();

let roots = db.user_roots(bob.id).await.unwrap();

assert_eq!(roots.len(), 2);

assert!(roots.iter().any(|r| r.path == "." && r.mode == Mode::Ro));

db.set_user_roots(bob.id, &[]).await.unwrap();

db.update_user(bob.id, None, None, None, Some(&[]))

.await

.unwrap();

assert!(db.user_roots(bob.id).await.unwrap().is_empty());

// Password update.

let new_h = crate::auth::hash_password("bobpass2").unwrap();

db.update_user_password(bob.id, &new_h).await.unwrap();

edit(&db, bob.id, Some(&new_h), None, None).await;

assert!(

db.verify_password("bob", "bobpass1")

.await

@@ -887,11 +967,11 @@ mod tests {

);

// Admin flag + count (only active admins count).

db.set_user_admin(bob.id, true).await.unwrap();

edit(&db, bob.id, None, Some(true), None).await;

assert_eq!(db.count_admins().await.unwrap(), 2);

db.set_user_active(bob.id, false).await.unwrap();

edit(&db, bob.id, None, None, Some(false)).await;

assert_eq!(db.count_admins().await.unwrap(), 1);

db.set_user_admin(bob.id, false).await.unwrap();

edit(&db, bob.id, None, Some(false), None).await;

// Deletion.

assert!(db.delete_user(bob.id).await.unwrap());

@@ -963,6 +1043,34 @@ mod tests {

assert!(!db.delete_share(s1.id, admin.id).await.unwrap());

}

#[tokio::test]

async fn revoking_a_path_takes_its_descendants_only() {

let (db, admin) = db_with_admin().await;

let mk = async |token: &str, target: &str| {

db.create_share(admin.id, token, target, false, Mode::Ro, None)

.await

.unwrap();

};

mk("t-self", "docs").await;

mk("t-child", "docs/a.txt").await;

mk("t-deep", "docs/inner/b.txt").await;

// A sibling whose name merely starts with "docs" must survive.

mk("t-sibling", "docs2/c.txt").await;

mk("t-other", "src").await;

// SQL wildcards in a path are literal characters, not patterns.

mk("t-wild", "do%s/d.txt").await;

assert_eq!(db.revoke_shares_at("docs").await.unwrap(), 3);

for gone in ["t-self", "t-child", "t-deep"] {

assert!(db.share_by_token(gone).await.unwrap().is_none(), "{gone}");

}

for kept in ["t-sibling", "t-other", "t-wild"] {

assert!(db.share_by_token(kept).await.unwrap().is_some(), "{kept}");

}

// Revoking a path nobody shared is a no-op, not an error.

assert_eq!(db.revoke_shares_at("nothing/here").await.unwrap(), 0);

}

#[tokio::test]

async fn settings_round_trip() {

let (db, _admin) = db_with_admin().await;

▾Mserver/src/fs.rs

+32-16@ 496c1c5 @ master

@@ -424,13 +424,15 @@ fn resolve_path_or_new(

}

/// Rename (or move within the same directory) an item.

/// Returns the path the item was renamed *away from*, so the caller can

/// revoke anything (a share) that still names it.

pub fn rename_item(

server_root: &Path,

root_rel: &str,

req_rel: &str,

new_name: &str,

overwrite: bool,

) -> Result<(), FsError> {

) -> Result<PathBuf, FsError> {

validate_component(new_name)?;

let from = resolve_path(server_root, root_rel, req_rel)?;

let parent = from

@@ -438,20 +440,25 @@ pub fn rename_item(

.ok_or_else(|| FsError::Invalid("invalid path".to_string()))?;

let to = parent.join(new_name);

// Renaming onto itself is a no-op (the overwrite path below would

// delete the file before the rename).

// delete the file before the rename). Nothing was vacated.

if to == from {

return Ok(());

return Ok(to);

}

// `rename` replaces a file target atomically; no remove-then-rename gap.

if to.exists() && (!overwrite || to.is_dir() || from.is_dir()) {

return Err(FsError::Conflict);

}

std::fs::rename(&from, &to).map_err(|e| io_err(e, &to))?;

Ok(())

Ok(from)

}

/// Delete a file or a directory tree. Returns whether it was a directory.

pub fn remove_item(server_root: &Path, root_rel: &str, req_rel: &str) -> Result<bool, FsError> {

/// Delete a file or a directory tree. Returns whether it was a directory, and

/// the path that is now gone (so the caller can revoke shares naming it).

pub fn remove_item(

server_root: &Path,

root_rel: &str,

req_rel: &str,

) -> Result<(bool, PathBuf), FsError> {

let full = resolve_path(server_root, root_rel, req_rel)?;

let is_dir = full.is_dir();

if is_dir {

@@ -459,7 +466,7 @@ pub fn remove_item(server_root: &Path, root_rel: &str, req_rel: &str) -> Result<

} else {

std::fs::remove_file(&full).map_err(|e| io_err(e, &full))?;

}

Ok(is_dir)

Ok((is_dir, full))

}

/// Overwrite an existing file's contents (the editor's save path).

@@ -524,6 +531,8 @@ pub(crate) fn is_within_or_eq(base: &Path, p: &Path) -> bool {

/// Move an item (possibly across roots). `dst_dir_rel` is the destination

/// directory (relative to `dst_root_rel`); the item keeps its base name.

///

/// Returns the path the item was moved *away from*, like [`rename_item`].

pub fn move_item(

server_root: &Path,

src_root_rel: &str,

@@ -531,7 +540,7 @@ pub fn move_item(

dst_root_rel: &str,

dst_dir_rel: &str,

overwrite: bool,

) -> Result<(), FsError> {

) -> Result<PathBuf, FsError> {

let from = resolve_path(server_root, src_root_rel, src_rel)?;

let dst_dir = resolve_dir(server_root, dst_root_rel, dst_dir_rel)?;

let name = from

@@ -540,9 +549,10 @@ pub fn move_item(

.to_owned();

let to = dst_dir.join(&name);

// A no-op (item already at the destination) — treat as success.

// A no-op (item already at the destination) — treat as success. Nothing

// was vacated.

if to == from {

return Ok(());

return Ok(to);

}

// Refuse moving a directory into itself or a descendant.

@@ -554,7 +564,7 @@ pub fn move_item(

check_move_conflict(&to, &from, overwrite)?;

match std::fs::rename(&from, &to) {

Ok(()) => Ok(()),

Ok(()) => Ok(from),

Err(e) if e.kind() == std::io::ErrorKind::CrossesDevices => {

copy_recursive(&from, &to)?;

if from.is_dir() {

@@ -562,7 +572,7 @@ pub fn move_item(

} else {

std::fs::remove_file(&from).map_err(|_| FsError::Forbidden)?;

}

Ok(())

Ok(from)

}

Err(e) => Err(io_err(e, &to)),

}

@@ -960,7 +970,8 @@ mod tests {

fn rename_moves_file_and_dir() {

let t = T::new();

let root = t.root.canonicalize().unwrap();

rename_item(&root, ".", "file.txt", "renamed.txt", false).unwrap();

let vacated = rename_item(&root, ".", "file.txt", "renamed.txt", false).unwrap();

assert_eq!(vacated, root.join("file.txt"));

assert!(!root.join("file.txt").exists());

assert_eq!(

std::fs::read_to_string(root.join("renamed.txt")).unwrap(),

@@ -1021,9 +1032,13 @@ mod tests {

fn remove_file_and_dir() {

let t = T::new();

let root = t.root.canonicalize().unwrap();

assert!(!remove_item(&root, ".", "file.txt").unwrap());

let (is_dir, gone) = remove_item(&root, ".", "file.txt").unwrap();

assert!(!is_dir);

assert_eq!(gone, root.join("file.txt"));

assert!(!root.join("file.txt").exists());

assert!(remove_item(&root, ".", "docs").unwrap());

let (is_dir, gone) = remove_item(&root, ".", "docs").unwrap();

assert!(is_dir);

assert_eq!(gone, root.join("docs"));

assert!(!root.join("docs").exists());

assert!(matches!(

remove_item(&root, ".", "file.txt"),

@@ -1097,7 +1112,8 @@ mod tests {

fn move_file_and_dir_across_dirs() {

let t = T::new();

let root = t.root.canonicalize().unwrap();

move_item(&root, ".", "file.txt", ".", "src", false).unwrap();

let vacated = move_item(&root, ".", "file.txt", ".", "src", false).unwrap();

assert_eq!(vacated, root.join("file.txt"));

assert!(!root.join("file.txt").exists());

assert!(root.join("src/file.txt").exists());

move_item(&root, ".", "src", ".", "docs", false).unwrap();

▾Mserver/src/lib.rs

+31-1@ 496c1c5 @ master

@@ -73,10 +73,40 @@ pub async fn run() -> anyhow::Result<()> {

tracing::info!(root = %cli.root.display(), "filebrowser-ng starting");

tracing::info!(addr = %addr, "listening (pass --bind 0.0.0.0 to expose beyond localhost)");

axum::serve(listener, app).await?;

axum::serve(listener, app)

.with_graceful_shutdown(shutdown_signal())

.await?;

Ok(())

}

/// Resolves on Ctrl-C or SIGTERM (what a container runtime sends on stop).

/// Without this, a restart cuts in-flight uploads and archive downloads

/// mid-stream instead of letting them finish.

async fn shutdown_signal() {

let ctrl_c = async {

let _ = tokio::signal::ctrl_c().await;

};

#[cfg(unix)]

{

use tokio::signal::unix::{SignalKind, signal};

let mut term = match signal(SignalKind::terminate()) {

Ok(s) => s,

// No SIGTERM handler: Ctrl-C alone still stops the server.

Err(e) => {

tracing::warn!(error = %e, "cannot listen for SIGTERM");

return ctrl_c.await;

}

};

tokio::select! {

() = ctrl_c => {}

_ = term.recv() => {}

}

#[cfg(not(unix))]

ctrl_c.await;

tracing::info!("shutting down, waiting for in-flight requests");

}

/// The root folder's own name; "/" has none, so fall back to the full path.

pub fn root_file_name(root: &std::path::Path) -> String {

root.file_name()

▾Mserver/tests/api_files.rs

+11-5@ 496c1c5 @ master

@@ -1020,17 +1020,23 @@ async fn download_revalidates_with_last_modified() {

let env = Env::new().await;

let admin = env.admin().await;

let path = format!("{}?action=download", root_path("editme.txt"));

let file = env.root.path().join("editme.txt");

// A file written in the last two seconds gets no validator.

// A file written in the last two seconds gets no validator. Pin the mtime

// to "now" first: the fixture is written during `Env` setup, which under a

// loaded parallel run can take longer than that two-second window.

std::fs::File::options()

.write(true)

.open(&file)

.unwrap()

.set_modified(std::time::SystemTime::now())

.unwrap();

let r = admin.get(&path).await;

assert_eq!(r.status, StatusCode::OK);

assert!(r.header("last-modified").is_none());

// Backdate the file so the validator appears.

let f = std::fs::File::options()

.write(true)

.open(env.root.path().join("editme.txt"))

.unwrap();

let f = std::fs::File::options().write(true).open(&file).unwrap();

f.set_modified(

std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1_700_000_000),

)

▾Mserver/tests/api_shares.rs

+70@ 496c1c5 @ master

@@ -521,3 +521,73 @@ async fn read_only_root_cannot_be_shared_writably() {

assert_eq!(r.status, StatusCode::OK, "{}", r.text());

assert_eq!(r.json()["writable"], true);

}

/// A share names a path, and a delete, rename or move frees that path. The

/// share must die with it, otherwise a *new* item that later takes the freed

/// name inherits the old link's audience.

#[tokio::test]

async fn mutating_a_target_revokes_its_shares() {

let env = Env::new().await;

let admin = env.admin().await;

let files = |rel: &str| format!("/api/files/1/{rel}");

let alive = async |token: &str| {

admin.get(&format!("/api/share/{token}")).await.status == StatusCode::OK

};

// 1. Delete revokes the share on the deleted item *and* on its children.

let parent = share(&admin, "docs", false, None).await;

let child = share(&admin, "docs/a.txt", false, None).await;

let bystander = share(&admin, "src/main.rs", false, None).await;

let (parent, child, bystander) = (

parent["token"].as_str().unwrap().to_string(),

child["token"].as_str().unwrap().to_string(),

bystander["token"].as_str().unwrap().to_string(),

);

assert_eq!(admin.delete(&files("docs")).await.status, StatusCode::OK);

assert!(!alive(&parent).await, "share on the deleted folder");

assert!(!alive(&child).await, "share on a file inside it");

assert!(alive(&bystander).await, "an unrelated share must survive");

// 2. Rename frees the old name, so the share on it goes too. The proof

// that this matters: a new file takes the freed name right after.

let renamed = share(&admin, "notes.md", false, None).await;

let renamed = renamed["token"].as_str().unwrap().to_string();

let r = admin

.post_json(

&files("notes.md"),

&json!({"op": "rename", "new_name": "notes-old.md"}),

)

.await;

assert_eq!(r.status, StatusCode::OK, "{}", r.text());

assert!(!alive(&renamed).await, "share on the vacated name");

std::fs::write(env.file("notes.md"), "a different file entirely").unwrap();

assert!(

!alive(&renamed).await,

"the old link must not pick up the new file at that path"

);

// 3. Move frees the source path the same way.

let moved = share(&admin, "config.json", false, None).await;

let moved = moved["token"].as_str().unwrap().to_string();

let r = admin

.post_json(

&files("config.json"),

&json!({"op": "move", "dst_root_id": 1, "dst": "src"}),

)

.await;

assert_eq!(r.status, StatusCode::OK, "{}", r.text());

assert!(!alive(&moved).await, "share on the vacated source path");

// 4. A copy frees nothing, so it revokes nothing.

let copied = share(&admin, "editme.txt", false, None).await;

let copied = copied["token"].as_str().unwrap().to_string();

let r = admin

.post_json(

&files("editme.txt"),

&json!({"op": "copy", "dst_root_id": 1, "dst": "src"}),

)

.await;

assert_eq!(r.status, StatusCode::OK, "{}", r.text());

assert!(alive(&copied).await, "a copy must leave the share alone");

}