Select a live GitHub session cookie across multiple browser stores#18
Merged
Conversation
GetGitHubSession previously returned kooky's cookies[0] filtered only by expiry, so with multiple cookie stores (multiple profiles in any browser, multiple browsers, or Firefox containers) a stale/logged-out user_session could win — passing expiry checks but failing on use. Selection now: gather user_session candidates with per-store identity (FilePath+Container, host-only github.com); prefer stores whose same store has logged_in=yes; and when more than one logged-in candidate remains, validate against GitHub and take the first live one (a lone candidate is used as-is — no point spending a request). It is a picker, not a gate: if validation is inconclusive it returns the most-recent candidate and lets the caller surface the authoritative error. Validation is injected (cookies cannot import session) so extract-token stays offline by passing nil. Adds the package's first cookie tests over a pure, kooky-free core.
kooky derives Creation from the SQLite rowid on the Chromium family (a real cookie's creation reads back as the year 2185), so recency was a false signal dressed up as meaningful. Selection now orders candidates by store key alone — deterministic across kooky's nondeterministic discovery order — and relies on validation, not recency, to choose among multiple live candidates. The actual #15 fix (the logged_in filter) is unaffected.
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.
Problem
cookies.GetGitHubSessionread everyuser_sessioncookie across all browsers/profiles via kooky (filtered only by expiry) and returnedcookies[0]— the first in kooky's nondeterministic store-discovery order. When more than one cookie store holds an unexpireduser_session(multiple profiles in any browser, multiple browsers installed, or Firefox containers — not Firefox-specific), a stale/logged-out cookie can win. It passes expiry checks but fails on use: the repo page returns nouploadToken;check-tokenreturns 302.Closes #15.
Change
Selection now:
user_sessioncandidates with per-store identity (FilePath()+Container), scoped to host-onlygithub.com.logged_in=yes(GitHub sets it on login, removes it on logout) — a free, local, browser-agnostic signal that alone fixes the reported scenario.logged_in=yescan't catch.It's a picker, not a gate: if validation is inconclusive (all fail, or offline), it returns the first candidate (by store key) and lets the caller surface the authoritative error.
check-tokenremains the explicit validity gate.Validation is injected (
validate func(*http.Cookie) error) becauseinternal/sessionimportsinternal/cookies, socookiescan't importsession.extract-tokenpassesnilto stay offline.Notes
--token/GH_SESSION_TOKENbypassGetGitHubSessionentirely and are unaffected.Creationfrom the SQLite rowid on the Chromium family (a real cookie's creation reads back as the year 2185), so it's a false signal. Candidates are ordered by store key alone — deterministic across kooky's nondeterministic discovery order — and validation, not recency, chooses among multiple live candidates.--browser/--profileflag, a multi-account warning, and a clearer logged-out error message.Tests
Adds
internal/cookies/cookies_test.go(the package's first cookie tests) over a pure, kooky-free core — real structs + a stub validator func, no kooky/HTTP mocks. Covers the #15 regression, cross-store correlation guard, subdomain exclusion,logged_invalue handling, and the fullselectSessiondecision matrix (single-skips-validation, short-circuit, fall-through, all-dead fallback, determinism).go build/vet,go test -race -cover ./...all pass.Verified end-to-end against real browser cookies:
logged_inis read from a live Chrome store, the per-store correlation works, andcheck-tokenresolves + validates the session.