detached 128e7ed

fix(desktop): redact browser launch error details

commit
128e7eddbc3e6c574f6400f668d78c050f13b564
author
brice · · 2026-05-23 05:11 UTC
parent
7c67957
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +58 -6
Split
UnifiedSplit
M internal/infrastructure/desktop/adapter.go +2 -2
@@ -422,13 +422,13 @@ func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, er
422422
423423 execPath, err := resolveExecutablePath()
424424 if err != nil {
425- return fmt.Errorf("failed to resolve dumber executable for URL %q: %w", uri, err)
425+ return fmt.Errorf("failed to resolve dumber executable for requested URL: %w", err)
426426 }
427427
428428 cmd := exec.Command(execPath, "browse", uri)
429429 cmd.Env = sanitizedChildEnv(os.Environ())
430430 if err := spawnDetachedProcess(cmd); err != nil {
431- return fmt.Errorf("failed to launch dumber browse for URL %q: %w", uri, err)
431+ return fmt.Errorf("failed to launch dumber browse for requested URL: %w", err)
432432 }
433433 return nil
434434 }
@@ -422,13 +422,13 @@ func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, er
422 422
423 execPath, err := resolveExecutablePath()423 execPath, err := resolveExecutablePath()
424 if err != nil {424 if err != nil {
425- return fmt.Errorf("failed to resolve dumber executable for URL %q: %w", uri, err)425+ return fmt.Errorf("failed to resolve dumber executable for requested URL: %w", err)
426 }426 }
427 427
428 cmd := exec.Command(execPath, "browse", uri)428 cmd := exec.Command(execPath, "browse", uri)
429 cmd.Env = sanitizedChildEnv(os.Environ())429 cmd.Env = sanitizedChildEnv(os.Environ())
430 if err := spawnDetachedProcess(cmd); err != nil {430 if err := spawnDetachedProcess(cmd); err != nil {
431- return fmt.Errorf("failed to launch dumber browse for URL %q: %w", uri, err)431+ return fmt.Errorf("failed to launch dumber browse for requested URL: %w", err)
432 }432 }
433 return nil433 return nil
434 }434 }
M internal/infrastructure/desktop/adapter_test.go +42 -0
@@ -1,7 +1,9 @@
11 package desktop
22
33 import (
4+ "errors"
45 "os"
6+ "os/exec"
57 "path/filepath"
68 "strings"
79 "testing"
@@ -174,6 +176,46 @@ func TestSessionSpawner_SpawnWithSession_OmitsEngineOverrideWhenNotConfigured(t
174176 }
175177 }
176178
179+func TestLaunchBrowserBrowseURL_ResolveErrorDoesNotLeakURL(t *testing.T) {
180+ wantErr := errors.New("resolve failed")
181+
182+ err := launchBrowserBrowseURL(
183+ "https://example.com/private?token=secret",
184+ func() (string, error) { return "", wantErr },
185+ nil,
186+ )
187+
188+ if err == nil {
189+ t.Fatal("expected resolve error")
190+ }
191+ if !errors.Is(err, wantErr) {
192+ t.Fatalf("expected error %v, got %v", wantErr, err)
193+ }
194+ if strings.Contains(err.Error(), "token=secret") || strings.Contains(err.Error(), "https://example.com") {
195+ t.Fatalf("expected error to redact URL, got %q", err)
196+ }
197+}
198+
199+func TestLaunchBrowserBrowseURL_SpawnErrorDoesNotLeakURL(t *testing.T) {
200+ wantErr := errors.New("spawn failed")
201+
202+ err := launchBrowserBrowseURL(
203+ "https://example.com/private?token=secret",
204+ func() (string, error) { return "/usr/bin/dumber", nil },
205+ func(*exec.Cmd) error { return wantErr },
206+ )
207+
208+ if err == nil {
209+ t.Fatal("expected spawn error")
210+ }
211+ if !errors.Is(err, wantErr) {
212+ t.Fatalf("expected error %v, got %v", wantErr, err)
213+ }
214+ if strings.Contains(err.Error(), "token=secret") || strings.Contains(err.Error(), "https://example.com") {
215+ t.Fatalf("expected error to redact URL, got %q", err)
216+ }
217+}
218+
177219 func requireNoError(t *testing.T, err error) {
178220 t.Helper()
179221 if err != nil {
@@ -1,7 +1,9 @@
1 package desktop1 package desktop
2 2
3 import (3 import (
4+ "errors"
4 "os"5 "os"
6+ "os/exec"
5 "path/filepath"7 "path/filepath"
6 "strings"8 "strings"
7 "testing"9 "testing"
@@ -174,6 +176,46 @@ func TestSessionSpawner_SpawnWithSession_OmitsEngineOverrideWhenNotConfigured(t
174 }176 }
175 }177 }
176 178
179+func TestLaunchBrowserBrowseURL_ResolveErrorDoesNotLeakURL(t *testing.T) {
180+ wantErr := errors.New("resolve failed")
181+
182+ err := launchBrowserBrowseURL(
183+ "https://example.com/private?token=secret",
184+ func() (string, error) { return "", wantErr },
185+ nil,
186+ )
187+
188+ if err == nil {
189+ t.Fatal("expected resolve error")
190+ }
191+ if !errors.Is(err, wantErr) {
192+ t.Fatalf("expected error %v, got %v", wantErr, err)
193+ }
194+ if strings.Contains(err.Error(), "token=secret") || strings.Contains(err.Error(), "https://example.com") {
195+ t.Fatalf("expected error to redact URL, got %q", err)
196+ }
197+}
198+
199+func TestLaunchBrowserBrowseURL_SpawnErrorDoesNotLeakURL(t *testing.T) {
200+ wantErr := errors.New("spawn failed")
201+
202+ err := launchBrowserBrowseURL(
203+ "https://example.com/private?token=secret",
204+ func() (string, error) { return "/usr/bin/dumber", nil },
205+ func(*exec.Cmd) error { return wantErr },
206+ )
207+
208+ if err == nil {
209+ t.Fatal("expected spawn error")
210+ }
211+ if !errors.Is(err, wantErr) {
212+ t.Fatalf("expected error %v, got %v", wantErr, err)
213+ }
214+ if strings.Contains(err.Error(), "token=secret") || strings.Contains(err.Error(), "https://example.com") {
215+ t.Fatalf("expected error to redact URL, got %q", err)
216+ }
217+}
218+
177 func requireNoError(t *testing.T, err error) {219 func requireNoError(t *testing.T, err error) {
178 t.Helper()220 t.Helper()
179 if err != nil {221 if err != nil {
M internal/infrastructure/desktop/browser_launcher.go +3 -3
@@ -32,7 +32,7 @@ func NewBrowserLauncher(relay port.BrowserLaunchRelay) *BrowserLauncher {
3232 // LaunchURL forwards the URL to a running instance when possible, otherwise spawns a new one.
3333 func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error {
3434 if l == nil {
35- return nil
35+ return errors.New("browser launcher is unavailable")
3636 }
3737
3838 if l.relay != nil {
@@ -41,7 +41,7 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error {
4141 if delivered && errors.Is(err, ErrBrowserLaunchRelayUnconfirmed) {
4242 return ErrBrowserLaunchUnconfirmed
4343 }
44- return fmt.Errorf("forward dumber browser URL %q: %w", url, err)
44+ return fmt.Errorf("forward dumber browser URL: %w", err)
4545 }
4646 if delivered {
4747 return nil
@@ -49,7 +49,7 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error {
4949 }
5050
5151 if err := launchBrowserBrowseURL(url, l.resolveExecutablePath, l.startDetachedProcess); err != nil {
52- return fmt.Errorf("launch dumber browse for URL %q: %w", url, err)
52+ return fmt.Errorf("launch dumber browse: %w", err)
5353 }
5454 return nil
5555 }
@@ -32,7 +32,7 @@ func NewBrowserLauncher(relay port.BrowserLaunchRelay) *BrowserLauncher {
32 // LaunchURL forwards the URL to a running instance when possible, otherwise spawns a new one.32 // LaunchURL forwards the URL to a running instance when possible, otherwise spawns a new one.
33 func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error {33 func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error {
34 if l == nil {34 if l == nil {
35- return nil35+ return errors.New("browser launcher is unavailable")
36 }36 }
37 37
38 if l.relay != nil {38 if l.relay != nil {
@@ -41,7 +41,7 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error {
41 if delivered && errors.Is(err, ErrBrowserLaunchRelayUnconfirmed) {41 if delivered && errors.Is(err, ErrBrowserLaunchRelayUnconfirmed) {
42 return ErrBrowserLaunchUnconfirmed42 return ErrBrowserLaunchUnconfirmed
43 }43 }
44- return fmt.Errorf("forward dumber browser URL %q: %w", url, err)44+ return fmt.Errorf("forward dumber browser URL: %w", err)
45 }45 }
46 if delivered {46 if delivered {
47 return nil47 return nil
@@ -49,7 +49,7 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error {
49 }49 }
50 50
51 if err := launchBrowserBrowseURL(url, l.resolveExecutablePath, l.startDetachedProcess); err != nil {51 if err := launchBrowserBrowseURL(url, l.resolveExecutablePath, l.startDetachedProcess); err != nil {
52- return fmt.Errorf("launch dumber browse for URL %q: %w", url, err)52+ return fmt.Errorf("launch dumber browse: %w", err)
53 }53 }
54 return nil54 return nil
55 }55 }
M internal/infrastructure/desktop/browser_launcher_test.go +11 -1
@@ -93,5 +93,15 @@ func TestBrowserLauncher_LaunchURL_PropagatesRelayError(t *testing.T) {
9393 err := launcher.LaunchURL(context.Background(), "https://example.com")
9494
9595 require.Error(t, err)
96- assert.ErrorIs(t, err, wantErr)
96+ require.ErrorIs(t, err, wantErr)
97+ assert.NotContains(t, err.Error(), "https://example.com")
98+}
99+
100+func TestBrowserLauncher_LaunchURL_NilLauncherReturnsError(t *testing.T) {
101+ var launcher *BrowserLauncher
102+
103+ err := launcher.LaunchURL(context.Background(), "https://example.com")
104+
105+ require.Error(t, err)
106+ assert.Contains(t, err.Error(), "unavailable")
97107 }
@@ -93,5 +93,15 @@ func TestBrowserLauncher_LaunchURL_PropagatesRelayError(t *testing.T) {
93 err := launcher.LaunchURL(context.Background(), "https://example.com")93 err := launcher.LaunchURL(context.Background(), "https://example.com")
94 94
95 require.Error(t, err)95 require.Error(t, err)
96- assert.ErrorIs(t, err, wantErr)96+ require.ErrorIs(t, err, wantErr)
97+ assert.NotContains(t, err.Error(), "https://example.com")
98+}
99+
100+func TestBrowserLauncher_LaunchURL_NilLauncherReturnsError(t *testing.T) {
101+ var launcher *BrowserLauncher
102+
103+ err := launcher.LaunchURL(context.Background(), "https://example.com")
104+
105+ require.Error(t, err)
106+ assert.Contains(t, err.Error(), "unavailable")
97 }107 }

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