Guard browser redirects and outgoing sign-in budget
3 files changed,
+237
−4
internal/home/browser_canary_test.go+24 −2internal/home/browser_driver_boundaries_test.go+188 −0scripts/credential-browser.mjs+25 −2
CI passed — run details
diff --git a/internal/home/browser_canary_test.go b/internal/home/browser_canary_test.go| index d8b1a1d..73334f4 100644 |
| --- a/internal/home/browser_canary_test.go |
| +++ b/internal/home/browser_canary_test.go |
| @@ -58,6 +58,8 @@ func TestCredentialBrowser(t *testing.T) { |
| directoryRequests := 0 |
| directoryCookies := false |
| version := "local-browser-test" |
| + versionRedirect := "" |
| + versionHops := 0 |
| router := app.Router() |
| server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| mu.Lock() |
| @@ -69,8 +71,25 @@ func TestCredentialBrowser(t *testing.T) { |
| directoryCookies = directoryCookies || r.Header.Get("Cookie") != "" |
| } |
| currentVersion := version |
| + redirect := versionRedirect |
| + if r.URL.Path == "/api/version" { |
| + versionHops++ |
| + } |
| + hop := versionHops |
| mu.Unlock() |
| + if r.URL.Path == "/version-hop" { |
| + http.Redirect(w, r, "/api/version", http.StatusFound) |
| + return |
| + } |
| if r.URL.Path == "/api/version" { |
| + if redirect == "version-redirect-other-url" && r.URL.RawQuery == "" { |
| + http.Redirect(w, r, "/api/version?hop=1", http.StatusFound) |
| + return |
| + } |
| + if redirect == "version-redirect-roundtrip" && hop == 1 { |
| + http.Redirect(w, r, "/version-hop", http.StatusFound) |
| + return |
| + } |
| _, _ = w.Write([]byte(currentVersion)) |
| return |
| } |
| @@ -116,7 +135,7 @@ func TestCredentialBrowser(t *testing.T) { |
| } |
| return cmd.CombinedOutput() |
| } |
| - for _, name := range []string{"wrong-version", "wrong-origin", "mailbox-permissions", "unconfigured-live", "recovery-existing", "recovery-symlink"} { |
| + for _, name := range []string{"version-redirect-other-url", "version-redirect-roundtrip", "wrong-version", "wrong-origin", "mailbox-permissions", "unconfigured-live", "recovery-existing", "recovery-symlink"} { |
| t.Run(name, func(t *testing.T) { |
| cfg := make(map[string]any) |
| for k, v := range base { |
| @@ -124,6 +143,8 @@ func TestCredentialBrowser(t *testing.T) { |
| } |
| mu.Lock() |
| before := posts |
| + versionRedirect = name |
| + versionHops = 0 |
| if name == "wrong-version" { |
| version = "wrong" |
| } |
| @@ -171,7 +192,7 @@ func TestCredentialBrowser(t *testing.T) { |
| t.Fatal("invalid refusal report") |
| } |
| expected := "configuration" |
| - if name == "wrong-version" { |
| + if name == "wrong-version" || strings.HasPrefix(name, "version-redirect-") { |
| expected = "version" |
| } |
| if result.Stage != expected { |
| @@ -180,6 +201,7 @@ func TestCredentialBrowser(t *testing.T) { |
| mu.Lock() |
| unchanged := posts == before |
| version = "local-browser-test" |
| + versionRedirect = "" |
| mu.Unlock() |
| if !unchanged { |
| t.Fatal("refusal sent a POST") |
diff --git a/internal/home/browser_driver_boundaries_test.go b/internal/home/browser_driver_boundaries_test.go| new file mode 100644 |
| index 0000000..9c1aef3 |
| --- /dev/null |
| +++ b/internal/home/browser_driver_boundaries_test.go |
| @@ -0,0 +1,188 @@ |
| +package home |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "encoding/json" |
| + "io" |
| + "log" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "os" |
| + "os/exec" |
| + "path/filepath" |
| + "runtime" |
| + "strconv" |
| + "strings" |
| + "sync" |
| + "testing" |
| + "time" |
| +) |
| + |
| +func browserDescendants(pid int) []int { |
| + data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/task/" + strconv.Itoa(pid) + "/children") |
| + if err != nil { |
| + return nil |
| + } |
| + var all []int |
| + for _, field := range strings.Fields(string(data)) { |
| + child, err := strconv.Atoi(field) |
| + if err == nil { |
| + all = append(all, child) |
| + all = append(all, browserDescendants(child)...) |
| + } |
| + } |
| + return all |
| +} |
| + |
| +func browserProcessRunning(pid int) bool { |
| + data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") |
| + if err != nil { |
| + return false |
| + } |
| + end := strings.LastIndex(string(data), ")") |
| + return end < 0 || !strings.HasPrefix(string(data[end+1:]), " Z ") |
| +} |
| + |
| +func TestBrowserDriverOutgoingBudgetAndCancellation(t *testing.T) { |
| + if os.Getenv("HOME_BROWSER_PLAYWRIGHT") == "" { |
| + t.Skip() |
| + } |
| + if runtime.GOOS != "linux" { |
| + t.Skip() |
| + } |
| + for _, mode := range []string{"redirect307", "redirect308", "repeat", "pending-mail-cancel"} { |
| + t.Run(mode, func(t *testing.T) { |
| + origins := Origins{"https://id.home-browser.test", "https://directory.home-browser.test"} |
| + app := newTestAppWithOrigins(t, origins) |
| + router := app.Router() |
| + dir := t.TempDir() |
| + mailbox := filepath.Join(dir, "mailbox.json") |
| + if err := os.WriteFile(mailbox, []byte(`{"sequence":0,"url":""}`), 0600); err != nil { |
| + t.Fatal(err) |
| + } |
| + var mu sync.Mutex |
| + posts := 0 |
| + pending := make(chan struct{}) |
| + var once sync.Once |
| + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + if r.URL.Path == "/api/version" { |
| + _, _ = w.Write([]byte("local-browser-test")) |
| + return |
| + } |
| + if r.Method == "POST" && r.URL.Path == "/signin" { |
| + mu.Lock() |
| + posts++ |
| + mu.Unlock() |
| + once.Do(func() { close(pending) }) |
| + if strings.HasPrefix(mode, "redirect") { |
| + status := 307 |
| + if mode == "redirect308" { |
| + status = 308 |
| + } |
| + http.Redirect(w, r, "/signin", status) |
| + return |
| + } |
| + } |
| + if mode == "repeat" && r.URL.Path == "/repeat.js" { |
| + w.Header().Set("Content-Type", "text/javascript") |
| + _, _ = w.Write([]byte(`for(let i=0;i<6;i++)fetch('/signin',{method:'POST',body:new URLSearchParams({address:'browser@example.invalid'})}).catch(()=>{});`)) |
| + return |
| + } |
| + if mode == "repeat" && r.Method == "GET" && r.URL.Path == "/signin" { |
| + rec := httptest.NewRecorder() |
| + router.ServeHTTP(rec, r) |
| + for k, values := range rec.Header() { |
| + w.Header()[k] = values |
| + } |
| + w.WriteHeader(rec.Code) |
| + _, _ = w.Write(bytes.Replace(rec.Body.Bytes(), []byte("</body>"), []byte(`<script src="/repeat.js"></script></body>`), 1)) |
| + return |
| + } |
| + router.ServeHTTP(w, r) |
| + })) |
| + server.Config.ErrorLog = log.New(io.Discard, "", 0) |
| + server.StartTLS() |
| + defer server.Close() |
| + u, _ := url.Parse(server.URL) |
| + port, _ := strconv.Atoi(u.Port()) |
| + cfg := map[string]any{"mode": "local", "credentialOrigin": origins.Credential, "directoryOrigin": origins.Directory, "localPort": port, "version": "local-browser-test", "address": "browser@example.invalid", "mailboxFile": mailbox} |
| + data, _ := json.Marshal(cfg) |
| + config := filepath.Join(dir, "config.json") |
| + if err := os.WriteFile(config, data, 0600); err != nil { |
| + t.Fatal(err) |
| + } |
| + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) |
| + defer cancel() |
| + cmd := exec.CommandContext(ctx, "node", "../../scripts/credential-browser.mjs") |
| + cmd.Cancel = func() error { return cmd.Process.Signal(os.Interrupt) } |
| + cmd.WaitDelay = 5 * time.Second |
| + cmd.Env = append(os.Environ(), "HOME_BROWSER_CONFIG="+config, "HOME_BROWSER_LOCAL=1", "HOME_BROWSER_LIVE=0") |
| + var output bytes.Buffer |
| + cmd.Stdout = &output |
| + cmd.Stderr = &output |
| + if err := cmd.Start(); err != nil { |
| + t.Fatal(err) |
| + } |
| + finished := make(chan error, 1) |
| + go func() { finished <- cmd.Wait() }() |
| + select { |
| + case <-pending: |
| + case <-time.After(5 * time.Second): |
| + cancel() |
| + <-finished |
| + t.Fatal("browser never reached signin") |
| + } |
| + descendants := browserDescendants(cmd.Process.Pid) |
| + chromium := 0 |
| + for _, pid := range descendants { |
| + name, _ := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/comm") |
| + if strings.Contains(string(name), "chrome") { |
| + chromium++ |
| + } |
| + } |
| + if chromium == 0 { |
| + cancel() |
| + <-finished |
| + t.Fatal("no Chromium descendants observed") |
| + } |
| + time.Sleep(300 * time.Millisecond) |
| + cancel() |
| + select { |
| + case <-finished: |
| + case <-time.After(7 * time.Second): |
| + t.Fatal("cancel did not stop driver") |
| + } |
| + deadline := time.Now().Add(3 * time.Second) |
| + for time.Now().Before(deadline) { |
| + running := false |
| + for _, pid := range descendants { |
| + running = running || browserProcessRunning(pid) |
| + } |
| + if !running { |
| + break |
| + } |
| + time.Sleep(20 * time.Millisecond) |
| + } |
| + for _, pid := range descendants { |
| + if browserProcessRunning(pid) { |
| + process, _ := os.FindProcess(pid) |
| + _ = process.Kill() |
| + t.Error("browser descendant survived cancellation") |
| + } |
| + } |
| + mu.Lock() |
| + actual := posts |
| + mu.Unlock() |
| + if mode == "pending-mail-cancel" { |
| + if actual != 1 { |
| + t.Fatalf("pending-mail POST count=%d", actual) |
| + } |
| + } else if actual != 4 { |
| + t.Fatalf("server observed %d signin POSTs, want bounded4", actual) |
| + } |
| + t.Logf("server signin POSTs=%d; observed Chromium children=%d; all descendants exited", actual, chromium) |
| + }) |
| + } |
| +} |
diff --git a/scripts/credential-browser.mjs b/scripts/credential-browser.mjs| index 36cabe0..6ab5408 100644 |
| --- a/scripts/credential-browser.mjs |
| +++ b/scripts/credential-browser.mjs |
| @@ -58,24 +58,45 @@ try { |
| context = await browser.newContext({ ignoreHTTPSErrors: local }); |
| let external = false; |
| let directoryCookie = false; |
| + let verified = false; |
| + let signinPosts = 0; |
| + let budgetExceeded = false; |
| await context.route('**/*', async route => { |
| const req = route.request(); |
| const u = new URL(req.url()); |
| if (![credential.origin, directory.origin].includes(u.origin)) { external = true; await route.abort(); return; } |
| + if (req.method() === 'POST' && (!verified || req.frame().page() !== page)) { await route.abort(); return; } |
| if (u.origin === directory.origin && (await req.allHeaders()).cookie) directoryCookie = true; |
| await route.continue(); |
| }); |
| const page = await context.newPage(); |
| page.setDefaultTimeout(15000); |
| + const cdp = await context.newCDPSession(page); |
| + await cdp.send('Fetch.enable', { patterns: [{ urlPattern: '*', requestStage: 'Request' }] }); |
| + cdp.on('Fetch.requestPaused', async event => { |
| + try { |
| + const url = new URL(event.request.url); |
| + if (event.request.method === 'POST' && url.pathname === '/signin') { |
| + if (!verified || signinPosts >= 4) { |
| + budgetExceeded = true; |
| + await cdp.send('Fetch.failRequest', { requestId: event.requestId, errorReason: 'Aborted' }); |
| + return; |
| + } |
| + signinPosts++; |
| + } |
| + await cdp.send('Fetch.continueRequest', { requestId: event.requestId }); |
| + } catch { budgetExceeded = true; } |
| + }); |
| stage = 'version'; |
| for (const origin of [credential.origin, directory.origin]) { |
| const response = await page.goto(origin + '/api/version'); |
| + check(response.url() === origin + '/api/version' && response.request().redirectedFrom() === null); |
| check(response.status() === 200 && (await response.text()).trim() === cfg.version); |
| if (!local) check(response.headers()['x-carlos-version'] === cfg.version); |
| } |
| + verified = true; |
| report.checks.push('paired-version'); |
| stage = 'virtual-authenticator'; |
| - const cdp = await context.newCDPSession(page); |
| await cdp.send('WebAuthn.enable'); |
| const { authenticatorId } = await cdp.send('WebAuthn.addVirtualAuthenticator', { options: { |
| protocol: 'ctap2', transport: 'internal', hasResidentKey: true, hasUserVerification: true, |
| @@ -85,6 +106,7 @@ try { |
| async function mailLink() { |
| const deadline = Date.now() + 90000; |
| while (Date.now() < deadline) { |
| + check(!budgetExceeded); |
| const value = await privateJSON(cfg.mailboxFile); |
| if (value.sequence === sequence + 1) { |
| const u = new URL(value.url); |
| @@ -170,7 +192,8 @@ try { |
| check(new URL(page.url()).pathname === '/passkey/confirm'); |
| await cookies(false); |
| report.checks.push('recovery-single-use', 'directory-cookie-isolation'); |
| - check(!external); |
| + check(!external && !budgetExceeded && signinPosts === 4); |
| + report.signinPosts = signinPosts; |
| await cdp.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId }); |
| stage = 'complete'; |
| } catch { |