fix(lite): state-save hardening — locking, torn records, .bak rotation, name sanitising - #191
Merged
Conversation
…n, name sanitising
Four fixes to the save/restore path, all in the class the restore matrix exists to guard:
losing saved sessions quietly.
- g_userEmptied is set and read UNDER THE LOCK, with the session list it was judged from.
It is written from the control-pipe thread (closeSessionAt) and read on the UI thread,
and it gates both the zero-session refusal and the .bak delete — the two decisions that
can cost saved sessions.
- stateFileSessionCount counts only records that would actually RESTORE (>=4 tabs), not
every line beginning "S\t". A record cut mid-write parses to nothing, so counting it
would let the save rotate that wreckage into the .bak, over the one generation still
holding the sessions, and call a good file "not empty".
- Rotation keeps a generation only when the primary is worth keeping, and drops the .bak
only when the user emptied the window on purpose — otherwise the next launch reads the
fallback and brings back exactly the sessions they just closed. A primary that exists but
cannot be read right now (AV scan, transient lock) still counts as worth keeping.
- Instance names from the HKCU registry are sanitised before becoming filenames: an older
build or a hand edit could hold "..\..\x", which would delete state outside the state
directory.
Also: saveSessionState logs when it returns on an empty path. That was the last way to get
"restore doesn't work" with nothing in the log, on exactly the redirected or policy-locked
profile the field reports come from.
New matrix cell `inplace-fallback` covers the half of the atomic write nothing else reached:
the temp file cannot be CREATED at all (policy-locked profile, DLP/AV agent blocking new
files), so the save falls back to writing in place. publish-blocked locks the PRIMARY, which
forces a failed publish rather than a failed create, so this path shipped untested. It must
still keep both promises: a generation in the .bak, and that .bak dropped on a deliberate
empty.
lite suite green (30 matrix cells + log/diagnose suites).
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.
Four fixes to the save/restore path, all in the class the restore matrix exists to guard: losing saved sessions quietly.
g_userEmptiedis set and read under the lock, with the session list it was judged from. It's written from the control-pipe thread (closeSessionAt) and read on the UI thread, and it gates both the zero-session refusal and the.bakdelete — the two decisions that can cost saved sessions.stateFileSessionCountcounts only records that would actually restore (≥4 tabs), not every line beginningS\t. A record cut mid-write parses to nothing, so counting it would let the save rotate that wreckage into the.bak— over the one generation still holding the sessions — and call a good file "not empty"..bakonly when the user emptied the window on purpose. Otherwise the next launch reads the fallback and brings back exactly the sessions they just closed. A primary that exists but can't be read right now (AV scan, transient lock) still counts as worth keeping...\..\x, which would delete state outside the state directory.Also:
saveSessionStatenow logs when it returns on an empty path. That was the last remaining way to get "restore doesn't work" with nothing in the log — on exactly the redirected or policy-locked profile the field reports come from.New coverage
inplace-fallbackcovers the half of the atomic write nothing else reached: the temp file can't be created at all (policy-locked profile, DLP/AV agent blocking new files), so the save falls back to writing in place.publish-blockedlocks the primary, which forces a failed publish rather than a failed create — so this path shipped untested. It must still keep both promises: a generation in the.bak, and that.bakdropped on a deliberate empty.Verification
Full lite suite green — 30 matrix cells plus the log and diagnose suites:
Authored by Boris; landed as-is.