feat(agentex): identity-link storage with encrypted credentials at rest - #409
Merged
Conversation
Part of the event-driven agents work (the Slack and Linear gateways). Those
gateways dispatch every turn as a single shared bot service account, so an
agent acts with one fixed identity regardless of who asked. This is the
storage groundwork for running a turn as the invoking human instead.
Adds the mapping from an external chat identity (Slack/Linear user) to an SGP
user, along with that user's SGP API key held encrypted.
Why hold a credential at all: reading a user's connected integrations from
the secrets service requires authenticating AS that user — the owner of a
user-scoped secret is derived from the calling identity and cannot be passed
as a parameter. An event-driven turn has no user session (a webhook is a
server-to-server POST, so there is no caller credential to forward), so
acting as a person requires a stored credential belonging to them. Keys are
minted with an expiry so what's held is bounded.
identity_links
provider + external_team_id + external_user_id -> sgp_user_id/account
credential_ciphertext, credential_expires_at -> nullable
linked_via -> explicit|email_match|manual
revoked_at -> tombstone, not delete
`provider` is a discriminator from the start rather than a Slack-only table,
since both gateways need this and retrofitting one later is worse.
Two partial unique indexes, scoped to active rows so revoked rows stay for
audit: one active SGP identity per external identity (the mapping must be a
function), and one active external identity per SGP user within a workspace
(two accounts pointing at one user is the shape an identity hijack takes).
Encryption uses Fernet (authenticated, so a tampered ciphertext fails to
decrypt rather than yielding garbage we'd then present as a bearer token).
The key comes from AGENTEX_CREDENTIAL_ENCRYPTION_KEY, delivered with the
other platform secrets and deliberately not stored in this database, so a
database dump alone is not sufficient to use the credentials. It fails
closed: a missing or malformed key raises rather than writing plaintext.
Deliberate asymmetry in the repository: the credential is reachable through
exactly one method, and IdentityLinkEntity carries `has_credential: bool`
rather than the key. Entities get logged, cached and serialized, so the
fewer paths that can place key material in one, the better.
Nothing consumes this yet — no behavior changes. The gateway wiring and the
link flow follow separately.
Testing:
- 13 unit tests for encryption, focused on failing loudly (missing key,
malformed key, tampered ciphertext, key rotated out from under stored rows)
- 17 repository integration tests covering the partial indexes, the
supersede/revoke transaction and ciphertext round-tripping. These collect
cleanly but could not be executed locally: the shared fixture starts a
MongoDB container that will not run in this environment (mongod exits 51).
The same behaviors were verified by hand against a live Postgres, including
the regression that superseding a link used to leave a still-valid
credential on the tombstoned row.
- Migration linter reports no findings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment on lines
+159
to
+160
| credential: str | None = None, | ||
| credential_expires_at: datetime | None = None, |
There was a problem hiding this comment.
Unbounded credential expiration
credential and credential_expires_at are independent optional arguments, so callers can store a credential with a null expiration; credential_is_usable then treats that credential as indefinitely usable, weakening the intended bounded credential lifecycle.
Knowledge Base Used: Agent access and API keys
Prompt To Fix With AI
This is a comment left during a code review.
Path: agentex/src/domain/repositories/identity_link_repository.py
Line: 159-160
Comment:
**Unbounded credential expiration**
`credential` and `credential_expires_at` are independent optional arguments, so callers can store a credential with a null expiration; `credential_is_usable` then treats that credential as indefinitely usable, weakening the intended bounded credential lifecycle.
**Knowledge Base Used:** [Agent access and API keys](https://app.greptile.com/scale-ai/-/custom-context/knowledge-base/scaleapi/scale-agentex/-/docs/agent-access-and-api-keys.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
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
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.
What
Part of the event-driven agents work — the Slack and Linear gateways that let a
platform-side webhook invoke an agent. Those gateways ship today, but every turn they
dispatch runs as a single shared bot service account, so the agent acts with one fixed
identity no matter who asked. This PR is the storage groundwork for running a turn as
the invoking human instead.
It adds the mapping from an external chat identity (Slack/Linear user) to an SGP user,
plus that user's SGP API key held encrypted at rest.
Why hold a credential at all. Reading a user's connected integrations (Notion,
Linear, …) from the secrets service requires authenticating as that user — the
owner of a user-scoped secret is derived from the calling identity and cannot be passed
as a parameter. An event-driven turn has no user session: a webhook is a
server-to-server POST, so there is no caller credential to forward. Making an agent act
as the person who asked therefore requires a stored credential belonging to them. Keys
are minted with an expiry so what's held is bounded rather than indefinite.
Nothing consumes this yet — no behavior changes. Gateway wiring and the link flow
follow in a separate PR.
The linking flow this enables
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 simultaneously,
with a server-side nonce tying two requests together.
[this PR]marks what landshere; everything else is the follow-up.
Why a nonce and not 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 asthemselves 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. Parking the
verified identity server-side and handing out an opaque token means nothing in the URL
is meaningful, so nothing in it is forgeable.
Change
provideris a discriminator from the start rather than a Slack-only table, since bothgateways need this and retrofitting one later is worse than carrying a column now.
revoked_at IS NULLso revoked rows remainfor audit:
one user is the shape an identity hijack takes, and no legitimate case needs it
credential_encryption.py— Fernet, so a tampered ciphertext fails to decryptinstead of yielding garbage we'd then present downstream as a bearer token. The key
comes from
AGENTEX_CREDENTIAL_ENCRYPTION_KEY, delivered with the other platformsecrets and deliberately not stored in this database: a database dump alone is not
sufficient to use the credentials. Fails closed — a missing or malformed key raises
rather than writing plaintext.
method (
get_credential), returning a bare string. Every other read returnsIdentityLinkEntity, which carrieshas_credential: booland no key. Entities getlogged, cached and serialized, so the fewer code paths that can put key material in
one, the better.
upsert_linkrevokes-then-inserts in one transaction (auditable history, andre-linking is idempotent against the partial index rather than racing it), and clears
the superseded row's ciphertext.
cryptographypromoted to a direct dependency. It was already present transitively,but the correctness of a stored credential shouldn't depend on another package
continuing to pull it in.
Migration safety
Schema-only, and there is nothing to backfill even in principle — the mapping can't be
derived from existing data, it only comes into being when a user authenticates both
identities in one moment.
IF NOT EXISTS— re-running isa no-op.
CONCURRENTLYinside anautocommit_block(). The table is empty hereso a plain build would also be safe; this keeps it consistent with the repo rule and
removes the question.
sgp_user_id/sgp_account_idbelong to SGP and the provider idsare external, so there's nothing local to reference.
Testing
13 unit tests for encryption, focused on failing loudly: missing key, blank key,
malformed key, tampered ciphertext, and a key rotated out from under existing rows
(which must surface as "re-link needed", never as "no credential").
17 repository integration tests covering the partial indexes, the supersede/revoke
transaction, and ciphertext round-tripping. These exist because the behaviors that
matter here are enforced by the schema and by SQL rather than by Python — a mock-based
test would assert the intent instead of the constraint.
Includes the regression that superseding a link previously tombstoned the old row but
left its ciphertext intact, so every re-link accumulated another still-valid key at
rest (
test_superseding_a_link_clears_the_old_credential). That one was found by alive round-trip, not by a mock.
These could not be run on my machine — the shared
isolated_repositoriesfixturestarts a MongoDB container that won't boot locally (
mongodexits 51 under RancherDesktop) — so they were verified by hand against a live Postgres and then confirmed in
CI: 17/17 passed against CI's Postgres, no skips.
ruffclean; pre-commit hooks pass. Full CI green (unit 679 passed, integration 162passed), with
identity_link_repository.pyandcredential_encryption.pyboth at100% line coverage.
Before anything uses this
AGENTEX_CREDENTIAL_ENCRYPTION_KEYmust be present in the environment, or everycredential path raises (by design — it will not silently store plaintext). Generate
with:
🤖 Generated with Claude Code