diff --git a/packages/junior-github/SETUP.md b/packages/junior-github/SETUP.md index 8451919563..34dd5c3685 100644 --- a/packages/junior-github/SETUP.md +++ b/packages/junior-github/SETUP.md @@ -156,7 +156,7 @@ Use `additionalUserScopes` only when a human-identity integration flow requires - The plugin classifies GitHub traffic from the forwarded HTTP request. Reads use `installation-read`, while `GET /user` uses `user-read`. Allowlisted App-owned mutations and Git smart-HTTP pushes use `installation-write`. User-attachment uploads to `uploads.github.com/user-attachments/assets` use `user-write`. Unknown REST writes and GraphQL mutations are denied. - `user-read` and explicitly human `user-write` operations require the actor, or an explicitly delegated user subject, to authorize the GitHub App through the private OAuth flow. Junior-owned issue, pull request, review, inline review comment, and branch operations do not fall back to user OAuth. - Headless resource-event turns use the `resource-event` system actor and may receive the same installation grants. This lets Junior respond to subscribed pull request events by committing and pushing fixes without inheriting a subscriber's OAuth credential. -- Git commits use Junior as author and committer. Resolvable human run actors are credited once with `Co-Authored-By` trailers. +- Git commits use Junior as author and committer. Resolvable human run actors are credited once with `Co-Authored-By` trailers. When the current actor has a linked GitHub identity, Junior prefers a GitHub noreply address so external automation can resolve a login for assignment. - Installation credential leases are cached on the host by grant name and reused across sandboxes until near expiry. User grants stay actor-scoped. Upstream 403 after injection clears the cached lease, issues a new token, and retries the hop once before recording permission denied. - Sandbox does not receive raw tokens via env; host applies Authorization header transforms for GitHub API and upload calls. @@ -187,7 +187,7 @@ The plugin uses installation credentials for read-only GitHub traffic, workflow Committing and pushing code uses more than one GitHub surface: -- Creating the local Git commit does not call GitHub. Junior sets the GitHub App bot as author and committer and credits resolvable human actors with `Co-Authored-By` trailers. +- Creating the local Git commit does not call GitHub. Junior sets the GitHub App bot as author and committer and credits resolvable human actors with `Co-Authored-By` trailers, preferring a linked GitHub noreply email for the current actor when present. - Pushing a branch with Git smart HTTP (`git push`) uses the `installation-write` grant and requires the App installation to have `Contents: write`. Workflow-file changes also require the installation to have `Workflows: write`. - The smart-HTTP classifier does not distinguish Junior-managed branches or independently detect force updates or ref deletion. Use GitHub branch protection and limit the App installation to repositories where Junior may push. - REST Git database and ref writes are denied by the current write allowlist. Use Git smart HTTP (`git push`) for branch updates instead. diff --git a/packages/junior-github/src/git-config.ts b/packages/junior-github/src/git-config.ts index 4acc1c5ed4..340fd7e5c1 100644 --- a/packages/junior-github/src/git-config.ts +++ b/packages/junior-github/src/git-config.ts @@ -4,6 +4,7 @@ import type { Actor, SandboxPrepareHookContext, + User, } from "@sentry/junior-plugin-api"; function cleanIdentityPart(value: unknown): string { @@ -55,6 +56,28 @@ function actorEmail(actor?: Actor): string | undefined { return /^[^\s@<>]+@[^\s@<>]+\.[^\s@<>]+$/.test(email) ? email : undefined; } +/** + * Prefer a GitHub noreply address when the linked user has a GitHub identity. + * That form encodes a resolvable login for downstream assignee automation. + */ +function githubNoreplyEmail(user?: User): string | undefined { + for (const identity of user?.identities ?? []) { + if (identity.provider !== "github") { + continue; + } + const login = cleanIdentityPart(identity.handle); + const userId = cleanIdentityPart(identity.providerSubjectId); + if (!login || login.toLowerCase().endsWith("[bot]")) { + continue; + } + if (/^[1-9]\d*$/.test(userId)) { + return `${userId}+${login}@users.noreply.github.com`; + } + return `${login}@users.noreply.github.com`; + } + return undefined; +} + /** * Stable identity key for an actor, matching the distinctness rule * `instructionActors` uses to build `run.actors` (identity ids only, never @@ -78,14 +101,23 @@ function actorIdentityKey(actor: Actor): string { * denying the commit. Dedupes by identity and resolved email so the same human * under two display profiles, or an actor matching the bot identity, only ever * produces one line. + * + * When the current actor has a linked GitHub identity, prefer that noreply + * address so external automation can resolve a login for assignment. */ export function additionalActorCoauthorTrailers(args: { actors?: Actor[]; botEmail: string; + /** Linked user for the current run actor, when already resolved. */ + currentUser?: User; }): string[] { if (!args.actors || args.actors.length === 0) { return []; } + const currentActorKey = args.actors[0] + ? actorIdentityKey(args.actors[0]) + : undefined; + const currentNoreply = githubNoreplyEmail(args.currentUser); const seenEmails = new Set([args.botEmail.toLowerCase()]); const seenActors = new Set(); const trailers: string[] = []; @@ -95,7 +127,9 @@ export function additionalActorCoauthorTrailers(args: { continue; } const name = actorName(candidate); - const email = actorEmail(candidate); + const email = + (currentActorKey === actorKey ? currentNoreply : undefined) || + actorEmail(candidate); if (!name || !email) { continue; } diff --git a/packages/junior-github/src/plugin.ts b/packages/junior-github/src/plugin.ts index 5cfc3de5a5..4af9a00ed4 100644 --- a/packages/junior-github/src/plugin.ts +++ b/packages/junior-github/src/plugin.ts @@ -327,7 +327,7 @@ export function githubPlugin( await configureGit(ctx, "credential.helper", ""); await configureGit(ctx, "http.emptyAuth", "true"); }, - beforeToolExecute(ctx) { + async beforeToolExecute(ctx) { if (ctx.tool.name !== "bash") { return; } @@ -339,9 +339,18 @@ export function githubPlugin( ctx.env.set("JUNIOR_GIT_AUTHOR_EMAIL", botEmail); ctx.env.set("GIT_COMMITTER_NAME", botName); ctx.env.set("GIT_COMMITTER_EMAIL", botEmail); + let currentUser; + if (ctx.actor) { + try { + currentUser = (await ctx.users.resolveActor())?.user; + } catch { + currentUser = undefined; + } + } const actorTrailers = additionalActorCoauthorTrailers({ actors: [...(ctx.actor ? [ctx.actor] : []), ...(ctx.actors ?? [])], botEmail, + currentUser, }); ctx.env.set( "JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS", diff --git a/packages/junior-github/tests/github-plugin.test.ts b/packages/junior-github/tests/github-plugin.test.ts index 1e4e7afe49..f05c1d058f 100644 --- a/packages/junior-github/tests/github-plugin.test.ts +++ b/packages/junior-github/tests/github-plugin.test.ts @@ -64,7 +64,11 @@ type TestActor = { userName?: string; }; -function beforeToolContext(actor: TestActor, actors?: TestActor[]) { +function beforeToolContext( + actor: TestActor, + actors?: TestActor[], + resolveActor?: ToolRegistrationHookContext["users"]["resolveActor"], +) { const env: Record = {}; let denial: string | undefined; @@ -93,6 +97,9 @@ function beforeToolContext(actor: TestActor, actors?: TestActor[]) { db, actor, ...(actors ? { actors } : undefined), + users: { + resolveActor: resolveActor ?? (async () => undefined), + }, tool: { input: { command: "git commit -m test" }, name: "bash", @@ -3486,7 +3493,7 @@ Conversation: \`local:test:old-conversation\` ); }); - it("throws GitHubPluginSetupError when bot identity environment variables are missing", () => { + it("throws GitHubPluginSetupError when bot identity environment variables are missing", async () => { delete process.env.GITHUB_APP_BOT_NAME; delete process.env.GITHUB_APP_BOT_EMAIL; @@ -3498,12 +3505,12 @@ Conversation: \`local:test:old-conversation\` userName: "dcramer", }); - expect(() => { - plugin.hooks?.beforeToolExecute?.(before.ctx as never); - }).toThrow("Missing GITHUB_APP_BOT_NAME"); + await expect( + plugin.hooks?.beforeToolExecute?.(before.ctx as never), + ).rejects.toThrow("Missing GITHUB_APP_BOT_NAME"); }); - it("injects Junior author and committer identity", () => { + it("injects Junior author and committer identity", async () => { process.env.GITHUB_APP_BOT_NAME = "sentry-junior[bot]"; process.env.GITHUB_APP_BOT_EMAIL = "bot@example.com"; @@ -3515,7 +3522,7 @@ Conversation: \`local:test:old-conversation\` userName: "dcramer", }); - plugin.hooks?.beforeToolExecute?.(before.ctx as never); + await plugin.hooks?.beforeToolExecute?.(before.ctx as never); expect(before.denial).toBeUndefined(); expect(before.env).toMatchObject({ @@ -3531,6 +3538,48 @@ Conversation: \`local:test:old-conversation\` ); }); + it("prefers linked GitHub noreply emails for the current actor", async () => { + process.env.GITHUB_APP_BOT_NAME = "sentry-junior[bot]"; + process.env.GITHUB_APP_BOT_EMAIL = "bot@example.com"; + + const plugin = githubPlugin(); + const before = beforeToolContext( + { + email: "david@example.com", + fullName: "David Cramer", + userId: "U039RR91S", + userName: "dcramer", + }, + undefined, + async () => ({ + identity: { + id: "slack-id", + provider: "slack", + providerSubjectId: "U039RR91S", + }, + user: { + id: "user-1", + email: "david@example.com", + displayName: "David Cramer", + identities: [ + { + id: "gh-id", + provider: "github", + providerSubjectId: "1473041", + handle: "dcramer", + }, + ], + }, + }), + ); + + await plugin.hooks?.beforeToolExecute?.(before.ctx as never); + + expect(before.env.JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS).toBe( + "Co-Authored-By: David Cramer <1473041+dcramer@users.noreply.github.com>", + ); + }); + it("records Junior as Git author and committer with human attribution", async () => { const { copyFileSync, @@ -3553,7 +3602,7 @@ Conversation: \`local:test:old-conversation\` userId: "U039RR91S", userName: "dcramer", }); - githubPlugin().hooks?.beforeToolExecute?.(before.ctx as never); + await githubPlugin().hooks?.beforeToolExecute?.(before.ctx as never); const hook = await prepareCommitMsgHookFixture("unused\n"); const repoDir = mkdtempSync(join(tmpdir(), "junior-github-commit-")); @@ -3603,7 +3652,7 @@ Conversation: \`local:test:old-conversation\` } }); - it("credits the primary and additional run actors as co-author trailers", () => { + it("credits the primary and additional run actors as co-author trailers", async () => { process.env.GITHUB_APP_BOT_NAME = "sentry-junior[bot]"; process.env.GITHUB_APP_BOT_EMAIL = "bot@example.com"; @@ -3637,7 +3686,7 @@ Conversation: \`local:test:old-conversation\` before.env.JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS = "Co-Authored-By: Model Supplied "; - plugin.hooks?.beforeToolExecute?.(before.ctx as never); + await plugin.hooks?.beforeToolExecute?.(before.ctx as never); expect(before.denial).toBeUndefined(); expect(before.env.JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS).toBe( @@ -3645,7 +3694,7 @@ Conversation: \`local:test:old-conversation\` ); }); - it("omits a steering actor without a resolvable name or email, without denying the commit", () => { + it("omits a steering actor without a resolvable name or email, without denying the commit", async () => { process.env.GITHUB_APP_BOT_NAME = "sentry-junior[bot]"; process.env.GITHUB_APP_BOT_EMAIL = "bot@example.com"; @@ -3675,7 +3724,7 @@ Conversation: \`local:test:old-conversation\` }, ]); - plugin.hooks?.beforeToolExecute?.(before.ctx as never); + await plugin.hooks?.beforeToolExecute?.(before.ctx as never); expect(before.denial).toBeUndefined(); expect(before.env.JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS).toBe( @@ -3683,7 +3732,7 @@ Conversation: \`local:test:old-conversation\` ); }); - it("uses a later resolvable profile for a duplicate actor identity", () => { + it("uses a later resolvable profile for a duplicate actor identity", async () => { process.env.GITHUB_APP_BOT_NAME = "sentry-junior[bot]"; process.env.GITHUB_APP_BOT_EMAIL = "bot@example.com"; @@ -3704,14 +3753,14 @@ Conversation: \`local:test:old-conversation\` }, ]); - plugin.hooks?.beforeToolExecute?.(before.ctx as never); + await plugin.hooks?.beforeToolExecute?.(before.ctx as never); expect(before.env.JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS).toBe( "Co-Authored-By: David Cramer ", ); }); - it("dedups additional actors by resolved email and drops one matching the bot email", () => { + it("dedups additional actors by resolved email and drops one matching the bot email", async () => { process.env.GITHUB_APP_BOT_NAME = "sentry-junior[bot]"; process.env.GITHUB_APP_BOT_EMAIL = "bot@example.com"; @@ -3750,7 +3799,7 @@ Conversation: \`local:test:old-conversation\` }, ]); - plugin.hooks?.beforeToolExecute?.(before.ctx as never); + await plugin.hooks?.beforeToolExecute?.(before.ctx as never); expect(before.denial).toBeUndefined(); expect(before.env.JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS).toBe( @@ -3758,7 +3807,7 @@ Conversation: \`local:test:old-conversation\` ); }); - it("credits the primary actor in a single-actor run", () => { + it("credits the primary actor in a single-actor run", async () => { process.env.GITHUB_APP_BOT_NAME = "sentry-junior[bot]"; process.env.GITHUB_APP_BOT_EMAIL = "bot@example.com"; @@ -3774,7 +3823,7 @@ Conversation: \`local:test:old-conversation\` before.env.JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS = "Co-Authored-By: Model Supplied "; - plugin.hooks?.beforeToolExecute?.(before.ctx as never); + await plugin.hooks?.beforeToolExecute?.(before.ctx as never); expect(before.denial).toBeUndefined(); expect(before.env.JUNIOR_GIT_ACTOR_COAUTHOR_TRAILERS).toBe( @@ -3865,7 +3914,7 @@ Conversation: \`local:test:old-conversation\` rmSync(dir, { recursive: true, force: true }); }); - it("uses Junior author identity when the human actor is unresolved", () => { + it("uses Junior author identity when the human actor is unresolved", async () => { process.env.GITHUB_APP_BOT_NAME = "sentry-junior[bot]"; process.env.GITHUB_APP_BOT_EMAIL = "bot@example.com"; @@ -3876,7 +3925,7 @@ Conversation: \`local:test:old-conversation\` userName: "U039RR91S", }); - plugin.hooks?.beforeToolExecute?.(before.ctx as never); + await plugin.hooks?.beforeToolExecute?.(before.ctx as never); expect(before.denial).toBeUndefined(); expect(before.env).toMatchObject({ diff --git a/packages/junior-plugin-api/src/tools.ts b/packages/junior-plugin-api/src/tools.ts index 6e24d3fce6..cfee578806 100644 --- a/packages/junior-plugin-api/src/tools.ts +++ b/packages/junior-plugin-api/src/tools.ts @@ -170,6 +170,13 @@ export interface BeforeToolExecuteHookContext extends PluginContext { input: Record; name: string; }; + /** + * Resolve the current actor's stored identity and linked user. + * Same contract as tool registration; used for commit attribution. + */ + users: { + resolveActor(): Promise<{ identity: Identity; user?: User } | undefined>; + }; } /** diff --git a/packages/junior/src/chat/plugins/agent-hooks.ts b/packages/junior/src/chat/plugins/agent-hooks.ts index a3a033998c..d89db75837 100644 --- a/packages/junior/src/chat/plugins/agent-hooks.ts +++ b/packages/junior/src/chat/plugins/agent-hooks.ts @@ -43,7 +43,10 @@ import { createResourceEventSubscription } from "@/chat/resource-events/store"; import { RESOURCE_SUBSCRIPTION_DEFAULT_TTL_MS } from "@/chat/resource-events/tool-support"; import { getSlackToolContext } from "@/chat/slack/tool-support/context"; -import { resolveViewerUser } from "@/chat/plugins/viewer"; +import { + readActorIdentity, + resolveViewerUser, +} from "@/chat/plugins/viewer"; import type { ToolRuntimeContext } from "@/chat/tools/types"; import type { SandboxCommandInput, @@ -1622,6 +1625,10 @@ export function createPluginHookRunner( name: tool.name, input: nextInput, }, + users: { + resolveActor: async () => + input.actor ? await readActorIdentity(input.actor) : undefined, + }, env: { get(key) { return env[key] ?? normalizeEnv(nextInput.env)[key];