From d9bae02ff5849dffb7b85546a1dc18aaeab80ccc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 00:47:54 +0000 Subject: [PATCH 01/16] feat(fs): positional reads, writes and symlinks that work on Windows Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- crates/tapline-fs/src/file.rs | 229 ++++++++++++++++++++++++++++++++++ crates/tapline-fs/src/lib.rs | 2 + 2 files changed, 231 insertions(+) create mode 100644 crates/tapline-fs/src/file.rs diff --git a/crates/tapline-fs/src/file.rs b/crates/tapline-fs/src/file.rs new file mode 100644 index 0000000..bde3f39 --- /dev/null +++ b/crates/tapline-fs/src/file.rs @@ -0,0 +1,229 @@ +use std::fs::File; +use std::io; +use std::path::Path; + +#[cfg(unix)] +pub fn read_exact_at(file: &File, buffer: &mut [u8], offset: u64) -> io::Result<()> { + use std::os::unix::fs::FileExt; + + file.read_exact_at(buffer, offset) +} + +#[cfg(windows)] +pub fn read_exact_at(file: &File, buffer: &mut [u8], offset: u64) -> io::Result<()> { + use std::os::windows::fs::FileExt; + + let wanted = buffer.len(); + let mut filled = 0_usize; + while filled < wanted { + let Some(rest) = buffer.get_mut(filled..) else { + break; + }; + match file.seek_read(rest, offset.saturating_add(filled as u64)) { + Ok(0) => { + return Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + "the file ended before the requested range was filled", + )); + } + Ok(count) => filled = filled.saturating_add(count), + Err(e) if e.kind() == io::ErrorKind::Interrupted => {} + Err(e) => return Err(e), + } + } + Ok(()) +} + +#[cfg(unix)] +pub fn write_all_at(file: &File, data: &[u8], offset: u64) -> io::Result<()> { + use std::os::unix::fs::FileExt; + + file.write_all_at(data, offset) +} + +#[cfg(windows)] +pub fn write_all_at(file: &File, data: &[u8], offset: u64) -> io::Result<()> { + use std::os::windows::fs::FileExt; + + let total = data.len(); + let mut written = 0_usize; + while written < total { + let Some(rest) = data.get(written..) else { + break; + }; + match file.seek_write(rest, offset.saturating_add(written as u64)) { + Ok(0) => { + return Err(io::Error::new( + io::ErrorKind::WriteZero, + "the write made no progress", + )); + } + Ok(count) => written = written.saturating_add(count), + Err(e) if e.kind() == io::ErrorKind::Interrupted => {} + Err(e) => return Err(e), + } + } + Ok(()) +} + +#[cfg(unix)] +pub fn symlink(target: &Path, link: &Path) -> io::Result<()> { + std::os::unix::fs::symlink(target, link) +} + +#[cfg(windows)] +pub fn symlink(target: &Path, link: &Path) -> io::Result<()> { + let points_at_a_directory = link + .parent() + .map_or_else(|| target.to_path_buf(), |parent| parent.join(target)) + .is_dir(); + + if points_at_a_directory { + std::os::windows::fs::symlink_dir(target, link) + } else { + std::os::windows::fs::symlink_file(target, link) + } +} + +#[cfg(unix)] +pub fn set_mode(path: &Path, mode: u32) -> io::Result<()> { + use std::os::unix::fs::PermissionsExt; + + let mut permissions = std::fs::metadata(path)?.permissions(); + permissions.set_mode(mode); + std::fs::set_permissions(path, permissions) +} + +#[cfg(windows)] +pub fn set_mode(path: &Path, mode: u32) -> io::Result<()> { + let mut permissions = std::fs::metadata(path)?.permissions(); + permissions.set_readonly(mode & 0o200 == 0); + std::fs::set_permissions(path, permissions) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + struct Scratch(PathBuf); + + impl Scratch { + fn new(name: &str) -> Self { + let base = std::env::var("TAPLINE_TEST_DIR").map_or_else( + |_| { + PathBuf::from(std::env::var("HOME").unwrap_or_else(|_| ".".into())) + .join(".cache/tapline-test") + }, + PathBuf::from, + ); + let path = base.join(name); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).expect("the scratch directory must be creatable"); + Self(path) + } + + fn join(&self, name: &str) -> PathBuf { + self.0.join(name) + } + } + + impl Drop for Scratch { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn a_positional_write_lands_at_the_offset_it_names() { + let scratch = Scratch::new("file-write-at"); + let path = scratch.join("out.bin"); + let file = File::options() + .create(true) + .truncate(true) + .write(true) + .read(true) + .open(&path) + .expect("create"); + file.set_len(9).expect("allocate"); + + write_all_at(&file, b"ghi", 6).expect("tail"); + write_all_at(&file, b"abc", 0).expect("head"); + write_all_at(&file, b"def", 3).expect("middle"); + + assert_eq!(std::fs::read(&path).expect("read back"), b"abcdefghi"); + } + + #[test] + fn a_positional_read_returns_the_range_it_names() { + let scratch = Scratch::new("file-read-at"); + let path = scratch.join("in.bin"); + std::fs::write(&path, b"0123456789").expect("seed"); + + let file = File::open(&path).expect("open"); + let mut buffer = [0_u8; 4]; + read_exact_at(&file, &mut buffer, 3).expect("read"); + + assert_eq!(&buffer, b"3456"); + } + + #[test] + fn a_read_running_past_the_end_is_an_error_not_a_short_buffer() { + let scratch = Scratch::new("file-read-short"); + let path = scratch.join("small.bin"); + std::fs::write(&path, b"abc").expect("seed"); + + let file = File::open(&path).expect("open"); + let mut buffer = [0_u8; 8]; + let error = read_exact_at(&file, &mut buffer, 0).expect_err("the range is not there"); + + assert_eq!(error.kind(), io::ErrorKind::UnexpectedEof); + } + + #[test] + fn an_empty_read_is_satisfied_without_touching_the_file() { + let scratch = Scratch::new("file-read-empty"); + let path = scratch.join("empty.bin"); + std::fs::write(&path, b"").expect("seed"); + + let file = File::open(&path).expect("open"); + read_exact_at(&file, &mut [], 0).expect("nothing was asked for"); + } + + #[test] + fn a_symlink_resolves_to_what_it_points_at() { + let scratch = Scratch::new("file-symlink"); + let target = scratch.join("real.txt"); + let link = scratch.join("link.txt"); + std::fs::write(&target, b"contents").expect("seed"); + + symlink(Path::new("real.txt"), &link).expect("symlink"); + + assert_eq!(std::fs::read(&link).expect("read through"), b"contents"); + } + + #[test] + fn clearing_the_write_bit_marks_the_file_read_only() { + let scratch = Scratch::new("file-mode"); + let path = scratch.join("locked.txt"); + std::fs::write(&path, b"seed").expect("seed"); + + set_mode(&path, 0o444).expect("set mode"); + assert!( + std::fs::metadata(&path) + .expect("stat") + .permissions() + .readonly(), + "the file is still writable" + ); + + set_mode(&path, 0o644).expect("restore"); + assert!( + !std::fs::metadata(&path) + .expect("stat") + .permissions() + .readonly(), + "the write bit did not come back" + ); + } +} diff --git a/crates/tapline-fs/src/lib.rs b/crates/tapline-fs/src/lib.rs index 57ce87b..191f025 100644 --- a/crates/tapline-fs/src/lib.rs +++ b/crates/tapline-fs/src/lib.rs @@ -1,3 +1,5 @@ +mod file; mod path; +pub use file::{read_exact_at, set_mode, symlink, write_all_at}; pub use path::{PathError, SafePath, validate_path, validate_symlink}; From b3d90702a88cb08d76e9bf4d0c3790c1534071a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 00:47:54 +0000 Subject: [PATCH 02/16] fix(rt-tokio): drive the file sink through the portable primitives Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- Cargo.lock | 1 + crates/tapline-rt-tokio/Cargo.toml | 1 + crates/tapline-rt-tokio/src/sink.rs | 5 ++--- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4409cb9..31b5ef0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1468,6 +1468,7 @@ version = "0.1.0" dependencies = [ "rustls", "tapline-crypto", + "tapline-fs", "tapline-io", "tapline-net", "tapline-proto", diff --git a/crates/tapline-rt-tokio/Cargo.toml b/crates/tapline-rt-tokio/Cargo.toml index 50f659c..c8fd123 100644 --- a/crates/tapline-rt-tokio/Cargo.toml +++ b/crates/tapline-rt-tokio/Cargo.toml @@ -14,6 +14,7 @@ rust-version.workspace = true [dependencies] rustls = { workspace = true } tapline-crypto = { workspace = true } +tapline-fs = { workspace = true } tapline-io = { workspace = true } tokio = { workspace = true, features = ["net", "rt", "io-util", "time", "fs", "macros", "sync"] } tokio-rustls = { workspace = true } diff --git a/crates/tapline-rt-tokio/src/sink.rs b/crates/tapline-rt-tokio/src/sink.rs index 7f809f2..ba225f8 100644 --- a/crates/tapline-rt-tokio/src/sink.rs +++ b/crates/tapline-rt-tokio/src/sink.rs @@ -1,6 +1,5 @@ use std::fs::File; use std::io; -use std::os::unix::fs::FileExt; use std::path::Path; use tapline_io::Sink; @@ -28,14 +27,14 @@ impl FileSink { pub fn read_at(&self, offset: u64, len: usize) -> io::Result> { let mut buffer = vec![0_u8; len]; - self.file.read_exact_at(&mut buffer, offset)?; + tapline_fs::read_exact_at(&self.file, &mut buffer, offset)?; Ok(buffer) } } impl Sink for FileSink { async fn write_at(&self, offset: u64, data: &[u8]) -> io::Result<()> { - self.file.write_all_at(data, offset) + tapline_fs::write_all_at(&self.file, data, offset) } async fn allocate(&self, len: u64) -> io::Result<()> { From c596118b7d43d8caa8ad3380ba926b4c3f79fa07 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 00:47:55 +0000 Subject: [PATCH 03/16] fix(auth): keep the token file private where there are no unix modes Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- crates/tapline-auth/src/store.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/crates/tapline-auth/src/store.rs b/crates/tapline-auth/src/store.rs index d7742b6..0fc89f8 100644 --- a/crates/tapline-auth/src/store.rs +++ b/crates/tapline-auth/src/store.rs @@ -232,14 +232,16 @@ fn write_all(path: &Path, entries: &[(String, String)]) -> Result<(), TokenStore fn create_private(path: &Path, contents: &str) -> Result<(), TokenStoreError> { use std::io::Write; - use std::os::unix::fs::OpenOptionsExt; let temporary = path.with_extension("tmp"); - let mut file = std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .mode(0o600) + let mut options = std::fs::OpenOptions::new(); + options.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options .open(&temporary) .map_err(|e| TokenStoreError::Backend(e.to_string()))?; @@ -252,6 +254,7 @@ fn create_private(path: &Path, contents: &str) -> Result<(), TokenStoreError> { std::fs::rename(&temporary, path).map_err(|e| TokenStoreError::Backend(e.to_string())) } +#[cfg(unix)] fn check_permissions(path: &Path) -> Result<(), TokenStoreError> { use std::os::unix::fs::PermissionsExt; @@ -264,6 +267,12 @@ fn check_permissions(path: &Path) -> Result<(), TokenStoreError> { Ok(()) } +#[cfg(not(unix))] +fn check_permissions(path: &Path) -> Result<(), TokenStoreError> { + std::fs::metadata(path).map_err(|e| TokenStoreError::Backend(e.to_string()))?; + Ok(()) +} + fn write_file(path: &Path, token: &StoredToken) -> Result<(), TokenStoreError> { let mut entries = read_all(path)?; entries.retain(|(account, _)| account != &token.account); @@ -324,6 +333,7 @@ mod tests { "the standard alphabet is not this one" ); } + #[cfg(unix)] use std::os::unix::fs::PermissionsExt; struct Scratch(PathBuf); @@ -393,6 +403,7 @@ mod tests { assert!(store.accounts().expect("accounts").is_empty()); } + #[cfg(unix)] #[test] fn the_file_is_created_private_and_never_widens() { let scratch = Scratch::new("tokens-perms"); @@ -408,6 +419,7 @@ mod tests { assert_eq!(mode, 0o600); } + #[cfg(unix)] #[test] fn a_world_readable_token_file_is_refused_rather_than_used() { let scratch = Scratch::new("tokens-insecure"); From e932eb5b93138c2c782c009540453d5f3c05a5f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 00:47:55 +0000 Subject: [PATCH 04/16] fix(session): install through the portable filesystem primitives Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- crates/tapline/src/install.rs | 8 ++++++++ crates/tapline/src/session.rs | 21 +++++---------------- crates/tapline/src/validate.rs | 3 +-- crates/tapline/tests/differential.rs | 1 + crates/tapline/tests/gmod.rs | 1 + 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/crates/tapline/src/install.rs b/crates/tapline/src/install.rs index f617c47..f54eeed 100644 --- a/crates/tapline/src/install.rs +++ b/crates/tapline/src/install.rs @@ -205,6 +205,14 @@ mod tests { assert!(filter.include_dlc); } + #[test] + fn the_steamcmd_policy_makes_everything_runnable_and_the_manifest_one_does_not() { + assert_eq!(FileModes::SteamCmd.mode_for(true), 0o755); + assert_eq!(FileModes::SteamCmd.mode_for(false), 0o755); + assert_eq!(FileModes::Manifest.mode_for(true), 0o755); + assert_eq!(FileModes::Manifest.mode_for(false), 0o644); + } + #[test] fn concurrency_defaults_to_something_a_cdn_will_tolerate() { let concurrency = InstallOptions::default().concurrency; diff --git a/crates/tapline/src/session.rs b/crates/tapline/src/session.rs index ea4b9c8..352bc85 100644 --- a/crates/tapline/src/session.rs +++ b/crates/tapline/src/session.rs @@ -1,6 +1,5 @@ use crate::{InstallError, InstallOptions, InstallReport}; use std::collections::HashMap; -use std::path::Path; use std::sync::Arc; use tapline_cdn::{Host, HostPool, fetch_chunk_bytes, fetch_manifest}; use tapline_event::{Event, Plan}; @@ -1285,10 +1284,9 @@ impl Session { &entry.manifest, &options.install_dir, |path, offset, len| { - use std::os::unix::fs::FileExt; let file = std::fs::File::open(path)?; let mut buffer = vec![0_u8; len]; - file.read_exact_at(&mut buffer, offset)?; + tapline_fs::read_exact_at(&file, &mut buffer, offset)?; Ok(buffer) }, ); @@ -1598,7 +1596,7 @@ fn create_symlinks( std::fs::create_dir_all(parent)?; } let _ = std::fs::remove_file(&link_path); - std::os::unix::fs::symlink(&resolved_target, &link_path)?; + tapline_fs::symlink(&resolved_target, &link_path)?; report.files += 1; } Ok(()) @@ -1649,7 +1647,7 @@ fn finalize_file(file: PendingFile) -> Result { file.sink .sync_blocking() .map_err(|error| InstallError::Io(error.to_string()))?; - set_permissions(&file.target, file.mode)?; + tapline_fs::set_mode(&file.target, file.mode)?; drop(file.sink); let mut extended = Vec::new(); @@ -1791,14 +1789,6 @@ fn apply_outcome( Ok(()) } -fn set_permissions(path: &Path, mode: u32) -> std::io::Result<()> { - use std::os::unix::fs::PermissionsExt; - - let mut permissions = std::fs::metadata(path)?.permissions(); - permissions.set_mode(mode); - std::fs::set_permissions(path, permissions) -} - fn now_unix() -> u64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -1829,8 +1819,7 @@ mod tests { assert_eq!(order, vec!["saved".to_owned()]); } - use super::*; - + #[cfg(unix)] #[test] fn executables_get_the_bit_a_launcher_needs() { use std::os::unix::fs::PermissionsExt; @@ -1847,7 +1836,7 @@ mod tests { std::fs::write(&path, b"#!/bin/sh\n").expect("write"); let mode_of = |policy: crate::FileModes, executable: bool| { - set_permissions(&path, policy.mode_for(executable)).expect("chmod"); + tapline_fs::set_mode(&path, policy.mode_for(executable)).expect("chmod"); std::fs::metadata(&path).expect("stat").permissions().mode() & 0o777 }; diff --git a/crates/tapline/src/validate.rs b/crates/tapline/src/validate.rs index a0661d1..cb7fc46 100644 --- a/crates/tapline/src/validate.rs +++ b/crates/tapline/src/validate.rs @@ -170,10 +170,9 @@ mod tests { } fn real_read(path: &Path, offset: u64, len: usize) -> std::io::Result> { - use std::os::unix::fs::FileExt; let file = std::fs::File::open(path)?; let mut buffer = vec![0_u8; len]; - file.read_exact_at(&mut buffer, offset)?; + tapline_fs::read_exact_at(&file, &mut buffer, offset)?; Ok(buffer) } diff --git a/crates/tapline/tests/differential.rs b/crates/tapline/tests/differential.rs index b86176d..d91ef5f 100644 --- a/crates/tapline/tests/differential.rs +++ b/crates/tapline/tests/differential.rs @@ -1,3 +1,4 @@ +#![cfg(unix)] #![allow(clippy::expect_used, clippy::unwrap_used)] use std::collections::BTreeMap; diff --git a/crates/tapline/tests/gmod.rs b/crates/tapline/tests/gmod.rs index 258b591..aa30c77 100644 --- a/crates/tapline/tests/gmod.rs +++ b/crates/tapline/tests/gmod.rs @@ -1,3 +1,4 @@ +#![cfg(unix)] #![allow(clippy::expect_used, clippy::unwrap_used)] use std::os::unix::fs::PermissionsExt; From d05aceb9eb5defc6b44446721162c6a5012a4416 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 00:47:55 +0000 Subject: [PATCH 05/16] ci: cross-check the workspace for x86_64-pc-windows-gnu Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- .github/workflows/ci.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d7d395..88fe8c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,31 @@ jobs: # deterministic. They are run by hand against real Steam. - run: cargo test --workspace --all-features + # The port to Windows is only a port while something checks it. Cross-checking + # from the Linux runners is enough to catch the regression that matters -- a + # `std::os::unix` import creeping back in -- and costs no new runner. `ring` + # compiles C for the target, so this needs the mingw toolchain the same way + # binary-size needs musl-tools. + windows: + name: Windows cross-check + runs-on: self-hosted + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + components: clippy + targets: x86_64-pc-windows-gnu + - uses: Swatinem/rust-cache@v2 + + - name: Install the mingw toolchain + run: sudo apt-get update && sudo apt-get install -y mingw-w64 + + - run: cargo clippy --workspace --all-targets --target x86_64-pc-windows-gnu -- -D warnings + + # clippy --all-targets type-checks but never links. The CLI is what a + # Windows user actually runs, so link it. + - run: cargo build --target x86_64-pc-windows-gnu -p tapline-cli + # The dependency floor is a floor: this fails on a new licence, a yanked crate, # an advisory, or anything from the deny list in deny.toml (openssl, hyper, # reqwest, prost, a C build). From ffde770926189c25d98a4ca332906005945c65f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 01:06:14 +0000 Subject: [PATCH 06/16] fix(fs): take a directory symlink off the path before relinking it Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- crates/tapline-fs/src/file.rs | 63 +++++++++++++++++++++++++++++++++++ crates/tapline-fs/src/lib.rs | 2 +- crates/tapline/src/session.rs | 2 +- 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/crates/tapline-fs/src/file.rs b/crates/tapline-fs/src/file.rs index bde3f39..d4a44b8 100644 --- a/crates/tapline-fs/src/file.rs +++ b/crates/tapline-fs/src/file.rs @@ -85,6 +85,31 @@ pub fn symlink(target: &Path, link: &Path) -> io::Result<()> { } } +pub fn remove_existing(path: &Path) -> io::Result<()> { + let file_type = match std::fs::symlink_metadata(path) { + Ok(metadata) => metadata.file_type(), + Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(e) => return Err(e), + }; + if names_a_directory(&file_type) { + std::fs::remove_dir(path) + } else { + std::fs::remove_file(path) + } +} + +#[cfg(unix)] +fn names_a_directory(file_type: &std::fs::FileType) -> bool { + file_type.is_dir() +} + +#[cfg(windows)] +fn names_a_directory(file_type: &std::fs::FileType) -> bool { + use std::os::windows::fs::FileTypeExt; + + file_type.is_dir() || file_type.is_symlink_dir() +} + #[cfg(unix)] pub fn set_mode(path: &Path, mode: u32) -> io::Result<()> { use std::os::unix::fs::PermissionsExt; @@ -202,6 +227,44 @@ mod tests { assert_eq!(std::fs::read(&link).expect("read through"), b"contents"); } + #[test] + fn removing_a_symlink_takes_the_link_and_leaves_its_target() { + let scratch = Scratch::new("file-remove-link"); + let target = scratch.join("real"); + std::fs::create_dir_all(&target).expect("seed"); + std::fs::write(target.join("inside.txt"), b"kept").expect("seed"); + let link = scratch.join("link"); + symlink(Path::new("real"), &link).expect("symlink"); + + remove_existing(&link).expect("remove"); + + assert!(!link.exists(), "the link is still there"); + assert!( + target.join("inside.txt").is_file(), + "the target was removed along with the link" + ); + } + + #[test] + fn removing_a_path_that_is_not_there_is_not_an_error() { + let scratch = Scratch::new("file-remove-missing"); + remove_existing(&scratch.join("never-existed")).expect("nothing to remove"); + } + + #[test] + fn a_symlink_replaces_whatever_stood_in_its_place() { + let scratch = Scratch::new("file-relink"); + let target = scratch.join("real.txt"); + std::fs::write(&target, b"contents").expect("seed"); + let link = scratch.join("link.txt"); + std::fs::write(&link, b"stale").expect("seed"); + + remove_existing(&link).expect("remove"); + symlink(Path::new("real.txt"), &link).expect("symlink"); + + assert_eq!(std::fs::read(&link).expect("read through"), b"contents"); + } + #[test] fn clearing_the_write_bit_marks_the_file_read_only() { let scratch = Scratch::new("file-mode"); diff --git a/crates/tapline-fs/src/lib.rs b/crates/tapline-fs/src/lib.rs index 191f025..6386d57 100644 --- a/crates/tapline-fs/src/lib.rs +++ b/crates/tapline-fs/src/lib.rs @@ -1,5 +1,5 @@ mod file; mod path; -pub use file::{read_exact_at, set_mode, symlink, write_all_at}; +pub use file::{read_exact_at, remove_existing, set_mode, symlink, write_all_at}; pub use path::{PathError, SafePath, validate_path, validate_symlink}; diff --git a/crates/tapline/src/session.rs b/crates/tapline/src/session.rs index 352bc85..8afbe1c 100644 --- a/crates/tapline/src/session.rs +++ b/crates/tapline/src/session.rs @@ -1595,7 +1595,7 @@ fn create_symlinks( if let Some(parent) = link_path.parent() { std::fs::create_dir_all(parent)?; } - let _ = std::fs::remove_file(&link_path); + let _ = tapline_fs::remove_existing(&link_path); tapline_fs::symlink(&resolved_target, &link_path)?; report.files += 1; } From 3ddf824fdf9b78c143dee66866d572c9fcb1cc92 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 01:06:14 +0000 Subject: [PATCH 07/16] ci: hand the jobs a read-only token and leave none on the runner Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- .github/workflows/ci.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88fe8c9..4998089 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,15 @@ env: # A dependency that fails to build is a dependency we should not have. CARGO_NET_RETRY: 3 +# Every job here reads the repository and writes nothing back, so the token that +# reaches them needs nothing more. It matters because these are self-hosted +# runners executing pull-request code: a build script from a fork runs with +# whatever the workflow was handed. `persist-credentials: false` on each checkout +# keeps the token out of .git/config on the runner for the same reason; nothing +# in the workspace is a git dependency, so no step needs it to fetch. +permissions: + contents: read + jobs: # Per-commit gate. clippy --all-targets type-checks every bin, test and example # in the workspace, which is most of the value here; `cargo test` runs beside it @@ -19,6 +28,8 @@ jobs: runs-on: self-hosted steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@stable with: components: rustfmt, clippy @@ -47,6 +58,8 @@ jobs: runs-on: self-hosted steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@stable with: components: clippy @@ -69,6 +82,8 @@ jobs: runs-on: self-hosted steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@stable # The official cargo-deny action runs in Docker, and these runners have no # Docker daemon. This installs the same binary directly. @@ -84,6 +99,8 @@ jobs: runs-on: self-hosted steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@stable with: targets: x86_64-unknown-linux-musl @@ -141,6 +158,8 @@ jobs: runs-on: self-hosted steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - name: No heavy crates in the metadata build @@ -160,6 +179,8 @@ jobs: runs-on: self-hosted steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - uses: actions/setup-node@v4 From 6babb8ebec8e89d378bed4bdfa01d87f26e0bae3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 01:08:09 +0000 Subject: [PATCH 08/16] fix(auth): keep the token file out of the working directory on Windows Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- crates/tapline-auth/src/store.rs | 57 ++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/crates/tapline-auth/src/store.rs b/crates/tapline-auth/src/store.rs index 0fc89f8..cd4bd0c 100644 --- a/crates/tapline-auth/src/store.rs +++ b/crates/tapline-auth/src/store.rs @@ -108,13 +108,8 @@ const KEYRING_SERVICE: &str = "tapline"; impl TokenStore { #[must_use] pub fn default_file() -> Self { - let base = std::env::var("XDG_CONFIG_HOME") - .map(PathBuf::from) - .unwrap_or_else(|_| { - PathBuf::from(std::env::var("HOME").unwrap_or_else(|_| ".".into())).join(".config") - }); Self::File { - path: base.join("tapline").join("tokens"), + path: config_dir().join("tapline").join("tokens"), } } @@ -230,6 +225,32 @@ fn write_all(path: &Path, entries: &[(String, String)]) -> Result<(), TokenStore Ok(()) } +fn config_dir() -> PathBuf { + resolve_config_dir(configured_config_dir(), home_dir()) +} + +#[cfg(not(windows))] +fn configured_config_dir() -> Option { + std::env::var_os("XDG_CONFIG_HOME").map(PathBuf::from) +} + +#[cfg(windows)] +fn configured_config_dir() -> Option { + std::env::var_os("APPDATA") + .or_else(|| std::env::var_os("LOCALAPPDATA")) + .map(PathBuf::from) +} + +fn home_dir() -> Option { + std::env::var_os("HOME") + .or_else(|| std::env::var_os("USERPROFILE")) + .map(PathBuf::from) +} + +fn resolve_config_dir(configured: Option, home: Option) -> PathBuf { + configured.unwrap_or_else(|| home.unwrap_or_else(|| PathBuf::from(".")).join(".config")) +} + fn create_private(path: &Path, contents: &str) -> Result<(), TokenStoreError> { use std::io::Write; @@ -403,6 +424,30 @@ mod tests { assert!(store.accounts().expect("accounts").is_empty()); } + #[test] + fn the_platform_config_directory_wins_when_the_environment_names_one() { + assert_eq!( + resolve_config_dir( + Some(PathBuf::from("configured")), + Some(PathBuf::from("home")) + ), + PathBuf::from("configured") + ); + } + + #[test] + fn without_one_the_tokens_sit_under_the_home_directory() { + assert_eq!( + resolve_config_dir(None, Some(PathBuf::from("home"))), + PathBuf::from("home/.config") + ); + } + + #[test] + fn with_no_home_either_the_path_is_still_a_config_directory() { + assert_eq!(resolve_config_dir(None, None), PathBuf::from("./.config")); + } + #[cfg(unix)] #[test] fn the_file_is_created_private_and_never_widens() { From 246cb3e4be2942c251dc9be9b6a920bb6eb25a7a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 01:10:18 +0000 Subject: [PATCH 09/16] fix(session): report why a stale path could not make way for a symlink Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- crates/tapline-fs/src/file.rs | 34 +++++++++++++++++++++++++++++++++- crates/tapline/src/session.rs | 2 +- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/tapline-fs/src/file.rs b/crates/tapline-fs/src/file.rs index d4a44b8..ebd4033 100644 --- a/crates/tapline-fs/src/file.rs +++ b/crates/tapline-fs/src/file.rs @@ -91,10 +91,14 @@ pub fn remove_existing(path: &Path) -> io::Result<()> { Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()), Err(e) => return Err(e), }; - if names_a_directory(&file_type) { + let removed = if names_a_directory(&file_type) { std::fs::remove_dir(path) } else { std::fs::remove_file(path) + }; + match removed { + Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), + outcome => outcome, } } @@ -251,6 +255,34 @@ mod tests { remove_existing(&scratch.join("never-existed")).expect("nothing to remove"); } + #[test] + fn a_directory_with_something_in_it_refuses_to_be_removed() { + let scratch = Scratch::new("file-remove-occupied"); + let occupied = scratch.join("occupied"); + std::fs::create_dir_all(&occupied).expect("seed"); + std::fs::write(occupied.join("inside.txt"), b"in the way").expect("seed"); + + remove_existing(&occupied).expect_err("a non-empty directory is not ours to delete"); + assert!( + occupied.join("inside.txt").is_file(), + "the contents were removed anyway" + ); + } + + #[test] + fn an_empty_directory_gives_way_to_a_link() { + let scratch = Scratch::new("file-remove-empty-dir"); + let target = scratch.join("real.txt"); + std::fs::write(&target, b"contents").expect("seed"); + let link = scratch.join("link.txt"); + std::fs::create_dir_all(&link).expect("seed"); + + remove_existing(&link).expect("remove"); + symlink(Path::new("real.txt"), &link).expect("symlink"); + + assert_eq!(std::fs::read(&link).expect("read through"), b"contents"); + } + #[test] fn a_symlink_replaces_whatever_stood_in_its_place() { let scratch = Scratch::new("file-relink"); diff --git a/crates/tapline/src/session.rs b/crates/tapline/src/session.rs index 8afbe1c..390920a 100644 --- a/crates/tapline/src/session.rs +++ b/crates/tapline/src/session.rs @@ -1595,7 +1595,7 @@ fn create_symlinks( if let Some(parent) = link_path.parent() { std::fs::create_dir_all(parent)?; } - let _ = tapline_fs::remove_existing(&link_path); + tapline_fs::remove_existing(&link_path)?; tapline_fs::symlink(&resolved_target, &link_path)?; report.files += 1; } From 52f159fa7666f43083c6a386ebcd1b72c6dae868 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 01:49:46 +0000 Subject: [PATCH 10/16] ci: run on GitHub's runners by default, self-hosted by choice Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- .github/workflows/ci.yml | 43 ++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4998089..379c1c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,21 +11,33 @@ env: CARGO_NET_RETRY: 3 # Every job here reads the repository and writes nothing back, so the token that -# reaches them needs nothing more. It matters because these are self-hosted -# runners executing pull-request code: a build script from a fork runs with -# whatever the workflow was handed. `persist-credentials: false` on each checkout -# keeps the token out of .git/config on the runner for the same reason; nothing -# in the workspace is a git dependency, so no step needs it to fetch. +# reaches them needs nothing more. It matters most when a runner is self-hosted +# and executing pull-request code: a build script from a fork runs with whatever +# the workflow was handed. `persist-credentials: false` on each checkout keeps +# the token out of .git/config on the runner for the same reason; nothing in the +# workspace is a git dependency, so no step needs it to fetch. permissions: contents: read +# Where these jobs run. GitHub's own runners are the default because they are +# always there and cost nothing on a public repository, and because a pull +# request from a fork cannot reach our self-hosted ones at all. The self-hosted +# box is faster when it is up, so to send the jobs back to it set the repository +# variable CI_RUNNER to `self-hosted` (Settings > Secrets and variables > +# Actions > Variables); deleting the variable brings them back here. Nothing in +# this file assumes one or the other: every job installs what it needs. +# +# There is no way to fail over automatically. A job addressed to a runner that +# is offline queues rather than failing, and the API that would say whether one +# is online needs administration rights the workflow token does not have. + jobs: # Per-commit gate. clippy --all-targets type-checks every bin, test and example - # in the workspace, which is most of the value here; `cargo test` runs beside it - # rather than after, since the runners are 32-core and neither job is the + # in the workspace, which is most of the value here; `cargo test` runs beside + # it rather than after, since the jobs run in parallel and neither is the # bottleneck. check: - runs-on: self-hosted + runs-on: ${{ vars.CI_RUNNER || 'ubuntu-latest' }} steps: - uses: actions/checkout@v4 with: @@ -55,7 +67,7 @@ jobs: # binary-size needs musl-tools. windows: name: Windows cross-check - runs-on: self-hosted + runs-on: ${{ vars.CI_RUNNER || 'ubuntu-latest' }} steps: - uses: actions/checkout@v4 with: @@ -79,14 +91,15 @@ jobs: # an advisory, or anything from the deny list in deny.toml (openssl, hyper, # reqwest, prost, a C build). deny: - runs-on: self-hosted + runs-on: ${{ vars.CI_RUNNER || 'ubuntu-latest' }} steps: - uses: actions/checkout@v4 with: persist-credentials: false - uses: dtolnay/rust-toolchain@stable - # The official cargo-deny action runs in Docker, and these runners have no - # Docker daemon. This installs the same binary directly. + # The official cargo-deny action runs in Docker, which the self-hosted + # runners have no daemon for. This installs the same binary directly and + # works wherever the job lands. - uses: taiki-e/install-action@v2 with: tool: cargo-deny @@ -96,7 +109,7 @@ jobs: # Also proves the static musl build works, which needs a musl-targeting C # compiler because rustls's `ring` backend compiles C for the target. binary-size: - runs-on: self-hosted + runs-on: ${{ vars.CI_RUNNER || 'ubuntu-latest' }} steps: - uses: actions/checkout@v4 with: @@ -155,7 +168,7 @@ jobs: # The metadata build is what a webapp links: it must not drag in rayon, cap-std, # clap or serde. Asserted against the dependency graph rather than hoped for. minimal-graph: - runs-on: self-hosted + runs-on: ${{ vars.CI_RUNNER || 'ubuntu-latest' }} steps: - uses: actions/checkout@v4 with: @@ -176,7 +189,7 @@ jobs: bindings: name: JS bindings - runs-on: self-hosted + runs-on: ${{ vars.CI_RUNNER || 'ubuntu-latest' }} steps: - uses: actions/checkout@v4 with: From 0d65834545cfdcfeed0cfb554619d3fc5aa79e0a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 01:51:50 +0000 Subject: [PATCH 11/16] fix(deps): take rustls past the plaintext-handshake advisory Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31b5ef0..cf5a6bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1018,9 +1018,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.43" +version = "0.23.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0283386ce02abc0151e1761d08802dfe86c173b0b494af5cbc086574e453da06" +checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" dependencies = [ "once_cell", "ring", From d715a8af8ecb13743e779c70ac78b98ed12a7362 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 02:03:20 +0000 Subject: [PATCH 12/16] feat(auth): give the Windows token file a private ACL, and refuse it without one Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- Cargo.lock | 1 + crates/tapline-auth/Cargo.toml | 28 ++++- crates/tapline-auth/src/lib.rs | 3 + crates/tapline-auth/src/sddl.rs | 146 ++++++++++++++++++++++ crates/tapline-auth/src/store.rs | 90 +++++++++++++- crates/tapline-auth/src/windows_acl.rs | 163 +++++++++++++++++++++++++ 6 files changed, 428 insertions(+), 3 deletions(-) create mode 100644 crates/tapline-auth/src/sddl.rs create mode 100644 crates/tapline-auth/src/windows_acl.rs diff --git a/Cargo.lock b/Cargo.lock index cf5a6bf..7e3ddc6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1284,6 +1284,7 @@ dependencies = [ "rsa", "tapline-ids", "tapline-vdf", + "windows-sys 0.61.2", "zeroize", ] diff --git a/crates/tapline-auth/Cargo.toml b/crates/tapline-auth/Cargo.toml index 43bdb77..5da6aed 100644 --- a/crates/tapline-auth/Cargo.toml +++ b/crates/tapline-auth/Cargo.toml @@ -26,5 +26,29 @@ optional = true default-features = false features = ["sync-secret-service", "crypto-rust"] -[lints] -workspace = true +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61", features = [ + "Win32_Foundation", + "Win32_Security", + "Win32_Security_Authorization", + "Win32_Storage_FileSystem", + "Win32_System_Threading", +] } + +# Restated rather than inherited from the workspace, for one word: the workspace +# forbids unsafe outright, and `forbid` cannot be opted out of. Making the token +# file private on Windows means calling the Win32 security APIs by hand -- the +# crates that wrap them are either unmaintained or too new to trust with a +# refresh token -- so this crate says `deny` instead and `src/windows_acl.rs` +# allows it for itself. Nothing else here may, and on every other platform that +# module is not compiled at all. Everything below matches the workspace; if the +# workspace lints change, change them here too. +[lints.rust] +unsafe_code = "deny" + +[lints.clippy] +unwrap_used = "deny" +expect_used = "deny" +panic = "deny" +indexing_slicing = "deny" +todo = "deny" diff --git a/crates/tapline-auth/src/lib.rs b/crates/tapline-auth/src/lib.rs index 4bdac27..cfb3839 100644 --- a/crates/tapline-auth/src/lib.rs +++ b/crates/tapline-auth/src/lib.rs @@ -1,6 +1,9 @@ mod local; mod password; +mod sddl; mod store; +#[cfg(windows)] +mod windows_acl; pub use local::{ LocalAccount, discover, discover_in, libraries, most_recent, parse_libraries, parse_login_users, diff --git a/crates/tapline-auth/src/sddl.rs b/crates/tapline-auth/src/sddl.rs new file mode 100644 index 0000000..28f061a --- /dev/null +++ b/crates/tapline-auth/src/sddl.rs @@ -0,0 +1,146 @@ +#![cfg_attr(not(windows), allow(dead_code))] + +const LOCAL_SYSTEM: &str = "S-1-5-18"; +const ADMINISTRATORS: &str = "S-1-5-32-544"; +const LOCAL_SYSTEM_ALIAS: &str = "SY"; +const ADMINISTRATORS_ALIAS: &str = "BA"; + +const ALLOW_ACE_TYPES: [&str; 4] = ["A", "OA", "XA", "ZA"]; + +pub fn private_dacl(owner: &str) -> String { + format!("D:P(A;;FA;;;{owner})") +} + +pub fn shared_with(dacl: &str, owner: &str) -> Option { + let Some(body) = dacl_body(dacl) else { + return Some("everyone".to_owned()); + }; + + for ace in aces(body) { + let mut fields = ace.split(';'); + let Some(kind) = fields.next() else { + continue; + }; + if !ALLOW_ACE_TYPES.contains(&kind.trim()) { + continue; + } + let Some(holder) = fields.nth(4) else { + continue; + }; + let holder = holder.trim(); + if !is_ours(holder, owner) { + return Some(holder.to_owned()); + } + } + None +} + +fn is_ours(holder: &str, owner: &str) -> bool { + holder.eq_ignore_ascii_case(owner) + || holder.eq_ignore_ascii_case(LOCAL_SYSTEM) + || holder.eq_ignore_ascii_case(ADMINISTRATORS) + || holder.eq_ignore_ascii_case(LOCAL_SYSTEM_ALIAS) + || holder.eq_ignore_ascii_case(ADMINISTRATORS_ALIAS) +} + +fn dacl_body(descriptor: &str) -> Option<&str> { + let at = descriptor.find("D:")?; + let body = descriptor.get(at.saturating_add(2)..)?; + let end = body.find("S:").unwrap_or(body.len()); + let body = body.get(..end)?; + if body.contains("NO_ACCESS_CONTROL") { + return None; + } + Some(body) +} + +fn aces(body: &str) -> impl Iterator { + body.split('(').skip(1).filter_map(|ace| { + let end = ace.find(')')?; + ace.get(..end) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + const OWNER: &str = "S-1-5-21-1111111111-2222222222-3333333333-1001"; + + #[test] + fn the_descriptor_we_write_grants_the_owner_alone() { + let dacl = private_dacl(OWNER); + assert_eq!(dacl, format!("D:P(A;;FA;;;{OWNER})")); + assert_eq!(shared_with(&dacl, OWNER), None); + } + + #[test] + fn a_descriptor_with_no_dacl_at_all_is_open_to_everyone() { + assert_eq!( + shared_with("O:BAG:BA", OWNER), + Some("everyone".to_owned()), + "a security descriptor with no DACL grants everyone access" + ); + } + + #[test] + fn an_explicitly_absent_dacl_is_open_to_everyone() { + assert_eq!( + shared_with("D:NO_ACCESS_CONTROL", OWNER), + Some("everyone".to_owned()) + ); + } + + #[test] + fn the_accounts_that_can_reach_any_file_anyway_do_not_count() { + let dacl = format!("D:P(A;;FA;;;SY)(A;;FA;;;BA)(A;;FA;;;{OWNER})"); + assert_eq!(shared_with(&dacl, OWNER), None); + + let spelled_out = format!("D:P(A;;FA;;;S-1-5-18)(A;;FA;;;S-1-5-32-544)(A;;FA;;;{OWNER})"); + assert_eq!(shared_with(&spelled_out, OWNER), None); + } + + #[test] + fn another_account_with_any_access_at_all_is_reported() { + let dacl = format!("D:P(A;;FA;;;{OWNER})(A;;0x1200a9;;;BU)"); + assert_eq!(shared_with(&dacl, OWNER), Some("BU".to_owned())); + } + + #[test] + fn everyone_is_reported_even_when_the_owner_is_listed_first() { + let dacl = format!("D:AI(A;;FA;;;{OWNER})(A;ID;FA;;;WD)"); + assert_eq!(shared_with(&dacl, OWNER), Some("WD".to_owned())); + } + + #[test] + fn a_denial_of_someone_else_is_not_a_grant_to_them() { + let dacl = format!("D:P(D;;FA;;;WD)(A;;FA;;;{OWNER})"); + assert_eq!(shared_with(&dacl, OWNER), None); + } + + #[test] + fn an_owner_sid_matches_whatever_case_windows_hands_back() { + let dacl = private_dacl(&OWNER.to_lowercase()); + assert_eq!(shared_with(&dacl, OWNER), None); + } + + #[test] + fn a_system_acl_after_the_dacl_is_not_read_as_a_grant() { + let dacl = format!("D:P(A;;FA;;;{OWNER})S:AI(AU;SAFA;FA;;;WD)"); + assert_eq!(shared_with(&dacl, OWNER), None); + } + + #[test] + fn an_owner_and_group_prefix_does_not_hide_the_dacl() { + let dacl = format!("O:{OWNER}G:BAD:P(A;;FA;;;{OWNER})"); + assert_eq!(shared_with(&dacl, OWNER), None); + + let shared = format!("O:{OWNER}G:BAD:P(A;;FA;;;{OWNER})(A;;FA;;;WD)"); + assert_eq!(shared_with(&shared, OWNER), Some("WD".to_owned())); + } + + #[test] + fn an_empty_dacl_grants_nobody_anything() { + assert_eq!(shared_with("D:P", OWNER), None); + } +} diff --git a/crates/tapline-auth/src/store.rs b/crates/tapline-auth/src/store.rs index cd4bd0c..b8673c5 100644 --- a/crates/tapline-auth/src/store.rs +++ b/crates/tapline-auth/src/store.rs @@ -74,6 +74,7 @@ pub enum TokenStoreError { Backend(String), Malformed, Insecure { mode: u32 }, + Shared { holder: String }, } impl fmt::Display for TokenStoreError { @@ -85,6 +86,10 @@ impl fmt::Display for TokenStoreError { f, "the token file is mode {mode:o}; it must not be readable by other users" ), + Self::Shared { holder } => write!( + f, + "the token file grants access to {holder}; it must not be readable by other users" + ), } } } @@ -251,6 +256,27 @@ fn resolve_config_dir(configured: Option, home: Option) -> Pat configured.unwrap_or_else(|| home.unwrap_or_else(|| PathBuf::from(".")).join(".config")) } +#[cfg(windows)] +fn create_private(path: &Path, contents: &str) -> Result<(), TokenStoreError> { + use std::io::Write; + + let temporary = path.with_extension("tmp"); + let owner = crate::windows_acl::current_user_sid() + .map_err(|e| TokenStoreError::Backend(e.to_string()))?; + let mut file = + crate::windows_acl::create_with_dacl(&temporary, &crate::sddl::private_dacl(&owner)) + .map_err(|e| TokenStoreError::Backend(e.to_string()))?; + + file.write_all(contents.as_bytes()) + .map_err(|e| TokenStoreError::Backend(e.to_string()))?; + file.sync_all() + .map_err(|e| TokenStoreError::Backend(e.to_string()))?; + drop(file); + + std::fs::rename(&temporary, path).map_err(|e| TokenStoreError::Backend(e.to_string())) +} + +#[cfg(not(windows))] fn create_private(path: &Path, contents: &str) -> Result<(), TokenStoreError> { use std::io::Write; @@ -288,7 +314,20 @@ fn check_permissions(path: &Path) -> Result<(), TokenStoreError> { Ok(()) } -#[cfg(not(unix))] +#[cfg(windows)] +fn check_permissions(path: &Path) -> Result<(), TokenStoreError> { + let owner = crate::windows_acl::current_user_sid() + .map_err(|e| TokenStoreError::Backend(e.to_string()))?; + let dacl = + crate::windows_acl::dacl_of(path).map_err(|e| TokenStoreError::Backend(e.to_string()))?; + + match crate::sddl::shared_with(&dacl, &owner) { + Some(holder) => Err(TokenStoreError::Shared { holder }), + None => Ok(()), + } +} + +#[cfg(not(any(unix, windows)))] fn check_permissions(path: &Path) -> Result<(), TokenStoreError> { std::fs::metadata(path).map_err(|e| TokenStoreError::Backend(e.to_string()))?; Ok(()) @@ -448,6 +487,55 @@ mod tests { assert_eq!(resolve_config_dir(None, None), PathBuf::from("./.config")); } + #[cfg(windows)] + #[test] + fn the_file_is_created_reachable_by_nobody_else() { + let scratch = Scratch::new("tokens-acl"); + let store = scratch.store(); + store.save(&token("someone")).expect("save"); + + let path = scratch.0.join("tokens"); + let owner = crate::windows_acl::current_user_sid().expect("our own sid"); + let dacl = crate::windows_acl::dacl_of(&path).expect("read the dacl back"); + assert_eq!( + crate::sddl::shared_with(&dacl, &owner), + None, + "a freshly written token file is reachable by someone else: {dacl}" + ); + + assert_eq!( + store.load("someone").expect("load"), + Some(token("someone")), + "the file we just wrote was refused as insecure" + ); + } + + #[cfg(windows)] + #[test] + fn a_token_file_anyone_can_read_is_refused_rather_than_used() { + let scratch = Scratch::new("tokens-acl-widened"); + let store = scratch.store(); + store.save(&token("someone")).expect("save"); + + let path = scratch.0.join("tokens"); + let widened = std::process::Command::new("icacls") + .arg(&path) + .arg("/grant") + .arg("*S-1-1-0:(R)") + .output() + .expect("icacls ships with Windows"); + assert!( + widened.status.success(), + "could not widen the acl for the test: {}", + String::from_utf8_lossy(&widened.stderr) + ); + + assert!( + matches!(store.load("someone"), Err(TokenStoreError::Shared { .. })), + "a token file readable by Everyone was accepted" + ); + } + #[cfg(unix)] #[test] fn the_file_is_created_private_and_never_widens() { diff --git a/crates/tapline-auth/src/windows_acl.rs b/crates/tapline-auth/src/windows_acl.rs new file mode 100644 index 0000000..205c0bf --- /dev/null +++ b/crates/tapline-auth/src/windows_acl.rs @@ -0,0 +1,163 @@ +#![allow(unsafe_code)] + +use std::ffi::{OsStr, OsString}; +use std::fs::File; +use std::io; +use std::os::windows::ffi::{OsStrExt, OsStringExt}; +use std::os::windows::io::FromRawHandle; +use std::path::Path; +use std::ptr; + +use windows_sys::Win32::Foundation::{ + CloseHandle, GENERIC_WRITE, HANDLE, INVALID_HANDLE_VALUE, LocalFree, +}; +use windows_sys::Win32::Security::Authorization::{ + ConvertSecurityDescriptorToStringSecurityDescriptorW, ConvertSidToStringSidW, + ConvertStringSecurityDescriptorToSecurityDescriptorW, GetNamedSecurityInfoW, SDDL_REVISION_1, + SE_FILE_OBJECT, +}; +use windows_sys::Win32::Security::{ + DACL_SECURITY_INFORMATION, GetTokenInformation, PSECURITY_DESCRIPTOR, SECURITY_ATTRIBUTES, + TOKEN_QUERY, TOKEN_USER, TokenUser, +}; +use windows_sys::Win32::Storage::FileSystem::{CREATE_ALWAYS, CreateFileW, FILE_ATTRIBUTE_NORMAL}; +use windows_sys::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken}; + +const ATTRIBUTES_SIZE: u32 = size_of::() as u32; + +pub fn current_user_sid() -> io::Result { + let mut token: HANDLE = ptr::null_mut(); + if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) } == 0 { + return Err(io::Error::last_os_error()); + } + + let mut wanted = 0_u32; + unsafe { GetTokenInformation(token, TokenUser, ptr::null_mut(), 0, &mut wanted) }; + if wanted == 0 { + let failure = io::Error::last_os_error(); + unsafe { CloseHandle(token) }; + return Err(failure); + } + + let mut buffer = vec![0_u8; wanted as usize]; + let read = unsafe { + GetTokenInformation( + token, + TokenUser, + buffer.as_mut_ptr().cast(), + wanted, + &mut wanted, + ) + }; + let failure = io::Error::last_os_error(); + unsafe { CloseHandle(token) }; + if read == 0 { + return Err(failure); + } + + let mut text: *mut u16 = ptr::null_mut(); + let converted = unsafe { + let user = buffer.as_ptr().cast::(); + ConvertSidToStringSidW((*user).User.Sid, &mut text) + }; + if converted == 0 { + return Err(io::Error::last_os_error()); + } + Ok(unsafe { take_wide(text) }) +} + +pub fn create_with_dacl(path: &Path, dacl: &str) -> io::Result { + let mut descriptor: PSECURITY_DESCRIPTOR = ptr::null_mut(); + let wide_dacl = wide(OsStr::new(dacl)); + let built = unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + wide_dacl.as_ptr(), + SDDL_REVISION_1, + &mut descriptor, + ptr::null_mut(), + ) + }; + if built == 0 { + return Err(io::Error::last_os_error()); + } + + let attributes = SECURITY_ATTRIBUTES { + nLength: ATTRIBUTES_SIZE, + lpSecurityDescriptor: descriptor, + bInheritHandle: 0, + }; + let wide_path = wide(path.as_os_str()); + let handle = unsafe { + CreateFileW( + wide_path.as_ptr(), + GENERIC_WRITE, + 0, + &attributes, + CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, + ptr::null_mut(), + ) + }; + let failure = io::Error::last_os_error(); + unsafe { LocalFree(descriptor.cast()) }; + + if handle.is_null() || handle == INVALID_HANDLE_VALUE { + return Err(failure); + } + Ok(unsafe { File::from_raw_handle(handle.cast()) }) +} + +pub fn dacl_of(path: &Path) -> io::Result { + let wide_path = wide(path.as_os_str()); + let mut descriptor: PSECURITY_DESCRIPTOR = ptr::null_mut(); + let status = unsafe { + GetNamedSecurityInfoW( + wide_path.as_ptr(), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + &mut descriptor, + ) + }; + if status != 0 { + return Err(io::Error::from_raw_os_error(status as i32)); + } + + let mut text: *mut u16 = ptr::null_mut(); + let mut length = 0_u32; + let converted = unsafe { + ConvertSecurityDescriptorToStringSecurityDescriptorW( + descriptor, + SDDL_REVISION_1, + DACL_SECURITY_INFORMATION, + &mut text, + &mut length, + ) + }; + let failure = io::Error::last_os_error(); + unsafe { LocalFree(descriptor.cast()) }; + + if converted == 0 { + return Err(failure); + } + Ok(unsafe { take_wide(text) }) +} + +fn wide(value: &OsStr) -> Vec { + value.encode_wide().chain(std::iter::once(0)).collect() +} + +unsafe fn take_wide(text: *mut u16) -> String { + let mut length = 0_usize; + while unsafe { *text.add(length) } != 0 { + length = length.saturating_add(1); + } + let value = OsString::from_wide(unsafe { std::slice::from_raw_parts(text, length) }) + .to_string_lossy() + .into_owned(); + unsafe { LocalFree(text.cast()) }; + value +} From d3036136ace3f116c74025aa0d9f9b4804d9dcf6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 02:03:20 +0000 Subject: [PATCH 13/16] ci: run the suite on a real Windows machine Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- .github/workflows/ci.yml | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 379c1c4..0079837 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,37 @@ jobs: # Windows user actually runs, so link it. - run: cargo build --target x86_64-pc-windows-gnu -p tapline-cli + # The cross-check above type-checks Windows code but never runs it, and the + # token store now calls the Win32 security APIs by hand. Unsafe FFI nobody + # executes is unsafe FFI nobody has checked, so this runs the suite on a real + # Windows machine. GitHub's Windows runners are free on a public repository, + # and this is the only job that cannot fall back to the self-hosted box -- + # those are Linux. + windows-native: + name: Windows tests + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@stable + with: + components: clippy + - uses: Swatinem/rust-cache@v2 + + # Creating a symbolic link on Windows needs a privilege an ordinary + # account does not have, and tapline creates them when a depot asks for + # them. Developer Mode is the supported way to grant it without elevating, + # and without it the symlink tests cannot run at all. + - name: Turn on Developer Mode, for symlinks + run: | + reg add "HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock" /t REG_DWORD /f /v AllowDevelopmentWithoutDevLicense /d 1 + + # Default features, not --all-features: the `keyring` feature is pinned to + # the secret-service backend, which is Linux's. The Linux job covers it. + - run: cargo clippy --workspace --all-targets -- -D warnings + - run: cargo test --workspace + # The dependency floor is a floor: this fails on a new licence, a yanked crate, # an advisory, or anything from the deny list in deny.toml (openssl, hyper, # reqwest, prost, a C build). From 99dcb3171d65ad6330f25cf4775b669df16c9712 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 02:11:10 +0000 Subject: [PATCH 14/16] fix(auth): match the owner by SID, not by the text Windows hands back The Windows job caught this on its first run: every token-store test failed with `Shared { holder: "LA" }`. We write the owner's numeric SID into the DACL, but a descriptor read back from a file renders each well-known SID as its two-letter SDDL alias, so the account that wrote S-1-5-21-...-500 reads back as `LA` and the text comparison refused it its own file. A user running as the built-in Administrator would have hit the same wall. `shared_with` now takes the owner comparison as a parameter, since only something that can resolve both forms can say they are one account. On Windows that resolves each grantee with ConvertStringSidToSidW and asks EqualSid; a grantee that will not resolve reads as somebody else, which refuses the file rather than trusting it. The parsing that the eleven portable tests cover is unchanged, and a twelfth now pins the alias case. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- crates/tapline-auth/src/sddl.rs | 68 +++++++++++++++++++------- crates/tapline-auth/src/store.rs | 9 +++- crates/tapline-auth/src/windows_acl.rs | 46 +++++++++++++++-- 3 files changed, 100 insertions(+), 23 deletions(-) diff --git a/crates/tapline-auth/src/sddl.rs b/crates/tapline-auth/src/sddl.rs index 28f061a..c28d019 100644 --- a/crates/tapline-auth/src/sddl.rs +++ b/crates/tapline-auth/src/sddl.rs @@ -11,7 +11,16 @@ pub fn private_dacl(owner: &str) -> String { format!("D:P(A;;FA;;;{owner})") } -pub fn shared_with(dacl: &str, owner: &str) -> Option { +/// Names the first account other than the owner that this descriptor grants +/// access to, or `None` if the file is the owner's alone. +/// +/// `ours` answers whether one grantee is the owner. It is a parameter rather +/// than a string comparison because Windows does not hand a SID back the way it +/// was written: a descriptor read from a file renders every well-known SID as +/// its two-letter SDDL alias, so the account that wrote `S-1-5-21-...-500` reads +/// back as `LA`. Only something that can resolve both forms to real SIDs can say +/// they are one account, and that is Windows. +pub fn shared_with(dacl: &str, ours: impl Fn(&str) -> bool) -> Option { let Some(body) = dacl_body(dacl) else { return Some("everyone".to_owned()); }; @@ -28,15 +37,15 @@ pub fn shared_with(dacl: &str, owner: &str) -> Option { continue; }; let holder = holder.trim(); - if !is_ours(holder, owner) { + if !is_ours(holder, &ours) { return Some(holder.to_owned()); } } None } -fn is_ours(holder: &str, owner: &str) -> bool { - holder.eq_ignore_ascii_case(owner) +fn is_ours(holder: &str, ours: &impl Fn(&str) -> bool) -> bool { + ours(holder) || holder.eq_ignore_ascii_case(LOCAL_SYSTEM) || holder.eq_ignore_ascii_case(ADMINISTRATORS) || holder.eq_ignore_ascii_case(LOCAL_SYSTEM_ALIAS) @@ -67,17 +76,24 @@ mod tests { const OWNER: &str = "S-1-5-21-1111111111-2222222222-3333333333-1001"; + /// What the comparison used to be, before Windows' alias substitution made + /// a real SID comparison necessary. It is still what every case below but + /// the alias one needs. + fn owned_by(owner: &str) -> impl Fn(&str) -> bool + '_ { + move |holder| holder.eq_ignore_ascii_case(owner) + } + #[test] fn the_descriptor_we_write_grants_the_owner_alone() { let dacl = private_dacl(OWNER); assert_eq!(dacl, format!("D:P(A;;FA;;;{OWNER})")); - assert_eq!(shared_with(&dacl, OWNER), None); + assert_eq!(shared_with(&dacl, owned_by(OWNER)), None); } #[test] fn a_descriptor_with_no_dacl_at_all_is_open_to_everyone() { assert_eq!( - shared_with("O:BAG:BA", OWNER), + shared_with("O:BAG:BA", owned_by(OWNER)), Some("everyone".to_owned()), "a security descriptor with no DACL grants everyone access" ); @@ -86,7 +102,7 @@ mod tests { #[test] fn an_explicitly_absent_dacl_is_open_to_everyone() { assert_eq!( - shared_with("D:NO_ACCESS_CONTROL", OWNER), + shared_with("D:NO_ACCESS_CONTROL", owned_by(OWNER)), Some("everyone".to_owned()) ); } @@ -94,53 +110,71 @@ mod tests { #[test] fn the_accounts_that_can_reach_any_file_anyway_do_not_count() { let dacl = format!("D:P(A;;FA;;;SY)(A;;FA;;;BA)(A;;FA;;;{OWNER})"); - assert_eq!(shared_with(&dacl, OWNER), None); + assert_eq!(shared_with(&dacl, owned_by(OWNER)), None); let spelled_out = format!("D:P(A;;FA;;;S-1-5-18)(A;;FA;;;S-1-5-32-544)(A;;FA;;;{OWNER})"); - assert_eq!(shared_with(&spelled_out, OWNER), None); + assert_eq!(shared_with(&spelled_out, owned_by(OWNER)), None); } #[test] fn another_account_with_any_access_at_all_is_reported() { let dacl = format!("D:P(A;;FA;;;{OWNER})(A;;0x1200a9;;;BU)"); - assert_eq!(shared_with(&dacl, OWNER), Some("BU".to_owned())); + assert_eq!(shared_with(&dacl, owned_by(OWNER)), Some("BU".to_owned())); } #[test] fn everyone_is_reported_even_when_the_owner_is_listed_first() { let dacl = format!("D:AI(A;;FA;;;{OWNER})(A;ID;FA;;;WD)"); - assert_eq!(shared_with(&dacl, OWNER), Some("WD".to_owned())); + assert_eq!(shared_with(&dacl, owned_by(OWNER)), Some("WD".to_owned())); } #[test] fn a_denial_of_someone_else_is_not_a_grant_to_them() { let dacl = format!("D:P(D;;FA;;;WD)(A;;FA;;;{OWNER})"); - assert_eq!(shared_with(&dacl, OWNER), None); + assert_eq!(shared_with(&dacl, owned_by(OWNER)), None); } #[test] fn an_owner_sid_matches_whatever_case_windows_hands_back() { let dacl = private_dacl(&OWNER.to_lowercase()); - assert_eq!(shared_with(&dacl, OWNER), None); + assert_eq!(shared_with(&dacl, owned_by(OWNER)), None); } #[test] fn a_system_acl_after_the_dacl_is_not_read_as_a_grant() { let dacl = format!("D:P(A;;FA;;;{OWNER})S:AI(AU;SAFA;FA;;;WD)"); - assert_eq!(shared_with(&dacl, OWNER), None); + assert_eq!(shared_with(&dacl, owned_by(OWNER)), None); } #[test] fn an_owner_and_group_prefix_does_not_hide_the_dacl() { let dacl = format!("O:{OWNER}G:BAD:P(A;;FA;;;{OWNER})"); - assert_eq!(shared_with(&dacl, OWNER), None); + assert_eq!(shared_with(&dacl, owned_by(OWNER)), None); let shared = format!("O:{OWNER}G:BAD:P(A;;FA;;;{OWNER})(A;;FA;;;WD)"); - assert_eq!(shared_with(&shared, OWNER), Some("WD".to_owned())); + assert_eq!(shared_with(&shared, owned_by(OWNER)), Some("WD".to_owned())); + } + + #[test] + fn an_alias_windows_substituted_for_our_own_sid_is_still_us() { + // The regression the Windows job caught: we write the owner's numeric + // SID, Windows reads it back as `LA`, and comparing the text refuses + // the owner their own file. + let dacl = "D:P(A;;FA;;;LA)"; + assert_eq!( + shared_with(dacl, owned_by(OWNER)), + Some("LA".to_owned()), + "text alone cannot tell that an alias is the owner" + ); + assert_eq!( + shared_with(dacl, |holder| holder == "LA"), + None, + "a caller that can resolve the alias is believed" + ); } #[test] fn an_empty_dacl_grants_nobody_anything() { - assert_eq!(shared_with("D:P", OWNER), None); + assert_eq!(shared_with("D:P", owned_by(OWNER)), None); } } diff --git a/crates/tapline-auth/src/store.rs b/crates/tapline-auth/src/store.rs index b8673c5..1b130ce 100644 --- a/crates/tapline-auth/src/store.rs +++ b/crates/tapline-auth/src/store.rs @@ -321,7 +321,10 @@ fn check_permissions(path: &Path) -> Result<(), TokenStoreError> { let dacl = crate::windows_acl::dacl_of(path).map_err(|e| TokenStoreError::Backend(e.to_string()))?; - match crate::sddl::shared_with(&dacl, &owner) { + let shared = crate::sddl::shared_with(&dacl, |grantee| { + crate::windows_acl::same_account(grantee, &owner) + }); + match shared { Some(holder) => Err(TokenStoreError::Shared { holder }), None => Ok(()), } @@ -498,7 +501,9 @@ mod tests { let owner = crate::windows_acl::current_user_sid().expect("our own sid"); let dacl = crate::windows_acl::dacl_of(&path).expect("read the dacl back"); assert_eq!( - crate::sddl::shared_with(&dacl, &owner), + crate::sddl::shared_with(&dacl, |grantee| crate::windows_acl::same_account( + grantee, &owner + )), None, "a freshly written token file is reachable by someone else: {dacl}" ); diff --git a/crates/tapline-auth/src/windows_acl.rs b/crates/tapline-auth/src/windows_acl.rs index 205c0bf..d16f3f1 100644 --- a/crates/tapline-auth/src/windows_acl.rs +++ b/crates/tapline-auth/src/windows_acl.rs @@ -13,12 +13,12 @@ use windows_sys::Win32::Foundation::{ }; use windows_sys::Win32::Security::Authorization::{ ConvertSecurityDescriptorToStringSecurityDescriptorW, ConvertSidToStringSidW, - ConvertStringSecurityDescriptorToSecurityDescriptorW, GetNamedSecurityInfoW, SDDL_REVISION_1, - SE_FILE_OBJECT, + ConvertStringSecurityDescriptorToSecurityDescriptorW, ConvertStringSidToSidW, + GetNamedSecurityInfoW, SDDL_REVISION_1, SE_FILE_OBJECT, }; use windows_sys::Win32::Security::{ - DACL_SECURITY_INFORMATION, GetTokenInformation, PSECURITY_DESCRIPTOR, SECURITY_ATTRIBUTES, - TOKEN_QUERY, TOKEN_USER, TokenUser, + DACL_SECURITY_INFORMATION, EqualSid, GetTokenInformation, PSECURITY_DESCRIPTOR, PSID, + SECURITY_ATTRIBUTES, TOKEN_QUERY, TOKEN_USER, TokenUser, }; use windows_sys::Win32::Storage::FileSystem::{CREATE_ALWAYS, CreateFileW, FILE_ATTRIBUTE_NORMAL}; use windows_sys::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken}; @@ -146,6 +146,44 @@ pub fn dacl_of(path: &Path) -> io::Result { Ok(unsafe { take_wide(text) }) } +/// Whether an SDDL grantee names the same account as `owner`. +/// +/// Windows does not render a SID back the way it was written. A descriptor read +/// from a file substitutes the two-letter alias for every SID that has one, so +/// the local administrator writes `S-1-5-21-...-500` and reads back `LA`, and +/// the built-in accounts likewise. Comparing the text refuses an account its own +/// file, so both sides are resolved to real SIDs and Windows is asked whether +/// they match. A grantee that will not resolve is a grantee we cannot vouch for, +/// and reads as somebody else. +pub fn same_account(grantee: &str, owner: &str) -> bool { + if grantee.eq_ignore_ascii_case(owner) { + return true; + } + let (Some(theirs), Some(ours)) = (sid_of(grantee), sid_of(owner)) else { + return false; + }; + unsafe { EqualSid(theirs.0, ours.0) != 0 } +} + +/// A SID Windows allocated for us, freed when it goes out of scope. +struct Sid(PSID); + +impl Drop for Sid { + fn drop(&mut self) { + unsafe { LocalFree(self.0.cast()) }; + } +} + +fn sid_of(text: &str) -> Option { + let wide_text = wide(OsStr::new(text)); + let mut sid: PSID = ptr::null_mut(); + let converted = unsafe { ConvertStringSidToSidW(wide_text.as_ptr(), &mut sid) }; + if converted == 0 { + return None; + } + Some(Sid(sid)) +} + fn wide(value: &OsStr) -> Vec { value.encode_wide().chain(std::iter::once(0)).collect() } From c2c36e90d0ff4a3857397def2b423a086d341372 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 02:13:11 +0000 Subject: [PATCH 15/16] fix(auth): copy the token header out before reading it `GetTokenInformation` writes a TOKEN_USER into a byte buffer, and a Vec is aligned to one byte. Reading it as a TOKEN_USER where it lies is undefined whenever the allocation happens not to be aligned, which is not something the language promises either way. `read_unaligned` copies the header out first; the SID stays where it is, inside a buffer that outlives the call using it. The public filesystem and ACL helpers also gained the documentation they should have had. It is worth writing because the two implementations of each one differ in ways a caller has to know about: a Windows positional read can stop short, a symbolic link has two kinds there, and a mode reduces to the read-only flag. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- crates/tapline-auth/src/sddl.rs | 2 ++ crates/tapline-auth/src/windows_acl.rs | 18 +++++++++++++-- crates/tapline-fs/src/file.rs | 32 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/crates/tapline-auth/src/sddl.rs b/crates/tapline-auth/src/sddl.rs index c28d019..3447972 100644 --- a/crates/tapline-auth/src/sddl.rs +++ b/crates/tapline-auth/src/sddl.rs @@ -7,6 +7,8 @@ const ADMINISTRATORS_ALIAS: &str = "BA"; const ALLOW_ACE_TYPES: [&str; 4] = ["A", "OA", "XA", "ZA"]; +/// An access-control list granting `owner` full control and naming nobody else, +/// protected so that nothing is inherited from the directory above it. pub fn private_dacl(owner: &str) -> String { format!("D:P(A;;FA;;;{owner})") } diff --git a/crates/tapline-auth/src/windows_acl.rs b/crates/tapline-auth/src/windows_acl.rs index d16f3f1..b296888 100644 --- a/crates/tapline-auth/src/windows_acl.rs +++ b/crates/tapline-auth/src/windows_acl.rs @@ -25,6 +25,8 @@ use windows_sys::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken} const ATTRIBUTES_SIZE: u32 = size_of::() as u32; +/// The SID of the account this process is running as, in the text form SDDL +/// uses. pub fn current_user_sid() -> io::Result { let mut token: HANDLE = ptr::null_mut(); if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) } == 0 { @@ -55,10 +57,15 @@ pub fn current_user_sid() -> io::Result { return Err(failure); } + // Windows wrote a TOKEN_USER into a byte buffer, and a Vec is aligned to + // one byte. Reading it as a TOKEN_USER in place would be undefined whenever + // the allocation happened to land unaligned, so copy the header out first. + // The SID it points at stays where it is, inside `buffer`, which outlives + // the call below. let mut text: *mut u16 = ptr::null_mut(); let converted = unsafe { - let user = buffer.as_ptr().cast::(); - ConvertSidToStringSidW((*user).User.Sid, &mut text) + let user = ptr::read_unaligned(buffer.as_ptr().cast::()); + ConvertSidToStringSidW(user.User.Sid, &mut text) }; if converted == 0 { return Err(io::Error::last_os_error()); @@ -66,6 +73,12 @@ pub fn current_user_sid() -> io::Result { Ok(unsafe { take_wide(text) }) } +/// Creates (or replaces) a file carrying `dacl`, an SDDL access-control list, +/// from the moment it exists. +/// +/// Setting the list afterwards would leave a window in which the file is +/// readable by whoever the parent directory says, so it is passed to the +/// creation call instead. pub fn create_with_dacl(path: &Path, dacl: &str) -> io::Result { let mut descriptor: PSECURITY_DESCRIPTOR = ptr::null_mut(); let wide_dacl = wide(OsStr::new(dacl)); @@ -107,6 +120,7 @@ pub fn create_with_dacl(path: &Path, dacl: &str) -> io::Result { Ok(unsafe { File::from_raw_handle(handle.cast()) }) } +/// Reads back the access-control list on `path` as SDDL. pub fn dacl_of(path: &Path) -> io::Result { let wide_path = wide(path.as_os_str()); let mut descriptor: PSECURITY_DESCRIPTOR = ptr::null_mut(); diff --git a/crates/tapline-fs/src/file.rs b/crates/tapline-fs/src/file.rs index ebd4033..7705bc7 100644 --- a/crates/tapline-fs/src/file.rs +++ b/crates/tapline-fs/src/file.rs @@ -2,6 +2,8 @@ use std::fs::File; use std::io; use std::path::Path; +/// Fills `buffer` from `offset` without moving the file's cursor, so several +/// threads can read one handle at once. #[cfg(unix)] pub fn read_exact_at(file: &File, buffer: &mut [u8], offset: u64) -> io::Result<()> { use std::os::unix::fs::FileExt; @@ -9,6 +11,12 @@ pub fn read_exact_at(file: &File, buffer: &mut [u8], offset: u64) -> io::Result< file.read_exact_at(buffer, offset) } +/// Fills `buffer` from `offset` without moving the file's cursor, so several +/// threads can read one handle at once. +/// +/// Windows' positional read may stop short of what was asked for, where the +/// unix one either fills the buffer or fails, so this loops until it is full +/// and reports a file that ended early as `UnexpectedEof`. #[cfg(windows)] pub fn read_exact_at(file: &File, buffer: &mut [u8], offset: u64) -> io::Result<()> { use std::os::windows::fs::FileExt; @@ -34,6 +42,8 @@ pub fn read_exact_at(file: &File, buffer: &mut [u8], offset: u64) -> io::Result< Ok(()) } +/// Writes `data` at `offset` without moving the file's cursor, so several +/// threads can write one handle at once. #[cfg(unix)] pub fn write_all_at(file: &File, data: &[u8], offset: u64) -> io::Result<()> { use std::os::unix::fs::FileExt; @@ -41,6 +51,11 @@ pub fn write_all_at(file: &File, data: &[u8], offset: u64) -> io::Result<()> { file.write_all_at(data, offset) } +/// Writes `data` at `offset` without moving the file's cursor, so several +/// threads can write one handle at once. +/// +/// Windows' positional write may take less than it was given, so this loops +/// until all of `data` has landed. #[cfg(windows)] pub fn write_all_at(file: &File, data: &[u8], offset: u64) -> io::Result<()> { use std::os::windows::fs::FileExt; @@ -66,11 +81,17 @@ pub fn write_all_at(file: &File, data: &[u8], offset: u64) -> io::Result<()> { Ok(()) } +/// Links `link` to `target`, where `target` is read relative to `link`. #[cfg(unix)] pub fn symlink(target: &Path, link: &Path) -> io::Result<()> { std::os::unix::fs::symlink(target, link) } +/// Links `link` to `target`, where `target` is read relative to `link`. +/// +/// Windows has two kinds of symbolic link and picks the wrong one silently, so +/// this resolves the target against the link's own directory to decide which to +/// create. Creating either needs Developer Mode or the privilege to do it. #[cfg(windows)] pub fn symlink(target: &Path, link: &Path) -> io::Result<()> { let points_at_a_directory = link @@ -85,6 +106,11 @@ pub fn symlink(target: &Path, link: &Path) -> io::Result<()> { } } +/// Removes whatever is at `path`, and succeeds if nothing is. +/// +/// A directory symlink is removed as a directory, which is what Windows wants +/// and what unix does not mind; the link is removed either way, never the thing +/// it points at. pub fn remove_existing(path: &Path) -> io::Result<()> { let file_type = match std::fs::symlink_metadata(path) { Ok(metadata) => metadata.file_type(), @@ -114,6 +140,7 @@ fn names_a_directory(file_type: &std::fs::FileType) -> bool { file_type.is_dir() || file_type.is_symlink_dir() } +/// Applies a unix mode to `path`. #[cfg(unix)] pub fn set_mode(path: &Path, mode: u32) -> io::Result<()> { use std::os::unix::fs::PermissionsExt; @@ -123,6 +150,11 @@ pub fn set_mode(path: &Path, mode: u32) -> io::Result<()> { std::fs::set_permissions(path, permissions) } +/// Applies a unix mode to `path`, as far as Windows has anywhere to put one. +/// +/// Windows has no permission bits on a file in this sense, only a read-only +/// flag, so the write bit is the part that survives: a mode with none sets it, +/// a mode with one clears it. The rest is carried by the file's ACL instead. #[cfg(windows)] pub fn set_mode(path: &Path, mode: u32) -> io::Result<()> { let mut permissions = std::fs::metadata(path)?.permissions(); From 9ef7bb71921f97c3b9f7ba72b383d8e55277f9aa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 02:16:26 +0000 Subject: [PATCH 16/16] fix(ci): keep the working tree LF, so the fixtures survive a Windows clone The native Windows job failed on `steamcmds_own_record_round_trips_byte_for_byte`. Nothing was wrong with the code: git for Windows converts line endings on checkout by default, so the captured appmanifest arrived with CRLF while the writer emits LF, the way steamcmd itself writes the file on every platform. `.gitattributes` pins the working tree to LF everywhere, and marks the files captured from other programs as binary so that nothing rewrites them at all. `git add --renormalize .` changes no tracked file, so this only decides what a clone gets, never what is stored. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DNHjLfzv2AYb2c3dsnx6N4 --- .gitattributes | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..2f347d3 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,12 @@ +# Git for Windows converts line endings on checkout by default, which is fine +# for source but not for a file this repository compares byte for byte. Pinning +# the working tree to LF everywhere keeps a Windows clone identical to a Linux +# one, and keeps the diff of a file edited on either the change that was made. +* text=auto eol=lf + +# Captured verbatim from other programs and compared against what we write, so +# nothing may rewrite them -- not the committed bytes, not a checkout. Marking +# them binary is what says that, whatever a clone is configured to do. +*.acf -text +*.bin -text +*.txtpb -text