Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions crates/harness-runtime/src/human.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,21 +614,13 @@ fn rollback(harness: &Harness, prefix: &Path, to: Option<&str>) -> Result<()> {
}));
};

if present.exposed.as_deref() == Some(version) {
println!(
"{} already runs {version}; nothing to do.",
declared.command
);
return Ok(());
}

let rolled = setup_core::software::rollback(&declared, prefix, version)?;
println!(
"{} now runs {}.",
rolled.executable.display(),
rolled.version
);
if let Some(previous) = present.exposed {
if let Some(previous) = present.exposed.filter(|previous| previous != version) {
println!(" it ran {previous} before this, and that tree is still here");
}
Ok(())
Expand Down
38 changes: 21 additions & 17 deletions crates/harness-runtime/src/software.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
//! evict every configuration backup the target had.
//!
//! Software operations hold a prefix lock and revalidate the plan's bounded
//! content observation before recovery or effects. Install stages the version
//! before exposing its entry point. A per-command journal owns staging recovery;
//! installed-version ownership and exact-plan replay remain separate boundaries.
//! content observation before effects. Install stages the version before exposing
//! its entry point. A pending per-command journal refuses new operations; implicit
//! recovery would perform effects that the new plan did not authorize.

use std::fs;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -244,7 +244,7 @@ fn acquire_prefix(
root: &Path,
operation: Operation,
expected_software_digest: &str,
) -> Result<setup_core::lock::TargetLock> {
) -> Result<(setup_core::lock::TargetLock, software::Writer)> {
// A plan binds the bytes below this prefix, not just its absolute name.
// Check before creating bookkeeping, then again under the writer lock.
let before = setup_core::software_prefix::observe(root, harness.control_directory)?;
Expand Down Expand Up @@ -278,6 +278,10 @@ fn acquire_prefix(
true
};

// Contention must refuse before this provider creates any bookkeeping that
// another command's prefix observation would include as an unrelated change.
let writer = software::Writer::acquire(root)?;

// The lock lives in this provider's own dotted directory inside the prefix,
// not at its root. `acquire` takes a control directory, and a `target.lock`
// sitting beside `bin/` in a program directory would be both misnamed and
Expand All @@ -299,7 +303,7 @@ fn acquire_prefix(
}
guard.annotate(&format!("{} {operation}", harness.provider_id))?;

Ok(guard)
Ok((guard, writer))
}

fn installation_input(
Expand Down Expand Up @@ -395,12 +399,13 @@ pub(crate) fn apply(
"software_remove downloads nothing, so it takes no --software-artifact",
));
}
let _guard = acquire_prefix(harness, &root, operation, expected_software_digest)?;
let recovered = setup_core::software::recover(&declared, &root)?;
let removed = software::remove(&declared, &root)?;
software::require_idle(&declared, &root)?;
let (_guard, writer) = acquire_prefix(harness, &root, operation, expected_software_digest)?;
software::require_idle(&declared, &root)?;
let removed = writer.remove(&declared)?;
return Ok(serde_json::json!({
"state": "verified",
"recovered": recovered,
"recovered": [],
"operation": operation.as_str(),
"command": declared.command,
"version": declared.version,
Expand All @@ -411,11 +416,11 @@ pub(crate) fn apply(
let (artifact, input) =
installation_input(&declared, operation, planned_artifacts, downloaded)?;

// Reject incomplete or incorrect inputs before creating prefix bookkeeping
// or recovering any previously observed staging. The kernel consumes this
// same held input and verifies the extraction stream before promotion.
// Recovery only touches physical directories named by its staging journal.
let _guard = acquire_prefix(harness, &root, operation, expected_software_digest)?;
// Validate the held input before any bookkeeping. A pending transaction
// refuses before the lock and again under it; a new plan owns no recovery.
software::require_idle(&declared, &root)?;
let (_guard, writer) = acquire_prefix(harness, &root, operation, expected_software_digest)?;
software::require_idle(&declared, &root)?;

// Re-checked here, not trusted from the plan: applying happens later, and
// the prefix could have been emptied in between. The plan's digest binds
Expand All @@ -436,12 +441,11 @@ pub(crate) fn apply(
));
}

let recovered = setup_core::software::recover(&declared, &root)?;
let installed = software::install_verified(&declared, artifact, input, &root)?;
let installed = writer.install_verified(&declared, artifact, input)?;

Ok(serde_json::json!({
"state": "verified",
"recovered": recovered,
"recovered": [],
"operation": operation.as_str(),
"command": declared.command,
"version": installed.version,
Expand Down
29 changes: 28 additions & 1 deletion crates/harness-runtime/src/wire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9119,14 +9119,41 @@ mod tests {
// took of the thing this provider actually owns.
let target = seeded("software-slots");
let file = downloaded(&target, TEST_PAYLOAD);
plan_then_install(&target, "software_install", Some(&file));
let prefix = ready_prefix(&target);
let writer = setup_core::software::Writer::acquire(Path::new(&prefix)).unwrap();
let planned = software_plan(&target, "software_install");
let extra = apply_args(&target, &prefix, "software_install", &planned, Some(&file));
let borrowed: Vec<&str> = extra.iter().map(String::as_str).collect();
let error = refuse(args("apply-operation", &target, &borrowed));
assert_eq!(error.reason(), Some(WireReason::ProviderUnavailable));
assert!(!Path::new(&prefix).join(TEST.control_directory).exists());
drop(writer);
assert_eq!(
apply_planned(&target, &prefix, "software_install", &planned, Some(&file))["state"],
"verified"
);

let slots = target.join(TEST.control_directory).join("backups");
let taken = fs::read_dir(&slots).map_or(0, Iterator::count);
assert_eq!(
taken, 0,
"a software install captured a configuration backup"
);

// A matching version marker must not hide a foreign human rollback launcher.
let exposed = Path::new(&prefix).join("bin/test-harness");
fs::remove_file(&exposed).unwrap();
fs::write(&exposed, b"foreign launcher").unwrap();
let error = crate::human::run(
&TEST,
crate::human::Command::Rollback {
prefix: PathBuf::from(&prefix),
to: Some(TEST.software.unwrap().version.to_owned()),
},
)
.unwrap_err();
assert_eq!(error.reason(), Some(WireReason::RecoveryRequired));
assert_eq!(fs::read(exposed).unwrap(), b"foreign launcher");
}

#[test]
Expand Down
54 changes: 48 additions & 6 deletions crates/setup-core/src/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ fn release_in_process(path: &Path) {
pub struct TargetLock {
file: File,
path: PathBuf,
identity: (u64, u64),
}

impl TargetLock {
Expand All @@ -87,8 +88,8 @@ impl TargetLock {
));
}

let acquire_os_lock = || -> Result<File> {
let open_lock = || -> std::io::Result<File> {
let acquire_os_lock = || -> Result<(File, (u64, u64))> {
let open_lock = || -> std::io::Result<(File, (u64, u64))> {
let parent = control_directory
.parent()
.ok_or_else(|| std::io::Error::other("control directory has no parent"))?;
Expand Down Expand Up @@ -117,9 +118,9 @@ impl TargetLock {
if !metadata.is_file() || metadata.nlink() != 1 {
return Err(std::io::Error::other("lock is not a regular file"));
}
Ok(file.into_std())
Ok((file.into_std(), (metadata.dev(), metadata.ino())))
};
let file = open_lock().map_err(|source| {
let (file, identity) = open_lock().map_err(|source| {
Error::new(
ReasonCode::StateUnavailable,
format!("cannot open lock file {}", path.display()),
Expand All @@ -133,11 +134,15 @@ impl TargetLock {
)
.with_source(source)
})?;
Ok(file)
Ok((file, identity))
};

match acquire_os_lock() {
Ok(file) => Ok(Self { file, path }),
Ok((file, identity)) => Ok(Self {
file,
path,
identity,
}),
Err(error) => {
// The in-process claim must not outlive a failed acquisition, or
// the next attempt would be refused by this process forever.
Expand All @@ -153,6 +158,43 @@ impl TargetLock {
&self.path
}

/// Verify that the named lock still refers to this held regular file.
///
/// # Errors
/// Refuses an inaccessible, replaced, linked or aliased lock entry.
pub fn revalidate(&self) -> Result<()> {
let check = || -> std::io::Result<()> {
let parent = self
.path
.parent()
.ok_or_else(|| std::io::Error::other("lock has no parent"))?;
let directory = Dir::open_ambient_dir(parent, cap_std::ambient_authority())?;
let named = directory.symlink_metadata(LOCK_FILE_NAME)?;
if !named.is_file()
|| named.is_symlink()
|| named.nlink() != 1
|| (named.dev(), named.ino()) != self.identity
{
return Err(std::io::Error::other("the held lock entry changed"));
}
#[cfg(windows)]
{
use cap_std::fs::MetadataExt as _;
if named.file_attributes() & 0x400 != 0 {
return Err(std::io::Error::other("the lock is a reparse point"));
}
}
Ok(())
};
check().map_err(|source| {
Error::new(
ReasonCode::StateUnavailable,
"the prefix writer lock was replaced",
)
.with_source(source)
})
}

/// Record a non-secret owner note inside the lock file.
///
/// The note is diagnostic only. Nothing reads it to make a decision, because
Expand Down
Loading
Loading