Compare commits

...

3 Commits

Author SHA1 Message Date
Lambda
393f377718 feat(github): split canView / canManage in settings tab for read-only members (MUL-2413)
Wires the frontend half of the read-only RFC. The Settings → GitHub tab
now always issues the installation list query for any workspace member
(the backend gates it via `RequireWorkspaceMember` after PR #2886) and
gets `can_manage` straight from the API response. The render matrix
covers the six cases the RFC calls out:

- configured + connected + admin   → Disconnect + (optional) Connected by
- configured + connected + member  → read-only "Connected to" + read_only_hint
- configured + not connected + admin   → Connect button + dev description
- configured + not connected + member  → contact_admin_to_connect hint
- not configured + admin               → operator banner + disabled Connect
- not configured + member              → contact_admin_to_connect hint

New i18n keys (en + zh-Hans): read_only_hint, connected_by, contact_admin_to_connect.
The unused github.manage_hint string is removed (its non-admin branch
now resolves to one of the two new hints depending on connection state).

GitHubInstallation gains an optional `connected_by` display name so the
UI can render the "Connected by {name}" line without further changes
once the backend exposes the field.

Co-authored-by: multica-agent <github@multica.ai>
2026-05-20 01:57:13 +08:00
Lambda
723c5cb459 fix(github): redact installation_id from realtime broadcasts (MUL-2413)
GET /github/installations strips the numeric installation_id for non-admin
members, but the github_installation:created / uninstall / suspend WS
events were still publishing it, so the same handle was reachable from
any workspace client subscribed to the workspace scope. Broadcast both
payload variants without it — the frontend uses these events only to
invalidate the installations query, so admins re-query the list endpoint
to recover the management handle.

Also adds a router-level test that mounts the production middleware split
(member-visible list vs. owner/admin connect+delete) so a future routing
change can't silently widen the write surface.

Co-authored-by: multica-agent <github@multica.ai>
2026-05-20 01:42:12 +08:00
Lambda
27851ae161 feat(github): expose read-only installation list to workspace members (MUL-2413)
Relax `GET /api/workspaces/{id}/github/installations` from owner/admin-only
to any workspace member so the Settings → Integrations tab no longer renders
blank for non-admins (the original symptom of MUL-2413).

The handler now reads the caller's role from the workspace middleware:
- owner / admin keep the full row including the numeric `installation_id`
  (the connect / disconnect handle) and receive `can_manage: true`.
- every other role (member / guest) receives rows with `installation_id`
  omitted and `can_manage: false`, giving them visibility into "is GitHub
  wired up?" without the management handle.

`GET /github/connect` and `DELETE /github/installations/{id}` stay under
the admin/owner middleware group — this PR only relaxes the read path.

Tests: `TestListGitHubInstallations_RoleGating` exercises admin, owner,
member, and guest paths against the real DB-backed handler fixture and
asserts the field stripping + `can_manage` contract.

Refs: MUL-2413
Co-authored-by: multica-agent <github@multica.ai>
2026-05-20 01:24:51 +08:00
8 changed files with 469 additions and 30 deletions

View File

@@ -14,11 +14,18 @@ export type GitHubMergeableState = string;
export interface GitHubInstallation {
id: string;
workspace_id: string;
installation_id: number;
/** GitHub's numeric installation id — the management handle used by the
* connect / disconnect flows. Omitted when the caller cannot manage
* integrations (see `ListGitHubInstallationsResponse.can_manage`). */
installation_id?: number;
account_login: string;
account_type: "User" | "Organization";
account_avatar_url: string | null;
created_at: string;
/** Display name of the workspace member who connected this installation.
* Optional because older backends and minimum-visibility deployments may
* omit it; the UI renders the "connected by" line only when present. */
connected_by?: string;
}
export interface GitHubPullRequest {
@@ -57,6 +64,11 @@ export interface ListGitHubInstallationsResponse {
installations: GitHubInstallation[];
/** Whether the deployment has GitHub App credentials configured. When false, the Connect button is hidden / disabled. */
configured: boolean;
/** Whether the caller can connect / disconnect installations. Non-admin
* members get `false` along with installations that omit `installation_id`.
* Older backends predating MUL-2413 omit the field; treat absence as
* `false` for read-only safety. */
can_manage?: boolean;
}
export interface GitHubConnectResponse {

View File

@@ -187,7 +187,9 @@
"disconnect_confirm_action": "Disconnect",
"not_configured": "GitHub integration is not configured for this deployment. Operators must set",
"not_configured_and": "and",
"manage_hint": "Only admins and owners can manage the GitHub connection.",
"read_only_hint": "Read-only view. Only workspace admins and owners can connect or disconnect GitHub.",
"contact_admin_to_connect": "GitHub is not connected for this workspace. Ask an admin or owner to set up the GitHub App.",
"connected_by": "Connected by {{name}}",
"toast_not_configured": "GitHub integration is not configured for this deployment",
"toast_open_failed": "Failed to open GitHub install",
"toast_disconnect_failed": "Failed to disconnect GitHub App",

View File

@@ -187,7 +187,9 @@
"disconnect_confirm_action": "断开",
"not_configured": "当前部署没有配置 GitHub 集成。运维需要设置",
"not_configured_and": "和",
"manage_hint": "只有管理员和所有者可以管理 GitHub 连接。",
"read_only_hint": "只读视图。只有管理员和所有者可以连接或断开 GitHub。",
"contact_admin_to_connect": "当前工作区尚未连接 GitHub。请让管理员或所有者完成 GitHub App 的配置。",
"connected_by": "由 {{name}} 连接",
"toast_not_configured": "当前部署未配置 GitHub 集成",
"toast_open_failed": "打开 GitHub 安装页失败",
"toast_disconnect_failed": "断开 GitHub App 失败",

View File

@@ -22,11 +22,21 @@ const workspaceRef = vi.hoisted(() => ({
repos: [{ url: "https://github.com/acme/api" }] as { url: string }[],
},
}));
type MemberRole = "owner" | "admin" | "member" | "guest";
const membersRef = vi.hoisted(() => ({
current: [{ user_id: "user-1", role: "owner" as const }],
current: [{ user_id: "user-1", role: "owner" as MemberRole }],
}));
const installationsRef = vi.hoisted(() => ({
current: { installations: [] as { id: string; account_login: string }[], configured: true },
current: {
installations: [] as {
id: string;
account_login: string;
installation_id?: number;
connected_by?: string;
}[],
configured: true,
can_manage: true as boolean,
},
}));
vi.mock("@tanstack/react-query", () => ({
@@ -124,7 +134,7 @@ function resetFixtures() {
repos: [{ url: "https://github.com/acme/api" }],
};
membersRef.current = [{ user_id: "user-1", role: "owner" }];
installationsRef.current = { installations: [], configured: true };
installationsRef.current = { installations: [], configured: true, can_manage: true };
}
describe("GitHubTab", () => {
@@ -184,7 +194,8 @@ describe("GitHubTab", () => {
const user = userEvent.setup();
installationsRef.current = {
configured: true,
installations: [{ id: "inst-42", account_login: "acme" }],
can_manage: true,
installations: [{ id: "inst-42", account_login: "acme", installation_id: 42 }],
};
mockDeleteInstallation.mockResolvedValue(undefined);
@@ -208,12 +219,58 @@ describe("GitHubTab", () => {
workspaceRef.current.settings = { github_enabled: false };
installationsRef.current = {
configured: true,
installations: [{ id: "inst-1", account_login: "acme" }],
can_manage: true,
installations: [{ id: "inst-1", account_login: "acme", installation_id: 1 }],
};
render(<GitHubTab />, { wrapper: I18nWrapper });
expect(screen.getByRole("button", { name: /^Disconnect$/ })).toBeTruthy();
});
it("non-admin sees the existing connection but no Connect/Disconnect controls", () => {
membersRef.current = [{ user_id: "user-1", role: "member" }];
installationsRef.current = {
configured: true,
can_manage: false,
installations: [{ id: "inst-1", account_login: "acme" }],
};
render(<GitHubTab />, { wrapper: I18nWrapper });
expect(screen.getByText(/Connected to acme/i)).toBeTruthy();
expect(screen.getByText(/Read-only view\./i)).toBeTruthy();
expect(screen.queryByRole("button", { name: /^Connect GitHub$/ })).toBeNull();
expect(screen.queryByRole("button", { name: /^Disconnect$/ })).toBeNull();
});
it("non-admin with no connection sees the contact-admin hint", () => {
membersRef.current = [{ user_id: "user-1", role: "member" }];
installationsRef.current = {
configured: true,
can_manage: false,
installations: [],
};
render(<GitHubTab />, { wrapper: I18nWrapper });
expect(screen.getByText(/Ask an admin or owner/i)).toBeTruthy();
expect(screen.queryByRole("button", { name: /^Connect GitHub$/ })).toBeNull();
});
it("renders the connected_by line when the backend provides it", () => {
installationsRef.current = {
configured: true,
can_manage: true,
installations: [
{
id: "inst-7",
account_login: "acme",
installation_id: 7,
connected_by: "Jiayuan",
},
],
};
render(<GitHubTab />, { wrapper: I18nWrapper });
expect(screen.getByText(/Connected by Jiayuan/)).toBeTruthy();
});
it("repositories shortcut navigates to the repositories tab", async () => {
const user = userEvent.setup();
render(<GitHubTab />, { wrapper: I18nWrapper });

View File

@@ -48,15 +48,21 @@ export function GitHubTab() {
const { data: members = [] } = useQuery(memberListOptions(wsId));
const currentMember = members.find((m) => m.user_id === user?.id) ?? null;
const canManage = currentMember?.role === "owner" || currentMember?.role === "admin";
// `canView` gates the read-only installation list (every workspace member
// sees it after MUL-2413); `canManage` gates the Connect / Disconnect
// actions and comes from the backend response (`can_manage`) so the
// frontend never claims management rights the server would reject.
const canView = !!currentMember;
const { data: installationData } = useQuery({
...githubInstallationsOptions(wsId),
enabled: !!wsId && canManage,
enabled: !!wsId && canView,
});
const installations = installationData?.installations ?? [];
const configured = installationData?.configured ?? false;
const canManage = installationData?.can_manage === true;
const connected = installations.length > 0;
const primaryInstallation = installations[0] ?? null;
const flags = deriveGitHubSettings(workspace);
const [savingKey, setSavingKey] = useState<SettingsKey | null>(null);
@@ -169,12 +175,21 @@ export function GitHubTab() {
<div className="space-y-1">
<p className="text-sm font-medium">{t(($) => $.github.connection_title)}</p>
{connected ? (
<p className="text-xs text-muted-foreground">
{t(($) => $.github.connected_to, {
login: installations.map((i) => i.account_login).join(", "),
})}
</p>
) : (
<>
<p className="text-xs text-muted-foreground">
{t(($) => $.github.connected_to, {
login: installations.map((i) => i.account_login).join(", "),
})}
</p>
{primaryInstallation?.connected_by && (
<p className="text-xs text-muted-foreground">
{t(($) => $.github.connected_by, {
name: primaryInstallation.connected_by!,
})}
</p>
)}
</>
) : canManage ? (
<p className="text-xs text-muted-foreground">
{t(($) => $.github.connection_description_prefix)}{" "}
<code className="rounded bg-muted px-1 py-0.5 text-[10px]">
@@ -183,19 +198,23 @@ export function GitHubTab() {
{t(($) => $.github.connection_description_suffix)}{" "}
<strong>{t(($) => $.github.connection_description_done)}</strong>.
</p>
) : (
<p className="text-xs text-muted-foreground">
{t(($) => $.github.contact_admin_to_connect)}
</p>
)}
</div>
</div>
{canManage && (
<div className="flex items-center gap-2">
{connected && installations[0] ? (
{connected && primaryInstallation ? (
// Disconnect must stay reachable even when the master switch
// is off — disconnect is a separate intent (revoke the App
// grant) from hiding the feature.
<Button
variant="outline"
size="sm"
onClick={() => setDisconnectTarget(installations[0]!.id)}
onClick={() => setDisconnectTarget(primaryInstallation.id)}
>
{t(($) => $.github.disconnect)}
</Button>
@@ -228,9 +247,9 @@ export function GitHubTab() {
</p>
)}
{!canManage && (
{!canManage && connected && (
<p className="text-xs text-muted-foreground">
{t(($) => $.github.manage_hint)}
{t(($) => $.github.read_only_hint)}
</p>
)}
</CardContent>

View File

@@ -326,6 +326,11 @@ func NewRouterWithOptions(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus
r.Get("/members", h.ListMembersWithUser)
r.Post("/leave", h.LeaveWorkspace)
r.Get("/invitations", h.ListWorkspaceInvitations)
// Listing GitHub installations is member-visible so the
// integrations tab no longer renders blank for non-admins;
// the handler strips the management handle and adds a
// can_manage hint so the UI can gate connect/disconnect.
r.Get("/github/installations", h.ListGitHubInstallations)
})
// Admin-level access
r.Group(func(r chi.Router) {
@@ -342,12 +347,12 @@ func NewRouterWithOptions(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus
// Owner-only access
r.With(middleware.RequireWorkspaceRoleFromURL(queries, "id", "owner")).Delete("/", h.DeleteWorkspace)
// GitHub integration — admin-only operations live here so the
// nesting matches the rest of /api/workspaces/{id}/* routes.
// GitHub integration — connect / disconnect remain admin-only;
// the read-only list endpoint lives in the member-level group
// above so non-admins can see the workspace's connection state.
r.Group(func(r chi.Router) {
r.Use(middleware.RequireWorkspaceRoleFromURL(queries, "id", "owner", "admin"))
r.Get("/github/connect", h.GitHubConnect)
r.Get("/github/installations", h.ListGitHubInstallations)
r.Delete("/github/installations/{installationId}", h.DeleteGitHubInstallation)
})
})

View File

@@ -22,16 +22,26 @@ import (
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/multica-ai/multica/server/internal/middleware"
db "github.com/multica-ai/multica/server/pkg/db/generated"
"github.com/multica-ai/multica/server/pkg/protocol"
)
// ── Response shapes ─────────────────────────────────────────────────────────
// GitHubInstallationResponse is the JSON shape returned by the installation
// list endpoint and broadcast on installation-related WS events.
//
// InstallationID is admin-only: the numeric GitHub installation_id is the
// management handle used by the Connect/Disconnect flows, so non-admin
// members receive responses with the field omitted. The list handler gates
// it by role; realtime broadcasts always omit it because the WS fanout has
// no per-recipient view (admins re-query the list endpoint on invalidation
// to recover the management handle).
type GitHubInstallationResponse struct {
ID string `json:"id"`
WorkspaceID string `json:"workspace_id"`
InstallationID int64 `json:"installation_id"`
InstallationID *int64 `json:"installation_id,omitempty"`
AccountLogin string `json:"account_login"`
AccountType string `json:"account_type"`
AccountAvatarURL *string `json:"account_avatar_url"`
@@ -83,10 +93,11 @@ type GitHubConnectResponse struct {
}
func githubInstallationToResponse(i db.GithubInstallation) GitHubInstallationResponse {
instID := i.InstallationID
return GitHubInstallationResponse{
ID: uuidToString(i.ID),
WorkspaceID: uuidToString(i.WorkspaceID),
InstallationID: i.InstallationID,
InstallationID: &instID,
AccountLogin: i.AccountLogin,
AccountType: i.AccountType,
AccountAvatarURL: textToPtr(i.AccountAvatarUrl),
@@ -94,6 +105,19 @@ func githubInstallationToResponse(i db.GithubInstallation) GitHubInstallationRes
}
}
// githubInstallationToBroadcast returns the same shape as the list endpoint's
// per-role response with the numeric `installation_id` stripped. Realtime
// events fan out to every WS client subscribed to the workspace, so the
// payload must match the weakest-role view — admin/owner clients re-query
// the list endpoint to recover the management handle. The frontend uses
// these events only to invalidate the installations query, so it does not
// read `installation_id` off the broadcast.
func githubInstallationToBroadcast(i db.GithubInstallation) GitHubInstallationResponse {
resp := githubInstallationToResponse(i)
resp.InstallationID = nil
return resp
}
func githubPullRequestToResponse(p db.GithubPullRequest) GitHubPullRequestResponse {
return GitHubPullRequestResponse{
ID: uuidToString(p.ID),
@@ -325,7 +349,7 @@ func (h *Handler) GitHubSetupCallback(w http.ResponseWriter, r *http.Request) {
return
}
h.publish(protocol.EventGitHubInstallationCreated, workspaceID, "system", "", map[string]any{
"installation": githubInstallationToResponse(inst),
"installation": githubInstallationToBroadcast(inst),
})
http.Redirect(w, r, settingsURL+"&github_connected=1", http.StatusFound)
}
@@ -378,12 +402,21 @@ func fetchInstallationAccount(ctx context.Context, installationID int64) (login,
// ── Listing / disconnect ────────────────────────────────────────────────────
// ListGitHubInstallations returns the workspace's connected GitHub
// installations to any workspace member. Connect/disconnect remain
// admin-only at the router level, so the response carries a `can_manage`
// hint and strips the numeric `installation_id` for non-admin callers —
// they get visibility into "is GitHub wired up, and by whom?" without the
// management handle.
func (h *Handler) ListGitHubInstallations(w http.ResponseWriter, r *http.Request) {
workspaceID := chi.URLParam(r, "id")
wsUUID, ok := parseUUIDOrBadRequest(w, workspaceID, "workspace id")
if !ok {
return
}
member, _ := middleware.MemberFromContext(r.Context())
canManage := roleAllowed(member.Role, "owner", "admin")
rows, err := h.Queries.ListGitHubInstallationsByWorkspace(r.Context(), wsUUID)
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to list installations")
@@ -391,9 +424,17 @@ func (h *Handler) ListGitHubInstallations(w http.ResponseWriter, r *http.Request
}
out := make([]GitHubInstallationResponse, 0, len(rows))
for _, row := range rows {
out = append(out, githubInstallationToResponse(row))
resp := githubInstallationToResponse(row)
if !canManage {
resp.InstallationID = nil
}
out = append(out, resp)
}
writeJSON(w, http.StatusOK, map[string]any{"installations": out, "configured": isGitHubConfigured()})
writeJSON(w, http.StatusOK, map[string]any{
"installations": out,
"configured": isGitHubConfigured(),
"can_manage": canManage,
})
}
func (h *Handler) DeleteGitHubInstallation(w http.ResponseWriter, r *http.Request) {
@@ -536,9 +577,12 @@ func (h *Handler) handleInstallationEvent(ctx context.Context, body []byte) {
slog.Warn("github: delete installation failed", "err", err, "installation_id", p.Installation.ID)
return
}
// Broadcast the internal row id only — the numeric installation_id is
// a management handle that non-admin members are not allowed to see.
// The frontend invalidates the installations query on this event and
// does not read the broadcast payload directly.
h.publish(protocol.EventGitHubInstallationDeleted, uuidToString(deleted.WorkspaceID), "system", "", map[string]any{
"installation_id": p.Installation.ID,
"id": uuidToString(deleted.ID),
"id": uuidToString(deleted.ID),
})
case "created", "new_permissions_accepted", "unsuspend":
// We don't know which workspace this maps to from the webhook

View File

@@ -7,12 +7,15 @@ import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"reflect"
"testing"
"time"
"github.com/go-chi/chi/v5"
"github.com/multica-ai/multica/server/internal/middleware"
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
@@ -1038,3 +1041,298 @@ func TestWebhook_PullRequest_MetadataPreservesMergeable(t *testing.T) {
t.Errorf("expected mergeable_state preserved as clean after metadata event, got %+v", rows[0].MergeableState)
}
}
// TestListGitHubInstallations_RoleGating covers the read-only relaxation
// in MUL-2413: the endpoint is now reachable by any workspace member, but
// the handler strips the numeric installation_id and reports `can_manage`
// based on the caller's role. Admins / owners still receive the full row.
func TestListGitHubInstallations_RoleGating(t *testing.T) {
if testHandler == nil {
t.Skip("handler test fixture not initialized (no DB?)")
}
ctx := context.Background()
const installationID int64 = 42424242
if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
WorkspaceID: parseUUID(testWorkspaceID),
InstallationID: installationID,
AccountLogin: "role-gating-acct",
AccountType: "Organization",
}); err != nil {
t.Fatalf("CreateGitHubInstallation: %v", err)
}
t.Cleanup(func() {
testPool.Exec(ctx, `DELETE FROM github_installation WHERE workspace_id = $1`, testWorkspaceID)
})
call := func(t *testing.T, role string) map[string]any {
t.Helper()
req := httptest.NewRequest(http.MethodGet, "/api/workspaces/"+testWorkspaceID+"/github/installations", nil)
req = withURLParam(req, "id", testWorkspaceID)
req = req.WithContext(middleware.SetMemberContext(req.Context(), testWorkspaceID, db.Member{Role: role}))
w := httptest.NewRecorder()
testHandler.ListGitHubInstallations(w, req)
if w.Code != http.StatusOK {
t.Fatalf("ListGitHubInstallations(%s): %d %s", role, w.Code, w.Body.String())
}
var body map[string]any
if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
t.Fatalf("decode body (%s): %v", role, err)
}
return body
}
t.Run("admin sees installation_id + can_manage true", func(t *testing.T) {
body := call(t, "admin")
if got, _ := body["can_manage"].(bool); !got {
t.Errorf("can_manage = %v, want true", body["can_manage"])
}
installs, _ := body["installations"].([]any)
if len(installs) == 0 {
t.Fatalf("expected at least one installation row, got %v", installs)
}
row, _ := installs[0].(map[string]any)
gotID, ok := row["installation_id"].(float64)
if !ok {
t.Fatalf("admin response missing installation_id: %v", row)
}
if int64(gotID) != installationID {
t.Errorf("installation_id = %v, want %d", gotID, installationID)
}
})
t.Run("owner sees installation_id + can_manage true", func(t *testing.T) {
body := call(t, "owner")
if got, _ := body["can_manage"].(bool); !got {
t.Errorf("can_manage = %v, want true", body["can_manage"])
}
installs, _ := body["installations"].([]any)
row, _ := installs[0].(map[string]any)
if _, ok := row["installation_id"]; !ok {
t.Errorf("owner response missing installation_id: %v", row)
}
})
t.Run("member sees row without installation_id and can_manage false", func(t *testing.T) {
body := call(t, "member")
canManage, _ := body["can_manage"].(bool)
if canManage {
t.Errorf("can_manage = true, want false for non-admin member")
}
installs, _ := body["installations"].([]any)
if len(installs) == 0 {
t.Fatalf("member should still see installation rows, got %v", installs)
}
row, _ := installs[0].(map[string]any)
if _, present := row["installation_id"]; present {
t.Errorf("installation_id must be omitted for non-admin members, row=%v", row)
}
// Display fields the read-only view still needs must round-trip.
if got, _ := row["account_login"].(string); got != "role-gating-acct" {
t.Errorf("account_login = %q, want role-gating-acct", got)
}
})
t.Run("guest is treated as read-only and can_manage is false", func(t *testing.T) {
body := call(t, "guest")
if canManage, _ := body["can_manage"].(bool); canManage {
t.Errorf("can_manage = true, want false for guest")
}
installs, _ := body["installations"].([]any)
row, _ := installs[0].(map[string]any)
if _, present := row["installation_id"]; present {
t.Errorf("installation_id must be omitted for guest, row=%v", row)
}
})
}
// TestGitHubRoutes_RoleGating exercises the router-level middleware split
// introduced in MUL-2413: GET installations runs under
// RequireWorkspaceMemberFromURL while connect / delete remain behind
// RequireWorkspaceRoleFromURL(owner, admin). The handler-level tests above
// inject a member into context directly and so do not cover the middleware
// itself — a future routing change that accidentally moved one of the
// admin-only routes into the member group would slip past them.
func TestGitHubRoutes_RoleGating(t *testing.T) {
if testHandler == nil {
t.Skip("handler test fixture not initialized (no DB?)")
}
ctx := context.Background()
const slug = "github-routes-role-gating"
_, _ = testPool.Exec(ctx, `DELETE FROM workspace WHERE slug = $1`, slug)
var wsID string
if err := testPool.QueryRow(ctx, `
INSERT INTO workspace (name, slug, description, issue_prefix)
VALUES ($1, $2, $3, $4)
RETURNING id
`, "GitHub Routes Role Gating", slug, "github routes role gating", "GRG").Scan(&wsID); err != nil {
t.Fatalf("create workspace: %v", err)
}
// Three workspace members + one outsider. We attach the requesting user
// via the X-User-ID header so the middleware reads them off the auth
// boundary just like a real request.
mkUser := func(t *testing.T, label string) string {
t.Helper()
var id string
email := fmt.Sprintf("github-routes-%s-%s@multica.ai", slug, label)
if err := testPool.QueryRow(ctx, `
INSERT INTO "user" (name, email) VALUES ($1, $2) RETURNING id
`, "GHR "+label, email).Scan(&id); err != nil {
t.Fatalf("create user %s: %v", label, err)
}
return id
}
adminUserID := mkUser(t, "admin")
memberUserID := mkUser(t, "member")
outsiderUserID := mkUser(t, "outsider")
for _, m := range []struct {
userID, role string
}{
{adminUserID, "admin"},
{memberUserID, "member"},
} {
if _, err := testPool.Exec(ctx, `
INSERT INTO member (workspace_id, user_id, role) VALUES ($1, $2, $3)
`, wsID, m.userID, m.role); err != nil {
t.Fatalf("insert member (%s): %v", m.role, err)
}
}
const installationID int64 = 90909090
createdInst, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
WorkspaceID: parseUUID(wsID),
InstallationID: installationID,
AccountLogin: "routes-acct",
AccountType: "User",
})
if err != nil {
t.Fatalf("CreateGitHubInstallation: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM workspace WHERE id = $1`, wsID)
for _, uid := range []string{adminUserID, memberUserID, outsiderUserID} {
_, _ = testPool.Exec(context.Background(), `DELETE FROM "user" WHERE id = $1`, uid)
}
})
// Build a router subtree mirroring the production wiring at
// server/cmd/server/router.go for the workspace-scoped GitHub routes.
// Mounting the real middleware is what makes this a routing-level test —
// the role split has to come from the chi groups, not from the handler.
router := chi.NewRouter()
router.Route("/api/workspaces/{id}", func(r chi.Router) {
r.Group(func(r chi.Router) {
r.Use(middleware.RequireWorkspaceMemberFromURL(testHandler.Queries, "id"))
r.Get("/github/installations", testHandler.ListGitHubInstallations)
})
r.Group(func(r chi.Router) {
r.Use(middleware.RequireWorkspaceRoleFromURL(testHandler.Queries, "id", "owner", "admin"))
r.Get("/github/connect", testHandler.GitHubConnect)
r.Delete("/github/installations/{installationId}", testHandler.DeleteGitHubInstallation)
})
})
exercise := func(t *testing.T, method, path, userID string) int {
t.Helper()
req := httptest.NewRequest(method, path, nil)
if userID != "" {
req.Header.Set("X-User-ID", userID)
}
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
return rec.Code
}
t.Run("GET installations is reachable by members", func(t *testing.T) {
if code := exercise(t, http.MethodGet, "/api/workspaces/"+wsID+"/github/installations", memberUserID); code != http.StatusOK {
t.Errorf("member GET installations: want 200, got %d", code)
}
if code := exercise(t, http.MethodGet, "/api/workspaces/"+wsID+"/github/installations", adminUserID); code != http.StatusOK {
t.Errorf("admin GET installations: want 200, got %d", code)
}
})
t.Run("GET installations rejects non-members", func(t *testing.T) {
// Outsider hits the workspace middleware before the handler — the
// middleware translates a missing membership row into 404.
if code := exercise(t, http.MethodGet, "/api/workspaces/"+wsID+"/github/installations", outsiderUserID); code != http.StatusNotFound {
t.Errorf("outsider GET installations: want 404, got %d", code)
}
})
t.Run("GET connect remains owner/admin only", func(t *testing.T) {
if code := exercise(t, http.MethodGet, "/api/workspaces/"+wsID+"/github/connect", adminUserID); code != http.StatusOK {
t.Errorf("admin GET connect: want 200, got %d", code)
}
if code := exercise(t, http.MethodGet, "/api/workspaces/"+wsID+"/github/connect", memberUserID); code != http.StatusForbidden {
t.Errorf("member GET connect: want 403, got %d", code)
}
if code := exercise(t, http.MethodGet, "/api/workspaces/"+wsID+"/github/connect", outsiderUserID); code != http.StatusNotFound {
t.Errorf("outsider GET connect: want 404, got %d", code)
}
})
t.Run("DELETE installation remains owner/admin only", func(t *testing.T) {
// Member: 403 — middleware rejects before the handler runs.
if code := exercise(t, http.MethodDelete, "/api/workspaces/"+wsID+"/github/installations/"+uuidToString(createdInst.ID), memberUserID); code != http.StatusForbidden {
t.Errorf("member DELETE installation: want 403, got %d", code)
}
// Outsider: 404 — workspace not found.
if code := exercise(t, http.MethodDelete, "/api/workspaces/"+wsID+"/github/installations/"+uuidToString(createdInst.ID), outsiderUserID); code != http.StatusNotFound {
t.Errorf("outsider DELETE installation: want 404, got %d", code)
}
// Admin: 204 and the row goes away.
if code := exercise(t, http.MethodDelete, "/api/workspaces/"+wsID+"/github/installations/"+uuidToString(createdInst.ID), adminUserID); code != http.StatusNoContent {
t.Errorf("admin DELETE installation: want 204, got %d", code)
}
var remaining int
if err := testPool.QueryRow(ctx, `SELECT COUNT(*) FROM github_installation WHERE id = $1`, uuidToString(createdInst.ID)).Scan(&remaining); err != nil {
t.Fatalf("verify deletion: %v", err)
}
if remaining != 0 {
t.Errorf("expected installation row gone after admin DELETE, got %d remaining", remaining)
}
})
}
// TestGitHubInstallationBroadcastRedaction guards Emacs' finding on PR #2886:
// the realtime payloads we publish on installation create / uninstall must
// not carry the numeric `installation_id`. The frontend uses these events
// only to invalidate the installations query, so an admin client recovers
// the management handle via the list endpoint — which already gates the
// numeric id by role.
func TestGitHubInstallationBroadcastRedaction(t *testing.T) {
inst := db.GithubInstallation{
InstallationID: 123456789,
AccountLogin: "broadcast-acct",
AccountType: "User",
}
got := githubInstallationToBroadcast(inst)
if got.InstallationID != nil {
t.Errorf("broadcast payload must omit installation_id, got %v", *got.InstallationID)
}
if got.AccountLogin != "broadcast-acct" {
t.Errorf("expected account_login preserved, got %q", got.AccountLogin)
}
// Sanity: the JSON encoding actually drops the field (omitempty + nil
// pointer). A future change to the response shape could re-introduce
// the field through a different name; the JSON check is the real
// assertion against the wire format clients see.
raw, err := json.Marshal(got)
if err != nil {
t.Fatalf("marshal broadcast payload: %v", err)
}
var generic map[string]any
if err := json.Unmarshal(raw, &generic); err != nil {
t.Fatalf("unmarshal broadcast payload: %v", err)
}
if _, present := generic["installation_id"]; present {
t.Errorf("installation_id leaked into broadcast JSON: %s", string(raw))
}
}