mirror of
https://github.com/esengine/DeepSeek-Reasonix.git
synced 2026-11-07 10:45:59 -10:00
fix(cli): TUI diagnostics, keyring timeout, Reasonix-home paths / 启动诊断、keyring 超时与 home 路径加固 (#7663)
* fix(cli): harden TUI diagnostics, keyring migration, and home paths Problem: - Interactive TUI freezes left zero-byte cli-tui logs and unrecoverable terminals on Windows ConPTY and stuck D-Bus startups (#7435, #7507). - Global hooks/doctor/capdiag could miss the real Reasonix home on Windows AppData and REASONIX_HOME isolation (#7411, #7331). - Legacy keyring migration blocked startup when Secret Service hung. - #7600's automatic HOME rewrite risked breaking isolation contracts. Root cause: - Diagnostics started after heavy work and never flushed milestones/heartbeats. - Hooks resolved global settings via derived ~/.reasonix rather than the exact Reasonix home used by the rest of the product. - Keyring probes ran unbounded per key with file import ordered after D-Bus. - Subprocess HOME rewriting conflates service HOME with Reasonix isolation. Fix: - Start TUI diagnostics before config/controller work; record milestones and flush immediately; 1s heartbeat with 10s stall → goroutine dump + Program.Kill. - Unify hook/capdiag/doctor on exact ReasonixHomeDir; public paths show <reasonix-home>. - Legacy credential migration: files first, then shared 1s keyring budget with empty-success markers; timeout leaves no marker for retry. - Doctor warns when process HOME ≠ account home without REASONIX_HOME; do not auto-rewrite subprocess HOME (#7600 not adopted). Verification: - go test ./internal/cli ./internal/config ./internal/hook ./internal/capdiag ./internal/doctor -count=1 Co-authored-by: Zhiyao Wen <[email protected]> Co-authored-by: zhaotian1 <[email protected]> * fix(cli): isolate keyring probes and tighten migration markers Problem: - In-process keyring timeouts could leave hung D-Bus goroutines and collapse found/absent/error/timeout into a single false, risking wrong migration-done markers (#7507 audit). - Keyring markers also skipped legacy credential *file* import. - doctor HOME warning leaked absolute account paths into shareable reports. - lint failed on an unused diagnostics mutex; CodeQL flagged SHA-256 of env names as secret-like storage. Root cause: - Timeout wrappers cannot cancel platform keyring calls in-process. - skip() treated keyring markers as global migration completion. - redactHome only knows the process HOME, not the OS account home. Fix: - Run legacy keyring batch probes in an isolated helper child with a shared 1s deadline; parent Kill+Wait on timeout (no parent goroutine leak). - Four-state outcomes: found / absent / error / timeout; write markers only on confirmed absent. - File import always runs first; markers only filter keyring probes. - Marker paths use sanitized env names under legacy-keyring-checked/. - doctor reports HOME mismatch without embedding either path. - Watchdog WaitGroup drains before closing the diagnostic log file. Verification: - go test ./internal/config ./internal/doctor ./internal/cli ./internal/hook ./internal/capdiag -count=1 - go vet ./internal/cli ./internal/config ./internal/doctor * fix(cli): keyring four-state to metal and secret-free helper Problem: - Platform keyring probes still returned (string, bool), so D-Bus/unlock failures were treated as absent and could write migration markers. - The isolated helper printed keyring secrets on stdout, making any process that can launch reasonix a credential oracle. - Watchdog Close waited only 2s, so a stuck Stack/Sync/Kill could write a closed diagnostic file. - Marker names folded distinct keys onto "_" collisions. Root cause: - Four-state status stopped at the batch layer; the Secret Service path collapsed every error to false. - Helper was designed as a value RPC instead of an in-child store step. - Close used a timed wait instead of joining the watchdog fully. Fix: - legacyKeyringProbe returns found/absent/error at the platform layer (ErrNotFound/empty search → absent; transport/unlock/session errors → error). - Helper stores found secrets via StoreCredentialLines and emits only key+status metadata on stdout (no value field). - Parent never receives secret Values from the helper path; markers only on confirmed absent. - Watchdog WaitGroup is joined fully before closing the log file. - Marker basenames use base64.RawURLEncoding of the env name. Verification: - go test ./internal/config ./internal/cli ./internal/doctor -count=1 - New tests: error does not write marker; helper JSON has no secrets; RawURL marker collision-free; four-state in-process outcomes * fix(cli): drop keyring helper oracle; atomic store-if-absent Problem: - The REASONIX_LEGACY_KEYRING_HELPER entrypoint let any process that can launch reasonix dump keyring secrets into a caller-controlled REASONIX_HOME/.env (disk oracle after stdout was scrubbed). - Legacy import could overwrite a concurrent settings write or tombstone because presence was checked outside the credential lock. Root cause: - Migration used an external helper to bound D-Bus hangs, which reopened a secret export surface via StoreCredentialLines + controllable home. - store used unconditional StoreCredentialLines after a TOCTOU check. Fix: - Remove the helper entrypoint entirely (main + env dispatch). - Linux/BSD Secret Service probes use godbus CallWithContext under a shared 1s batch deadline so stuck D-Bus calls cancel in-process. - Found secrets are written only via storeCredentialIfAbsentAndNotCleared (same advisory lock re-checks missing/not-cleared before write). - Channel-forced tests cover user-value and tombstone races without sleep. Verification: - go test ./internal/config ./internal/cli ./internal/doctor -count=1 - REASONIX_LEGACY_KEYRING_HELPER=1 reasonix --version is a normal version print (no helper path, no oracle home writes) * fix(cli): private ctx-bound D-Bus keyring and real probe races Problem: - dbus.SessionBus left process-global godbus workers that failed goleak under Ubuntu/race/coverage. - GetProperty and session Close used non-context call paths, so a stuck bus could still hang CLI past the 1s migration budget. - Race tests called store-if-absent directly and never forced the production probe→store window. Root cause: - Shared SessionBus + GetProperty(Background) escaped the batch ctx. - Channel tests interleaving happened outside lookupLegacyKeyringBatch. Fix: - Open a private ConnectSessionBus per probe, race Dial/Auth/Hello against the shared ctx, and always Close the connection. - Resolve Collections and Close sessions only via CallWithContext(ctx). - Drive store-if-absent races through a blocking probe stub inside lookupLegacyKeyringBatch (user write and tombstone cases). - go mod tidy promotes godbus to a direct dependency. Verification: - go test ./internal/config -count=1 - go test -race ./internal/config -count=1 - go test ./internal/control -count=1 * fix(cli): bind keyring bus connect to batch context Problem: - ConnectSessionBus had no dbus.WithContext, so a stuck Auth/Hello after Dial could keep the connect goroutine and late reaper parked forever even though the caller returned at 1s. Root cause: - Connection lifecycle was only raced externally; godbus never received the migration ctx to Close the transport on deadline. Fix: - SessionBusPrivateNoAutoStartup(dbus.WithContext(ctx)) then Auth+Hello, so cancel closes the transport and unblocks Auth/Hello I/O. - Avoid dbus-launch (NoAutoStartup): missing session address fails closed instead of hanging in CombinedOutput. - Outer select still bounds the caller; late *Conn is always closed. - Document residual: pure Dial hang before newConn remains a godbus limit. Verification: - go test ./internal/config -count=1 - go test -race ./internal/config -count=1 - go test ./internal/control -count=1 - GOOS=linux go test ./internal/config -c * ci: allow Windows smoke runner variance Problem: The Windows smoke step regularly takes 7-9 minutes and the current run exceeded its 10-minute job budget even though vet and build succeeded. Root cause: The job-level timeout leaves too little headroom for hosted Windows runner variance. Fix: Raise only the pull-request Windows smoke job budget to 15 minutes while retaining the per-package 3-minute Go test timeout. Verification: YAML parse; git diff --check; go test ./internal/cli ./internal/config -count=1. --------- Co-authored-by: Zhiyao Wen <[email protected]> Co-authored-by: zhaotian1 <[email protected]>
This commit is contained in:
24 files changed
+1109
-100
No files matched your search
@@ -136,10 +136,12 @@ jobs:
|
||||
# Pull requests run a Windows smoke suite: the packages that actually
|
||||
# carry *_windows.go code, the startup/checkpoint packages with portable
|
||||
# filesystem contracts, plus cmd/. The full ./... sweep stays on pushes
|
||||
# to main-v2; the Unix legs always run the full suite.
|
||||
# to main-v2; the Unix legs always run the full suite. Keep the job-level
|
||||
# budget above normal 7-9 minute runner variance; each package test binary
|
||||
# remains independently bounded by Go's 3 minute timeout below.
|
||||
- name: test (Windows smoke)
|
||||
if: env.RUN_STEPS == 'true' && runner.os == 'Windows' && github.event_name == 'pull_request'
|
||||
timeout-minutes: 10
|
||||
timeout-minutes: 15
|
||||
env:
|
||||
REASONIX_RELEASE_CACHE_GUARD: "1"
|
||||
WINDOWS_SANDBOX_WAIT_MS: "20000"
|
||||
|
||||
@@ -16,6 +16,7 @@ require (
|
||||
github.com/bmatcuk/doublestar/v4 v4.10.0
|
||||
github.com/charmbracelet/colorprofile v0.4.3
|
||||
github.com/charmbracelet/x/ansi v0.11.7
|
||||
github.com/godbus/dbus/v5 v5.2.2
|
||||
github.com/joho/godotenv v1.5.1
|
||||
github.com/kevinburke/ssh_config v1.6.0
|
||||
github.com/larksuite/oapi-sdk-go/v3 v3.9.5
|
||||
@@ -54,7 +55,6 @@ require (
|
||||
github.com/danieljoos/wincred v1.2.3 // indirect
|
||||
github.com/dlclark/regexp2/v2 v2.2.1 // indirect
|
||||
github.com/go-ole/go-ole v1.3.0 // indirect
|
||||
github.com/godbus/dbus/v5 v5.2.2 // indirect
|
||||
github.com/gogo/protobuf v1.3.2 // indirect
|
||||
github.com/gorilla/websocket v1.5.0 // indirect
|
||||
github.com/kr/fs v0.1.0 // indirect
|
||||
|
||||
+21
-25
@@ -57,19 +57,19 @@ func Collect(opts Options) Report {
|
||||
if cfgErr != nil {
|
||||
issues = append(issues, Issue{
|
||||
Severity: "error", Code: "config.load_failed", Subsystem: "config",
|
||||
Message: "failed to load configuration: " + sanitizeErrTextWithPaths(cfgErr.Error(), root, home),
|
||||
Message: "failed to load configuration: " + sanitizeErrTextWithPaths(cfgErr.Error(), root, home, reasonixHome),
|
||||
Remediation: "Fix reasonix.toml / config.toml syntax, then re-run doctor capabilities",
|
||||
})
|
||||
}
|
||||
|
||||
disp := func(p string) string { return displayPath(p, root, home) }
|
||||
disp := func(p string) string { return displayPath(p, root, home, reasonixHome) }
|
||||
|
||||
instr, instructionIssues := collectInstructions(root, home, disp)
|
||||
skillsR, skillIssues := collectSkills(root, home, reasonixHome, cfg, disp)
|
||||
cmdsR, cmdIssues := collectCommands(root, disp)
|
||||
hooksR, hookIssues := collectHooks(root, home, reasonixHome, cfg, disp)
|
||||
pluginsR, pluginIssues := collectPlugins(reasonixHome, disp)
|
||||
mcpR, mcpIssues := collectMCP(cfg, root, home, disp)
|
||||
mcpR, mcpIssues := collectMCP(cfg, root, home, reasonixHome, disp)
|
||||
|
||||
issues = append(issues, instructionIssues...)
|
||||
issues = append(issues, skillIssues...)
|
||||
@@ -80,10 +80,10 @@ func Collect(opts Options) Report {
|
||||
|
||||
// Runtime host merge (desktop) or live probe (CLI).
|
||||
if opts.Live {
|
||||
liveIssues := probeLiveMCP(&mcpR, cfg, root, home, opts.LiveTimeout)
|
||||
liveIssues := probeLiveMCP(&mcpR, cfg, root, home, reasonixHome, opts.LiveTimeout)
|
||||
issues = append(issues, liveIssues...)
|
||||
} else if opts.RuntimeHost != nil {
|
||||
mergeRuntimeHost(&mcpR, opts.RuntimeHost, root, home, &issues)
|
||||
mergeRuntimeHost(&mcpR, opts.RuntimeHost, root, home, reasonixHome, &issues)
|
||||
}
|
||||
|
||||
sortIssues(issues)
|
||||
@@ -287,14 +287,10 @@ func collectCommands(root string, disp func(string) string) (AssetReport, []Issu
|
||||
|
||||
func collectHooks(root, home, reasonixHome string, cfg *config.Config, disp func(string) string) (HookReport, []Issue) {
|
||||
var issues []Issue
|
||||
// Prefer explicit home for settings when tests isolate HOME.
|
||||
homeDir := home
|
||||
if reasonixHome != "" && home == "" {
|
||||
homeDir = filepath.Dir(reasonixHome)
|
||||
}
|
||||
insp := hook.Inspect(hook.LoadOptions{
|
||||
ProjectRoot: root,
|
||||
HomeDir: homeDir,
|
||||
ProjectRoot: root,
|
||||
HomeDir: home,
|
||||
ReasonixHomeDir: reasonixHome,
|
||||
})
|
||||
runtimeOptions := hook.RuntimeOptions{}
|
||||
if cfg != nil {
|
||||
@@ -325,7 +321,7 @@ func collectHooks(root, home, reasonixHome string, cfg *config.Config, disp func
|
||||
}
|
||||
for _, e := range insp.Entries {
|
||||
rep.Entries = append(rep.Entries, HookEntry{
|
||||
Event: string(e.Event), Match: e.Match, Command: redactCommandDisplay(e.Command, root, home),
|
||||
Event: string(e.Event), Match: e.Match, Command: redactCommandDisplay(e.Command, root, home, reasonixHome),
|
||||
ContextFile: disp(e.ContextFile), Description: e.Description, TimeoutMS: e.Timeout,
|
||||
Scope: string(e.Scope), Source: disp(e.Source), Blocking: hook.IsBlocking(e.Event),
|
||||
})
|
||||
@@ -465,7 +461,7 @@ func collectPlugins(reasonixHome string, disp func(string) string) (PluginPackag
|
||||
return rep, issues
|
||||
}
|
||||
|
||||
func collectMCP(cfg *config.Config, root, home string, disp func(string) string) (MCPReport, []Issue) {
|
||||
func collectMCP(cfg *config.Config, root, home, reasonixHome string, disp func(string) string) (MCPReport, []Issue) {
|
||||
var issues []Issue
|
||||
rep := MCPReport{Servers: []MCPServerInfo{}}
|
||||
if cfg == nil {
|
||||
@@ -501,7 +497,7 @@ func collectMCP(cfg *config.Config, root, home string, disp func(string) string)
|
||||
}
|
||||
}
|
||||
if info.Transport == "stdio" {
|
||||
info.Command = redactCommandDisplay(p.Command, root, home)
|
||||
info.Command = redactCommandDisplay(p.Command, root, home, reasonixHome)
|
||||
} else {
|
||||
info.URLHost = urlHostOnly(p.URL)
|
||||
}
|
||||
@@ -581,7 +577,7 @@ func commandExists(cmd string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
func mergeRuntimeHost(rep *MCPReport, host *plugin.Host, root, home string, issues *[]Issue) {
|
||||
func mergeRuntimeHost(rep *MCPReport, host *plugin.Host, root, home, reasonixHome string, issues *[]Issue) {
|
||||
if host == nil {
|
||||
return
|
||||
}
|
||||
@@ -615,13 +611,13 @@ func mergeRuntimeHost(rep *MCPReport, host *plugin.Host, root, home string, issu
|
||||
}
|
||||
}
|
||||
for _, f := range host.Failures() {
|
||||
errText := sanitizeErrTextWithPaths(f.Error, root, home)
|
||||
errText := sanitizeErrTextWithPaths(f.Error, root, home, reasonixHome)
|
||||
if i, ok := byName[f.Name]; ok {
|
||||
rep.Servers[i].RuntimeStatus = "failed"
|
||||
rep.Servers[i].Error = errText
|
||||
rep.Servers[i].StartupStage = f.Stage
|
||||
rep.Servers[i].StartupElapsedMS = f.Elapsed.Milliseconds()
|
||||
rep.Servers[i].Stderr = sanitizeErrTextWithPaths(f.Stderr, root, home)
|
||||
rep.Servers[i].Stderr = sanitizeErrTextWithPaths(f.Stderr, root, home, reasonixHome)
|
||||
}
|
||||
*issues = append(*issues, Issue{
|
||||
Severity: "error", Code: "mcp.start_failed", Subsystem: "mcp",
|
||||
@@ -669,10 +665,10 @@ func sanitizeErr(err error) string {
|
||||
// sanitizeErrText redacts secrets and machine-local identity from diagnostic
|
||||
// strings. Prefer sanitizeErrTextWithPaths when workspace/home are known.
|
||||
func sanitizeErrText(s string) string {
|
||||
return sanitizeErrTextWithPaths(s, "", "")
|
||||
return sanitizeErrTextWithPaths(s, "", "", "")
|
||||
}
|
||||
|
||||
func sanitizeErrTextWithPaths(s, workspace, home string) string {
|
||||
func sanitizeErrTextWithPaths(s, workspace, home, reasonixHome string) string {
|
||||
s = strings.TrimSpace(s)
|
||||
if s == "" {
|
||||
return s
|
||||
@@ -705,7 +701,7 @@ func sanitizeErrTextWithPaths(s, workspace, home string) string {
|
||||
s = redactBearer(s)
|
||||
|
||||
// Absolute paths: rewrite with displayPath when possible.
|
||||
s = redactAbsolutePaths(s, workspace, home)
|
||||
s = redactAbsolutePaths(s, workspace, home, reasonixHome)
|
||||
|
||||
// Cap length after redaction.
|
||||
const max = 400
|
||||
@@ -757,7 +753,7 @@ func redactBearer(s string) string {
|
||||
}
|
||||
}
|
||||
|
||||
func redactAbsolutePaths(s, workspace, home string) string {
|
||||
func redactAbsolutePaths(s, workspace, home, reasonixHome string) string {
|
||||
// Walk for POSIX and Windows absolute path-like tokens.
|
||||
var b strings.Builder
|
||||
i := 0
|
||||
@@ -785,7 +781,7 @@ func redactAbsolutePaths(s, workspace, home string) string {
|
||||
token := s[start:j]
|
||||
// Only rewrite if it looks like a path with a directory separator beyond root.
|
||||
if strings.ContainsAny(token, `/\`) && len(token) > 1 {
|
||||
b.WriteString(displayPath(token, workspace, home))
|
||||
b.WriteString(displayPath(token, workspace, home, reasonixHome))
|
||||
} else {
|
||||
b.WriteString(token)
|
||||
}
|
||||
@@ -794,7 +790,7 @@ func redactAbsolutePaths(s, workspace, home string) string {
|
||||
return b.String()
|
||||
}
|
||||
|
||||
func redactCommandDisplay(cmd, root, home string) string {
|
||||
func redactCommandDisplay(cmd, root, home, reasonixHome string) string {
|
||||
cmd = strings.TrimSpace(cmd)
|
||||
if cmd == "" {
|
||||
return ""
|
||||
@@ -804,7 +800,7 @@ func redactCommandDisplay(cmd, root, home string) string {
|
||||
if len(fields) == 0 {
|
||||
return ""
|
||||
}
|
||||
return displayPath(fields[0], root, home)
|
||||
return displayPath(fields[0], root, home, reasonixHome)
|
||||
}
|
||||
|
||||
// ioDiscard avoids importing io in every call site for skill.Options.Stderr.
|
||||
|
||||
@@ -122,6 +122,41 @@ auto_start = false
|
||||
}
|
||||
}
|
||||
|
||||
func TestCollectUsesExactReasonixHomeForGlobalHooks(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
home := t.TempDir()
|
||||
reasonixHome := filepath.Join(home, "AppData", "Roaming", "reasonix")
|
||||
write(t, filepath.Join(reasonixHome, "settings.json"), `{
|
||||
"hooks": {
|
||||
"SessionStart": [{"command": "echo exact-home"}]
|
||||
}
|
||||
}`)
|
||||
|
||||
report := capdiag.Collect(capdiag.Options{
|
||||
Root: root,
|
||||
HomeDir: home,
|
||||
ReasonixHomeDir: reasonixHome,
|
||||
})
|
||||
if report.Summary.Hooks != 1 || len(report.Hooks.Entries) != 1 {
|
||||
t.Fatalf("hooks = %+v, summary = %+v", report.Hooks, report.Summary)
|
||||
}
|
||||
for _, source := range report.Hooks.Sources {
|
||||
if source.Scope == "global" {
|
||||
if source.Status != "ok" || source.HookCount != 1 {
|
||||
t.Fatalf("global hook source = %+v, want exact Reasonix home settings", source)
|
||||
}
|
||||
if source.Path != "<reasonix-home>/settings.json" && source.Path != "<reasonix-home>\\settings.json" {
|
||||
// displayPath uses ToSlash
|
||||
if !strings.Contains(source.Path, "<reasonix-home>") {
|
||||
t.Fatalf("global path = %q, want <reasonix-home> prefix", source.Path)
|
||||
}
|
||||
}
|
||||
return
|
||||
}
|
||||
}
|
||||
t.Fatal("global hook source not reported")
|
||||
}
|
||||
|
||||
func TestMissingConventionDirsNoWarning(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
home := t.TempDir()
|
||||
|
||||
@@ -20,7 +20,7 @@ func lookPath(cmd string) (string, error) {
|
||||
// connection results, and always closes the Host (including stdio children).
|
||||
// Persistence (startup stats / schema cache) is disabled so --live stays
|
||||
// free of cache/state side effects under Reasonix home.
|
||||
func probeLiveMCP(rep *MCPReport, cfg *config.Config, root, home string, timeout time.Duration) []Issue {
|
||||
func probeLiveMCP(rep *MCPReport, cfg *config.Config, root, home, reasonixHome string, timeout time.Duration) []Issue {
|
||||
var issues []Issue
|
||||
if cfg == nil {
|
||||
return issues
|
||||
@@ -99,13 +99,13 @@ func probeLiveMCP(rep *MCPReport, cfg *config.Config, root, home string, timeout
|
||||
}
|
||||
}
|
||||
for _, f := range host.Failures() {
|
||||
errText := sanitizeErrTextWithPaths(f.Error, root, home)
|
||||
errText := sanitizeErrTextWithPaths(f.Error, root, home, reasonixHome)
|
||||
if i, ok := byName[f.Name]; ok {
|
||||
rep.Servers[i].RuntimeStatus = "failed"
|
||||
rep.Servers[i].Error = errText
|
||||
rep.Servers[i].StartupStage = f.Stage
|
||||
rep.Servers[i].StartupElapsedMS = f.Elapsed.Milliseconds()
|
||||
rep.Servers[i].Stderr = sanitizeErrTextWithPaths(f.Stderr, root, home)
|
||||
rep.Servers[i].Stderr = sanitizeErrTextWithPaths(f.Stderr, root, home, reasonixHome)
|
||||
}
|
||||
issues = append(issues, Issue{
|
||||
Severity: "error", Code: "mcp.start_failed", Subsystem: "mcp",
|
||||
|
||||
@@ -9,9 +9,10 @@ import (
|
||||
|
||||
// displayPath rewrites absolute paths for safe reports:
|
||||
// - under workspace → <workspace>/...
|
||||
// - under home → ~/...
|
||||
// - under Reasonix home → <reasonix-home>/...
|
||||
// - under OS home → ~/...
|
||||
// - elsewhere → <external>/basename (no full external path, no username)
|
||||
func displayPath(p, workspace, home string) string {
|
||||
func displayPath(p, workspace, home, reasonixHome string) string {
|
||||
p = strings.TrimSpace(p)
|
||||
if p == "" {
|
||||
return ""
|
||||
@@ -36,6 +37,18 @@ func displayPath(p, workspace, home string) string {
|
||||
return "<workspace>/" + filepath.ToSlash(rel)
|
||||
}
|
||||
}
|
||||
if rh := strings.TrimSpace(reasonixHome); rh != "" {
|
||||
if abs, err := filepath.Abs(rh); err == nil {
|
||||
rh = abs
|
||||
}
|
||||
rh = filepath.Clean(rh)
|
||||
if clean == rh {
|
||||
return "<reasonix-home>"
|
||||
}
|
||||
if rel, err := filepath.Rel(rh, clean); err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
|
||||
return "<reasonix-home>/" + filepath.ToSlash(rel)
|
||||
}
|
||||
}
|
||||
if home == "" {
|
||||
if h, err := os.UserHomeDir(); err == nil {
|
||||
home = h
|
||||
|
||||
@@ -10,7 +10,7 @@ func TestSanitizeErrTextRedactsSecretsAndPaths(t *testing.T) {
|
||||
ws := t.TempDir()
|
||||
in := "stdio plugin \"x\": command \"npx\" not found on PATH; PATH=\"" + home + "/bin:/usr/bin\" Bearer sk-secret-token " +
|
||||
ws + "/secret.env stderr: Authorization=Bearer abc.def"
|
||||
out := sanitizeErrTextWithPaths(in, ws, home)
|
||||
out := sanitizeErrTextWithPaths(in, ws, home, "")
|
||||
if strings.Contains(out, home) {
|
||||
t.Fatalf("home path leaked: %q", out)
|
||||
}
|
||||
|
||||
@@ -48,6 +48,10 @@ type chatTUI struct {
|
||||
ctrl control.SessionAPI
|
||||
label string
|
||||
missing string // missing-key warning surfaced once in the banner, "" when ready
|
||||
// diagnostics is the process-owned TUI log/watchdog started before terminal
|
||||
// takeover. Nil in unit tests that construct chatTUI without chatREPL.
|
||||
diagnostics *tuiDiagnostics
|
||||
firstFrameLogged bool
|
||||
|
||||
width int
|
||||
height int
|
||||
@@ -845,6 +849,17 @@ func suspendWithMouseReset() tea.Cmd {
|
||||
}
|
||||
|
||||
func (m chatTUI) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
|
||||
// Feed the startup/stall watchdog so freezes leave a goroutine dump and
|
||||
// recover the terminal instead of a zero-byte log (#7435).
|
||||
if m.diagnostics != nil {
|
||||
m.diagnostics.markProgress()
|
||||
}
|
||||
logFirstFrame := false
|
||||
if m.diagnostics != nil && !m.firstFrameLogged {
|
||||
if _, ok := msg.(tea.WindowSizeMsg); ok {
|
||||
logFirstFrame = true
|
||||
}
|
||||
}
|
||||
wasAtBottom := m.viewport.AtBottom()
|
||||
prevLines := len(m.transcript)
|
||||
prevWidth := m.width
|
||||
@@ -856,6 +871,12 @@ func (m chatTUI) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
|
||||
|
||||
next, cmd := m.update(msg)
|
||||
cm := next.(chatTUI)
|
||||
if logFirstFrame {
|
||||
cm.firstFrameLogged = true
|
||||
if cm.diagnostics != nil {
|
||||
cm.diagnostics.Milestone("first_frame")
|
||||
}
|
||||
}
|
||||
|
||||
contentW := transcriptContentWidth(cm.width, cm.nativeScrollback)
|
||||
cm.viewport.SetWidth(contentW)
|
||||
|
||||
+14
-6
@@ -1049,16 +1049,18 @@ func chatREPL(args []string, version string) int {
|
||||
fmt.Fprintln(os.Stderr, i18n.M.ErrorPrefix, err)
|
||||
return 1
|
||||
}
|
||||
// Bubble Tea owns the terminal from the resume picker through controller
|
||||
// shutdown. Start diagnostics before config/controller work so hangs leave a
|
||||
// non-zero log with milestones (#7435, #7507).
|
||||
diagnostics := startTUIDiagnostics(config.ReasonixHomeDir())
|
||||
defer diagnostics.Close()
|
||||
diagnostics.Milestone("config_load_begin")
|
||||
cfg, err := config.Load()
|
||||
if err == nil {
|
||||
configureCLIThemeWithStyle(cfg.UITheme(), cfg.UIThemeStyle())
|
||||
cliCursorShape = cfg.UICursorShape()
|
||||
}
|
||||
// Bubble Tea owns the terminal from the resume picker through controller
|
||||
// shutdown. Route process logs and plugin stderr to a private, bounded file
|
||||
// for that whole lifetime; user-facing warnings arrive as typed TUI events.
|
||||
diagnostics := startTUIDiagnostics(config.ReasonixHomeDir())
|
||||
defer diagnostics.Close()
|
||||
diagnostics.Milestone("config_load_done")
|
||||
|
||||
// Decide whether we're starting fresh or resuming. --resume opens an
|
||||
// interactive picker; --continue / -c jumps straight into the newest.
|
||||
@@ -1154,6 +1156,7 @@ func chatREPL(args []string, version string) int {
|
||||
Stderr: diagnostics.Writer(),
|
||||
OnSessionRecovered: cliSessionRecoveredHandler(leases),
|
||||
}
|
||||
diagnostics.Milestone("controller_build_begin")
|
||||
ctrl, err := setupProfileWithOverrides(ctx, *model, *maxSteps, false, sink, profile, overrides)
|
||||
if err != nil && errors.Is(err, boot.ErrUnknownModel) && isInteractive() && config.SourcePath() == "" {
|
||||
// True first run whose default model can't resolve: guide setup, then retry.
|
||||
@@ -1169,6 +1172,7 @@ func chatREPL(args []string, version string) int {
|
||||
fmt.Fprintln(os.Stderr, i18n.M.ErrorPrefix, err)
|
||||
return 1
|
||||
}
|
||||
diagnostics.Milestone("controller_build_done")
|
||||
|
||||
// Decide where this conversation's auto-save lands. A resume reuses the
|
||||
// file so closing/reopening keeps appending to the same history; a fresh
|
||||
@@ -1231,9 +1235,10 @@ func chatREPL(args []string, version string) int {
|
||||
}
|
||||
|
||||
m := newChatTUI(ctrl, missing, eventCh, termW)
|
||||
m.diagnostics = diagnostics
|
||||
m.planMode = permissions.plan
|
||||
m.leases = leases
|
||||
if cfg, err := config.Load(); err == nil {
|
||||
if cfg != nil {
|
||||
m.outputStyle = cfg.Agent.OutputStyle // shown as the active entry in /output-style
|
||||
m.statuslineCmd = cfg.Statusline.Command // custom status-line command, "" = built-in row
|
||||
m.showReasoning = cfg.UI.ShowReasoning // /verbose persistence: start with config default
|
||||
@@ -1310,7 +1315,9 @@ func chatREPL(args []string, version string) int {
|
||||
// Non-Termux terminals use an alt-screen transcript viewport. Termux stays
|
||||
// in the normal buffer so native touch scrollback and soft-keyboard focus
|
||||
// keep working; finalized transcript lines are emitted via tea.Println.
|
||||
diagnostics.Milestone("terminal_takeover_begin")
|
||||
p := tea.NewProgram(m)
|
||||
diagnostics.StartWatchdog(p)
|
||||
// SSH drop (SIGHUP) or service stop (SIGTERM): persist the conversation
|
||||
// before the terminal goes away, then unwind through the normal close path
|
||||
// so resume picks up the interrupted session (#3772).
|
||||
@@ -1323,6 +1330,7 @@ func chatREPL(args []string, version string) int {
|
||||
}()
|
||||
final, runErr := p.Run()
|
||||
signal.Stop(hangup)
|
||||
diagnostics.Milestone("terminal_released")
|
||||
// Close the active controller plus any retired ones from /model switches.
|
||||
// Retired controllers were stashed rather than closed at switch time
|
||||
// because Controller.Close() runs SessionEnd hooks and kills plugin
|
||||
|
||||
@@ -1,18 +1,25 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
tea "charm.land/bubbletea/v2"
|
||||
)
|
||||
|
||||
const (
|
||||
tuiDiagnosticLogLimit = 4 << 20
|
||||
tuiDiagnosticLogRetention = 7 * 24 * time.Hour
|
||||
tuiWatchdogInterval = time.Second
|
||||
tuiWatchdogStall = 10 * time.Second
|
||||
)
|
||||
|
||||
// tuiDiagnostics owns process-level diagnostics while an interactive terminal
|
||||
@@ -20,6 +27,9 @@ const (
|
||||
// plugin stderr must go to a private file instead of bypassing its renderer.
|
||||
// Failure to create that file degrades to io.Discard: typed event.Notice values
|
||||
// still carry user-facing warnings through the TUI.
|
||||
//
|
||||
// Milestones and a 1s heartbeat keep the log non-empty during hangs so Windows
|
||||
// ConPTY freezes (#7435) and stuck D-Bus startups leave recoverable evidence.
|
||||
type tuiDiagnostics struct {
|
||||
previous *slog.Logger
|
||||
logger *slog.Logger
|
||||
@@ -27,10 +37,16 @@ type tuiDiagnostics struct {
|
||||
file *os.File
|
||||
path string
|
||||
close sync.Once
|
||||
|
||||
lastProgress atomic.Int64 // unix nano of last Update/View progress
|
||||
stopWatch chan struct{}
|
||||
watchOnce sync.Once
|
||||
watchWG sync.WaitGroup
|
||||
killed atomic.Bool
|
||||
}
|
||||
|
||||
func startTUIDiagnostics(reasonixHome string) *tuiDiagnostics {
|
||||
d := &tuiDiagnostics{previous: slog.Default(), writer: io.Discard}
|
||||
d := &tuiDiagnostics{previous: slog.Default(), writer: io.Discard, stopWatch: make(chan struct{})}
|
||||
if logDir := tuiDiagnosticLogDir(reasonixHome); logDir != "" {
|
||||
if err := os.MkdirAll(logDir, 0o700); err == nil {
|
||||
pruneTUIDiagnosticLogs(logDir, time.Now())
|
||||
@@ -43,9 +59,47 @@ func startTUIDiagnostics(reasonixHome string) *tuiDiagnostics {
|
||||
}
|
||||
d.logger = slog.New(slog.NewTextHandler(d.writer, &slog.HandlerOptions{Level: slog.LevelInfo}))
|
||||
slog.SetDefault(d.logger)
|
||||
d.markProgress()
|
||||
d.Milestone("diagnostics_started")
|
||||
return d
|
||||
}
|
||||
|
||||
// Milestone records a startup/runtime phase and flushes the log immediately so a
|
||||
// subsequent hang still leaves a non-zero diagnostic file.
|
||||
func (d *tuiDiagnostics) Milestone(name string) {
|
||||
if d == nil {
|
||||
return
|
||||
}
|
||||
d.markProgress()
|
||||
msg := fmt.Sprintf("milestone=%s t=%s", strings.TrimSpace(name), time.Now().UTC().Format(time.RFC3339Nano))
|
||||
_, _ = fmt.Fprintln(d.Writer(), msg)
|
||||
d.Sync()
|
||||
}
|
||||
|
||||
// Sync flushes the diagnostic file to disk when possible.
|
||||
func (d *tuiDiagnostics) Sync() {
|
||||
if d == nil || d.file == nil {
|
||||
return
|
||||
}
|
||||
_ = d.file.Sync()
|
||||
}
|
||||
|
||||
// markProgress records that the TUI event loop is alive.
|
||||
func (d *tuiDiagnostics) markProgress() {
|
||||
if d == nil {
|
||||
return
|
||||
}
|
||||
d.lastProgress.Store(time.Now().UnixNano())
|
||||
}
|
||||
|
||||
// Path returns the diagnostic log path (empty when falling back to Discard).
|
||||
func (d *tuiDiagnostics) Path() string {
|
||||
if d == nil {
|
||||
return ""
|
||||
}
|
||||
return d.path
|
||||
}
|
||||
|
||||
func (d *tuiDiagnostics) Writer() io.Writer {
|
||||
if d == nil || d.writer == nil {
|
||||
return io.Discard
|
||||
@@ -53,17 +107,86 @@ func (d *tuiDiagnostics) Writer() io.Writer {
|
||||
return d.writer
|
||||
}
|
||||
|
||||
// StartWatchdog arms a 1s heartbeat. If the TUI event loop makes no progress for
|
||||
// 10s, it dumps all goroutines, syncs the log, and kills the Bubble Tea program
|
||||
// so the terminal is restored instead of remaining frozen (#7435).
|
||||
func (d *tuiDiagnostics) StartWatchdog(p *tea.Program) {
|
||||
if d == nil || p == nil {
|
||||
return
|
||||
}
|
||||
d.watchOnce.Do(func() {
|
||||
d.markProgress()
|
||||
d.watchWG.Add(1)
|
||||
go func() {
|
||||
defer d.watchWG.Done()
|
||||
d.watch(p)
|
||||
}()
|
||||
})
|
||||
}
|
||||
|
||||
func (d *tuiDiagnostics) watch(p *tea.Program) {
|
||||
ticker := time.NewTicker(tuiWatchdogInterval)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
select {
|
||||
case <-d.stopWatch:
|
||||
return
|
||||
case now := <-ticker.C:
|
||||
last := time.Unix(0, d.lastProgress.Load())
|
||||
age := now.Sub(last)
|
||||
_, _ = fmt.Fprintf(d.Writer(), "heartbeat t=%s last_progress_age=%s\n",
|
||||
now.UTC().Format(time.RFC3339Nano), age.Round(time.Millisecond))
|
||||
d.Sync()
|
||||
if age < tuiWatchdogStall || d.killed.Load() {
|
||||
continue
|
||||
}
|
||||
d.killed.Store(true)
|
||||
d.dumpGoroutines("watchdog_stall")
|
||||
d.Sync()
|
||||
// Kill restores the terminal; Quit alone can hang if Update is blocked.
|
||||
p.Kill()
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (d *tuiDiagnostics) dumpGoroutines(reason string) {
|
||||
if d == nil {
|
||||
return
|
||||
}
|
||||
buf := make([]byte, 1<<20)
|
||||
for {
|
||||
n := runtime.Stack(buf, true)
|
||||
if n < len(buf) {
|
||||
buf = buf[:n]
|
||||
break
|
||||
}
|
||||
buf = make([]byte, len(buf)*2)
|
||||
}
|
||||
_, _ = fmt.Fprintf(d.Writer(), "goroutine_dump reason=%s bytes=%d\n%s\n", reason, len(buf), buf)
|
||||
}
|
||||
|
||||
func (d *tuiDiagnostics) Close() {
|
||||
if d == nil {
|
||||
return
|
||||
}
|
||||
d.close.Do(func() {
|
||||
select {
|
||||
case <-d.stopWatch:
|
||||
default:
|
||||
close(d.stopWatch)
|
||||
}
|
||||
// Wait for the watchdog to fully exit before closing the log. A timed
|
||||
// wait left a window where runtime.Stack / Sync / Kill could still write
|
||||
// the file after Close returned.
|
||||
d.watchWG.Wait()
|
||||
// Do not overwrite a logger deliberately installed by another owner
|
||||
// after the TUI started.
|
||||
if slog.Default() == d.logger && d.previous != nil {
|
||||
slog.SetDefault(d.previous)
|
||||
}
|
||||
if d.file != nil {
|
||||
_ = d.file.Sync()
|
||||
_ = d.file.Close()
|
||||
}
|
||||
})
|
||||
|
||||
@@ -144,3 +144,26 @@ func TestCLIProfileBuildOptionsUseResolvedLocaleForAutoPricing(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestTUIDiagnosticsMilestoneFlushesNonEmptyLog(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
d := startTUIDiagnostics(home)
|
||||
t.Cleanup(d.Close)
|
||||
d.Milestone("config_load_begin")
|
||||
d.Milestone("controller_build_done")
|
||||
if d.Path() == "" {
|
||||
t.Fatal("expected diagnostic log path")
|
||||
}
|
||||
body, err := os.ReadFile(d.Path())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(body) == 0 {
|
||||
t.Fatal("diagnostic log must not be empty after milestones")
|
||||
}
|
||||
for _, want := range []string{"diagnostics_started", "config_load_begin", "controller_build_done"} {
|
||||
if !strings.Contains(string(body), want) {
|
||||
t.Fatalf("log missing %q:\n%s", want, body)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/joho/godotenv"
|
||||
|
||||
@@ -63,7 +64,13 @@ var credentialSourceTracker = struct {
|
||||
var userCredentialEditMu sync.Mutex
|
||||
|
||||
var storedCredentialValueLookup = storedCredentialValue
|
||||
var legacyKeyringCredentialValueLookup = legacyKeyringCredentialValue
|
||||
|
||||
// legacyKeyringProbeLookup is the test-facing single-key hook returning the full
|
||||
// four-state outcome under a caller-owned context (shared 1s migration budget).
|
||||
var legacyKeyringProbeLookup = legacyKeyringProbe
|
||||
|
||||
// legacyKeyringLookupTimeout is the shared budget for one legacy keyring scan.
|
||||
var legacyKeyringLookupTimeout = time.Second
|
||||
|
||||
// CredentialResolver resolves credentials repeatedly for one caller-owned view
|
||||
// build. It keeps expensive global credential-store lookups bounded to one per
|
||||
@@ -276,6 +283,34 @@ func StoreCredentialLines(lines []string) (string, error) {
|
||||
return storeCredentialAssignmentsLocked(assignments)
|
||||
}
|
||||
|
||||
// storeCredentialIfAbsentAndNotCleared writes key=value only when the current
|
||||
// credential store still lacks a value and a cleared tombstone for key. The
|
||||
// re-check and write share LockUserCredentialEdits so a concurrent settings
|
||||
// save or tombstone cannot be overwritten by a stale keyring import.
|
||||
// stored is false when the write was skipped because the key is already present
|
||||
// or cleared; err is non-nil only on lock/IO failures.
|
||||
func storeCredentialIfAbsentAndNotCleared(key, value string) (stored bool, err error) {
|
||||
key = strings.TrimSpace(key)
|
||||
if key == "" || !isCredentialKey(key) {
|
||||
return false, nil
|
||||
}
|
||||
if strings.ContainsAny(value, "\r\n") {
|
||||
return false, fmt.Errorf("credential value for %s contains a newline", key)
|
||||
}
|
||||
unlock, err := LockUserCredentialEdits()
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
defer unlock()
|
||||
if credentialCurrentStoreHasKey(key) || credentialCurrentStoreClearedKey(key) {
|
||||
return false, nil
|
||||
}
|
||||
if _, err := storeCredentialAssignmentsLocked(map[string]string{key: value}); err != nil {
|
||||
return false, err
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
func storeCredentialAssignmentsLocked(assignments map[string]string) (string, error) {
|
||||
if err := storeCredentialsInFile(UserCredentialsPath(), assignments); err != nil {
|
||||
return "", err
|
||||
|
||||
@@ -3,16 +3,37 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strings"
|
||||
|
||||
"github.com/zalando/go-keyring"
|
||||
)
|
||||
|
||||
func legacyKeyringCredentialValue(key string) (string, bool) {
|
||||
// legacyKeyringProbe reads one legacy credential from the platform keyring.
|
||||
// keyring.ErrNotFound (and empty values) are absent; other errors stay error.
|
||||
// ctx is checked before the call so a shared batch budget can stop the scan;
|
||||
// platform Get is not context-aware on these OSes.
|
||||
func legacyKeyringProbe(ctx context.Context, key string) legacyKeyringOutcome {
|
||||
key = strings.TrimSpace(key)
|
||||
if key == "" {
|
||||
return "", false
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
if err := ctx.Err(); err != nil {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
}
|
||||
value, err := keyring.Get(credentialsKeyringService, key)
|
||||
return value, err == nil && value != ""
|
||||
if err != nil {
|
||||
if errors.Is(err, keyring.ErrNotFound) {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
if ctx.Err() != nil {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
}
|
||||
return legacyKeyringOutcome{Status: legacyKeyringError}
|
||||
}
|
||||
if strings.TrimSpace(value) == "" {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: value}
|
||||
}
|
||||
@@ -0,0 +1,75 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// legacyKeyringStatus classifies one keyring probe outcome for migration.
|
||||
// Only absent may write a migration-done marker.
|
||||
type legacyKeyringStatus string
|
||||
|
||||
const (
|
||||
legacyKeyringFound legacyKeyringStatus = "found"
|
||||
legacyKeyringAbsent legacyKeyringStatus = "absent"
|
||||
legacyKeyringError legacyKeyringStatus = "error"
|
||||
legacyKeyringTimeout legacyKeyringStatus = "timeout"
|
||||
)
|
||||
|
||||
// legacyKeyringOutcome is the four-state result for one env key.
|
||||
// Value is populated only for found probes inside this process and is scrubbed
|
||||
// before the outcome is returned to migration callers after store-if-absent.
|
||||
type legacyKeyringOutcome struct {
|
||||
Status legacyKeyringStatus
|
||||
Value string
|
||||
}
|
||||
|
||||
// lookupLegacyKeyringBatch probes keys under a single shared deadline in-process.
|
||||
// There is no external helper entrypoint: secrets never leave the process via
|
||||
// stdout or a caller-controlled REASONIX_HOME dump path.
|
||||
func lookupLegacyKeyringBatch(keys []string, budget time.Duration) map[string]legacyKeyringOutcome {
|
||||
out := make(map[string]legacyKeyringOutcome, len(keys))
|
||||
if len(keys) == 0 {
|
||||
return out
|
||||
}
|
||||
if budget <= 0 {
|
||||
budget = time.Second
|
||||
}
|
||||
ctx, cancel := context.WithTimeout(context.Background(), budget)
|
||||
defer cancel()
|
||||
|
||||
for _, key := range keys {
|
||||
if err := ctx.Err(); err != nil {
|
||||
out[key] = legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
continue
|
||||
}
|
||||
o := legacyKeyringProbeLookup(ctx, key)
|
||||
switch o.Status {
|
||||
case legacyKeyringFound:
|
||||
if strings.TrimSpace(o.Value) == "" {
|
||||
out[key] = legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
continue
|
||||
}
|
||||
stored, err := storeCredentialIfAbsentAndNotCleared(key, o.Value)
|
||||
o.Value = "" // scrub before returning
|
||||
if err != nil {
|
||||
out[key] = legacyKeyringOutcome{Status: legacyKeyringError}
|
||||
continue
|
||||
}
|
||||
if !stored {
|
||||
// Current store already has a value or tombstone; do not apply
|
||||
// the legacy keyring secret. Report found so migration does not
|
||||
// re-probe endlessly without writing an absent marker.
|
||||
out[key] = legacyKeyringOutcome{Status: legacyKeyringFound}
|
||||
continue
|
||||
}
|
||||
out[key] = legacyKeyringOutcome{Status: legacyKeyringFound}
|
||||
case legacyKeyringAbsent, legacyKeyringError, legacyKeyringTimeout:
|
||||
out[key] = legacyKeyringOutcome{Status: o.Status}
|
||||
default:
|
||||
out[key] = legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
@@ -0,0 +1,229 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/base64"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestLookupLegacyKeyringBatchInProcessFourState(t *testing.T) {
|
||||
oldLookup := legacyKeyringProbeLookup
|
||||
t.Cleanup(func() { legacyKeyringProbeLookup = oldLookup })
|
||||
|
||||
legacyKeyringProbeLookup = func(_ context.Context, key string) legacyKeyringOutcome {
|
||||
switch key {
|
||||
case "FOUND":
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: "secret"}
|
||||
case "EMPTY":
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: ""}
|
||||
case "ERR":
|
||||
return legacyKeyringOutcome{Status: legacyKeyringError}
|
||||
default:
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
}
|
||||
got := lookupLegacyKeyringBatch([]string{"FOUND", "EMPTY", "MISSING", "ERR"}, time.Second)
|
||||
if got["FOUND"].Status != legacyKeyringFound || got["FOUND"].Value != "" {
|
||||
t.Fatalf("FOUND = %+v, want found with scrubbed value", got["FOUND"])
|
||||
}
|
||||
if !credentialCurrentStoreHasKey("FOUND") {
|
||||
t.Fatal("FOUND secret should have been stored via store-if-absent")
|
||||
}
|
||||
if got["EMPTY"].Status != legacyKeyringAbsent {
|
||||
t.Fatalf("EMPTY = %+v, want absent", got["EMPTY"])
|
||||
}
|
||||
if got["MISSING"].Status != legacyKeyringAbsent {
|
||||
t.Fatalf("MISSING = %+v, want absent", got["MISSING"])
|
||||
}
|
||||
if got["ERR"].Status != legacyKeyringError {
|
||||
t.Fatalf("ERR = %+v, want error", got["ERR"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestLegacyKeyringErrorDoesNotWriteMarker(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
t.Setenv("REASONIX_HOME", home)
|
||||
t.Setenv("REASONIX_CREDENTIALS_STORE", "file")
|
||||
|
||||
oldLookup := legacyKeyringProbeLookup
|
||||
t.Cleanup(func() { legacyKeyringProbeLookup = oldLookup })
|
||||
|
||||
legacyKeyringProbeLookup = func(context.Context, string) legacyKeyringOutcome {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringError}
|
||||
}
|
||||
outcomes := lookupLegacyKeyringBatch([]string{"DEEPSEEK_API_KEY"}, time.Second)
|
||||
if outcomes["DEEPSEEK_API_KEY"].Status != legacyKeyringError {
|
||||
t.Fatalf("status = %+v", outcomes["DEEPSEEK_API_KEY"])
|
||||
}
|
||||
if outcomes["DEEPSEEK_API_KEY"].Status == legacyKeyringAbsent {
|
||||
_ = markLegacyKeyringMigrationDone("DEEPSEEK_API_KEY")
|
||||
}
|
||||
if legacyKeyringMigrationDone("DEEPSEEK_API_KEY") {
|
||||
t.Fatal("error outcome must not write a migration marker")
|
||||
}
|
||||
}
|
||||
|
||||
func TestLookupLegacyKeyringBatchSharedContextTimeout(t *testing.T) {
|
||||
oldLookup := legacyKeyringProbeLookup
|
||||
oldTimeout := legacyKeyringLookupTimeout
|
||||
legacyKeyringLookupTimeout = 40 * time.Millisecond
|
||||
t.Cleanup(func() {
|
||||
legacyKeyringProbeLookup = oldLookup
|
||||
legacyKeyringLookupTimeout = oldTimeout
|
||||
})
|
||||
|
||||
legacyKeyringProbeLookup = func(ctx context.Context, key string) legacyKeyringOutcome {
|
||||
if key == "FAST" {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
// Block until the shared budget cancels; must not leave a hung goroutine
|
||||
// after the batch returns (probe returns when ctx is done).
|
||||
<-ctx.Done()
|
||||
return legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
}
|
||||
|
||||
start := time.Now()
|
||||
got := lookupLegacyKeyringBatch([]string{"FAST", "SLOW"}, legacyKeyringLookupTimeout)
|
||||
elapsed := time.Since(start)
|
||||
if got["FAST"].Status != legacyKeyringAbsent {
|
||||
t.Fatalf("FAST = %+v", got["FAST"])
|
||||
}
|
||||
if got["SLOW"].Status != legacyKeyringTimeout {
|
||||
t.Fatalf("SLOW = %+v, want timeout", got["SLOW"])
|
||||
}
|
||||
if elapsed > 250*time.Millisecond {
|
||||
t.Fatalf("elapsed %v, shared budget did not bound the scan", elapsed)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLookupLegacyKeyringBatchDoesNotClobberUserWrite forces the production
|
||||
// probe→store window: the batch has already decided the key needs import
|
||||
// (probe returns found), then the user writes a new value before store-if-absent.
|
||||
func TestLookupLegacyKeyringBatchDoesNotClobberUserWrite(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
t.Setenv("REASONIX_HOME", home)
|
||||
t.Setenv("REASONIX_CREDENTIALS_STORE", "file")
|
||||
|
||||
oldLookup := legacyKeyringProbeLookup
|
||||
t.Cleanup(func() { legacyKeyringProbeLookup = oldLookup })
|
||||
|
||||
probeReached := make(chan struct{})
|
||||
releaseProbe := make(chan struct{})
|
||||
var once sync.Once
|
||||
legacyKeyringProbeLookup = func(ctx context.Context, key string) legacyKeyringOutcome {
|
||||
if key != "DEEPSEEK_API_KEY" {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
once.Do(func() { close(probeReached) })
|
||||
select {
|
||||
case <-releaseProbe:
|
||||
case <-ctx.Done():
|
||||
return legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
}
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: "sk-old-keyring"}
|
||||
}
|
||||
|
||||
done := make(chan map[string]legacyKeyringOutcome, 1)
|
||||
go func() {
|
||||
done <- lookupLegacyKeyringBatch([]string{"DEEPSEEK_API_KEY"}, time.Second)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-probeReached:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("probe did not reach the interleaving checkpoint")
|
||||
}
|
||||
if _, err := SetCredential("DEEPSEEK_API_KEY", "sk-user-new"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
close(releaseProbe)
|
||||
|
||||
got := <-done
|
||||
if got["DEEPSEEK_API_KEY"].Status != legacyKeyringFound {
|
||||
t.Fatalf("batch status = %+v, want found (store skipped)", got["DEEPSEEK_API_KEY"])
|
||||
}
|
||||
val, ok := envFileValue(UserCredentialsPath(), "DEEPSEEK_API_KEY")
|
||||
if !ok || val != "sk-user-new" {
|
||||
t.Fatalf("credential = (%q, %v), want sk-user-new (keyring must not clobber)", val, ok)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLookupLegacyKeyringBatchDoesNotReviveTombstone forces the same production
|
||||
// probe→store window against a cleared tombstone written after the probe starts.
|
||||
func TestLookupLegacyKeyringBatchDoesNotReviveTombstone(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
t.Setenv("REASONIX_HOME", home)
|
||||
t.Setenv("REASONIX_CREDENTIALS_STORE", "file")
|
||||
|
||||
// Start empty: no value and no tombstone so the batch will probe.
|
||||
oldLookup := legacyKeyringProbeLookup
|
||||
t.Cleanup(func() { legacyKeyringProbeLookup = oldLookup })
|
||||
|
||||
probeReached := make(chan struct{})
|
||||
releaseProbe := make(chan struct{})
|
||||
var once sync.Once
|
||||
legacyKeyringProbeLookup = func(ctx context.Context, key string) legacyKeyringOutcome {
|
||||
if key != "DEEPSEEK_API_KEY" {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
once.Do(func() { close(probeReached) })
|
||||
select {
|
||||
case <-releaseProbe:
|
||||
case <-ctx.Done():
|
||||
return legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
}
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: "sk-old-keyring"}
|
||||
}
|
||||
|
||||
done := make(chan map[string]legacyKeyringOutcome, 1)
|
||||
go func() {
|
||||
done <- lookupLegacyKeyringBatch([]string{"DEEPSEEK_API_KEY"}, time.Second)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-probeReached:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("probe did not reach the interleaving checkpoint")
|
||||
}
|
||||
// Write then clear while probe is blocked: tombstone must win.
|
||||
if _, err := SetCredential("DEEPSEEK_API_KEY", "sk-temp"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := RemoveCredential("DEEPSEEK_API_KEY"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !credentialCurrentStoreClearedKey("DEEPSEEK_API_KEY") {
|
||||
t.Fatal("expected cleared tombstone before releasing probe")
|
||||
}
|
||||
close(releaseProbe)
|
||||
|
||||
got := <-done
|
||||
if got["DEEPSEEK_API_KEY"].Status != legacyKeyringFound {
|
||||
// store-if-absent skipped → still reported found (already handled)
|
||||
t.Fatalf("batch status = %+v", got["DEEPSEEK_API_KEY"])
|
||||
}
|
||||
if credentialCurrentStoreHasKey("DEEPSEEK_API_KEY") {
|
||||
t.Fatal("keyring import revived a tombstoned credential")
|
||||
}
|
||||
if !credentialCurrentStoreClearedKey("DEEPSEEK_API_KEY") {
|
||||
t.Fatal("tombstone missing after batch")
|
||||
}
|
||||
}
|
||||
|
||||
func TestLegacyKeyringMarkerUsesRawURLBase64(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
t.Setenv("REASONIX_HOME", home)
|
||||
key := "A/B+C="
|
||||
path := legacyKeyringMigrationMarkerPath(key)
|
||||
wantName := base64.RawURLEncoding.EncodeToString([]byte(key))
|
||||
if !strings.HasSuffix(path, wantName) {
|
||||
t.Fatalf("marker path = %q, want suffix %q", path, wantName)
|
||||
}
|
||||
other := legacyKeyringMigrationMarkerPath("A_B_C_")
|
||||
if path == other {
|
||||
t.Fatalf("marker collision between %q and A_B_C_", key)
|
||||
}
|
||||
}
|
||||
@@ -3,48 +3,231 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
ss "github.com/zalando/go-keyring/secret_service"
|
||||
dbus "github.com/godbus/dbus/v5"
|
||||
)
|
||||
|
||||
func legacyKeyringCredentialValue(key string) (string, bool) {
|
||||
// Secret Service constants (same service Reasonix historically used via
|
||||
// zalando/go-keyring). Every D-Bus call uses the shared migration context so a
|
||||
// stuck bus cannot hang CLI startup past the batch deadline.
|
||||
const (
|
||||
ssServiceName = "org.freedesktop.secrets"
|
||||
ssServicePath = "/org/freedesktop/secrets"
|
||||
ssServiceInterface = "org.freedesktop.Secret.Service"
|
||||
ssCollectionInterface = "org.freedesktop.Secret.Collection"
|
||||
ssItemInterface = "org.freedesktop.Secret.Item"
|
||||
ssSessionInterface = "org.freedesktop.Secret.Session"
|
||||
ssCollectionsIface = "org.freedesktop.Secret.Service"
|
||||
ssCollectionsProp = "Collections"
|
||||
ssLoginCollection = "/org/freedesktop/secrets/collection/login"
|
||||
ssLoginAlias = "/org/freedesktop/secrets/aliases/default"
|
||||
ssPropertiesInterface = "org.freedesktop.DBus.Properties"
|
||||
)
|
||||
|
||||
type ssSecret struct {
|
||||
Session dbus.ObjectPath
|
||||
Parameters []byte
|
||||
Value []byte
|
||||
ContentType string `dbus:"content_type"`
|
||||
}
|
||||
|
||||
// legacyKeyringProbe reads one legacy credential from Secret Service under ctx.
|
||||
// It opens a private, caller-owned session-bus connection (never dbus.SessionBus),
|
||||
// bounds Dial/Auth/Hello by ctx, routes every method/property call through
|
||||
// CallWithContext(ctx), and always closes the connection before returning so
|
||||
// godbus worker goroutines cannot leak into goleak-checked tests.
|
||||
func legacyKeyringProbe(ctx context.Context, key string) legacyKeyringOutcome {
|
||||
key = strings.TrimSpace(key)
|
||||
if key == "" {
|
||||
return "", false
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
if err := ctx.Err(); err != nil {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
}
|
||||
|
||||
svc, err := ss.NewSecretService()
|
||||
conn, err := openPrivateSessionBus(ctx)
|
||||
if err != nil {
|
||||
return "", false
|
||||
return mapKeyringCtxErr(ctx, err)
|
||||
}
|
||||
defer svc.Conn.Close()
|
||||
defer func() { _ = conn.Close() }()
|
||||
|
||||
collection := svc.GetLoginCollection()
|
||||
svc := conn.Object(ssServiceName, ssServicePath)
|
||||
collectionPath, err := ssResolveLoginCollection(ctx, svc)
|
||||
if err != nil {
|
||||
return mapKeyringCtxErr(ctx, err)
|
||||
}
|
||||
if err := ssUnlock(ctx, svc, collectionPath); err != nil {
|
||||
return mapKeyringCtxErr(ctx, err)
|
||||
}
|
||||
|
||||
collection := conn.Object(ssServiceName, collectionPath)
|
||||
search := map[string]string{
|
||||
"username": key,
|
||||
"service": credentialsKeyringService,
|
||||
}
|
||||
if err := svc.Unlock(collection.Path()); err != nil {
|
||||
return "", false
|
||||
var results []dbus.ObjectPath
|
||||
if err := collection.CallWithContext(ctx, ssCollectionInterface+".SearchItems", 0, search).Store(&results); err != nil {
|
||||
return mapKeyringCtxErr(ctx, err)
|
||||
}
|
||||
results, err := svc.SearchItems(collection, search)
|
||||
if err != nil || len(results) == 0 {
|
||||
return "", false
|
||||
if len(results) == 0 {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
|
||||
session, err := svc.OpenSession()
|
||||
if err != nil {
|
||||
return "", false
|
||||
var disregard dbus.Variant
|
||||
var sessionPath dbus.ObjectPath
|
||||
if err := svc.CallWithContext(ctx, ssServiceInterface+".OpenSession", 0, "plain", dbus.MakeVariant("")).Store(&disregard, &sessionPath); err != nil {
|
||||
return mapKeyringCtxErr(ctx, err)
|
||||
}
|
||||
defer svc.Close(session)
|
||||
// Always close the Secret Service session with the remaining budget (never
|
||||
// context.Background) so a stuck Close still respects the migration deadline.
|
||||
defer func() {
|
||||
session := conn.Object(ssServiceName, sessionPath)
|
||||
_ = session.CallWithContext(ctx, ssSessionInterface+".Close", 0).Err
|
||||
}()
|
||||
|
||||
if err := svc.Unlock(results[0]); err != nil {
|
||||
return "", false
|
||||
if err := ssUnlock(ctx, svc, results[0]); err != nil {
|
||||
return mapKeyringCtxErr(ctx, err)
|
||||
}
|
||||
secret, err := svc.GetSecret(results[0], session.Path())
|
||||
if err != nil || secret == nil || len(secret.Value) == 0 {
|
||||
return "", false
|
||||
|
||||
var secret ssSecret
|
||||
item := conn.Object(ssServiceName, results[0])
|
||||
if err := item.CallWithContext(ctx, ssItemInterface+".GetSecret", 0, sessionPath).Store(&secret); err != nil {
|
||||
return mapKeyringCtxErr(ctx, err)
|
||||
}
|
||||
return string(secret.Value), true
|
||||
if len(secret.Value) == 0 {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: string(secret.Value)}
|
||||
}
|
||||
|
||||
// openPrivateSessionBus dials a private session-bus connection (Auth+Hello)
|
||||
// without using the process-global SessionBus cache.
|
||||
//
|
||||
// Connection lifecycle is bound to ctx via dbus.WithContext(ctx): when the
|
||||
// migration budget expires, godbus closes the transport so Auth/Hello that have
|
||||
// already obtained a *Conn unblock instead of hanging forever. We never call
|
||||
// dbus-launch (NoAutoStartup): missing session address fails closed as error,
|
||||
// which is correct for headless/CI and avoids an uncancellable CombinedOutput.
|
||||
//
|
||||
// Dial itself is still not fully context-cancellable in godbus before newConn
|
||||
// installs WithContext; the outer select bounds the caller's wait, and any late
|
||||
// *Conn is always closed.
|
||||
func openPrivateSessionBus(ctx context.Context) (*dbus.Conn, error) {
|
||||
if err := ctx.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
type result struct {
|
||||
conn *dbus.Conn
|
||||
err error
|
||||
}
|
||||
ch := make(chan result, 1)
|
||||
go func() {
|
||||
conn, err := connectPrivateSessionBus(ctx)
|
||||
ch <- result{conn: conn, err: err}
|
||||
}()
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
// Drain so the connect goroutine is reaped when WithContext causes
|
||||
// Auth/Hello to return; always Close a late *Conn.
|
||||
go func() {
|
||||
r := <-ch
|
||||
if r.conn != nil {
|
||||
_ = r.conn.Close()
|
||||
}
|
||||
}()
|
||||
return nil, ctx.Err()
|
||||
case r := <-ch:
|
||||
if r.err != nil {
|
||||
if r.conn != nil {
|
||||
_ = r.conn.Close()
|
||||
}
|
||||
if ctx.Err() != nil {
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
return nil, r.err
|
||||
}
|
||||
if err := ctx.Err(); err != nil {
|
||||
_ = r.conn.Close()
|
||||
return nil, err
|
||||
}
|
||||
return r.conn, nil
|
||||
}
|
||||
}
|
||||
|
||||
// connectPrivateSessionBus opens a private, context-bound session bus and
|
||||
// completes Auth+Hello. Prefer NoAutoStartup so we never block in dbus-launch.
|
||||
func connectPrivateSessionBus(ctx context.Context) (*dbus.Conn, error) {
|
||||
// WithContext: parent cancel → conn.Close → unblocks Auth transport I/O and
|
||||
// Hello Call waiters once the *Conn exists.
|
||||
conn, err := dbus.SessionBusPrivateNoAutoStartup(dbus.WithContext(ctx))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := conn.Auth(nil); err != nil {
|
||||
_ = conn.Close()
|
||||
return nil, err
|
||||
}
|
||||
if err := conn.Hello(); err != nil {
|
||||
_ = conn.Close()
|
||||
return nil, err
|
||||
}
|
||||
return conn, nil
|
||||
}
|
||||
|
||||
func ssResolveLoginCollection(ctx context.Context, svc dbus.BusObject) (dbus.ObjectPath, error) {
|
||||
path := dbus.ObjectPath(ssLoginCollection)
|
||||
val, err := ssGetProperty(ctx, svc, ssCollectionsIface, ssCollectionsProp)
|
||||
if err != nil {
|
||||
// Fall back to the default alias when Collections is unavailable.
|
||||
return dbus.ObjectPath(ssLoginAlias), nil
|
||||
}
|
||||
paths, _ := val.Value().([]dbus.ObjectPath)
|
||||
for _, p := range paths {
|
||||
if p == path {
|
||||
return path, nil
|
||||
}
|
||||
}
|
||||
return dbus.ObjectPath(ssLoginAlias), nil
|
||||
}
|
||||
|
||||
// ssGetProperty is CallWithContext-based Properties.Get. BusObject.GetProperty
|
||||
// uses a non-context Call and would escape the migration deadline.
|
||||
func ssGetProperty(ctx context.Context, obj dbus.BusObject, iface, name string) (dbus.Variant, error) {
|
||||
var val dbus.Variant
|
||||
err := obj.CallWithContext(ctx, ssPropertiesInterface+".Get", 0, iface, name).Store(&val)
|
||||
if err != nil {
|
||||
return dbus.Variant{}, err
|
||||
}
|
||||
return val, nil
|
||||
}
|
||||
|
||||
func ssUnlock(ctx context.Context, svc dbus.BusObject, target dbus.ObjectPath) error {
|
||||
var unlocked []dbus.ObjectPath
|
||||
var prompt dbus.ObjectPath
|
||||
if err := svc.CallWithContext(ctx, ssServiceInterface+".Unlock", 0, []dbus.ObjectPath{target}).Store(&unlocked, &prompt); err != nil {
|
||||
return err
|
||||
}
|
||||
// Migration must not wait on an interactive prompt (would hang CLI startup).
|
||||
if prompt != "/" && prompt != "" {
|
||||
for _, p := range unlocked {
|
||||
if p == target || target == dbus.ObjectPath(ssLoginAlias) {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
return fmt.Errorf("secret service unlock requires interactive prompt")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func mapKeyringCtxErr(ctx context.Context, err error) legacyKeyringOutcome {
|
||||
if err == nil {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
if ctx.Err() != nil {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringTimeout}
|
||||
}
|
||||
return legacyKeyringOutcome{Status: legacyKeyringError}
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
@@ -15,7 +16,9 @@ func TestMain(m *testing.M) {
|
||||
// keyring has no environment variable in front of it, so legacy migration
|
||||
// would read the credentials of whoever is running the tests. Cases that
|
||||
// exercise the lookup substitute their own.
|
||||
legacyKeyringCredentialValueLookup = func(string) (string, bool) { return "", false }
|
||||
legacyKeyringProbeLookup = func(context.Context, string) legacyKeyringOutcome {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
testenv.RunWithIsolatedUserState(m)
|
||||
}
|
||||
|
||||
@@ -23,8 +26,9 @@ func TestMain(m *testing.M) {
|
||||
// stay green either way, so the substitution itself is what gets asserted.
|
||||
func TestKeyringLookupStaysOutOfTheRealStore(t *testing.T) {
|
||||
for _, key := range []string{"DEEPSEEK_API_KEY", "OPENAI_API_KEY", "ANTHROPIC_API_KEY"} {
|
||||
if value, ok := legacyKeyringCredentialValueLookup(key); ok || value != "" {
|
||||
t.Fatalf("%s resolved through the real keyring in tests (ok=%v, %d bytes)", key, ok, len(value))
|
||||
o := legacyKeyringProbeLookup(context.Background(), key)
|
||||
if o.Status != legacyKeyringAbsent || o.Value != "" {
|
||||
t.Fatalf("%s resolved through the real keyring in tests: %+v", key, o)
|
||||
}
|
||||
}
|
||||
}
|
||||
+72
-10
@@ -2,6 +2,7 @@ package config
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -370,17 +371,13 @@ func normalizedMCPMigrationRoots(roots []string) []string {
|
||||
|
||||
func migrateLegacyCredentialsIfNeededForRoot(root string) error {
|
||||
missing := map[string]string{}
|
||||
skip := func(key string) bool {
|
||||
// File import ignores keyring markers: a marker only means "do not re-probe
|
||||
// keyring for this env name", never "skip legacy credential files".
|
||||
skipStore := func(key string) bool {
|
||||
return credentialCurrentStoreHasKey(key) || credentialCurrentStoreClearedKey(key)
|
||||
}
|
||||
for _, key := range credentialEnvNamesForRoot(root) {
|
||||
if skip(key) {
|
||||
continue
|
||||
}
|
||||
if value, ok := legacyKeyringCredentialValueLookup(key); ok {
|
||||
missing[key] = value
|
||||
}
|
||||
}
|
||||
// Prefer legacy credential files first so a healthy file import does not
|
||||
// depend on Secret Service / D-Bus (#7507).
|
||||
for _, src := range legacyCredentialsPaths() {
|
||||
if src == "" {
|
||||
continue
|
||||
@@ -391,11 +388,44 @@ func migrateLegacyCredentialsIfNeededForRoot(root string) error {
|
||||
}
|
||||
assignments := parseCredentialLines(strings.Split(string(data), "\n"))
|
||||
for key, value := range assignments {
|
||||
if _, exists := missing[key]; !exists && !skip(key) {
|
||||
if _, exists := missing[key]; !exists && !skipStore(key) {
|
||||
missing[key] = value
|
||||
}
|
||||
}
|
||||
}
|
||||
keys := credentialEnvNamesForRoot(root)
|
||||
needKeyring := make([]string, 0, len(keys))
|
||||
for _, key := range keys {
|
||||
if skipStore(key) {
|
||||
continue
|
||||
}
|
||||
if _, exists := missing[key]; exists {
|
||||
continue
|
||||
}
|
||||
// Marker only filters keyring probes.
|
||||
if legacyKeyringMigrationDone(key) {
|
||||
continue
|
||||
}
|
||||
needKeyring = append(needKeyring, key)
|
||||
}
|
||||
if len(needKeyring) > 0 {
|
||||
outcomes := lookupLegacyKeyringBatch(needKeyring, legacyKeyringLookupTimeout)
|
||||
for _, key := range needKeyring {
|
||||
o := outcomes[key]
|
||||
switch o.Status {
|
||||
case legacyKeyringFound:
|
||||
// Secret was stored by the probe path (helper or in-process).
|
||||
// Do not trust Value from the parent-visible outcome map.
|
||||
case legacyKeyringAbsent:
|
||||
// Confirmed empty probe only — never on error/timeout.
|
||||
_ = markLegacyKeyringMigrationDone(key)
|
||||
case legacyKeyringError, legacyKeyringTimeout:
|
||||
// Leave unmarked so the next launch retries.
|
||||
default:
|
||||
// Unknown status: treat as timeout (no marker).
|
||||
}
|
||||
}
|
||||
}
|
||||
if len(missing) == 0 {
|
||||
return nil
|
||||
}
|
||||
@@ -403,6 +433,38 @@ func migrateLegacyCredentialsIfNeededForRoot(root string) error {
|
||||
return err
|
||||
}
|
||||
|
||||
func legacyKeyringMigrationMarkerPath(key string) string {
|
||||
home := ReasonixHomeDir()
|
||||
key = strings.TrimSpace(key)
|
||||
if strings.TrimSpace(home) == "" || key == "" {
|
||||
return ""
|
||||
}
|
||||
// Env var names are identifiers, not secrets. RawURL base64 is collision-free
|
||||
// and filesystem-safe without hashing secret material.
|
||||
name := base64.RawURLEncoding.EncodeToString([]byte(key))
|
||||
return filepath.Join(home, "state", "legacy-keyring-checked", name)
|
||||
}
|
||||
|
||||
func legacyKeyringMigrationDone(key string) bool {
|
||||
path := legacyKeyringMigrationMarkerPath(key)
|
||||
if path == "" {
|
||||
return false
|
||||
}
|
||||
_, err := os.Stat(path)
|
||||
return err == nil
|
||||
}
|
||||
|
||||
func markLegacyKeyringMigrationDone(key string) error {
|
||||
path := legacyKeyringMigrationMarkerPath(key)
|
||||
if path == "" {
|
||||
return nil
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||
return err
|
||||
}
|
||||
return os.WriteFile(path, []byte("v1\n"), 0o644)
|
||||
}
|
||||
|
||||
func credentialLines(assignments map[string]string) []string {
|
||||
keys := make([]string, 0, len(assignments))
|
||||
for key := range assignments {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
@@ -715,14 +716,14 @@ func TestMigrateImportsLegacyCredentialsEvenWhenPrimaryConfigExists(t *testing.T
|
||||
|
||||
func TestMigrateImportsLegacyKeyringCredentials(t *testing.T) {
|
||||
legacyHome(t)
|
||||
old := legacyKeyringCredentialValueLookup
|
||||
legacyKeyringCredentialValueLookup = func(key string) (string, bool) {
|
||||
old := legacyKeyringProbeLookup
|
||||
legacyKeyringProbeLookup = func(_ context.Context, key string) legacyKeyringOutcome {
|
||||
if key == "DEEPSEEK_API_KEY" {
|
||||
return "sk-old-keyring", true
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: "sk-old-keyring"}
|
||||
}
|
||||
return "", false
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
t.Cleanup(func() { legacyKeyringCredentialValueLookup = old })
|
||||
t.Cleanup(func() { legacyKeyringProbeLookup = old })
|
||||
|
||||
res, err := MigrateLegacyIfNeeded()
|
||||
if err != nil {
|
||||
@@ -760,14 +761,14 @@ api_key_env = "WORKSPACE_ONLY_KEY"
|
||||
`), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
old := legacyKeyringCredentialValueLookup
|
||||
legacyKeyringCredentialValueLookup = func(key string) (string, bool) {
|
||||
old := legacyKeyringProbeLookup
|
||||
legacyKeyringProbeLookup = func(_ context.Context, key string) legacyKeyringOutcome {
|
||||
if key == "WORKSPACE_ONLY_KEY" {
|
||||
return "sk-workspace", true
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: "sk-workspace"}
|
||||
}
|
||||
return "", false
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
t.Cleanup(func() { legacyKeyringCredentialValueLookup = old })
|
||||
t.Cleanup(func() { legacyKeyringProbeLookup = old })
|
||||
|
||||
if err := MigrateLegacyCredentialsForRoot(project); err != nil {
|
||||
t.Fatalf("MigrateLegacyCredentialsForRoot: %v", err)
|
||||
@@ -791,14 +792,14 @@ func TestMigrateLegacyCredentialsSkipsKeyringWhenIsolated(t *testing.T) {
|
||||
t.Setenv("REASONIX_HOME", isolated)
|
||||
t.Setenv("REASONIX_CREDENTIALS_STORE", "file")
|
||||
|
||||
old := legacyKeyringCredentialValueLookup
|
||||
legacyKeyringCredentialValueLookup = func(key string) (string, bool) {
|
||||
old := legacyKeyringProbeLookup
|
||||
legacyKeyringProbeLookup = func(_ context.Context, key string) legacyKeyringOutcome {
|
||||
if key == "DEEPSEEK_API_KEY" {
|
||||
return "legacy-keyring-value", true
|
||||
return legacyKeyringOutcome{Status: legacyKeyringFound, Value: "legacy-keyring-value"}
|
||||
}
|
||||
return "", false
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
t.Cleanup(func() { legacyKeyringCredentialValueLookup = old })
|
||||
t.Cleanup(func() { legacyKeyringProbeLookup = old })
|
||||
|
||||
if err := MigrateLegacyCredentialsForRoot("."); err != nil {
|
||||
t.Fatalf("MigrateLegacyCredentialsForRoot: %v", err)
|
||||
@@ -1064,3 +1065,67 @@ func TestMigrateSupportData(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestMigrateLegacyCredentialsFileImportIgnoresKeyringMarker(t *testing.T) {
|
||||
_, dest, _ := legacyHome(t)
|
||||
if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(dest, []byte(`default_model = "deepseek-flash/deepseek-chat"`), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := markLegacyKeyringMigrationDone("DEEPSEEK_API_KEY"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
paths := legacyCredentialsPaths()
|
||||
if len(paths) == 0 {
|
||||
t.Skip("no legacy credentials paths on this platform")
|
||||
}
|
||||
target := paths[0]
|
||||
if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(target, []byte("DEEPSEEK_API_KEY=sk-from-file\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
oldLookup := legacyKeyringProbeLookup
|
||||
legacyKeyringProbeLookup = func(context.Context, string) legacyKeyringOutcome {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringAbsent}
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
legacyKeyringProbeLookup = oldLookup
|
||||
})
|
||||
|
||||
if err := MigrateLegacyCredentialsForRoot("."); err != nil {
|
||||
t.Fatalf("MigrateLegacyCredentialsForRoot: %v", err)
|
||||
}
|
||||
data, err := os.ReadFile(UserCredentialsPath())
|
||||
if err != nil {
|
||||
t.Fatalf("read migrated credentials: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(data), "sk-from-file") {
|
||||
t.Fatalf("file import blocked by keyring marker: %q", data)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMigrateLegacyCredentialsErrorDoesNotWriteKeyringMarker(t *testing.T) {
|
||||
legacyHome(t)
|
||||
oldLookup := legacyKeyringProbeLookup
|
||||
legacyKeyringProbeLookup = func(context.Context, string) legacyKeyringOutcome {
|
||||
return legacyKeyringOutcome{Status: legacyKeyringError}
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
legacyKeyringProbeLookup = oldLookup
|
||||
})
|
||||
|
||||
if err := MigrateLegacyCredentialsForRoot("."); err != nil {
|
||||
t.Fatalf("MigrateLegacyCredentialsForRoot: %v", err)
|
||||
}
|
||||
if legacyKeyringMigrationDone("DEEPSEEK_API_KEY") {
|
||||
t.Fatal("keyring error must not write a migration-done marker")
|
||||
}
|
||||
if credentialCurrentStoreHasKey("DEEPSEEK_API_KEY") {
|
||||
t.Fatal("keyring error must not store a credential")
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"io"
|
||||
"net/url"
|
||||
"os"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
@@ -148,6 +149,12 @@ func Collect(opts Options) Report {
|
||||
if bashConfigIgnored {
|
||||
warnings = append(warnings, `config requests [sandbox] bash = "enforce", but Windows does not provide an OS-level Bash sandbox; the setting is fixed to "off" and bash runs unconfined`)
|
||||
}
|
||||
// Supervised deployments sometimes override HOME onto a service config dir
|
||||
// while Reasonix isolation should use REASONIX_HOME. Do not rewrite
|
||||
// subprocess HOME automatically (#7600 rejected); surface the mismatch.
|
||||
if warn := homeIsolationWarning(); warn != "" {
|
||||
warnings = append(warnings, warn)
|
||||
}
|
||||
report := Report{
|
||||
Version: opts.Version,
|
||||
OS: runtime.GOOS,
|
||||
@@ -359,6 +366,46 @@ func valueOr(s, fallback string) string {
|
||||
return s
|
||||
}
|
||||
|
||||
// homeIsolationWarning detects a process HOME that differs from the OS account
|
||||
// home while REASONIX_HOME is unset. Services should keep the real account HOME
|
||||
// and isolate Reasonix state with REASONIX_HOME instead of rewriting HOME.
|
||||
func homeIsolationWarning() string {
|
||||
if strings.TrimSpace(os.Getenv("REASONIX_HOME")) != "" {
|
||||
return ""
|
||||
}
|
||||
envHome := strings.TrimSpace(os.Getenv("HOME"))
|
||||
if envHome == "" {
|
||||
// Windows services often set USERPROFILE rather than HOME.
|
||||
envHome = strings.TrimSpace(os.Getenv("USERPROFILE"))
|
||||
}
|
||||
if envHome == "" {
|
||||
return ""
|
||||
}
|
||||
acct, err := user.Current()
|
||||
if err != nil || acct == nil || strings.TrimSpace(acct.HomeDir) == "" {
|
||||
return ""
|
||||
}
|
||||
envClean := filepath.Clean(envHome)
|
||||
acctClean := filepath.Clean(acct.HomeDir)
|
||||
if samePathFold(envClean, acctClean) {
|
||||
return ""
|
||||
}
|
||||
// Do not embed either absolute path: when HOME is overridden, redactHome
|
||||
// cannot mask the account home, and shareable doctor output must stay free
|
||||
// of machine-local identity.
|
||||
return "process HOME differs from the OS account home; keep the real account HOME for services and isolate Reasonix with REASONIX_HOME"
|
||||
}
|
||||
|
||||
func samePathFold(a, b string) bool {
|
||||
if a == b {
|
||||
return true
|
||||
}
|
||||
if runtime.GOOS == "windows" {
|
||||
return strings.EqualFold(a, b)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// redactHome rewrites a path under the user's home directory to start with "~",
|
||||
// so a shared diagnostics report doesn't carry the account name. Paths outside
|
||||
// home are returned unchanged.
|
||||
|
||||
@@ -3,6 +3,7 @@ package doctor
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -155,3 +156,30 @@ func TestCollectFlagsIgnoredEnforceConfig(t *testing.T) {
|
||||
t.Fatalf("plain off must not claim the config was ignored:\n%s", plain)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHomeIsolationWarningDetectsMismatch(t *testing.T) {
|
||||
// Prefer a synthetic mismatch without requiring root privileges.
|
||||
acct, err := user.Current()
|
||||
if err != nil || acct == nil || strings.TrimSpace(acct.HomeDir) == "" {
|
||||
t.Skip("account home unavailable")
|
||||
}
|
||||
t.Setenv("REASONIX_HOME", "")
|
||||
serviceHome := filepath.Join(t.TempDir(), "service-home")
|
||||
t.Setenv("HOME", serviceHome)
|
||||
t.Setenv("USERPROFILE", serviceHome)
|
||||
got := homeIsolationWarning()
|
||||
if got == "" {
|
||||
t.Fatal("expected HOME mismatch warning")
|
||||
}
|
||||
if !strings.Contains(got, "REASONIX_HOME") {
|
||||
t.Fatalf("warning = %q, want REASONIX_HOME guidance", got)
|
||||
}
|
||||
// Shareable output must not embed either absolute home path.
|
||||
if strings.Contains(got, serviceHome) || strings.Contains(got, acct.HomeDir) {
|
||||
t.Fatalf("warning leaked a home path: %q", got)
|
||||
}
|
||||
t.Setenv("REASONIX_HOME", t.TempDir())
|
||||
if got := homeIsolationWarning(); got != "" {
|
||||
t.Fatalf("REASONIX_HOME set should silence warning, got %q", got)
|
||||
}
|
||||
}
|
||||
+21
-3
@@ -214,7 +214,14 @@ func ContextFileUsable(path string) bool {
|
||||
// LoadOptions configure Load.
|
||||
type LoadOptions struct {
|
||||
ProjectRoot string
|
||||
HomeDir string
|
||||
// HomeDir overrides the OS user home used by legacy callers and tests. The
|
||||
// derived global path is <HomeDir>/.reasonix unless ReasonixHomeDir is set.
|
||||
HomeDir string
|
||||
// ReasonixHomeDir is the exact current Reasonix home (settings.json lives
|
||||
// directly under it). When set, it takes precedence over HomeDir for global
|
||||
// settings and plugin hooks so Windows %APPDATA%/reasonix and REASONIX_HOME
|
||||
// isolation stay consistent across hook/doctor/capdiag (#7411, #7331).
|
||||
ReasonixHomeDir string
|
||||
// Trusted is retained for source compatibility. Project hooks are enabled
|
||||
// automatically now, so callers no longer need to set it.
|
||||
Trusted bool
|
||||
@@ -231,8 +238,12 @@ func Load(opts LoadOptions) []ResolvedHook {
|
||||
appendResolved(&out, s, ScopeProject, p)
|
||||
}
|
||||
}
|
||||
appendPluginHooks(&out, reasonixHome(opts.HomeDir), opts.ProjectRoot)
|
||||
g := GlobalSettingsPath(opts.HomeDir)
|
||||
reasonixHomeDir := reasonixHomeForOptions(opts)
|
||||
appendPluginHooks(&out, reasonixHomeDir, opts.ProjectRoot)
|
||||
g := filepath.Join(reasonixHomeDir, SettingsFilename)
|
||||
if reasonixHomeDir == "" {
|
||||
g = GlobalSettingsPath(opts.HomeDir)
|
||||
}
|
||||
if s := readSettings(g); s != nil {
|
||||
appendResolved(&out, s, ScopeGlobal, g)
|
||||
} else if !pathExists(g) {
|
||||
@@ -1580,6 +1591,13 @@ func reasonixHome(override string) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
func reasonixHomeForOptions(opts LoadOptions) string {
|
||||
if dir := strings.TrimSpace(opts.ReasonixHomeDir); dir != "" {
|
||||
return filepath.Clean(dir)
|
||||
}
|
||||
return reasonixHome(opts.HomeDir)
|
||||
}
|
||||
|
||||
func legacyGlobalSettingsPath(homeDir string) string {
|
||||
dir := legacyReasonixHome(homeDir)
|
||||
if dir == "" {
|
||||
|
||||
@@ -1024,6 +1024,23 @@ func TestReasonixHomeOverridesGlobalHookPaths(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadOptionsReasonixHomeDirUsesExactGlobalHookPath(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
reasonixHome := filepath.Join(home, "AppData", "Roaming", "reasonix")
|
||||
settingsPath := filepath.Join(reasonixHome, SettingsFilename)
|
||||
if err := os.MkdirAll(reasonixHome, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(settingsPath, []byte(`{"hooks":{"Stop":[{"command":"echo exact"}]}}`), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
hooks := Load(LoadOptions{HomeDir: home, ReasonixHomeDir: reasonixHome})
|
||||
if len(hooks) != 1 || hooks[0].Command != "echo exact" || hooks[0].Source != settingsPath {
|
||||
t.Fatalf("Load hooks = %+v, want exact Reasonix home hook", hooks)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReasonixHomeDoesNotFallBackToLegacyWhenIsolated(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
reasonixHome := filepath.Join(t.TempDir(), "rx-home")
|
||||
|
||||
@@ -67,9 +67,13 @@ func Inspect(opts LoadOptions) Inspection {
|
||||
}
|
||||
|
||||
// Plugin hooks (enabled packages only — same as Load).
|
||||
appendPluginInspect(&out, reasonixHome(opts.HomeDir), opts.ProjectRoot)
|
||||
reasonixHomeDir := reasonixHomeForOptions(opts)
|
||||
appendPluginInspect(&out, reasonixHomeDir, opts.ProjectRoot)
|
||||
|
||||
g := GlobalSettingsPath(opts.HomeDir)
|
||||
g := filepath.Join(reasonixHomeDir, SettingsFilename)
|
||||
if reasonixHomeDir == "" {
|
||||
g = GlobalSettingsPath(opts.HomeDir)
|
||||
}
|
||||
st := inspectSettingsFile(g, ScopeGlobal)
|
||||
if st.Status == "missing" {
|
||||
if legacy := legacyGlobalSettingsPath(opts.HomeDir); legacy != "" {
|
||||
|
||||
Reference in new issue
Block a user