From 33477f284bc4b3bf04f06650e381b404d6e35e7f Mon Sep 17 00:00:00 2001 From: ssmithx Date: Tue, 29 Sep 2026 22:36:31 +0000 Subject: [PATCH] fix(files): file purchased content into FileBrowser folders again Every paid download logged "filing into filebrowser/Music/... failed (non-fatal): Permission denied". The purchase played in-app but never appeared in Files. FileBrowser's folders belong to its rootless container range (host uid 100000, mode 755). This service is host uid 1000, outside that range, so it can read them but not create files in them. New container::filebrowser::save_new_file: - Writes directly when the folder allows it. - Otherwise writes through `podman unshare`, where that uid range is ours: to a temp file, then chowned to the folder's owner, set to 0644, and hard-linked into place. FileBrowser never sees a partial file and an existing file is never replaced. A missing folder is created and given its parent's owner. No sudo. - Keeps the "name (2).ext" de-duplication the RPC did inline. Checked the unshare script on amishparadise in a scratch folder owned like FileBrowser's: new folder + file OK, owner/mode right, no clobber, no temp file left, and the service can read the result. Co-Authored-By: Claude Opus 5.5 --- core/archipelago/src/api/rpc/content.rs | 28 +-- core/archipelago/src/container/filebrowser.rs | 210 +++++++++++++++++- 2 files changed, 215 insertions(+), 23 deletions(-) diff --git a/core/archipelago/src/api/rpc/content.rs b/core/archipelago/src/api/rpc/content.rs index 6eacf538..d4b2b2da 100644 --- a/core/archipelago/src/api/rpc/content.rs +++ b/core/archipelago/src/api/rpc/content.rs @@ -678,28 +678,12 @@ impl RpcHandler { .unwrap_or("download") .to_string(); let dir = self.config.data_dir.join("filebrowser").join(folder); - if let Err(e) = tokio::fs::create_dir_all(&dir).await { - tracing::warn!("paid download: cannot create {}: {e}", dir.display()); - } else { - // Don't clobber an existing file of the same name: "x.jpg" - // → "x (2).jpg" etc. - let mut target = dir.join(&base); - let (stem, ext) = match base.rsplit_once('.') { - Some((s, e)) if !s.is_empty() => (s.to_string(), format!(".{e}")), - _ => (base.clone(), String::new()), - }; - let mut n = 2; - while target.exists() { - target = dir.join(format!("{stem} ({n}){ext}")); - n += 1; - } - match tokio::fs::write(&target, &bytes).await { - Ok(()) => tracing::info!("paid download: filed into {}", target.display()), - Err(e) => tracing::warn!( - "paid download: filing into {} failed (non-fatal): {e}", - target.display() - ), - } + match crate::container::filebrowser::save_new_file(&dir, &base, &bytes).await { + Ok(path) => tracing::info!("paid download: filed into {}", path.display()), + Err(e) => tracing::warn!( + "paid download: filing into {} failed (non-fatal): {e:#}", + dir.display() + ), } } diff --git a/core/archipelago/src/container/filebrowser.rs b/core/archipelago/src/container/filebrowser.rs index e51b11fe..18845c82 100644 --- a/core/archipelago/src/container/filebrowser.rs +++ b/core/archipelago/src/container/filebrowser.rs @@ -5,7 +5,7 @@ //! starting the container with `--config /data/.filebrowser.json`. use anyhow::{Context, Result}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use tokio::fs; use crate::update::host_sudo; @@ -117,6 +117,123 @@ fn shell_quote(s: &str) -> String { s.replace('\'', "'\\''") } +/// Save `bytes` into FileBrowser's storage as a new file in `dir`, named +/// `name` or, if that's taken, `name (2)`, `name (3)`… Never overwrites. +/// Returns the path written. +/// +/// FileBrowser's folders belong to its rootless container range (host uid +/// 100000, mode 755), so this service — host uid 1000, outside that range — +/// can read them but not write into them, and filing a purchase into Files +/// failed with EACCES (2026-09-29). When a direct write is refused, the file +/// is written through `podman unshare`, where that range is ours, and given +/// the folder's owner so FileBrowser manages it like its own uploads. +pub async fn save_new_file(dir: &Path, name: &str, bytes: &[u8]) -> Result { + save_new_file_with(dir, name, bytes, write_via_userns).await +} + +async fn save_new_file_with( + dir: &Path, + name: &str, + bytes: &[u8], + fallback: F, +) -> Result +where + F: FnOnce(PathBuf, Vec) -> Fut, + Fut: std::future::Future>, +{ + let target = unused_name(dir, name); + match write_direct(dir, &target, bytes).await { + Ok(()) => Ok(target), + Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { + fallback(target.clone(), bytes.to_vec()) + .await + .with_context(|| format!("writing {} via podman unshare", target.display()))?; + Ok(target) + } + Err(e) => Err(e).with_context(|| format!("writing {}", target.display())), + } +} + +/// `dir/name`, or the first free `dir/stem (n).ext` from n = 2. +fn unused_name(dir: &Path, name: &str) -> PathBuf { + let mut target = dir.join(name); + let (stem, ext) = match name.rsplit_once('.') { + Some((s, e)) if !s.is_empty() => (s.to_string(), format!(".{e}")), + _ => (name.to_string(), String::new()), + }; + let mut n = 2; + while target.exists() { + target = dir.join(format!("{stem} ({n}){ext}")); + n += 1; + } + target +} + +async fn write_direct(dir: &Path, target: &Path, bytes: &[u8]) -> std::io::Result<()> { + use tokio::io::AsyncWriteExt; + fs::create_dir_all(dir).await?; + let mut f = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(target) + .await?; + let written = async { + f.write_all(bytes).await?; + f.flush().await + } + .await; + if written.is_err() { + let _ = fs::remove_file(target).await; + } + written +} + +/// Write `bytes` (piped on stdin) to `target` from inside the rootless user +/// namespace. It goes to a temp file first and is hard-linked into place, so +/// FileBrowser never sees a partial file and an existing file is never +/// replaced (`ln` refuses an existing name). +async fn write_via_userns(target: PathBuf, bytes: Vec) -> Result<()> { + use tokio::io::AsyncWriteExt; + const SCRIPT: &str = r#"set -eu +dst=$1 +dir=$(dirname -- "$dst") +if [ ! -d "$dir" ]; then + mkdir -- "$dir" + chown --reference="$(dirname -- "$dir")" -- "$dir" +fi +tmp="$dir/.archy-saving.$$" +trap 'rm -f -- "$tmp"' EXIT +cat > "$tmp" +chown --reference="$dir" -- "$tmp" +chmod 0644 -- "$tmp" +ln -- "$tmp" "$dst" +"#; + let mut child = tokio::process::Command::new("podman") + .args(["unshare", "sh", "-c", SCRIPT, "sh"]) + .arg(&target) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::piped()) + .spawn() + .context("Failed to run podman unshare")?; + let mut stdin = child.stdin.take().context("podman unshare stdin")?; + let fed = stdin.write_all(&bytes).await; + drop(stdin); + let out = child + .wait_with_output() + .await + .context("Failed to wait for podman unshare")?; + if !out.status.success() { + anyhow::bail!( + "podman unshare exited with {}: {}", + out.status, + String::from_utf8_lossy(&out.stderr).trim() + ); + } + fed.context("Failed to pipe the file to podman unshare")?; + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -151,4 +268,95 @@ mod tests { let second = ensure_config(&paths).await.unwrap(); assert_eq!(second, EnsureOutcome::Unchanged); } + + #[test] + fn unused_name_numbers_duplicates_and_keeps_the_extension() { + let dir = tempfile::tempdir().unwrap(); + let d = dir.path(); + assert_eq!(unused_name(d, "song.mp3"), d.join("song.mp3")); + std::fs::write(d.join("song.mp3"), b"").unwrap(); + assert_eq!(unused_name(d, "song.mp3"), d.join("song (2).mp3")); + std::fs::write(d.join("song (2).mp3"), b"").unwrap(); + assert_eq!(unused_name(d, "song.mp3"), d.join("song (3).mp3")); + std::fs::write(d.join("README"), b"").unwrap(); + assert_eq!(unused_name(d, "README"), d.join("README (2)")); + std::fs::write(d.join(".hidden"), b"").unwrap(); + assert_eq!(unused_name(d, ".hidden"), d.join(".hidden (2)")); + } + + #[tokio::test] + async fn save_new_file_writes_directly_into_a_writable_folder() { + let dir = tempfile::tempdir().unwrap(); + let music = dir.path().join("Music"); + let path = save_new_file_with(&music, "a.mp3", b"abc", |_, _| async { + anyhow::bail!("fallback must not run") + }) + .await + .unwrap(); + assert_eq!(path, music.join("a.mp3")); + assert_eq!(std::fs::read(&path).unwrap(), b"abc"); + } + + #[tokio::test] + async fn save_new_file_never_overwrites_an_existing_file() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.mp3"), b"original").unwrap(); + let path = save_new_file_with(dir.path(), "a.mp3", b"new", |_, _| async { + anyhow::bail!("fallback must not run") + }) + .await + .unwrap(); + assert_eq!(path, dir.path().join("a (2).mp3")); + assert_eq!( + std::fs::read(dir.path().join("a.mp3")).unwrap(), + b"original" + ); + } + + /// Regression (2026-09-29): filing a purchase into a FileBrowser folder + /// owned by the container's uid range failed with EACCES. A refused + /// write must go through the user-namespace fallback, with the same + /// target and bytes. + #[tokio::test] + async fn a_refused_write_goes_through_the_userns_fallback() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let music = dir.path().join("Music"); + std::fs::create_dir(&music).unwrap(); + std::fs::set_permissions(&music, std::fs::Permissions::from_mode(0o555)).unwrap(); + if std::fs::File::create(music.join("probe")).is_ok() { + return; // running as root: mode bits don't refuse the write + } + + let seen = std::sync::Mutex::new(None); + let path = save_new_file_with(&music, "a.mp3", b"abc", |target, bytes| { + *seen.lock().unwrap() = Some((target, bytes)); + async { Ok(()) } + }) + .await + .unwrap(); + assert_eq!(path, music.join("a.mp3")); + assert_eq!( + seen.into_inner().unwrap(), + Some((music.join("a.mp3"), b"abc".to_vec())) + ); + std::fs::set_permissions(&music, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + + #[tokio::test] + async fn a_failed_fallback_is_reported() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o555)).unwrap(); + if std::fs::File::create(dir.path().join("probe")).is_ok() { + return; + } + let err = save_new_file_with(dir.path(), "a.mp3", b"abc", |_, _| async { + anyhow::bail!("no podman") + }) + .await + .unwrap_err(); + assert!(format!("{err:#}").contains("no podman")); + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); + } }