Files
multica/server/internal/handler/github_test.go
Jiang Bohan 329eed7a22 fix(github): address review — null actor, role gating, configured guard, scoped uninstall broadcast
- listeners: use optionalUUID(e.ActorID) so the system actor on the github-driven issue:updated event no longer panics activity / notification listeners; merged-PR → issue done now produces a status_changed activity and inbox entry.
- IntegrationsTab: gate the admin-only installations query on canManage so members no longer hit /github/installations 403; the configured/not-configured copy is also scoped to admins.
- backend: introduce isGitHubConfigured() requiring both GITHUB_APP_SLUG and GITHUB_WEBHOOK_SECRET, and surface that single flag from list-installations + connect endpoints so the frontend Connect button stays disabled until both are set.
- DeleteGitHubInstallationByInstallationID now RETURNs workspace_id; webhook handler publishes github_installation:deleted scoped to the right workspace so already-open Settings tabs invalidate in real time. ErrNoRows on a re-fired delete short-circuits cleanly.
- tests: focused webhook integration coverage (auto-link + merge → done, cancelled preservation, uninstall returns workspace).

Co-authored-by: multica-agent <github@multica.ai>
2026-05-08 18:20:02 +08:00

372 lines
12 KiB
Go

package handler
import (
"bytes"
"context"
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
"encoding/json"
"net/http"
"net/http/httptest"
"reflect"
"testing"
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
func TestExtractIdentifiers(t *testing.T) {
cases := []struct {
name string
in []string
want []string
}{
{
name: "branch_name",
in: []string{"", "", "mul-1510/fix-login"},
want: []string{"MUL-1510"},
},
{
name: "title_and_body",
in: []string{"Fix MUL-82", "Closes MUL-1510 and ABC-7", ""},
want: []string{"MUL-82", "MUL-1510", "ABC-7"},
},
{
name: "dedupe_across_fields",
in: []string{"MUL-1", "MUL-1 again", "mul-1/branch"},
want: []string{"MUL-1"},
},
{
name: "ignore_email_and_versions",
in: []string{"reply@user-1 v1.2-3 here", "", ""},
// Word-boundary regex still matches "user-1"; identifier prefix is
// any 2..10 letters/digits, so this is intentional. The downstream
// workspace prefix check in lookupIssueByIdentifier filters it.
want: []string{"USER-1"},
},
{
name: "no_match",
in: []string{"plain text", "no idents", ""},
want: []string{},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := extractIdentifiers(tc.in...)
if len(got) == 0 && len(tc.want) == 0 {
return
}
if !reflect.DeepEqual(got, tc.want) {
t.Errorf("extractIdentifiers() = %v, want %v", got, tc.want)
}
})
}
}
func TestDerivePRState(t *testing.T) {
cases := []struct {
state string
draft bool
merged bool
want string
}{
{"open", false, false, "open"},
{"open", true, false, "draft"},
{"closed", false, false, "closed"},
{"closed", false, true, "merged"},
{"closed", true, true, "merged"}, // merged trumps draft
}
for _, tc := range cases {
got := derivePRState(tc.state, tc.draft, tc.merged)
if got != tc.want {
t.Errorf("derivePRState(%q, draft=%v, merged=%v) = %q, want %q",
tc.state, tc.draft, tc.merged, got, tc.want)
}
}
}
func TestVerifyWebhookSignature(t *testing.T) {
secret := "shared-secret"
body := []byte(`{"action":"opened"}`)
mac := hmac.New(sha256.New, []byte(secret))
mac.Write(body)
good := "sha256=" + hex.EncodeToString(mac.Sum(nil))
if !verifyWebhookSignature(secret, good, body) {
t.Error("expected valid signature to verify")
}
if verifyWebhookSignature(secret, "sha256=deadbeef", body) {
t.Error("expected bad hex to fail")
}
if verifyWebhookSignature(secret, "", body) {
t.Error("expected empty header to fail")
}
if verifyWebhookSignature(secret, "sha1=whatever", body) {
t.Error("expected non-sha256 prefix to fail")
}
if verifyWebhookSignature("other-secret", good, body) {
t.Error("expected wrong secret to fail")
}
}
func TestStateRoundTrip(t *testing.T) {
t.Setenv("GITHUB_WEBHOOK_SECRET", "test-secret-123")
wsID := "11111111-2222-3333-4444-555555555555"
tok, err := signState(wsID)
if err != nil {
t.Fatalf("signState: %v", err)
}
got, ok := verifyState(tok)
if !ok {
t.Fatal("verifyState rejected a freshly-signed token")
}
if got != wsID {
t.Errorf("verifyState() = %q, want %q", got, wsID)
}
// Tampering with the workspace portion must fail (signature is bound
// to it). Replace the leading UUID's first hex digit.
tampered := "01111111" + tok[8:]
if _, ok := verifyState(tampered); ok {
t.Error("tampered state token should fail to verify")
}
// Wrong secret rejects.
t.Setenv("GITHUB_WEBHOOK_SECRET", "different")
if _, ok := verifyState(tok); ok {
t.Error("token signed with old secret should fail under a new one")
}
}
func TestSignStateRequiresSecret(t *testing.T) {
t.Setenv("GITHUB_WEBHOOK_SECRET", "")
if _, err := signState("ws"); err == nil {
t.Error("signState should error when secret is unset")
}
}
// TestWebhook_MergedPR_AdvancesLinkedIssueToDone exercises the end-to-end
// auto-link + merge-sync path: install a workspace, fire a `pull_request`
// webhook with the issue identifier in the title, and verify (a) the PR row
// is upserted, (b) it is linked to the issue, (c) the issue transitions to
// 'done'. The system actor on that issue:updated event is what previously
// panicked the activity / notification listeners — having this test pass
// while listeners are wired up is the regression guard.
func TestWebhook_MergedPR_AdvancesLinkedIssueToDone(t *testing.T) {
if testHandler == nil {
t.Skip("handler test fixture not initialized (no DB?)")
}
ctx := context.Background()
secret := "merge-sync-test-secret"
t.Setenv("GITHUB_WEBHOOK_SECRET", secret)
// Seed an issue we expect the webhook to close out.
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "PR auto-merge test",
"status": "in_progress",
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateIssue: %d %s", w.Code, w.Body.String())
}
var created IssueResponse
json.NewDecoder(w.Body).Decode(&created)
t.Cleanup(func() {
testPool.Exec(ctx, `DELETE FROM issue_pull_request WHERE issue_id = $1`, created.ID)
testPool.Exec(ctx, `DELETE FROM github_pull_request WHERE workspace_id = $1`, testWorkspaceID)
testPool.Exec(ctx, `DELETE FROM github_installation WHERE workspace_id = $1`, testWorkspaceID)
testPool.Exec(ctx, `DELETE FROM activity_log WHERE issue_id = $1`, created.ID)
testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, created.ID)
})
// Wire up an installation row for the webhook to attribute to.
const installationID int64 = 99887766
if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
WorkspaceID: parseUUID(testWorkspaceID),
InstallationID: installationID,
AccountLogin: "merge-sync-acct",
AccountType: "User",
}); err != nil {
t.Fatalf("CreateGitHubInstallation: %v", err)
}
// Build a minimal pull_request webhook payload referencing the issue.
body := map[string]any{
"action": "closed",
"pull_request": map[string]any{
"number": 1234,
"html_url": "https://github.com/acme/widget/pull/1234",
"title": "Fix login " + created.Identifier,
"body": "",
"state": "closed",
"draft": false,
"merged": true,
"merged_at": "2026-04-29T00:00:00Z",
"closed_at": "2026-04-29T00:00:00Z",
"created_at": "2026-04-28T00:00:00Z",
"updated_at": "2026-04-29T00:00:00Z",
"head": map[string]any{"ref": "fix/login"},
"user": map[string]any{"login": "octocat", "avatar_url": ""},
},
"repository": map[string]any{
"name": "widget",
"owner": map[string]any{"login": "acme"},
},
"installation": map[string]any{"id": installationID},
}
raw, _ := json.Marshal(body)
mac := hmac.New(sha256.New, []byte(secret))
mac.Write(raw)
sig := "sha256=" + hex.EncodeToString(mac.Sum(nil))
w = httptest.NewRecorder()
req2 := httptest.NewRequest("POST", "/api/webhooks/github", bytes.NewReader(raw))
req2.Header.Set("X-GitHub-Event", "pull_request")
req2.Header.Set("X-Hub-Signature-256", sig)
testHandler.HandleGitHubWebhook(w, req2)
if w.Code != http.StatusAccepted {
t.Fatalf("webhook: expected 202, got %d (%s)", w.Code, w.Body.String())
}
// Verify PR row + link + issue status.
pr, err := testHandler.Queries.GetGitHubPullRequest(ctx, db.GetGitHubPullRequestParams{
WorkspaceID: parseUUID(testWorkspaceID),
RepoOwner: "acme",
RepoName: "widget",
PrNumber: 1234,
})
if err != nil {
t.Fatalf("GetGitHubPullRequest: %v", err)
}
if pr.State != "merged" {
t.Errorf("expected pr state merged, got %q", pr.State)
}
linked, err := testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID))
if err != nil {
t.Fatalf("ListPullRequestsByIssue: %v", err)
}
if len(linked) != 1 {
t.Fatalf("expected 1 linked PR, got %d", len(linked))
}
updated, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
if err != nil {
t.Fatalf("GetIssue: %v", err)
}
if updated.Status != "done" {
t.Errorf("expected issue status 'done', got %q", updated.Status)
}
}
// TestWebhook_MergedPR_PreservesCancelled guards the "do not stomp cancelled"
// rule: cancelling an issue then merging a linked PR must leave the issue
// cancelled.
func TestWebhook_MergedPR_PreservesCancelled(t *testing.T) {
if testHandler == nil {
t.Skip("handler test fixture not initialized (no DB?)")
}
ctx := context.Background()
secret := "cancelled-secret"
t.Setenv("GITHUB_WEBHOOK_SECRET", secret)
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "Already cancelled",
"status": "cancelled",
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateIssue: %d %s", w.Code, w.Body.String())
}
var created IssueResponse
json.NewDecoder(w.Body).Decode(&created)
t.Cleanup(func() {
testPool.Exec(ctx, `DELETE FROM issue_pull_request WHERE issue_id = $1`, created.ID)
testPool.Exec(ctx, `DELETE FROM github_pull_request WHERE workspace_id = $1`, testWorkspaceID)
testPool.Exec(ctx, `DELETE FROM github_installation WHERE workspace_id = $1`, testWorkspaceID)
testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, created.ID)
})
const installationID int64 = 11223344
if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
WorkspaceID: parseUUID(testWorkspaceID),
InstallationID: installationID,
AccountLogin: "cancelled-acct",
AccountType: "User",
}); err != nil {
t.Fatalf("CreateGitHubInstallation: %v", err)
}
body, _ := json.Marshal(map[string]any{
"action": "closed",
"pull_request": map[string]any{
"number": 7, "html_url": "https://x", "title": "Closes " + created.Identifier,
"state": "closed", "merged": true, "draft": false,
"merged_at": "2026-04-29T00:00:00Z", "closed_at": "2026-04-29T00:00:00Z",
"created_at": "2026-04-28T00:00:00Z", "updated_at": "2026-04-29T00:00:00Z",
"head": map[string]any{"ref": "x"}, "user": map[string]any{"login": "u"},
},
"repository": map[string]any{"name": "r", "owner": map[string]any{"login": "o"}},
"installation": map[string]any{"id": installationID},
})
mac := hmac.New(sha256.New, []byte(secret))
mac.Write(body)
sig := "sha256=" + hex.EncodeToString(mac.Sum(nil))
w = httptest.NewRecorder()
req2 := httptest.NewRequest("POST", "/api/webhooks/github", bytes.NewReader(body))
req2.Header.Set("X-GitHub-Event", "pull_request")
req2.Header.Set("X-Hub-Signature-256", sig)
testHandler.HandleGitHubWebhook(w, req2)
updated, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
if err != nil {
t.Fatalf("GetIssue: %v", err)
}
if updated.Status != "cancelled" {
t.Errorf("expected status to remain 'cancelled', got %q", updated.Status)
}
}
// TestWebhook_UninstallReturnsWorkspaceForBroadcast guards #4: the uninstall
// path must look up the workspace_id BEFORE deleting the row so the
// resulting `github_installation:deleted` event is broadcast scoped to that
// workspace (the realtime listener drops events with empty workspace_id).
func TestWebhook_UninstallReturnsWorkspaceForBroadcast(t *testing.T) {
if testHandler == nil {
t.Skip("handler test fixture not initialized (no DB?)")
}
ctx := context.Background()
const installationID int64 = 55443322
if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
WorkspaceID: parseUUID(testWorkspaceID),
InstallationID: installationID,
AccountLogin: "uninstall-test",
AccountType: "User",
}); err != nil {
t.Fatalf("CreateGitHubInstallation: %v", err)
}
t.Cleanup(func() {
testPool.Exec(ctx, `DELETE FROM github_installation WHERE workspace_id = $1`, testWorkspaceID)
})
deleted, err := testHandler.Queries.DeleteGitHubInstallationByInstallationID(ctx, installationID)
if err != nil {
t.Fatalf("DeleteGitHubInstallationByInstallationID: %v", err)
}
if uuidToString(deleted.WorkspaceID) != testWorkspaceID {
t.Errorf("expected returned workspace_id %s, got %s", testWorkspaceID, uuidToString(deleted.WorkspaceID))
}
// Re-deleting must surface ErrNoRows so the handler can short-circuit
// the broadcast (and not panic).
if _, err := testHandler.Queries.DeleteGitHubInstallationByInstallationID(ctx, installationID); err == nil {
t.Error("expected ErrNoRows on second delete, got nil")
}
}