mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-17 20:45:50 +00:00
fix: scope the temp lease per write and fold duplicated windows helpers
This commit is contained in:
parent
4e2ccc9bdd
commit
51683d5541
12 changed files with 264 additions and 283 deletions
|
|
@ -225,7 +225,9 @@ pub use output::{ErrorPayload, Response};
|
|||
pub use permission_report::PermissionReport;
|
||||
pub use permission_state::PermissionState;
|
||||
pub use point::Point;
|
||||
pub use private_file_ops::{PrivateFileOps, install_private_file_ops};
|
||||
pub use private_file_ops::{
|
||||
PrivateFileOps, bounded_read, install_private_file_ops, temporary_file_name,
|
||||
};
|
||||
pub use process_id::ProcessId;
|
||||
pub use process_identity::ProcessIdentity;
|
||||
pub use recovery_hint::RecoveryHint;
|
||||
|
|
|
|||
|
|
@ -1,11 +1,7 @@
|
|||
use std::fs::File;
|
||||
use std::fs::OpenOptions;
|
||||
use std::hash::{BuildHasher, RandomState};
|
||||
use std::io::{Read, Write};
|
||||
use std::io::Write;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
|
||||
static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
pub(crate) fn open_private_lock(path: &Path, create: bool) -> std::io::Result<File> {
|
||||
crate::private_file_ops::with_active_ops(|ops| ops.open_private_lock(path, create))
|
||||
|
|
@ -49,7 +45,7 @@ pub(crate) fn read_private_bounded_portable(
|
|||
max_bytes: u64,
|
||||
) -> std::io::Result<Vec<u8>> {
|
||||
let file = open_private_read(path)?;
|
||||
read_bounded(file, max_bytes)
|
||||
crate::private_file_ops::bounded_read(file, max_bytes)
|
||||
}
|
||||
|
||||
pub(crate) fn read_regular_bounded(path: &Path, max_bytes: u64) -> std::io::Result<Vec<u8>> {
|
||||
|
|
@ -61,7 +57,7 @@ pub(crate) fn read_regular_bounded(path: &Path, max_bytes: u64) -> std::io::Resu
|
|||
};
|
||||
validate_regular(&file)?;
|
||||
validate_local_filesystem(&file)?;
|
||||
read_bounded(file, max_bytes)
|
||||
crate::private_file_ops::bounded_read(file, max_bytes)
|
||||
}
|
||||
|
||||
pub(crate) fn write_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
|
||||
|
|
@ -177,33 +173,15 @@ fn open_private_read(path: &Path) -> std::io::Result<File> {
|
|||
Ok(file)
|
||||
}
|
||||
|
||||
fn read_bounded(file: File, max_bytes: u64) -> std::io::Result<Vec<u8>> {
|
||||
let metadata = file.metadata()?;
|
||||
if metadata.len() > max_bytes {
|
||||
return Err(invalid_input("file exceeds its read limit"));
|
||||
}
|
||||
let capacity = usize::try_from(metadata.len().min(max_bytes)).unwrap_or(usize::MAX);
|
||||
let mut bytes = Vec::with_capacity(capacity);
|
||||
file.take(max_bytes.saturating_add(1))
|
||||
.read_to_end(&mut bytes)?;
|
||||
if bytes.len() as u64 > max_bytes {
|
||||
return Err(invalid_input("file grew beyond its read limit"));
|
||||
}
|
||||
Ok(bytes)
|
||||
}
|
||||
|
||||
fn create_temporary(path: &Path) -> std::io::Result<(PathBuf, File)> {
|
||||
let file_name = path
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.ok_or_else(|| invalid_input("private file path has an invalid filename"))?;
|
||||
for _ in 0..32 {
|
||||
let nonce = RandomState::new().hash_one((
|
||||
std::process::id(),
|
||||
TEMP_COUNTER.fetch_add(1, Ordering::Relaxed),
|
||||
std::time::SystemTime::now(),
|
||||
let temporary = path.with_file_name(crate::private_file_ops::temporary_file_name(
|
||||
std::ffi::OsStr::new(file_name),
|
||||
));
|
||||
let temporary = path.with_file_name(format!(".{file_name}.{nonce:016x}.tmp"));
|
||||
let mut options = OpenOptions::new();
|
||||
options.write(true).create_new(true);
|
||||
configure_unix(&mut options, 0o600);
|
||||
|
|
|
|||
|
|
@ -1,6 +1,53 @@
|
|||
use std::ffi::{OsStr, OsString};
|
||||
use std::fs::File;
|
||||
use std::hash::{BuildHasher, RandomState};
|
||||
use std::io::Read;
|
||||
use std::path::Path;
|
||||
use std::sync::OnceLock;
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
|
||||
static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
/// Reads `file` fully on behalf of a `PrivateFileOps` implementation,
|
||||
/// rejecting files larger than `max_bytes` before allocating and detecting
|
||||
/// growth past the limit during the read.
|
||||
pub fn bounded_read(file: File, max_bytes: u64) -> std::io::Result<Vec<u8>> {
|
||||
let metadata = file.metadata()?;
|
||||
if metadata.len() > max_bytes {
|
||||
return Err(read_limit_error("file exceeds its read limit"));
|
||||
}
|
||||
let capacity = usize::try_from(metadata.len().min(max_bytes)).unwrap_or(usize::MAX);
|
||||
let mut bytes = Vec::with_capacity(capacity);
|
||||
file.take(max_bytes.saturating_add(1))
|
||||
.read_to_end(&mut bytes)?;
|
||||
if bytes.len() as u64 > max_bytes {
|
||||
return Err(read_limit_error("file grew beyond its read limit"));
|
||||
}
|
||||
Ok(bytes)
|
||||
}
|
||||
|
||||
fn read_limit_error(message: &'static str) -> std::io::Error {
|
||||
std::io::Error::new(std::io::ErrorKind::InvalidData, message)
|
||||
}
|
||||
|
||||
/// Produces one `.{name}.{nonce:016x}.tmp` candidate for a temporary that
|
||||
/// will be promoted over a destination whose leaf name is `name`.
|
||||
///
|
||||
/// The hashed nonce keeps the name unpredictable to a same-privilege racer;
|
||||
/// callers loop over fresh candidates when creation collides. Every
|
||||
/// `PrivateFileOps` implementation names its temporaries through this one
|
||||
/// scheme.
|
||||
pub fn temporary_file_name(name: &OsStr) -> OsString {
|
||||
let nonce = RandomState::new().hash_one((
|
||||
std::process::id(),
|
||||
TEMP_COUNTER.fetch_add(1, Ordering::Relaxed),
|
||||
std::time::SystemTime::now(),
|
||||
));
|
||||
let mut temporary = OsString::from(".");
|
||||
temporary.push(name);
|
||||
temporary.push(format!(".{nonce:016x}.tmp"));
|
||||
temporary
|
||||
}
|
||||
|
||||
/// Platform seam for the five private-file primitives.
|
||||
///
|
||||
|
|
|
|||
|
|
@ -7,14 +7,13 @@ use common::{ad_adapter_create_with_session, ad_adapter_destroy, with_isolated_h
|
|||
use std::ffi::CString;
|
||||
use std::time::Duration;
|
||||
|
||||
/// Runs alone in its own process because private-file install state is
|
||||
/// process-global: on Windows the first adapter construction installs
|
||||
/// `WindowsPrivateFile`, whose atomic writes hold a process-lifetime temp
|
||||
/// lease inside the written file's parent. If another adapter-creating test
|
||||
/// ran first in this process, the session writes below would plant that
|
||||
/// lease inside the session directory and the same-process gc removal would
|
||||
/// fail by design. In the product, session writes and `session gc` never
|
||||
/// share a process, so the isolation here models the real topology.
|
||||
/// Runs alone in its own process for env-var hygiene: the isolated HOME
|
||||
/// swap is process-wide, so a dedicated process keeps it from interleaving
|
||||
/// with adapter state other suites establish. The historical gc hazard is
|
||||
/// gone — on Windows the installed `WindowsPrivateFile` now scopes its temp
|
||||
/// lease to each atomic write, so no process-lifetime directory handle
|
||||
/// lingers inside the session directory and same-process gc removal (as
|
||||
/// exercised below) succeeds against everything this process wrote.
|
||||
#[test]
|
||||
fn session_scoped_adapter_holds_liveness_until_destroyed() {
|
||||
with_isolated_home(|| {
|
||||
|
|
|
|||
|
|
@ -5,50 +5,10 @@ use agent_desktop_core::session::{
|
|||
};
|
||||
use common::{
|
||||
AdResult, ad_adapter_create_with_session, ad_adapter_destroy, ad_check_permissions,
|
||||
ad_free_string, ad_status,
|
||||
ad_free_string, ad_status, with_isolated_home,
|
||||
};
|
||||
use std::ffi::CString;
|
||||
use std::fs;
|
||||
use std::sync::Mutex;
|
||||
|
||||
static HOME_LOCK: Mutex<()> = Mutex::new(());
|
||||
|
||||
struct TestHome {
|
||||
_lock: std::sync::MutexGuard<'static, ()>,
|
||||
dir: std::path::PathBuf,
|
||||
previous: Option<std::ffi::OsString>,
|
||||
}
|
||||
|
||||
impl TestHome {
|
||||
fn new() -> Self {
|
||||
let lock = HOME_LOCK.lock().unwrap();
|
||||
let dir = std::env::temp_dir().join(format!(
|
||||
"agent-desktop-ffi-session-trace-{}",
|
||||
std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap()
|
||||
.as_nanos()
|
||||
));
|
||||
fs::create_dir_all(&dir).unwrap();
|
||||
let previous = std::env::var_os("HOME");
|
||||
unsafe { std::env::set_var("HOME", &dir) };
|
||||
Self {
|
||||
_lock: lock,
|
||||
dir,
|
||||
previous,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for TestHome {
|
||||
fn drop(&mut self) {
|
||||
match self.previous.as_ref() {
|
||||
Some(previous) => unsafe { std::env::set_var("HOME", previous) },
|
||||
None => unsafe { std::env::remove_var("HOME") },
|
||||
}
|
||||
let _ = fs::remove_dir_all(&self.dir);
|
||||
}
|
||||
}
|
||||
|
||||
fn trace_segments(session_id: &str) -> Vec<std::path::PathBuf> {
|
||||
fs::read_dir(trace_dir(session_id).unwrap())
|
||||
|
|
@ -78,53 +38,55 @@ unsafe fn call_status(session_id: &str) {
|
|||
|
||||
#[test]
|
||||
fn traced_ffi_commands_reuse_one_process_segment_and_emit_ordered_boundaries() {
|
||||
let _home = TestHome::new();
|
||||
let manifest = start_session(StartSessionOptions {
|
||||
name: None,
|
||||
trace: SessionTraceMode::On,
|
||||
..Default::default()
|
||||
})
|
||||
.unwrap();
|
||||
with_isolated_home(|| {
|
||||
let manifest = start_session(StartSessionOptions {
|
||||
name: None,
|
||||
trace: SessionTraceMode::On,
|
||||
..Default::default()
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
unsafe {
|
||||
call_status(&manifest.id);
|
||||
call_status(&manifest.id);
|
||||
}
|
||||
unsafe {
|
||||
call_status(&manifest.id);
|
||||
call_status(&manifest.id);
|
||||
}
|
||||
|
||||
let segments = trace_segments(&manifest.id);
|
||||
assert_eq!(segments.len(), 1);
|
||||
let events: Vec<serde_json::Value> = fs::read_to_string(&segments[0])
|
||||
.unwrap()
|
||||
.lines()
|
||||
.map(|line| serde_json::from_str(line).unwrap())
|
||||
.filter(|event: &serde_json::Value| event["command"].as_str() == Some("status"))
|
||||
.collect();
|
||||
let boundaries: Vec<_> = events
|
||||
.iter()
|
||||
.filter_map(|event| event["event"].as_str())
|
||||
.filter(|event| matches!(*event, "command.start" | "command.end"))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
boundaries,
|
||||
[
|
||||
"command.start",
|
||||
"command.end",
|
||||
"command.start",
|
||||
"command.end"
|
||||
]
|
||||
);
|
||||
let segments = trace_segments(&manifest.id);
|
||||
assert_eq!(segments.len(), 1);
|
||||
let events: Vec<serde_json::Value> = fs::read_to_string(&segments[0])
|
||||
.unwrap()
|
||||
.lines()
|
||||
.map(|line| serde_json::from_str(line).unwrap())
|
||||
.filter(|event: &serde_json::Value| event["command"].as_str() == Some("status"))
|
||||
.collect();
|
||||
let boundaries: Vec<_> = events
|
||||
.iter()
|
||||
.filter_map(|event| event["event"].as_str())
|
||||
.filter(|event| matches!(*event, "command.start" | "command.end"))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
boundaries,
|
||||
[
|
||||
"command.start",
|
||||
"command.end",
|
||||
"command.start",
|
||||
"command.end"
|
||||
]
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn manifestless_session_does_not_create_trace_files() {
|
||||
let _home = TestHome::new();
|
||||
let session_id = "plain-session";
|
||||
unsafe {
|
||||
let session = CString::new(session_id).unwrap();
|
||||
let adapter = ad_adapter_create_with_session(session.as_ptr());
|
||||
assert!(!adapter.is_null());
|
||||
let _ = ad_check_permissions(adapter);
|
||||
ad_adapter_destroy(adapter);
|
||||
}
|
||||
assert!(!trace_dir(session_id).unwrap().exists());
|
||||
with_isolated_home(|| {
|
||||
let session_id = "plain-session";
|
||||
unsafe {
|
||||
let session = CString::new(session_id).unwrap();
|
||||
let adapter = ad_adapter_create_with_session(session.as_ptr());
|
||||
assert!(!adapter.is_null());
|
||||
let _ = ad_check_permissions(adapter);
|
||||
ad_adapter_destroy(adapter);
|
||||
}
|
||||
assert!(!trace_dir(session_id).unwrap().exists());
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ pub(crate) enum ComApartment {
|
|||
}
|
||||
|
||||
impl ComApartment {
|
||||
#[cfg(test)]
|
||||
#[cfg(any(test, target_os = "windows"))]
|
||||
pub(crate) fn permits_co_uninitialize(self) -> bool {
|
||||
matches!(self, ComApartment::OwnedMta)
|
||||
}
|
||||
|
|
@ -118,7 +118,7 @@ pub(crate) fn apartment_probe_reports_mta(hresult: i32, apartment_type: i32) ->
|
|||
|
||||
fn com_bootstrap_failure(message: &str, hresult: i32) -> AdapterError {
|
||||
AdapterError::new(ErrorCode::Internal, message)
|
||||
.with_platform_detail(format!("COM HRESULT 0x{:08X}", hresult as u32))
|
||||
.with_platform_detail(crate::system::permissions::com_hresult_detail(hresult))
|
||||
.with_suggestion(
|
||||
"Verify the host process allows COM initialization, then rerun the command",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -7,48 +7,33 @@ const E_ACCESSDENIED: i32 = 0x8007_0005_u32 as i32;
|
|||
|
||||
#[cfg(target_os = "windows")]
|
||||
mod imp {
|
||||
use windows_sys::Win32::Foundation::{S_FALSE, S_OK};
|
||||
use windows_sys::Win32::System::Com::{
|
||||
CLSCTX_INPROC_SERVER, COINIT_MULTITHREADED, CoCreateInstance, CoInitializeEx,
|
||||
CoUninitialize,
|
||||
};
|
||||
use windows_sys::core::GUID;
|
||||
use windows_sys::core::{GUID, IID_IUnknown, IUnknown_Vtbl};
|
||||
|
||||
use crate::system::com_runtime::classify_co_initialize_hresult;
|
||||
|
||||
const CLSID_CUIAUTOMATION: GUID = GUID::from_u128(0xff48dba4_60ef_4201_aa87_54103eef594e);
|
||||
const IID_IUNKNOWN: GUID = GUID::from_u128(0x00000000_0000_0000_c000_000000000046);
|
||||
const RPC_E_CHANGED_MODE: i32 = 0x8001_0106_u32 as i32;
|
||||
|
||||
#[repr(C)]
|
||||
struct ComObject {
|
||||
vtable: *const ComVtable,
|
||||
}
|
||||
|
||||
#[repr(C)]
|
||||
struct ComVtable {
|
||||
query_interface: usize,
|
||||
add_ref: usize,
|
||||
release: unsafe extern "system" fn(this: *mut core::ffi::c_void) -> u32,
|
||||
}
|
||||
|
||||
const _: () = assert!(size_of::<ComVtable>() == 3 * size_of::<usize>());
|
||||
|
||||
pub(super) fn probe_uia_access() -> i32 {
|
||||
unsafe {
|
||||
let init_status = CoInitializeEx(core::ptr::null(), COINIT_MULTITHREADED as u32);
|
||||
if init_status < 0 && init_status != RPC_E_CHANGED_MODE {
|
||||
return init_status;
|
||||
}
|
||||
let balance_apartment = init_status == S_OK || init_status == S_FALSE;
|
||||
let apartment = match classify_co_initialize_hresult(init_status) {
|
||||
Ok(apartment) => apartment,
|
||||
Err(failure) => return failure,
|
||||
};
|
||||
let mut instance: *mut core::ffi::c_void = core::ptr::null_mut();
|
||||
let create_status = CoCreateInstance(
|
||||
&CLSID_CUIAUTOMATION,
|
||||
core::ptr::null_mut(),
|
||||
CLSCTX_INPROC_SERVER,
|
||||
&IID_IUNKNOWN,
|
||||
&IID_IUnknown,
|
||||
&mut instance,
|
||||
);
|
||||
release_instance(instance);
|
||||
if balance_apartment {
|
||||
if apartment.permits_co_uninitialize() {
|
||||
CoUninitialize();
|
||||
}
|
||||
create_status
|
||||
|
|
@ -59,9 +44,9 @@ mod imp {
|
|||
if instance.is_null() {
|
||||
return;
|
||||
}
|
||||
let object = instance.cast::<ComObject>();
|
||||
let vtable = unsafe { *instance.cast::<*const IUnknown_Vtbl>() };
|
||||
unsafe {
|
||||
((*(*object).vtable).release)(instance);
|
||||
((*vtable).Release)(instance);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -85,23 +70,17 @@ mod imp {
|
|||
|
||||
pub(crate) fn report(deadline: Deadline) -> Result<PermissionReport, AdapterError> {
|
||||
ensure_budget(deadline)?;
|
||||
let report = PermissionReport {
|
||||
accessibility: accessibility_report_state(),
|
||||
screen_recording: screen_recording_report_state(),
|
||||
automation: automation_report_state(),
|
||||
};
|
||||
ensure_budget(deadline)?;
|
||||
Ok(report)
|
||||
report_from_probed_uia(deadline, imp::probe_uia_access())
|
||||
}
|
||||
|
||||
pub(crate) fn request_report(deadline: Deadline) -> Result<PermissionReport, AdapterError> {
|
||||
request_report_with(deadline, imp::probe_uia_access, report)
|
||||
request_report_with(deadline, imp::probe_uia_access, report_from_probed_uia)
|
||||
}
|
||||
|
||||
fn request_report_with(
|
||||
deadline: Deadline,
|
||||
probe: impl FnOnce() -> i32,
|
||||
report: impl FnOnce(Deadline) -> Result<PermissionReport, AdapterError>,
|
||||
report: impl FnOnce(Deadline, i32) -> Result<PermissionReport, AdapterError>,
|
||||
) -> Result<PermissionReport, AdapterError> {
|
||||
ensure_budget(deadline)?;
|
||||
let hresult = probe();
|
||||
|
|
@ -109,7 +88,20 @@ fn request_report_with(
|
|||
if matches!(map_uia_access(hresult), PermissionState::Denied { .. }) {
|
||||
return Err(uia_access_denied_error(hresult));
|
||||
}
|
||||
report(deadline)
|
||||
report(deadline, hresult)
|
||||
}
|
||||
|
||||
fn report_from_probed_uia(
|
||||
deadline: Deadline,
|
||||
uia_hresult: i32,
|
||||
) -> Result<PermissionReport, AdapterError> {
|
||||
let report = PermissionReport {
|
||||
accessibility: map_uia_access(uia_hresult),
|
||||
screen_recording: screen_recording_report_state(),
|
||||
automation: automation_report_state(),
|
||||
};
|
||||
ensure_budget(deadline)?;
|
||||
Ok(report)
|
||||
}
|
||||
|
||||
pub(crate) fn map_uia_access(hresult: i32) -> PermissionState {
|
||||
|
|
@ -146,10 +138,6 @@ pub(crate) fn com_hresult_detail(hresult: i32) -> String {
|
|||
}
|
||||
}
|
||||
|
||||
fn accessibility_report_state() -> PermissionState {
|
||||
map_uia_access(imp::probe_uia_access())
|
||||
}
|
||||
|
||||
fn screen_recording_report_state() -> PermissionState {
|
||||
map_capture_availability(imp::probe_capture_availability())
|
||||
}
|
||||
|
|
@ -158,7 +146,7 @@ fn automation_report_state() -> PermissionState {
|
|||
PermissionState::NotRequired
|
||||
}
|
||||
|
||||
fn ensure_budget(deadline: Deadline) -> Result<(), AdapterError> {
|
||||
pub(crate) fn ensure_budget(deadline: Deadline) -> Result<(), AdapterError> {
|
||||
if deadline.is_expired() {
|
||||
Err(deadline.timeout_error())
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -73,7 +73,7 @@ fn request_on_a_denied_probe_is_a_structured_error_not_a_prompt() {
|
|||
let error = request_report_with(
|
||||
Deadline::after(1_000).unwrap(),
|
||||
|| 0x8007_0005_u32 as i32,
|
||||
|_| panic!("a denied probe must not fall through to the report"),
|
||||
|_, _| panic!("a denied probe must not fall through to the report"),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
|
|
@ -82,9 +82,15 @@ fn request_on_a_denied_probe_is_a_structured_error_not_a_prompt() {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn request_on_a_granted_probe_returns_the_probe_report() {
|
||||
let report = request_report_with(Deadline::after(1_000).unwrap(), || 0, report).unwrap();
|
||||
fn request_on_a_granted_probe_reports_accessibility_from_that_single_probe() {
|
||||
let report = request_report_with(
|
||||
Deadline::after(1_000).unwrap(),
|
||||
|| 0,
|
||||
report_from_probed_uia,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(report.accessibility, PermissionState::Granted);
|
||||
assert_eq!(report.automation, PermissionState::NotRequired);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
//!
|
||||
//! Four measured behaviors drive four modules: per-component reparse-point
|
||||
//! rejection (`path`), `ReplaceFileW`-based atomic promotion with a
|
||||
//! per-process temp lease (`replace`), `TokenOwner` foreign-principal
|
||||
//! write-scoped temp lease (`replace`), `TokenOwner` foreign-principal
|
||||
//! detection (`owner`), and control-call-disciplined storage locality
|
||||
//! (`locality`). Each override mirrors the portable default's observable
|
||||
//! semantics — parent handling, create/append/lock open modes, the hashed
|
||||
|
|
@ -30,10 +30,10 @@ mod path;
|
|||
mod replace;
|
||||
|
||||
use std::fs::{File, OpenOptions};
|
||||
use std::io::{ErrorKind, Read, Write};
|
||||
use std::io::{ErrorKind, Write};
|
||||
use std::path::Path;
|
||||
|
||||
use agent_desktop_core::PrivateFileOps;
|
||||
use agent_desktop_core::{PrivateFileOps, bounded_read};
|
||||
|
||||
/// Windows implementation of core's private-file seam, installed once per
|
||||
/// process by the binary and FFI entry points.
|
||||
|
|
@ -58,16 +58,11 @@ impl PrivateFileOps for WindowsPrivateFile {
|
|||
.ok_or_else(|| invalid_input("private file path has an invalid filename"))?;
|
||||
path::ensure_private_directory_chain(parent)?;
|
||||
validate_destination_if_present(path)?;
|
||||
let lease = replace::lease_temp_directory(parent)?;
|
||||
let lease = replace::acquire_write_lease(parent)?;
|
||||
let (temporary, file) = replace::create_private_temp_file(&lease, destination_name)?;
|
||||
let result = write_all_and_sync(file, bytes).and_then(|()| {
|
||||
replace::promote_temp_to_destination(path, &temporary)?;
|
||||
validate_written_destination(path)
|
||||
});
|
||||
if result.is_err() {
|
||||
let _ = std::fs::remove_file(&temporary);
|
||||
}
|
||||
result
|
||||
write_all_and_sync(file, bytes)?;
|
||||
replace::promote_temp_to_destination(path, &temporary)?;
|
||||
validate_written_destination(path)
|
||||
}
|
||||
|
||||
fn open_private_append(&self, path: &Path) -> std::io::Result<File> {
|
||||
|
|
@ -100,7 +95,7 @@ impl PrivateFileOps for WindowsPrivateFile {
|
|||
options.read(true);
|
||||
let file = path::open_leaf_regular_no_follow(path, &mut options, "private file")?;
|
||||
owner::require_owned_by_token_owner(&file, "private file")?;
|
||||
read_bounded(file, max_bytes)
|
||||
bounded_read(file, max_bytes)
|
||||
}
|
||||
|
||||
fn ensure_private(&self, path: &Path) -> std::io::Result<()> {
|
||||
|
|
@ -126,21 +121,6 @@ fn write_all_and_sync(mut file: File, bytes: &[u8]) -> std::io::Result<()> {
|
|||
file.sync_all()
|
||||
}
|
||||
|
||||
fn read_bounded(file: File, max_bytes: u64) -> std::io::Result<Vec<u8>> {
|
||||
let metadata = file.metadata()?;
|
||||
if metadata.len() > max_bytes {
|
||||
return Err(invalid_input("file exceeds its read limit"));
|
||||
}
|
||||
let capacity = usize::try_from(metadata.len().min(max_bytes)).unwrap_or(usize::MAX);
|
||||
let mut bytes = Vec::with_capacity(capacity);
|
||||
file.take(max_bytes.saturating_add(1))
|
||||
.read_to_end(&mut bytes)?;
|
||||
if bytes.len() as u64 > max_bytes {
|
||||
return Err(invalid_input("file grew beyond its read limit"));
|
||||
}
|
||||
Ok(bytes)
|
||||
}
|
||||
|
||||
fn invalid_input(message: &'static str) -> std::io::Error {
|
||||
std::io::Error::new(ErrorKind::InvalidData, message)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,28 +14,30 @@
|
|||
//! destination; an absent destination means no reader holds it, so that
|
||||
//! branch falls back to the `MoveFileExW`-backed `std::fs::rename`.
|
||||
//!
|
||||
//! Temp files live in a per-process lease directory inside the destination's
|
||||
//! parent, which inherits the same profile ACL. The lease handle is held for
|
||||
//! the life of the process with a share mode that deliberately omits
|
||||
//! `FILE_SHARE_DELETE`, making the directory undeletable while its owner
|
||||
//! lives; a sweeper probes stale lease directories with `DELETE` access and
|
||||
//! reclaims only those whose owning process is gone. That narrowed share
|
||||
//! mode applies exclusively to this internal lease handle — artifact opens
|
||||
//! keep Rust's default wide `FILE_SHARE_READ|WRITE|DELETE` mask, because any
|
||||
//! hardened open that narrows it re-introduces the measured sharing-failure
|
||||
//! cluster. The sweep runs lazily, once per parent per process on first
|
||||
//! write, never per write. Temp names reuse core's hashed-nonce scheme so
|
||||
//! they stay unpredictable to a same-privilege racer.
|
||||
//! Temp files live in a write-scoped lease directory inside the
|
||||
//! destination's parent, which inherits the same profile ACL. Each atomic
|
||||
//! write creates its own lease directory — named with the pid plus a
|
||||
//! per-write nonce, so concurrent same-parent writes in one process never
|
||||
//! collide — and holds its handle for the duration of the write with a share
|
||||
//! mode that deliberately omits `FILE_SHARE_DELETE`. That held handle is the
|
||||
//! live-writer guard: a sweep before each write probes lease directories
|
||||
//! with `DELETE` access and reclaims only those no live writer holds. The
|
||||
//! lease handle is dropped and the directory removed on every exit path, so
|
||||
//! a long-lived process retains no directory handle that would defeat
|
||||
//! same-process snapshot pruning. The narrowed share mode applies
|
||||
//! exclusively to this internal lease handle — artifact opens keep Rust's
|
||||
//! default wide `FILE_SHARE_READ|WRITE|DELETE` mask, because any hardened
|
||||
//! open that narrows it re-introduces the measured sharing-failure cluster.
|
||||
//! Temp names reuse core's hashed-nonce scheme so they stay unpredictable to
|
||||
//! a same-privilege racer.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::ffi::OsStr;
|
||||
use std::fs::{File, OpenOptions};
|
||||
use std::hash::{BuildHasher, RandomState};
|
||||
use std::io::ErrorKind;
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::os::windows::fs::OpenOptionsExt;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::sync::{Arc, Mutex, OnceLock};
|
||||
|
||||
use windows_sys::Win32::Foundation::{
|
||||
ERROR_ACCESS_DENIED, ERROR_FILE_NOT_FOUND, ERROR_SHARING_VIOLATION,
|
||||
|
|
@ -48,14 +50,13 @@ use windows_sys::Win32::Storage::FileSystem::{
|
|||
use super::{invalid_input, locality, owner, path};
|
||||
|
||||
const TEMP_LEASE_PREFIX: &str = ".agent-desktop-tmp-p";
|
||||
const LEASE_CREATE_ATTEMPTS: usize = 32;
|
||||
const TEMP_CREATE_ATTEMPTS: usize = 32;
|
||||
const MEASURED_REPLACE_FLAGS: u32 = 0;
|
||||
|
||||
static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
pub(super) struct TempDirLease {
|
||||
directory: PathBuf,
|
||||
_liveness_handle: File,
|
||||
liveness_handle: Option<File>,
|
||||
}
|
||||
|
||||
impl TempDirLease {
|
||||
|
|
@ -64,64 +65,69 @@ impl TempDirLease {
|
|||
}
|
||||
}
|
||||
|
||||
pub(super) fn lease_temp_directory(parent: &Path) -> std::io::Result<Arc<TempDirLease>> {
|
||||
static ACTIVE_LEASES: OnceLock<Mutex<HashMap<PathBuf, Arc<TempDirLease>>>> = OnceLock::new();
|
||||
let registry = ACTIVE_LEASES.get_or_init(|| Mutex::new(HashMap::new()));
|
||||
let mut leases = registry
|
||||
.lock()
|
||||
.unwrap_or_else(|poisoned| poisoned.into_inner());
|
||||
if let Some(lease) = leases.get(parent) {
|
||||
return Ok(lease.clone());
|
||||
impl Drop for TempDirLease {
|
||||
fn drop(&mut self) {
|
||||
drop(self.liveness_handle.take());
|
||||
let _ = std::fs::remove_dir_all(&self.directory);
|
||||
}
|
||||
sweep_stale_lease_directories(parent);
|
||||
let lease = Arc::new(establish_lease(parent)?);
|
||||
leases.insert(parent.to_path_buf(), lease.clone());
|
||||
Ok(lease)
|
||||
}
|
||||
|
||||
fn establish_lease(parent: &Path) -> std::io::Result<TempDirLease> {
|
||||
let directory = parent.join(own_lease_name());
|
||||
match std::fs::create_dir(&directory) {
|
||||
Ok(()) => {}
|
||||
Err(error) if error.kind() == ErrorKind::AlreadyExists => {
|
||||
let _ = std::fs::remove_dir_all(&directory);
|
||||
match std::fs::create_dir(&directory) {
|
||||
Ok(()) => {}
|
||||
Err(retry) if retry.kind() == ErrorKind::AlreadyExists => {}
|
||||
Err(retry) => return Err(retry),
|
||||
pub(super) fn acquire_write_lease(parent: &Path) -> std::io::Result<TempDirLease> {
|
||||
sweep_stale_lease_directories(parent);
|
||||
for _ in 0..LEASE_CREATE_ATTEMPTS {
|
||||
let directory = parent.join(fresh_lease_name());
|
||||
match std::fs::create_dir(&directory) {
|
||||
Ok(()) => {}
|
||||
Err(error) if error.kind() == ErrorKind::AlreadyExists => continue,
|
||||
Err(error) => return Err(error),
|
||||
}
|
||||
match open_verified_liveness_handle(&directory) {
|
||||
Ok(handle) => {
|
||||
return Ok(TempDirLease {
|
||||
directory,
|
||||
liveness_handle: Some(handle),
|
||||
});
|
||||
}
|
||||
Err(error) if error.kind() == ErrorKind::NotFound => continue,
|
||||
Err(error) => {
|
||||
let _ = std::fs::remove_dir_all(&directory);
|
||||
return Err(error);
|
||||
}
|
||||
}
|
||||
Err(error) => return Err(error),
|
||||
}
|
||||
let liveness_handle = OpenOptions::new()
|
||||
Err(std::io::Error::new(
|
||||
ErrorKind::AlreadyExists,
|
||||
"could not allocate a private temp lease directory",
|
||||
))
|
||||
}
|
||||
|
||||
fn open_verified_liveness_handle(directory: &Path) -> std::io::Result<File> {
|
||||
let handle = OpenOptions::new()
|
||||
.read(true)
|
||||
.share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE)
|
||||
.custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT)
|
||||
.open(&directory)?;
|
||||
path::require_verified_lease_directory(&liveness_handle)?;
|
||||
owner::require_owned_by_token_owner(&liveness_handle, "the private temp directory")?;
|
||||
locality::require_local_for_private_write(&liveness_handle, "the private temp directory")?;
|
||||
Ok(TempDirLease {
|
||||
directory,
|
||||
_liveness_handle: liveness_handle,
|
||||
})
|
||||
.open(directory)?;
|
||||
path::require_verified_lease_directory(&handle)?;
|
||||
owner::require_owned_by_token_owner(&handle, "the private temp directory")?;
|
||||
locality::require_local_for_private_write(&handle, "the private temp directory")?;
|
||||
Ok(handle)
|
||||
}
|
||||
|
||||
fn own_lease_name() -> String {
|
||||
format!("{TEMP_LEASE_PREFIX}{}", std::process::id())
|
||||
fn fresh_lease_name() -> String {
|
||||
let nonce = RandomState::new().hash_one(std::time::SystemTime::now());
|
||||
format!("{TEMP_LEASE_PREFIX}{}-{nonce:016x}", std::process::id())
|
||||
}
|
||||
|
||||
fn sweep_stale_lease_directories(parent: &Path) {
|
||||
let Ok(entries) = std::fs::read_dir(parent) else {
|
||||
return;
|
||||
};
|
||||
let own_name = own_lease_name();
|
||||
for entry in entries.flatten() {
|
||||
let name = entry.file_name();
|
||||
let Some(name_text) = name.to_str() else {
|
||||
continue;
|
||||
};
|
||||
if !name_text.starts_with(TEMP_LEASE_PREFIX) || name_text == own_name {
|
||||
if !name_text.starts_with(TEMP_LEASE_PREFIX) {
|
||||
continue;
|
||||
}
|
||||
let candidate = entry.path();
|
||||
|
|
@ -146,14 +152,11 @@ pub(super) fn create_private_temp_file(
|
|||
destination_name: &str,
|
||||
) -> std::io::Result<(PathBuf, File)> {
|
||||
for _ in 0..TEMP_CREATE_ATTEMPTS {
|
||||
let nonce = RandomState::new().hash_one((
|
||||
std::process::id(),
|
||||
TEMP_COUNTER.fetch_add(1, Ordering::Relaxed),
|
||||
std::time::SystemTime::now(),
|
||||
));
|
||||
let temporary = lease
|
||||
.directory()
|
||||
.join(format!(".{destination_name}.{nonce:016x}.tmp"));
|
||||
.join(agent_desktop_core::temporary_file_name(OsStr::new(
|
||||
destination_name,
|
||||
)));
|
||||
match OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
|
|
|
|||
|
|
@ -186,8 +186,17 @@ fn write_atomic_replaces_a_destination_held_open_by_a_wide_share_reader() {
|
|||
);
|
||||
}
|
||||
|
||||
fn temp_lease_entries(parent: &Path) -> Vec<String> {
|
||||
std::fs::read_dir(parent)
|
||||
.unwrap()
|
||||
.flatten()
|
||||
.map(|entry| entry.file_name().to_string_lossy().into_owned())
|
||||
.filter(|name| name.starts_with(".agent-desktop-tmp-"))
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_first_write_into_a_parent_reclaims_a_dead_processes_lease_directory() {
|
||||
fn a_write_reclaims_an_orphan_lease_directory_no_live_writer_holds() {
|
||||
let scratch = Scratch::new("sweep-stale");
|
||||
let stale = scratch.path().join(".agent-desktop-tmp-p4294967295");
|
||||
std::fs::create_dir(&stale).unwrap();
|
||||
|
|
@ -199,16 +208,12 @@ fn the_first_write_into_a_parent_reclaims_a_dead_processes_lease_directory() {
|
|||
|
||||
assert!(
|
||||
!stale.exists(),
|
||||
"a lease directory with no live owner must be reclaimed"
|
||||
"a lease directory with no live holder must be reclaimed"
|
||||
);
|
||||
let own_lease = scratch
|
||||
.path()
|
||||
.join(format!(".agent-desktop-tmp-p{}", std::process::id()));
|
||||
assert!(own_lease.is_dir(), "this process's lease must exist");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_lease_directory_whose_owner_still_lives_survives_the_sweep() {
|
||||
fn a_lease_directory_held_without_share_delete_survives_a_concurrent_writes_sweep() {
|
||||
let scratch = Scratch::new("sweep-live");
|
||||
let foreign = scratch.path().join(".agent-desktop-tmp-p1");
|
||||
std::fs::create_dir(&foreign).unwrap();
|
||||
|
|
@ -231,27 +236,46 @@ fn a_lease_directory_whose_owner_still_lives_survives_the_sweep() {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn temporaries_are_confined_to_the_lease_directory_and_consumed_on_success() {
|
||||
fn a_successful_write_consumes_its_temporary_and_its_lease_directory() {
|
||||
let scratch = Scratch::new("temp-confinement");
|
||||
let ops = WindowsPrivateFile::new();
|
||||
|
||||
ops.write_atomic(&scratch.path().join("artifact.json"), b"payload")
|
||||
.unwrap();
|
||||
|
||||
let own_lease_name = format!(".agent-desktop-tmp-p{}", std::process::id());
|
||||
for entry in std::fs::read_dir(scratch.path()).unwrap().flatten() {
|
||||
let name = entry.file_name().to_string_lossy().into_owned();
|
||||
assert!(
|
||||
name == "artifact.json" || name == own_lease_name,
|
||||
"unexpected sibling {name}: temporaries must live inside the lease directory"
|
||||
);
|
||||
}
|
||||
let leftovers: Vec<_> = std::fs::read_dir(scratch.path().join(&own_lease_name))
|
||||
let siblings: Vec<String> = std::fs::read_dir(scratch.path())
|
||||
.unwrap()
|
||||
.flatten()
|
||||
.map(|entry| entry.file_name().to_string_lossy().into_owned())
|
||||
.collect();
|
||||
assert!(
|
||||
leftovers.is_empty(),
|
||||
"a successful write must consume its temporary"
|
||||
assert_eq!(
|
||||
siblings,
|
||||
vec!["artifact.json"],
|
||||
"temporaries must live inside the lease directory and both must be consumed"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_atomic_leaves_no_temp_lease_residue_on_success_or_failure() {
|
||||
let scratch = Scratch::new("no-residue");
|
||||
let destination = scratch.path().join("artifact.json");
|
||||
let ops = WindowsPrivateFile::new();
|
||||
|
||||
ops.write_atomic(&destination, b"first").unwrap();
|
||||
assert_eq!(
|
||||
temp_lease_entries(scratch.path()),
|
||||
Vec::<String>::new(),
|
||||
"a successful write must leave no .agent-desktop-tmp-* residue"
|
||||
);
|
||||
|
||||
let held = open_reader_with_share(&destination, FILE_SHARE_READ);
|
||||
ops.write_atomic(&destination, b"second")
|
||||
.expect_err("promotion over a no-share-delete holder must fail");
|
||||
drop(held);
|
||||
assert_eq!(
|
||||
temp_lease_entries(scratch.path()),
|
||||
Vec::<String>::new(),
|
||||
"a failed write must leave no .agent-desktop-tmp-* residue"
|
||||
);
|
||||
assert_eq!(std::fs::read(&destination).unwrap(), b"first");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
use agent_desktop_core::{AdapterError, AdapterSession, Deadline, ErrorCode};
|
||||
|
||||
use crate::system::com_runtime::classify_mta_usage_hresult;
|
||||
use crate::system::permissions::com_hresult_detail;
|
||||
use crate::system::permissions::{com_hresult_detail, ensure_budget};
|
||||
|
||||
type MtaUsageRelease = Box<dyn FnOnce(usize) -> i32 + Send + Sync>;
|
||||
|
||||
|
|
@ -105,14 +105,6 @@ fn mta_usage_release_failure(hresult: i32) -> AdapterError {
|
|||
)
|
||||
}
|
||||
|
||||
fn ensure_budget(deadline: Deadline) -> Result<(), AdapterError> {
|
||||
if deadline.is_expired() {
|
||||
Err(deadline.timeout_error())
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
mod imp {
|
||||
use windows_sys::Win32::System::Com::{
|
||||
|
|
|
|||
Loading…
Reference in a new issue