fix(navigation): bind URI callbacks to their WebView and keep only web origins
URI change callbacks are ignored once their WebView is no longer bound to the pane, so late engine callbacks cannot prompt or recreate per-pane state. The requester shown in the confirmation is only taken from http(s) documents; any other document clears it.
- commit
- 13aa777d272288ce2db68f483ee8de30e29ec865
- author
- brice · · 2026-10-15 07:32 UTC
- parent
- a3a4b09
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +94 -31
M internal/ui/coordinator/content/callbacks.go +1 -1
| @@ -89,7 +89,7 @@ func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.P | |||
| 89 | 89 | c.onProgressChanged(paneID, wv, identity, progress) | |
| 90 | 90 | }, | |
| 91 | 91 | OnURIChanged: func(uri string) { | |
| 92 | - | c.handleURIChanged(ctx, paneID, wv, uri) | |
| 92 | + | c.handleURIChanged(ctx, paneID, wv, identity, uri) | |
| 93 | 93 | }, | |
| 94 | 94 | OnLinkHover: func(uri string) { | |
| 95 | 95 | c.onLinkHover(paneID, uri) | |
| @@ -89,7 +89,7 @@ func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.P | |||||
| 89 | c.onProgressChanged(paneID, wv, identity, progress) | 89 | c.onProgressChanged(paneID, wv, identity, progress) | ||
| 90 | }, | 90 | }, | ||
| 91 | OnURIChanged: func(uri string) { | 91 | OnURIChanged: func(uri string) { | ||
| 92 | - | c.handleURIChanged(ctx, paneID, wv, uri) | 92 | + | c.handleURIChanged(ctx, paneID, wv, identity, uri) |
| 93 | }, | 93 | }, | ||
| 94 | OnLinkHover: func(uri string) { | 94 | OnLinkHover: func(uri string) { | ||
| 95 | c.onLinkHover(paneID, uri) | 95 | c.onLinkHover(paneID, uri) | ||
M internal/ui/coordinator/content/navigation.go +26 -13
| @@ -2,6 +2,7 @@ package content | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | neturl "net/url" | |
| 5 | 6 | "strings" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/dumber/internal/application/port" | |
| @@ -397,10 +398,9 @@ type externalPrompt struct { | |||
| 397 | 398 | // the system handler. A pane has at most one outstanding prompt: further | |
| 398 | 399 | // external URIs from that pane are dropped until it is answered, so a page | |
| 399 | 400 | // cannot flood the window's prompt queue. | |
| 400 | - | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | |
| 401 | + | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, identity webViewIdentity, uri string) { | |
| 401 | 402 | log := logging.FromContext(ctx) | |
| 402 | - | identity, ok := identityForWebView(wv) | |
| 403 | - | if !ok || c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { | |
| 403 | + | if c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { | |
| 404 | 404 | log.Info().Str("pane_id", string(paneID)).Msg("external scheme launch unavailable; blocked") | |
| 405 | 405 | return | |
| 406 | 406 | } | |
| @@ -429,8 +429,7 @@ func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.Pa | |||
| 429 | 429 | delete(c.externalPrompts, paneID) | |
| 430 | 430 | } | |
| 431 | 431 | c.externalPromptMu.Unlock() | |
| 432 | - | bound, ok := identityForWebView(c.GetWebView(paneID)) | |
| 433 | - | current = current && ok && bound == identity | |
| 432 | + | current = current && c.isBoundWebView(paneID, identity) | |
| 434 | 433 | ||
| 435 | 434 | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed).Bool("stale", !current). | |
| 436 | 435 | Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | |
| @@ -441,19 +440,33 @@ func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.Pa | |||
| 441 | 440 | }) | |
| 442 | 441 | } | |
| 443 | 442 | ||
| 444 | - | // rememberExternalPromptOrigin records the host of the last web page shown in | |
| 445 | - | // the pane, so a later external-launch prompt can name the requesting site. | |
| 443 | + | // rememberExternalPromptOrigin records the host of the http(s) document shown | |
| 444 | + | // in the pane, so a later external-launch prompt can name the requesting site. | |
| 445 | + | // Any other document clears it: the prompt then says "A page" rather than | |
| 446 | + | // attributing the request to a previous site. | |
| 446 | 447 | func (c *Coordinator) rememberExternalPromptOrigin(paneID entity.PaneID, uri string) { | |
| 447 | - | host := logging.SafeURLHost(uri) | |
| 448 | + | host := "" | |
| 449 | + | if parsed, err := neturl.Parse(uri); err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") { | |
| 450 | + | host = parsed.Hostname() | |
| 451 | + | } | |
| 452 | + | c.externalPromptMu.Lock() | |
| 453 | + | defer c.externalPromptMu.Unlock() | |
| 448 | 454 | if host == "" { | |
| 455 | + | delete(c.externalPromptOrigins, paneID) | |
| 449 | 456 | return | |
| 450 | 457 | } | |
| 451 | - | c.externalPromptMu.Lock() | |
| 452 | 458 | if c.externalPromptOrigins == nil { | |
| 453 | 459 | c.externalPromptOrigins = make(map[entity.PaneID]string) | |
| 454 | 460 | } | |
| 455 | 461 | c.externalPromptOrigins[paneID] = host | |
| 456 | - | c.externalPromptMu.Unlock() | |
| 462 | + | } | |
| 463 | + | ||
| 464 | + | // isBoundWebView reports whether identity is still the WebView bound to the | |
| 465 | + | // pane. Engine callbacks are posted asynchronously and can arrive after the | |
| 466 | + | // pane was released or rebound. | |
| 467 | + | func (c *Coordinator) isBoundWebView(paneID entity.PaneID, identity webViewIdentity) bool { | |
| 468 | + | bound, ok := identityForWebView(c.GetWebView(paneID)) | |
| 469 | + | return ok && bound == identity | |
| 457 | 470 | } | |
| 458 | 471 | ||
| 459 | 472 | // clearExternalPrompt invalidates the pane's outstanding external-launch | |
| @@ -467,8 +480,8 @@ func (c *Coordinator) clearExternalPrompt(paneID entity.PaneID) { | |||
| 467 | 480 | ||
| 468 | 481 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | |
| 469 | 482 | // and SPA navigation tracking. | |
| 470 | - | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | |
| 471 | - | if uri == "" { | |
| 483 | + | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity, uri string) { | |
| 484 | + | if uri == "" || !c.isBoundWebView(paneID, identity) { | |
| 472 | 485 | return | |
| 473 | 486 | } | |
| 474 | 487 | ||
| @@ -490,7 +503,7 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID | |||
| 490 | 503 | // with a redirect to claude://, vscode://, ...), so the launch is never | |
| 491 | 504 | // automatic: the user must confirm it. Do not navigate back: going back | |
| 492 | 505 | // to an OAuth endpoint re-issues the redirect and loops. | |
| 493 | - | c.promptExternalLaunch(ctx, paneID, wv, uri) | |
| 506 | + | c.promptExternalLaunch(ctx, paneID, identity, uri) | |
| 494 | 507 | return | |
| 495 | 508 | } | |
| 496 | 509 | ||
| @@ -2,6 +2,7 @@ package content | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | neturl "net/url" | |||
| 5 | "strings" | 6 | "strings" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/dumber/internal/application/port" | 8 | "github.com/bnema/dumber/internal/application/port" | ||
| @@ -397,10 +398,9 @@ type externalPrompt struct { | |||||
| 397 | // the system handler. A pane has at most one outstanding prompt: further | 398 | // the system handler. A pane has at most one outstanding prompt: further | ||
| 398 | // external URIs from that pane are dropped until it is answered, so a page | 399 | // external URIs from that pane are dropped until it is answered, so a page | ||
| 399 | // cannot flood the window's prompt queue. | 400 | // cannot flood the window's prompt queue. | ||
| 400 | - | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | 401 | + | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, identity webViewIdentity, uri string) { |
| 401 | log := logging.FromContext(ctx) | 402 | log := logging.FromContext(ctx) | ||
| 402 | - | identity, ok := identityForWebView(wv) | 403 | + | if c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { |
| 403 | - | if !ok || c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { | |||
| 404 | log.Info().Str("pane_id", string(paneID)).Msg("external scheme launch unavailable; blocked") | 404 | log.Info().Str("pane_id", string(paneID)).Msg("external scheme launch unavailable; blocked") | ||
| 405 | return | 405 | return | ||
| 406 | } | 406 | } | ||
| @@ -429,8 +429,7 @@ func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.Pa | |||||
| 429 | delete(c.externalPrompts, paneID) | 429 | delete(c.externalPrompts, paneID) | ||
| 430 | } | 430 | } | ||
| 431 | c.externalPromptMu.Unlock() | 431 | c.externalPromptMu.Unlock() | ||
| 432 | - | bound, ok := identityForWebView(c.GetWebView(paneID)) | 432 | + | current = current && c.isBoundWebView(paneID, identity) |
| 433 | - | current = current && ok && bound == identity | |||
| 434 | 433 | ||||
| 435 | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed).Bool("stale", !current). | 434 | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed).Bool("stale", !current). | ||
| 436 | Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | 435 | Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | ||
| @@ -441,19 +440,33 @@ func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.Pa | |||||
| 441 | }) | 440 | }) | ||
| 442 | } | 441 | } | ||
| 443 | 442 | ||||
| 444 | - | // rememberExternalPromptOrigin records the host of the last web page shown in | 443 | + | // rememberExternalPromptOrigin records the host of the http(s) document shown |
| 445 | - | // the pane, so a later external-launch prompt can name the requesting site. | 444 | + | // in the pane, so a later external-launch prompt can name the requesting site. |
| 445 | + | // Any other document clears it: the prompt then says "A page" rather than | |||
| 446 | + | // attributing the request to a previous site. | |||
| 446 | func (c *Coordinator) rememberExternalPromptOrigin(paneID entity.PaneID, uri string) { | 447 | func (c *Coordinator) rememberExternalPromptOrigin(paneID entity.PaneID, uri string) { | ||
| 447 | - | host := logging.SafeURLHost(uri) | 448 | + | host := "" |
| 449 | + | if parsed, err := neturl.Parse(uri); err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") { | |||
| 450 | + | host = parsed.Hostname() | |||
| 451 | + | } | |||
| 452 | + | c.externalPromptMu.Lock() | |||
| 453 | + | defer c.externalPromptMu.Unlock() | |||
| 448 | if host == "" { | 454 | if host == "" { | ||
| 455 | + | delete(c.externalPromptOrigins, paneID) | |||
| 449 | return | 456 | return | ||
| 450 | } | 457 | } | ||
| 451 | - | c.externalPromptMu.Lock() | |||
| 452 | if c.externalPromptOrigins == nil { | 458 | if c.externalPromptOrigins == nil { | ||
| 453 | c.externalPromptOrigins = make(map[entity.PaneID]string) | 459 | c.externalPromptOrigins = make(map[entity.PaneID]string) | ||
| 454 | } | 460 | } | ||
| 455 | c.externalPromptOrigins[paneID] = host | 461 | c.externalPromptOrigins[paneID] = host | ||
| 456 | - | c.externalPromptMu.Unlock() | 462 | + | } |
| 463 | + | ||||
| 464 | + | // isBoundWebView reports whether identity is still the WebView bound to the | |||
| 465 | + | // pane. Engine callbacks are posted asynchronously and can arrive after the | |||
| 466 | + | // pane was released or rebound. | |||
| 467 | + | func (c *Coordinator) isBoundWebView(paneID entity.PaneID, identity webViewIdentity) bool { | |||
| 468 | + | bound, ok := identityForWebView(c.GetWebView(paneID)) | |||
| 469 | + | return ok && bound == identity | |||
| 457 | } | 470 | } | ||
| 458 | 471 | ||||
| 459 | // clearExternalPrompt invalidates the pane's outstanding external-launch | 472 | // clearExternalPrompt invalidates the pane's outstanding external-launch | ||
| @@ -467,8 +480,8 @@ func (c *Coordinator) clearExternalPrompt(paneID entity.PaneID) { | |||||
| 467 | 480 | ||||
| 468 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | 481 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | ||
| 469 | // and SPA navigation tracking. | 482 | // and SPA navigation tracking. | ||
| 470 | - | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | 483 | + | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity, uri string) { |
| 471 | - | if uri == "" { | 484 | + | if uri == "" || !c.isBoundWebView(paneID, identity) { |
| 472 | return | 485 | return | ||
| 473 | } | 486 | } | ||
| 474 | 487 | ||||
| @@ -490,7 +503,7 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID | |||||
| 490 | // with a redirect to claude://, vscode://, ...), so the launch is never | 503 | // with a redirect to claude://, vscode://, ...), so the launch is never | ||
| 491 | // automatic: the user must confirm it. Do not navigate back: going back | 504 | // automatic: the user must confirm it. Do not navigate back: going back | ||
| 492 | // to an OAuth endpoint re-issues the redirect and loops. | 505 | // to an OAuth endpoint re-issues the redirect and loops. | ||
| 493 | - | c.promptExternalLaunch(ctx, paneID, wv, uri) | 506 | + | c.promptExternalLaunch(ctx, paneID, identity, uri) |
| 494 | return | 507 | return | ||
| 495 | } | 508 | } | ||
| 496 | 509 | ||||
M internal/ui/coordinator/content/navigation_security_test.go +67 -17
| @@ -19,6 +19,15 @@ type externalLaunchRecorder struct { | |||
| 19 | 19 | launched []string | |
| 20 | 20 | } | |
| 21 | 21 | ||
| 22 | + | const testPaneID = entity.PaneID("pane-1") | |
| 23 | + | ||
| 24 | + | // uriChanged delivers a URI change to testPaneID as the engine callback | |
| 25 | + | // installed for wv. | |
| 26 | + | func uriChanged(ctx context.Context, c *Coordinator, wv port.WebView, uri string) { | |
| 27 | + | identity, _ := identityForWebView(wv) | |
| 28 | + | c.handleURIChanged(ctx, testPaneID, wv, identity, uri) | |
| 29 | + | } | |
| 30 | + | ||
| 22 | 31 | // externalWebView returns a WebView mock with ID 1 and the given generation. | |
| 23 | 32 | func externalWebView(t *testing.T, generation uint64) *mocks.MockWebView { | |
| 24 | 33 | wv := mocks.NewMockWebView(t) | |
| @@ -47,10 +56,11 @@ func TestHandleURIChanged_ExternalSchemeWithoutConfirmerIsBlocked(t *testing.T) | |||
| 47 | 56 | ||
| 48 | 57 | launchCount := 0 | |
| 49 | 58 | c := &Coordinator{ | |
| 59 | + | webViews: map[entity.PaneID]port.WebView{"pane-1": wv}, | |
| 50 | 60 | onLaunchExternalURL: func(string) { launchCount++ }, | |
| 51 | 61 | } | |
| 52 | 62 | ||
| 53 | - | c.handleURIChanged(ctx, "pane-1", wv, "vscode://file/etc/passwd") | |
| 63 | + | uriChanged(ctx, c, wv, "vscode://file/etc/passwd") | |
| 54 | 64 | ||
| 55 | 65 | assert.Equal(t, 0, launchCount) | |
| 56 | 66 | } | |
| @@ -67,7 +77,7 @@ func TestHandleURIChanged_ExternalSchemeLaunchesOnlyAfterConfirmation(t *testing | |||
| 67 | 77 | rec.install(c) | |
| 68 | 78 | ||
| 69 | 79 | uri := "claude://login/google-auth?code=abc" | |
| 70 | - | c.handleURIChanged(ctx, "pane-1", wv, uri) | |
| 80 | + | uriChanged(ctx, c, wv, uri) | |
| 71 | 81 | ||
| 72 | 82 | assert.Equal(t, []string{uri}, rec.prompts) | |
| 73 | 83 | assert.Empty(t, rec.launched) | |
| @@ -88,7 +98,7 @@ func TestHandleURIChanged_ExternalSchemeDeniedDoesNotLaunchOrGoBack(t *testing.T | |||
| 88 | 98 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 89 | 99 | rec.install(c) | |
| 90 | 100 | ||
| 91 | - | c.handleURIChanged(ctx, "pane-1", wv, "spotify://track/123") | |
| 101 | + | uriChanged(ctx, c, wv, "spotify://track/123") | |
| 92 | 102 | rec.respond(false) | |
| 93 | 103 | ||
| 94 | 104 | assert.Empty(t, rec.launched) | |
| @@ -106,15 +116,15 @@ func TestHandleURIChanged_OneOutstandingPromptPerPane(t *testing.T) { | |||
| 106 | 116 | rec.install(c) | |
| 107 | 117 | ||
| 108 | 118 | // Alternating URIs must not queue more prompts while one is pending. | |
| 109 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://a") | |
| 110 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://b") | |
| 111 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://a") | |
| 119 | + | uriChanged(ctx, c, wv, "claude://a") | |
| 120 | + | uriChanged(ctx, c, wv, "claude://b") | |
| 121 | + | uriChanged(ctx, c, wv, "claude://a") | |
| 112 | 122 | assert.Equal(t, []string{"claude://a"}, rec.prompts) | |
| 113 | 123 | ||
| 114 | 124 | // Once answered, the pane may prompt again. | |
| 115 | 125 | rec.respond(false) | |
| 116 | 126 | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 117 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://b") | |
| 127 | + | uriChanged(ctx, c, wv, "claude://b") | |
| 118 | 128 | assert.Len(t, rec.prompts, 2) | |
| 119 | 129 | } | |
| 120 | 130 | ||
| @@ -129,7 +139,7 @@ func TestHandleURIChanged_StalePromptAfterReleaseDoesNotLaunch(t *testing.T) { | |||
| 129 | 139 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 130 | 140 | rec.install(c) | |
| 131 | 141 | ||
| 132 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://login?code=x") | |
| 142 | + | uriChanged(ctx, c, wv, "claude://login?code=x") | |
| 133 | 143 | // Pane closed while the prompt was queued. | |
| 134 | 144 | c.clearExternalPrompt("pane-1") | |
| 135 | 145 | delete(c.webViews, "pane-1") | |
| @@ -142,20 +152,21 @@ func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) { | |||
| 142 | 152 | t.Parallel() | |
| 143 | 153 | ||
| 144 | 154 | ctx := context.Background() | |
| 145 | - | paneID := entity.PaneID("pane-1") | |
| 146 | - | wv := mocks.NewMockWebView(t) | |
| 155 | + | paneID := testPaneID | |
| 156 | + | wv := externalWebView(t, 1) | |
| 147 | 157 | wv.EXPECT().IsLoading().Return(false).Once() | |
| 148 | 158 | ||
| 149 | 159 | var recordedPane entity.PaneID | |
| 150 | 160 | var recordedURI string | |
| 151 | 161 | c := &Coordinator{ | |
| 162 | + | webViews: map[entity.PaneID]port.WebView{paneID: wv}, | |
| 152 | 163 | onPaneURIUpdated: func(paneID entity.PaneID, uri string) { | |
| 153 | 164 | recordedPane = paneID | |
| 154 | 165 | recordedURI = uri | |
| 155 | 166 | }, | |
| 156 | 167 | } | |
| 157 | 168 | ||
| 158 | - | c.handleURIChanged(ctx, paneID, wv, "https://example.com/app#state") | |
| 169 | + | uriChanged(ctx, c, wv, "https://example.com/app#state") | |
| 159 | 170 | ||
| 160 | 171 | assert.Equal(t, paneID, recordedPane) | |
| 161 | 172 | assert.Equal(t, "https://example.com/app#state", recordedURI) | |
| @@ -172,7 +183,7 @@ func TestHandleURIChanged_StopErrorDoesNotPanic(t *testing.T) { | |||
| 172 | 183 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 173 | 184 | rec.install(c) | |
| 174 | 185 | ||
| 175 | - | c.handleURIChanged(ctx, "pane-1", wv, "vscode://file/etc/passwd") | |
| 186 | + | uriChanged(ctx, c, wv, "vscode://file/etc/passwd") | |
| 176 | 187 | ||
| 177 | 188 | assert.Len(t, rec.prompts, 1) | |
| 178 | 189 | assert.Empty(t, rec.launched) | |
| @@ -189,11 +200,11 @@ func TestHandleURIChanged_OldAnswerCannotLaunchReplacementPrompt(t *testing.T) { | |||
| 189 | 200 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 190 | 201 | rec.install(c) | |
| 191 | 202 | ||
| 192 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://old") | |
| 203 | + | uriChanged(ctx, c, wv, "claude://old") | |
| 193 | 204 | oldRespond := rec.respond | |
| 194 | 205 | // The pane is invalidated, then the same WebView asks again. | |
| 195 | 206 | c.clearExternalPrompt("pane-1") | |
| 196 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://new") | |
| 207 | + | uriChanged(ctx, c, wv, "claude://new") | |
| 197 | 208 | newRespond := rec.respond | |
| 198 | 209 | ||
| 199 | 210 | oldRespond(true) | |
| @@ -215,7 +226,7 @@ func TestHandleURIChanged_RebindToNewGenerationMakesPromptStale(t *testing.T) { | |||
| 215 | 226 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 216 | 227 | rec.install(c) | |
| 217 | 228 | ||
| 218 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://login?code=x") | |
| 229 | + | uriChanged(ctx, c, wv, "claude://login?code=x") | |
| 219 | 230 | c.webViews["pane-1"] = rebound | |
| 220 | 231 | ||
| 221 | 232 | rec.respond(true) | |
| @@ -234,8 +245,47 @@ func TestHandleURIChanged_PromptNamesLastWebOrigin(t *testing.T) { | |||
| 234 | 245 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 235 | 246 | rec.install(c) | |
| 236 | 247 | ||
| 237 | - | c.handleURIChanged(ctx, "pane-1", wv, "https://claude.ai/login/popup-google-auth?code=x") | |
| 238 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://login") | |
| 248 | + | uriChanged(ctx, c, wv, "https://claude.ai/login/popup-google-auth?code=x") | |
| 249 | + | uriChanged(ctx, c, wv, "claude://login") | |
| 239 | 250 | ||
| 240 | 251 | assert.Equal(t, []string{"claude.ai"}, rec.origins) | |
| 241 | 252 | } | |
| 253 | + | ||
| 254 | + | func TestHandleURIChanged_IgnoresCallbackFromReleasedWebView(t *testing.T) { | |
| 255 | + | t.Parallel() | |
| 256 | + | ||
| 257 | + | ctx := context.Background() | |
| 258 | + | wv := externalWebView(t, 1) | |
| 259 | + | ||
| 260 | + | rec := &externalLaunchRecorder{} | |
| 261 | + | c := &Coordinator{} | |
| 262 | + | rec.install(c) | |
| 263 | + | ||
| 264 | + | // The callback was queued before the pane released its WebView. | |
| 265 | + | uriChanged(ctx, c, wv, "https://site.example/") | |
| 266 | + | uriChanged(ctx, c, wv, "claude://login") | |
| 267 | + | ||
| 268 | + | assert.Empty(t, rec.prompts) | |
| 269 | + | assert.Empty(t, c.externalPromptOrigins) | |
| 270 | + | } | |
| 271 | + | ||
| 272 | + | func TestHandleURIChanged_NonHTTPDocumentClearsOrigin(t *testing.T) { | |
| 273 | + | t.Parallel() | |
| 274 | + | ||
| 275 | + | for _, doc := range []string{"data:text/html,hi", "file://trusted.example/page.html"} { | |
| 276 | + | ctx := context.Background() | |
| 277 | + | wv := externalWebView(t, 1) | |
| 278 | + | wv.EXPECT().IsLoading().Return(true).Times(2) | |
| 279 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 280 | + | ||
| 281 | + | rec := &externalLaunchRecorder{} | |
| 282 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 283 | + | rec.install(c) | |
| 284 | + | ||
| 285 | + | uriChanged(ctx, c, wv, "https://trusted.example/") | |
| 286 | + | uriChanged(ctx, c, wv, doc) | |
| 287 | + | uriChanged(ctx, c, wv, "claude://login") | |
| 288 | + | ||
| 289 | + | assert.Equal(t, []string{""}, rec.origins, doc) | |
| 290 | + | } | |
| 291 | + | } | |
| @@ -19,6 +19,15 @@ type externalLaunchRecorder struct { | |||||
| 19 | launched []string | 19 | launched []string | ||
| 20 | } | 20 | } | ||
| 21 | 21 | ||||
| 22 | + | const testPaneID = entity.PaneID("pane-1") | |||
| 23 | + | ||||
| 24 | + | // uriChanged delivers a URI change to testPaneID as the engine callback | |||
| 25 | + | // installed for wv. | |||
| 26 | + | func uriChanged(ctx context.Context, c *Coordinator, wv port.WebView, uri string) { | |||
| 27 | + | identity, _ := identityForWebView(wv) | |||
| 28 | + | c.handleURIChanged(ctx, testPaneID, wv, identity, uri) | |||
| 29 | + | } | |||
| 30 | + | ||||
| 22 | // externalWebView returns a WebView mock with ID 1 and the given generation. | 31 | // externalWebView returns a WebView mock with ID 1 and the given generation. | ||
| 23 | func externalWebView(t *testing.T, generation uint64) *mocks.MockWebView { | 32 | func externalWebView(t *testing.T, generation uint64) *mocks.MockWebView { | ||
| 24 | wv := mocks.NewMockWebView(t) | 33 | wv := mocks.NewMockWebView(t) | ||
| @@ -47,10 +56,11 @@ func TestHandleURIChanged_ExternalSchemeWithoutConfirmerIsBlocked(t *testing.T) | |||||
| 47 | 56 | ||||
| 48 | launchCount := 0 | 57 | launchCount := 0 | ||
| 49 | c := &Coordinator{ | 58 | c := &Coordinator{ | ||
| 59 | + | webViews: map[entity.PaneID]port.WebView{"pane-1": wv}, | |||
| 50 | onLaunchExternalURL: func(string) { launchCount++ }, | 60 | onLaunchExternalURL: func(string) { launchCount++ }, | ||
| 51 | } | 61 | } | ||
| 52 | 62 | ||||
| 53 | - | c.handleURIChanged(ctx, "pane-1", wv, "vscode://file/etc/passwd") | 63 | + | uriChanged(ctx, c, wv, "vscode://file/etc/passwd") |
| 54 | 64 | ||||
| 55 | assert.Equal(t, 0, launchCount) | 65 | assert.Equal(t, 0, launchCount) | ||
| 56 | } | 66 | } | ||
| @@ -67,7 +77,7 @@ func TestHandleURIChanged_ExternalSchemeLaunchesOnlyAfterConfirmation(t *testing | |||||
| 67 | rec.install(c) | 77 | rec.install(c) | ||
| 68 | 78 | ||||
| 69 | uri := "claude://login/google-auth?code=abc" | 79 | uri := "claude://login/google-auth?code=abc" | ||
| 70 | - | c.handleURIChanged(ctx, "pane-1", wv, uri) | 80 | + | uriChanged(ctx, c, wv, uri) |
| 71 | 81 | ||||
| 72 | assert.Equal(t, []string{uri}, rec.prompts) | 82 | assert.Equal(t, []string{uri}, rec.prompts) | ||
| 73 | assert.Empty(t, rec.launched) | 83 | assert.Empty(t, rec.launched) | ||
| @@ -88,7 +98,7 @@ func TestHandleURIChanged_ExternalSchemeDeniedDoesNotLaunchOrGoBack(t *testing.T | |||||
| 88 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | 98 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | ||
| 89 | rec.install(c) | 99 | rec.install(c) | ||
| 90 | 100 | ||||
| 91 | - | c.handleURIChanged(ctx, "pane-1", wv, "spotify://track/123") | 101 | + | uriChanged(ctx, c, wv, "spotify://track/123") |
| 92 | rec.respond(false) | 102 | rec.respond(false) | ||
| 93 | 103 | ||||
| 94 | assert.Empty(t, rec.launched) | 104 | assert.Empty(t, rec.launched) | ||
| @@ -106,15 +116,15 @@ func TestHandleURIChanged_OneOutstandingPromptPerPane(t *testing.T) { | |||||
| 106 | rec.install(c) | 116 | rec.install(c) | ||
| 107 | 117 | ||||
| 108 | // Alternating URIs must not queue more prompts while one is pending. | 118 | // Alternating URIs must not queue more prompts while one is pending. | ||
| 109 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://a") | 119 | + | uriChanged(ctx, c, wv, "claude://a") |
| 110 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://b") | 120 | + | uriChanged(ctx, c, wv, "claude://b") |
| 111 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://a") | 121 | + | uriChanged(ctx, c, wv, "claude://a") |
| 112 | assert.Equal(t, []string{"claude://a"}, rec.prompts) | 122 | assert.Equal(t, []string{"claude://a"}, rec.prompts) | ||
| 113 | 123 | ||||
| 114 | // Once answered, the pane may prompt again. | 124 | // Once answered, the pane may prompt again. | ||
| 115 | rec.respond(false) | 125 | rec.respond(false) | ||
| 116 | wv.EXPECT().Stop(ctx).Return(nil).Once() | 126 | wv.EXPECT().Stop(ctx).Return(nil).Once() | ||
| 117 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://b") | 127 | + | uriChanged(ctx, c, wv, "claude://b") |
| 118 | assert.Len(t, rec.prompts, 2) | 128 | assert.Len(t, rec.prompts, 2) | ||
| 119 | } | 129 | } | ||
| 120 | 130 | ||||
| @@ -129,7 +139,7 @@ func TestHandleURIChanged_StalePromptAfterReleaseDoesNotLaunch(t *testing.T) { | |||||
| 129 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | 139 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | ||
| 130 | rec.install(c) | 140 | rec.install(c) | ||
| 131 | 141 | ||||
| 132 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://login?code=x") | 142 | + | uriChanged(ctx, c, wv, "claude://login?code=x") |
| 133 | // Pane closed while the prompt was queued. | 143 | // Pane closed while the prompt was queued. | ||
| 134 | c.clearExternalPrompt("pane-1") | 144 | c.clearExternalPrompt("pane-1") | ||
| 135 | delete(c.webViews, "pane-1") | 145 | delete(c.webViews, "pane-1") | ||
| @@ -142,20 +152,21 @@ func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) { | |||||
| 142 | t.Parallel() | 152 | t.Parallel() | ||
| 143 | 153 | ||||
| 144 | ctx := context.Background() | 154 | ctx := context.Background() | ||
| 145 | - | paneID := entity.PaneID("pane-1") | 155 | + | paneID := testPaneID |
| 146 | - | wv := mocks.NewMockWebView(t) | 156 | + | wv := externalWebView(t, 1) |
| 147 | wv.EXPECT().IsLoading().Return(false).Once() | 157 | wv.EXPECT().IsLoading().Return(false).Once() | ||
| 148 | 158 | ||||
| 149 | var recordedPane entity.PaneID | 159 | var recordedPane entity.PaneID | ||
| 150 | var recordedURI string | 160 | var recordedURI string | ||
| 151 | c := &Coordinator{ | 161 | c := &Coordinator{ | ||
| 162 | + | webViews: map[entity.PaneID]port.WebView{paneID: wv}, | |||
| 152 | onPaneURIUpdated: func(paneID entity.PaneID, uri string) { | 163 | onPaneURIUpdated: func(paneID entity.PaneID, uri string) { | ||
| 153 | recordedPane = paneID | 164 | recordedPane = paneID | ||
| 154 | recordedURI = uri | 165 | recordedURI = uri | ||
| 155 | }, | 166 | }, | ||
| 156 | } | 167 | } | ||
| 157 | 168 | ||||
| 158 | - | c.handleURIChanged(ctx, paneID, wv, "https://example.com/app#state") | 169 | + | uriChanged(ctx, c, wv, "https://example.com/app#state") |
| 159 | 170 | ||||
| 160 | assert.Equal(t, paneID, recordedPane) | 171 | assert.Equal(t, paneID, recordedPane) | ||
| 161 | assert.Equal(t, "https://example.com/app#state", recordedURI) | 172 | assert.Equal(t, "https://example.com/app#state", recordedURI) | ||
| @@ -172,7 +183,7 @@ func TestHandleURIChanged_StopErrorDoesNotPanic(t *testing.T) { | |||||
| 172 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | 183 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | ||
| 173 | rec.install(c) | 184 | rec.install(c) | ||
| 174 | 185 | ||||
| 175 | - | c.handleURIChanged(ctx, "pane-1", wv, "vscode://file/etc/passwd") | 186 | + | uriChanged(ctx, c, wv, "vscode://file/etc/passwd") |
| 176 | 187 | ||||
| 177 | assert.Len(t, rec.prompts, 1) | 188 | assert.Len(t, rec.prompts, 1) | ||
| 178 | assert.Empty(t, rec.launched) | 189 | assert.Empty(t, rec.launched) | ||
| @@ -189,11 +200,11 @@ func TestHandleURIChanged_OldAnswerCannotLaunchReplacementPrompt(t *testing.T) { | |||||
| 189 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | 200 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | ||
| 190 | rec.install(c) | 201 | rec.install(c) | ||
| 191 | 202 | ||||
| 192 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://old") | 203 | + | uriChanged(ctx, c, wv, "claude://old") |
| 193 | oldRespond := rec.respond | 204 | oldRespond := rec.respond | ||
| 194 | // The pane is invalidated, then the same WebView asks again. | 205 | // The pane is invalidated, then the same WebView asks again. | ||
| 195 | c.clearExternalPrompt("pane-1") | 206 | c.clearExternalPrompt("pane-1") | ||
| 196 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://new") | 207 | + | uriChanged(ctx, c, wv, "claude://new") |
| 197 | newRespond := rec.respond | 208 | newRespond := rec.respond | ||
| 198 | 209 | ||||
| 199 | oldRespond(true) | 210 | oldRespond(true) | ||
| @@ -215,7 +226,7 @@ func TestHandleURIChanged_RebindToNewGenerationMakesPromptStale(t *testing.T) { | |||||
| 215 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | 226 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | ||
| 216 | rec.install(c) | 227 | rec.install(c) | ||
| 217 | 228 | ||||
| 218 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://login?code=x") | 229 | + | uriChanged(ctx, c, wv, "claude://login?code=x") |
| 219 | c.webViews["pane-1"] = rebound | 230 | c.webViews["pane-1"] = rebound | ||
| 220 | 231 | ||||
| 221 | rec.respond(true) | 232 | rec.respond(true) | ||
| @@ -234,8 +245,47 @@ func TestHandleURIChanged_PromptNamesLastWebOrigin(t *testing.T) { | |||||
| 234 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | 245 | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | ||
| 235 | rec.install(c) | 246 | rec.install(c) | ||
| 236 | 247 | ||||
| 237 | - | c.handleURIChanged(ctx, "pane-1", wv, "https://claude.ai/login/popup-google-auth?code=x") | 248 | + | uriChanged(ctx, c, wv, "https://claude.ai/login/popup-google-auth?code=x") |
| 238 | - | c.handleURIChanged(ctx, "pane-1", wv, "claude://login") | 249 | + | uriChanged(ctx, c, wv, "claude://login") |
| 239 | 250 | ||||
| 240 | assert.Equal(t, []string{"claude.ai"}, rec.origins) | 251 | assert.Equal(t, []string{"claude.ai"}, rec.origins) | ||
| 241 | } | 252 | } | ||
| 253 | + | ||||
| 254 | + | func TestHandleURIChanged_IgnoresCallbackFromReleasedWebView(t *testing.T) { | |||
| 255 | + | t.Parallel() | |||
| 256 | + | ||||
| 257 | + | ctx := context.Background() | |||
| 258 | + | wv := externalWebView(t, 1) | |||
| 259 | + | ||||
| 260 | + | rec := &externalLaunchRecorder{} | |||
| 261 | + | c := &Coordinator{} | |||
| 262 | + | rec.install(c) | |||
| 263 | + | ||||
| 264 | + | // The callback was queued before the pane released its WebView. | |||
| 265 | + | uriChanged(ctx, c, wv, "https://site.example/") | |||
| 266 | + | uriChanged(ctx, c, wv, "claude://login") | |||
| 267 | + | ||||
| 268 | + | assert.Empty(t, rec.prompts) | |||
| 269 | + | assert.Empty(t, c.externalPromptOrigins) | |||
| 270 | + | } | |||
| 271 | + | ||||
| 272 | + | func TestHandleURIChanged_NonHTTPDocumentClearsOrigin(t *testing.T) { | |||
| 273 | + | t.Parallel() | |||
| 274 | + | ||||
| 275 | + | for _, doc := range []string{"data:text/html,hi", "file://trusted.example/page.html"} { | |||
| 276 | + | ctx := context.Background() | |||
| 277 | + | wv := externalWebView(t, 1) | |||
| 278 | + | wv.EXPECT().IsLoading().Return(true).Times(2) | |||
| 279 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |||
| 280 | + | ||||
| 281 | + | rec := &externalLaunchRecorder{} | |||
| 282 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |||
| 283 | + | rec.install(c) | |||
| 284 | + | ||||
| 285 | + | uriChanged(ctx, c, wv, "https://trusted.example/") | |||
| 286 | + | uriChanged(ctx, c, wv, doc) | |||
| 287 | + | uriChanged(ctx, c, wv, "claude://login") | |||
| 288 | + | ||||
| 289 | + | assert.Equal(t, []string{""}, rec.origins, doc) | |||
| 290 | + | } | |||
| 291 | + | } | |||