Repository navigation
feat(editor): protect config saves — atomic write + staleness guard - #117
Merged
Merged
Conversation
Closes #116. Core (profiles.core.config.io.yaml_io): - write_text_atomic(): temp file in the target directory, fsync, then os.replace — a crash or power loss mid-save can no longer truncate the user's only .profiles file - content_hash(): SHA-256 hex of a text, the version marker for the staleness guard Server (/api/config): - GET /content now also returns content_hash (hash of the served text) - POST /save accepts an optional baseline_hash: a mismatch with the on-disk file — an external edit landing between the editor's load and its Save — answers 409 instead of silently clobbering; overwrite_changes=true is the "overwrite anyway" escape hatch; the field is optional, so older callers keep the no-check behavior - POST /save and /save-as write through write_text_atomic; both responses carry content_hash (the next save's baseline) Web (ConfigEditorModal): - every load/revert records the baseline hash; every save sends it - a 409 parks a "File changed on disk" ConfirmDialog offering "Reload disk version" (re-fetch, adopt external edits) or "Overwrite anyway" (retry with overwrite_changes) - a successful save refreshes the baseline from the response's content_hash Tests: yaml_io atomic-write/hash unit tests, 409-guard route tests (stale, matching, overwrite-anyway, no-baseline legacy, vanished file), client contract tests, and three editor flow tests (overwrite path, reload path, baseline refresh). Suite 1505 pass, web 246 pass, ruff/pylint (9.89)/pyright clean.
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
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 #116.
What
Two independent protections for the user's only
.profilesfile, exactly as scoped in #116:1. Atomic writes (torn-write protection)
profiles.core.config.io.yaml_iogainswrite_text_atomic(): text goes to a temp file in the target directory (soos.replacestays on one filesystem and remains atomic), is flushed +fsynced, then renamed over the target. A crash or power loss mid-save can no longer leave a truncated config behind. Both/api/config/saveand/api/config/save-asroute through it.2. Staleness guard (silent-clobber protection)
GET /api/config/contentnow also returnscontent_hash(SHA-256 of the served text) — previously the client had no version marker at all.POST /api/config/saveaccepts an optionalbaseline_hash: when the on-disk file no longer matches, it answers 409 ("The configuration file changed on disk since it was loaded") instead of writing.overwrite_changes=trueis the "overwrite anyway" escape hatch. A vanished file also 409s rather than being silently recreated.content_hashof what was written — the next save's baseline.Web (ConfigEditorModal)
ConfirmDialogparks on the in-flight save (same promise pattern as the compliance gate):overwrite_changes: true.Not in scope
/save-as— the editor's baseline describes the file it loaded, not the save-as target; the existing 409 existing-target guard covers that path (documented in the docstring)..bakfiles — the issue's rejected alternatives.Tests
write_text_atomic(writes, replaces, no temp litter — including on a forcedos.replacefailure) +content_hashunit tests.overwrite_changeswins; no-baseline legacy pin; vanished file 409; save-as response hash.ruff format/checkclean, pylint 9.89/10, pyright 0 errors.Docs: the
/api/configendpoint contracts indocs/advanced/advanced-guide.en.md+guide-avance.fr.mdupdated.dist/rebuilt and committed per convention.