Repository navigation
Delete the staged login copy when a project is deleted - #254
Merged
Merged
Conversation
BrowserLoginActivity.writeOwnerOnly stages the cookies under <project>.txt.new and renames it onto <project>.txt, so a write that fails halfway cannot truncate a good login. If the app dies between the write and the rename, the staged file stays on disk. CleanupActivity.forgetLogin deletes every file StoragePaths.isLoginSibling accepts. Its pattern took the login name, an optional ".bak" and an optional ".<pid>.tmp", which are the engine's own leftovers, and never the staged name. So deleting the project left a full session in private storage. The suffix lived as a literal in BrowserLoginActivity and the pattern lived in StoragePaths, which is how the two could disagree. Move the staged name into StoragePaths.stagedLoginFile and build the pattern from the same constant. Reported by the engine-side session while reviewing httrack #1992. Signed-off-by: Xavier Roche <xroche@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <xroche@gmail.com>
Review found two mutations the first round's tests let through. Staging into another directory under the same name kept the matcher agreeing with itself, so the assertion passed while savedLoginFiles, which lists only the logins directory, could no longer see the file. The leak was back with nothing red. Assert the whole path. Calling stagedLoginFile and then opening the stream on an open-coded name left both grepped substrings in place, so the wiring test stayed green while nothing was ever saved. Assert the stream is opened on the staged file. The test named a consequence it never read, so rename it after what it checks. isLoginSibling's first argument takes a login file name, not a project name, so say so in the name and state the summary in the parameters' own order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <xroche@gmail.com>
xroche
changed the title
xroche
enabled auto-merge (squash)
October 21, 2026 06:11
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
1 participant
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.
Saving a browser login stages the cookies under a
.newname, then renames that onto the file the engine reads. That way a write failing halfway cannot truncate a good login. But if the app dies between the two, the staged file stays, and the pattern that picks what a project deletion removes never matched it. So deleting the project left a live session behind.The suffix was a literal in
BrowserLoginActivityand the pattern was inStoragePaths, so the two could disagree. Both now read one constant, and a test ties the matcher to the name the write stages.Found while reviewing xroche/httrack#1992.