Skip to content

fix(tasks): let team members drive runs of slack-originated tasks - #72590

Merged
VojtechBartos merged 1 commit into
masterfrom
vojtab/slack-run-actor-auth
Jul 22, 2026
Merged

fix(tasks): let team members drive runs of slack-originated tasks#72590
VojtechBartos merged 1 commit into
masterfrom
vojtab/slack-run-actor-auth

Conversation

@VojtechBartos

@VojtechBartos VojtechBartos commented Jul 21, 2026

Copy link
Copy Markdown
Member

Problem

Since #66212, sandbox credentials for Slack task runs are minted for whoever spoke last in the thread instead of the task creator. But the runs API still only lets the creator drive a run (task_control_q), so when anyone else replies in a thread, the sandbox 404s on all its callbacks: the Slack reply (relay_message), the workflow heartbeat (append_log), and the completion signal (status PATCH). The run does the work invisibly, times out after 45 minutes of "inactivity", and the thread goes silent akano reply, no error.

Slack threads are multiplayer by design (#60260: any same-team user can send follow-ups), so the API gate and the credential model disagreed.

Origin: https://posthog.slack.com/archives/C09SK2PAGKF/p1784574152329129

Changes

task_accessible_for_run_view now also allows access when the task is Slack-originated: any team member can drive and watch runs of origin_product == SLACK tasks, mirroring what the Slack side already permits.

How did you test this code?

TestTaskRunSlackTaskTeamControl (4 parameterized cases): a non-creator teammate can PATCH/GET runs of a Slack task (both 404 without the fix, verified against unpatched source), and still 404s on a user_created task. Neighboring auth suites (108 tests) pass.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

None, internal authorization behavior only.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Authored with Claude Code (skills: /improving-drf-endpoints, /writing-tests). Root-caused from production logs: resumed runs with sandbox callbacks failing 404 "Task not found", then completing exactly at the inactivity timeout with no heartbeats and no Slack post. First iteration gated on the run's recorded actor; reworked to the task-origin gate at the DRI's direction since per-message actors make the actor comparison racy.

@VojtechBartos VojtechBartos self-assigned this Jul 21, 2026
Slack threads are multiplayer: any same-team user can steer a task via
thread follow-ups, and the sandbox's credentials are minted for whoever
spoke last. But run-mutating API actions were gated on task_control_q
(creator-only), so a non-creator actor's sandbox 404'd on all callbacks:
relay_message (the reply), append_log (the workflow heartbeat), and the
status PATCH (completion signal). The run did the work invisibly,
starved the workflow of heartbeats, timed out after 45 minutes, and the
thread died silently.

Mirror the Slack-side semantics in task_accessible_for_run_view: runs of
slack-originated tasks are drivable and watchable by any team member.
Task-level control (edit/delete) stays creator-only, and non-slack tasks
are unaffected.
@VojtechBartos
VojtechBartos force-pushed the vojtab/slack-run-actor-auth branch from 391006e to eb818bf Compare July 21, 2026 14:27
@VojtechBartos VojtechBartos changed the title fix(tasks): let a run's recorded slack actor drive its run fix(tasks): let team members drive runs of slack-originated tasks Jul 21, 2026
@VojtechBartos
VojtechBartos requested a review from a team July 21, 2026 14:36
@VojtechBartos
VojtechBartos marked this pull request as ready for review July 21, 2026 14:36
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 21, 2026 14:36
@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Security Review

The new authorization exception relies on a client-writable task origin. An API caller can label a task as Slack-originated and opt it into team-wide run access without verified Slack provenance.

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
products/tasks/backend/facade/api.py:1797
**Client-Controlled Origin Grants Run Access**

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.

Reviews (1): Last reviewed commit: "fix(tasks): let team members drive runs ..." | Re-trigger Greptile

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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Client-Controlled Origin Grants Run Access

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
This is a comment left during a code review.
Path: products/tasks/backend/facade/api.py
Line: 1797

Comment:
**Client-Controlled Origin Grants Run Access**

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](https://app.greptile.com/posthog-org-19734/-/custom-context?memory=9655b466-451a-401a-9ba0-5bf3e7b7f9f8))

**Learned From**
[PostHog/posthog#31236](https://github.com/PostHog/posthog/pull/31236)

How can I resolve this? If you propose a fix, please make it concise.

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))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Restrict Slack run control to authenticated thread participants

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 AI
Do 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

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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 connection_token to mint a write-capable 24-hour token for direct access to another user's sandbox. Restrict the exception to the user recorded as the run's current Slack actor, or authenticate the sandbox callback through a dedicated run-scoped capability while retaining task_control_q for user-facing control endpoints.

@veria-ai

veria-ai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

PR overview

This pull request updates task run handling so team members can drive or control runs for tasks that originated from Slack. The touched backend API logic adjusts the authorization gate around Slack-originated task runs and sandbox/run control endpoints.

There is one open security issue in the authorization change: the current exception appears to apply broadly to authenticated team members rather than only to the intended Slack actor or a scoped callback capability. As a result, a team member with the relevant Slack task and run identifiers could access another user's run logs/artifacts, control or cancel the run, and mint a temporary write-capable sandbox token. No issues have been addressed yet, so the PR still needs a targeted authorization tightening before it is in a good security state.

Open issues (1)

Fixed/addressed: 0 · PR risk: 7/10

@posthog

posthog Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

🦔 ReviewHog reviewed this pull request

Found 0 must fix, 0 should fix, 2 consider.

Published 2 findings (view the review).

@posthog

posthog Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReviewHog Report

Changes

Issues: 2 issues

Files (1)
  • products/tasks/backend/facade/api.py

Comment on lines +1796 to +1797
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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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-only

consider bug

Why we think it's a valid issue
  • Checked: Both run-control gates and every run-mutating action's call path in products/tasks/backend/presentation/views/api.py and .../facade/api.py, plus task_control_q/TEAM_VISIBLE_ORIGIN_PRODUCTS in products/tasks/backend/visibility.py.
  • Found: The asymmetry is real and unconditional. After the fix, task_accessible_for_run_view(for_control=True) returns true for any teammate on a Slack task (facade api.py:1797, | Q(origin_product=SLACK)), but create (views/api.py:925-926) bypasses that gate entirely — it calls self._task_id() then bootstrap_task_run_get_task_for_run_control (facade api.py:2991 → 1764), which filters on bare task_control_q(user_id). SLACK is absent from TEAM_VISIBLE_ORIGIN_PRODUCTS (visibility.py:22-27), so for a non-creator that Q is false → 404. Every other control action (append_log, relay_message, set_output, start, cancel, command, resume_in_cloud, status PATCH) routes through _ensure_task_accessible() and is covered. So create is the single run-control action left uncovered, and the docstring's claim (facade api.py:1790) that 'the runs API … lets team members drive … Slack tasks' runs' overreaches for the creation path.
  • Impact: The premise is confirmed, but the practical reach is narrow. The production failure this PR targets is the sandbox callbacks (relay/append_log/completion PATCH) — all fixed. Run creation in the multiplayer Slack resume flow happens internally via task.create_run() in the warm service (logic/services/warm.py:170), not through the HTTP create endpoint, and the sandbox never calls create. The only way to hit the gap is a non-creator manually POSTing a fresh run on a Slack task via UI/CLI/API — a real but secondary path, and the reviewer's own suggestion flags it as possibly-intentional scope. It is a genuine, directly-related auth/doc inconsistency worth recording, not noise, but it does not carry the urgency of the callback bug the PR actually fixes.
  • Priority: Lowering should_fix → consider: the motivating incident is fully addressed, the create path is outside the sandbox-callback set that drove the change, and whether run-creation-by-non-creator is in scope is a genuine design question (narrow the docstring vs. extend the carve-out) rather than a blocking defect.
Issue description

The new Q(origin_product=Task.OriginProduct.SLACK) exception is added only inside task_accessible_for_run_view (lines 1796-1797). But run control for this task has a second, structurally parallel gate that was not touched: _get_task_for_run_control (line 1762-1764) still filters on the bare task_control_q(user_id) with no Slack carve-out. _get_task_for_run_control backs bootstrap_task_run (line 2991), which is what TaskRunViewSet.create calls to create a new run (products/tasks/backend/presentation/views/api.py:925-941) — and notably, that view action never calls _ensure_task_accessible()/task_accessible_for_run_view() at all, so it never gets the fix applied even indirectly. The result: a non-creator teammate who can now GET/PATCH an existing Slack-task run (per the new exception and this PR's own test suite) will still get a 404 the moment they try to create/bootstrap a new run on that same Slack-originated task, contradicting the docstring's claim (lines 1790-1792) that 'the runs API mirrors that and lets team members drive and watch Slack tasks' runs' — that mirroring is incomplete for the run-creation path.

Suggested fix

Confirm 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 _get_task_for_run_control (e.g. Task.objects.filter(id=task_id, team_id=team_id).filter(task_control_q(user_id) | Q(origin_product=Task.OriginProduct.SLACK)).first()) so bootstrap_task_run behaves consistently with task_accessible_for_run_view(for_control=True).

Prompt to fix with AI (copy-paste)
## Context
@products/tasks/backend/facade/api.py#L1796-1797
@products/tasks/backend/facade/api.py#L1762-1764

<issue_description>
The new `Q(origin_product=Task.OriginProduct.SLACK)` exception is added only inside `task_accessible_for_run_view` (lines 1796-1797). But run control for this task has a second, structurally parallel gate that was not touched: `_get_task_for_run_control` (line 1762-1764) still filters on the bare `task_control_q(user_id)` with no Slack carve-out. `_get_task_for_run_control` backs `bootstrap_task_run` (line 2991), which is what `TaskRunViewSet.create` calls to create a new run (`products/tasks/backend/presentation/views/api.py:925-941`) — and notably, that view action never calls `_ensure_task_accessible()`/`task_accessible_for_run_view()` at all, so it never gets the fix applied even indirectly. The result: a non-creator teammate who can now GET/PATCH an existing Slack-task run (per the new exception and this PR's own test suite) will still get a 404 the moment they try to create/bootstrap a new run on that same Slack-originated task, contradicting the docstring's claim (lines 1790-1792) that 'the runs API mirrors that and lets team members drive and watch Slack tasks' runs' — that mirroring is incomplete for the run-creation path.
</issue_description>

<issue_validation>
- **Checked:** Both run-control gates and every run-mutating action's call path in `products/tasks/backend/presentation/views/api.py` and `.../facade/api.py`, plus `task_control_q`/`TEAM_VISIBLE_ORIGIN_PRODUCTS` in `products/tasks/backend/visibility.py`.
- **Found:** The asymmetry is real and unconditional. After the fix, `task_accessible_for_run_view(for_control=True)` returns true for *any* teammate on a Slack task (facade api.py:1797, `| Q(origin_product=SLACK)`), but `create` (views/api.py:925-926) bypasses that gate entirely — it calls `self._task_id()` then `bootstrap_task_run` → `_get_task_for_run_control` (facade api.py:2991 → 1764), which filters on bare `task_control_q(user_id)`. SLACK is absent from `TEAM_VISIBLE_ORIGIN_PRODUCTS` (visibility.py:22-27), so for a non-creator that Q is false → 404. Every *other* control action (append_log, relay_message, set_output, start, cancel, command, resume_in_cloud, status PATCH) routes through `_ensure_task_accessible()` and is covered. So `create` is the single run-control action left uncovered, and the docstring's claim (facade api.py:1790) that 'the runs API … lets team members drive … Slack tasks' runs' overreaches for the creation path.
- **Impact:** The premise is confirmed, but the practical reach is narrow. The production failure this PR targets is the sandbox callbacks (relay/append_log/completion PATCH) — all fixed. Run creation in the multiplayer Slack resume flow happens internally via `task.create_run()` in the warm service (`logic/services/warm.py:170`), not through the HTTP `create` endpoint, and the sandbox never calls `create`. The only way to hit the gap is a non-creator manually POSTing a fresh run on a Slack task via UI/CLI/API — a real but secondary path, and the reviewer's own suggestion flags it as possibly-intentional scope. It is a genuine, directly-related auth/doc inconsistency worth recording, not noise, but it does not carry the urgency of the callback bug the PR actually fixes.
- **Priority:** Lowering should_fix → consider: the motivating incident is fully addressed, the create path is outside the sandbox-callback set that drove the change, and whether run-creation-by-non-creator is in scope is a genuine design question (narrow the docstring vs. extend the carve-out) rather than a blocking defect.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Confirm 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 `_get_task_for_run_control` (e.g. `Task.objects.filter(id=task_id, team_id=team_id).filter(task_control_q(user_id) | Q(origin_product=Task.OriginProduct.SLACK)).first()`) so `bootstrap_task_run` behaves consistently with `task_accessible_for_run_view(for_control=True)`.
</potential_solution>

Comment on lines +1796 to +1797
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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slack-origin OR clause is unconditional on for_control, so TaskRunLivingArtifactViewSet never gates its mutating Slack-posting actions

consider security

Why we think it's a valid issue
  • Checked: TaskRunLivingArtifactViewSet._ensure_task_accessible (views/api.py:2122-2131), its create (2173-2183) and edit (2221-2244, task:write) actions, the facade create_task_run_living_artifact/edit_task_run_living_artifact (facade api.py:2458-2522), and the parallel relay_message path that the PR also opens.
  • Found: The mechanical claim holds. This viewset calls task_accessible_for_run_view(..., bypass_visibility=...) with no for_control arg → always the task_visibility_q | Q(origin_product=SLACK) branch, and since the OR is unconditional, every SLACK-origin task passes regardless of channel. Its mutating create/edit inherit this, and the facade functions only re-scope by team/task/run via _get_visible_run with no actor check before delivering into the mapped Slack thread. So yes, post-PR any team member with task:write can create/edit living artifacts on any SLACK task, including non-public-channel ones that task_visibility_q alone would have blocked.
  • Impact (why not must_fix): This is functionally the same deliberate widening the PR already makes for relay_message (also a Slack-thread write, opened for any team member on SLACK tasks via for_control=True+carve-out). Living-artifact create/edit is part of the same sandbox-callback delivery family that must work for the last-speaker actor; gating it back out would reintroduce the exact 404 the PR fixes for the delivery path. It adds no new trust decision beyond relay_message. Everything stays team-scoped (team_id enforced throughout — no cross-tenant reach), and practical reachability by a human is low: the feed (TaskViewSet, unchanged) does not expose non-visible Slack tasks, so a non-creator can't discover the task UUID needed to reach these endpoints. The genuinely over-broad edge (DM/personal-thread Slack tasks, where the multiplayer premise doesn't hold) is the general 'carve-out keyed on origin, not on verified thread membership' concern raised elsewhere — not unique to this viewset — and is low-severity within a trusted team.
  • Found: The 'for_control never threaded' aspect is pre-existing: create/edit always defaulted to for_control=False, and this PR only adds the OR clause; for SLACK tasks the carve-out is unconditional so for_control is moot anyway.
  • Priority: Lowering must_fix → consider: a real, directly-related, test-uncovered distinct call site worth a maintainer confirming is intended, but not a critical hole — team-scoped, low reachability, and the same intended Slack-delivery widening as relay_message rather than a separately dangerous surface.
Issue description

The new scope_q | Q(origin_product=Task.OriginProduct.SLACK) clause is applied to whichever scope_q the caller selected via for_control, but it does not itself distinguish read from control access — it widens both branches equally. TaskRunLivingArtifactViewSet._ensure_task_accessible (products/tasks/backend/presentation/views/api.py:2122-2129) calls task_accessible_for_run_view(task_id, self.team_id, user_id, bypass_visibility=bypass_visibility) and never passes for_control at all, so it always resolves to for_control=False — i.e. always the task_visibility_q(user_id) | Q(origin_product=SLACK) branch — even for its create (products/tasks/backend/presentation/views/api.py:2173-2181) and edit (products/tasks/backend/presentation/views/api.py:2222+) actions. Those two actions create/edit a task run's 'living artifact', and per the viewset's own docstrings/description ('Slack canvases/messages/files... Slack adapters deliver into the mapped Slack thread') and the facade implementation (create_task_run_living_artifact/edit_task_run_living_artifact, products/tasks/backend/facade/api.py:2458-2521, calling into products/tasks/backend/logic/services/living_artifacts.py), this posts/edits content directly into the mapped Slack thread. Before this PR, a non-creator team member could only reach these actions if task_visibility_q already held (creator, legacy-unowned, a TEAM_VISIBLE_ORIGIN_PRODUCTS origin, or a public-channel task) — Slack tasks are deliberately excluded from TEAM_VISIBLE_ORIGIN_PRODUCTS and are not necessarily public-channel (a Slack DM/personal thread task would not satisfy task_visibility_q for a non-creator). After this PR, because the OR clause is unconditional and this call site never sets for_control=True, literally any team member can now create/edit living artifacts — i.e. post or rewrite messages/canvases/files in another user's mapped Slack thread — for any task merely labeled origin_product=SLACK, regardless of channel type or actual thread participation. This is a distinct call site and mechanism from the run-control endpoints (TaskRunViewSet) already flagged elsewhere in this review: here the vulnerability is compounded by for_control never being threaded through at all, so even a hypothetical fix that keys the SLACK exception off for_control in task_accessible_for_run_view would not close this hole unless this viewset also starts passing for_control=True for create/edit. TestTaskRunSlackTaskTeamControl (products/tasks/backend/tests/test_api.py:9949) only exercises PATCH/GET on TaskRunViewSet and does not cover this viewset, so the gap has no regression coverage either.

Suggested fix

Have TaskRunLivingArtifactViewSet._ensure_task_accessible pass for_control=True for its mutating create/edit actions (mirroring TaskRunViewSet._ensure_task_accessible's for_control=not is_read_only pattern), and resolve the origin-trust question raised elsewhere in this review (verifying origin_product == SLACK via a server-controlled signal, e.g. presence of a mapped Slack channel/thread reference, rather than the raw client-settable field) before relying on it to gate a Slack-thread-writing action.

Prompt to fix with AI (copy-paste)
## Context
@products/tasks/backend/facade/api.py#L1796-1797

<issue_description>
The new `scope_q | Q(origin_product=Task.OriginProduct.SLACK)` clause is applied to whichever `scope_q` the caller selected via `for_control`, but it does not itself distinguish read from control access — it widens both branches equally. `TaskRunLivingArtifactViewSet._ensure_task_accessible` (products/tasks/backend/presentation/views/api.py:2122-2129) calls `task_accessible_for_run_view(task_id, self.team_id, user_id, bypass_visibility=bypass_visibility)` and never passes `for_control` at all, so it always resolves to `for_control=False` — i.e. always the `task_visibility_q(user_id) | Q(origin_product=SLACK)` branch — even for its `create` (products/tasks/backend/presentation/views/api.py:2173-2181) and `edit` (products/tasks/backend/presentation/views/api.py:2222+) actions. Those two actions create/edit a task run's 'living artifact', and per the viewset's own docstrings/description ('Slack canvases/messages/files... Slack adapters deliver into the mapped Slack thread') and the facade implementation (`create_task_run_living_artifact`/`edit_task_run_living_artifact`, products/tasks/backend/facade/api.py:2458-2521, calling into `products/tasks/backend/logic/services/living_artifacts.py`), this posts/edits content directly into the mapped Slack thread. Before this PR, a non-creator team member could only reach these actions if `task_visibility_q` already held (creator, legacy-unowned, a `TEAM_VISIBLE_ORIGIN_PRODUCTS` origin, or a public-channel task) — Slack tasks are deliberately excluded from `TEAM_VISIBLE_ORIGIN_PRODUCTS` and are not necessarily public-channel (a Slack DM/personal thread task would not satisfy `task_visibility_q` for a non-creator). After this PR, because the OR clause is unconditional and this call site never sets `for_control=True`, literally any team member can now `create`/`edit` living artifacts — i.e. post or rewrite messages/canvases/files in another user's mapped Slack thread — for any task merely labeled `origin_product=SLACK`, regardless of channel type or actual thread participation. This is a distinct call site and mechanism from the run-control endpoints (`TaskRunViewSet`) already flagged elsewhere in this review: here the vulnerability is compounded by `for_control` never being threaded through at all, so even a hypothetical fix that keys the `SLACK` exception off `for_control` in `task_accessible_for_run_view` would not close this hole unless this viewset also starts passing `for_control=True` for `create`/`edit`. `TestTaskRunSlackTaskTeamControl` (products/tasks/backend/tests/test_api.py:9949) only exercises PATCH/GET on `TaskRunViewSet` and does not cover this viewset, so the gap has no regression coverage either.
</issue_description>

<issue_validation>
- **Checked:** `TaskRunLivingArtifactViewSet._ensure_task_accessible` (views/api.py:2122-2131), its `create` (2173-2183) and `edit` (2221-2244, `task:write`) actions, the facade `create_task_run_living_artifact`/`edit_task_run_living_artifact` (facade api.py:2458-2522), and the parallel `relay_message` path that the PR also opens.
- **Found:** The mechanical claim holds. This viewset calls `task_accessible_for_run_view(..., bypass_visibility=...)` with no `for_control` arg → always the `task_visibility_q | Q(origin_product=SLACK)` branch, and since the OR is unconditional, every SLACK-origin task passes regardless of channel. Its mutating `create`/`edit` inherit this, and the facade functions only re-scope by team/task/run via `_get_visible_run` with no actor check before delivering into the mapped Slack thread. So yes, post-PR any team member with `task:write` can create/edit living artifacts on any SLACK task, including non-public-channel ones that `task_visibility_q` alone would have blocked.
- **Impact (why not must_fix):** This is functionally the *same* deliberate widening the PR already makes for `relay_message` (also a Slack-thread write, opened for any team member on SLACK tasks via `for_control=True`+carve-out). Living-artifact create/edit is part of the same sandbox-callback delivery family that must work for the last-speaker actor; gating it back out would reintroduce the exact 404 the PR fixes for the delivery path. It adds no new trust decision beyond relay_message. Everything stays team-scoped (`team_id` enforced throughout — no cross-tenant reach), and practical reachability by a human is low: the feed (`TaskViewSet`, unchanged) does not expose non-visible Slack tasks, so a non-creator can't discover the task UUID needed to reach these endpoints. The genuinely over-broad edge (DM/personal-thread Slack tasks, where the multiplayer premise doesn't hold) is the general 'carve-out keyed on origin, not on verified thread membership' concern raised elsewhere — not unique to this viewset — and is low-severity within a trusted team.
- **Found:** The 'for_control never threaded' aspect is pre-existing: create/edit always defaulted to `for_control=False`, and this PR only adds the OR clause; for SLACK tasks the carve-out is unconditional so `for_control` is moot anyway.
- **Priority:** Lowering must_fix → consider: a real, directly-related, test-uncovered distinct call site worth a maintainer confirming is intended, but not a critical hole — team-scoped, low reachability, and the same intended Slack-delivery widening as relay_message rather than a separately dangerous surface.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Have `TaskRunLivingArtifactViewSet._ensure_task_accessible` pass `for_control=True` for its mutating `create`/`edit` actions (mirroring `TaskRunViewSet._ensure_task_accessible`'s `for_control=not is_read_only` pattern), and resolve the origin-trust question raised elsewhere in this review (verifying `origin_product == SLACK` via a server-controlled signal, e.g. presence of a mapped Slack channel/thread reference, rather than the raw client-settable field) before relying on it to gate a Slack-thread-writing action.
</potential_solution>

@VojtechBartos
VojtechBartos merged commit 184b4c3 into master Jul 22, 2026
381 checks passed
@VojtechBartos
VojtechBartos deleted the vojtab/slack-run-actor-auth branch July 22, 2026 12:27
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 22, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-22 13:00 UTC Run
prod-us ✅ Deployed 2026-07-22 13:19 UTC Run
prod-eu ✅ Deployed 2026-07-22 13:29 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants