detached e4b2016

fix(security): require trust for external scheme launches

commit
e4b2016a9f0f214e299b674150bbb46b59295a8b
author
brice · · 2026-06-20 09:49 UTC
committer
brice · · 2026-06-20 10:01 UTC
parent
2be8535
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
2 files · +128 -13
Unified
UnifiedSplit
M internal/ui/coordinator/content/navigation.go +7 -13
@@ -330,25 +330,19 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID
330330 isExternal := urlutil.IsExternalScheme(uri)
331331
332332 if isExternal {
333- log.Info().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external scheme detected, launching externally")
333+ log.Info().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external scheme URI change blocked")
334334
335- // Launch externally via injected callback
336- if c.onLaunchExternalURL != nil {
337- c.onLaunchExternalURL(uri)
338- } else {
339- log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external URL not launched: no handler registered")
340- }
341-
342- // Stop loading to prevent WebKit from showing an error page
343- // The page stays on the previous URL before the JS redirect
335+ // External schemes must only be launched from WebKit's navigation-policy path,
336+ // where NavigationAction.IsUserGesture is available and verified. URI changes can
337+ // be driven by page script or redirects, so this path only cleans up WebKit state.
344338 if err := wv.Stop(ctx); err != nil {
345- log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Err(err).Msg("stop webview for external URL")
339+ log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Err(err).Msg("stop webview for blocked external URL")
346340 }
347341
348- // Navigate back to avoid stale URI in omnibox/history
342+ // Navigate back to avoid stale URI in omnibox/history.
349343 if wv.CanGoBack() {
350344 if err := wv.GoBack(ctx); err != nil {
351- log.Warn().Str("pane_id", string(paneID)).Err(err).Msg("GoBack after external URL")
345+ log.Warn().Str("pane_id", string(paneID)).Err(err).Msg("GoBack after blocked external URL")
352346 }
353347 }
354348 return
@@ -330,25 +330,19 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID
330 isExternal := urlutil.IsExternalScheme(uri)330 isExternal := urlutil.IsExternalScheme(uri)
331 331
332 if isExternal {332 if isExternal {
333- log.Info().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external scheme detected, launching externally")333+ log.Info().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external scheme URI change blocked")
334 334
335- // Launch externally via injected callback335+ // External schemes must only be launched from WebKit's navigation-policy path,
336- if c.onLaunchExternalURL != nil {336+ // where NavigationAction.IsUserGesture is available and verified. URI changes can
337- c.onLaunchExternalURL(uri)337+ // be driven by page script or redirects, so this path only cleans up WebKit state.
338- } else {
339- log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external URL not launched: no handler registered")
340- }
341-
342- // Stop loading to prevent WebKit from showing an error page
343- // The page stays on the previous URL before the JS redirect
344 if err := wv.Stop(ctx); err != nil {338 if err := wv.Stop(ctx); err != nil {
345- log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Err(err).Msg("stop webview for external URL")339+ log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Err(err).Msg("stop webview for blocked external URL")
346 }340 }
347 341
348- // Navigate back to avoid stale URI in omnibox/history342+ // Navigate back to avoid stale URI in omnibox/history.
349 if wv.CanGoBack() {343 if wv.CanGoBack() {
350 if err := wv.GoBack(ctx); err != nil {344 if err := wv.GoBack(ctx); err != nil {
351- log.Warn().Str("pane_id", string(paneID)).Err(err).Msg("GoBack after external URL")345+ log.Warn().Str("pane_id", string(paneID)).Err(err).Msg("GoBack after blocked external URL")
352 }346 }
353 }347 }
354 return348 return
A internal/ui/coordinator/content/navigation_security_test.go +121 -0
@@ -0,0 +1,121 @@
1+package content
2+
3+import (
4+ "context"
5+ "errors"
6+ "testing"
7+
8+ "github.com/stretchr/testify/assert"
9+
10+ "github.com/bnema/dumber/internal/application/port/mocks"
11+ "github.com/bnema/dumber/internal/domain/entity"
12+)
13+
14+func TestHandleURIChanged_BlocksExternalSchemeWithoutLaunching(t *testing.T) {
15+ t.Parallel()
16+
17+ ctx := context.Background()
18+ paneID := entity.PaneID("pane-1")
19+ wv := mocks.NewMockWebView(t)
20+ wv.EXPECT().Stop(ctx).Return(nil).Once()
21+ wv.EXPECT().CanGoBack().Return(false).Once()
22+
23+ launchCount := 0
24+ c := &Coordinator{
25+ onLaunchExternalURL: func(uri string) {
26+ launchCount++
27+ },
28+ }
29+
30+ c.handleURIChanged(ctx, paneID, wv, "vscode://file/etc/passwd")
31+
32+ assert.Equal(t, 0, launchCount)
33+}
34+
35+func TestHandleURIChanged_BlocksExternalSchemeRedirectAndRestoresPreviousPage(t *testing.T) {
36+ t.Parallel()
37+
38+ ctx := context.Background()
39+ paneID := entity.PaneID("pane-1")
40+ wv := mocks.NewMockWebView(t)
41+ wv.EXPECT().Stop(ctx).Return(nil).Once()
42+ wv.EXPECT().CanGoBack().Return(true).Once()
43+ wv.EXPECT().GoBack(ctx).Return(nil).Once()
44+
45+ launchCount := 0
46+ c := &Coordinator{
47+ onLaunchExternalURL: func(uri string) {
48+ launchCount++
49+ },
50+ }
51+
52+ c.handleURIChanged(ctx, paneID, wv, "spotify://track/123")
53+
54+ assert.Equal(t, 0, launchCount)
55+}
56+
57+func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) {
58+ t.Parallel()
59+
60+ ctx := context.Background()
61+ paneID := entity.PaneID("pane-1")
62+ wv := mocks.NewMockWebView(t)
63+ wv.EXPECT().IsLoading().Return(false).Once()
64+
65+ var recordedPane entity.PaneID
66+ var recordedURI string
67+ c := &Coordinator{
68+ onPaneURIUpdated: func(paneID entity.PaneID, uri string) {
69+ recordedPane = paneID
70+ recordedURI = uri
71+ },
72+ }
73+
74+ c.handleURIChanged(ctx, paneID, wv, "https://example.com/app#state")
75+
76+ assert.Equal(t, paneID, recordedPane)
77+ assert.Equal(t, "https://example.com/app#state", recordedURI)
78+}
79+
80+func TestHandleURIChanged_StopErrorDoesNotPanic(t *testing.T) {
81+ t.Parallel()
82+
83+ ctx := context.Background()
84+ paneID := entity.PaneID("pane-1")
85+ wv := mocks.NewMockWebView(t)
86+ wv.EXPECT().Stop(ctx).Return(errors.New("stop failed")).Once()
87+ wv.EXPECT().CanGoBack().Return(false).Once()
88+
89+ launchCount := 0
90+ c := &Coordinator{
91+ onLaunchExternalURL: func(uri string) {
92+ launchCount++
93+ },
94+ }
95+
96+ c.handleURIChanged(ctx, paneID, wv, "vscode://file/etc/passwd")
97+
98+ assert.Equal(t, 0, launchCount)
99+}
100+
101+func TestHandleURIChanged_GoBackErrorDoesNotPanic(t *testing.T) {
102+ t.Parallel()
103+
104+ ctx := context.Background()
105+ paneID := entity.PaneID("pane-1")
106+ wv := mocks.NewMockWebView(t)
107+ wv.EXPECT().Stop(ctx).Return(nil).Once()
108+ wv.EXPECT().CanGoBack().Return(true).Once()
109+ wv.EXPECT().GoBack(ctx).Return(errors.New("goback failed")).Once()
110+
111+ launchCount := 0
112+ c := &Coordinator{
113+ onLaunchExternalURL: func(uri string) {
114+ launchCount++
115+ },
116+ }
117+
118+ c.handleURIChanged(ctx, paneID, wv, "spotify://track/123")
119+
120+ assert.Equal(t, 0, launchCount)
121+}
@@ -0,0 +1,121 @@
1+package content
2+
3+import (
4+ "context"
5+ "errors"
6+ "testing"
7+
8+ "github.com/stretchr/testify/assert"
9+
10+ "github.com/bnema/dumber/internal/application/port/mocks"
11+ "github.com/bnema/dumber/internal/domain/entity"
12+)
13+
14+func TestHandleURIChanged_BlocksExternalSchemeWithoutLaunching(t *testing.T) {
15+ t.Parallel()
16+
17+ ctx := context.Background()
18+ paneID := entity.PaneID("pane-1")
19+ wv := mocks.NewMockWebView(t)
20+ wv.EXPECT().Stop(ctx).Return(nil).Once()
21+ wv.EXPECT().CanGoBack().Return(false).Once()
22+
23+ launchCount := 0
24+ c := &Coordinator{
25+ onLaunchExternalURL: func(uri string) {
26+ launchCount++
27+ },
28+ }
29+
30+ c.handleURIChanged(ctx, paneID, wv, "vscode://file/etc/passwd")
31+
32+ assert.Equal(t, 0, launchCount)
33+}
34+
35+func TestHandleURIChanged_BlocksExternalSchemeRedirectAndRestoresPreviousPage(t *testing.T) {
36+ t.Parallel()
37+
38+ ctx := context.Background()
39+ paneID := entity.PaneID("pane-1")
40+ wv := mocks.NewMockWebView(t)
41+ wv.EXPECT().Stop(ctx).Return(nil).Once()
42+ wv.EXPECT().CanGoBack().Return(true).Once()
43+ wv.EXPECT().GoBack(ctx).Return(nil).Once()
44+
45+ launchCount := 0
46+ c := &Coordinator{
47+ onLaunchExternalURL: func(uri string) {
48+ launchCount++
49+ },
50+ }
51+
52+ c.handleURIChanged(ctx, paneID, wv, "spotify://track/123")
53+
54+ assert.Equal(t, 0, launchCount)
55+}
56+
57+func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) {
58+ t.Parallel()
59+
60+ ctx := context.Background()
61+ paneID := entity.PaneID("pane-1")
62+ wv := mocks.NewMockWebView(t)
63+ wv.EXPECT().IsLoading().Return(false).Once()
64+
65+ var recordedPane entity.PaneID
66+ var recordedURI string
67+ c := &Coordinator{
68+ onPaneURIUpdated: func(paneID entity.PaneID, uri string) {
69+ recordedPane = paneID
70+ recordedURI = uri
71+ },
72+ }
73+
74+ c.handleURIChanged(ctx, paneID, wv, "https://example.com/app#state")
75+
76+ assert.Equal(t, paneID, recordedPane)
77+ assert.Equal(t, "https://example.com/app#state", recordedURI)
78+}
79+
80+func TestHandleURIChanged_StopErrorDoesNotPanic(t *testing.T) {
81+ t.Parallel()
82+
83+ ctx := context.Background()
84+ paneID := entity.PaneID("pane-1")
85+ wv := mocks.NewMockWebView(t)
86+ wv.EXPECT().Stop(ctx).Return(errors.New("stop failed")).Once()
87+ wv.EXPECT().CanGoBack().Return(false).Once()
88+
89+ launchCount := 0
90+ c := &Coordinator{
91+ onLaunchExternalURL: func(uri string) {
92+ launchCount++
93+ },
94+ }
95+
96+ c.handleURIChanged(ctx, paneID, wv, "vscode://file/etc/passwd")
97+
98+ assert.Equal(t, 0, launchCount)
99+}
100+
101+func TestHandleURIChanged_GoBackErrorDoesNotPanic(t *testing.T) {
102+ t.Parallel()
103+
104+ ctx := context.Background()
105+ paneID := entity.PaneID("pane-1")
106+ wv := mocks.NewMockWebView(t)
107+ wv.EXPECT().Stop(ctx).Return(nil).Once()
108+ wv.EXPECT().CanGoBack().Return(true).Once()
109+ wv.EXPECT().GoBack(ctx).Return(errors.New("goback failed")).Once()
110+
111+ launchCount := 0
112+ c := &Coordinator{
113+ onLaunchExternalURL: func(uri string) {
114+ launchCount++
115+ },
116+ }
117+
118+ c.handleURIChanged(ctx, paneID, wv, "spotify://track/123")
119+
120+ assert.Equal(t, 0, launchCount)
121+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close