fix(snapshot): address process capture review findings
- commit
- 2e3ea8bdb7d733baf6e0b82b070a2e807a3517a3
- author
- brice · · 2026-07-06 07:58 UTC
- parent
- 755f4f5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +79 -14
M internal/usecase/daemon/process_restore.go +26 -7
| @@ -69,15 +69,11 @@ func agentSessionFlag(strategy, arg string) bool { | |||
| 69 | 69 | } | |
| 70 | 70 | ||
| 71 | 71 | func planProcessRestore(proc *snapcodec.Process, allow map[string]struct{}) processRestoreDecision { | |
| 72 | - | if proc == nil || len(proc.Argv) == 0 { | |
| 72 | + | if proc == nil || len(proc.Argv) == 0 || proc.Argv[0] == "" { | |
| 73 | 73 | return processRestoreDecision{Reason: "missing_process"} | |
| 74 | 74 | } | |
| 75 | - | strategy := proc.Strategy | |
| 76 | - | if strategy == "" { | |
| 77 | - | strategy = detectProcessStrategy(proc.Argv) | |
| 78 | - | } | |
| 79 | - | name := filepath.Base(proc.Argv[0]) | |
| 80 | - | if _, ok := allow[name]; !ok { | |
| 75 | + | strategy := normalizeProcessStrategy(proc.Strategy, proc.Argv) | |
| 76 | + | if _, ok := allow[processAllowlistKey(strategy, proc.Argv)]; !ok { | |
| 81 | 77 | return processRestoreDecision{Reason: "not_allowlisted"} | |
| 82 | 78 | } | |
| 83 | 79 | ||
| @@ -92,14 +88,17 @@ func planProcessRestore(proc *snapcodec.Process, allow map[string]struct{}) proc | |||
| 92 | 88 | if id != "" { | |
| 93 | 89 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"claude", "--resume", id}), Restore: true} | |
| 94 | 90 | } | |
| 91 | + | return processRestoreDecision{Command: "claude --continue", Restore: true} | |
| 95 | 92 | case processStrategyOpenCode: | |
| 96 | 93 | if id != "" { | |
| 97 | 94 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"opencode", "--session", id}), Restore: true} | |
| 98 | 95 | } | |
| 96 | + | return processRestoreDecision{Command: "opencode --continue", Restore: true} | |
| 99 | 97 | case processStrategyCodex: | |
| 100 | 98 | if id != "" { | |
| 101 | 99 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"codex", "resume", id}), Restore: true} | |
| 102 | 100 | } | |
| 101 | + | return processRestoreDecision{Command: "codex resume --last", Restore: true} | |
| 103 | 102 | } | |
| 104 | 103 | ||
| 105 | 104 | command, ok := shellQuoteArgv(proc.Argv) | |
| @@ -109,6 +108,26 @@ func planProcessRestore(proc *snapcodec.Process, allow map[string]struct{}) proc | |||
| 109 | 108 | return processRestoreDecision{Command: command, Restore: true} | |
| 110 | 109 | } | |
| 111 | 110 | ||
| 111 | + | func normalizeProcessStrategy(strategy string, argv []string) string { | |
| 112 | + | switch strategy { | |
| 113 | + | case processStrategyClaude, processStrategyCodex, processStrategyOpenCode, processStrategyPi: | |
| 114 | + | return strategy | |
| 115 | + | case "", processStrategyGeneric: | |
| 116 | + | return detectProcessStrategy(argv) | |
| 117 | + | default: | |
| 118 | + | return processStrategyGeneric | |
| 119 | + | } | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | func processAllowlistKey(strategy string, argv []string) string { | |
| 123 | + | switch strategy { | |
| 124 | + | case processStrategyClaude, processStrategyCodex, processStrategyOpenCode, processStrategyPi: | |
| 125 | + | return strategy | |
| 126 | + | default: | |
| 127 | + | return filepath.Base(argv[0]) | |
| 128 | + | } | |
| 129 | + | } | |
| 130 | + | ||
| 112 | 131 | func shellQuoteArgvMust(argv []string) string { | |
| 113 | 132 | command, _ := shellQuoteArgv(argv) | |
| 114 | 133 | return command | |
| @@ -69,15 +69,11 @@ func agentSessionFlag(strategy, arg string) bool { | |||||
| 69 | } | 69 | } | ||
| 70 | 70 | ||||
| 71 | func planProcessRestore(proc *snapcodec.Process, allow map[string]struct{}) processRestoreDecision { | 71 | func planProcessRestore(proc *snapcodec.Process, allow map[string]struct{}) processRestoreDecision { | ||
| 72 | - | if proc == nil || len(proc.Argv) == 0 { | 72 | + | if proc == nil || len(proc.Argv) == 0 || proc.Argv[0] == "" { |
| 73 | return processRestoreDecision{Reason: "missing_process"} | 73 | return processRestoreDecision{Reason: "missing_process"} | ||
| 74 | } | 74 | } | ||
| 75 | - | strategy := proc.Strategy | 75 | + | strategy := normalizeProcessStrategy(proc.Strategy, proc.Argv) |
| 76 | - | if strategy == "" { | 76 | + | if _, ok := allow[processAllowlistKey(strategy, proc.Argv)]; !ok { |
| 77 | - | strategy = detectProcessStrategy(proc.Argv) | |||
| 78 | - | } | |||
| 79 | - | name := filepath.Base(proc.Argv[0]) | |||
| 80 | - | if _, ok := allow[name]; !ok { | |||
| 81 | return processRestoreDecision{Reason: "not_allowlisted"} | 77 | return processRestoreDecision{Reason: "not_allowlisted"} | ||
| 82 | } | 78 | } | ||
| 83 | 79 | ||||
| @@ -92,14 +88,17 @@ func planProcessRestore(proc *snapcodec.Process, allow map[string]struct{}) proc | |||||
| 92 | if id != "" { | 88 | if id != "" { | ||
| 93 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"claude", "--resume", id}), Restore: true} | 89 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"claude", "--resume", id}), Restore: true} | ||
| 94 | } | 90 | } | ||
| 91 | + | return processRestoreDecision{Command: "claude --continue", Restore: true} | |||
| 95 | case processStrategyOpenCode: | 92 | case processStrategyOpenCode: | ||
| 96 | if id != "" { | 93 | if id != "" { | ||
| 97 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"opencode", "--session", id}), Restore: true} | 94 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"opencode", "--session", id}), Restore: true} | ||
| 98 | } | 95 | } | ||
| 96 | + | return processRestoreDecision{Command: "opencode --continue", Restore: true} | |||
| 99 | case processStrategyCodex: | 97 | case processStrategyCodex: | ||
| 100 | if id != "" { | 98 | if id != "" { | ||
| 101 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"codex", "resume", id}), Restore: true} | 99 | return processRestoreDecision{Command: shellQuoteArgvMust([]string{"codex", "resume", id}), Restore: true} | ||
| 102 | } | 100 | } | ||
| 101 | + | return processRestoreDecision{Command: "codex resume --last", Restore: true} | |||
| 103 | } | 102 | } | ||
| 104 | 103 | ||||
| 105 | command, ok := shellQuoteArgv(proc.Argv) | 104 | command, ok := shellQuoteArgv(proc.Argv) | ||
| @@ -109,6 +108,26 @@ func planProcessRestore(proc *snapcodec.Process, allow map[string]struct{}) proc | |||||
| 109 | return processRestoreDecision{Command: command, Restore: true} | 108 | return processRestoreDecision{Command: command, Restore: true} | ||
| 110 | } | 109 | } | ||
| 111 | 110 | ||||
| 111 | + | func normalizeProcessStrategy(strategy string, argv []string) string { | |||
| 112 | + | switch strategy { | |||
| 113 | + | case processStrategyClaude, processStrategyCodex, processStrategyOpenCode, processStrategyPi: | |||
| 114 | + | return strategy | |||
| 115 | + | case "", processStrategyGeneric: | |||
| 116 | + | return detectProcessStrategy(argv) | |||
| 117 | + | default: | |||
| 118 | + | return processStrategyGeneric | |||
| 119 | + | } | |||
| 120 | + | } | |||
| 121 | + | ||||
| 122 | + | func processAllowlistKey(strategy string, argv []string) string { | |||
| 123 | + | switch strategy { | |||
| 124 | + | case processStrategyClaude, processStrategyCodex, processStrategyOpenCode, processStrategyPi: | |||
| 125 | + | return strategy | |||
| 126 | + | default: | |||
| 127 | + | return filepath.Base(argv[0]) | |||
| 128 | + | } | |||
| 129 | + | } | |||
| 130 | + | ||||
| 112 | func shellQuoteArgvMust(argv []string) string { | 131 | func shellQuoteArgvMust(argv []string) string { | ||
| 113 | command, _ := shellQuoteArgv(argv) | 132 | command, _ := shellQuoteArgv(argv) | ||
| 114 | return command | 133 | return command | ||
M internal/usecase/daemon/process_restore_test.go +53 -7
| @@ -7,7 +7,7 @@ import ( | |||
| 7 | 7 | ) | |
| 8 | 8 | ||
| 9 | 9 | func TestProcessRestorePlan(t *testing.T) { | |
| 10 | - | allow := map[string]struct{}{ | |
| 10 | + | defaultAllow := map[string]struct{}{ | |
| 11 | 11 | "less": {}, | |
| 12 | 12 | "pi": {}, | |
| 13 | 13 | "claude": {}, | |
| @@ -17,6 +17,7 @@ func TestProcessRestorePlan(t *testing.T) { | |||
| 17 | 17 | tests := []struct { | |
| 18 | 18 | name string | |
| 19 | 19 | proc *snapcodec.Process | |
| 20 | + | allow map[string]struct{} | |
| 20 | 21 | restore bool | |
| 21 | 22 | command string | |
| 22 | 23 | reason string | |
| @@ -44,22 +45,55 @@ func TestProcessRestorePlan(t *testing.T) { | |||
| 44 | 45 | restore: true, | |
| 45 | 46 | command: "pi --continue", | |
| 46 | 47 | }, | |
| 48 | + | { | |
| 49 | + | name: "claude no ID continues", | |
| 50 | + | proc: &snapcodec.Process{Argv: []string{"claude"}, Strategy: processStrategyClaude}, | |
| 51 | + | restore: true, | |
| 52 | + | command: "claude --continue", | |
| 53 | + | }, | |
| 47 | 54 | { | |
| 48 | 55 | name: "opencode session command", | |
| 49 | 56 | proc: &snapcodec.Process{Argv: []string{"opencode"}, Strategy: processStrategyOpenCode, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc"}}, | |
| 50 | 57 | restore: true, | |
| 51 | 58 | command: "opencode --session abc", | |
| 52 | 59 | }, | |
| 60 | + | { | |
| 61 | + | name: "opencode no ID continues", | |
| 62 | + | proc: &snapcodec.Process{Argv: []string{"opencode"}, Strategy: processStrategyOpenCode}, | |
| 63 | + | restore: true, | |
| 64 | + | command: "opencode --continue", | |
| 65 | + | }, | |
| 53 | 66 | { | |
| 54 | 67 | name: "codex resume command", | |
| 55 | 68 | proc: &snapcodec.Process{Argv: []string{"codex"}, Strategy: processStrategyCodex, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc"}}, | |
| 56 | 69 | restore: true, | |
| 57 | 70 | command: "codex resume abc", | |
| 58 | 71 | }, | |
| 72 | + | { | |
| 73 | + | name: "codex no ID resumes last", | |
| 74 | + | proc: &snapcodec.Process{Argv: []string{"codex"}, Strategy: processStrategyCodex}, | |
| 75 | + | restore: true, | |
| 76 | + | command: "codex resume --last", | |
| 77 | + | }, | |
| 78 | + | { | |
| 79 | + | name: "agent strategy matches allowlist by strategy not argv basename", | |
| 80 | + | proc: &snapcodec.Process{Argv: []string{"less"}, Strategy: processStrategyClaude, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc"}}, | |
| 81 | + | allow: map[string]struct{}{"less": {}}, | |
| 82 | + | reason: "not_allowlisted", | |
| 83 | + | }, | |
| 84 | + | { | |
| 85 | + | name: "empty executable rejected", | |
| 86 | + | proc: &snapcodec.Process{Argv: []string{""}, Strategy: processStrategyGeneric}, | |
| 87 | + | reason: "missing_process", | |
| 88 | + | }, | |
| 59 | 89 | } | |
| 60 | 90 | ||
| 61 | 91 | for _, tt := range tests { | |
| 62 | 92 | t.Run(tt.name, func(t *testing.T) { | |
| 93 | + | allow := tt.allow | |
| 94 | + | if allow == nil { | |
| 95 | + | allow = defaultAllow | |
| 96 | + | } | |
| 63 | 97 | got := planProcessRestore(tt.proc, allow) | |
| 64 | 98 | if got.Restore != tt.restore || got.Command != tt.command || got.Reason != tt.reason { | |
| 65 | 99 | t.Fatalf("planProcessRestore() = %+v, want restore=%v command=%q reason=%q", got, tt.restore, tt.command, tt.reason) | |
| @@ -92,12 +126,24 @@ func TestExtractAgentSessionID(t *testing.T) { | |||
| 92 | 126 | } | |
| 93 | 127 | ||
| 94 | 128 | func TestShellQuoteArgv(t *testing.T) { | |
| 95 | - | got, ok := shellQuoteArgv([]string{"cmd", "two words", "it's"}) | |
| 96 | - | if !ok { | |
| 97 | - | t.Fatal("shellQuoteArgv() returned !ok") | |
| 129 | + | tests := []struct { | |
| 130 | + | name string | |
| 131 | + | argv []string | |
| 132 | + | want string | |
| 133 | + | ok bool | |
| 134 | + | }{ | |
| 135 | + | {name: "empty argv", ok: false}, | |
| 136 | + | {name: "spaces and single quote", argv: []string{"cmd", "two words", "it's"}, want: "cmd 'two words' 'it'\\''s'", ok: true}, | |
| 137 | + | {name: "shell metacharacters", argv: []string{"cmd", "$(touch /tmp/pwned)", "a;b", "line\nbreak"}, want: "cmd '$(touch /tmp/pwned)' 'a;b' 'line\nbreak'", ok: true}, | |
| 138 | + | {name: "empty argument", argv: []string{"cmd", ""}, want: "cmd ''", ok: true}, | |
| 98 | 139 | } | |
| 99 | - | want := "cmd 'two words' 'it'\\''s'" | |
| 100 | - | if got != want { | |
| 101 | - | t.Fatalf("shellQuoteArgv() = %q, want %q", got, want) | |
| 140 | + | ||
| 141 | + | for _, tt := range tests { | |
| 142 | + | t.Run(tt.name, func(t *testing.T) { | |
| 143 | + | got, ok := shellQuoteArgv(tt.argv) | |
| 144 | + | if ok != tt.ok || got != tt.want { | |
| 145 | + | t.Fatalf("shellQuoteArgv() = %q, %v; want %q, %v", got, ok, tt.want, tt.ok) | |
| 146 | + | } | |
| 147 | + | }) | |
| 102 | 148 | } | |
| 103 | 149 | } | |
| @@ -7,7 +7,7 @@ import ( | |||||
| 7 | ) | 7 | ) | ||
| 8 | 8 | ||||
| 9 | func TestProcessRestorePlan(t *testing.T) { | 9 | func TestProcessRestorePlan(t *testing.T) { | ||
| 10 | - | allow := map[string]struct{}{ | 10 | + | defaultAllow := map[string]struct{}{ |
| 11 | "less": {}, | 11 | "less": {}, | ||
| 12 | "pi": {}, | 12 | "pi": {}, | ||
| 13 | "claude": {}, | 13 | "claude": {}, | ||
| @@ -17,6 +17,7 @@ func TestProcessRestorePlan(t *testing.T) { | |||||
| 17 | tests := []struct { | 17 | tests := []struct { | ||
| 18 | name string | 18 | name string | ||
| 19 | proc *snapcodec.Process | 19 | proc *snapcodec.Process | ||
| 20 | + | allow map[string]struct{} | |||
| 20 | restore bool | 21 | restore bool | ||
| 21 | command string | 22 | command string | ||
| 22 | reason string | 23 | reason string | ||
| @@ -44,22 +45,55 @@ func TestProcessRestorePlan(t *testing.T) { | |||||
| 44 | restore: true, | 45 | restore: true, | ||
| 45 | command: "pi --continue", | 46 | command: "pi --continue", | ||
| 46 | }, | 47 | }, | ||
| 48 | + | { | |||
| 49 | + | name: "claude no ID continues", | |||
| 50 | + | proc: &snapcodec.Process{Argv: []string{"claude"}, Strategy: processStrategyClaude}, | |||
| 51 | + | restore: true, | |||
| 52 | + | command: "claude --continue", | |||
| 53 | + | }, | |||
| 47 | { | 54 | { | ||
| 48 | name: "opencode session command", | 55 | name: "opencode session command", | ||
| 49 | proc: &snapcodec.Process{Argv: []string{"opencode"}, Strategy: processStrategyOpenCode, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc"}}, | 56 | proc: &snapcodec.Process{Argv: []string{"opencode"}, Strategy: processStrategyOpenCode, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc"}}, | ||
| 50 | restore: true, | 57 | restore: true, | ||
| 51 | command: "opencode --session abc", | 58 | command: "opencode --session abc", | ||
| 52 | }, | 59 | }, | ||
| 60 | + | { | |||
| 61 | + | name: "opencode no ID continues", | |||
| 62 | + | proc: &snapcodec.Process{Argv: []string{"opencode"}, Strategy: processStrategyOpenCode}, | |||
| 63 | + | restore: true, | |||
| 64 | + | command: "opencode --continue", | |||
| 65 | + | }, | |||
| 53 | { | 66 | { | ||
| 54 | name: "codex resume command", | 67 | name: "codex resume command", | ||
| 55 | proc: &snapcodec.Process{Argv: []string{"codex"}, Strategy: processStrategyCodex, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc"}}, | 68 | proc: &snapcodec.Process{Argv: []string{"codex"}, Strategy: processStrategyCodex, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc"}}, | ||
| 56 | restore: true, | 69 | restore: true, | ||
| 57 | command: "codex resume abc", | 70 | command: "codex resume abc", | ||
| 58 | }, | 71 | }, | ||
| 72 | + | { | |||
| 73 | + | name: "codex no ID resumes last", | |||
| 74 | + | proc: &snapcodec.Process{Argv: []string{"codex"}, Strategy: processStrategyCodex}, | |||
| 75 | + | restore: true, | |||
| 76 | + | command: "codex resume --last", | |||
| 77 | + | }, | |||
| 78 | + | { | |||
| 79 | + | name: "agent strategy matches allowlist by strategy not argv basename", | |||
| 80 | + | proc: &snapcodec.Process{Argv: []string{"less"}, Strategy: processStrategyClaude, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc"}}, | |||
| 81 | + | allow: map[string]struct{}{"less": {}}, | |||
| 82 | + | reason: "not_allowlisted", | |||
| 83 | + | }, | |||
| 84 | + | { | |||
| 85 | + | name: "empty executable rejected", | |||
| 86 | + | proc: &snapcodec.Process{Argv: []string{""}, Strategy: processStrategyGeneric}, | |||
| 87 | + | reason: "missing_process", | |||
| 88 | + | }, | |||
| 59 | } | 89 | } | ||
| 60 | 90 | ||||
| 61 | for _, tt := range tests { | 91 | for _, tt := range tests { | ||
| 62 | t.Run(tt.name, func(t *testing.T) { | 92 | t.Run(tt.name, func(t *testing.T) { | ||
| 93 | + | allow := tt.allow | |||
| 94 | + | if allow == nil { | |||
| 95 | + | allow = defaultAllow | |||
| 96 | + | } | |||
| 63 | got := planProcessRestore(tt.proc, allow) | 97 | got := planProcessRestore(tt.proc, allow) | ||
| 64 | if got.Restore != tt.restore || got.Command != tt.command || got.Reason != tt.reason { | 98 | if got.Restore != tt.restore || got.Command != tt.command || got.Reason != tt.reason { | ||
| 65 | t.Fatalf("planProcessRestore() = %+v, want restore=%v command=%q reason=%q", got, tt.restore, tt.command, tt.reason) | 99 | t.Fatalf("planProcessRestore() = %+v, want restore=%v command=%q reason=%q", got, tt.restore, tt.command, tt.reason) | ||
| @@ -92,12 +126,24 @@ func TestExtractAgentSessionID(t *testing.T) { | |||||
| 92 | } | 126 | } | ||
| 93 | 127 | ||||
| 94 | func TestShellQuoteArgv(t *testing.T) { | 128 | func TestShellQuoteArgv(t *testing.T) { | ||
| 95 | - | got, ok := shellQuoteArgv([]string{"cmd", "two words", "it's"}) | 129 | + | tests := []struct { |
| 96 | - | if !ok { | 130 | + | name string |
| 97 | - | t.Fatal("shellQuoteArgv() returned !ok") | 131 | + | argv []string |
| 132 | + | want string | |||
| 133 | + | ok bool | |||
| 134 | + | }{ | |||
| 135 | + | {name: "empty argv", ok: false}, | |||
| 136 | + | {name: "spaces and single quote", argv: []string{"cmd", "two words", "it's"}, want: "cmd 'two words' 'it'\\''s'", ok: true}, | |||
| 137 | + | {name: "shell metacharacters", argv: []string{"cmd", "$(touch /tmp/pwned)", "a;b", "line\nbreak"}, want: "cmd '$(touch /tmp/pwned)' 'a;b' 'line\nbreak'", ok: true}, | |||
| 138 | + | {name: "empty argument", argv: []string{"cmd", ""}, want: "cmd ''", ok: true}, | |||
| 98 | } | 139 | } | ||
| 99 | - | want := "cmd 'two words' 'it'\\''s'" | 140 | + | |
| 100 | - | if got != want { | 141 | + | for _, tt := range tests { |
| 101 | - | t.Fatalf("shellQuoteArgv() = %q, want %q", got, want) | 142 | + | t.Run(tt.name, func(t *testing.T) { |
| 143 | + | got, ok := shellQuoteArgv(tt.argv) | |||
| 144 | + | if ok != tt.ok || got != tt.want { | |||
| 145 | + | t.Fatalf("shellQuoteArgv() = %q, %v; want %q, %v", got, ok, tt.want, tt.ok) | |||
| 146 | + | } | |||
| 147 | + | }) | |||
| 102 | } | 148 | } | ||
| 103 | } | 149 | } | ||