Merge pull request 'fix(handlers/terminal): surface AWS subprocess stderr in send-ssh-public-key Detail (mc#687)' (#755) from fix/687-send-ssh-public-key-detail into main

workspace-server/internal/handlers/discovery.go

@@ -292,8 +292,8 @@ func filterPeersByQuery(peers []map[string]interface{}, q string) []map[string]i
 	needle := strings.ToLower(q)
 	out := make([]map[string]interface{}, 0, len(peers))
 	for _, p := range peers {
-		name := p["name"].(string)
-		role := p["role"].(string)
+		name, _ := p["name"].(string)  // nil → "" — safe on empty-role rows
+		role, _ := p["role"].(string)  // nil → "" — queryPeerMaps sets nil when DB role is empty
 		if strings.Contains(strings.ToLower(name), needle) ||
 			strings.Contains(strings.ToLower(role), needle) {
 			out = append(out, p)

workspace-server/internal/handlers/discovery_test.go

@@ -394,6 +394,80 @@ func TestPeers_Q_NoMatches_RawBodyIsArrayNotNull(t *testing.T) {
 	}
 }
 
+// TestFilterPeersByQuery_NilRoleRegression is the regression gate for
+// mc#730/#731: queryPeerMaps sets peer["role"] = nil when the DB role column
+// is empty (discovery.go lines 337-341). filterPeersByQuery did a bare
+// type assertion p["role"].(string) which panics on nil. The fix uses the
+// comma-ok form so nil → "". The test passes a map with nil name and nil
+// role and asserts no panic + correct filter behaviour.
+func TestFilterPeersByQuery_NilRoleRegression(t *testing.T) {
+	cases := []struct {
+		name       string
+		peers      []map[string]interface{}
+		q          string
+		wantLen    int
+		wantIDs    []string
+	}{
+		{
+			name: "nil role matches on name",
+			peers: []map[string]interface{}{
+				{"id": "ws-a", "name": nil, "role": nil},
+				{"id": "ws-b", "name": "Alpha Builder", "role": nil},
+				{"id": "ws-c", "name": "Beta Builder", "role": nil},
+			},
+			q:       "alpha",
+			wantLen: 1,
+			wantIDs: []string{"ws-b"},
+		},
+		{
+			name: "nil name matches on nil role (empty string)",
+			peers: []map[string]interface{}{
+				{"id": "ws-x", "name": nil, "role": nil},
+				{"id": "ws-y", "name": "Dev Workspace", "role": nil},
+			},
+			q:       "",
+			wantLen: 2, // empty q is a no-op
+			wantIDs: []string{"ws-x", "ws-y"},
+		},
+		{
+			name: "all nil — no panic, returns input",
+			peers: []map[string]interface{}{
+				{"id": "ws-z", "name": nil, "role": nil},
+			},
+			q:       "anything",
+			wantLen: 0,
+			wantIDs: nil,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := filterPeersByQuery(tc.peers, tc.q)
+			if len(got) != tc.wantLen {
+				t.Fatalf("len: got %d, want %d", len(got), tc.wantLen)
+			}
+			gotIDs := make([]string, len(got))
+			for i, p := range got {
+				gotIDs[i] = p["id"].(string)
+			}
+			if tc.wantIDs != nil {
+				for _, id := range tc.wantIDs {
+					found := false
+					for _, g := range gotIDs {
+						if g == id {
+							found = true
+							break
+						}
+					}
+					if !found {
+						t.Errorf("missing id %q; got IDs: %v", id, gotIDs)
+					}
+				}
+			}
+		})
+	}
+}
+
 func keysOf(m map[string]struct{}) []string {
 	out := make([]string, 0, len(m))
 	for k := range m {

workspace-server/internal/handlers/terminal_diagnose.go

@@ -43,6 +43,27 @@ func (s *syncBuf) String() string {
 	return s.b.String()
 }
 
+// unwrapGoError extracts subprocess stderr from a Go-wrapped error that
+// includes combined output. e.g. from sendSSHPublicKey's
+// fmt.Errorf("send-ssh-public-key: %w (%s)", err, combinedOut), this
+// returns the " (%s)" portion — the actionable subprocess signal like
+// "AccessDeniedException: ... is not authorized to perform:
+// ec2-instance-connect:OpenTunnel". Returns "" when the output is
+// identical to the error string (no stderr captured).
+func unwrapGoError(errMsg string) string {
+	// Extract content between the last '(' and trailing ')'. The
+	// sendSSHPublicKey wrapper uses fmt.Errorf("...: %w (%s)", err, combinedOut)
+	// so the subprocess stderr is always the last parenthesised segment,
+	// e.g. "send-ssh-public-key: exit status 1 (AccessDeniedException: ...)"
+	// — note the closing ')' is at the very end with no trailing space.
+	open := strings.LastIndex(errMsg, "(")
+	if open < 0 {
+		return ""
+	}
+	inner := errMsg[open+1:]
+	return strings.TrimSuffix(inner, ")")
+}
+
 // HandleDiagnose handles GET /workspaces/:id/terminal/diagnose. It runs the
 // same per-step pipeline as HandleConnect (ssh-keygen → EIC send-key → tunnel
 // → ssh) but non-interactively, captures the first failing step and its
@@ -214,12 +235,18 @@ func (h *TerminalHandler) diagnoseRemote(ctx context.Context, workspaceID, insta
 	}
 
 	// Step 2: send-ssh-public-key (AWS Instance Connect)
+	// mc#687: populate Detail so the E2E smoke sees the AWS permission error
+	// verbatim. The subprocess stderr (e.g. "AccessDeniedException: ... is not
+	// authorized to perform: ec2-instance-connect:OpenTunnel") is captured by
+	// sendSSHPublicKey's CombinedOutput() and embedded in the Go error string.
 	t0 = time.Now()
 	if err := sendSSHPublicKey(ctx, region, instanceID, osUser, strings.TrimSpace(string(pubKey))); err != nil {
+		errMsg := err.Error()
 		return stop("send-ssh-public-key", diagnoseStep{
 			Name:       "send-ssh-public-key",
 			DurationMs: time.Since(t0).Milliseconds(),
-			Error:      err.Error(),
+			Error:      errMsg,
+			Detail:     unwrapGoError(errMsg),
 		})
 	}
 	res.Steps = append(res.Steps, diagnoseStep{Name: "send-ssh-public-key", OK: true, DurationMs: time.Since(t0).Milliseconds()})

workspace-server/internal/handlers/terminal_diagnose_test.go

@@ -245,3 +245,50 @@ func TestDiagnoseRemote_StopsAtSSHProbe(t *testing.T) {
 	}
 }
 
+// TestUnwrapGoError pins the unwrapGoError helper that extracts subprocess
+// stderr from the Go-wrapped error string produced by sendSSHPublicKey.
+// Regression gate for mc#687: the E2E smoke now reads detail (not error),
+// so detail MUST contain the actionable AWS permission signal.
+func TestUnwrapGoError(t *testing.T) {
+	cases := []struct {
+		name   string
+		input  string
+		want   string
+	}{
+		{
+			name:  "AWS permission denied",
+			input: "send-ssh-public-key: exec: exit status 1 (AccessDeniedException: User: arn:aws:iam::123456789012:role/TestRole is not authorized to perform: ec2-instance-connect:OpenTunnel)",
+			want:  "AccessDeniedException: User: arn:aws:iam::123456789012:role/TestRole is not authorized to perform: ec2-instance-connect:OpenTunnel",
+		},
+		{
+			name:  "generic exec error no output",
+			input: "send-ssh-public-key: exec: exit status 1",
+			want:  "",
+		},
+		{
+			name:  "empty string",
+			input: "",
+			want:  "",
+		},
+		{
+			name:  "short string below threshold",
+			input: "err",
+			want:  "",
+		},
+		{
+			name:  "no parentheses",
+			input: "send-ssh-public-key: something went wrong",
+			want:  "",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := unwrapGoError(tc.input)
+			if got != tc.want {
+				t.Errorf("unwrapGoError(%q): got %q, want %q", tc.input, got, tc.want)
+			}
+		})
+	}
+}
+