-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(tasks): let team members drive runs of slack-originated tasks #72590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1784,10 +1784,17 @@ def task_accessible_for_run_view( | |
| read actions, which the caller signals via ``bypass_visibility``. Run-mutating actions pass | ||
| ``for_control`` to use the narrower ``task_control_q`` — public-channel visibility lets | ||
| teammates watch a run, not drive it. | ||
|
|
||
| Slack-originated tasks are the exception: those threads are multiplayer, so any same-team | ||
| user can already steer the run from Slack (follow-ups record them as the run's actor, with | ||
| sandbox credentials minted for them). The runs API mirrors that and lets team members drive | ||
| and watch Slack tasks' runs — otherwise a non-creator actor's sandbox 404s on every callback | ||
| (reply relay, log-append heartbeat, completion PATCH) and the thread dies silently. | ||
| """ | ||
| task_filter = Task.objects.filter(id=task_id, team_id=team_id) | ||
| if not bypass_visibility: | ||
| task_filter = task_filter.filter(task_control_q(user_id) if for_control else task_visibility_q(user_id)) | ||
| scope_q = task_control_q(user_id) if for_control else task_visibility_q(user_id) | ||
| task_filter = task_filter.filter(scope_q | Q(origin_product=Task.OriginProduct.SLACK)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This OR condition makes every authenticated project member a controller of every Slack-originated task once they know its task/run IDs, without verifying that they are the Slack actor or even belong to the mapped Slack channel. The run controller can mint a 24-hour sandbox connection JWT and send agent commands, cancel the run, or relay arbitrary bot messages into the mapped Slack thread. Since Slack runs may operate with the initiating actor's GitHub credentials, this lets an unrelated project member drive that actor's sandbox and act through the Slack integration. Prompt To Fix With AIDo not authorize arbitrary team members solely from Task.origin_product. For Slack callback endpoints, authenticate a narrowly scoped sandbox/service capability. For browser/API control, require a verified mapping between the authenticated PostHog user and the Slack actor authorized for this mapped thread (and, if intended, validate Slack channel membership), then preserve the existing task_control_q gate for all other users. Add coverage that a same-team user who is not the mapped/authorized Slack participant cannot obtain a connection token, command/cancel a run, or relay a message.Severity: high | Confidence: 92% | React with 👍 if useful or 👎 if not
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High: Team-wide sandbox access This exception applies to every caller of this gate, not only callbacks from the current Slack actor. Any authenticated team member who obtains a Slack task and run ID can now read its logs and artifacts, control or cancel the run, and call
Comment on lines
+1796
to
+1797
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Slack-origin exception only patches one of two run-control gates, leaving run creation still creator-onlyWhy we think it's a valid issue
Issue descriptionThe new Suggested fixConfirm whether run creation for Slack-originated tasks by a non-creator teammate is an intentionally out-of-scope case (if so, narrow the docstring's 'the runs API mirrors that' claim to say which actions are covered) or apply the same origin-based exception to Prompt to fix with AI (copy-paste)
Comment on lines
+1796
to
+1797
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Slack-origin OR clause is unconditional on
|
||
| return task_filter.exists() | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The task creation API accepts
origin_product=SLACK, while this condition treats that value as proof that every teammate may control the run. A user can therefore create an ordinary task labeled as Slack-originated and give all team members access to run data and mutating actions such as cancellation, log append, message relay, and status updates. Slack origin must be assigned or verified by a server-controlled path before it grants this access.Rule Used: When implementing new features, ensure that owners... (source)
Learned From
PostHog/posthog#31236
Prompt To Fix With AI