fix(slack): scope incident resolve/decide clicks to the owning workspace#162
Open
arseniycodes wants to merge 1 commit into
Open
fix(slack): scope incident resolve/decide clicks to the owning workspace#162arseniycodes wants to merge 1 commit into
arseniycodes wants to merge 1 commit into
Conversation
handleSlackResolveIncident and handleProposalDecision looked up the incident/proposal by bare id and resolved/closed it before consulting any installation, and the underlying helpers (resolveIncident, confirm/dismissResolutionProposal) filter by id only — no project/tenant predicate. The interactivity payload is signed with the app signing secret and Slack only signs action_ids from a workspace's own messages, so this isn't forgeable by an ordinary user today — but it's a latent cross-workspace write with zero tenant enforcement at the mutation. Add clickWorkspaceOwnsPin(): before mutating, assert the workspace that sent the click (payload.team.id) owns the target's pinned Slack installation. Cross-workspace clicks are ignored. Unpinned/legacy rows fall through to the existing team-scoped fallback unchanged.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
handleSlackResolveIncidentandhandleProposalDecisionlook up the incident/proposal by bare id and resolve/close it before consulting any installation, and the DB helpers (resolveIncident,confirm/dismissResolutionProposal) filter on id only — no project/tenant predicate. So the only thing binding a resolve click to a tenant is the Slack signing-secret check on the request.Impact
Latent cross-workspace write: an interactivity payload carrying another tenant's incident/proposal id would resolve or silence their incident. It is not forgeable by an ordinary user — Slack signs only the action_ids our app set on that workspace's own messages — so this is defence-in-depth against a signing-secret compromise or a future refactor that lets an id flow from user-editable input. The dashboard-side equivalents already scope correctly; this closes the inconsistency.
Fix
Add
clickWorkspaceOwnsPin(pinnedInstallationId, payloadTeamId): before mutating, assert the clicking workspace owns the target's pinned Slack installation (itsteam_idmatchespayload.team.id). Cross-workspace clicks are logged and ignored. Unpinned/legacy rows return true and fall through to the existing team-scoped fallback, so legitimate same-workspace clicks are unaffected.Summary by cubic
Scopes Slack incident resolve/decision clicks to the owning workspace to prevent cross-workspace mutations. Cross-tenant clicks are logged and ignored; same-workspace and legacy/unpinned rows continue to work.
clickWorkspaceOwnsPin(...)to verify the clicking workspace (payload.team.id) matches the target’s pinned installation (slack_installations.team_id) before any mutation.handleSlackResolveIncidentandhandleProposalDecisionwith this check to close a latent cross-workspace write gap.Written for commit 894337a. Summary will update on new commits.