Skip to content

feat(agentex): run Slack turns as the invoking user, and the flow to link them - #410

Open
michael-chou359 wants to merge 3 commits into
mainfrom
mc/slack-user-scoped-identity
Open

feat(agentex): run Slack turns as the invoking user, and the flow to link them#410
michael-chou359 wants to merge 3 commits into
mainfrom
mc/slack-user-scoped-identity

Conversation

@michael-chou359

@michael-chou359 michael-chou359 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What

Part of the event-driven agents work, and the half of it that changes behavior.
#409 added storage for linking an external chat identity to an SGP user; nothing
consumed it. This wires it up and adds the flow that creates the links.

Before this, every Slack-originated turn ran as a single shared bot service account.
The agent acted with one fixed identity regardless of who asked, and its tools could
only ever reach what that bot could reach. After this, when the person who sent the
message has linked their account, the turn runs as them — so their own connected
integrations (Notion, Linear, …) resolve.

Unlinked users are unaffected. No link means the existing bot path, unchanged.

The linking flow

The mapping can't be derived — nothing in a webhook says anything about SGP. So it's
established in the one moment where both identities are authenticated at once, with a
server-side nonce tying two requests together.

1.  Slack event arrives
      Slack's HMAC proves the Slack side — only Slack could have signed it.
      We hold a trusted (team_id, user_id) and no idea who that is in SGP.
               │
2.  park it in a nonce → single-use, short TTL          link_nonce_service
      {provider, team_id, external_user_id, display_name, pending_turn}
      The browser receives only an opaque token.
               │
3.  user clicks → GET /integrations/slack/link?nonce=…  integrations.py
      NOT auth-whitelisted (unlike /slack), so the auth middleware runs and
      the user's own browser session supplies the SGP side.
               │
4.  both halves in ONE request → confirmation page naming each identity
               │
5.  POST → mint a key as that user  → encrypt + store → burn the nonce
               │
6.  every later event: resolve → decrypt → forward      slack_gateway_use_case
      the agent's tools now act as that person

Why a nonce rather than ids in the URL. A query string is user-editable. Given
?slack_user=<someone else>, an attacker could click their own link while signed in as
themselves and bind that person's chat identity to their own SGP account — after
which the victim's messages would run as the attacker, using the attacker's
integrations, with the victim's prompts landing in the attacker's account. An opaque
token means nothing in the URL is meaningful, so nothing in it is forgeable.

Change

Gateway (slack_gateway_use_case.py)

  • _turn_identity() resolves (team, user) → link → that user's principal and acting
    headers. Falls back to the bot when there's no usable link.
  • Task naming is now per-user for linked users (slack:{thread_ts}:{sgp_user_id}), so
    two people in one thread don't write into a shared task. Unlinked users keep the
    legacy key, so existing threads keep their history.
  • slack_user_id / sgp_user_id recorded in task_metadata for attribution.

Link flow (new)

  • link_nonce_service.py — create / peek / consume. peek is non-consuming so a
    refresh or a link-prefetching browser doesn't burn the nonce; consume is GETDEL,
    so single-use is enforced by Redis rather than by a read-then-delete race. Requires
    Redis and raises rather than degrading — a nonce store that silently forgets would
    turn into "linking randomly fails."

    At most one live nonce per identity. A nonce is a bearer token, so several live
    ones means several chances for a link to be redeemed by the wrong person — and
    consuming one doesn't invalidate its siblings, so a user who linked successfully
    would still have working tokens pointing at their Slack identity until each expired.
    create() therefore invalidates whatever nonce the identity already held, and
    create_or_reuse() re-sends the live link rather than minting a parallel one.
    claim_send() caps DMs about a given link at 2 (IDENTITY_LINK_MAX_DMS), resetting
    whenever a genuinely new nonce is minted so a new link is never withheld.

    Reuse deliberately does not extend the TTL — otherwise mentioning the agent every
    few minutes keeps one token alive indefinitely, and the bounded lifetime is the point.
    The pending turn is refreshed within the remaining window, so linking answers what
    the user most recently asked.

  • integrations.py — the two routes and the confirmation page. Deliberately under
    /integrations, not /slack: the latter is auth-whitelisted because Slack's
    signature is its auth, and a callback placed there would run unauthenticated, which
    defeats the whole mechanism.

  • adapter_identity_service.py — mints the user's key.

  • identity_link_service.py — cached resolution; uncached credential access.

Details that are easy to get wrong

  • Two API-key issuers exist and only one works. identity-service mints ssk_
    keys, which sgp-secrets accepts; egp-api-backend mints keys that authenticate fine
    against their own issuer and then fail at the vault with an opaque 401. Storing the
    wrong kind produces a link that looks healthy and silently reads nothing, so the
    client refuses anything not ssk_-shaped rather than discovering it mid-turn.
  • Minting needs the user's cookie, not an API key. POST /api-keys is guarded by a
    JWT-cookie guard that rejects x-api-key. That's why the browser leg is load-bearing
    and can't be scripted away — see the testing note below.
  • The identity-service URL has no default. It's deployment-specific, and a wrong
    default would POST a user's forwarded session credentials at whatever answers. Unset
    raises.
  • acting_headers() is never cached, though resolution is (with a shorter TTL for
    negative results). Caching a credential is how you end up serving a revoked one.
  • Every "can't act as them" case returns None, not a partial identity, so the
    gateway falls back explicitly instead of half-assuming a user.
  • The nonce is consumed only after a successful store, so a transient failure leaves
    the link clickable instead of sending the user back to Slack for a new one.
  • A missing encryption key returns 503 and stores nothing, rather than 500-ing straight
    after minting a real credential.

What this does NOT do

Nothing sends anyone a link. On an unlinked mention the gateway still runs as the
bot; it doesn't offer to connect. Step 1 above has no trigger, so today the flow is
only reachable through scripts/dev_seed_link_nonce.py. The DM trigger —
conversations.open + chat.postMessage, an ephemeral in-channel notice, and replaying
the stashed pending_turn after linking — is the next change. claim_send() is the
primitive it will call; nothing calls it yet.

Also still open, and worth settling in that PR rather than here: the confirmation page
is currently the only thing preventing someone from forwarding their own link to
another person, who could then click through and bind the sender's chat identity to
their own SGP account. The page names both identities, which covers a mis-click but
reduces to user vigilance against a deliberate attempt. Comparing the Slack account's
email against the SGP session's email would close it properly — the storage already has
IdentityLinkMethod.EMAIL_MATCH for exactly this.

So this PR is safe to merge but not yet user-visible, and not yet reachable in a way
that would expose the above.

Testing

  • 86 new unit tests — 75 across four new files (link service, nonce service,
    identity-service client, routes) and 11 added to the gateway suite; 156 collected
    across the five touched files. Full CI green (770 unit, 162 integration); ruff clean.

  • Coverage is aimed at the failure modes rather than the happy path: wrong-issuer key
    refused, unauthenticated caller mints nothing, expired/stale nonce refused, failed
    mint leaves the nonce intact, SGP account already linked to a different Slack user,
    unconfigured encryption key reported as 503 rather than 500, and the nonce token
    leaking nothing identifying.

  • For the nonce specifically: supersede-on-recreate, per-identity isolation (including
    the same user in two workspaces), TTL non-extension on reuse, pending-turn refresh
    with and without KEEPTTL, a stale pointer to a vanished nonce, a pointer corrupted
    to name a different identity's live token, the send cap and its reset, and repair of
    a counter left without an expiry by a crash between INCR and EXPIRE.

  • 14 integration tests against a real Redis. The unit tests above use a
    hand-written fake, so they assert our model of Redis rather than Redis — and where
    the model is wrong, they pass while production breaks. These cover the divergences
    that matter: the app runs decode_responses=False so real Redis returns bytes
    where the fake returns str (an undecoded pointer read would silently mint parallel
    tokens); GETDEL atomicity under five concurrent consumes; KEEPTTL genuinely
    preserving an expiry while rewriting the payload — which the fake cannot prove at
    all, and which the "reuse must not extend the lifetime" guarantee rests on; and
    INCR/EXPIRE/TTL returning what the send cap assumes.

    These depend only on redis_url, not isolated_repositories, so they need no
    Postgres or MongoDB — which keeps them at ~3s and runnable where the Mongo image
    won't boot. Verified locally against real Redis, stable across three runs.

⚠️ The live mint is unverified. POST /api-keys requires a genuine browser session,
so no test can stand in for it — everything up to the mint and everything after it is
covered, but the call itself has only been exercised against a stub. It needs one manual
pass against a deployed host before the DM trigger makes this reachable.

For the record: local integration tests error at fixture setup for want of a working
Docker socket. That reproduces identically on a clean checkout of main, so it's
environmental and not from this change; CI runs them.

Deploy notes

  • AGENTEX_CREDENTIAL_ENCRYPTION_KEY must be set, or linking returns 503 (by design —
    it will not store a credential in plaintext).
  • IDENTITY_SERVICE_URL must be set, or minting raises. No default, deliberately.
  • The agentex host must be a sibling subdomain of the SGP host, or the session cookie
    never arrives and step 3 can't resolve anyone.

🤖 Generated with Claude Code

Greptile Summary

This PR adds authenticated Slack-to-SGP identity linking and runs linked Slack turns using each invoking user's principal and delegated credential.

  • Adds a Redis-backed, expiring nonce flow and authenticated confirmation routes.
  • Mints, encrypts, stores, resolves, and delegates user credentials.
  • Separates linked-user task histories and records Slack/SGP attribution metadata.

Confidence Score: 2/5

The PR does not appear safe to merge until nonce redemption gates credential creation and linked task names uniquely scope each Slack conversation.

Concurrent confirmations can still mint and persist credentials before either request claims the nonce, and task names can still merge unrelated Slack conversations that share a timestamp and linked SGP user.

Files Needing Attention: agentex/src/api/routes/integrations.py; agentex/src/domain/use_cases/slack_gateway_use_case.py

Important Files Changed

Filename Overview
agentex/src/api/routes/integrations.py Adds the authenticated confirmation and credential-minting flow; the previously reported late nonce claim remains outstanding.
agentex/src/domain/services/link_nonce_service.py Adds opaque Redis nonce creation, reuse, expiry, send limiting, and atomic consumption primitives.
agentex/src/domain/services/identity_link_service.py Adds cached identity resolution and uncached credential retrieval for delegated turns.
agentex/src/adapters/identity_service/adapter_identity_service.py Adds a constrained identity-service client for minting expiring user-owned credentials.
agentex/src/domain/use_cases/slack_gateway_use_case.py Runs linked turns under invoking-user identities, but the previously reported globally colliding task key remains outstanding.

Sequence Diagram

sequenceDiagram
  participant Slack
  participant Nonce as Redis nonce store
  participant Browser
  participant AgentEx
  participant Identity as Identity service
  participant DB as Identity-link store
  participant Agent

  Slack->>Nonce: Park verified Slack identity
  Nonce-->>Browser: Opaque link token
  Browser->>AgentEx: GET link confirmation with SGP session
  AgentEx->>Nonce: Peek token
  AgentEx-->>Browser: Show Slack and SGP identities
  Browser->>AgentEx: POST confirmation
  AgentEx->>Identity: Mint user-owned API key
  Identity-->>AgentEx: One-time secret
  AgentEx->>DB: Encrypt and store identity link
  AgentEx->>Nonce: Consume token
  Slack->>AgentEx: Later Slack turn
  AgentEx->>DB: Resolve linked user and credential
  AgentEx->>Agent: Dispatch with user principal and delegation headers
Loading

Reviews (3): Last reviewed commit: "test(agentex): nonce integration tests a..." | Re-trigger Greptile

Context used (3)

…link them

Part of the event-driven agents work. Builds on the identity-link storage from
#409, which nothing consumed until now.

Before this, every Slack-originated turn ran as a single shared bot service
account, so the agent acted with one fixed identity no matter who asked. Its
tools could only ever reach whatever that bot could reach. Now, when the person
who sent the message has linked their account, the turn runs as them and their
own connected integrations (Notion, Linear, ...) resolve.

Two pieces:

1. Gateway wiring. `_turn_identity()` resolves the Slack (team, user) to a link
   and returns that user's principal plus acting headers; absent a link it falls
   back to the existing bot behavior, so unlinked users are unaffected. Task
   naming becomes per-user (`slack:{thread_ts}:{sgp_user_id}`) for linked users
   so two people in one thread don't share a task; unlinked users keep the
   legacy key. `slack_user_id` / `sgp_user_id` land in task_metadata for
   attribution.

2. The link flow. The mapping cannot be derived, since nothing in a webhook
   says anything about SGP, so it is established in the one moment where both
   identities are authenticated at once:

     - Slack's HMAC proves the Slack side; that verified identity is parked in a
       single-use, short-TTL nonce (`link_nonce_service`).
     - The user clicks through to `/integrations/slack/link`, which is
       deliberately NOT auth-whitelisted (unlike `/slack`), so the auth
       middleware turns their own browser session into the SGP side.
     - Both halves now present in one request: a confirmation page names each
       identity, then POST mints a key as that user, encrypts and stores it,
       and burns the nonce.

   The nonce matters: with ids in the query string an attacker could bind
   someone else's chat identity to their own SGP account by editing the URL,
   after which the victim's messages would run as the attacker. An opaque token
   means nothing in the URL is forgeable.

Notes on the pieces that are easy to get wrong:

- Keys must come from identity-service, not egp-api-backend. Both mint API keys
  and only the former's are accepted by sgp-secrets; the latter authenticate
  fine against their own issuer and then fail at the vault with an opaque 401.
  A stored key of the wrong kind yields a link that looks healthy and silently
  reads nothing, so the client refuses anything not `ssk_`-shaped.
- Minting requires the user's *cookie*. `POST /api-keys` is guarded by a JWT
  cookie guard that rejects `x-api-key`, which is why the browser leg is
  load-bearing and cannot be scripted away.
- The identity-service URL is read from the environment with no default. It is
  deployment-specific, and a wrong default would POST a user's forwarded
  session credentials at whatever answers.
- `acting_headers()` is never cached, while resolution is (with a shorter TTL
  for negative results). Caching a credential is how you serve a revoked one.
- Every "cannot act as them" case returns None rather than a partial identity,
  so the gateway falls back explicitly instead of half-assuming a user.

Not wired up yet: nothing sends the user a link. On an unlinked mention the
gateway still runs as the bot rather than offering to connect, so the flow is
only reachable via scripts/dev_seed_link_nonce.py. The DM trigger (plus its
rate-limiting and replay of the stashed turn) is the next change.

The live mint is also unverified: it needs a real browser session against a
deployed host, which no test can stand in for. Everything up to the mint, and
everything after it, is covered.

Testing: 69 new unit tests -- 58 across four new files (link service, nonce
service, identity-service client, routes) and 11 added to the gateway suite.
139 pass across the five touched test files; the full unit suite passes. Local
integration tests error at fixture setup for want of a working Docker socket,
which reproduces identically on a clean checkout of main.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@michael-chou359
michael-chou359 requested a review from a team as a code owner August 25, 2026 20:44
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown

✱ Stainless preview builds

This PR will update the agentex-sdk SDKs with the following commit messages.

openapi

feat(api): add slack link endpoints and request body to integrations

python

chore(internal): regenerate SDK with no functional changes

typescript

chore(internal): regenerate SDK with no functional changes

Edit this comment to update them. They will appear in their respective SDK's changelogs.

agentex-sdk-openapi studio · code · diff

Your SDK build had at least one new note diagnostic, which is a regression from the base state.
generate ✅

New diagnostics (4 note)
💡 Schema/IsAmbiguous: Missing type for schema
💡 Schema/IsAmbiguous: Missing type for schema
💡 Endpoint/NotConfigured: Skipped endpoint because it's not in your Stainless config: `get /integrations/slack/link`
💡 Endpoint/NotConfigured: Skipped endpoint because it's not in your Stainless config: `post /integrations/slack/link`
agentex-sdk-typescript studio · code · diff

Your SDK build had at least one new note diagnostic, which is a regression from the base state.
generate ⚠️build ⏭️lint ⏭️test ✅

New diagnostics (4 note)
💡 Schema/IsAmbiguous: Missing type for schema
💡 Schema/IsAmbiguous: Missing type for schema
💡 Endpoint/NotConfigured: Skipped endpoint because it's not in your Stainless config: `get /integrations/slack/link`
💡 Endpoint/NotConfigured: Skipped endpoint because it's not in your Stainless config: `post /integrations/slack/link`
agentex-sdk-python studio · code · diff

Your SDK build had at least one new note diagnostic, which is a regression from the base state.
generate ⚠️build ⏭️ (prev: build ✅) → lint ⏭️ (prev: lint ✅) → test ✅

New diagnostics (4 note)
💡 Schema/IsAmbiguous: Missing type for schema
💡 Schema/IsAmbiguous: Missing type for schema
💡 Endpoint/NotConfigured: Skipped endpoint because it's not in your Stainless config: `get /integrations/slack/link`
💡 Endpoint/NotConfigured: Skipped endpoint because it's not in your Stainless config: `post /integrations/slack/link`

This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push.
If you push custom code to the preview branch, re-run this workflow to update the comment.
Last updated: 2026-08-25 22:16:17 UTC

Comment on lines +202 to +208
try:
secret, actual_expiry = await IdentityServiceClient().mint_user_api_key(
sgp_user_id=sgp_user_id,
name=f"{_KEY_NAME_PREFIX}-{link_request.external_user_id}",
auth_headers=forwardable_headers(get_request_headers_to_forward(request)),
expires_on=expires_on,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security Nonce claimed after key minting

If two submissions of the same nonce overlap, both can pass peek() and mint durable API keys before either request consumes the nonce. The losing request can then fail during persistence or overwrite the first link, leaving an untracked key with no revocation path and potentially returning a server error.

How this was verified: The route mints and stores the key before calling the atomic nonce-consumption operation, and it ignores the consumption result.

Prompt To Fix With AI
This is a comment left during a code review.
Path: agentex/src/api/routes/integrations.py
Line: 202-208

Comment:
**Nonce claimed after key minting**

If two submissions of the same nonce overlap, both can pass `peek()` and mint durable API keys before either request consumes the nonce. The losing request can then fail during persistence or overwrite the first link, leaving an untracked key with no revocation path and potentially returning a server error.

**How this was verified:** The route mints and stores the key before calling the atomic nonce-consumption operation, and it ignores the consumption result.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Cursor Fix in Claude Code Fix in Codex

Comment on lines +764 to +766
if sgp_user_id:
return f"slack:{inbound.thread_ts}:{sgp_user_id}"
return f"slack:{inbound.thread_ts}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Task key omits Slack scope

If the same linked SGP user has threads with the same timestamp in different Slack workspaces or channels, this globally reused name resolves both turns to one task. The later turn can therefore mix prompts, metadata, agent configuration, and account context into an unrelated Slack conversation.

Knowledge Base Used: Task creation and state management

Prompt To Fix With AI
This is a comment left during a code review.
Path: agentex/src/domain/use_cases/slack_gateway_use_case.py
Line: 764-766

Comment:
**Task key omits Slack scope**

If the same linked SGP user has threads with the same timestamp in different Slack workspaces or channels, this globally reused name resolves both turns to one task. The later turn can therefore mix prompts, metadata, agent configuration, and account context into an unrelated Slack conversation.

**Knowledge Base Used:** [Task creation and state management](https://app.greptile.com/scale-ai/-/custom-context/knowledge-base/scaleapi/scale-agentex/-/docs/task-creation-and-state.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Cursor Fix in Claude Code Fix in Codex

michael-chou359 and others added 2 commits August 25, 2026 14:10
Repeated mentions previously minted a fresh nonce each time, leaving a user with
several separately-redeemable links. That is worse than untidy: a nonce is a
bearer token, so whoever holds one gets linked to that provider identity by
signing in as themselves. Several live tokens means several chances for one to be
redeemed by the wrong person, and consuming one did not invalidate its siblings —
so a user who linked successfully still had working tokens pointing at their
Slack identity until each expired on its own.

Now there is at most one live nonce per identity:

- `create()` invalidates whatever nonce that identity already held, via a
  `link_nonce_user:{provider}:{team}:{user}` pointer.
- `create_or_reuse()` returns the live token when there is one, so a second
  mention re-sends the same link instead of minting a parallel one.
- `claim_send()` caps DMs about a given link at 2 (IDENTITY_LINK_MAX_DMS). Past
  the cap the caller should fall back to an ephemeral in-channel notice rather
  than going silent. The counter is cleared whenever a fresh nonce is minted, so
  a genuinely new link is never withheld.

Two deliberate choices:

- Reuse does NOT extend the TTL. Otherwise mentioning the agent every few
  minutes keeps a single token alive indefinitely, and the bounded lifetime is
  the point of the nonce.
- The pending turn IS refreshed within the remaining window, so linking answers
  what the user most recently asked rather than their first attempt. Best-effort:
  without KEEPTTL the earlier question stands, which is worse UX than the newest
  but better than dropping the nonce and forcing a re-link.

The pointer is verified against the request's identity rather than trusted, so a
stale or corrupted pointer cannot hand one user a token that links someone
else's identity.

Testing: 18 new unit tests (30 in the file) covering supersede-on-recreate,
per-identity isolation including the same user in two workspaces, TTL
non-extension, pending-turn refresh with and without KEEPTTL, the stale and
cross-identity pointer cases, the send cap and its reset, and repair of a
counter left without an expiry by a crash between INCR and EXPIRE.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The unit tests for the nonce run against a hand-written _FakeRedis, so they assert
our *model* of Redis rather than Redis. Where the model is wrong the unit tests
pass and production breaks, so these cover the places it could be wrong:

- The app configures decode_responses=False, so real Redis returns bytes where the
  fake returns str. If the pointer read isn't decoded, create_or_reuse silently
  misses and mints a parallel token -- exactly the accumulation the pointer exists
  to prevent, and the fake cannot catch it.
- GETDEL really removing the key, and doing so atomically: five concurrent consumes
  of one token must produce exactly one winner. A read-then-delete would let two
  through and mint two credentials for one link.
- KEEPTTL really preserving an expiry while rewriting the value. This is the
  load-bearing one and the fake cannot prove it at all: if the payload rewrite
  dropped the TTL, someone mentioning the agent every few minutes would keep a
  token alive indefinitely, and the bounded lifetime is the whole point of a nonce.
- INCR/EXPIRE/TTL behaving as the send cap assumes, including real Redis reporting
  -1 for a key with no expiry (the crash-between-INCR-and-EXPIRE repair path) and
  ten concurrent claim_send calls yielding exactly _MAX_SENDS.

Depends only on the redis_url fixture rather than isolated_repositories, which also
starts Postgres and MongoDB. Nothing here needs either, and dropping them keeps the
file fast (~3s) and runnable where the Mongo image won't boot. The Redis container
is session-scoped and shared, so each test namespaces its keys by test name instead
of flushing the database out from under its neighbours.

14 tests, verified locally against a real Redis and stable across three runs.

Note for anyone else running integration tests on Rancher Desktop: scripts/
run_tests.py sets TESTCONTAINERS_HOST_OVERRIDE to the VM address, which the host
cannot reach, so Redis connections time out. Overriding it to 127.0.0.1 and
disabling the testcontainers reaper works:

  TESTCONTAINERS_RYUK_DISABLED=true \
  TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 \
  DOCKER_HOST=unix://$HOME/.rd/docker.sock \
  uv run python -m pytest tests/integration/test_link_nonce_service_redis.py

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant