Repository navigation
Say that Forget signs you out of every site in the app - #255
Merged
Merged
Conversation
The capture wrote field 1 of every Netscape record as TRUE, which is the include-subdomains flag. The engine reads it: cookie_is_host_only takes only an explicit FALSE, and the match in cookie_findf is domain_match && (!host_only || same_domain). So a session captured for one host was sent to every subdomain the crawl reached, and saved back with a leading dot. Writing FALSE alone would lose cookies instead. A host-only record reaches one host, and the capture only knew the hosts the user browsed, so a crawl starting on a host the login never opened would have gone out without a session. CookieManager answers for a host the browser never opened, so the login now also captures the hosts the crawl starts from, read off the address field. Forget cleared the whole store, which signed the user out of every other project, because the cookie store is per app and setDataDirectorySuffix is per process. It now expires the cookies our own hosts answer with, from each host and its parent domains, and falls back to the whole store only when one of those hosts still answers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <xroche@gmail.com>
The field-4 note explained a byte this change does not touch, and the test already pins it. The other two findings were a colon carrying the logic and a sentence over the length limit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <xroche@gmail.com>
Two review agents measured the cookie-scope half against the vendored engine and it does not hold. Writing field 1 as FALSE is a no-op for any captured host with three or more labels, because the engine's own match already needed the exact host. Where it does change something, it loses cookies: a start host added to the address field after the login goes out logged-out where master sends the cookie. The capture cannot tell a host-only cookie from one carrying Domain, so master guesses domain-wide and that version guessed host-only. Both are guesses. That half goes to its own PR with a design that recovers the real scope. So this PR keeps the Forget fix alone, and the crawl-host seed goes with the half it was protecting. Dropping the seed also drops its own bug: a Forget cleared the host set, and nothing re-seeded it, so the next capture covered fewer hosts than the first. Three fixes the review found in the clear itself. Every value now carries Secure, because Chromium refuses to set a __Host- or __Secure- cookie without it and those prefixes carry the session on most login sites. The host's own Domain form is tried, which a two-label host had no other way to reach. And the residual check asks only about the names it tried, so an unrelated consent cookie no longer reads as a live session and sends the clear on to every other site. The rules moved into BrowserLoginPolicy, where they are pure and tested directly. The source-text guard could not see a negated condition or a neutered callee, which the review demonstrated with four mutants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <xroche@gmail.com>
xroche
changed the title
The second review round killed the narrow clear. Its fallback could not fire for the case its own comment named: getCookie answers for one path, so a cookie on a deeper path never appears in the header, never enters the set of names tried, and never shows up in the re-read. The stated trigger and the stated reason were disjoint. Worse, it forgot less than the code it replaced. A path-scoped session cookie, a nameless cookie, and a session held by an iframe sign-on on a host onPageStarted never reported all survived it, where the old unconditional clear took them. The user presses this to change accounts, so leaving them signed in is the bad direction. Android offers no per-site removal and nothing that enumerates what a narrower pass missed, so there is no safe way to narrow this. The clear stays wide and the message now says so. Splitting the store needs a per-project WebView data directory, which means a separate process, and that is a design rather than a fix. 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 12:00
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.
Forget clears every site in the browser inside this app, not just this project's, and the message said only that the saved login was deleted.
I tried to narrow it to this login's own hosts. A review measured that version and it forgot less than this one does. Three cookies survived it:
getCookieanswersonPageStartednever reportsThe user presses this to change accounts, so staying signed in is the wrong way to fail. Android has no per-site removal, and nothing enumerates what a narrower pass missed. So the clear stays wide and the message now says what it does.
Splitting the store needs a per-project WebView data directory, which means a separate process.