diff --git a/decisions/decisions/adr-129-confined-rename-writes.md b/decisions/decisions/adr-129-confined-rename-writes.md new file mode 100644 index 00000000..f1a6ca4a --- /dev/null +++ b/decisions/decisions/adr-129-confined-rename-writes.md @@ -0,0 +1,107 @@ +--- +schema_version: 1 +id: RAC-01K8Q7MCP407 +type: decision +--- +# ADR-129: Confine Rename Writes to the Corpus Root + +## Context + +The corpus walker deliberately includes symlinked Markdown files on read-only +surfaces for compatibility. A rename is different: `decided rename --apply` +turns those paths into write targets. Following a symlink there can modify a +file outside the corpus the operator supplied, and a path can also be swapped +between planning and replacement. + +## Decision + +The native rename engine treats the requested corpus root as a mutation +boundary. + +- The root is canonicalized before a plan is built and again before a plan is + applied. +- Every target and relationship-bearing path in a plan must be a regular, + non-symlink path whose canonical destination remains below that root. +- A symlinked mutation path, an unresolvable path, or a path that resolves + outside the root produces a refused dry-run with a stable reason code and + the offending path; no file is written. +- Application repeats the containment and symlink checks immediately before + reading each file and immediately before replacing it. +- Unix staging opens each temporary final component with `O_NOFOLLOW` as a + final-component race guard; same-directory replacement uses the staged file + after the immediate root checks. Read-only discovery remains unchanged and + may still report symlinked Markdown files. + +## Status + +Accepted + +## Category + +Technical + +## Consequences + +Rename cannot silently write through a corpus symlink or escape the requested +root. A corpus that intentionally exposes a symlinked Markdown target must +materialize that file before renaming it; this is an explicit safety refusal, +not a partial edit. The extra metadata and canonicalization checks are bounded +by the number of files in the deterministic edit set. + +The protection is deliberately narrow. It does not change read-only walk +parity, rename ordering, identity semantics, or the exact-line stale-plan +check. `O_NOFOLLOW` closes the final-component race while staging on Unix; the +immediate rechecks provide the same root-boundary policy on other platforms. + +## Alternatives Considered + +### Follow symlinks as the walker does + +Rejected. Read compatibility is not authorization to mutate an arbitrary +target selected by a link. + +### Silently skip symlinked files + +Rejected. A skipped inbound reference would make a successful rename silently +incomplete. The dry-run must identify the path and refuse the whole plan. + +### Rename through directory handles only + +Rejected for this release. No-follow directory-handle APIs vary across the +supported platforms; canonical containment, immediate rechecks, and the Unix +final-component no-follow flag provide a deterministic cross-platform contract +without changing the CLI surface. + +## Code Constraints + +```yaml +version: 1 +eligibility: eligible +reason: "Rename safety is a deterministic source-level boundary with no model judgement." +rules: + - id: rename-confines-mutation-paths + kind: require_pattern + path_glob: "rust/rac-engine/src/rename.rs" + pattern: "check_mutation_path" + message: "Rename must recheck every mutation path against the canonical corpus root." + - id: rename-no-follow-final-write + kind: require_pattern + path_glob: "rust/rac-engine/src/rename.rs" + pattern: "O_NOFOLLOW" + message: "Unix rename staging must refuse a final-component symlink race." +``` + +## Related Decisions + +- adr-007 +- adr-023 +- adr-063 +- adr-080 +- adr-123 + +## Applies To + +- rust/rac-engine/src/rename.rs +- rust/rac-engine/tests/rename.rs +- rust/PORT-CONTRACT.d/16-closure-scaffold-writes.md +- docs/cli.md diff --git a/decisions/decisions/adr-130-transactional-rename-application.md b/decisions/decisions/adr-130-transactional-rename-application.md new file mode 100644 index 00000000..39c9fee8 --- /dev/null +++ b/decisions/decisions/adr-130-transactional-rename-application.md @@ -0,0 +1,115 @@ +--- +schema_version: 1 +id: RAC-01K8Q7MCP408 +type: decision +--- +# ADR-130: Transactional Rename Application + +## Context + +An artifact-id rename edits the target identity and every inbound reference. +Writing those files one at a time can leave the corpus half-renamed when a +later stale check, permission check, or filesystem replacement fails. A green +process exit must never hide a split identity/reference state. + +## Decision + +`decided rename --apply` uses a deterministic local transaction for all files in +the plan. + +- Every affected file is read, checked for exact `old_line` staleness, and + rendered in memory before any corpus path is replaced. +- Each rendered result is written and flushed to a hidden sibling staging file + in the same directory. Staging uses exclusive creation; Unix opens the + temporary final component with `O_NOFOLLOW`. +- During commit, each original moves to a hidden sibling backup and its staged + replacement moves into the original path. Files are processed in the + plan's first-seen path order. +- Any backup, replacement, or containment failure rolls committed files back + in reverse order from their backups. A successful rollback says `corpus + restored`; an incomplete rollback is reported explicitly with the paths that + could not be recovered. +- Successful commits remove all staging and backup files. Cleanup failures are + reported as a committed-but-cleanup-incomplete result; they never masquerade + as a clean success. + +The transaction remains bounded to the canonical root and the root-confined +mutation checks in ADR-129. Read-only walk behavior is unchanged. + +## Status + +Accepted + +## Category + +Technical + +## Consequences + +The identity and inbound references move together or the engine reports an +explicit failure. A later filesystem error can still make rollback impossible +if an external actor replaces a path during recovery, but the command reports +that condition rather than claiming success. Temporary siblings stay on the +same filesystem, so each rename operation is atomic at the individual-path +level and does not require a cross-volume coordination service. + +The commit is intentionally not a database transaction: no filesystem-wide +multi-path atomic primitive exists across the supported platforms. Backups and +reverse-order restoration provide deterministic recovery within the corpus +boundary. + +## Alternatives Considered + +### Continue writing files sequentially in place + +Rejected. A late stale or permission failure can leave references and identity +out of sync, which is precisely the integrity failure this decision closes. + +### Stage files but do not retain backups + +Rejected. Staging protects against a failure before commit, but cannot restore +files already replaced when a later rename fails. + +### Use a database or filesystem snapshot + +Rejected. The corpus is ordinary Markdown on filesystems with different +snapshot capabilities. Sibling backups preserve portability and keep the +mutation contract local and inspectable. + +## Code Constraints + +```yaml +version: 1 +eligibility: eligible +reason: "Transactional rename ordering and rollback are deterministic filesystem behavior." +rules: + - id: rename-preflights-before-commit + kind: require_pattern + path_glob: "rust/rac-engine/src/rename.rs" + pattern: "PreparedRenameFile" + message: "Rename must render all affected files before replacing any corpus path." + - id: rename-stages-sibling-files + kind: require_pattern + path_glob: "rust/rac-engine/src/rename.rs" + pattern: "create_new" + message: "Rename staging must use exclusive sibling temporary files." + - id: rename-rolls-back-on-failure + kind: require_pattern + path_glob: "rust/rac-engine/src/rename.rs" + pattern: "rollback_transaction" + message: "Rename commit failures must attempt deterministic reverse-order recovery." +``` + +## Related Decisions + +- adr-007 +- adr-023 +- adr-129 + +## Applies To + +- rust/rac-engine/src/rename.rs +- rust/rac-engine/src/output.rs +- rust/rac-engine/tests/rename.rs +- rust/PORT-CONTRACT.d/16-closure-scaffold-writes.md +- docs/cli.md diff --git a/docs/cli.md b/docs/cli.md index e9d379b1..bc5cb451 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -383,7 +383,12 @@ decided rename ADR-001 ADR-099 decisions/ --json # the plan as a stable dict the file, which is out of scope — so the rename refuses rather than leave `new-id` dangling. It also refuses an `old-id` that is unknown or ambiguous, and a `new-id` that is malformed or already names another artifact (which would create a duplicate -identity). Every refusal leaves the corpus untouched and exits `1`. +identity). The native engine additionally confines mutation to the canonical +corpus root (ADR-129): a Markdown symlink, an unresolvable edit path, or a path +that resolves outside the requested root is refused as `symlink-path` or +`path-outside-root`. The human dry run identifies the offending path; the +JSON plan carries it in `target_path`. Every refusal leaves the corpus +untouched and exits `1`. **Guarantees.** @@ -391,6 +396,11 @@ identity). Every refusal leaves the corpus untouched and exits `1`. ordered by path then line (ADR-002). - **Reversible** — applying `rename ` after a rename restores the original bytes. No semantic inference happens anywhere. +- **Transactional** — every affected file is preflighted and staged before + replacement. Same-directory backups allow reverse-order rollback when a + later replacement fails; the command reports `corpus restored` or an + explicit incomplete-recovery error rather than silently leaving a partial + rename (ADR-130). - **Clean afterwards** — after `--apply`, `decided relationships --validate` is clean: every inbound reference resolves to the renamed artifact. @@ -399,7 +409,7 @@ identity_field, files_changed, reference_edits, identity_edits, edits[] }`, wher each edit is `{ path, line, old_line, new_line, kind }` (`kind` is `"reference"` or `"identity"`). On refusal, `ok` is `false` and `reason` is one of the stable codes `old-ref-not-found`, `old-ref-ambiguous`, `new-ref-invalid`, `new-ref-collides`, -`old-ref-filename-only`. The `--apply` result is `{ applied, old_ref, new_ref, +`old-ref-filename-only`, `symlink-path`, `path-outside-root`. The `--apply` result is `{ applied, old_ref, new_ref, target_path, files_changed, reference_edits, identity_edits }`. In the editor, **RAC: Rename artifact id** runs this dry run, shows the affected diff --git a/rust/Cargo.lock b/rust/Cargo.lock index acb32acb..7edaa8e0 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -32,6 +32,7 @@ version = "0.26.2" dependencies = [ "globset", "inotify", + "libc", "markdown-it", "memmap2", "rayon", diff --git a/rust/PORT-CONTRACT.d/16-closure-scaffold-writes.md b/rust/PORT-CONTRACT.d/16-closure-scaffold-writes.md index 65dce0d0..63023656 100644 --- a/rust/PORT-CONTRACT.d/16-closure-scaffold-writes.md +++ b/rust/PORT-CONTRACT.d/16-closure-scaffold-writes.md @@ -172,7 +172,8 @@ Refusal routing is split: human refusal → STDERR (`Rename -> ` / blank / `✗ Refused: .`), JSON refusal → STDOUT (the full plan with `ok:false` and a stable `reason` code: `old-ref-not-found`, `old-ref-ambiguous`, `new-ref-collides` (target_path still set), -`new-ref-invalid`, `old-ref-filename-only`). +`new-ref-invalid`, `old-ref-filename-only`, `symlink-path`, or +`path-outside-root`). Plan semantics (all pinned): `new_ref` is stripped, then must match `^[A-Za-z][\w.-]*$` BEFORE any walk; `old_ref` resolves case-insensitively @@ -202,7 +203,21 @@ and identity edit across file(s).`. Apply JSON is the RenameResult (no `edits` array). `--apply` replaces exact lines (verified against `old_line`) and preserves the file's final-newline shape; the plan `directory` echoes the argv verbatim (trailing slash -kept) while edit paths are walk-normalized. +kept) while edit paths are walk-normalized. Mutation is confined to the +canonical corpus root (ADR-129): the target and every relationship-bearing +edit path must be a non-symlink regular path whose canonical destination is +under that root. A symlinked path or a path that cannot be resolved is a +whole-plan refusal with reason `symlink-path` or `path-outside-root`; the +human dry-run includes the offending path and JSON keeps it in +`target_path`. Apply repeats the checks immediately before each read and +replacement; Unix staging opens its final component with `O_NOFOLLOW` and +same-directory replacement uses the staged file. All affected files are +preflighted and staged before any replacement. Originals move to sibling +backups and a later failure triggers reverse-order rollback; the engine reports +`corpus restored` or an explicit `rollback incomplete` error. Successful +commits remove staging and backups; cleanup failures are reported rather than +silently ignored. Read-only discovery still yields symlinked Markdown files +for parity. ## 7. `rac migrate {metadata} [--dry-run] [--top-level] [--recursive] [--json]` diff --git a/rust/rac-engine/Cargo.toml b/rust/rac-engine/Cargo.toml index 5af26b1a..a7d899b7 100644 --- a/rust/rac-engine/Cargo.toml +++ b/rust/rac-engine/Cargo.toml @@ -26,3 +26,6 @@ serde_yaml = "0.9" [target.'cfg(target_os = "linux")'.dependencies] inotify = { version = "0.11", default-features = false } + +[target.'cfg(unix)'.dependencies] +libc = "0.2" diff --git a/rust/rac-engine/src/output.rs b/rust/rac-engine/src/output.rs index abd42ae2..0de2aa32 100644 --- a/rust/rac-engine/src/output.rs +++ b/rust/rac-engine/src/output.rs @@ -3604,6 +3604,12 @@ fn rename_reason_phrase(reason: Option<&str>) -> String { rewrite, and renaming files is out of scope" .to_string() } + Some(crate::rename::REASON_SYMLINK_PATH) => { + "one or more mutation paths are symlinks".to_string() + } + Some(crate::rename::REASON_PATH_OUTSIDE_ROOT) => { + "a mutation path is unresolved or outside the corpus root".to_string() + } Some(other) => other.to_string(), None => "unknown".to_string(), } @@ -3615,7 +3621,19 @@ pub fn render_rename_human(plan: &crate::rename::RenamePlan) -> String { let header = format!("Rename {} -> {}", plan.old_ref, plan.new_ref); if !plan.ok { let reason = rename_reason_phrase(plan.reason); - return format!("{header}\n\n{}", red(&format!("\u{2717} Refused: {reason}."))); + let path = match plan.reason { + Some(crate::rename::REASON_SYMLINK_PATH) + | Some(crate::rename::REASON_PATH_OUTSIDE_ROOT) => plan + .target_path + .as_deref() + .map(|path| format!(" Path: {path}.")) + .unwrap_or_default(), + _ => String::new(), + }; + return format!( + "{header}\n\n{}", + red(&format!("\u{2717} Refused: {reason}.{path}")) + ); } let mut lines = vec![header.clone(), "=".repeat(header.chars().count()), String::new()]; lines.push(format!( diff --git a/rust/rac-engine/src/rename.rs b/rust/rac-engine/src/rename.rs index e859ce44..58c5a2fd 100644 --- a/rust/rac-engine/src/rename.rs +++ b/rust/rac-engine/src/rename.rs @@ -9,6 +9,9 @@ //! naming a different alias of the same target is left untouched). use std::collections::HashSet; +use std::fs::{self, OpenOptions}; +use std::io::Write; +use std::path::{Path, PathBuf}; use crate::pycompat::{py_casefold, py_is_space, py_splitlines, py_strip}; use crate::relationships::{ @@ -22,6 +25,8 @@ pub const REASON_OLD_AMBIGUOUS: &str = "old-ref-ambiguous"; pub const REASON_NEW_COLLIDES: &str = "new-ref-collides"; pub const REASON_NEW_INVALID: &str = "new-ref-invalid"; pub const REASON_OLD_FILENAME_ONLY: &str = "old-ref-filename-only"; +pub const REASON_SYMLINK_PATH: &str = "symlink-path"; +pub const REASON_PATH_OUTSIDE_ROOT: &str = "path-outside-root"; // Where the rewritten identity token lived in the target file. pub const IDENTITY_FRONTMATTER: &str = "frontmatter_id"; @@ -73,6 +78,7 @@ impl RenamePlan { } /// The outcome of applying a plan to disk. +#[derive(Debug)] pub struct RenameResult { pub directory: String, pub old_ref: String, @@ -84,6 +90,187 @@ pub struct RenameResult { pub target_path: Option, } +#[derive(Debug)] +struct PathIssue { + reason: &'static str, + path: String, + detail: String, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum FailurePoint { + Stage(usize), + Backup(usize), + Replace(usize), +} + +#[derive(Debug, Clone, Copy)] +struct FailureInjector(Option); + +impl FailureInjector { + fn none() -> Self { + Self(None) + } + + #[cfg(test)] + fn at(point: FailurePoint) -> Self { + Self(Some(point)) + } + + fn should_fail(self, point: FailurePoint) -> bool { + self.0 == Some(point) + } +} + +struct PreparedRenameFile { + path: String, + text: String, + permissions: fs::Permissions, +} + +struct StagedRenameFile { + path: String, + staged: PathBuf, + backup: PathBuf, + backup_moved: bool, + installed: bool, +} + +fn path_issue(reason: &'static str, path: &str, detail: impl Into) -> PathIssue { + PathIssue { + reason, + path: path.to_string(), + detail: detail.into(), + } +} + +/// Check one path that a rename may mutate. The canonical root is resolved +/// once for the plan and again for application; the final path itself must +/// not be a symlink, and its resolved target must remain under that root. +/// +/// The walker intentionally yields symlinked Markdown files for read-only +/// parity, but rename is a mutation surface: following one here could write +/// through to an unrelated file outside the requested corpus. +fn check_mutation_path(root: &Path, path: &str) -> Result<(), PathIssue> { + let candidate = Path::new(path); + let metadata = fs::symlink_metadata(candidate).map_err(|error| PathIssue { + reason: REASON_PATH_OUTSIDE_ROOT, + path: path.to_string(), + detail: format!("cannot inspect path: {error}"), + })?; + if metadata.file_type().is_symlink() { + return Err(PathIssue { + reason: REASON_SYMLINK_PATH, + path: path.to_string(), + detail: "symlinked mutation paths are not permitted".to_string(), + }); + } + if !metadata.file_type().is_file() { + return Err(PathIssue { + reason: REASON_PATH_OUTSIDE_ROOT, + path: path.to_string(), + detail: "mutation paths must be regular files".to_string(), + }); + } + let canonical = fs::canonicalize(candidate).map_err(|error| PathIssue { + reason: REASON_PATH_OUTSIDE_ROOT, + path: path.to_string(), + detail: format!("cannot resolve path: {error}"), + })?; + if !canonical.starts_with(root) { + return Err(PathIssue { + reason: REASON_PATH_OUTSIDE_ROOT, + path: path.to_string(), + detail: format!( + "resolved path {} is outside corpus root {}", + canonical.display(), + root.display() + ), + }); + } + Ok(()) +} + +/// Check the parent directory of a temporary sibling or replacement path. +/// The final entry may not exist yet, so this is the root-boundary check that +/// applies before staging or restoring a transaction file. +fn check_sibling_parent(root: &Path, path: &Path) -> Result<(), PathIssue> { + let parent = path.parent().ok_or_else(|| { + path_issue( + REASON_PATH_OUTSIDE_ROOT, + &path.to_string_lossy(), + "path has no parent directory", + ) + })?; + let metadata = fs::metadata(parent).map_err(|error| { + path_issue( + REASON_PATH_OUTSIDE_ROOT, + &path.to_string_lossy(), + format!("cannot inspect parent directory: {error}"), + ) + })?; + if !metadata.is_dir() { + return Err(path_issue( + REASON_PATH_OUTSIDE_ROOT, + &path.to_string_lossy(), + "parent path is not a directory", + )); + } + let canonical = fs::canonicalize(parent).map_err(|error| { + path_issue( + REASON_PATH_OUTSIDE_ROOT, + &path.to_string_lossy(), + format!("cannot resolve parent directory: {error}"), + ) + })?; + if !canonical.starts_with(root) { + return Err(path_issue( + REASON_PATH_OUTSIDE_ROOT, + &path.to_string_lossy(), + format!( + "parent directory {} is outside corpus root {}", + canonical.display(), + root.display() + ), + )); + } + Ok(()) +} + +/// After the original has been moved to its backup, the destination must be +/// absent before the staged sibling is installed. A reappeared entry is +/// rejected rather than silently overwritten. +fn check_replacement_path(root: &Path, path: &str) -> Result<(), PathIssue> { + let candidate = Path::new(path); + match fs::symlink_metadata(candidate) { + Ok(metadata) if metadata.file_type().is_symlink() => Err(path_issue( + REASON_SYMLINK_PATH, + path, + "destination became a symlink during rename", + )), + Ok(_) => Err(path_issue( + REASON_PATH_OUTSIDE_ROOT, + path, + "destination reappeared during rename", + )), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + check_sibling_parent(root, candidate) + } + Err(error) => Err(path_issue( + REASON_PATH_OUTSIDE_ROOT, + path, + format!("cannot inspect replacement path: {error}"), + )), + } +} + +fn path_issue_message(phase: &str, issue: &PathIssue) -> String { + format!( + "rename: refusing to {phase} {}: {}", + issue.path, issue.detail + ) +} + fn refused( directory: &str, recursive: bool, @@ -224,7 +411,12 @@ fn relationship_reference_lines<'a>( /// `_reference_edits(items, target_path, old_ref, new_ref)` — every inbound /// relationship line whose leading reference token equals `old_ref`. -fn reference_edits(items: &[CorpusItem], old_ref: &str, new_ref: &str) -> Vec { +fn reference_edits( + items: &[CorpusItem], + root: &Path, + old_ref: &str, + new_ref: &str, +) -> Result, PathIssue> { let mut edits = Vec::new(); for item in items { let Some(spec) = item.spec else { continue }; @@ -244,6 +436,7 @@ fn reference_edits(items: &[CorpusItem], old_ref: &str, new_ref: &str) -> Vec Vec root, + Err(_) => { + return refused( + directory, + recursive, + old_ref, + &new_ref, + Some(directory.to_string()), + REASON_PATH_OUTSIDE_ROOT, + ) + } + }; + let items = corpus_items(directory, recursive); let rows: Vec = items .iter() @@ -557,6 +764,16 @@ pub fn compute_rename( .iter() .find(|item| item.path == target_path) .expect("resolved target is in the walked corpus"); + if let Err(issue) = check_mutation_path(&root, &target_path) { + return refused( + directory, + recursive, + old_ref, + &new_ref, + Some(issue.path), + issue.reason, + ); + } let (identity, identity_field) = match identity_edit(target_item, old_ref, &new_ref) { Ok(pair) => pair, Err(reason) => { @@ -564,7 +781,19 @@ pub fn compute_rename( } }; - let mut edits = reference_edits(&items, old_ref, &new_ref); + let mut edits = match reference_edits(&items, &root, old_ref, &new_ref) { + Ok(edits) => edits, + Err(issue) => { + return refused( + directory, + recursive, + old_ref, + &new_ref, + Some(issue.path), + issue.reason, + ) + } + }; let (line, old_line, new_line) = identity; edits.push(RenameEdit { path: target_path.clone(), @@ -588,11 +817,202 @@ pub fn compute_rename( } } -/// `apply_rename(plan)` — exact line replacements, original final-newline -/// shape preserved. A stale plan (the file changed since it was computed) -/// is the oracle's uncaught `ValueError` traceback (exit 1); surfaced here -/// as `Err(message)` for the command to fail with the same code. -pub fn apply_rename(plan: &RenamePlan) -> Result { +fn transaction_token() -> String { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|duration| duration.as_nanos()) + .unwrap_or_default(); + format!("{}-{nanos}", std::process::id()) +} + +fn temporary_sibling( + root: &Path, + path: &str, + token: &str, + index: usize, + kind: &str, +) -> Result { + let destination = Path::new(path); + check_sibling_parent(root, destination) + .map_err(|issue| path_issue_message("stage", &issue))?; + let parent = destination + .parent() + .ok_or_else(|| format!("rename: cannot stage {path}: path has no parent directory"))?; + for attempt in 0..100 { + let candidate = parent.join(format!( + ".asdecided-rename-{token}-{index}-{kind}-{attempt}.tmp" + )); + match fs::symlink_metadata(&candidate) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(candidate) + } + Ok(_) => continue, + Err(error) => { + return Err(format!( + "rename: cannot reserve {kind} path for {path}: {error}" + )) + } + } + } + Err(format!( + "rename: cannot reserve a unique {kind} path for {path}" + )) +} + +fn stage_text( + root: &Path, + prepared: &PreparedRenameFile, + token: &str, + index: usize, + injector: FailureInjector, +) -> Result { + if injector.should_fail(FailurePoint::Stage(index)) { + return Err(format!( + "injected staging failure for {}", + prepared.path + )); + } + let staged = temporary_sibling(root, &prepared.path, token, index, "stage")?; + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.custom_flags(libc::O_NOFOLLOW).mode(0o600); + } + let mut file = match options.open(&staged) { + Ok(file) => file, + Err(error) => { + return Err(format!( + "rename: cannot create staging file for {}: {error}", + prepared.path + )) + } + }; + if let Err(error) = file.write_all(prepared.text.as_bytes()) { + let _ = fs::remove_file(&staged); + return Err(format!( + "rename: cannot write staging file for {}: {error}", + prepared.path + )); + } + if let Err(error) = file.sync_all() { + let _ = fs::remove_file(&staged); + return Err(format!( + "rename: cannot flush staging file for {}: {error}", + prepared.path + )); + } + if let Err(error) = fs::set_permissions(&staged, prepared.permissions.clone()) { + let _ = fs::remove_file(&staged); + return Err(format!( + "rename: cannot preserve permissions for {}: {error}", + prepared.path + )); + } + Ok(staged) +} + +fn remove_temp(path: &Path, errors: &mut Vec) { + match fs::symlink_metadata(path) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => errors.push(format!("{}: {error}", path.display())), + Ok(metadata) if metadata.file_type().is_file() || metadata.file_type().is_symlink() => { + if let Err(error) = fs::remove_file(path) { + errors.push(format!("{}: {error}", path.display())); + } + } + Ok(_) => errors.push(format!("{}: temporary path is not a file", path.display())), + } +} + +fn remove_installed_path(root: &Path, path: &str) -> Result<(), String> { + match fs::symlink_metadata(path) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(format!("cannot inspect installed path {path}: {error}")), + Ok(metadata) if metadata.file_type().is_symlink() => { + fs::remove_file(path).map_err(|error| format!("cannot remove {path}: {error}")) + } + Ok(_) => { + check_mutation_path(root, path) + .map_err(|issue| path_issue_message("remove during rollback", &issue))?; + fs::remove_file(path).map_err(|error| format!("cannot remove {path}: {error}")) + } + } +} + +fn restore_backup(root: &Path, entry: &StagedRenameFile) -> Result<(), String> { + match fs::symlink_metadata(&entry.path) { + Ok(_) => return Err(format!("destination {} is occupied during rollback", entry.path)), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(format!( + "cannot inspect {} during rollback: {error}", + entry.path + )) + } + } + check_sibling_parent(root, Path::new(&entry.path)) + .map_err(|issue| path_issue_message("restore", &issue))?; + fs::rename(&entry.backup, &entry.path) + .map_err(|error| format!("cannot restore {}: {error}", entry.path)) +} + +fn rollback_transaction(root: &Path, entries: &mut [StagedRenameFile]) -> Vec { + let mut errors = Vec::new(); + for entry in entries.iter_mut().rev() { + if entry.installed { + if let Err(error) = remove_installed_path(root, &entry.path) { + errors.push(error); + } else { + entry.installed = false; + } + } + if entry.backup_moved { + if let Err(error) = restore_backup(root, entry) { + errors.push(error); + } else { + entry.backup_moved = false; + } + } + remove_temp(&entry.staged, &mut errors); + if !entry.backup_moved { + remove_temp(&entry.backup, &mut errors); + } + } + errors +} + +fn transaction_failure( + root: &Path, + entries: &mut [StagedRenameFile], + reason: impl Into, +) -> String { + let reason = reason.into(); + let rollback_errors = rollback_transaction(root, entries); + if rollback_errors.is_empty() { + format!("rename: transaction aborted: {reason}; corpus restored") + } else { + format!( + "rename: transaction aborted: {reason}; rollback incomplete: {}", + rollback_errors.join("; ") + ) + } +} + +fn cleanup_staging(entries: &[StagedRenameFile]) -> Vec { + let mut errors = Vec::new(); + for entry in entries { + remove_temp(&entry.staged, &mut errors); + remove_temp(&entry.backup, &mut errors); + } + errors +} + +fn apply_rename_transaction( + plan: &RenamePlan, + injector: FailureInjector, +) -> Result { if !plan.ok { return Ok(RenameResult { directory: plan.directory.clone(), @@ -606,14 +1026,28 @@ pub fn apply_rename(plan: &RenamePlan) -> Result { }); } - // Group by path, first-seen order (Python dict setdefault). + let root = fs::canonicalize(&plan.directory) + .map_err(|e| format!("rename: cannot resolve corpus root {}: {e}", plan.directory))?; + + // Group by path, first-seen order (Python dict setdefault), then complete + // the entire read/stale/render preflight before creating any replacement. let mut order: Vec<&str> = Vec::new(); for edit in &plan.edits { if !order.contains(&edit.path.as_str()) { order.push(&edit.path); } } + let mut prepared = Vec::with_capacity(order.len()); for path in order { + check_mutation_path(&root, path).map_err(|issue| path_issue_message("read", &issue))?; + let permissions = fs::metadata(path) + .map_err(|error| format!("rename: cannot inspect {path}: {error}"))? + .permissions(); + if permissions.readonly() { + return Err(format!( + "rename: cannot write {path}: file is read-only" + )); + } let original = std::fs::read_to_string(path) .map_err(|e| format!("rename: cannot read {path}: {e}"))?; let had_final_newline = original.ends_with('\n'); @@ -636,7 +1070,127 @@ pub fn apply_rename(plan: &RenamePlan) -> Result { if had_final_newline { text.push('\n'); } - std::fs::write(path, text).map_err(|e| format!("rename: cannot write {path}: {e}"))?; + prepared.push(PreparedRenameFile { + path: path.to_string(), + text, + permissions, + }); + } + + let token = transaction_token(); + let mut entries = Vec::with_capacity(prepared.len()); + for (index, prepared) in prepared.iter().enumerate() { + let staged = match stage_text(&root, prepared, &token, index, injector) { + Ok(staged) => staged, + Err(error) => { + let cleanup_errors = cleanup_staging(&entries); + return if cleanup_errors.is_empty() { + Err(format!("rename: staging failed: {error}; no corpus files were replaced")) + } else { + Err(format!( + "rename: staging failed: {error}; temporary cleanup failed: {}", + cleanup_errors.join("; ") + )) + }; + } + }; + let backup = match temporary_sibling(&root, &prepared.path, &token, index, "backup") { + Ok(backup) => backup, + Err(error) => { + let mut cleanup_errors = Vec::new(); + remove_temp(&staged, &mut cleanup_errors); + cleanup_errors.extend(cleanup_staging(&entries)); + return if cleanup_errors.is_empty() { + Err(format!("rename: staging failed: {error}; no corpus files were replaced")) + } else { + Err(format!( + "rename: staging failed: {error}; temporary cleanup failed: {}", + cleanup_errors.join("; ") + )) + }; + } + }; + entries.push(StagedRenameFile { + path: prepared.path.clone(), + staged, + backup, + backup_moved: false, + installed: false, + }); + } + + // Each replacement is a same-directory rename. Originals move to sibling + // backups first; any failure rolls committed entries back in reverse order. + for index in 0..entries.len() { + let path = entries[index].path.clone(); + if let Err(issue) = check_mutation_path(&root, &path) { + return Err(transaction_failure( + &root, + &mut entries, + path_issue_message("replace", &issue), + )); + } + if let Err(issue) = check_sibling_parent(&root, &entries[index].backup) { + return Err(transaction_failure( + &root, + &mut entries, + path_issue_message("backup", &issue), + )); + } + if injector.should_fail(FailurePoint::Backup(index)) { + return Err(transaction_failure( + &root, + &mut entries, + format!("injected backup failure for {path}"), + )); + } + if let Err(error) = fs::rename(&path, &entries[index].backup) { + return Err(transaction_failure( + &root, + &mut entries, + format!("cannot move {path} to its transaction backup: {error}"), + )); + } + entries[index].backup_moved = true; + + if injector.should_fail(FailurePoint::Replace(index)) { + return Err(transaction_failure( + &root, + &mut entries, + format!("injected replacement failure for {path}"), + )); + } + if let Err(issue) = check_replacement_path(&root, &path) { + return Err(transaction_failure( + &root, + &mut entries, + path_issue_message("replace", &issue), + )); + } + let staged_path = entries[index].staged.to_string_lossy().to_string(); + if let Err(issue) = check_mutation_path(&root, &staged_path) { + return Err(transaction_failure( + &root, + &mut entries, + path_issue_message("replace", &issue), + )); + } + if let Err(error) = fs::rename(&entries[index].staged, &path) { + return Err(transaction_failure( + &root, + &mut entries, + format!("cannot install replacement for {path}: {error}"), + )); + } + entries[index].installed = true; + } + + let cleanup_errors = cleanup_staging(&entries); + if !cleanup_errors.is_empty() { + return Err(format!( + "rename: transaction committed but temporary cleanup failed: {}", + cleanup_errors.join("; ") + )); } Ok(RenameResult { @@ -651,10 +1205,82 @@ pub fn apply_rename(plan: &RenamePlan) -> Result { }) } +/// `apply_rename(plan)` — exact line replacements, original final-newline +/// shape preserved. All files are preflighted and staged before replacement; +/// a later failure rolls earlier replacements back or reports incomplete +/// recovery explicitly. +pub fn apply_rename(plan: &RenamePlan) -> Result { + apply_rename_transaction(plan, FailureInjector::none()) +} + #[cfg(test)] mod tests { use super::*; + static NEXT_TRANSACTION_ROOT: std::sync::atomic::AtomicU64 = + std::sync::atomic::AtomicU64::new(0); + + fn transaction_root() -> PathBuf { + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("clock after epoch") + .as_nanos(); + let sequence = NEXT_TRANSACTION_ROOT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let root = std::env::temp_dir().join(format!( + "asdecided-rename-transaction-{}-{nonce}-{sequence}", + std::process::id(), + )); + fs::create_dir_all(&root).expect("create transaction root"); + root + } + + fn transaction_plan(root: &Path) -> RenamePlan { + let first = root.join("first.md"); + let second = root.join("second.md"); + fs::write(&first, "old\n").expect("write first transaction file"); + fs::write(&second, "old\n").expect("write second transaction file"); + RenamePlan { + directory: root.to_string_lossy().into_owned(), + recursive: true, + old_ref: "old".to_string(), + new_ref: "new".to_string(), + ok: true, + target_path: Some(first.to_string_lossy().into_owned()), + identity_field: Some(IDENTITY_FRONTMATTER), + reason: None, + edits: vec![ + RenameEdit { + path: first.to_string_lossy().into_owned(), + line: 1, + old_line: "old".to_string(), + new_line: "new".to_string(), + kind: KIND_IDENTITY, + }, + RenameEdit { + path: second.to_string_lossy().into_owned(), + line: 1, + old_line: "old".to_string(), + new_line: "new".to_string(), + kind: KIND_REFERENCE, + }, + ], + } + } + + fn assert_no_transaction_temps(root: &Path) { + for entry in fs::read_dir(root).expect("read transaction root") { + let name = entry + .expect("read transaction entry") + .file_name() + .to_string_lossy() + .into_owned(); + assert!( + !name.starts_with(".asdecided-rename-"), + "transaction temporary remains: {name}" + ); + } + } + #[test] fn new_ref_grammar() { assert!(valid_new_ref("ADR-099")); @@ -693,4 +1319,103 @@ mod tests { assert_eq!(frontmatter_id_line("ident: RAC-A"), None); assert_eq!(frontmatter_id_line("id RAC-A"), None); } + + #[test] + fn stale_plan_is_detected_before_any_replacement() { + let root = transaction_root(); + let plan = transaction_plan(&root); + let second = root.join("second.md"); + fs::write(&second, "changed\n").expect("make second file stale"); + + let error = apply_rename(&plan).expect_err("stale plan must fail"); + assert!(error.contains("stale plan"), "{error}"); + assert_eq!(fs::read_to_string(root.join("first.md")).unwrap(), "old\n"); + assert_eq!(fs::read_to_string(second).unwrap(), "changed\n"); + assert_no_transaction_temps(&root); + fs::remove_dir_all(root).expect("remove transaction root"); + } + + #[test] + fn replacement_failure_rolls_back_all_committed_files() { + let root = transaction_root(); + let plan = transaction_plan(&root); + + let error = apply_rename_transaction( + &plan, + FailureInjector::at(FailurePoint::Replace(1)), + ) + .expect_err("injected replacement must fail"); + assert!(error.contains("corpus restored"), "{error}"); + assert_eq!(fs::read_to_string(root.join("first.md")).unwrap(), "old\n"); + assert_eq!(fs::read_to_string(root.join("second.md")).unwrap(), "old\n"); + assert_no_transaction_temps(&root); + fs::remove_dir_all(root).expect("remove transaction root"); + } + + #[test] + fn staging_failure_leaves_corpus_untouched() { + let root = transaction_root(); + let plan = transaction_plan(&root); + + let error = apply_rename_transaction(&plan, FailureInjector::at(FailurePoint::Stage(1))) + .expect_err("injected staging must fail"); + assert!(error.contains("staging failed"), "{error}"); + assert!(error.contains("no corpus files were replaced"), "{error}"); + assert_eq!(fs::read_to_string(root.join("first.md")).unwrap(), "old\n"); + assert_eq!(fs::read_to_string(root.join("second.md")).unwrap(), "old\n"); + assert_no_transaction_temps(&root); + fs::remove_dir_all(root).expect("remove transaction root"); + } + + #[test] + fn backup_failure_rolls_back_prior_replacements() { + let root = transaction_root(); + let plan = transaction_plan(&root); + + let error = apply_rename_transaction(&plan, FailureInjector::at(FailurePoint::Backup(1))) + .expect_err("injected backup must fail"); + assert!(error.contains("corpus restored"), "{error}"); + assert_eq!(fs::read_to_string(root.join("first.md")).unwrap(), "old\n"); + assert_eq!(fs::read_to_string(root.join("second.md")).unwrap(), "old\n"); + assert_no_transaction_temps(&root); + fs::remove_dir_all(root).expect("remove transaction root"); + } + + #[test] + fn successful_transaction_replaces_all_files_and_cleans_backups() { + let root = transaction_root(); + let plan = transaction_plan(&root); + + let result = apply_rename(&plan).expect("transaction succeeds"); + assert!(result.applied); + assert_eq!(result.files_changed, 2); + assert_eq!(fs::read_to_string(root.join("first.md")).unwrap(), "new\n"); + assert_eq!(fs::read_to_string(root.join("second.md")).unwrap(), "new\n"); + assert_no_transaction_temps(&root); + fs::remove_dir_all(root).expect("remove transaction root"); + } + + #[cfg(unix)] + #[test] + fn read_only_file_fails_before_staging() { + if unsafe { libc::geteuid() } == 0 { + return; + } + use std::os::unix::fs::PermissionsExt; + + let root = transaction_root(); + let plan = transaction_plan(&root); + let first = root.join("first.md"); + fs::set_permissions(&first, fs::Permissions::from_mode(0o444)) + .expect("make first file read-only"); + + let error = apply_rename(&plan).expect_err("read-only file must fail"); + fs::set_permissions(&first, fs::Permissions::from_mode(0o644)) + .expect("restore first file permissions"); + assert!(error.contains("read-only"), "{error}"); + assert_eq!(fs::read_to_string(first).unwrap(), "old\n"); + assert_eq!(fs::read_to_string(root.join("second.md")).unwrap(), "old\n"); + assert_no_transaction_temps(&root); + fs::remove_dir_all(root).expect("remove transaction root"); + } } diff --git a/rust/rac-engine/tests/rename.rs b/rust/rac-engine/tests/rename.rs new file mode 100644 index 00000000..751dcfb0 --- /dev/null +++ b/rust/rac-engine/tests/rename.rs @@ -0,0 +1,78 @@ +//! Mutation-boundary tests for `decided rename`. + +#[cfg(unix)] +mod unix { + use std::fs; + use std::os::unix::fs::symlink; + use std::path::PathBuf; + + use rac_engine::output::render_rename_human; + use rac_engine::rename::{apply_rename, compute_rename, REASON_SYMLINK_PATH}; + + const DECISION: &str = "---\nschema_version: 1\nid: RAC-111111111111\ntype: decision\n---\n# Rename boundary\n\n## Context\n\nThe path must stay inside the corpus.\n\n## Decision\n\nReject symlinked mutation paths.\n\n## Consequences\n\nExternal targets remain unchanged.\n\n## Status\n\nAccepted\n"; + + fn scratch_root() -> PathBuf { + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("clock after epoch") + .as_nanos(); + std::env::temp_dir().join(format!( + "asdecided-rename-boundary-{}-{nonce}", + std::process::id() + )) + } + + #[test] + fn dry_run_rejects_symlink_and_apply_cannot_touch_external_target() { + let base = scratch_root(); + let root = base.join("corpus"); + let outside = base.join("outside"); + fs::create_dir_all(&root).expect("create scratch corpus"); + fs::create_dir_all(&outside).expect("create scratch dirs"); + let external = outside.join("decision.md"); + let linked = root.join("decision.md"); + fs::write(&external, DECISION).expect("write external decision"); + symlink(&external, &linked).expect("create corpus symlink"); + + let root_text = root.to_string_lossy().to_string(); + let plan = compute_rename(&root_text, "RAC-111111111111", "RAC-222222222222", true); + + assert!(!plan.ok); + assert_eq!(plan.reason, Some(REASON_SYMLINK_PATH)); + assert_eq!(plan.target_path.as_deref(), Some(linked.to_str().unwrap())); + let human = render_rename_human(&plan); + assert!(human.contains("symlink"), "{human}"); + assert!(human.contains(linked.to_str().unwrap()), "{human}"); + let result = apply_rename(&plan).expect("refused plans are not errors"); + assert!(!result.applied); + assert_eq!(fs::read_to_string(&external).unwrap(), DECISION); + + fs::remove_dir_all(base).expect("remove scratch corpus"); + } + + #[test] + fn apply_rechecks_path_before_replacement() { + let base = scratch_root(); + let root = base.join("corpus"); + fs::create_dir_all(&root).expect("create scratch corpus"); + let target = root.join("decision.md"); + let outside = base.join("external.md"); + fs::write(&target, DECISION).expect("write corpus decision"); + fs::write(&outside, DECISION).expect("write external decision"); + + let root_text = root.to_string_lossy().to_string(); + let plan = compute_rename(&root_text, "RAC-111111111111", "RAC-222222222222", true); + assert!(plan.ok, "unexpected refusal: {:?}", plan.reason); + + fs::remove_file(&target).expect("remove original target"); + symlink(&outside, &target).expect("swap target for symlink"); + let error = match apply_rename(&plan) { + Ok(_) => panic!("symlink swap must be rejected"), + Err(error) => error, + }; + assert!(error.contains("symlink"), "{error}"); + assert_eq!(fs::read_to_string(&outside).unwrap(), DECISION); + + fs::remove_dir_all(base).expect("remove scratch corpus"); + } +}