Commit eb0002c
Eric Bower
·
2026-08-15 13:31:36 -0400 EDT
parent f7542d3
fix: debug bridge
4 files changed,
+110,
-32
+13,
-2
| ... | ... | @@ -10,6 +10,10 @@ inputs: | |
| 10 | 10 | description: "SSH private key registered with pico.sh for authenticated pipe.pico.sh debug relay access" | |
| 11 | 11 | required: false | |
| 12 | 12 | default: "" | |
| 13 | + | pico_ssh_cert: | |
| 14 | + | description: "SSH public certificate key for authenticating with pico.sh debug relay" | |
| 15 | + | required: false | |
| 16 | + | default: "" | |
| 13 | 17 | debug_on_fail: | |
| 14 | 18 | description: "Automatically open an interactive pipe.pico.sh debug bridge if a job fails (true, false, auto)" | |
| 15 | 19 | required: false |
| ... | ... | @@ -34,7 +38,7 @@ inputs: | |
| 34 | 38 | runs: | |
| 35 | 39 | using: "composite" | |
| 36 | 40 | steps: | |
| 37 | - | - name: Set up SSH key for pico.sh debug relay | |
| 41 | + | - name: Set up SSH key and certificate for pico.sh debug relay | |
| 38 | 42 | if: inputs.pico_ssh_key != '' | |
| 39 | 43 | shell: bash | |
| 40 | 44 | run: | |
| ... | ... | @@ -42,6 +46,10 @@ runs: | |
| 42 | 46 | chmod 700 ~/.ssh | |
| 43 | 47 | echo "${{ inputs.pico_ssh_key }}" > ~/.ssh/id_pici_debug | |
| 44 | 48 | chmod 600 ~/.ssh/id_pici_debug | |
| 49 | + | if [ -n "${{ inputs.pico_ssh_cert }}" ]; then | |
| 50 | + | echo "${{ inputs.pico_ssh_cert }}" > ~/.ssh/id_pici_debug-cert.pub | |
| 51 | + | chmod 644 ~/.ssh/id_pici_debug-cert.pub | |
| 52 | + | fi | |
| 45 | 53 | ||
| 46 | 54 | - name: Install zmx | |
| 47 | 55 | shell: bash |
| ... | ... | @@ -82,10 +90,13 @@ runs: | |
| 82 | 90 | FLAGS="$FLAGS --summary-file $GITHUB_STEP_SUMMARY" | |
| 83 | 91 | fi | |
| 84 | 92 | ||
| 85 | - | # Pass SSH key if configured | |
| 93 | + | # Pass SSH key & certificate if configured | |
| 86 | 94 | if [ -f "$HOME/.ssh/id_pici_debug" ]; then | |
| 87 | 95 | FLAGS="$FLAGS -pk $HOME/.ssh/id_pici_debug" | |
| 88 | 96 | fi | |
| 97 | + | if [ -f "$HOME/.ssh/id_pici_debug-cert.pub" ]; then | |
| 98 | + | FLAGS="$FLAGS -ck $HOME/.ssh/id_pici_debug-cert.pub" | |
| 99 | + | fi | |
| 89 | 100 | ||
| 90 | 101 | echo "==> Executing: pici run $FLAGS" | |
| 91 | 102 | pici run $FLAGS \ |
+64,
-24
| ... | ... | @@ -8,8 +8,10 @@ import ( | |
| 8 | 8 | "net" | |
| 9 | 9 | "os" | |
| 10 | 10 | "os/exec" | |
| 11 | + | "os/signal" | |
| 11 | 12 | "path/filepath" | |
| 12 | 13 | "strings" | |
| 14 | + | "syscall" | |
| 13 | 15 | "time" | |
| 14 | 16 | ||
| 15 | 17 | "github.com/creack/pty" |
| ... | ... | @@ -75,13 +77,34 @@ func scrubEnvironment(environ []string) []string { | |
| 75 | 77 | return cleaned | |
| 76 | 78 | } | |
| 77 | 79 | ||
| 80 | + | // loadCertSigner parses the certificate file at certPath and wraps the key signer. | |
| 81 | + | func loadCertSigner(signer ssh.Signer, certPath string) (ssh.Signer, error) { | |
| 82 | + | certBytes, err := os.ReadFile(certPath) | |
| 83 | + | if err != nil { | |
| 84 | + | return nil, fmt.Errorf("read cert file (%s): %w", certPath, err) | |
| 85 | + | } | |
| 86 | + | pubKey, _, _, _, err := ssh.ParseAuthorizedKey(certBytes) | |
| 87 | + | if err != nil { | |
| 88 | + | return nil, fmt.Errorf("parse cert (%s): %w", certPath, err) | |
| 89 | + | } | |
| 90 | + | cert, ok := pubKey.(*ssh.Certificate) | |
| 91 | + | if !ok { | |
| 92 | + | return nil, fmt.Errorf("public key at %s is not an SSH certificate", certPath) | |
| 93 | + | } | |
| 94 | + | certSigner, err := ssh.NewCertSigner(cert, signer) | |
| 95 | + | if err != nil { | |
| 96 | + | return nil, fmt.Errorf("create cert signer: %w", err) | |
| 97 | + | } | |
| 98 | + | return certSigner, nil | |
| 99 | + | } | |
| 100 | + | ||
| 78 | 101 | // findSSHAuthMethod discovers an available SSH authentication method from: | |
| 79 | - | // 1. Specified keyLocation (and optional certificateLocation) | |
| 80 | - | // 2. PICO_SSH_KEY environment variable (file path or raw key content) | |
| 102 | + | // 1. Explicit keyLocation (and optional explicit certificateLocation) | |
| 103 | + | // 2. Explicit PICO_SSH_KEY environment variable (file path or raw key content) | |
| 81 | 104 | // 3. Active SSH agent ($SSH_AUTH_SOCK) | |
| 82 | 105 | // 4. Default user SSH keys (~/.ssh/id_ed25519, ~/.ssh/id_rsa) | |
| 83 | 106 | func findSSHAuthMethod(keyLocation string, certificateLocation string) (ssh.AuthMethod, error) { | |
| 84 | - | // 1. Specified key path | |
| 107 | + | // 1. Specified key path (-pk) | |
| 85 | 108 | if keyLocation != "" { | |
| 86 | 109 | keyBytes, err := os.ReadFile(keyLocation) | |
| 87 | 110 | if err != nil { |
| ... | ... | @@ -92,21 +115,9 @@ func findSSHAuthMethod(keyLocation string, certificateLocation string) (ssh.Auth | |
| 92 | 115 | return nil, fmt.Errorf("parse private key: %w", err) | |
| 93 | 116 | } | |
| 94 | 117 | if certificateLocation != "" { | |
| 95 | - | certBytes, err := os.ReadFile(certificateLocation) | |
| 96 | - | if err != nil { | |
| 97 | - | return nil, fmt.Errorf("read cert file: %w", err) | |
| 98 | - | } | |
| 99 | - | pubKey, _, _, _, err := ssh.ParseAuthorizedKey(certBytes) | |
| 118 | + | certSigner, err := loadCertSigner(signer, certificateLocation) | |
| 100 | 119 | if err != nil { | |
| 101 | - | return nil, fmt.Errorf("parse cert: %w", err) | |
| 102 | - | } | |
| 103 | - | cert, ok := pubKey.(*ssh.Certificate) | |
| 104 | - | if !ok { | |
| 105 | - | return nil, fmt.Errorf("public key is not a certificate") | |
| 106 | - | } | |
| 107 | - | certSigner, err := ssh.NewCertSigner(cert, signer) | |
| 108 | - | if err != nil { | |
| 109 | - | return nil, fmt.Errorf("new cert signer: %w", err) | |
| 120 | + | return nil, err | |
| 110 | 121 | } | |
| 111 | 122 | return ssh.PublicKeys(certSigner), nil | |
| 112 | 123 | } |
| ... | ... | @@ -123,6 +134,13 @@ func findSSHAuthMethod(keyLocation string, certificateLocation string) (ssh.Auth | |
| 123 | 134 | } | |
| 124 | 135 | signer, err := ssh.ParsePrivateKey(keyBytes) | |
| 125 | 136 | if err == nil { | |
| 137 | + | if certificateLocation != "" { | |
| 138 | + | certSigner, err := loadCertSigner(signer, certificateLocation) | |
| 139 | + | if err != nil { | |
| 140 | + | return nil, err | |
| 141 | + | } | |
| 142 | + | return ssh.PublicKeys(certSigner), nil | |
| 143 | + | } | |
| 126 | 144 | return ssh.PublicKeys(signer), nil | |
| 127 | 145 | } | |
| 128 | 146 | } |
| ... | ... | @@ -135,12 +153,19 @@ func findSSHAuthMethod(keyLocation string, certificateLocation string) (ssh.Auth | |
| 135 | 153 | } | |
| 136 | 154 | } | |
| 137 | 155 | ||
| 138 | - | // 4. Standard ~/.ssh/ files | |
| 156 | + | // 4. Standard ~/.ssh/ files (default fallback when no key is explicitly passed) | |
| 139 | 157 | if homeDir, err := os.UserHomeDir(); err == nil { | |
| 140 | 158 | for _, keyName := range []string{"id_ed25519", "id_rsa"} { | |
| 141 | - | p := filepath.Join(homeDir, ".ssh", keyName) | |
| 142 | - | if keyBytes, err := os.ReadFile(p); err == nil { | |
| 159 | + | keyPath := filepath.Join(homeDir, ".ssh", keyName) | |
| 160 | + | if keyBytes, err := os.ReadFile(keyPath); err == nil { | |
| 143 | 161 | if signer, err := ssh.ParsePrivateKey(keyBytes); err == nil { | |
| 162 | + | if certificateLocation != "" { | |
| 163 | + | certSigner, err := loadCertSigner(signer, certificateLocation) | |
| 164 | + | if err != nil { | |
| 165 | + | return nil, err | |
| 166 | + | } | |
| 167 | + | return ssh.PublicKeys(certSigner), nil | |
| 168 | + | } | |
| 144 | 169 | return ssh.PublicKeys(signer), nil | |
| 145 | 170 | } | |
| 146 | 171 | } |
| ... | ... | @@ -182,21 +207,30 @@ func startDebugBridge(ctx context.Context, cfg DebugBridgeConfig) error { | |
| 182 | 207 | User: "pici", | |
| 183 | 208 | Auth: []ssh.AuthMethod{auth}, | |
| 184 | 209 | HostKeyCallback: ssh.InsecureIgnoreHostKey(), | |
| 185 | - | Timeout: 10 * time.Second, | |
| 210 | + | Timeout: 5 * time.Second, | |
| 186 | 211 | } | |
| 187 | 212 | ||
| 188 | 213 | fmt.Fprintf(cfg.Stdout, "š connecting debug bridge to %s (topic: %s)...\n", cfg.Host, cfg.Topic) //nolint:errcheck | |
| 189 | - | client, err := ssh.Dial("tcp", cfg.Host, sshConfig) | |
| 214 | + | ||
| 215 | + | dialer := net.Dialer{Timeout: 5 * time.Second} | |
| 216 | + | conn, err := dialer.DialContext(ctx, "tcp", cfg.Host) | |
| 190 | 217 | if err != nil { | |
| 191 | 218 | return fmt.Errorf("dial ssh (%s): %w", cfg.Host, err) | |
| 192 | 219 | } | |
| 193 | - | defer client.Close() | |
| 220 | + | defer func() { _ = conn.Close() }() | |
| 221 | + | ||
| 222 | + | sshConn, chans, reqs, err := ssh.NewClientConn(conn, cfg.Host, sshConfig) | |
| 223 | + | if err != nil { | |
| 224 | + | return fmt.Errorf("ssh handshake (%s): %w", cfg.Host, err) | |
| 225 | + | } | |
| 226 | + | client := ssh.NewClient(sshConn, chans, reqs) | |
| 227 | + | defer func() { _ = client.Close() }() | |
| 194 | 228 | ||
| 195 | 229 | session, err := client.NewSession() | |
| 196 | 230 | if err != nil { | |
| 197 | 231 | return fmt.Errorf("ssh session: %w", err) | |
| 198 | 232 | } | |
| 199 | - | defer session.Close() | |
| 233 | + | defer func() { _ = session.Close() }() | |
| 200 | 234 | ||
| 201 | 235 | sshStdin, err := session.StdinPipe() | |
| 202 | 236 | if err != nil { |
| ... | ... | @@ -243,6 +277,10 @@ func startDebugBridge(ctx context.Context, cfg DebugBridgeConfig) error { | |
| 243 | 277 | ctxWithTimeout, cancel := context.WithTimeout(ctx, cfg.Timeout) | |
| 244 | 278 | defer cancel() | |
| 245 | 279 | ||
| 280 | + | sigCh := make(chan os.Signal, 1) | |
| 281 | + | signal.Notify(sigCh, os.Interrupt, syscall.SIGTERM) | |
| 282 | + | defer signal.Stop(sigCh) | |
| 283 | + | ||
| 246 | 284 | errCh := make(chan error, 3) | |
| 247 | 285 | ||
| 248 | 286 | go func() { |
| ... | ... | @@ -262,6 +300,8 @@ func startDebugBridge(ctx context.Context, cfg DebugBridgeConfig) error { | |
| 262 | 300 | select { | |
| 263 | 301 | case <-ctxWithTimeout.Done(): | |
| 264 | 302 | fmt.Fprintf(cfg.Stdout, "\nā° debug session timed out after %s\n", cfg.Timeout) //nolint:errcheck | |
| 303 | + | case <-sigCh: | |
| 304 | + | fmt.Fprintf(cfg.Stdout, "\nā¹ debug session aborted by signal\n") //nolint:errcheck | |
| 265 | 305 | case <-errCh: | |
| 266 | 306 | fmt.Fprintf(cfg.Stdout, "\nš debug session ended\n") //nolint:errcheck | |
| 267 | 307 | } |
M
main.go
+6,
-6
| ... | ... | @@ -1259,13 +1259,13 @@ func writeSummaryMarkdown(filePath string, repoName, jobID string, jobSessions [ | |
| 1259 | 1259 | statusIcon := map[string]string{"success": "ā ", "failed": "ā"}[status] | |
| 1260 | 1260 | ||
| 1261 | 1261 | var sb strings.Builder | |
| 1262 | - | sb.WriteString(fmt.Sprintf("### Pici CI Summary: `%s` (Job `%s`)\n\n", repoName, jobID)) | |
| 1263 | - | sb.WriteString(fmt.Sprintf("**Status:** %s %s | **Exit Code:** `%d` | **Duration:** `%s`\n\n", statusIcon, strings.ToUpper(status), exitCode, duration)) | |
| 1262 | + | fmt.Fprintf(&sb, "### Pici CI Summary: `%s` (Job `%s`)\n\n", repoName, jobID) | |
| 1263 | + | fmt.Fprintf(&sb, "**Status:** %s %s | **Exit Code:** `%d` | **Duration:** `%s`\n\n", statusIcon, strings.ToUpper(status), exitCode, duration) | |
| 1264 | 1264 | ||
| 1265 | 1265 | if debugActive && debugTopic != "" { | |
| 1266 | 1266 | sb.WriteString("### š§ Remote Debug Active\n\n") | |
| 1267 | 1267 | sb.WriteString("To attach to the interactive shell session for this job:\n\n") | |
| 1268 | - | sb.WriteString(fmt.Sprintf("```bash\nssh -t pipe.pico.sh pipe %s\n```\n\n", debugTopic)) | |
| 1268 | + | fmt.Fprintf(&sb, "```bash\nssh -t pipe.pico.sh pipe %s\n```\n\n", debugTopic) | |
| 1269 | 1269 | } | |
| 1270 | 1270 | ||
| 1271 | 1271 | sb.WriteString("| Session | Status | Exit Code | Duration |\n") |
| ... | ... | @@ -1289,7 +1289,7 @@ func writeSummaryMarkdown(filePath string, repoName, jobID string, jobSessions [ | |
| 1289 | 1289 | } | |
| 1290 | 1290 | sIcon = map[string]string{"running": "š", "success": "ā ", "failed": "ā"}[sStatus] | |
| 1291 | 1291 | } | |
| 1292 | - | sb.WriteString(fmt.Sprintf("| `%s` | %s %s | `%s` | %s |\n", s.Short, sIcon, sStatus, sExit, sDur)) | |
| 1292 | + | fmt.Fprintf(&sb, "| `%s` | %s %s | `%s` | %s |\n", s.Short, sIcon, sStatus, sExit, sDur) | |
| 1293 | 1293 | } | |
| 1294 | 1294 | sb.WriteString("\n") | |
| 1295 | 1295 |
| ... | ... | @@ -1308,14 +1308,14 @@ func writeSummaryMarkdown(filePath string, repoName, jobID string, jobSessions [ | |
| 1308 | 1308 | if err != nil { | |
| 1309 | 1309 | history = fmt.Sprintf("(history unavailable: %v)", err) | |
| 1310 | 1310 | } | |
| 1311 | - | sb.WriteString(fmt.Sprintf("<details><summary><b>%s</b> (exit %s)</summary>\n\n```\n%s\n```\n\n</details>\n\n", s.Short, state.exitCode, strings.TrimSpace(history))) | |
| 1311 | + | fmt.Fprintf(&sb, "<details><summary><b>%s</b> (exit %s)</summary>\n\n```\n%s\n```\n\n</details>\n\n", s.Short, state.exitCode, strings.TrimSpace(history)) | |
| 1312 | 1312 | } | |
| 1313 | 1313 | ||
| 1314 | 1314 | f, err := os.OpenFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) | |
| 1315 | 1315 | if err != nil { | |
| 1316 | 1316 | return fmt.Errorf("open summary file: %w", err) | |
| 1317 | 1317 | } | |
| 1318 | - | defer f.Close() | |
| 1318 | + | defer func() { _ = f.Close() }() | |
| 1319 | 1319 | ||
| 1320 | 1320 | if _, err := f.WriteString(sb.String()); err != nil { | |
| 1321 | 1321 | return fmt.Errorf("write summary file: %w", err) |
+27,
-0
| ... | ... | @@ -1469,3 +1469,30 @@ func TestScrubEnvironment(t *testing.T) { | |
| 1469 | 1469 | } | |
| 1470 | 1470 | } | |
| 1471 | 1471 | } | |
| 1472 | + | ||
| 1473 | + | func TestFindSSHAuthMethod_ExplicitKeyAndCert(t *testing.T) { | |
| 1474 | + | tempDir := t.TempDir() | |
| 1475 | + | keyPath := filepath.Join(tempDir, "id_ed25519") | |
| 1476 | + | ||
| 1477 | + | // Generate a temporary ED25519 key | |
| 1478 | + | cmd := exec.Command("ssh-keygen", "-t", "ed25519", "-N", "", "-f", keyPath) | |
| 1479 | + | if err := cmd.Run(); err != nil { | |
| 1480 | + | t.Skipf("ssh-keygen not available or failed: %v", err) | |
| 1481 | + | } | |
| 1482 | + | ||
| 1483 | + | // Test 1: Explicit key path | |
| 1484 | + | auth, err := findSSHAuthMethod(keyPath, "") | |
| 1485 | + | if err != nil { | |
| 1486 | + | t.Fatalf("findSSHAuthMethod failed: %v", err) | |
| 1487 | + | } | |
| 1488 | + | if auth == nil { | |
| 1489 | + | t.Fatal("expected non-nil auth method") | |
| 1490 | + | } | |
| 1491 | + | ||
| 1492 | + | // Test 2: Non-existent cert path explicitly specified returns error | |
| 1493 | + | _, err = findSSHAuthMethod(keyPath, filepath.Join(tempDir, "nonexistent-cert.pub")) | |
| 1494 | + | if err == nil { | |
| 1495 | + | t.Fatal("expected error when non-existent certificate is explicitly specified") | |
| 1496 | + | } | |
| 1497 | + | } | |
| 1498 | + |