Skip to content

fix(config): timeout OAuth subprocesses#1694

Open
kawacukennedy wants to merge 2 commits into
floatpane:masterfrom
kawacukennedy:fix/oauth-context-timeout
Open

fix(config): timeout OAuth subprocesses#1694
kawacukennedy wants to merge 2 commits into
floatpane:masterfrom
kawacukennedy:fix/oauth-context-timeout

Conversation

@kawacukennedy

@kawacukennedy kawacukennedy commented Jul 20, 2026

Copy link
Copy Markdown

What?

Add context timeouts to both OAuth subprocess calls (30s for token retrieval, 5 min for browser flow). Capture stderr into a buffer instead of leaking it to os.Stderr.

Why?

Both exec.Command calls suppressed the noctx linter and could hang indefinitely if the Python helper script stalled. Stderr was piped directly to os.Stderr, which could leak sensitive information (tokens, error traces) to logs.

Both exec.Command calls in config/oauth.go suppressed the noctx linter
and could hang indefinitely if the Python helper script stalls (e.g.
waiting for network, user input, or a broken process).

- GetOAuth2Token: add 30-second timeout via exec.CommandContext; capture
  stderr into a buffer so error messages are included in the returned
  error instead of leaking to the application's stderr.
- RunOAuth2Flow: add 5-minute timeout (user needs time for browser-based
  consent); capture stderr for richer error reporting on failure.

This also fixes the previous behavior where the Python subprocess's
stderr was piped directly to os.Stderr, which could leak sensitive
information (tokens, error traces) to logs or terminal output.
@kawacukennedy
kawacukennedy requested a review from a team as a code owner July 20, 2026 01:04
@floatpanebot floatpanebot added area/oauth OAuth / XOAUTH2 / auth flows area/config Configuration / settings labels Jul 20, 2026
@kawacukennedy kawacukennedy changed the title fix(config): add context timeouts to OAuth subprocess calls fix(config): timeout OAuth subprocesses Jul 20, 2026
@floatpanebot floatpanebot added the size/S Diff: 11–50 lines label Jul 20, 2026
@floatpanebot

Copy link
Copy Markdown
Member

Benchmark report — no significant change

Metrics worse: 0 · better: 0 (threshold: ±3%).

benchstat output
goos: linux
goarch: amd64
pkg: github.com/floatpane/matcha/backend
cpu: AMD EPYC 7763 64-Core Processor                
                           │    old.txt    │               new.txt               │
                           │    sec/op     │    sec/op      vs base              │
ParseSearchQuery_Simple-4    2.632µ ±  31%   2.976µ ± 188%       ~ (p=0.258 n=6)
ParseSearchQuery_Complex-4   6.883µ ± 128%   8.541µ ±  45%       ~ (p=0.485 n=6)
TokenizeSearchQuery-4        4.417µ ±  33%   4.161µ ± 182%       ~ (p=1.000 n=6)
geomean                      4.309µ          4.729µ         +9.75%

                           │  old.txt   │              new.txt               │
                           │    B/op    │    B/op     vs base                │
ParseSearchQuery_Simple-4    26.00 ± 0%   26.00 ± 0%       ~ (p=1.000 n=6) ¹
ParseSearchQuery_Complex-4   762.0 ± 0%   762.0 ± 0%       ~ (p=1.000 n=6) ¹
TokenizeSearchQuery-4        176.0 ± 0%   176.0 ± 0%       ~ (p=1.000 n=6) ¹
geomean                      151.6        151.6       +0.00%
¹ all samples are equal

                           │  old.txt   │              new.txt               │
                           │ allocs/op  │ allocs/op   vs base                │
ParseSearchQuery_Simple-4    2.000 ± 0%   2.000 ± 0%       ~ (p=1.000 n=6) ¹
ParseSearchQuery_Complex-4   23.00 ± 0%   23.00 ± 0%       ~ (p=1.000 n=6) ¹
TokenizeSearchQuery-4        9.000 ± 0%   9.000 ± 0%       ~ (p=1.000 n=6) ¹
geomean                      7.453        7.453       +0.00%
¹ all samples are equal

pkg: github.com/floatpane/matcha/tui
                    │    old.txt    │               new.txt               │
                    │    sec/op     │    sec/op     vs base               │
LogPanelView-4         166.1µ ± 10%   176.7µ ± 13%        ~ (p=0.485 n=6)
SearchOverlayView-4    166.7µ ±  3%   170.8µ ±  7%        ~ (p=0.132 n=6)
InboxConstruction-4   1091.2µ ± 39%   939.6µ ± 20%  -13.90% (p=0.041 n=6)
geomean                311.5µ         304.9µ         -2.12%

                    │    old.txt    │               new.txt               │
                    │     B/op      │     B/op       vs base              │
LogPanelView-4        44.67Ki ±  0%   44.67Ki ± 51%       ~ (p=1.000 n=6)
SearchOverlayView-4   56.14Ki ± 41%   56.14Ki ± 41%       ~ (p=0.844 n=6)
InboxConstruction-4   874.3Ki ±  0%   874.3Ki ±  0%       ~ (p=0.394 n=6)
geomean               129.9Ki         129.9Ki        -0.00%

                    │   old.txt   │              new.txt              │
                    │  allocs/op  │  allocs/op   vs base              │
LogPanelView-4         714.0 ± 0%    714.0 ± 0%       ~ (p=1.000 n=6)
SearchOverlayView-4    926.0 ± 0%    926.0 ± 0%       ~ (p=1.000 n=6)
InboxConstruction-4   3.478k ± 0%   3.478k ± 0%       ~ (p=0.517 n=6)
geomean               1.320k        1.320k       -0.01%

auto-generated by benchmarks.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Configuration / settings area/oauth OAuth / XOAUTH2 / auth flows size/S Diff: 11–50 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants