Skip to content

Persist state through the shared atomic writer - #6

Merged
kridaydave merged 1 commit into
mainfrom
k5/pass-taproot-dedupe-atomic-write
Oct 6, 2026
Merged

kridaydave merged 1 commit into
mainfrom
k5/pass-taproot-dedupe-atomic-write

Conversation

@kridaydave

Copy link
Copy Markdown
Contributor

StateEngine::save carried its own copy of the tempfile / write / fsync / persist / fsync-parent sequence. util::atomic_write implements the same sequence line for line and already serves the registry, keystore and fabric policy writers.

// before: 20 lines of tempfile plumbing inside engine.rs
// after:
pub fn save(path: &Path, signed: &SignedState) -> Result<(), TaprootError> {
    let bytes = serde_json::to_vec_pretty(signed)?;
    crate::util::atomic_write(path, &bytes)
}

Both copies created the parent directory, used tempfile::NamedTempFile in that directory, called sync_all() on the temp file and the parent, and mapped persist errors to TaprootError::Io. No behaviour change: same atomicity, same durability, same error type.

Diff stat: src/engine.rs, 2 insertions, 20 deletions.

Checks

  • cargo fmt --all -- --check clean locally.
  • cargo clippy --locked -- -D warnings (the CI clippy line) clean locally, exit 0.
  • cargo check --locked clean locally.
  • cargo clippy --locked --all-targets -- -D warnings reports the one pre-existing bool_assert_comparison warning at src/fabric.rs:234. That is the known expected warning in the lib test target; the CI gate does not pass --all-targets, so it is not a new finding.

Not shipped

  • Policy.allowed_branches is written by taproot fabric policy-set and by POST /v1/policy/:repo, and read back by policy-get, but never enforced on either push path. Reproduced: with allowed_branches: ["main"] set for org/app, taproot registry push of a state whose base.branch is evil-branch printed pushed and exited 0. Same gap in src/server.rs:71-81. Enforcement is a behaviour change to a security-adjacent guard, so it needs a red-first test I cannot run on this box.
  • Policy.blocked_env_keys and Policy.require_check_strict are stored and never read by any enforcement path.
  • The local push policy check at src/cli.rs:1884 uses resolve_fabric_path(None), so it reads .taproot/fabric under the current directory. RegistryPushArgs has no --fabric flag, so a policy stored anywhere else is silently skipped.

StateEngine::save carried its own copy of the tempfile-persist-fsync
sequence that util::atomic_write already implements, line for line. One
implementation, one place to audit.
@kridaydave
kridaydave merged commit c6a0717 into main Oct 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant