From 715d2394773fbfac85d170a3f371c6595b9cb2ed Mon Sep 17 00:00:00 2001 From: PJ Date: Wed, 12 Aug 2026 16:48:52 +0530 Subject: [PATCH] fix(chrome): clear storage over cdp instead of scripting an opaque origin Launch runs while the tab is still on about:blank, whose opaque origin denies storage access, so localStorage.clear() threw SecurityError and every web run died at launch. Storage.clearDataForOrigin needs no navigation. The exception helper lands here because "Uncaught" is what hid this for so long. Claude-Session: https://claude.ai/code/session_01Fj4wJUikdABuMQEETwW55J --- internal/driver/chrome/driver.go | 75 +++++++++++++++++++++++---- internal/driver/chrome/driver_test.go | 52 +++++++++++++++++++ internal/driver/chrome/launch_test.go | 47 +++++++++++++++++ 3 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 internal/driver/chrome/launch_test.go diff --git a/internal/driver/chrome/driver.go b/internal/driver/chrome/driver.go index 793e104..2a1a898 100644 --- a/internal/driver/chrome/driver.go +++ b/internal/driver/chrome/driver.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "net/url" "strconv" "strings" "sync" @@ -14,6 +15,7 @@ import ( "github.com/chromedp/cdproto/network" "github.com/chromedp/cdproto/page" "github.com/chromedp/cdproto/runtime" + "github.com/chromedp/cdproto/storage" "github.com/chromedp/chromedp" "github.com/chromedp/chromedp/kb" @@ -91,17 +93,8 @@ func New() *Driver { func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, _ map[string]string) error { if clearState { - if err := chromedp.Run(d.tabCtx, network.ClearBrowserCookies()); err != nil { - return fmt.Errorf("clear cookies: %w", err) - } - if err := chromedp.Run(d.tabCtx, chromedp.ActionFunc(func(ctx context.Context) error { - _, exp, err := runtime.Evaluate(`localStorage.clear(); sessionStorage.clear();`).Do(ctx) - if exp != nil { - return fmt.Errorf("clear storage: %s", exp.Text) - } + if err := d.clearState(bundleID); err != nil { return err - })); err != nil { - return fmt.Errorf("clear storage: %w", err) } } if err := chromedp.Run(d.tabCtx, chromedp.Navigate(bundleID)); err != nil { @@ -125,6 +118,66 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, _ return nil } +// clearState wipes the target's stored data before the application loads. +// Script cannot do it: the tab still sits on about:blank, whose opaque origin +// denies storage access, so `localStorage.clear()` throws SecurityError and +// every web run dies at launch. The Storage domain clears by origin instead, +// which needs no navigation. sessionStorage is per-tab and outside that +// domain's reach; it only survives when a relaunch reuses a tab already on +// the target origin, which is the one case where script can reach it. +func (d *Driver) clearState(bundleID string) error { + if err := chromedp.Run(d.tabCtx, network.ClearBrowserCookies()); err != nil { + return fmt.Errorf("clear cookies: %w", err) + } + origin := securityOrigin(bundleID) + if origin == "" { + return nil + } + clearForOrigin := storage.ClearDataForOrigin(origin, string(storage.TypeAll)) + if err := chromedp.Run(d.tabCtx, clearForOrigin); err != nil { + return fmt.Errorf("clear storage for %s: %w", origin, err) + } + script := fmt.Sprintf( + `location.origin === %q && (sessionStorage.clear(), true)`, origin) + return chromedp.Run(d.tabCtx, chromedp.ActionFunc(func(ctx context.Context) error { + _, exception, err := runtime.Evaluate(script).Do(ctx) + if err != nil { + return fmt.Errorf("clear session storage: %w", err) + } + if exception != nil { + return fmt.Errorf("clear session storage: %s", exceptionMessage(exception)) + } + return nil + })) +} + +// securityOrigin returns the scheme://host[:port] the Storage domain keys data +// by, or "" for a target that has no such origin (data:, file:, about:blank), +// where there is no per-origin storage to clear. +func securityOrigin(bundleID string) string { + parsed, err := url.Parse(bundleID) + if err != nil || parsed.Host == "" { + return "" + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return "" + } + return parsed.Scheme + "://" + parsed.Host +} + +// exceptionMessage renders a page exception for an error string. The +// description carries the actual message ("SecurityError: Failed to read the +// 'localStorage' property..."); Text alone is the useless "Uncaught". +func exceptionMessage(exception *runtime.ExceptionDetails) string { + if exception == nil { + return "" + } + if exception.Exception != nil && exception.Exception.Description != "" { + return exception.Exception.Description + } + return exception.Text +} + func (d *Driver) Terminate(_ context.Context) error { d.tabCancel() d.allocCancel() @@ -483,7 +536,7 @@ func (d *Driver) InstallBundle(ctx context.Context, source []byte) error { return fmt.Errorf("evaluate bundle: %w", err) } if exception != nil { - return fmt.Errorf("bundle threw: %s", exception.Text) + return fmt.Errorf("bundle threw: %s", exceptionMessage(exception)) } return nil }), diff --git a/internal/driver/chrome/driver_test.go b/internal/driver/chrome/driver_test.go index 166d8d8..4eabdb9 100644 --- a/internal/driver/chrome/driver_test.go +++ b/internal/driver/chrome/driver_test.go @@ -6,10 +6,62 @@ import ( "context" "encoding/json" "errors" + "net/http" + "net/http/httptest" "testing" "time" + + "github.com/chromedp/chromedp" ) +// TestLaunch_ClearStateWipesStorageForTheTargetOrigin covers the CLI's default +// path (--clear-data). The tab sits on about:blank when Launch runs, an opaque +// origin that denies storage access, so clearing by script there throws +// SecurityError and kills every web run before the app loads. +func TestLaunch_ClearStateWipesStorageForTheTargetOrigin(t *testing.T) { + const page = `` + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html") + _, _ = w.Write([]byte(page)) + })) + defer server.Close() + + d := New() + defer d.Terminate(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + if err := d.Launch(ctx, server.URL, true, nil); err != nil { + t.Fatalf("Launch with clearState on a fresh tab: %v", err) + } + if err := d.Launch(ctx, server.URL, false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + var visits string + if err := chromedp.Run(d.tabCtx, + chromedp.Evaluate(`localStorage.getItem("visits")`, &visits)); err != nil { + t.Fatalf("read localStorage: %v", err) + } + if visits != "2" { + t.Fatalf("visits = %q, want 2 (two loads, storage kept)", visits) + } + + if err := d.Launch(ctx, server.URL, true, nil); err != nil { + t.Fatalf("Launch with clearState on the target origin: %v", err) + } + if err := chromedp.Run(d.tabCtx, + chromedp.Evaluate(`localStorage.getItem("visits")`, &visits)); err != nil { + t.Fatalf("read localStorage: %v", err) + } + if visits != "1" { + t.Errorf("visits = %q, want 1 (storage cleared before the app loaded)", visits) + } +} + // TestActionMethods_HonorCallerCancellation confirms the DeviceDriver action // methods route through runCtx so a cancelled caller context aborts the CDP // round-trip instead of blocking on d.tabCtx. Without this a hung browser would diff --git a/internal/driver/chrome/launch_test.go b/internal/driver/chrome/launch_test.go new file mode 100644 index 0000000..23d5f68 --- /dev/null +++ b/internal/driver/chrome/launch_test.go @@ -0,0 +1,47 @@ +package chrome + +import ( + "testing" + + "github.com/chromedp/cdproto/runtime" +) + +func TestSecurityOrigin(t *testing.T) { + cases := map[string]string{ + "http://localhost:8088": "http://localhost:8088", + "http://localhost:5173/": "http://localhost:5173", + "https://app.example.com/a/b?c=d": "https://app.example.com", + "data:text/html,hi": "", + "file:///tmp/index.html": "", + "about:blank": "", + "": "", + "://not a url": "", + } + for target, want := range cases { + if got := securityOrigin(target); got != want { + t.Errorf("securityOrigin(%q) = %q, want %q", target, got, want) + } + } +} + +// TestExceptionMessage_PrefersDescription pins the useful half of a page +// exception. ExceptionDetails.Text is "Uncaught" for every throw; the +// description is where the error a caller can act on lives. +func TestExceptionMessage_PrefersDescription(t *testing.T) { + withDescription := &runtime.ExceptionDetails{ + Text: "Uncaught", + Exception: &runtime.RemoteObject{ + Description: "SecurityError: Access is denied for this document.", + }, + } + if got := exceptionMessage(withDescription); got != withDescription.Exception.Description { + t.Errorf("exceptionMessage = %q, want the description", got) + } + textOnly := &runtime.ExceptionDetails{Text: "Uncaught SyntaxError"} + if got := exceptionMessage(textOnly); got != "Uncaught SyntaxError" { + t.Errorf("exceptionMessage = %q, want the text fallback", got) + } + if got := exceptionMessage(nil); got != "" { + t.Errorf("exceptionMessage(nil) = %q, want empty", got) + } +}