Skip to content

Commit 1679e7d

Browse files
committed
Make executable registration ownership-aware and recoverable.
1 parent 690fbc5 commit 1679e7d

10 files changed

Lines changed: 673 additions & 101 deletions

File tree

‎CHANGES.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## Unreleased
99
### Fixed
10+
- asimov-registry, asimov-installer: Reject executable collisions and invalid
11+
binary entries, roll back failed registration/uninstall, and preserve links
12+
reassigned to other modules during uninstall
1013
- asimov-module: Read and resolve installed module directories while retaining
1114
legacy manifest lookup, and support lookup from an explicit state root
1215
- asimov-installer: Stage upgrades before replacing working installations,

‎rust/TODO.md‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,16 +107,10 @@ reproductions and build results are distinguished from source-review findings.
107107
final path segment without validating status or tag-route shape. Test failed
108108
redirects and asset fallback using injectable local release endpoints.
109109

110-
- [ ] **Make executable registration ownership-aware and recoverable.**
111-
`Registry::add_module` in `lib/asimov-registry/src/registry.rs` commits the
112-
module directory before linking binaries, and `add_binary` removes an existing
113-
destination without checking its owner. Uninstall in installer removes names
114-
from the manifest regardless of their current targets. Preflight collisions,
115-
validate executable entries, and roll back failed publication; test two
116-
modules providing the same name and partial link failures.
117-
Extend upgrade transaction locking to all registry mutations and recover
118-
interrupted upgrades before general registry reads, not only upgrade retries.
119-
Upgrade publication currently has a brief reader-visible directory/link gap;
110+
- [ ] **Coordinate low-level registry mutations and read-time recovery.**
111+
Extend publication locking to legacy migration, enable/disable, and low-level
112+
removal APIs. Recover interrupted publications before general registry reads.
113+
Publication currently has a brief reader-visible directory/link gap;
120114
assess generation-based publication and power-loss durability separately.
121115

122116
- [ ] **Make legacy registry migration atomic and observable.**

‎rust/lib/asimov-installer/src/installer.rs‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,9 @@ impl Installer {
8989
/// Stage a replacement before publishing it, preserving enabled state.
9090
///
9191
/// Publication failures roll back the prior installation. An interrupted
92-
/// publication is recovered on the next upgrade attempt. Callers must exclude
93-
/// concurrent install/uninstall and enable/disable operations on the registry.
92+
/// publication is recovered on the next registration, upgrade, or uninstall.
93+
/// Publication shares a lock with registration and uninstall; callers must
94+
/// exclude low-level registry mutations and enable/disable operations.
9495
///
9596
/// ```rust,no_run
9697
/// # use asimov_installer::{Installer, InstallOptions};
@@ -134,19 +135,7 @@ impl Installer {
134135
}
135136

136137
pub async fn uninstall_module(&self, module_name: &ModuleName) -> Result<(), UninstallError> {
137-
let manifest = self.registry.read_manifest(module_name).await?;
138-
139-
self.registry.disable_module(module_name).await?;
140-
141-
for program in &manifest.manifest.provides.programs {
142-
self.registry
143-
.remove_binary(program)
144-
.await
145-
.map_err(|e| UninstallError::RemoveBinary(program.into(), e))?;
146-
}
147-
148-
self.registry.remove_module(module_name).await?;
149-
138+
self.registry.uninstall_module(module_name).await?;
150139
Ok(())
151140
}
152141

@@ -297,7 +286,7 @@ impl Installer {
297286
) -> Result<(), FinishInstallError> {
298287
let (module_name, module_dir) = self.stage_install(preinstalled, work_dir).await?;
299288
self.registry
300-
.add_module(&module_name, module_dir.path())
289+
.add_module_owned(&module_name, module_dir)
301290
.await?;
302291
Ok(())
303292
}

‎rust/lib/asimov-installer/src/installer/error.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ pub enum UpgradeError {
4444

4545
#[derive(Debug, Error)]
4646
pub enum UninstallError {
47+
#[error("uninstall publication or recovery failed: {0}")]
48+
Publication(#[from] io::Error),
4749
#[error("unable to read module manifest file: {0}")]
4850
Read(#[from] registry::ManifestError),
4951
#[error(transparent)]

‎rust/lib/asimov-registry/src/registry.rs‎

Lines changed: 7 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use tokio::io;
88

99
pub mod error;
1010
use error::*;
11+
mod registration;
1112
mod upgrade;
1213

1314
pub use asimov_module::layout::MANIFEST_FILE_NAME;
@@ -86,47 +87,6 @@ impl Registry {
8687
self.install_dir.join(module_name.as_str())
8788
}
8889

89-
pub async fn add_module(
90-
&self,
91-
module_name: &ModuleName,
92-
dir: impl AsRef<Path>,
93-
) -> Result<(), AddModuleError> {
94-
if self.is_module_installed(module_name).await.unwrap_or(false) {
95-
return Err(AddModuleError::AlreadyInstalled);
96-
}
97-
98-
let module_dir = self.module_dir(module_name);
99-
100-
tokio::fs::rename(dir.as_ref(), &module_dir)
101-
.await
102-
.map_err(|e| AddModuleError::Install(dir.as_ref().into(), module_dir.clone(), e))?;
103-
104-
let bin_dir = module_dir.join(BIN_DIR_NAME);
105-
106-
let mut entries = match tokio::fs::read_dir(&bin_dir).await {
107-
Ok(entries) => entries,
108-
Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(()),
109-
Err(err) => return Err(AddModuleError::ReadBinDir(bin_dir, err)),
110-
};
111-
112-
while let Some(entry) = entries
113-
.next_entry()
114-
.await
115-
.map_err(|e| AddModuleError::ReadBinDir(bin_dir.clone(), e))?
116-
{
117-
let program_name = entry.file_name();
118-
let Some(program_name) = program_name.to_str() else {
119-
continue;
120-
};
121-
122-
self.add_binary(program_name, &entry.path())
123-
.await
124-
.map_err(|e| AddModuleError::AddBinary(program_name.into(), e))?;
125-
}
126-
127-
Ok(())
128-
}
129-
13090
pub async fn read_manifest(
13191
&self,
13292
module_name: &ModuleName,
@@ -167,6 +127,9 @@ impl Registry {
167127
.map_err(Into::into)
168128
}
169129

130+
/// Remove only the module directory. Prefer [`Self::uninstall_module`] to
131+
/// transactionally remove the directory and its owned links together.
132+
/// This low-level operation must not overlap a publication transaction.
170133
pub async fn remove_module(&self, module_name: &ModuleName) -> Result<(), RemoveModuleError> {
171134
self.migrate_legacy_manifest(module_name).await;
172135

@@ -181,22 +144,9 @@ impl Registry {
181144
.map_err(|e| RemoveModuleError::RemoveModuleDir(module_dir, e))
182145
}
183146

184-
async fn add_binary(&self, program_name: &str, binary_path: &Path) -> io::Result<()> {
185-
let target_path = match self
186-
.exec_dir
187-
.parent()
188-
.and_then(|parent| binary_path.strip_prefix(parent).ok())
189-
{
190-
Some(suffix) => PathBuf::from("..").join(suffix),
191-
None => binary_path.into(),
192-
};
193-
194-
let link_path = self.exec_dir.join(program_name);
195-
let _ = self.remove_binary(program_name).await;
196-
197-
create_symlink(&target_path, &link_path, false).await
198-
}
199-
147+
/// Unconditionally remove an executable entry, without checking ownership.
148+
/// Prefer [`Self::uninstall_module`] when removing a module. This low-level
149+
/// operation must not overlap a publication transaction.
200150
pub async fn remove_binary(&self, name: impl AsRef<str>) -> Result<(), RemoveBinaryError> {
201151
let name = name.as_ref();
202152

‎rust/lib/asimov-registry/src/registry/error.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ pub enum CreateFileTreeError {
1515

1616
#[derive(Debug, Error)]
1717
pub enum AddModuleError {
18+
#[error("registration failed: {0}")]
19+
Publication(#[from] io::Error),
1820
#[error("module is already installed")]
1921
AlreadyInstalled,
2022
#[error("failed to install module from `{0}` to `{1}`: {2}")]

0 commit comments

Comments
 (0)