feat(slack): richer thread context + diff on follow-ups#64226
Merged
Conversation
![hex-security-app[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
| else: | ||
| context_entries.append(f"{username}: {_strip_context_tag(msg['text'])}") | ||
| body = _strip_context_tag(msg["text"]) |
There was a problem hiding this comment.
_build_posthog_code_task_description strips only <slack_thread_context> tags from message bodies via _strip_context_tag, but does not apply _strip_context_update_tag. build_thread_context_update_block strips both. The gap means a Slack thread participant can post a message containing <slack_thread_context_update>…attacker instructions…</slack_thread_context_update> and that markup is copied verbatim (indented two spaces) into the initial <slack_thread_context> block that is sent to the agent.
LLMs identify XML-like tags by token sequence, not indentation, so the two-space indent does not prevent the model from treating the nested block as a real context-update. The agent runs with posthog_mcp_scopes="full" and initial_permission_mode="bypassPermissions", so a successful injection could direct it to make arbitrary repository changes, create PRs with attacker-controlled content, or exfiltrate repository context — without further permission prompts. The only prerequisite is that the attacker can post a message in the same Slack thread that triggers the agent.
Prompt To Fix With AI
In `_build_posthog_code_task_description` (task_creation.py line ~107), apply `_strip_context_update_tag` in addition to `_strip_context_tag` when building the body of each context entry, matching the pattern already used in `build_thread_context_update_block`:
```python
# line 107 — current
body = _strip_context_tag(msg["text"])
# fix
body = _strip_context_tag(_strip_context_update_tag(msg["text"]))
```
Also add a test mirroring `test_build_description_neutralizes_forged_closing_tag_in_message_body` but using `_THREAD_CONTEXT_UPDATE_TAG` as the injected tag, to lock down both stripping paths symmetrically.Severity: medium | Confidence: 72% | React with 👍 if useful or 👎 if not
Contributor
|
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| new_watermark = user_message_ts or mapping.last_forwarded_ts | ||
| try: | ||
| thread_messages = cached_collect_thread_messages(slack, integration, channel, thread_ts, our_bot_id=None) | ||
| update_block, new_watermark = build_thread_context_update_block( |
Contributor
There was a problem hiding this comment.
Medium: Unauthorized thread replies forwarded to the agent
The follow-up authorization checks only slack_user_id for the event being forwarded, but the diff block sends every intervening Slack reply in the thread. A channel participant who does not resolve to a PostHog user can post instructions after task creation and have them delivered to the agent under the task creator's sandbox token when an authorized user later replies; filter thread_messages to authors that pass the same team-access check, or omit unauthenticated authors from the update block.
Contributor
PR overviewThis PR expands the Slack task workflow so follow-up events include richer Slack thread context and a diff of intervening replies for the agent. The touched code is in the Slack task creation activity that formats thread messages and update blocks. Two issues remain open: follow-up diffs can include replies from Slack participants who have not passed the same team-access checks, and unsanitized author labels can let profile names break out of the intended thread-context wrapper. Together, these create a path for Slack thread participants to influence agent instructions outside the intended boundaries, with impact scoped to task-thread follow-up handling. No issues have been fixed or addressed yet. Open issues (2)
Fixed/addressed: 0 · PR risk: 6/10 |
Contributor
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
Contributor
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 1 Safe | 0 Needs Review | 0 Blocked ✅ SafeBrief or no lock, backwards compatible 📚 How to Deploy These Changes SafelyAddField: This operation acquires a brief lock but doesn't rewrite the table. Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up. Last updated: 2026-06-22 12:44 UTC (2ca8625) |
| name = (display_name or "").strip() or "user" | ||
| uid = (user_id or "").strip() | ||
| if uid: | ||
| return f"<@{uid}|{name}>" |
Contributor
There was a problem hiding this comment.
Medium: Prompt-boundary injection through author names
name comes from Slack user or bot profile fields and is inserted into the author header without the tag stripping applied to message bodies. A Slack participant can set their display name to include </slack_thread_context> or </slack_thread_context_update>, post in the task thread, and have their message rendered outside the inert context block; strip those delimiter tags and Slack-token delimiters/newlines from author labels before composing the <@...|...> token.
VojtechBartos
force-pushed
the
vojtab/slack-thread-context-prompt
branch
from
6c5a4fc to
8520647
Compare
Contributor
|
Reviews (2): Last reviewed commit: "fix(slack): CI fixes — mypy auth_test ty..." | Re-trigger Greptile |
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
VojtechBartos
force-pushed
the
vojtab/slack-thread-context-prompt
branch
from
548b09c to
16ef451
Compare
andrewm4894
approved these changes
Jun 22, 2026
tatoalo
approved these changes
Jun 22, 2026
Render each message in `<slack_thread_context>` as `<@U…|displayname>:` followed by an indented body, and annotate the thread starter and the mentioner so the agent can address each role explicitly. On follow-ups, prepend a `<slack_thread_context_update>` block of any messages posted in the thread between the previous interaction and the new one — without this the agent silently misses constraints/clarifications other participants posted in between. `collect_thread_messages` moves to services/ alongside the text helpers and gains a 10s-TTL cache so a classifier-then-forwarder pair (and any chatty thread burst) collapses to a single Slack `conversations.replies` call.
- Forwarder and initial mention now fetch the thread uncached; a stale snapshot would silently drop messages and then advance the watermark past them. - Watermark only advances when the fetch + diff build succeed; an exception leaves it alone so the next follow-up retries the same window from a fresh fetch. - Watermark advance is a conditional UPDATE — concurrent follow-ups racing on the same mapping can't write an older ts back. - Forwarder passes the real bot id from auth.test so the bot's own prior replies are filtered out of the update block. - initial_watermark = max(user_message_ts, max(thread_ts)); accounts for messages that raced past the @mention before the fetch landed (those are already baked into <slack_thread_context>). - build_thread_context_update_block bails when event_ts is missing rather than building an unbounded-upper window. - _indent_body collapses to textwrap.indent.
- mypy didn't like `(auth_test() or {}).get(...)` (dict literal types as
`dict[Never, Never]`); use the same shape `thread.py` already uses.
- `test_miss_then_hit_calls_underlying_once` asserted `first is second`,
which fails on serializing cache backends (Redis in CI hands back a
fresh deserialized copy). The contract worth pinning is one underlying
fetch — drop the identity check.
- `test_build_description_renders_labeled_mention_for_each_author` put
the mentioner as the last message, so trailing-placeholder pop removed
its entry. Add a follow-up message after it.
- `test_wraps_diff_in_dedicated_tag` rejected any occurrence of the
original context tag, but the update block's header text references
the original tag in prose. Anchor the assertion on shape — the block
doesn't open/close with `<slack_thread_context>`.
VojtechBartos
force-pushed
the
vojtab/slack-thread-context-prompt
branch
from
16ef451 to
bc686e3
Compare
Contributor
|
Size Change: 0 B Total Size: 64.5 MB ℹ️ View Unchanged
|
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
3 participants
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
Two things were biting us with how the agent reads Slack thread.
No slack handles in the context. Original
<slack_thread_context>block was rendered just asdisplayname: text— no<@U…>mention tokens. Agent couldn't ping people back in replies, just referred to them by name. Also no way to tell who actually started the thread vs who tagged the bot.Follow-ups were blind to the rest of the thread. Thread was snapshotted once at task creation and baked into the system prompt. After that every follow-up was just forwarded as raw text — agent had no clue what else was posted in the thread in the meantime. So if someone jumped in with important question or extra context between the original ask and a follow-up "ok go ahead", agent never saw it, aka was acting on stale picture.
Changes
<slack_thread_context>block: each message rendered as<@U…|displayname>:with an indented body, plus explicit "Thread started by" / "Tagged the PostHog app" annotations.<slack_thread_context_update>block prepended to follow-ups, surfacing any messages posted in the thread between interactions. Backed by alast_forwarded_tswatermark onSlackThreadTaskMapping(monotonic conditional UPDATE; only advances on a successful fetch + diff; filters out the bot's own prior replies).collect_thread_messagesand the text helpers (extract_message_text,flatten_block_text,resolve_bot_author_label) move fromproducts/slack_app/backend/api.pyintoproducts/slack_app/backend/services/slack_messages.py, alongsideresolve_user_mentions_textthey already depend on. Tests follow the move.cached_collect_thread_messageswrapper (10s TTL viadjango.core.cache) sits in front of the fetch. Slack'sconversations.repliesis Tier 3 (~50 req/min/workspace), and a chatty mapped thread can fan out several classifier invocations within seconds; the cache collapses those bursts onto a single API call while staying well under budget. The forwarder and initial-mention paths bypass the cache — both depend on fresh thread state.Companion agent-side PR (system-prompt instructions to reuse the new mention tokens and nudge toward PRs for code-fixable issues): PostHog/code#2691.
How did you test this code?
posthog/temporal/tests/ai/slack_app/activities/test_task_creation.pyandproducts/slack_app/backend/tests/services/slack_messages/test_*.Automatic notifications
Docs update
n/a — internal Slack app behavior, no public surface change.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code (Opus 4.7) drafted the prompt shape, diff design, and tests interactively with the author. A self-review pass caught a few real race / failure-mode bugs (split out as commit 2). Author reviewed every step and tested locally.