Skip to content

Resolve actor identity once and store it explicitly #2439

Open

Description

@sentry-junior

Junior's runtime Actor is typed (chat/actor.ts), but stored Conversation state flattens it to a Slack-only shape. Later write and read paths then guess the Identity again from platform strings, the Source, the Location, and id patterns. Each guess can drift, and they caused #2431 and its follow-up audit.

Current state

End state

The source of truth is the merged spec on main: openspec/changes/actor-identity-end-state/ (#2444, updated by #2489). It matches the plan below. It fixes the target and the rules. Each step settles its own implementation details in its PR, after it re-checks the spec against current main. A change to a decided rule needs David's OK.

  • Owner: the Identity that started the root Conversation: the human for Slack, web, and fork; the one system Identity for automations, plugin dispatch, and the local CLI. Set once at insert. It never transfers. An Automation has a creator, not an Owner.
  • The Owner column is the existing junior_conversations.actor_identity_id. Code and docs call it the Owner. No new column and no rename.
  • Children store no Owner. They read the root Owner, and child spend rolls up to it.
  • One system Identity, with the fixed id system (ref(identity): Set author Identity ids at ingress #2482). "Which system" is provenance (Source and Keep a durable record of who and why for Junior actions #2211). Dashboard labels come from the Source and do not change to "System".
  • Users have many Identities. Spend, People, participants, and Locations roll up by User.
  • Web users use their sign-in Identity. The mock viewer has a mock Identity. Junior stops writing junior/<email>.
  • Bots are kind bot with no User.
  • Every new user Message has an author Identity. Assistant output has none.
  • Events are user Messages. The author is the person or bot who wrote the content, or the system Identity for a system event. Event authors are not participants.
  • The event author is never the Turn Actor. Event Turns stay system-acted, and credentials come only from the Watch or Automation grant. design.md explains the confused deputy risk. Junior still resolves the author to its Identity and linked User, for the Message label and the trust check.
  • Trust: an event author is trusted when Junior verifies them on an allowed domain (the feat(events): Treat linked requester PR comments as their instruction #2260 / feat(dispatch): Start DM dispatches for linked provider identities #2217 check) or their login is on trustedAuthors. Trusted text counts like a chat message. Untrusted or unknown authors do not run. Plan reading: Junior drops their events before batching, so the agent never sees them. System events run as today, with the current untrusted-text handling.
  • Legacy data rule (plan decision): migrate an old data shape only when special code exists because of it, and only with a rule that a read-only dry run proves. Then delete that code.
    • Needs migration: the stored Actor JSON (actor_json), user Messages with no author Identity, the payload copy of the author id, the per-name scheduler / local-cli / event system rows (merge into system, after the labels come from the Source), Owner copies on child Conversations, and the two unused columns creator_identity_id / credential_subject_identity_id.
    • No migration: old junior/<email> web Identities (valid Identities linked to the right User; a merge into the sign-in Identity is optional), and Redis and Turn checkpoint data (Junior waits for it to expire: 7-day Redis TTL; the plan waits 14 days).
    • Old event Messages get the system Identity as author. Plan reading of David's answer 2: old user Messages whose author cannot be proven stay with no author, with no guess. A human Message stored as system could later read as a system or automation Message in history.
  • Owner repair (Personal spend omits web conversations and loses the web actor #2431) is separate (plan decision): it fixes wrong data, not a code path. A root with a system Source (scheduler, event, local) that a human took over goes back to system; this is provable because the Source is set-once. A Slack or web root with a system Owner is repaired only if the dry run proves the rule. Humans who lose a root keep access as participants.

Decided by David Cramer in the Slack thread

  • The Owner is the person who started the Conversation, or the system for an automation. Owners never transfer.
  • There should be no unknown actors on any Message from now on.
  • The local CLI user can be the system Identity.
  • A web user is the User they sign in with, through the normal User and Identity tables.
  • The schema should be normalized.
  • Assistant output is just an event. It needs no author Identity.
  • Web users always have an Identity. The mock viewer mocks one.
  • Child spend rolls up to the root Owner.
  • Slack bots have an Identity but no User.
  • Inbound event authors: Junior finds the correct Identity and verifies it the same way as everywhere else. Junior ignores the instructions of someone without access.
  • Most events are system events, with the system Identity as author. PR feedback and comments are authored by the person or bot who wrote them.
  • Legacy junior/<email> rows: leave them as they are, unless a migration path is clearly correct.
  • Participants: "same as today: users." Verified event authors as participants: "not right now maybe we will change our mind later."
  • "also we'd allow list usernames in juniors config for github events (so we could allow list some bots)"
  • "call it trusted authors or sometihng", then: "yes "trusted" is what we want here, but you can use the suffix that makes sense"
  • "any sender who doesnt have the allowed domain email address or is unknown isnt trusted. bots and or other users could be listed under trusted"
  • On what trusted text may do: "trusted authors are the same as normal authors... its just that untrusted authors we dont run"
  • On the event author as Turn Actor: "I agree, lets not have it be the turn actor as it expands the security concerns. you need to document that (confused deputy concerns for example). knowing who it is that maps to our internal users is valuable tho so keep that"
  • On splitting batches by trust and hiding untrusted entries: "untrusted events wont actually ever be seen so i think (2/3) is not relevant? system events might have untrusted text but we already handle that"
  • On Cursor Bugbot: "yeah Bugbot and sentry will end up on trusted actors"
  • His answers to the 14 proposals in docs(openspec): Define the Actor identity end state #2444: (1) bot kind: "bots need to be annotated as bots, thats important"; (2) bypass Identity: "you can find what makes sense"; (3) doneBy fields: "yes"; (4) upsert keeps kind and user: "correct"; (5) events: "yes but an event is a message so"; (6) case-insensitive match: "yes"; (7) Owner repair: "sure defer to you but needs to be correct"; (8) Locations by User: "yep"; (9) author shape: "yep. and users have many identities associated to them"; (10) system key: "thats meaningless to me, make sure its good arch and ill trust you"; (11) Slack bot subject: "yep"; (12) mock key: "idk up to you make it correct"; (13) Automation creator: "sure, we dont really have "owners" on automations anyways"; (14) instruction record: "ok".
  • His answers on old data and the Owner column: (1) old event Messages get the system Identity as author: "(1) yes"; (2) old user Messages whose author cannot be proven: "(2), why not just make them system? if they dont have an author just make them without an author? should be uncommon"; (3) keep junior_conversations.actor_identity_id as the Owner column: "(3) i guess? usually theres just like one actor.. everything else is metadata".

Handed off by David (2, 7, 10, 12): each is a current proposal in the spec that its step confirms. The Owner repair runs only for the rules that the dry run proves, after David's OK.

Resolved questions

  • Mixed event batch (drop untrusted entries, or keep them as context): resolved by his "untrusted events wont actually ever be seen" answer. Plan reading: Junior drops untrusted authored events before batching. Only trusted and system entries are batched.
  • Cursor Bugbot on trustedAuthors: resolved by his "Bugbot and sentry" answer. Plan reading: he wrote "trusted actors", and the plan reads this as the GitHub plugin trustedAuthors list. Cursor Bugbot and Sentry's GitHub bot go on that list in the deployed app config. Step 4 confirms the exact logins from real event senders. That config ships with the trust rule, or their PR feedback stops starting runs.
  • Old data and the Owner column: resolved by his three answers above. Plan decision: the new Owner column work (old steps 4a/4b) is removed, because ref(identity): Set author Identity ids at ingress #2482 makes the existing column keep its first value, which is the Owner rule.

No open questions remain.

Steps (each ships on its own)
Next: step 3.

  1. One Actor-to-Identity mapping: ref(identity): Resolve Actors to Identity ids behind one entry point #2443 (merged).
  2. Carry the Identity id from ingress. Readers: ref(identity): Accept Identity ids on Inbound messages and Turns #2473 (merged). Writers: ref(identity): Set author Identity ids at ingress #2482 (merged, not released yet). The step 8 Redis window starts when a release with ref(identity): Set author Identity ids at ingress #2482 runs on every worker.
  3. System root labels come from the Source, new system Conversations use system, code calls the column the Owner, every reader uses the root Owner (including child spend and Locations by User), and children stop storing an Owner. Readers ship before children stop storing it.
  4. Real Identities and trust: web uses the sign-in Identity, mock and bypass Identities, bots get the bot kind, event authors, the trust rule, and trustedAuthors with Bugbot and Sentry. Needs feat(events): Treat linked requester PR comments as their instruction #2260.
  5. Read-only dry run of every migration and repair rule. Post the counts on Resolve actor identity once and store it explicitly #2439, then David's OK.
  6. One data migration release, only for the rules the dry run proved.
  7. Owner repair (Personal spend omits web conversations and loses the web actor #2431), only for the rules the dry run proved.
  8. After the Redis window (14 days after step 2 runs on every worker): delete the fallbacks, require an author on new user Messages, and remove the stored Slack-shaped Actor from Conversations and Redis. Also the Guardian, session log, and dashboard author shape changes. Needs fix(agents): Record whose work a child agent does in its session log #2181.
  9. One release later: drop the old columns, move the durable rules to the owning READMEs, and delete the spec.

Release rules: readers before writers, safe under rolling deploys, column drops one release after the code stops using them, no production backfill without explicit approval, and a dry run before any migration or repair.

Dependencies: #2260 before 4, #2181 before 8. #2211 owns which Automation or dispatch did the work.

Not covered: keying user credentials by User, the closed Actor schema, and naming the local CLI human. Visibility rows written wrong before #2440 belong to #2382. Known gaps are listed in design.md.

Related: #2260, #2431, #2432, #2436, #2437, #2440, #2443, #2444, #2473, #2482, #2489.

via David Cramer.

--

View Session [Sentry]

Activity

  1. 1 remaining item

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions