fix(credentials): complete key-store durability follow-ups - #705
Closed
yxlyx wants to merge 1 commit into
Closed
Conversation
Atomic credential replacement synced file contents but not the containing directory, so a hard power loss could lose the rename. Sync the exact destination directory retained by createFileAtomic after replacement, avoiding a path-reopen race if a symlinked parent changes concurrently. Exercise the real Linux/Windows file-backed key-store path through repeated merge/load cycles, and describe its platform security accurately: POSIX files use mode 0600, while Windows inherits the home directory ACL. The separately merged symlink policy remains item 1 of #405; this commit completes items 2–3 without duplicating it. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
Owner
Owner
|
Shipped in v0.0.283 via #703 ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #405 after the symlink-policy portion landed separately in #685.
What changed
createFileAtomicafter an atomic credential-file replacement on POSIX.Why
Problem / failure mode
Atomic replacement synced the new file contents but not the directory entry, so a hard power loss could lose the rename. Reopening the containing directory from the destination path after replacement would add a race: a symlinked parent could be retargeted between rename and fsync, causing the wrong directory to be synced or reporting failure after the replacement already succeeded. The Linux/Windows key-store path also lacked live-platform test coverage, and user-facing help overstated Windows as providing POSIX 0600 semantics.
Reason for this approach
createFileAtomicalready retains the exact directory handle used for its temporary file and rename. Syncingatomic.dirafteratomic.replacepins the actual destination directory without a second path traversal. Tests use the real file-backedstoreKeypath on its live platforms and verify preservation and permissions rather than duplicating serialization logic in a mock.Constraints and trade-offs
Windows keeps its existing weaker durability/security model: directory
FlushFileBuffersis not portable and Zig file modes do not express Windows ACLs, so the implementation relies on atomic replacement, synced file contents, and the containing home-directory ACL. The docs now say this explicitly. Multi-process read-modify-write locking is a separate pre-existing concern and is outside #405's deferred items.Rejected alternatives
dirname(dest)after rename was rejected because pathname resolution can race with symlink-parent retargeting..default_filecarries no equivalent ACL guarantee.Verification
scripts/eval-tier1.sh: green (1865passed + 1 platform skip,465/465TUI,17/17PTY).1865+ 1 skip,465/465TUI, invariants, and SDK all passed; one unrelated intermittenttest-tui-resize-anchor.pyprobe missed a repaint once and then passed immediately viapython3 scripts/test-tui-resize-anchor.py zig-out/bin/graff.atomic.dirlifetime and the path-race fix; no in-scope blockers remain.