mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-07-27 01:22:16 +00:00
refactor!: remove speculative Win32 private-file layer from core, add real Windows/Linux test lanes (#106)
This commit is contained in:
parent
3f3d69a863
commit
8ad66b8f21
24 changed files with 334 additions and 1235 deletions
65
.github/workflows/ci.yml
vendored
65
.github/workflows/ci.yml
vendored
|
|
@ -195,6 +195,71 @@ jobs:
|
|||
- name: NPM wrapper smoke
|
||||
run: scripts/ci-npm-wrapper-smoke.sh
|
||||
|
||||
test-linux:
|
||||
name: Test (Linux)
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 20
|
||||
permissions:
|
||||
contents: read
|
||||
steps:
|
||||
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
|
||||
|
||||
- name: Install Rust toolchain
|
||||
run: rustup show
|
||||
|
||||
- name: Cache cargo registry
|
||||
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
|
||||
with:
|
||||
path: |
|
||||
~/.cargo/registry/index/
|
||||
~/.cargo/registry/cache/
|
||||
~/.cargo/git/db/
|
||||
key: ${{ runner.os }}-cargo-${{ hashFiles('Cargo.lock', 'Cargo.toml', 'crates/**/Cargo.toml', 'src/Cargo.toml') }}
|
||||
restore-keys: ${{ runner.os }}-cargo-
|
||||
|
||||
- name: Core and Linux unit tests
|
||||
run: scripts/cargo-test-isolated-home.sh test --locked -p agent-desktop-core -p agent-desktop-linux --lib
|
||||
|
||||
test-windows:
|
||||
name: Test (Windows)
|
||||
runs-on: windows-latest
|
||||
timeout-minutes: 20
|
||||
permissions:
|
||||
contents: read
|
||||
steps:
|
||||
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
|
||||
|
||||
- name: Install Rust toolchain
|
||||
run: rustup show
|
||||
|
||||
- name: Cache cargo registry
|
||||
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
|
||||
with:
|
||||
path: |
|
||||
~/.cargo/registry/index/
|
||||
~/.cargo/registry/cache/
|
||||
~/.cargo/git/db/
|
||||
key: ${{ runner.os }}-cargo-${{ hashFiles('Cargo.lock', 'Cargo.toml', 'crates/**/Cargo.toml', 'src/Cargo.toml') }}
|
||||
restore-keys: ${{ runner.os }}-cargo-
|
||||
|
||||
# Core resolves its data dir via HOME then USERPROFILE (crates/core/src/refs.rs
|
||||
# fn home_dir), so pointing HOME at a fresh runner-temp directory is enough to
|
||||
# keep tests off the real profile — the bash isolation script the macOS/Linux
|
||||
# lanes use doesn't apply here since this step runs under pwsh. CARGO_HOME and
|
||||
# RUSTUP_HOME must be resolved from the real USERPROFILE first (rustup/cargo
|
||||
# default them to %USERPROFILE%\.rustup and %USERPROFILE%\.cargo) so repointing
|
||||
# USERPROFILE at the empty test home doesn't strand the installed toolchain.
|
||||
- name: Core and Windows unit tests (isolated HOME)
|
||||
shell: pwsh
|
||||
run: |
|
||||
$env:CARGO_HOME = if ($env:CARGO_HOME) { $env:CARGO_HOME } else { Join-Path $env:USERPROFILE '.cargo' }
|
||||
$env:RUSTUP_HOME = if ($env:RUSTUP_HOME) { $env:RUSTUP_HOME } else { Join-Path $env:USERPROFILE '.rustup' }
|
||||
$testHome = Join-Path $env:RUNNER_TEMP ("agent-desktop-test-home-" + [guid]::NewGuid())
|
||||
New-Item -ItemType Directory -Path $testHome | Out-Null
|
||||
$env:HOME = $testHome
|
||||
$env:USERPROFILE = $testHome
|
||||
cargo test --locked -p agent-desktop-core -p agent-desktop-windows --lib
|
||||
|
||||
ffi-python-smoke:
|
||||
name: FFI Python Smoke
|
||||
runs-on: macos-latest
|
||||
|
|
|
|||
17
CLAUDE.md
17
CLAUDE.md
|
|
@ -7,10 +7,14 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
|
|||
```bash
|
||||
cargo build # Debug build
|
||||
cargo build --release # Release build (<15MB target)
|
||||
cargo test --lib --workspace # Run all unit tests
|
||||
cargo test --workspace # Run every test (lib, binary, and integration targets)
|
||||
cargo test --lib --workspace # Library unit tests only; SKIPS the agent-desktop binary crate
|
||||
cargo test -p agent-desktop # Binary crate tests (CLI contract, dispatch, batch, policy)
|
||||
cargo test --lib -p agent-desktop-core # Test core crate only
|
||||
cargo test --lib -p agent-desktop-macos # Test macOS crate only
|
||||
cargo test test_name # Run a single test by name
|
||||
cargo check -p agent-desktop-core --all-targets --target x86_64-pc-windows-msvc # Core must cross-compile
|
||||
cargo check -p agent-desktop-core --all-targets --target x86_64-unknown-linux-gnu
|
||||
cargo clippy --all-targets -- -D warnings # Lint (must pass, zero warnings)
|
||||
cargo fmt --all -- --check # Format check
|
||||
cargo fmt --all # Auto-format
|
||||
|
|
@ -375,11 +379,22 @@ for the actionability preflight (`get_live_*`), and `is_protected_process`
|
|||
## CI Requirements
|
||||
|
||||
- GitHub Actions macOS runner executes full test suite on every PR
|
||||
- Windows and Linux runners execute the core unit tests plus their native platform crate on every PR
|
||||
- `cargo tree -p agent-desktop-core` must not contain platform crate names
|
||||
- `cargo clippy --all-targets -- -D warnings`
|
||||
- `cargo test --workspace`
|
||||
- Binary size check: fail if release binary exceeds 15MB
|
||||
|
||||
### Core platform-conditional code
|
||||
|
||||
`agent-desktop-core` must stay executable on every supported OS. A `#[cfg]` branch in core
|
||||
that no CI lane runs is a hypothesis, not shipped code — either add a lane that executes
|
||||
it or keep it out of core. Platform hardening belongs behind `PlatformAdapter` or in the
|
||||
platform crate, written on that platform against a lane that runs it. Core carried 1,062
|
||||
LOC of never-executed Win32 file I/O into v0.5.0; it failed 225 of 940 tests on first
|
||||
contact with Windows and was deleted. See
|
||||
`docs/solutions/best-practices/never-ship-platform-code-that-ci-cannot-execute.md`.
|
||||
|
||||
## Commands
|
||||
|
||||
58 commands spanning App/Window, Observation, Interaction, Scroll, Keyboard,
|
||||
|
|
|
|||
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -39,7 +39,6 @@ dependencies = [
|
|||
"smallvec",
|
||||
"thiserror",
|
||||
"tracing",
|
||||
"windows-sys",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
|
|
|||
|
|
@ -21,7 +21,6 @@ rustc-hash = "2.1"
|
|||
libc = "0.2"
|
||||
smallvec = { version = "1.13", features = ["serde", "union"] }
|
||||
agent-desktop-core = { path = "crates/core" }
|
||||
windows-sys = { version = "0.61.2" }
|
||||
|
||||
[workspace.lints.rust]
|
||||
unsafe_op_in_unsafe_fn = "warn"
|
||||
|
|
|
|||
|
|
@ -15,15 +15,5 @@ base64.workspace = true
|
|||
libc.workspace = true
|
||||
smallvec.workspace = true
|
||||
|
||||
[target.'cfg(windows)'.dependencies]
|
||||
windows-sys = { workspace = true, features = [
|
||||
"Win32_Foundation",
|
||||
"Win32_Security",
|
||||
"Win32_Security_Authorization",
|
||||
"Win32_Storage_FileSystem",
|
||||
"Win32_System_SystemServices",
|
||||
"Win32_System_Threading",
|
||||
] }
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
|
|
|||
|
|
@ -15,11 +15,13 @@ pub trait SystemOps: Send + Sync {
|
|||
Err(AdapterError::not_supported("acquire_interaction_lease"))
|
||||
}
|
||||
|
||||
/// An adapter that never overrides this has not probed permissions at all,
|
||||
/// which is not the same fact as a user denying one. `Unknown` routes through
|
||||
/// `unknown_accessibility_means_unsupported` to `PLATFORM_NOT_SUPPORTED`
|
||||
/// rather than the misleading `PERM_DENIED`.
|
||||
fn permission_report(&self, _deadline: Deadline) -> Result<PermissionReport, AdapterError> {
|
||||
Ok(PermissionReport {
|
||||
accessibility: PermissionState::Denied {
|
||||
suggestion: "Platform adapter not available".into(),
|
||||
},
|
||||
accessibility: PermissionState::Unknown,
|
||||
screen_recording: PermissionState::Unknown,
|
||||
automation: PermissionState::NotRequired,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -18,6 +18,17 @@ fn default_surfaces_fail_closed() {
|
|||
assert!(DefaultOnly.supported_surfaces().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_permission_report_is_unknown_not_denied() {
|
||||
let report = DefaultOnly
|
||||
.permission_report(crate::Deadline::standard().unwrap())
|
||||
.expect("default permission report must not error");
|
||||
|
||||
assert_eq!(report.accessibility, crate::PermissionState::Unknown);
|
||||
assert!(!report.accessibility_denied());
|
||||
assert!(DefaultOnly.unknown_accessibility_means_unsupported());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_open_session_is_explicitly_unsupported() {
|
||||
let affinity = crate::SessionAffinity {
|
||||
|
|
|
|||
|
|
@ -30,7 +30,9 @@ fn get_full_inlines_references() {
|
|||
.expect("get full");
|
||||
let content = v["content"].as_str().expect("string");
|
||||
assert!(content.contains("--- references/workflows.md ---"));
|
||||
assert!(content.contains("--- references/macos.md ---"));
|
||||
if cfg!(target_os = "macos") {
|
||||
assert!(content.contains("--- references/macos.md ---"));
|
||||
}
|
||||
assert!(content.contains("@s8f3k2p9:e1"));
|
||||
assert!(content.contains("session start` does not activate later processes"));
|
||||
assert!(content.contains("session-owned ref still requires the same `--session`"));
|
||||
|
|
|
|||
|
|
@ -1,5 +1,4 @@
|
|||
use std::fs::File;
|
||||
#[cfg(not(windows))]
|
||||
use std::fs::OpenOptions;
|
||||
use std::hash::{BuildHasher, RandomState};
|
||||
use std::io::{Read, Write};
|
||||
|
|
@ -8,44 +7,29 @@ use std::sync::atomic::{AtomicU64, Ordering};
|
|||
|
||||
static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
#[cfg(windows)]
|
||||
#[path = "private_file_windows.rs"]
|
||||
pub(crate) mod windows;
|
||||
|
||||
pub(crate) fn open_private_lock(path: &Path, create: bool) -> std::io::Result<File> {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
windows::open_lock(path, create)
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let parent = path
|
||||
.parent()
|
||||
.ok_or_else(|| invalid_input("private file path has no parent"))?;
|
||||
crate::private_file_parent::ensure_private(parent)?;
|
||||
let mut options = OpenOptions::new();
|
||||
options.read(true).write(true).create(create);
|
||||
configure_unix(&mut options, 0o600);
|
||||
let file = options.open(path)?;
|
||||
validate_private_regular(&file)?;
|
||||
Ok(file)
|
||||
}
|
||||
let parent = path
|
||||
.parent()
|
||||
.ok_or_else(|| invalid_input("private file path has no parent"))?;
|
||||
crate::private_file_parent::ensure_private(parent)?;
|
||||
let mut options = OpenOptions::new();
|
||||
options.read(true).write(true).create(create);
|
||||
configure_unix(&mut options, 0o600);
|
||||
let file = options.open(path)?;
|
||||
validate_private_regular(&file)?;
|
||||
Ok(file)
|
||||
}
|
||||
|
||||
/// Read access is requested alongside append because callers lock the returned
|
||||
/// handle, and Windows `LockFileEx` fails with `ERROR_ACCESS_DENIED` on a handle
|
||||
/// opened for append only.
|
||||
pub(crate) fn open_private_append(path: &Path) -> std::io::Result<File> {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
windows::open_append(path)
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let mut options = OpenOptions::new();
|
||||
options.create(true).append(true);
|
||||
configure_unix(&mut options, 0o600);
|
||||
let file = options.open(path)?;
|
||||
validate_private_regular(&file)?;
|
||||
Ok(file)
|
||||
}
|
||||
let mut options = OpenOptions::new();
|
||||
options.read(true).create(true).append(true);
|
||||
configure_unix(&mut options, 0o600);
|
||||
let file = options.open(path)?;
|
||||
validate_private_regular(&file)?;
|
||||
Ok(file)
|
||||
}
|
||||
|
||||
pub(crate) fn read_private_bounded(path: &Path, max_bytes: u64) -> std::io::Result<Vec<u8>> {
|
||||
|
|
@ -54,9 +38,6 @@ pub(crate) fn read_private_bounded(path: &Path, max_bytes: u64) -> std::io::Resu
|
|||
}
|
||||
|
||||
pub(crate) fn read_regular_bounded(path: &Path, max_bytes: u64) -> std::io::Result<Vec<u8>> {
|
||||
#[cfg(windows)]
|
||||
let file = windows::open_regular_read(path)?;
|
||||
#[cfg(not(windows))]
|
||||
let file = {
|
||||
let mut options = OpenOptions::new();
|
||||
options.read(true);
|
||||
|
|
@ -106,7 +87,7 @@ fn write_atomic_with(
|
|||
file.sync_all()?;
|
||||
#[cfg(test)]
|
||||
crash_before_rename_if_requested(path);
|
||||
replace_atomic(&file, &temporary, path)?;
|
||||
replace_atomic(&temporary, path)?;
|
||||
validate_private_regular(&open_private_read(path)?)?;
|
||||
sync_parent(parent)
|
||||
})();
|
||||
|
|
@ -158,8 +139,6 @@ pub(crate) fn validate_private_regular(file: &File) -> std::io::Result<std::fs::
|
|||
return Err(permission_denied("private file must not be hard-linked"));
|
||||
}
|
||||
}
|
||||
#[cfg(windows)]
|
||||
windows::validate_private(file)?;
|
||||
Ok(metadata)
|
||||
}
|
||||
|
||||
|
|
@ -168,25 +147,16 @@ pub(crate) fn validate_regular(file: &File) -> std::io::Result<std::fs::Metadata
|
|||
if !metadata.is_file() {
|
||||
return Err(permission_denied("path is not a regular file"));
|
||||
}
|
||||
#[cfg(windows)]
|
||||
windows::validate_regular(file)?;
|
||||
Ok(metadata)
|
||||
}
|
||||
|
||||
fn open_private_read(path: &Path) -> std::io::Result<File> {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
windows::open_read(path)
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let mut options = OpenOptions::new();
|
||||
options.read(true);
|
||||
configure_unix(&mut options, 0);
|
||||
let file = options.open(path)?;
|
||||
validate_private_regular(&file)?;
|
||||
Ok(file)
|
||||
}
|
||||
let mut options = OpenOptions::new();
|
||||
options.read(true);
|
||||
configure_unix(&mut options, 0);
|
||||
let file = options.open(path)?;
|
||||
validate_private_regular(&file)?;
|
||||
Ok(file)
|
||||
}
|
||||
|
||||
fn read_bounded(file: File, max_bytes: u64) -> std::io::Result<Vec<u8>> {
|
||||
|
|
@ -216,26 +186,17 @@ fn create_temporary(path: &Path) -> std::io::Result<(PathBuf, File)> {
|
|||
std::time::SystemTime::now(),
|
||||
));
|
||||
let temporary = path.with_file_name(format!(".{file_name}.{nonce:016x}.tmp"));
|
||||
#[cfg(windows)]
|
||||
match windows::create_new(&temporary) {
|
||||
Ok(file) => return Ok((temporary, file)),
|
||||
let mut options = OpenOptions::new();
|
||||
options.write(true).create_new(true);
|
||||
configure_unix(&mut options, 0o600);
|
||||
match options.open(&temporary) {
|
||||
Ok(file) => {
|
||||
validate_private_regular(&file)?;
|
||||
return Ok((temporary, file));
|
||||
}
|
||||
Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue,
|
||||
Err(error) => return Err(error),
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let mut options = OpenOptions::new();
|
||||
options.write(true).create_new(true);
|
||||
configure_unix(&mut options, 0o600);
|
||||
match options.open(&temporary) {
|
||||
Ok(file) => {
|
||||
validate_private_regular(&file)?;
|
||||
return Ok((temporary, file));
|
||||
}
|
||||
Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue,
|
||||
Err(error) => return Err(error),
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(std::io::Error::new(
|
||||
std::io::ErrorKind::AlreadyExists,
|
||||
|
|
@ -270,13 +231,7 @@ fn sync_user_directory(path: &Path) -> std::io::Result<()> {
|
|||
OpenOptions::new().read(true).open(path)?.sync_all()
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn replace_atomic(file: &File, source: &Path, destination: &Path) -> std::io::Result<()> {
|
||||
windows::replace_atomic(file, source, destination)
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
fn replace_atomic(_file: &File, source: &Path, destination: &Path) -> std::io::Result<()> {
|
||||
fn replace_atomic(source: &Path, destination: &Path) -> std::io::Result<()> {
|
||||
std::fs::rename(source, destination)
|
||||
}
|
||||
|
||||
|
|
@ -288,7 +243,7 @@ fn configure_unix(options: &mut OpenOptions, mode: u32) {
|
|||
.custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW | libc::O_NONBLOCK);
|
||||
}
|
||||
|
||||
#[cfg(not(any(unix, windows)))]
|
||||
#[cfg(not(unix))]
|
||||
fn configure_unix(_options: &mut OpenOptions, _mode: u32) {}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
|
|
@ -322,12 +277,7 @@ fn validate_local_filesystem(file: &File) -> std::io::Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn validate_local_filesystem(file: &File) -> std::io::Result<()> {
|
||||
windows::validate_local(file)
|
||||
}
|
||||
|
||||
#[cfg(not(any(target_os = "macos", target_os = "linux", windows)))]
|
||||
#[cfg(not(any(target_os = "macos", target_os = "linux")))]
|
||||
fn validate_local_filesystem(_file: &File) -> std::io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -352,3 +302,7 @@ pub(super) fn permission_denied(message: &'static str) -> std::io::Error {
|
|||
#[cfg(test)]
|
||||
#[path = "private_file_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "private_file_lock_tests.rs"]
|
||||
mod lock_tests;
|
||||
|
|
|
|||
37
crates/core/src/private_file_lock_tests.rs
Normal file
37
crates/core/src/private_file_lock_tests.rs
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
use super::*;
|
||||
|
||||
fn scratch_dir(name: &str) -> PathBuf {
|
||||
let dir = std::env::temp_dir().join(format!(
|
||||
"agent-desktop-{name}-{}-{}",
|
||||
std::process::id(),
|
||||
crate::refs::new_snapshot_id()
|
||||
));
|
||||
crate::private_file_parent::ensure_private(&dir).expect("private scratch directory");
|
||||
dir
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn appended_private_files_are_lockable() {
|
||||
let path = scratch_dir("append-lock").join("trace.jsonl");
|
||||
|
||||
let file = open_private_append(&path).expect("append handle");
|
||||
file.lock()
|
||||
.expect("append handles must be lockable on every platform");
|
||||
file.unlock().expect("unlock");
|
||||
|
||||
drop(file);
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_handles_reopen_and_relock_after_close() {
|
||||
let path = scratch_dir("append-relock").join("trace.jsonl");
|
||||
|
||||
for _ in 0..2 {
|
||||
let file = open_private_append(&path).expect("append handle");
|
||||
file.lock().expect("lock");
|
||||
file.unlock().expect("unlock");
|
||||
}
|
||||
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
|
|
@ -1,53 +1,39 @@
|
|||
use std::path::Path;
|
||||
|
||||
pub(super) fn ensure_private(path: &Path) -> std::io::Result<()> {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
super::private_file::windows::ensure_private_parent(path)
|
||||
ensure_directory_path(path)?;
|
||||
let metadata = std::fs::symlink_metadata(path)?;
|
||||
if metadata.file_type().is_symlink() || !metadata.is_dir() {
|
||||
return Err(super::private_file::permission_denied(
|
||||
"private file parent must be a real directory",
|
||||
));
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
#[cfg(unix)]
|
||||
{
|
||||
ensure_directory_path(path)?;
|
||||
let metadata = std::fs::symlink_metadata(path)?;
|
||||
if metadata.file_type().is_symlink() || !metadata.is_dir() {
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
if metadata.uid() != unsafe { libc::geteuid() } {
|
||||
return Err(super::private_file::permission_denied(
|
||||
"private file parent must be a real directory",
|
||||
"private file parent is not owned by the effective user",
|
||||
));
|
||||
}
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
if metadata.uid() != unsafe { libc::geteuid() } {
|
||||
return Err(super::private_file::permission_denied(
|
||||
"private file parent is not owned by the effective user",
|
||||
));
|
||||
}
|
||||
if metadata.mode() & 0o077 != 0 {
|
||||
return Err(super::private_file::permission_denied(
|
||||
"private file parent is accessible by group or other users",
|
||||
));
|
||||
}
|
||||
if metadata.mode() & 0o077 != 0 {
|
||||
return Err(super::private_file::permission_denied(
|
||||
"private file parent is accessible by group or other users",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(super) fn ensure_user(path: &Path) -> std::io::Result<()> {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
super::private_file::windows::ensure_user_parent(path)
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
ensure_directory_path(path)?;
|
||||
let metadata = std::fs::metadata(path)?;
|
||||
if !metadata.is_dir() {
|
||||
return Err(super::private_file::permission_denied(
|
||||
"user output parent must be a directory",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
ensure_directory_path(path)?;
|
||||
let metadata = std::fs::metadata(path)?;
|
||||
if !metadata.is_dir() {
|
||||
return Err(super::private_file::permission_denied(
|
||||
"user output parent must be a directory",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
|
|
@ -96,7 +82,7 @@ fn ensure_directory_path(path: &Path) -> std::io::Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(not(any(unix, windows)))]
|
||||
#[cfg(not(unix))]
|
||||
fn ensure_directory_path(path: &Path) -> std::io::Result<()> {
|
||||
std::fs::create_dir_all(path)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,299 +0,0 @@
|
|||
use std::fs::File;
|
||||
use std::mem::size_of;
|
||||
use std::os::windows::io::AsRawHandle;
|
||||
use std::path::Path;
|
||||
use std::ptr::null;
|
||||
use windows_sys::Win32::Foundation::{
|
||||
ERROR_ALREADY_EXISTS, ERROR_INVALID_PARAMETER, GENERIC_READ, GENERIC_WRITE, GetLastError,
|
||||
HANDLE,
|
||||
};
|
||||
use windows_sys::Win32::Storage::FileSystem::{
|
||||
BY_HANDLE_FILE_INFORMATION, CREATE_NEW, CreateDirectoryW, DELETE, FILE_APPEND_DATA,
|
||||
FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_REPARSE_POINT, FILE_REMOTE_PROTOCOL_INFO,
|
||||
FILE_SHARE_READ, FILE_SHARE_WRITE, FileRemoteProtocolInfo, GetFileInformationByHandle,
|
||||
GetFileInformationByHandleEx, OPEN_ALWAYS, OPEN_EXISTING, READ_CONTROL,
|
||||
};
|
||||
|
||||
#[path = "private_file_windows_guard.rs"]
|
||||
mod guard;
|
||||
#[path = "private_file_windows_open.rs"]
|
||||
mod open;
|
||||
#[path = "private_file_windows_path.rs"]
|
||||
mod path;
|
||||
#[path = "private_file_windows_rename.rs"]
|
||||
mod rename;
|
||||
#[path = "private_file_windows_security.rs"]
|
||||
mod security;
|
||||
|
||||
use open::FileOpen;
|
||||
use security::PrivateSecurity;
|
||||
|
||||
const LEAF_SHARING: u32 = FILE_SHARE_READ | FILE_SHARE_WRITE;
|
||||
const GUARD_SHARING: u32 = FILE_SHARE_READ;
|
||||
const TEMP_ACCESS: u32 = GENERIC_WRITE | READ_CONTROL | DELETE;
|
||||
|
||||
pub(super) fn open_lock(path: &Path, create: bool) -> std::io::Result<File> {
|
||||
let creation = if create { OPEN_ALWAYS } else { OPEN_EXISTING };
|
||||
open_private(
|
||||
path,
|
||||
GENERIC_READ | GENERIC_WRITE | READ_CONTROL,
|
||||
creation,
|
||||
true,
|
||||
)
|
||||
}
|
||||
|
||||
pub(super) fn open_append(path: &Path) -> std::io::Result<File> {
|
||||
open_private(path, FILE_APPEND_DATA | READ_CONTROL, OPEN_ALWAYS, false)
|
||||
}
|
||||
|
||||
pub(super) fn open_read(path: &Path) -> std::io::Result<File> {
|
||||
open_private(path, GENERIC_READ | READ_CONTROL, OPEN_EXISTING, false)
|
||||
}
|
||||
|
||||
pub(super) fn open_regular_read(path: &Path) -> std::io::Result<File> {
|
||||
let path = path::normalized(path)?;
|
||||
path::validate_file_name(&path)?;
|
||||
with_leaf_parent(&path, false, None, || {
|
||||
FileOpen::leaf(&path, GENERIC_READ, OPEN_EXISTING, null()).execute()
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) fn create_new(path: &Path) -> std::io::Result<File> {
|
||||
open_private(path, TEMP_ACCESS, CREATE_NEW, false)
|
||||
}
|
||||
|
||||
pub(crate) fn ensure_private_parent(path: &Path) -> std::io::Result<()> {
|
||||
let path = path::normalized(path)?;
|
||||
let security = PrivateSecurity::new_directory()?;
|
||||
guard::with_ancestor_guards(&path, true, Some(&security), |guards| {
|
||||
security::validate_private_acl(guards.leaf()?)
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn ensure_user_parent(path: &Path) -> std::io::Result<()> {
|
||||
let path = path::normalized(path)?;
|
||||
let security = PrivateSecurity::new_directory()?;
|
||||
guard::with_ancestor_guards(&path, true, Some(&security), |_| Ok(()))
|
||||
}
|
||||
|
||||
pub(super) fn validate_private(file: &File) -> std::io::Result<()> {
|
||||
let info = file_information(file)?;
|
||||
if info.dwFileAttributes & (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_REPARSE_POINT) != 0 {
|
||||
return Err(super::permission_denied(
|
||||
"private path must be a regular non-reparse file",
|
||||
));
|
||||
}
|
||||
if info.nNumberOfLinks != 1 {
|
||||
return Err(super::permission_denied(
|
||||
"private file must not be hard-linked",
|
||||
));
|
||||
}
|
||||
validate_local(file)?;
|
||||
security::validate_private_acl(file)
|
||||
}
|
||||
|
||||
pub(super) fn validate_regular(file: &File) -> std::io::Result<()> {
|
||||
let info = file_information(file)?;
|
||||
if info.dwFileAttributes & (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_REPARSE_POINT) != 0 {
|
||||
return Err(super::permission_denied(
|
||||
"path must be a regular non-reparse file",
|
||||
));
|
||||
}
|
||||
validate_local(file)
|
||||
}
|
||||
|
||||
pub(super) fn validate_local(file: &File) -> std::io::Result<()> {
|
||||
let mut remote = remote_protocol_query();
|
||||
let success = unsafe {
|
||||
GetFileInformationByHandleEx(
|
||||
raw_handle(file),
|
||||
FileRemoteProtocolInfo,
|
||||
(&raw mut remote).cast(),
|
||||
size_of::<FILE_REMOTE_PROTOCOL_INFO>() as u32,
|
||||
)
|
||||
};
|
||||
let error = if success == 0 {
|
||||
unsafe { GetLastError() }
|
||||
} else {
|
||||
0
|
||||
};
|
||||
classify_locality(success != 0, error)
|
||||
}
|
||||
|
||||
fn classify_locality(success: bool, error: u32) -> std::io::Result<()> {
|
||||
if success {
|
||||
return Err(super::permission_denied(
|
||||
"network filesystems are not accepted here",
|
||||
));
|
||||
}
|
||||
if error == ERROR_INVALID_PARAMETER {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(std::io::Error::new(
|
||||
std::io::ErrorKind::Unsupported,
|
||||
"cannot verify that the Windows storage is local",
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn replace_atomic(
|
||||
source_file: &File,
|
||||
source: &Path,
|
||||
destination: &Path,
|
||||
) -> std::io::Result<()> {
|
||||
let source = path::normalized(source)?;
|
||||
let destination = path::normalized(destination)?;
|
||||
path::validate_file_name(&source)?;
|
||||
path::validate_file_name(&destination)?;
|
||||
let source_parent = parent(&source)?;
|
||||
let destination_parent = parent(&destination)?;
|
||||
if source_parent == destination_parent {
|
||||
return guard::with_ancestor_guards(source_parent, false, None, |_guards| {
|
||||
rename::replace(source_file, &destination)
|
||||
});
|
||||
}
|
||||
guard::with_ancestor_guards(source_parent, false, None, |_source_guards| {
|
||||
guard::with_ancestor_guards(destination_parent, false, None, |_destination_guards| {
|
||||
rename::replace(source_file, &destination)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn open_private(
|
||||
path: &Path,
|
||||
access: u32,
|
||||
creation: u32,
|
||||
create_parent: bool,
|
||||
) -> std::io::Result<File> {
|
||||
let path = path::normalized(path)?;
|
||||
path::validate_file_name(&path)?;
|
||||
let (directory_security, file_security) = if create_parent {
|
||||
let (directory, file) = PrivateSecurity::new_pair()?;
|
||||
(Some(directory), file)
|
||||
} else {
|
||||
(None, PrivateSecurity::new_file()?)
|
||||
};
|
||||
with_leaf_parent(&path, create_parent, directory_security.as_ref(), || {
|
||||
let file = FileOpen::leaf(&path, access, creation, file_security.attributes()).execute()?;
|
||||
validate_private(&file)?;
|
||||
Ok(file)
|
||||
})
|
||||
}
|
||||
|
||||
fn with_leaf_parent<T>(
|
||||
path: &Path,
|
||||
create_parent: bool,
|
||||
security: Option<&PrivateSecurity>,
|
||||
operation: impl FnOnce() -> std::io::Result<T>,
|
||||
) -> std::io::Result<T> {
|
||||
guard::with_ancestor_guards(parent(path)?, create_parent, security, |_guards| {
|
||||
operation()
|
||||
})
|
||||
}
|
||||
|
||||
fn create_private_directory(path: &Path, security: &PrivateSecurity) -> std::io::Result<()> {
|
||||
let path = path::wide_normalized(path)?;
|
||||
if unsafe { CreateDirectoryW(path.as_ptr(), security.attributes()) } != 0 {
|
||||
return Ok(());
|
||||
}
|
||||
let code = unsafe { GetLastError() };
|
||||
if code == ERROR_ALREADY_EXISTS {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(std::io::Error::from_raw_os_error(code as i32))
|
||||
}
|
||||
}
|
||||
|
||||
fn open_guarded_directory(path: &Path) -> std::io::Result<File> {
|
||||
FileOpen::guarded_directory(path, GENERIC_READ | READ_CONTROL, OPEN_EXISTING).execute()
|
||||
}
|
||||
|
||||
fn validate_directory(file: &File) -> std::io::Result<()> {
|
||||
let info = file_information(file)?;
|
||||
if info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == 0
|
||||
|| info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != 0
|
||||
{
|
||||
return Err(super::permission_denied(
|
||||
"private file parent must be a real non-reparse directory",
|
||||
));
|
||||
}
|
||||
validate_local(file)
|
||||
}
|
||||
|
||||
fn file_information(file: &File) -> std::io::Result<BY_HANDLE_FILE_INFORMATION> {
|
||||
let mut info = BY_HANDLE_FILE_INFORMATION::default();
|
||||
if unsafe { GetFileInformationByHandle(raw_handle(file), &raw mut info) } == 0 {
|
||||
Err(std::io::Error::last_os_error())
|
||||
} else {
|
||||
Ok(info)
|
||||
}
|
||||
}
|
||||
|
||||
fn raw_handle(file: &File) -> HANDLE {
|
||||
file.as_raw_handle()
|
||||
}
|
||||
|
||||
fn parent(path: &Path) -> std::io::Result<&Path> {
|
||||
path.parent()
|
||||
.filter(|parent| !parent.as_os_str().is_empty())
|
||||
.ok_or_else(|| invalid_input("private file path has no parent"))
|
||||
}
|
||||
|
||||
fn remote_protocol_query() -> FILE_REMOTE_PROTOCOL_INFO {
|
||||
FILE_REMOTE_PROTOCOL_INFO {
|
||||
StructureVersion: 2,
|
||||
StructureSize: size_of::<FILE_REMOTE_PROTOCOL_INFO>() as u16,
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn invalid_input(message: &'static str) -> std::io::Error {
|
||||
std::io::Error::new(std::io::ErrorKind::InvalidInput, message)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn ancestor_guards_exclude_delete_sharing() {
|
||||
assert_ne!(
|
||||
windows_sys::Win32::Storage::FileSystem::FILE_FLAG_OPEN_REPARSE_POINT,
|
||||
0
|
||||
);
|
||||
assert_eq!(LEAF_SHARING, FILE_SHARE_READ | FILE_SHARE_WRITE);
|
||||
assert_eq!(
|
||||
LEAF_SHARING & windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE,
|
||||
0
|
||||
);
|
||||
assert_eq!(GUARD_SHARING, FILE_SHARE_READ);
|
||||
assert_eq!(GUARD_SHARING & FILE_SHARE_WRITE, 0);
|
||||
assert_eq!(
|
||||
GUARD_SHARING & windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE,
|
||||
0
|
||||
);
|
||||
assert_ne!(TEMP_ACCESS & DELETE, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_protocol_query_uses_the_current_contract_shape() {
|
||||
let query = remote_protocol_query();
|
||||
|
||||
assert_eq!(query.StructureVersion, 2);
|
||||
assert_eq!(
|
||||
usize::from(query.StructureSize),
|
||||
size_of::<FILE_REMOTE_PROTOCOL_INFO>()
|
||||
);
|
||||
assert_eq!(query.Protocol, 0);
|
||||
assert_eq!(ERROR_INVALID_PARAMETER, 87);
|
||||
assert!(classify_locality(false, ERROR_INVALID_PARAMETER).is_ok());
|
||||
assert_eq!(
|
||||
classify_locality(true, 0).unwrap_err().kind(),
|
||||
std::io::ErrorKind::PermissionDenied
|
||||
);
|
||||
assert_eq!(
|
||||
classify_locality(false, 5).unwrap_err().kind(),
|
||||
std::io::ErrorKind::Unsupported
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -1,84 +0,0 @@
|
|||
use std::fs::File;
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
|
||||
use super::security::PrivateSecurity;
|
||||
|
||||
pub(super) struct AncestorGuards {
|
||||
handles: Vec<File>,
|
||||
}
|
||||
|
||||
impl AncestorGuards {
|
||||
fn acquire(
|
||||
path: &Path,
|
||||
create: bool,
|
||||
security: Option<&PrivateSecurity>,
|
||||
) -> std::io::Result<Self> {
|
||||
let absolute = super::path::normalized(path)?;
|
||||
let mut current = PathBuf::new();
|
||||
let mut handles = Vec::new();
|
||||
for component in absolute.components() {
|
||||
current.push(component);
|
||||
if matches!(component, Component::Prefix(_)) {
|
||||
continue;
|
||||
}
|
||||
let handle = match super::open_guarded_directory(¤t) {
|
||||
Ok(handle) => handle,
|
||||
Err(error) if create && error.kind() == std::io::ErrorKind::NotFound => {
|
||||
let security = security.ok_or_else(|| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::Unsupported,
|
||||
"secure Windows directory creation requires a private descriptor",
|
||||
)
|
||||
})?;
|
||||
super::create_private_directory(¤t, security)?;
|
||||
super::open_guarded_directory(¤t)?
|
||||
}
|
||||
Err(error) => return Err(error),
|
||||
};
|
||||
super::validate_directory(&handle)?;
|
||||
handles.push(handle);
|
||||
}
|
||||
if handles.is_empty() {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::PermissionDenied,
|
||||
"Windows private path has no guardable directory",
|
||||
));
|
||||
}
|
||||
Ok(Self { handles })
|
||||
}
|
||||
|
||||
pub(super) fn leaf(&self) -> std::io::Result<&File> {
|
||||
self.handles.last().ok_or_else(|| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::PermissionDenied,
|
||||
"Windows private path lost its ancestor guards",
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn with_ancestor_guards<T>(
|
||||
path: &Path,
|
||||
create: bool,
|
||||
security: Option<&PrivateSecurity>,
|
||||
operation: impl FnOnce(&AncestorGuards) -> std::io::Result<T>,
|
||||
) -> std::io::Result<T> {
|
||||
let guards = AncestorGuards::acquire(path, create, security)?;
|
||||
operation(&guards)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn lifetime_contract(guards: &AncestorGuards) -> std::io::Result<&File> {
|
||||
guards.leaf()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn operation_borrows_the_live_guard_chain() {
|
||||
let contract: for<'a> fn(&'a AncestorGuards) -> std::io::Result<&'a File> =
|
||||
lifetime_contract;
|
||||
let _ = contract;
|
||||
}
|
||||
}
|
||||
|
|
@ -1,77 +0,0 @@
|
|||
use std::fs::File;
|
||||
use std::os::windows::io::FromRawHandle;
|
||||
use std::path::Path;
|
||||
use std::ptr::null_mut;
|
||||
use windows_sys::Win32::Foundation::{HANDLE, INVALID_HANDLE_VALUE};
|
||||
use windows_sys::Win32::Security::SECURITY_ATTRIBUTES;
|
||||
use windows_sys::Win32::Storage::FileSystem::{
|
||||
CreateFileW, FILE_ATTRIBUTE_NORMAL, FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT,
|
||||
};
|
||||
|
||||
pub(super) struct FileOpen<'a> {
|
||||
path: &'a Path,
|
||||
access: u32,
|
||||
creation: u32,
|
||||
directory: bool,
|
||||
sharing: u32,
|
||||
security: *const SECURITY_ATTRIBUTES,
|
||||
}
|
||||
|
||||
impl<'a> FileOpen<'a> {
|
||||
pub(super) fn leaf(
|
||||
path: &'a Path,
|
||||
access: u32,
|
||||
creation: u32,
|
||||
security: *const SECURITY_ATTRIBUTES,
|
||||
) -> Self {
|
||||
Self {
|
||||
path,
|
||||
access,
|
||||
creation,
|
||||
directory: false,
|
||||
sharing: super::LEAF_SHARING,
|
||||
security,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn guarded_directory(path: &'a Path, access: u32, creation: u32) -> Self {
|
||||
Self {
|
||||
path,
|
||||
access,
|
||||
creation,
|
||||
directory: true,
|
||||
sharing: super::GUARD_SHARING,
|
||||
security: std::ptr::null(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn execute(self) -> std::io::Result<File> {
|
||||
let path = super::path::wide_normalized(self.path)?;
|
||||
let flags = FILE_FLAG_OPEN_REPARSE_POINT
|
||||
| if self.directory {
|
||||
FILE_FLAG_BACKUP_SEMANTICS
|
||||
} else {
|
||||
FILE_ATTRIBUTE_NORMAL
|
||||
};
|
||||
let handle = unsafe {
|
||||
CreateFileW(
|
||||
path.as_ptr(),
|
||||
self.access,
|
||||
self.sharing,
|
||||
self.security,
|
||||
self.creation,
|
||||
flags,
|
||||
null_mut(),
|
||||
)
|
||||
};
|
||||
file_from_handle(handle)
|
||||
}
|
||||
}
|
||||
|
||||
fn file_from_handle(handle: HANDLE) -> std::io::Result<File> {
|
||||
if handle == INVALID_HANDLE_VALUE {
|
||||
Err(std::io::Error::last_os_error())
|
||||
} else {
|
||||
Ok(unsafe { File::from_raw_handle(handle) })
|
||||
}
|
||||
}
|
||||
|
|
@ -1,247 +0,0 @@
|
|||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::path::{Component, Path, PathBuf, Prefix};
|
||||
|
||||
const VERBATIM: &[u16] = &[b'\\' as u16, b'\\' as u16, b'?' as u16, b'\\' as u16];
|
||||
const VERBATIM_UNC: &[u16] = &[
|
||||
b'\\' as u16,
|
||||
b'\\' as u16,
|
||||
b'?' as u16,
|
||||
b'\\' as u16,
|
||||
b'U' as u16,
|
||||
b'N' as u16,
|
||||
b'C' as u16,
|
||||
b'\\' as u16,
|
||||
];
|
||||
|
||||
pub(super) fn normalized(path: &Path) -> std::io::Result<PathBuf> {
|
||||
validate_path_nul(path)?;
|
||||
validate_components(path)?;
|
||||
let absolute = if path.is_absolute() {
|
||||
path.to_path_buf()
|
||||
} else {
|
||||
if path.has_root() || matches!(path.components().next(), Some(Component::Prefix(_))) {
|
||||
return Err(invalid_input(
|
||||
"drive-relative and root-relative Windows paths are not accepted here",
|
||||
));
|
||||
}
|
||||
std::env::current_dir()?.join(path)
|
||||
};
|
||||
let mut normalized = PathBuf::new();
|
||||
for component in absolute.components() {
|
||||
match component {
|
||||
Component::CurDir => {}
|
||||
Component::ParentDir => {
|
||||
return Err(invalid_input(
|
||||
"Windows private paths must not contain parent traversal",
|
||||
));
|
||||
}
|
||||
_ => normalized.push(component),
|
||||
}
|
||||
}
|
||||
if !normalized.is_absolute() {
|
||||
return Err(invalid_input(
|
||||
"Windows path did not normalize to an absolute path",
|
||||
));
|
||||
}
|
||||
validate_components(&normalized)?;
|
||||
Ok(normalized)
|
||||
}
|
||||
|
||||
pub(super) fn wide_normalized(path: &Path) -> std::io::Result<Vec<u16>> {
|
||||
validate_path_nul(path)?;
|
||||
validate_components(path)?;
|
||||
let absolute: Vec<u16> = path.as_os_str().encode_wide().collect();
|
||||
let mut verbatim = if absolute.starts_with(VERBATIM) {
|
||||
absolute
|
||||
} else if absolute.starts_with(&[b'\\' as u16, b'\\' as u16]) {
|
||||
let mut path = Vec::with_capacity(VERBATIM_UNC.len() + absolute.len() - 2 + 1);
|
||||
path.extend_from_slice(VERBATIM_UNC);
|
||||
path.extend_from_slice(&absolute[2..]);
|
||||
path
|
||||
} else if is_drive_absolute(&absolute) {
|
||||
let mut path = Vec::with_capacity(VERBATIM.len() + absolute.len() + 1);
|
||||
path.extend_from_slice(VERBATIM);
|
||||
path.extend_from_slice(&absolute);
|
||||
path
|
||||
} else {
|
||||
return Err(invalid_input(
|
||||
"Windows path did not normalize to an absolute path",
|
||||
));
|
||||
};
|
||||
if verbatim.len() + 1 > 32_767 {
|
||||
return Err(invalid_input("Windows verbatim path exceeds 32,767 units"));
|
||||
}
|
||||
verbatim.push(0);
|
||||
Ok(verbatim)
|
||||
}
|
||||
|
||||
pub(super) fn validate_file_name(path: &Path) -> std::io::Result<()> {
|
||||
let file_name = path
|
||||
.file_name()
|
||||
.ok_or_else(|| invalid_input("private path has no filename"))?;
|
||||
validate_component(file_name)
|
||||
}
|
||||
|
||||
fn validate_components(path: &Path) -> std::io::Result<()> {
|
||||
for component in path.components() {
|
||||
match component {
|
||||
Component::Prefix(prefix) => validate_prefix(prefix.kind())?,
|
||||
Component::Normal(component) => validate_component(component)?,
|
||||
Component::ParentDir => {
|
||||
return Err(invalid_input(
|
||||
"Windows private paths must not contain parent traversal",
|
||||
));
|
||||
}
|
||||
Component::RootDir | Component::CurDir => {}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_prefix(prefix: Prefix<'_>) -> std::io::Result<()> {
|
||||
match prefix {
|
||||
Prefix::Disk(_)
|
||||
| Prefix::UNC(_, _)
|
||||
| Prefix::VerbatimDisk(_)
|
||||
| Prefix::VerbatimUNC(_, _) => Ok(()),
|
||||
Prefix::DeviceNS(_) | Prefix::Verbatim(_) => Err(invalid_input(
|
||||
"Windows device namespace paths are not accepted here",
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_component(component: &std::ffi::OsStr) -> std::io::Result<()> {
|
||||
let wide: Vec<u16> = component.encode_wide().collect();
|
||||
if wide.contains(&0) {
|
||||
return Err(invalid_input("Windows paths must not contain NUL"));
|
||||
}
|
||||
if wide.contains(&(b':' as u16)) {
|
||||
return Err(invalid_input(
|
||||
"Windows alternate data streams are not accepted here",
|
||||
));
|
||||
}
|
||||
if matches!(wide.last(), Some(last) if *last == b'.' as u16 || *last == b' ' as u16) {
|
||||
return Err(invalid_input(
|
||||
"Windows private path components must not end in a dot or space",
|
||||
));
|
||||
}
|
||||
if is_dos_reserved(&wide) {
|
||||
return Err(invalid_input(
|
||||
"Windows DOS device basenames are not accepted here",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn is_dos_reserved(component: &[u16]) -> bool {
|
||||
let base_end = component
|
||||
.iter()
|
||||
.position(|unit| *unit == b'.' as u16)
|
||||
.unwrap_or(component.len());
|
||||
let mut base = &component[..base_end];
|
||||
while matches!(base.last(), Some(last) if *last == b' ' as u16 || *last == b'.' as u16) {
|
||||
base = &base[..base.len() - 1];
|
||||
}
|
||||
ascii_eq(base, b"NUL")
|
||||
|| ascii_eq(base, b"CON")
|
||||
|| ascii_eq(base, b"AUX")
|
||||
|| ascii_eq(base, b"PRN")
|
||||
|| ascii_eq(base, b"CLOCK$")
|
||||
|| numbered_device(base, b"COM")
|
||||
|| numbered_device(base, b"LPT")
|
||||
}
|
||||
|
||||
fn numbered_device(base: &[u16], prefix: &[u8; 3]) -> bool {
|
||||
base.len() == 4
|
||||
&& ascii_eq(&base[..3], prefix)
|
||||
&& matches!(base[3], unit if (b'1' as u16..=b'9' as u16).contains(&unit) || matches!(unit, 0x00b9 | 0x00b2 | 0x00b3))
|
||||
}
|
||||
|
||||
fn ascii_eq(value: &[u16], expected: &[u8]) -> bool {
|
||||
value.len() == expected.len()
|
||||
&& value.iter().zip(expected).all(|(actual, expected)| {
|
||||
u8::try_from(*actual).is_ok_and(|actual| actual.to_ascii_uppercase() == *expected)
|
||||
})
|
||||
}
|
||||
|
||||
fn validate_path_nul(path: &Path) -> std::io::Result<()> {
|
||||
if path.as_os_str().encode_wide().any(|unit| unit == 0) {
|
||||
return Err(invalid_input("Windows paths must not contain NUL"));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn is_drive_absolute(path: &[u16]) -> bool {
|
||||
matches!(path, [drive, colon, slash, ..] if *drive != b'\\' as u16 && *colon == b':' as u16 && *slash == b'\\' as u16)
|
||||
}
|
||||
|
||||
fn invalid_input(message: &'static str) -> std::io::Error {
|
||||
std::io::Error::new(std::io::ErrorKind::InvalidInput, message)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn file_names_reject_windows_aliasing_syntax() {
|
||||
assert!(validate_file_name(Path::new("state:stream")).is_err());
|
||||
assert!(validate_file_name(Path::new("state.")).is_err());
|
||||
assert!(validate_file_name(Path::new("state ")).is_err());
|
||||
assert!(validate_file_name(Path::new("state.json")).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dos_device_names_are_rejected_case_insensitively_with_extensions() {
|
||||
for name in [
|
||||
"NUL",
|
||||
"con.txt",
|
||||
"AUX.json",
|
||||
"prn",
|
||||
"CLOCK$",
|
||||
"COM1.log",
|
||||
"com9",
|
||||
"LPT1.txt",
|
||||
"lpt9",
|
||||
"COM¹.txt",
|
||||
"LPT²",
|
||||
"lpt³.json",
|
||||
] {
|
||||
assert!(validate_file_name(Path::new(name)).is_err(), "{name}");
|
||||
}
|
||||
for name in ["COM0", "COM10", "LPT0", "LPT10", "console", "auxiliary"] {
|
||||
assert!(validate_file_name(Path::new(name)).is_ok(), "{name}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn drive_absolute_detection_requires_a_root_separator() {
|
||||
assert!(is_drive_absolute(&[
|
||||
b'C' as u16,
|
||||
b':' as u16,
|
||||
b'\\' as u16,
|
||||
b'x' as u16
|
||||
]));
|
||||
assert!(!is_drive_absolute(&[b'C' as u16, b':' as u16, b'x' as u16]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn drive_and_unc_paths_receive_verbatim_prefixes() {
|
||||
let drive = wide_normalized(Path::new(r"C:\state\refs.json")).unwrap();
|
||||
let unc = wide_normalized(Path::new(r"\\server\share\refs.json")).unwrap();
|
||||
|
||||
assert!(drive.starts_with(VERBATIM));
|
||||
assert!(unc.starts_with(VERBATIM_UNC));
|
||||
assert_eq!(drive.last(), Some(&0));
|
||||
assert_eq!(unc.last(), Some(&0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn long_drive_path_is_encoded_without_legacy_max_path_truncation() {
|
||||
let path = format!(r"C:\state\{}\refs.json", "a".repeat(300));
|
||||
let encoded = wide_normalized(Path::new(&path)).unwrap();
|
||||
|
||||
assert!(encoded.starts_with(VERBATIM));
|
||||
assert!(encoded.len() > 260);
|
||||
}
|
||||
}
|
||||
|
|
@ -1,114 +0,0 @@
|
|||
use std::fs::File;
|
||||
use std::mem::{offset_of, size_of};
|
||||
use std::os::windows::io::AsRawHandle;
|
||||
use std::path::Path;
|
||||
use std::ptr::null_mut;
|
||||
use windows_sys::Win32::Storage::FileSystem::{
|
||||
FILE_RENAME_INFO, FileRenameInfo, SetFileInformationByHandle,
|
||||
};
|
||||
|
||||
pub(super) struct RenameBuffer {
|
||||
words: Vec<usize>,
|
||||
byte_len: u32,
|
||||
}
|
||||
|
||||
impl RenameBuffer {
|
||||
fn new(destination: &Path) -> std::io::Result<Self> {
|
||||
let name = super::path::wide_normalized(destination)?;
|
||||
if name.last() != Some(&0) {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"Windows rename target is not NUL-terminated",
|
||||
));
|
||||
}
|
||||
let name_units = name.len().checked_sub(1).ok_or_else(|| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"Windows rename target is not NUL-terminated",
|
||||
)
|
||||
})?;
|
||||
let name_bytes = name_units.checked_mul(size_of::<u16>()).ok_or_else(|| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"Windows rename target is too large",
|
||||
)
|
||||
})?;
|
||||
let stored_name_bytes = name.len().checked_mul(size_of::<u16>()).ok_or_else(|| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"Windows rename target is too large",
|
||||
)
|
||||
})?;
|
||||
let byte_len = offset_of!(FILE_RENAME_INFO, FileName)
|
||||
.checked_add(stored_name_bytes)
|
||||
.ok_or_else(|| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"Windows rename buffer is too large",
|
||||
)
|
||||
})?;
|
||||
let mut words = vec![0usize; byte_len.div_ceil(size_of::<usize>())];
|
||||
let info = words.as_mut_ptr().cast::<FILE_RENAME_INFO>();
|
||||
unsafe {
|
||||
(*info).Anonymous.ReplaceIfExists = true;
|
||||
(*info).RootDirectory = null_mut();
|
||||
(*info).FileNameLength = name_bytes as u32;
|
||||
std::ptr::copy_nonoverlapping(
|
||||
name.as_ptr(),
|
||||
(&raw mut (*info).FileName).cast::<u16>(),
|
||||
name.len(),
|
||||
);
|
||||
}
|
||||
Ok(Self {
|
||||
words,
|
||||
byte_len: byte_len as u32,
|
||||
})
|
||||
}
|
||||
|
||||
fn as_ptr(&self) -> *const std::ffi::c_void {
|
||||
self.words.as_ptr().cast()
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn replace(source: &File, destination: &Path) -> std::io::Result<()> {
|
||||
let rename = RenameBuffer::new(destination)?;
|
||||
if unsafe {
|
||||
SetFileInformationByHandle(
|
||||
source.as_raw_handle(),
|
||||
FileRenameInfo,
|
||||
rename.as_ptr(),
|
||||
rename.byte_len,
|
||||
)
|
||||
} == 0
|
||||
{
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
source.sync_all()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn replacement_contract_requires_the_open_source_descriptor() {
|
||||
let contract: fn(&File, &Path) -> std::io::Result<()> = replace;
|
||||
let _ = contract;
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rename_layout_declares_the_terminal_nul_for_one_unit_basename() {
|
||||
let rename = RenameBuffer::new(Path::new(r"C:\x")).unwrap();
|
||||
let info = rename.as_ptr().cast::<FILE_RENAME_INFO>();
|
||||
let file_name_bytes = unsafe { (*info).FileNameLength } as usize;
|
||||
let nul_index = file_name_bytes / size_of::<u16>();
|
||||
let nul = unsafe { *(&raw const (*info).FileName).cast::<u16>().add(nul_index) };
|
||||
|
||||
assert!(rename.byte_len as usize >= size_of::<FILE_RENAME_INFO>());
|
||||
assert!(
|
||||
offset_of!(FILE_RENAME_INFO, FileName) + file_name_bytes + size_of::<u16>()
|
||||
<= rename.byte_len as usize
|
||||
);
|
||||
assert_eq!(nul, 0);
|
||||
}
|
||||
}
|
||||
|
|
@ -1,241 +0,0 @@
|
|||
use std::ffi::c_void;
|
||||
use std::fs::File;
|
||||
use std::mem::{size_of, zeroed};
|
||||
use std::os::windows::io::AsRawHandle;
|
||||
use std::ptr::null_mut;
|
||||
use std::sync::Arc;
|
||||
use windows_sys::Win32::Foundation::{CloseHandle, LocalFree};
|
||||
use windows_sys::Win32::Security::Authorization::{GetSecurityInfo, SE_FILE_OBJECT};
|
||||
use windows_sys::Win32::Security::{
|
||||
ACCESS_ALLOWED_ACE, ACL, ACL_REVISION, ACL_SIZE_INFORMATION, AclSizeInformation,
|
||||
AddAccessAllowedAceEx, CONTAINER_INHERIT_ACE, DACL_SECURITY_INFORMATION, EqualSid, GetAce,
|
||||
GetAclInformation, GetLengthSid, GetSecurityDescriptorControl, GetTokenInformation,
|
||||
InitializeAcl, InitializeSecurityDescriptor, OBJECT_INHERIT_ACE, OWNER_SECURITY_INFORMATION,
|
||||
PSID, SE_DACL_PROTECTED, SECURITY_ATTRIBUTES, SECURITY_DESCRIPTOR,
|
||||
SetSecurityDescriptorControl, SetSecurityDescriptorDacl, SetSecurityDescriptorOwner,
|
||||
TOKEN_QUERY, TOKEN_USER, TokenUser,
|
||||
};
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_ALL_ACCESS;
|
||||
use windows_sys::Win32::System::SystemServices::{
|
||||
ACCESS_ALLOWED_ACE_TYPE, SECURITY_DESCRIPTOR_REVISION,
|
||||
};
|
||||
use windows_sys::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken};
|
||||
|
||||
pub(super) fn validate_private_acl(file: &File) -> std::io::Result<()> {
|
||||
let (current_sid, _) = current_user_sid()?;
|
||||
let mut owner = null_mut();
|
||||
let mut dacl = null_mut();
|
||||
let mut descriptor = null_mut();
|
||||
let status = unsafe {
|
||||
GetSecurityInfo(
|
||||
file.as_raw_handle(),
|
||||
SE_FILE_OBJECT,
|
||||
OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION,
|
||||
&raw mut owner,
|
||||
null_mut(),
|
||||
&raw mut dacl,
|
||||
null_mut(),
|
||||
&raw mut descriptor,
|
||||
)
|
||||
};
|
||||
if status != 0 {
|
||||
return Err(std::io::Error::from_raw_os_error(status as i32));
|
||||
}
|
||||
let current_sid_ptr = current_sid.as_ptr().cast_mut().cast();
|
||||
let result = if owner.is_null() || unsafe { EqualSid(owner, current_sid_ptr) } == 0 {
|
||||
Err(permission_denied(
|
||||
"private path is not owned by the current user",
|
||||
))
|
||||
} else {
|
||||
validate_owner_only_dacl(descriptor, dacl, current_sid_ptr)
|
||||
};
|
||||
unsafe { LocalFree(descriptor) };
|
||||
result
|
||||
}
|
||||
|
||||
fn validate_owner_only_dacl(
|
||||
descriptor: *mut c_void,
|
||||
dacl: *mut ACL,
|
||||
current_sid: PSID,
|
||||
) -> std::io::Result<()> {
|
||||
let mut control = 0;
|
||||
let mut revision = 0;
|
||||
if dacl.is_null()
|
||||
|| unsafe { GetSecurityDescriptorControl(descriptor, &raw mut control, &raw mut revision) }
|
||||
== 0
|
||||
|| control & SE_DACL_PROTECTED == 0
|
||||
{
|
||||
return Err(permission_denied(
|
||||
"private path must have a protected owner-only DACL",
|
||||
));
|
||||
}
|
||||
let mut acl_info = ACL_SIZE_INFORMATION::default();
|
||||
if unsafe {
|
||||
GetAclInformation(
|
||||
dacl,
|
||||
(&raw mut acl_info).cast(),
|
||||
size_of::<ACL_SIZE_INFORMATION>() as u32,
|
||||
AclSizeInformation,
|
||||
)
|
||||
} == 0
|
||||
|| acl_info.AceCount != 1
|
||||
{
|
||||
return Err(permission_denied(
|
||||
"private path DACL must contain exactly one owner entry",
|
||||
));
|
||||
}
|
||||
let mut raw_ace = null_mut();
|
||||
if unsafe { GetAce(dacl, 0, &raw mut raw_ace) } == 0 || raw_ace.is_null() {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let ace = unsafe { &*raw_ace.cast::<ACCESS_ALLOWED_ACE>() };
|
||||
let ace_sid = (&raw const ace.SidStart).cast_mut().cast();
|
||||
if ace.Header.AceType != ACCESS_ALLOWED_ACE_TYPE as u8
|
||||
|| ace.Mask & FILE_ALL_ACCESS != FILE_ALL_ACCESS
|
||||
|| unsafe { EqualSid(ace_sid, current_sid) } == 0
|
||||
{
|
||||
return Err(permission_denied(
|
||||
"private path grants access outside the current user",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(super) struct PrivateSecurity {
|
||||
_sid: Arc<Vec<usize>>,
|
||||
_acl: Vec<usize>,
|
||||
_descriptor: Box<SECURITY_DESCRIPTOR>,
|
||||
attributes: SECURITY_ATTRIBUTES,
|
||||
}
|
||||
|
||||
impl PrivateSecurity {
|
||||
pub(super) fn new_directory() -> std::io::Result<Self> {
|
||||
let (sid, sid_bytes) = current_user_sid()?;
|
||||
Self::new_with_sid(Arc::new(sid), sid_bytes, true)
|
||||
}
|
||||
|
||||
pub(super) fn new_file() -> std::io::Result<Self> {
|
||||
let (sid, sid_bytes) = current_user_sid()?;
|
||||
Self::new_with_sid(Arc::new(sid), sid_bytes, false)
|
||||
}
|
||||
|
||||
pub(super) fn new_pair() -> std::io::Result<(Self, Self)> {
|
||||
let (sid, sid_bytes) = current_user_sid()?;
|
||||
let sid = Arc::new(sid);
|
||||
let directory = Self::new_with_sid(Arc::clone(&sid), sid_bytes, true)?;
|
||||
let file = Self::new_with_sid(sid, sid_bytes, false)?;
|
||||
Ok((directory, file))
|
||||
}
|
||||
|
||||
fn new_with_sid(
|
||||
sid: Arc<Vec<usize>>,
|
||||
sid_bytes: usize,
|
||||
directory: bool,
|
||||
) -> std::io::Result<Self> {
|
||||
let sid_ptr = sid.as_ref().as_ptr().cast_mut().cast();
|
||||
let acl_bytes =
|
||||
size_of::<ACL>() + size_of::<ACCESS_ALLOWED_ACE>() - size_of::<u32>() + sid_bytes;
|
||||
let mut acl = aligned_words(acl_bytes);
|
||||
let acl_ptr = acl.as_mut_ptr().cast::<ACL>();
|
||||
if unsafe { InitializeAcl(acl_ptr, acl_bytes as u32, ACL_REVISION) } == 0 {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let inheritance = if directory {
|
||||
CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE
|
||||
} else {
|
||||
0
|
||||
};
|
||||
if unsafe {
|
||||
AddAccessAllowedAceEx(acl_ptr, ACL_REVISION, inheritance, FILE_ALL_ACCESS, sid_ptr)
|
||||
} == 0
|
||||
{
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let mut descriptor = Box::new(unsafe { zeroed::<SECURITY_DESCRIPTOR>() });
|
||||
let descriptor_ptr = (&raw mut *descriptor).cast();
|
||||
if unsafe { InitializeSecurityDescriptor(descriptor_ptr, SECURITY_DESCRIPTOR_REVISION) }
|
||||
== 0
|
||||
|| unsafe { SetSecurityDescriptorOwner(descriptor_ptr, sid_ptr, 0) } == 0
|
||||
|| unsafe { SetSecurityDescriptorDacl(descriptor_ptr, 1, acl_ptr, 0) } == 0
|
||||
|| unsafe {
|
||||
SetSecurityDescriptorControl(descriptor_ptr, SE_DACL_PROTECTED, SE_DACL_PROTECTED)
|
||||
} == 0
|
||||
{
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let attributes = SECURITY_ATTRIBUTES {
|
||||
nLength: size_of::<SECURITY_ATTRIBUTES>() as u32,
|
||||
lpSecurityDescriptor: descriptor_ptr,
|
||||
bInheritHandle: 0,
|
||||
};
|
||||
Ok(Self {
|
||||
_sid: sid,
|
||||
_acl: acl,
|
||||
_descriptor: descriptor,
|
||||
attributes,
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) fn attributes(&self) -> *const SECURITY_ATTRIBUTES {
|
||||
&raw const self.attributes
|
||||
}
|
||||
}
|
||||
|
||||
fn current_user_sid() -> std::io::Result<(Vec<usize>, usize)> {
|
||||
let mut token = null_mut();
|
||||
if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &raw mut token) } == 0 {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let result = (|| {
|
||||
let mut bytes = 0;
|
||||
unsafe { GetTokenInformation(token, TokenUser, null_mut(), 0, &raw mut bytes) };
|
||||
if bytes == 0 {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let mut buffer = aligned_words(bytes as usize);
|
||||
if unsafe {
|
||||
GetTokenInformation(
|
||||
token,
|
||||
TokenUser,
|
||||
buffer.as_mut_ptr().cast(),
|
||||
bytes,
|
||||
&raw mut bytes,
|
||||
)
|
||||
} == 0
|
||||
{
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let sid = unsafe { (*buffer.as_ptr().cast::<TOKEN_USER>()).User.Sid };
|
||||
let sid_bytes = unsafe { GetLengthSid(sid) } as usize;
|
||||
if sid_bytes == 0 {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let mut owned = aligned_words(sid_bytes);
|
||||
unsafe {
|
||||
std::ptr::copy_nonoverlapping(sid.cast::<u8>(), owned.as_mut_ptr().cast(), sid_bytes)
|
||||
};
|
||||
Ok((owned, sid_bytes))
|
||||
})();
|
||||
unsafe { CloseHandle(token) };
|
||||
result
|
||||
}
|
||||
|
||||
fn aligned_words(byte_len: usize) -> Vec<usize> {
|
||||
vec![0; byte_len.div_ceil(size_of::<usize>())]
|
||||
}
|
||||
|
||||
fn permission_denied(message: &'static str) -> std::io::Error {
|
||||
std::io::Error::new(std::io::ErrorKind::PermissionDenied, message)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn private_acl_is_protected_and_owner_only_by_construction() {
|
||||
assert_ne!(SE_DACL_PROTECTED, 0);
|
||||
assert_ne!(FILE_ALL_ACCESS, 0);
|
||||
assert_eq!(ACL_REVISION, 2);
|
||||
}
|
||||
}
|
||||
|
|
@ -32,11 +32,13 @@ mod tests {
|
|||
use super::*;
|
||||
|
||||
fn lock_path(name: &str) -> std::path::PathBuf {
|
||||
std::env::temp_dir().join(format!(
|
||||
"agent-desktop-{name}-{}-{}.lock",
|
||||
std::process::id(),
|
||||
crate::refs::new_snapshot_id()
|
||||
))
|
||||
std::env::temp_dir()
|
||||
.join(format!(
|
||||
"agent-desktop-{name}-{}-{}",
|
||||
std::process::id(),
|
||||
crate::refs::new_snapshot_id()
|
||||
))
|
||||
.join("store.lock")
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -90,10 +90,13 @@ fn absolute_screenshot_path_is_not_embedded() {
|
|||
write_segment(
|
||||
&trace_dir,
|
||||
"100-1000.jsonl",
|
||||
&[&format!(
|
||||
r#"{{"event":"action.artifacts","screenshot_pre":"{}","ts_ms":1,"seq":1}}"#,
|
||||
sentinel.display()
|
||||
)],
|
||||
&[&serde_json::json!({
|
||||
"event": "action.artifacts",
|
||||
"screenshot_pre": sentinel.to_string_lossy(),
|
||||
"ts_ms": 1,
|
||||
"seq": 1,
|
||||
})
|
||||
.to_string()],
|
||||
);
|
||||
let (html, stats) = export_html(
|
||||
&trace_dir,
|
||||
|
|
|
|||
|
|
@ -81,9 +81,9 @@ pub(crate) use acquire_adapter;
|
|||
///
|
||||
/// Every method delegates to the `PlatformAdapter` trait defaults, all of
|
||||
/// which return `not_supported()` errors. `permission_report()` returns
|
||||
/// `Denied` via the trait default, so `ad_check_permissions` yields
|
||||
/// `ErrPermDenied` on a stub build — the expected signal for a CI runner
|
||||
/// that has no OS accessibility permission.
|
||||
/// `Unknown` via the trait default, so `ad_check_permissions` yields
|
||||
/// `ErrPlatformNotSupported` on a stub build — the honest signal that the
|
||||
/// adapter never probed permissions, rather than that one was denied.
|
||||
#[cfg(feature = "stub-adapter")]
|
||||
struct StubAdapter;
|
||||
|
||||
|
|
|
|||
|
|
@ -93,27 +93,27 @@ fn stub_ad_version_always_succeeds() {
|
|||
}
|
||||
}
|
||||
|
||||
/// `ad_check_permissions` maps the stub adapter's `Denied` permission state
|
||||
/// to `ErrPermDenied (-1)`, not `ErrPlatformNotSupported (-8)`. This is the
|
||||
/// documented exception. Cross-platform callers must treat both codes as
|
||||
/// "adapter not operational here".
|
||||
/// `ad_check_permissions` reports `ErrPlatformNotSupported (-8)` on an adapter
|
||||
/// that never probes permissions. Not having measured a permission is a
|
||||
/// different fact from a user having denied one, and `ErrPermDenied (-1)` sends
|
||||
/// callers looking for a grant that does not exist on that platform.
|
||||
#[cfg(feature = "stub-adapter")]
|
||||
#[test]
|
||||
fn stub_ad_check_permissions_returns_err_perm_denied() {
|
||||
fn stub_ad_check_permissions_returns_err_platform_not_supported() {
|
||||
with_adapter(|adapter| unsafe {
|
||||
let rc = ad_check_permissions(adapter);
|
||||
assert_eq!(
|
||||
rc,
|
||||
AdResult::ErrPermDenied,
|
||||
"stub adapter permission_report() returns Denied → ErrPermDenied (-1), \
|
||||
not ErrPlatformNotSupported (-8). Both mean the adapter is not operational."
|
||||
AdResult::ErrPlatformNotSupported,
|
||||
"an adapter that does not probe permissions reports Unknown, which must \
|
||||
surface as ErrPlatformNotSupported (-8), never ErrPermDenied (-1)."
|
||||
);
|
||||
let msg = ad_last_error_message();
|
||||
assert!(
|
||||
!msg.is_null(),
|
||||
"last-error message must be set on ErrPermDenied"
|
||||
"last-error message must be set on ErrPlatformNotSupported"
|
||||
);
|
||||
assert_eq!(ad_last_error_code(), AdResult::ErrPermDenied);
|
||||
assert_eq!(ad_last_error_code(), AdResult::ErrPlatformNotSupported);
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -165,7 +165,7 @@ agent-desktop/
|
|||
│ │ ├── snapshot_surface.rs # SnapshotSurface (predeclared cross-platform surface vocabulary)
|
||||
│ │ ├── snapshot.rs # SnapshotEngine (filter, allocate, serialize)
|
||||
│ │ ├── session/ # session start/end/list/gc, manifest, liveness
|
||||
│ │ ├── private_file*.rs # 0600-equivalent private artifact writing (Unix perms + Windows ACL family)
|
||||
│ │ ├── private_file*.rs # 0600-equivalent private artifact writing (portable std::fs on every target; Windows ACL hardening not yet built, see Phase 2.1)
|
||||
│ │ ├── trace.rs / trace_read/ # JSONL reliability trace, segment merge, HTML viewer
|
||||
│ │ ├── error_code.rs # ErrorCode enum
|
||||
│ │ ├── adapter_error.rs / app_error.rs # AdapterError, AppError
|
||||
|
|
@ -952,13 +952,13 @@ Every sub-phase 2.0–2.15 below is held to the same definition of done, stated
|
|||
|
||||
**Goal:** empirically map Windows accessibility reality with raw, no-Rust scripts before any adapter code exists, producing a committed evidence corpus the Rust sub-phases implement against — and feeding every contradiction back into this document.
|
||||
|
||||
**Scope:** a `probes/windows/` directory of raw scripts — PowerShell using .NET managed UIA (`System.Windows.Automation`, preinstalled with .NET Framework 4.8) plus small C# programs compiled with `csc.exe` where UIA3 COM specifics differ from the managed wrapper. The corpus must cover, each as a runnable script with captured JSON/text output committed beside it: (1) full-tree dumps of Notepad, Explorer, Settings, and one Electron app (VS Code or Slack) including every property read per node; (2) a pattern-availability census per ControlType (Invoke, Toggle, Value, RangeValue, ExpandCollapse, SelectionItem, Scroll, ScrollItem, Text, Window, LegacyIAccessible); (3) every interaction exercised raw — invoke, toggle, set value, select, expand/collapse, scroll via pattern AND wheel, text get/selection/caret/insert, focus; (4) SendInput synthesis experiments — keyboard incl. modifier chords and UTF-16 chunking limits, mouse click/move/wheel/drag; (5) `ElementFromPoint` hit-testing incl. deliberately occluded and zero-size targets; (6) `CacheRequest` batched reads timed against per-property reads; (7) AutomationId coverage census across Win32 / WinForms / WPF / Electron; (8) event-handler observations (which UIA events actually fire, ordering, MTA threading behavior); (9) elevation/UIPI behavior against an elevated process; (10) RDP-session and DPI/multi-monitor bounds behavior. Alongside the scripts: `probes/windows/FINDINGS.md` — a findings ledger mapping every experiment to observed behavior and a doc-alignment verdict (confirms this document / contradicts it / new edge case).
|
||||
**Scope:** a `probes/windows/` directory of raw scripts — PowerShell using .NET managed UIA (`System.Windows.Automation`, preinstalled with .NET Framework 4.8) plus small C# programs compiled with `csc.exe` where UIA3 COM specifics differ from the managed wrapper. The corpus must cover, each as a runnable script with captured JSON/text output committed beside it: (1) full-tree dumps of Notepad, Explorer, Settings, and one Electron app (VS Code or Slack) including every property read per node; (2) a pattern-availability census per ControlType (Invoke, Toggle, Value, RangeValue, ExpandCollapse, SelectionItem, Scroll, ScrollItem, Text, Window, LegacyIAccessible); (3) every interaction exercised raw — invoke, toggle, set value, select, expand/collapse, scroll via pattern AND wheel, text get/selection/caret/insert, focus; (4) SendInput synthesis experiments — keyboard incl. modifier chords and UTF-16 chunking limits, mouse click/move/wheel/drag; (5) `ElementFromPoint` hit-testing incl. deliberately occluded and zero-size targets; (6) `CacheRequest` batched reads timed against per-property reads; (7) AutomationId coverage census across Win32 / WinForms / WPF / Electron; (8) event-handler observations (which UIA events actually fire, ordering, MTA threading behavior); (9) elevation/UIPI behavior against an elevated process; (10) RDP-session and DPI/multi-monitor bounds behavior; (11) private-file I/O primitives — whether atomic rename over a concurrently-open handle requires `FILE_SHARE_DELETE`, whether an elevated process owns new objects as `TokenOwner` (e.g. `BUILTIN\Administrators`) rather than `TokenUser`, whether `GetFileInformationByHandleEx(FileRemoteProtocolInfo)` reliably distinguishes local from remote volumes, and what ancestor-vs-leaf ACL validation contract parity with the unix leaf-only rule actually needs (see `docs/solutions/best-practices/never-ship-platform-code-that-ci-cannot-execute.md`). Alongside the scripts: `probes/windows/FINDINGS.md` — a findings ledger mapping every experiment to observed behavior and a doc-alignment verdict (confirms this document / contradicts it / new edge case).
|
||||
|
||||
**Key APIs:** System.Windows.Automation, UIA3 COM (IUIAutomation) via csc.exe shims, SendInput, ElementFromPoint, CacheRequest.
|
||||
|
||||
**Depends on:** nothing — this is the entry point; the dev VM needs only its preinstalled .NET, PowerShell, and git.
|
||||
|
||||
**Exit criteria:** the script corpus and captured outputs are committed and re-runnable on the dev VM; the findings ledger covers tree, patterns, interactions, input, hit-testing, batching, identity, events, elevation, and session behavior with no open "unknown" rows; every ledger entry that contradicts this document has a matching amendment to this document landed in the same PR (see the source-of-truth feedback rule in the Platform Delivery Model); no Rust adapter sub-phase (2.2 onward) starts until this exit gate is green.
|
||||
**Exit criteria:** the script corpus and captured outputs are committed and re-runnable on the dev VM; the findings ledger covers tree, patterns, interactions, input, hit-testing, batching, identity, events, elevation, session, and private-file I/O behavior with no open "unknown" rows; every ledger entry that contradicts this document has a matching amendment to this document landed in the same PR (see the source-of-truth feedback rule in the Platform Delivery Model); no Rust adapter sub-phase (2.2 onward) starts until this exit gate is green.
|
||||
|
||||
**Est. PR size:** ~1.5k lines (scripts + ledger; no Rust).
|
||||
|
||||
|
|
@ -972,15 +972,15 @@ Every sub-phase 2.0–2.15 below is held to the same definition of done, stated
|
|||
- `CoInitializeEx(NULL, COINIT_MULTITHREADED)` + `SetProcessDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2)` bootstrap at process start
|
||||
- `WindowsAdapterSession` implementing `AdapterSession` via `open_session` — owns COM apartment state so later sub-phases don't reinvent COM lifecycle
|
||||
- Re-verify the dependency pins below against crates.io + supply-chain policy (pinned at 2026-04 research time — see New Dependencies)
|
||||
- Fix the `private_file_windows_security` `AceSize` validation gap (`crates/core/src/private_file_windows_security.rs`, currently cfg-gated dead code on non-Windows CI) so the private-artifact-writing path is real before any Windows code writes a refmap or trace file
|
||||
- Implement Windows private-file hardening from scratch, behind `PlatformAdapter` or as a Windows-gated dependency of the `agent-desktop-windows` crate — never as unconditional `agent-desktop-core` surface (see `docs/solutions/best-practices/never-ship-platform-code-that-ci-cannot-execute.md`) — satisfying, with evidence from 2.0's probes: `FILE_SHARE_DELETE` on every concurrently-open handle across an atomic replace; owner validation against `TokenOwner`, not `TokenUser`; no locality inference from `FileRemoteProtocolInfo`; and an ancestor-vs-leaf validation contract decided deliberately, matching or explicitly diverging from the unix leaf-only rule, so the private-artifact-writing path is real before any Windows code writes a refmap or trace file
|
||||
|
||||
**Key APIs:** `CoInitializeEx`, `SetProcessDpiAwarenessContext`, Win32 ACL / `AceSize` validation (private-file security)
|
||||
**Key APIs:** `CoInitializeEx`, `SetProcessDpiAwarenessContext`, Win32 ACL / `TokenOwner` validation, `FILE_SHARE_DELETE` (private-file hardening)
|
||||
|
||||
**Depends on:** nothing (opening sub-phase)
|
||||
|
||||
**Exit criteria:** workspace green on Windows CI; `WindowsAdapter` constructs and satisfies the trait; every command returns honest `PLATFORM_NOT_SUPPORTED` on Windows; the permission probe is unit-tested against mocked COM security state.
|
||||
**Exit criteria:** workspace green on Windows CI; `WindowsAdapter` constructs and satisfies the trait; every command returns honest `PLATFORM_NOT_SUPPORTED` on Windows; the permission probe is unit-tested against mocked COM security state; private-file hardening is unit-tested on the `windows-latest` CI lane, not merely `cargo check`-clean.
|
||||
|
||||
**Est. PR size:** ~0.8k LOC
|
||||
**Est. PR size:** ~1.3k LOC (bootstrap ~0.8k + from-scratch private-file hardening ~0.5k)
|
||||
|
||||
### 2.2 — UIA Element Wrapper & Tree Walk
|
||||
|
||||
|
|
@ -1170,11 +1170,11 @@ Every sub-phase 2.0–2.15 below is held to the same definition of done, stated
|
|||
**Scope:**
|
||||
- `ScreenshotBackend::Legacy` first (`PrintWindow(hwnd, hdc, PW_RENDERFULLCONTENT)` — mitigates DWM black frames), then `ScreenshotBackend::Modern` via `windows-capture`/WGC where the session supports it (P2-O13)
|
||||
- `screenshot --screen` honest display targeting (pairs with `list_displays` from 2.4)
|
||||
- Typed clipboard: `CF_UNICODETEXT` → `ClipboardContent::Text`, image via `CF_DIB`/PNG → `ClipboardContent::Image`, `CF_HDROP` file lists → `ClipboardContent::FileUrls`, all written through 0600-equivalent private files (see 2.1's ACL fix)
|
||||
- Typed clipboard: `CF_UNICODETEXT` → `ClipboardContent::Text`, image via `CF_DIB`/PNG → `ClipboardContent::Image`, `CF_HDROP` file lists → `ClipboardContent::FileUrls`, all written through 0600-equivalent private files (see 2.1's private-file hardening)
|
||||
|
||||
**Key APIs:** `PrintWindow`, `windows-capture`/`Windows.Graphics.Capture`, `OpenClipboard`/`GetClipboardData`/`SetClipboardData`
|
||||
|
||||
**Depends on:** 2.1 (private-file ACL fix), 2.4 (displays)
|
||||
**Depends on:** 2.1 (private-file hardening), 2.4 (displays)
|
||||
|
||||
**Exit criteria:** screenshot + clipboard e2e pass (clipboard tests hermetic, no real-desktop clipboard pollution); WGC gracefully degrades (falls back to `Legacy`, does not fail) in RDP-limited sessions.
|
||||
|
||||
|
|
@ -1652,7 +1652,7 @@ Same rendering shape, same [Cross-cutting sub-phase DoD](#cross-cutting-sub-phas
|
|||
|
||||
**Key APIs:** `org.freedesktop.portal.ScreenCast`, `org.freedesktop.portal.RemoteDesktop`, `XGetImage`, `wl-clipboard`/`xclip`
|
||||
|
||||
**Depends on:** 3.1 (private-file handling — Unix permissions already real, unlike the Windows ACL gap), 3.4 (displays)
|
||||
**Depends on:** 3.1 (private-file handling — Unix permissions already real; Windows private-file hardening is still to be built from scratch in 2.1), 3.4 (displays)
|
||||
|
||||
**Exit criteria:** screenshot + clipboard e2e pass (clipboard tests hermetic); the PipeWire portal flow is proven — the user approves via the XDG portal dialog once, subsequent calls bypass the dialog within the session grant window.
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,95 @@
|
|||
---
|
||||
title: Never ship platform code that CI cannot execute
|
||||
date: 2026-07-25
|
||||
category: best-practices
|
||||
module: crates/core private file I/O
|
||||
problem_type: process_gap
|
||||
component: tooling
|
||||
symptoms:
|
||||
- "A platform-conditional branch in core compiles everywhere but has only ever run on one OS."
|
||||
- "225 of 940 core unit tests fail the first time the suite is run on Windows."
|
||||
- "status fails closed on a plain local NTFS disk with 'cannot verify that the Windows storage is local'."
|
||||
- "A platform unit test compares two constants and can never fail."
|
||||
root_cause: process_gap
|
||||
resolution_type: code_removal
|
||||
severity: high
|
||||
tags: [platform-isolation, ci, windows, private-file, cfg, dead-code]
|
||||
---
|
||||
|
||||
# Never ship platform code that CI cannot execute
|
||||
|
||||
## Problem
|
||||
|
||||
`agent-desktop-core` is defined as platform-agnostic. Its domain logic honoured that —
|
||||
five `target_os` lines in the whole crate, every command test driven by `MockAdapter`.
|
||||
|
||||
Its filesystem layer did not. `private_file*` carried 1,062 LOC of Windows-only `unsafe`
|
||||
Win32: ACL construction, SID comparison, ancestor guard chains, reparse-point rejection,
|
||||
remote-volume detection. The Windows adapter it served was a 76-LOC stub with no
|
||||
`windows-sys` dependency and no implemented command — 14x more Windows code in
|
||||
"platform-agnostic" core than in the Windows crate.
|
||||
|
||||
CI ran `cargo check` on Windows and Linux, and `cargo test` on macOS alone. The Win32
|
||||
layer shipped in v0.5.0 type-checked and never once executed. Its first execution, on a
|
||||
real Windows machine, failed 225 of 940 core tests across four clusters:
|
||||
|
||||
- 122 sharing violations. `LEAF_SHARING` deliberately excluded `FILE_SHARE_DELETE`, so
|
||||
`SetFileInformationByHandle(FileRenameInfo)` collided with the validation handle the
|
||||
same code still held open. POSIX `rename(2)` ignores open descriptors; Windows does not.
|
||||
- 69 owner-only-DACL rejections. Owner validation compared against `TokenUser`; elevated
|
||||
tokens own new objects as `BUILTIN\Administrators`, which is how most CI runners execute.
|
||||
- 9 trace-writer access denials and 25 cascading assertion failures.
|
||||
- `status` dead on Windows: locality was inferred from
|
||||
`GetFileInformationByHandleEx(FileRemoteProtocolInfo)` failing with exactly
|
||||
`ERROR_INVALID_PARAMETER`. Local NTFS returns other codes, so the gate failed closed —
|
||||
and it ran on every ancestor from the volume root down.
|
||||
|
||||
The layer also diverged from the contract it claimed to implement. On unix, ownership and
|
||||
mode are checked on the leaf directory only; ancestors merely have to be non-symlink
|
||||
directories. The Windows path validated every ancestor and rejected any reparse point.
|
||||
Nobody decided that. It drifted, and no test could notice.
|
||||
|
||||
The single Windows behaviour test in the deleted code was:
|
||||
|
||||
```rust
|
||||
assert_eq!(LEAF_SHARING & FILE_SHARE_DELETE, 0);
|
||||
```
|
||||
|
||||
It asserted the defect was correct, compared constants to constants, and could never fail.
|
||||
|
||||
## Resolution
|
||||
|
||||
Deleted all 1,062 LOC and removed `windows-sys` from core entirely. Windows now uses the
|
||||
same portable `std::fs` path as every other non-unix target. This is honest: the hardening
|
||||
guarded refmap, trace, and session artifacts on a platform where no command can produce
|
||||
them.
|
||||
|
||||
Added real `cargo test` lanes for Windows and Linux so every platform-conditional branch
|
||||
in core is executed on every PR. That lane, not the deleted code, is the actual fix — it
|
||||
is what stops the same mistake reaching the Linux adapter, whose
|
||||
`validate_local_filesystem` had been equally unrun.
|
||||
|
||||
## Rules
|
||||
|
||||
- A `#[cfg]` branch that CI cannot execute is not shipped code, it is a hypothesis. Either
|
||||
add a lane that runs it, or do not merge it.
|
||||
- Do not write platform hardening ahead of the platform adapter it protects. Write it on
|
||||
that platform, against a lane that runs it, informed by probes.
|
||||
- A test that compares constants to constants is not a test. Every test must be able to
|
||||
fail.
|
||||
- When a platform layer needs a stricter policy than the shared contract, that is a
|
||||
contract change to make deliberately and document — not a detail to bury in one OS.
|
||||
|
||||
## Constraints for future Windows private-file hardening
|
||||
|
||||
When hardened Windows I/O returns, it belongs behind `PlatformAdapter` or in a
|
||||
Windows-gated dependency of the platform crate — never as unconditional core surface. It
|
||||
must satisfy, with evidence:
|
||||
|
||||
1. Atomic replace requires `FILE_SHARE_DELETE` on every concurrently-open handle to the
|
||||
target. POSIX rename semantics do not transfer.
|
||||
2. Owner validation compares against `TokenOwner`, not `TokenUser`.
|
||||
3. `FileRemoteProtocolInfo` cannot establish volume locality. Use a verified primitive or
|
||||
drop the gate rather than failing closed on local disks.
|
||||
4. Ancestor-wide validation must match the unix leaf-only contract, or the contract must
|
||||
change for both.
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
[toolchain]
|
||||
channel = "1.89.0"
|
||||
profile = "minimal"
|
||||
components = ["clippy", "rustfmt"]
|
||||
|
|
|
|||
Loading…
Reference in a new issue