Skip to content

Commit 8e6f13f

Browse files
committed
fix: harden slack settings management
1 parent 707d91a commit 8e6f13f

4 files changed

Lines changed: 86 additions & 1 deletion

File tree

integrations/slack-gateway/gateway_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,6 +1514,72 @@ func TestInstallTargetSelectionAPIPreservesClassifiedUpsertFailure(t *testing.T)
15141514
}
15151515
}
15161516

1517+
func TestInstallTargetSelectionAPIRejectsStaleRequestID(t *testing.T) {
1518+
backendCalled := false
1519+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1520+
backendCalled = true
1521+
t.Fatalf("backend should not be called for stale install selection")
1522+
}))
1523+
defer backend.Close()
1524+
1525+
gateway := newSlackGateway(config{
1526+
PublicURL: "https://gateway.example.test",
1527+
SlackClientID: "client-id",
1528+
SlackClientSecret: "client-secret",
1529+
SlackSigningSecret: "signing-secret",
1530+
OAuthStateSecret: "oauth-state-secret",
1531+
SlackAPIBaseURL: "https://slack.example.test/api",
1532+
SlackBotScopes: []string{"chat:write"},
1533+
BackendBaseURL: backend.URL,
1534+
BackendInternalToken: "backend-internal-token",
1535+
SpritzBaseURL: "https://spritz.example.test",
1536+
SpritzServiceToken: "spritz-service-token",
1537+
PrincipalID: "shared-slack-gateway",
1538+
HTTPTimeout: 5 * time.Second,
1539+
DedupeTTL: time.Minute,
1540+
}, slog.New(slog.NewTextHandler(io.Discard, nil)))
1541+
1542+
pendingState, err := gateway.state.generatePendingInstall(pendingInstallState{
1543+
RequestID: "install-request-current",
1544+
Installation: slackInstallation{
1545+
TeamID: "T_workspace_current",
1546+
InstallingUserID: "U_installer",
1547+
BotAccessToken: "xoxb-installed",
1548+
BotUserID: "U_bot",
1549+
APIAppID: "A_app_1",
1550+
},
1551+
})
1552+
if err != nil {
1553+
t.Fatalf("generate pending install state failed: %v", err)
1554+
}
1555+
requestBody, err := json.Marshal(map[string]any{
1556+
"requestId": "install-request-stale",
1557+
"presetInputs": map[string]any{
1558+
"agentId": "ag_456",
1559+
},
1560+
})
1561+
if err != nil {
1562+
t.Fatalf("marshal request body: %v", err)
1563+
}
1564+
1565+
req := httptest.NewRequest(http.MethodPost, "/api/slack/install/selection", bytes.NewReader(requestBody))
1566+
req.Header.Set("Content-Type", "application/json")
1567+
req.AddCookie(&http.Cookie{
1568+
Name: pendingInstallCookieName,
1569+
Value: pendingState,
1570+
Path: "/api/slack/install/selection",
1571+
})
1572+
rec := httptest.NewRecorder()
1573+
gateway.routes().ServeHTTP(rec, req)
1574+
1575+
if rec.Code != http.StatusBadRequest {
1576+
t.Fatalf("expected 400 for stale request id, got %d: %s", rec.Code, rec.Body.String())
1577+
}
1578+
if backendCalled {
1579+
t.Fatalf("backend was called for stale install selection")
1580+
}
1581+
}
1582+
15171583
func TestInstallRedirectUsesConfiguredSlackHost(t *testing.T) {
15181584
cfg := config{
15191585
PublicURL: "https://gateway.example.test",

integrations/slack-gateway/management_api.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,12 @@ func (g *slackGateway) handleInstallTargetSelectionAPIPost(w http.ResponseWriter
121121
writeAPIError(w, http.StatusBadRequest, "install state is invalid or expired")
122122
return
123123
}
124-
requestID := firstNonEmpty(strings.TrimSpace(body.RequestID), pendingInstall.RequestID)
124+
bodyRequestID := strings.TrimSpace(body.RequestID)
125+
if bodyRequestID != "" && bodyRequestID != pendingInstall.RequestID {
126+
writeAPIError(w, http.StatusBadRequest, "install request is stale")
127+
return
128+
}
129+
requestID := pendingInstall.RequestID
125130
if len(body.PresetInputs) == 0 {
126131
writeAPIError(w, http.StatusBadRequest, "presetInputs is required")
127132
return

ui/src/pages/settings.test.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,19 @@ describe('SettingsPage', () => {
215215
).toBeTruthy();
216216
});
217217

218+
it('redirects the Slack settings landing page to workspace settings', async () => {
219+
render(
220+
<MemoryRouter initialEntries={['/settings/slack']}>
221+
<Routes>
222+
<Route path="settings/*" element={<SettingsPage />} />
223+
</Routes>
224+
</MemoryRouter>,
225+
);
226+
227+
expect(await screen.findByRole('heading', { name: 'Slack Workspaces' })).toBeTruthy();
228+
expect(await screen.findByText('No Slack workspaces are installed.')).toBeTruthy();
229+
});
230+
218231
it('routes typed install picker failures to the install result page', async () => {
219232
const user = userEvent.setup();
220233
const installResult = {

ui/src/pages/settings.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,7 @@ export function SettingsPage() {
931931
<Routes>
932932
<Route element={<SettingsShell />}>
933933
<Route index element={<Navigate to="slack/workspaces" replace />} />
934+
<Route path="slack" element={<Navigate to="workspaces" replace />} />
934935
<Route path="slack/workspaces" element={<WorkspaceListPage />} />
935936
<Route path="slack/workspaces/target" element={<WorkspaceTargetPage />} />
936937
<Route path="slack/workspaces/test" element={<WorkspaceTestPage />} />

0 commit comments

Comments
 (0)