registrystack / registrystack/registry-stack
bregctl: finish the descriptor-relative path model (remaining pathname steps, removal bounds, RENAME_NOREPLACE fallback)
- Dominant language
- Rust
- Stars
- 2
- Forks
- 0
- Avg merge
- 2h 55m
- Merged PRs (30d)
- 130
Description
## Context
PR #851 (#831) moved `bregctl` operator path handling onto held directory descriptors: each component is resolved with `openat` and `O_NOFOLLOW`, and reads, creates, appends, renames, publications, listings and removals happen relative to the held descriptor (`crates/registry-bregctl/src/safe_path.rs`). The review of that PR found the places where the model is not yet complete. None is a regression from the PR; each fails closed today, but each keeps a pathname step the rest of the tool no longer has. #881 covers the module asset and reviewed-migrations readers; this issue collects the rest.
## Residuals
- Pathname existence prechecks remain in `load_or_start_import` (`data_lifecycle.rs`), `package_lifecycle.rs`, `audit_lifecycle.rs` and `lib.rs`. Each is followed by the descriptor-relative check that decides, so the precheck is a second name resolution with nothing to add; remove it or resolve once and reuse the entry.
- `run_import` resolves the checkpoint pathname again on every chunk while its state file is resolved once at start, the same split-pair exposure the export run had before #831 (pre-existing).
- `write_migration_files_with_fault` opens the migration destination a second time to record its identity after the bounded read already opened it. Revalidation re-reads and compares, so the invariant holds, but the two opens share the relink window's shape; capture the identity from the first descriptor.
- `registry-breg` still resolves its own writes by pathname (`package.rs`, `runtime_config.rs`), so the final publish step of `bregctl package` is not descriptor-relative.
- `remove_exact_file` (`test_lifecycle.rs`) checks the entry's identity with `stat` and then unlinks by name. POSIX has no identity-bound removal, so the check narrows the window to the gap between the two calls rather than closing it. The guarded names are the tool's own staging and receipt names. Decide whether to document this as the accepted bound or quarantine the name (rename, verify, then unlink) knowing the rename resolves the same name.
- `artifact_destination` depth is unbounded while `MAX_REMOVE_TREE_DEPTH` is 32, so a destination deeper than the removal bound can be created but not removed by the same tool.
- `SafeDir::read_entries` aborts the whole listing on ENOENT for a `DT_UNKNOWN` entry whose no-follow stat finds the name already gone; skipping the vanished entry matches what a directory listing promises.
- The hard-link publication (`publish_new_from`) has not been exercised on a filesystem that lacks `RENAME_NOREPLACE`; only macOS ran locally and the Linux path is the same `rustix` calls. Add a CI or container run on such a filesystem, or a fault test of the fallback arm.
## Acceptance
- Each listed site resolves its operator path once through a held descriptor, or the issue records why it stays.
- A deterministic ancestor-swap test beside the existing ones in `safe_path.rs` for each migrated site.
- The `bregctl` operate page's list of descriptor-resolved paths (added in #851) is updated with each site that moves.
Contributor guide
Research direction
Start with crates/registry-bregctl/src/safe_path.rs and trace the listed call sites in data_lifecycle.rs, package_lifecycle.rs, audit_lifecycle.rs, lib.rs, registry-breg/package.rs, runtime_config.rs, and test_lifecycle.rs. Review the existing ancestor-swap tests, then add coverage for each migrated site and the RENAME_NOREPLACE fallback. Done means the listed paths resolve through held descriptors, removal depth is consistent, vanished entries are skipped, and the bregctl operate page is updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- linux, macos, rust
- Domain
- cli, documentation, security
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100