From eae9ee66a0934e79ccc3d6f4fef461d9e6f1df14 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:51:13 +0000 Subject: [PATCH 1/8] feat(linear): Use app OAuth and native tools Co-Authored-By: David Cramer --- packages/junior-linear/README.md | 40 +- packages/junior-linear/skills/linear/SKILL.md | 4 +- .../skills/linear/references/api-surface.md | 53 +-- packages/junior-linear/src/plugin.ts | 71 ++-- packages/junior-linear/src/tools.ts | 341 ++++++++++++++++++ packages/junior-linear/tests/tools.test.ts | 107 ++++++ packages/junior-plugin-api/src/manifest.ts | 2 + .../junior/src/chat/capabilities/factory.ts | 22 +- .../credentials/installation-token-store.ts | 10 + .../src/chat/credentials/oauth-scope.ts | 2 +- .../credentials/state-adapter-token-store.ts | 103 ++++-- .../junior/src/chat/local/credential-sync.ts | 24 +- packages/junior/src/chat/oauth-flow.ts | 5 +- .../chat/plugins/auth/oauth-bearer-broker.ts | 244 +++++++------ .../chat/plugins/inline-manifest-source.ts | 1 + packages/junior/src/chat/plugins/manifest.ts | 5 + packages/junior/src/chat/plugins/registry.ts | 7 +- packages/junior/src/chat/plugins/types.ts | 3 + .../junior/src/handlers/oauth-callback.ts | 17 +- .../plugins/plugin-manifest-config.test.ts | 24 ++ .../tests/unit/plugins/sentry-broker.test.ts | 126 +++++-- .../state/state-adapter-token-store.test.ts | 24 +- 22 files changed, 955 insertions(+), 280 deletions(-) create mode 100644 packages/junior-linear/src/tools.ts create mode 100644 packages/junior-linear/tests/tools.test.ts create mode 100644 packages/junior/src/chat/credentials/installation-token-store.ts diff --git a/packages/junior-linear/README.md b/packages/junior-linear/README.md index 249884f9a8..a72989b237 100644 --- a/packages/junior-linear/README.md +++ b/packages/junior-linear/README.md @@ -1,41 +1,31 @@ # @sentry/junior-linear -`@sentry/junior-linear` adds Linear issue workflows to Junior through Linear's hosted MCP server. +`@sentry/junior-linear` adds native Linear GraphQL tools and issue webhooks to Junior. -Install it alongside `@sentry/junior`: +Install it alongside `@sentry/junior`, then register `linearPlugin()` in `plugins.ts`. -```bash -pnpm add @sentry/junior @sentry/junior-linear -``` +## OAuth app -Then add the plugin to the set exported from `plugins.ts`: +Create a Linear OAuth application and configure: -```ts title="plugins.ts" -import { defineJuniorPlugins } from "@sentry/junior"; -import { linearPlugin } from "@sentry/junior-linear"; +- Callback: `https:///api/oauth/callback/linear` +- Scopes: `read,write` +- Environment: `LINEAR_CLIENT_ID` and `LINEAR_CLIENT_SECRET` -export const plugins = defineJuniorPlugins([linearPlugin()]); -``` +Junior sends `actor=app`. A workspace admin authorizes the app once. Junior stores the access and refresh tokens for the installation, refreshes them behind a provider lock, and uses the app credential for interactive, scheduled, and event work. -This package does not require a shared `LINEAR_API_KEY` or a custom OAuth app for the default setup. Each user connects their own Linear account the first time Junior calls a Linear MCP tool. Junior sends the authorization link privately and resumes the same Slack thread automatically after the user authorizes. +The initial policy is a hard cutover from MCP: mutations use Junior app attribution, per-user OAuth is not retained, and the Linear workspace is the access boundary. There is no separate team allowlist in this version. A rejected refresh token is removed and requires an admin reinstall. -Linear operations use Linear's hosted MCP tools directly. When an issue is created through that path, Junior links it to the current conversation. +The native tools cover issue get/search/create/update, comment creation, and team, project, and workflow-state lookup. Created issues are linked to the current Junior conversation. + +## Webhooks To run watches or event tasks when Linear issues are created: -1. Set `LINEAR_WEBHOOK_SECRET` to the Linear webhook signing secret. +1. Set `LINEAR_WEBHOOK_SECRET`. 2. Create a Linear webhook for the `Issue` resource at `https:///api/webhooks/linear`. 3. Redeploy Junior. -The plugin verifies the `Linear-Signature` header and publishes `issue.created` for the issue identifier and the team key. Team event tasks use the Linear team key, such as `SRE`. Issue and team watches also accept an optional `match.teamKey` filter on trusted event data. - -Optional: set channel defaults when a Slack thread usually routes work to the same Linear destination: - -```bash -jr-rpc config set linear.team Platform -jr-rpc config set linear.project "Cross-team reliability" -``` - -These defaults are only fallbacks. If the user names a different team or project in the request, Junior should follow the explicit request instead. +The plugin verifies `Linear-Signature` and publishes `issue.created` for the issue identifier and team key. -Full setup guide: https://junior.sentry.dev/extend/linear-plugin/ +Optional conversation defaults remain available through `linear.team` and `linear.project`. Explicit user input wins over these defaults. diff --git a/packages/junior-linear/skills/linear/SKILL.md b/packages/junior-linear/skills/linear/SKILL.md index 2c8e687b9e..8bb6c3ce41 100644 --- a/packages/junior-linear/skills/linear/SKILL.md +++ b/packages/junior-linear/skills/linear/SKILL.md @@ -1,6 +1,6 @@ --- name: linear -description: Manage Linear issues through Linear's hosted MCP server. Use when users ask to create a Linear ticket, update a Linear issue, add a Linear comment, move work between states, assign work, or look up Linear issue, team, or project details from Slack context. +description: Manage Linear issues through Junior's native Linear GraphQL tools and installed OAuth app. Use when users ask to create a Linear ticket, update a Linear issue, add a Linear comment, move work between states, assign work, or look up Linear issue, team, or project details from Slack context. --- # Linear Operations @@ -69,7 +69,7 @@ If any gate fails, revise and re-check before calling the Linear create/update t 6. Execute: -- Create or update issues with Linear's live hosted MCP tools. Discover the current create/update tool and copy only fields justified by its live schema. +- Use the native `linear_*` tools. Resolve team, project, workflow state, and issue IDs with read tools before writes. - For updates, prefer partial changes over full rewrites. Fetch current issue state first if the mutation could overwrite structured fields or duplicate an existing comment. - Check for duplicates silently before creating a new issue when the request appears related to existing work. diff --git a/packages/junior-linear/skills/linear/references/api-surface.md b/packages/junior-linear/skills/linear/references/api-surface.md index 6fbf56570b..690380dcc1 100644 --- a/packages/junior-linear/skills/linear/references/api-surface.md +++ b/packages/junior-linear/skills/linear/references/api-surface.md @@ -2,36 +2,39 @@ Use this reference for any Linear operation. -## Provider capabilities +## Native tools -Linear's hosted MCP server is intended for authenticated remote MCP access to Linear data. -The current public docs describe support for finding, creating, and updating objects such as issues, projects, and comments. +- `linear_getIssue`: get one issue by UUID or identifier. +- `linear_searchIssues`: find issues before creation or mutation. +- `linear_createIssue`: create an issue as the installed Junior app. +- `linear_updateIssue`: update selected issue fields as the installed Junior app. +- `linear_createComment`: add a comment as the installed Junior app. +- `linear_listTeams`: resolve a team UUID. +- `linear_listProjects`: resolve an active project UUID. +- `linear_listWorkflowStates`: resolve one team's workflow state UUID. + +The tools call Linear's GraphQL API through Junior's credential broker. They do not use hosted MCP and do not use the requesting user's Linear identity. ## Linear issue model constraints - Every issue belongs to exactly one team. -- A new issue requires a title and a status; all other properties are optional. -- Workflow states are team-specific. The common default order is `Backlog > Todo > In Progress > Done > Canceled`, but teams can customize names and ordering. -- Priority is optional and limited to `low`, `medium`, `high`, or `urgent`. -- Labels can be workspace-scoped or team-scoped. -- Estimates are optional and team-configured. +- A new issue requires a title and team UUID. +- Workflow states are team-specific. Never infer a state UUID from a name. +- Linear priorities use numeric API values: `0` no priority, `1` urgent, `2` high, `3` medium, `4` low. +- Resolve project, state, team, and issue identifiers before a write. ## Operation patterns -| Intent | Minimum tool pattern | -| -------------------- | ---------------------------------------------------------------------------------------------------------------------------- | -| Inspect an issue | Resolve the issue by key, URL, or search query, then fetch current state before answering. | -| Create an issue | Confirm the team first, then create via the live hosted MCP tools with grounded title/body content and only fields justified by the current schema. | -| Update fields | Fetch current issue state first, then mutate via the live hosted MCP tools with only the requested fields. | -| Add a comment | Resolve the exact issue first, then add a concise comment with durable links and next steps. | -| Move state or assign | Read the current issue and team workflow first when state, workflow, or assignee ambiguity could cause the wrong mutation. | -| Check for duplicates | Search for an existing matching issue before opening a new one when the request appears related to ongoing work. | - -## Content expectations - -- Translate Slack-thread wording into stable product or engineering language. -- Preserve material links already present in the conversation, such as Sentry, GitHub, docs, repro, or dashboard URLs. -- Keep provenance concise. Mention Slack origin only when it helps future readers understand why the issue exists. -- Treat team, status, labels, estimate, cycle, and project as structured properties, not prose-only body content, when those fields are available and the values are actually known. -- Prefer partial updates over full rewrites. -- Label assumptions clearly when the thread leaves important details uncertain. +- Inspect: resolve the issue, then fetch current state. +- Create: search for duplicates, resolve the team, then call `linear_createIssue`. +- Update: fetch current state, then send only requested fields to `linear_updateIssue`. +- Comment: resolve the exact issue before `linear_createComment`. +- Move state: list the team's states before updating `stateId`. + +## Credential policy + +- One workspace admin installs the OAuth app with `actor=app`. +- Junior stores that OAuth grant for the installation and refreshes it behind a provider lock. +- Routine interactive, scheduled, and event work uses the same app credential. +- Mutations are attributed to the Junior Linear app. Do not claim that they were made as the Slack user. +- A rejected refresh requires an admin to reinstall the app. Do not fall back to per-user OAuth. diff --git a/packages/junior-linear/src/plugin.ts b/packages/junior-linear/src/plugin.ts index 2e33bd963d..33f9de3ca1 100644 --- a/packages/junior-linear/src/plugin.ts +++ b/packages/junior-linear/src/plugin.ts @@ -1,55 +1,16 @@ import { defineJuniorPlugin, - type AfterMcpToolHookContext, type PluginRegistration, } from "@sentry/junior-plugin-api"; -import { z } from "zod"; import { LINEAR_ISSUE_EVENTS, LINEAR_ISSUE_MATCH_FIELDS, } from "./resource-events/issue.js"; +import { createLinearTools } from "./tools.js"; import { createLinearWebhookRoute } from "./webhooks/handler.js"; import { linearWebhookSecret } from "./webhooks/secret.js"; -const saveIssueResultSchema = z - .object({ - issue: z - .object({ - identifier: z.string().trim().min(1), - url: z.url(), - }) - .passthrough(), - }) - .passthrough(); - -/** Link newly created Linear issues to the current Junior conversation. */ -async function annotateCreatedIssue( - ctx: AfterMcpToolHookContext, -): Promise { - if (ctx.tool.name !== "save_issue" || ctx.tool.arguments.id !== undefined) { - return; - } - if (!ctx.annotations) { - return; - } - const result = saveIssueResultSchema.safeParse(ctx.result.structuredContent); - if (!result.success) { - ctx.log.warn("linear.issue_annotation.skipped", { - "app.reason": "unexpected_save_response", - }); - return; - } - const identifier = result.data.issue.identifier.toUpperCase(); - await ctx.annotations.upsert({ - kind: "resource_link", - key: identifier, - label: identifier, - url: result.data.issue.url, - status: "open", - }); -} - -/** Register Linear's hosted MCP provider and conversation-link side effects. */ +/** Register Linear app OAuth, native GraphQL tools, and issue webhooks. */ export function linearPlugin(): PluginRegistration { return defineJuniorPlugin({ packageName: "@sentry/junior-linear", @@ -72,21 +33,36 @@ export function linearPlugin(): PluginRegistration { normalizeIdentifier: (identifier) => identifier.toUpperCase(), }, manifest: { + commandEnv: { + LINEAR_ACCESS_TOKEN: "host_managed_credential", + }, configKeys: ["team", "project"], + credentials: { + authTokenEnv: "LINEAR_ACCESS_TOKEN", + authTokenPlaceholder: "host_managed_credential", + domains: ["api.linear.app"], + type: "oauth-bearer", + }, description: - "Linear issue tracking via hosted MCP server and issue webhooks", + "Linear issue tracking via an installed OAuth app, native GraphQL tools, and issue webhooks", displayName: "Linear", envVars: { + LINEAR_CLIENT_ID: {}, + LINEAR_CLIENT_SECRET: {}, LINEAR_WEBHOOK_SECRET: {}, }, - mcp: { - transport: "http", - url: "https://mcp.linear.app/mcp", - }, name: "linear", + oauth: { + authorizeEndpoint: "https://linear.app/oauth/authorize", + authorizeParams: { actor: "app" }, + clientIdEnv: "LINEAR_CLIENT_ID", + clientSecretEnv: "LINEAR_CLIENT_SECRET", + scope: "read,write", + tokenEndpoint: "https://api.linear.app/oauth/token", + tokenSubject: "installation", + }, }, hooks: { - afterMcpTool: annotateCreatedIssue, routes(ctx) { return [ createLinearWebhookRoute({ @@ -95,6 +71,7 @@ export function linearPlugin(): PluginRegistration { }), ]; }, + tools: createLinearTools, }, }); } diff --git a/packages/junior-linear/src/tools.ts b/packages/junior-linear/src/tools.ts new file mode 100644 index 0000000000..a06b4325d3 --- /dev/null +++ b/packages/junior-linear/src/tools.ts @@ -0,0 +1,341 @@ +import { + definePluginTool, + PluginToolInputError, + type PluginToolDefinition, + type ToolRegistrationHookContext, +} from "@sentry/junior-plugin-api"; +import { z } from "zod"; + +const API_URL = "https://api.linear.app/graphql"; +const issueSchema = z.object({ + id: z.string(), + identifier: z.string(), + title: z.string(), + description: z.string().nullable().optional(), + priority: z.number().optional(), + url: z.string(), + state: z.object({ id: z.string(), name: z.string() }).nullable().optional(), + team: z.object({ id: z.string(), key: z.string(), name: z.string() }), + project: z.object({ id: z.string(), name: z.string() }).nullable().optional(), +}); +const issueResultSchema = z.object({ issue: issueSchema }); +const issueListResultSchema = z.object({ issues: z.array(issueSchema) }); +const teamSchema = z.object({ + id: z.string(), + key: z.string(), + name: z.string(), +}); +const projectSchema = z.object({ id: z.string(), name: z.string() }); +const workflowStateSchema = z.object({ + id: z.string(), + name: z.string(), + type: z.string(), +}); + +type GraphqlEnvelope = { data?: T; errors?: Array<{ message?: string }> }; + +async function linearGraphql( + ctx: ToolRegistrationHookContext, + operation: string, + query: string, + variables: Record, +): Promise { + const response = await ctx.egress.fetch({ + provider: "linear", + operation, + request: new Request(API_URL, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ query, variables }), + }), + }); + let body: GraphqlEnvelope; + try { + body = (await response.json()) as GraphqlEnvelope; + } catch { + throw new Error(`Linear returned invalid JSON (HTTP ${response.status}).`); + } + const message = body.errors + ?.map((error) => error.message?.trim()) + .filter(Boolean) + .join("; "); + if (!response.ok || message || !body.data) { + throw new PluginToolInputError( + message || `Linear request failed with HTTP ${response.status}.`, + ); + } + return body.data; +} + +const issueSelection = ` + id identifier title description priority url + state { id name } + team { id key name } + project { id name } +`; + +function issueInput(input: Record) { + return Object.fromEntries( + Object.entries(input).filter(([, value]) => value !== undefined), + ); +} + +/** Build Linear's native GraphQL tool surface. */ +export function createLinearTools( + ctx: ToolRegistrationHookContext, +): Record { + return { + getIssue: definePluginTool({ + annotations: { + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + readOnlyHint: true, + }, + description: + "Get one Linear issue by UUID or identifier such as ENG-123.", + inputSchema: z.object({ id: z.string().min(1) }).strict(), + outputSchema: issueResultSchema, + async execute({ id }) { + const data = await linearGraphql<{ + issue: z.input; + }>( + ctx, + "linear.issue.get", + `query GetIssue($id: String!) { issue(id: $id) { ${issueSelection} } }`, + { id }, + ); + return issueResultSchema.parse(data); + }, + }), + searchIssues: definePluginTool({ + annotations: { + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + readOnlyHint: true, + }, + description: + "Search Linear issue titles and descriptions for duplicate or target resolution.", + inputSchema: z + .object({ + query: z.string().min(1), + first: z.number().int().min(1).max(50).default(20), + }) + .strict(), + outputSchema: issueListResultSchema, + async execute({ query, first }) { + const data = await linearGraphql<{ + issues: { nodes: z.input[] }; + }>( + ctx, + "linear.issue.search", + `query SearchIssues($query: String!, $first: Int!) { issues(first: $first, filter: { or: [{ title: { containsIgnoreCase: $query } }, { description: { containsIgnoreCase: $query } }] }) { nodes { ${issueSelection} } } }`, + { query, first }, + ); + return issueListResultSchema.parse({ issues: data.issues.nodes }); + }, + }), + createIssue: definePluginTool({ + annotations: { + destructiveHint: false, + idempotentHint: false, + openWorldHint: true, + readOnlyHint: false, + }, + description: + "Create a Linear issue as the installed Junior app. teamId must be a Linear team UUID.", + inputSchema: z + .object({ + teamId: z.string().min(1), + title: z.string().min(1).max(255), + description: z.string().optional(), + projectId: z.string().optional(), + stateId: z.string().optional(), + assigneeId: z.string().optional(), + priority: z.number().int().min(0).max(4).optional(), + }) + .strict(), + outputSchema: issueResultSchema, + async execute(input) { + const data = await linearGraphql<{ + issueCreate: { + success: boolean; + issue: z.input | null; + }; + }>( + ctx, + "linear.issue.create", + `mutation CreateIssue($input: IssueCreateInput!) { issueCreate(input: $input) { success issue { ${issueSelection} } } }`, + { input: issueInput(input) }, + ); + if (!data.issueCreate.success || !data.issueCreate.issue) { + throw new Error("Linear did not create the issue."); + } + const result = issueResultSchema.parse({ + issue: data.issueCreate.issue, + }); + await ctx.annotations?.upsert({ + kind: "resource_link", + key: result.issue.identifier.toUpperCase(), + label: result.issue.identifier.toUpperCase(), + url: result.issue.url, + status: "open", + }); + return result; + }, + }), + updateIssue: definePluginTool({ + annotations: { + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + readOnlyHint: false, + }, + description: + "Update selected fields on an existing Linear issue as the installed Junior app.", + inputSchema: z + .object({ + id: z.string().min(1), + title: z.string().min(1).max(255).optional(), + description: z.string().nullable().optional(), + projectId: z.string().nullable().optional(), + stateId: z.string().optional(), + assigneeId: z.string().nullable().optional(), + priority: z.number().int().min(0).max(4).optional(), + }) + .strict(), + outputSchema: issueResultSchema, + async execute({ id, ...update }) { + if (Object.values(update).every((value) => value === undefined)) { + throw new PluginToolInputError( + "At least one issue field must be updated.", + ); + } + const data = await linearGraphql<{ + issueUpdate: { + success: boolean; + issue: z.input | null; + }; + }>( + ctx, + "linear.issue.update", + `mutation UpdateIssue($id: String!, $input: IssueUpdateInput!) { issueUpdate(id: $id, input: $input) { success issue { ${issueSelection} } } }`, + { id, input: issueInput(update) }, + ); + if (!data.issueUpdate.success || !data.issueUpdate.issue) { + throw new Error("Linear did not update the issue."); + } + return issueResultSchema.parse({ issue: data.issueUpdate.issue }); + }, + }), + createComment: definePluginTool({ + annotations: { + destructiveHint: false, + idempotentHint: false, + openWorldHint: true, + readOnlyHint: false, + }, + description: + "Add a comment to a Linear issue as the installed Junior app.", + inputSchema: z + .object({ issueId: z.string().min(1), body: z.string().min(1) }) + .strict(), + outputSchema: z.object({ + comment: z.object({ + id: z.string(), + body: z.string(), + url: z.string().nullable().optional(), + }), + }), + async execute({ issueId, body }) { + const data = await linearGraphql<{ + commentCreate: { + success: boolean; + comment: { id: string; body: string; url?: string | null } | null; + }; + }>( + ctx, + "linear.comment.create", + `mutation CreateComment($input: CommentCreateInput!) { commentCreate(input: $input) { success comment { id body } } }`, + { input: { issueId, body } }, + ); + if (!data.commentCreate.success || !data.commentCreate.comment) { + throw new Error("Linear did not create the comment."); + } + return { comment: data.commentCreate.comment }; + }, + }), + listTeams: definePluginTool({ + annotations: { + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + readOnlyHint: true, + }, + description: + "List Linear teams to resolve a team UUID before issue creation.", + inputSchema: z.object({}).strict(), + outputSchema: z.object({ teams: z.array(teamSchema) }), + async execute() { + const data = await linearGraphql<{ + teams: { nodes: z.input[] }; + }>( + ctx, + "linear.team.list", + "query ListTeams { teams { nodes { id key name } } }", + {}, + ); + return { teams: z.array(teamSchema).parse(data.teams.nodes) }; + }, + }), + listProjects: definePluginTool({ + annotations: { + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + readOnlyHint: true, + }, + description: "List Linear projects to resolve a project UUID.", + inputSchema: z + .object({ first: z.number().int().min(1).max(100).default(50) }) + .strict(), + outputSchema: z.object({ projects: z.array(projectSchema) }), + async execute({ first }) { + const data = await linearGraphql<{ + projects: { nodes: z.input[] }; + }>( + ctx, + "linear.project.list", + "query ListProjects($first: Int!) { projects(first: $first) { nodes { id name } } }", + { first }, + ); + return { projects: z.array(projectSchema).parse(data.projects.nodes) }; + }, + }), + listWorkflowStates: definePluginTool({ + annotations: { + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + readOnlyHint: true, + }, + description: "List the workflow states for one Linear team.", + inputSchema: z.object({ teamId: z.string().min(1) }).strict(), + outputSchema: z.object({ states: z.array(workflowStateSchema) }), + async execute({ teamId }) { + const data = await linearGraphql<{ + team: { states: { nodes: z.input[] } }; + }>( + ctx, + "linear.workflow-state.list", + "query ListWorkflowStates($teamId: String!) { team(id: $teamId) { states { nodes { id name type } } } }", + { teamId }, + ); + return { + states: z.array(workflowStateSchema).parse(data.team.states.nodes), + }; + }, + }), + }; +} diff --git a/packages/junior-linear/tests/tools.test.ts b/packages/junior-linear/tests/tools.test.ts new file mode 100644 index 0000000000..b810380942 --- /dev/null +++ b/packages/junior-linear/tests/tools.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it, vi } from "vitest"; +import type { ToolRegistrationHookContext } from "@sentry/junior-plugin-api"; +import { createLinearTools } from "../src/tools"; + +function issue() { + return { + id: "issue-id", + identifier: "ENG-123", + title: "Native Linear tools", + description: null, + priority: 3, + url: "https://linear.app/acme/issue/ENG-123/native-linear-tools", + state: { id: "state-id", name: "Todo" }, + team: { id: "team-id", key: "ENG", name: "Engineering" }, + project: null, + }; +} + +function toolContext(data: unknown) { + const requests: Array<{ + operation: string; + provider: string; + request: Request; + }> = []; + const upsert = vi.fn(async () => {}); + const ctx = { + annotations: { upsert }, + egress: { + fetch: async (input: { + operation: string; + provider: string; + request: Request; + }) => { + requests.push(input); + return Response.json({ data }); + }, + }, + }; + // @ts-expect-error test supplies the tool-owned context only + const tools = createLinearTools(ctx as ToolRegistrationHookContext); + return { requests, tools, upsert }; +} + +describe("Linear native tools", () => { + it("gets an issue through host-owned Linear egress", async () => { + const { requests, tools } = toolContext({ issue: issue() }); + + await expect( + tools.getIssue?.execute?.({ id: "ENG-123" }, { toolCallId: "get" }), + ).resolves.toEqual({ issue: issue() }); + + expect(requests).toHaveLength(1); + expect(requests[0]).toMatchObject({ + operation: "linear.issue.get", + provider: "linear", + }); + expect(requests[0]?.request.headers.has("authorization")).toBe(false); + }); + + it("creates and annotates an issue", async () => { + const { requests, tools, upsert } = toolContext({ + issueCreate: { success: true, issue: issue() }, + }); + + await expect( + tools.createIssue?.execute?.( + { teamId: "team-id", title: "Native Linear tools" }, + { toolCallId: "create" }, + ), + ).resolves.toEqual({ issue: issue() }); + + expect(requests[0]).toMatchObject({ + operation: "linear.issue.create", + provider: "linear", + }); + expect(upsert).toHaveBeenCalledWith({ + kind: "resource_link", + key: "ENG-123", + label: "ENG-123", + status: "open", + url: issue().url, + }); + }); + + it("surfaces GraphQL errors as repairable tool errors", async () => { + const { tools } = toolContext(undefined); + const ctx = { + egress: { + fetch: async () => + Response.json({ errors: [{ message: "Issue not found" }] }), + }, + }; + // @ts-expect-error test supplies the tool-owned context only + const failingTools = createLinearTools(ctx as ToolRegistrationHookContext); + + await expect( + failingTools.getIssue?.execute?.( + { id: "ENG-404" }, + { toolCallId: "missing" }, + ), + ).rejects.toMatchObject({ + message: "Issue not found", + name: "PluginToolInputError", + }); + expect(tools.getIssue).toBeDefined(); + }); +}); diff --git a/packages/junior-plugin-api/src/manifest.ts b/packages/junior-plugin-api/src/manifest.ts index 43b7b4c59a..bd8549294f 100644 --- a/packages/junior-plugin-api/src/manifest.ts +++ b/packages/junior-plugin-api/src/manifest.ts @@ -4,6 +4,8 @@ export interface PluginOAuthConfig { clientIdEnv: string; clientSecretEnv: string; scope?: string; + /** Store completed OAuth grants for the installation instead of the authorizing user. */ + tokenSubject?: "installation" | "user"; /** * Treat a provider token response with `scope: ""` like an omitted scope and * fall back to the requested scope string when storing the token. diff --git a/packages/junior/src/chat/capabilities/factory.ts b/packages/junior/src/chat/capabilities/factory.ts index e9dc775bc6..b2732a691c 100644 --- a/packages/junior/src/chat/capabilities/factory.ts +++ b/packages/junior/src/chat/capabilities/factory.ts @@ -5,7 +5,11 @@ import type { CredentialLease, } from "@/chat/credentials/broker"; import type { CredentialContext } from "@/chat/credentials/context"; -import { StateAdapterTokenStore } from "@/chat/credentials/state-adapter-token-store"; +import { + StateAdapterInstallationTokenStore, + StateAdapterTokenStore, +} from "@/chat/credentials/state-adapter-token-store"; +import type { InstallationTokenStore } from "@/chat/credentials/installation-token-store"; import type { UserTokenStore } from "@/chat/credentials/user-token-store"; import { pluginCatalogRuntime } from "@/chat/plugins/catalog-runtime"; import { getStateAdapter } from "@/chat/state/adapter"; @@ -20,8 +24,13 @@ export function createUserTokenStore(): UserTokenStore { return new StateAdapterTokenStore(getStateAdapter()); } +/** Create the token store used by installation OAuth grants. */ +export function createInstallationTokenStore(): InstallationTokenStore { + return new StateAdapterInstallationTokenStore(getStateAdapter()); +} + function createProviderCredentialRouter( - userTokenStore: UserTokenStore, + stateAdapter: StateAdapter, ): ProviderCredentialRouter { const brokersByProvider: Record = {}; @@ -31,7 +40,10 @@ function createProviderCredentialRouter( continue; } brokersByProvider[name] = pluginCatalogRuntime.createBroker(name, { - userTokenStore, + installationTokenStore: new StateAdapterInstallationTokenStore( + stateAdapter, + ), + userTokenStore: new StateAdapterTokenStore(stateAdapter), }); } @@ -42,9 +54,7 @@ function getSandboxEgressRouter(): ProviderCredentialRouter { const stateAdapter = getStateAdapter(); let router = sandboxEgressRouters.get(stateAdapter); if (!router) { - router = createProviderCredentialRouter( - new StateAdapterTokenStore(stateAdapter), - ); + router = createProviderCredentialRouter(stateAdapter); sandboxEgressRouters.set(stateAdapter, router); } return router; diff --git a/packages/junior/src/chat/credentials/installation-token-store.ts b/packages/junior/src/chat/credentials/installation-token-store.ts new file mode 100644 index 0000000000..d5cf087a66 --- /dev/null +++ b/packages/junior/src/chat/credentials/installation-token-store.ts @@ -0,0 +1,10 @@ +import type { StoredTokens } from "@/chat/credentials/user-token-store"; + +/** Persistent OAuth token storage shared by one Junior installation. */ +export interface InstallationTokenStore { + get(provider: string): Promise; + set(provider: string, tokens: StoredTokens): Promise; + delete(provider: string): Promise; + /** Run refresh-token rotation for one provider slot, or throw after a bounded wait. */ + withRefresh(provider: string, callback: () => Promise): Promise; +} diff --git a/packages/junior/src/chat/credentials/oauth-scope.ts b/packages/junior/src/chat/credentials/oauth-scope.ts index ab04a75808..0e60efe093 100644 --- a/packages/junior/src/chat/credentials/oauth-scope.ts +++ b/packages/junior/src/chat/credentials/oauth-scope.ts @@ -3,7 +3,7 @@ function parseScope(scope?: string): string[] { return []; } - return [...new Set(scope.split(/\s+/).filter(Boolean))].sort(); + return [...new Set(scope.split(/[\s,]+/).filter(Boolean))].sort(); } /** Normalize OAuth scope strings so persisted grants can be compared reliably. */ diff --git a/packages/junior/src/chat/credentials/state-adapter-token-store.ts b/packages/junior/src/chat/credentials/state-adapter-token-store.ts index 4d484480ec..f4ff590bcf 100644 --- a/packages/junior/src/chat/credentials/state-adapter-token-store.ts +++ b/packages/junior/src/chat/credentials/state-adapter-token-store.ts @@ -3,11 +3,13 @@ import type { StoredTokens, UserTokenStore, } from "@/chat/credentials/user-token-store"; +import type { InstallationTokenStore } from "@/chat/credentials/installation-token-store"; import { storedTokensSchema } from "@/chat/credentials/user-token-store"; import { sleep } from "@/chat/sleep"; import { acquireActiveLock } from "@/chat/state/locks"; const KEY_PREFIX = "oauth-token"; +const INSTALLATION_KEY_PREFIX = "oauth-installation-token"; const BUFFER_MS = 24 * 60 * 60 * 1000; // 24h buffer for refresh token lifetime const LONG_LIVED_TTL_MS = 365 * 24 * 60 * 60 * 1000; const REFRESH_LOCK_WAIT_MS = 30_000; @@ -17,8 +19,37 @@ function tokenKey(userId: string, provider: string): string { return `${KEY_PREFIX}:${userId}:${provider}`; } -function refreshLockKey(userId: string, provider: string): string { - return `${tokenKey(userId, provider)}:refresh`; +function installationTokenKey(provider: string): string { + return `${INSTALLATION_KEY_PREFIX}:${provider}`; +} + +function tokenTtlMs(tokens: StoredTokens): number { + const expiresAt = tokens.refreshTokenExpiresAt ?? tokens.expiresAt; + return expiresAt + ? Math.max(expiresAt - Date.now() + BUFFER_MS, BUFFER_MS) + : LONG_LIVED_TTL_MS; +} + +async function withTokenRefresh( + state: StateAdapter, + key: string, + callback: () => Promise, +): Promise { + const deadline = Date.now() + REFRESH_LOCK_WAIT_MS; + while (true) { + const lock = await acquireActiveLock(state, `${key}:refresh`); + if (lock) { + try { + return await callback(); + } finally { + await state.releaseLock(lock); + } + } + if (Date.now() >= deadline) { + throw new Error("Could not acquire OAuth token refresh lock"); + } + await sleep(REFRESH_LOCK_RETRY_MS); + } } export class StateAdapterTokenStore implements UserTokenStore { @@ -44,11 +75,11 @@ export class StateAdapterTokenStore implements UserTokenStore { tokens: StoredTokens, ): Promise { const parsed = storedTokensSchema.parse(tokens); - const expiresAt = parsed.refreshTokenExpiresAt ?? parsed.expiresAt; - const ttlMs = expiresAt - ? Math.max(expiresAt - Date.now() + BUFFER_MS, BUFFER_MS) - : LONG_LIVED_TTL_MS; - await this.state.set(tokenKey(userId, provider), parsed, ttlMs); + await this.state.set( + tokenKey(userId, provider), + parsed, + tokenTtlMs(parsed), + ); } async delete(userId: string, provider: string): Promise { @@ -61,21 +92,47 @@ export class StateAdapterTokenStore implements UserTokenStore { provider: string, callback: () => Promise, ): Promise { - const lockKey = refreshLockKey(userId, provider); - const deadline = Date.now() + REFRESH_LOCK_WAIT_MS; - while (true) { - const lock = await acquireActiveLock(this.state, lockKey); - if (lock) { - try { - return await callback(); - } finally { - await this.state.releaseLock(lock); - } - } - if (Date.now() >= deadline) { - throw new Error(`Could not acquire OAuth token refresh lock`); - } - await sleep(REFRESH_LOCK_RETRY_MS); - } + return await withTokenRefresh( + this.state, + tokenKey(userId, provider), + callback, + ); + } +} + +export class StateAdapterInstallationTokenStore implements InstallationTokenStore { + constructor(private readonly state: StateAdapter) {} + + async get(provider: string): Promise { + const stored = await this.state.get( + installationTokenKey(provider), + ); + return stored === null || stored === undefined + ? undefined + : storedTokensSchema.parse(stored); + } + + async set(provider: string, tokens: StoredTokens): Promise { + const parsed = storedTokensSchema.parse(tokens); + await this.state.set( + installationTokenKey(provider), + parsed, + tokenTtlMs(parsed), + ); + } + + async delete(provider: string): Promise { + await this.state.delete(installationTokenKey(provider)); + } + + async withRefresh( + provider: string, + callback: () => Promise, + ): Promise { + return await withTokenRefresh( + this.state, + installationTokenKey(provider), + callback, + ); } } diff --git a/packages/junior/src/chat/local/credential-sync.ts b/packages/junior/src/chat/local/credential-sync.ts index f503f557be..09751f1810 100644 --- a/packages/junior/src/chat/local/credential-sync.ts +++ b/packages/junior/src/chat/local/credential-sync.ts @@ -4,7 +4,11 @@ import { type PluginStoredTokens, } from "@sentry/junior-plugin-api"; import { z } from "zod"; -import { createUserTokenStore } from "@/chat/capabilities/factory"; +import { + createInstallationTokenStore, + createUserTokenStore, +} from "@/chat/capabilities/factory"; +import { pluginCatalogRuntime } from "@/chat/plugins/catalog-runtime"; const LOCAL_CREDENTIAL_SYNC_CONTEXT = "junior.local-credential-sync.v1"; const LOCAL_CREDENTIAL_SYNC_MAX_AGE_MS = 60_000; @@ -106,10 +110,18 @@ export async function receiveLocalOAuthCredential( ) { return Response.json({ error: "Invalid request" }, { status: 400 }); } - await createUserTokenStore().set( - "local-cli", - payload.data.provider, - payload.data.tokens, - ); + const provider = payload.data.provider; + if ( + pluginCatalogRuntime.getOAuthConfig(provider)?.tokenSubject === + "installation" + ) { + await createInstallationTokenStore().set(provider, payload.data.tokens); + } else { + await createUserTokenStore().set( + "local-cli", + provider, + payload.data.tokens, + ); + } return new Response(null, { status: 204 }); } diff --git a/packages/junior/src/chat/oauth-flow.ts b/packages/junior/src/chat/oauth-flow.ts index 9c6e7efe8a..b107e7dd31 100644 --- a/packages/junior/src/chat/oauth-flow.ts +++ b/packages/junior/src/chat/oauth-flow.ts @@ -308,7 +308,10 @@ export async function startOAuthFlow( const authorizationUrl = `${providerConfig.authorizeEndpoint}?${authorizeParams.toString()}`; const authorizationRequest = { authorizationUrl, - label: `Click here to link your ${formatProviderLabel(provider)} account`, + label: + providerConfig.tokenSubject === "installation" + ? `Click here to install ${formatProviderLabel(provider)}` + : `Click here to link your ${formatProviderLabel(provider)} account`, completionText: input.resumeSessionId ? "Once you've authorized, Junior will continue automatically." : "Once you've authorized, you'll see a confirmation in Slack.", diff --git a/packages/junior/src/chat/plugins/auth/oauth-bearer-broker.ts b/packages/junior/src/chat/plugins/auth/oauth-bearer-broker.ts index cc481bd7e9..4e351003d3 100644 --- a/packages/junior/src/chat/plugins/auth/oauth-bearer-broker.ts +++ b/packages/junior/src/chat/plugins/auth/oauth-bearer-broker.ts @@ -15,6 +15,7 @@ import type { StoredTokens, UserTokenStore, } from "@/chat/credentials/user-token-store"; +import type { InstallationTokenStore } from "@/chat/credentials/installation-token-store"; import { resolvePluginCommandEnv } from "@/chat/plugins/command-env"; import { resolveAuthTokenPlaceholder } from "./auth-token-placeholder"; import { resolveApiHeaderTransforms } from "./api-headers-broker"; @@ -155,7 +156,10 @@ function shouldRefreshStoredToken(stored: StoredTokens | undefined): boolean { export function createOAuthBearerBroker( manifest: PluginManifest, credentials: OAuthBearerCredentials, - deps: { userTokenStore: UserTokenStore }, + deps: { + installationTokenStore?: InstallationTokenStore; + userTokenStore: UserTokenStore; + }, ): CredentialBroker { const provider = manifest.name; const { domains, apiHeaders, authTokenEnv } = credentials; @@ -186,6 +190,110 @@ export function createOAuthBearerBroker( }; } + async function issueStoredToken(input: { + delete?: () => Promise; + get(): Promise; + set(tokens: StoredTokens): Promise; + withRefresh(callback: () => Promise): Promise; + reason: string; + subjectLabel: string; + }): Promise { + const oauth = manifest.oauth; + if (!oauth) { + throw new CredentialUnavailableError( + provider, + `No ${provider} credentials available.`, + ); + } + const stored = await input.get(); + if (!stored) { + throw new CredentialUnavailableError( + provider, + `No ${provider} credentials available.`, + ); + } + if (!hasRequiredOAuthScope(stored.scope, oauth.scope)) { + throw new CredentialUnavailableError( + provider, + `${input.subjectLabel} ${provider} connection needs to be reauthorized.`, + ); + } + if (!shouldRefreshStoredToken(stored)) { + if (canUseStoredToken(stored)) { + return buildLease( + stored.accessToken, + getLeaseExpiry(stored.expiresAt), + input.reason, + ); + } + throw new CredentialUnavailableError( + provider, + `${input.subjectLabel} ${provider} connection has expired.`, + ); + } + try { + return await input.withRefresh(async () => { + const latest = await input.get(); + if (!latest) { + throw new CredentialUnavailableError( + provider, + `No ${provider} credentials available.`, + ); + } + if (!hasRequiredOAuthScope(latest.scope, oauth.scope)) { + throw new CredentialUnavailableError( + provider, + `${input.subjectLabel} ${provider} connection needs to be reauthorized.`, + ); + } + if (!shouldRefreshStoredToken(latest) && canUseStoredToken(latest)) { + return buildLease( + latest.accessToken, + getLeaseExpiry(latest.expiresAt), + input.reason, + ); + } + const refreshed = await refreshAccessToken( + provider, + latest.refreshToken, + oauth, + latest.scope ?? oauth.scope, + ); + if (!hasRequiredOAuthScope(refreshed.scope, oauth.scope)) { + throw new CredentialUnavailableError( + provider, + `${input.subjectLabel} ${provider} connection needs to be reauthorized.`, + ); + } + const refreshedTokens = { + ...(latest.refreshTokenExpiresAt + ? { refreshTokenExpiresAt: latest.refreshTokenExpiresAt } + : undefined), + ...refreshed, + ...(latest.account ? { account: latest.account } : undefined), + }; + await input.set(refreshedTokens); + return buildLease( + refreshed.accessToken, + getLeaseExpiry(refreshed.expiresAt), + input.reason, + ); + }); + } catch (error) { + if (error instanceof CredentialUnavailableError) { + throw error; + } + if (error instanceof OAuthRefreshRejectedError) { + await input.delete?.(); + throw new CredentialUnavailableError( + provider, + `${input.subjectLabel} ${provider} connection has expired.`, + ); + } + throw error; + } + } + return { async issue(input) { const envToken = process.env[authTokenEnv]?.trim(); @@ -202,115 +310,41 @@ export function createOAuthBearerBroker( ); } - if (userSubjectId) { - const stored = await deps.userTokenStore.get(userSubjectId, provider); - if (stored) { - if (!hasRequiredOAuthScope(stored.scope, oauth.scope)) { - throw new CredentialUnavailableError( - provider, - `Your ${provider} connection needs to be reauthorized.`, - ); - } - - if (shouldRefreshStoredToken(stored)) { - try { - return await deps.userTokenStore.withRefresh( - userSubjectId, - provider, - async () => { - const latest = await deps.userTokenStore.get( - userSubjectId, - provider, - ); - if ( - latest && - !hasRequiredOAuthScope(latest.scope, oauth.scope) - ) { - throw new CredentialUnavailableError( - provider, - `Your ${provider} connection needs to be reauthorized.`, - ); - } - if ( - !shouldRefreshStoredToken(latest) && - canUseStoredToken(latest) - ) { - return buildLease( - latest.accessToken, - getLeaseExpiry(latest.expiresAt), - input.reason, - ); - } - if (!latest) { - throw new CredentialUnavailableError( - provider, - `No ${provider} credentials available.`, - ); - } - - const refreshed = await refreshAccessToken( - provider, - latest.refreshToken, - oauth, - latest.scope ?? oauth.scope, - ); - if (!hasRequiredOAuthScope(refreshed.scope, oauth.scope)) { - throw new CredentialUnavailableError( - provider, - `Your ${provider} connection needs to be reauthorized.`, - ); - } - const refreshedTokens = { - ...(latest.refreshTokenExpiresAt - ? { refreshTokenExpiresAt: latest.refreshTokenExpiresAt } - : undefined), - ...refreshed, - ...(latest.account ? { account: latest.account } : undefined), - }; - await deps.userTokenStore.set( - userSubjectId, - provider, - refreshedTokens, - ); - return buildLease( - refreshed.accessToken, - getLeaseExpiry(refreshed.expiresAt), - input.reason, - ); - }, - ); - } catch (error) { - if (error instanceof CredentialUnavailableError) { - throw error; - } - if (error instanceof OAuthRefreshRejectedError) { - throw new CredentialUnavailableError( - provider, - `Your ${provider} connection has expired.`, - ); - } - throw error; - } - } - - if (canUseStoredToken(stored)) { - return buildLease( - stored.accessToken, - getLeaseExpiry(stored.expiresAt), - input.reason, - ); - } - + if (oauth.tokenSubject === "installation") { + const installationTokenStore = deps.installationTokenStore; + if (!installationTokenStore) { throw new CredentialUnavailableError( provider, - `Your ${provider} connection has expired.`, + `No ${provider} installation token store is configured.`, ); } + return await issueStoredToken({ + delete: async () => await installationTokenStore.delete(provider), + get: async () => await installationTokenStore.get(provider), + set: async (tokens) => + await installationTokenStore.set(provider, tokens), + withRefresh: async (callback) => + await installationTokenStore.withRefresh(provider, callback), + reason: input.reason, + subjectLabel: "The installation's", + }); + } - throw new CredentialUnavailableError( - provider, - `No ${provider} credentials available.`, - ); + if (userSubjectId) { + return await issueStoredToken({ + get: async () => + await deps.userTokenStore.get(userSubjectId, provider), + set: async (tokens) => + await deps.userTokenStore.set(userSubjectId, provider, tokens), + withRefresh: async (callback) => + await deps.userTokenStore.withRefresh( + userSubjectId, + provider, + callback, + ), + reason: input.reason, + subjectLabel: "Your", + }); } if (envToken) { diff --git a/packages/junior/src/chat/plugins/inline-manifest-source.ts b/packages/junior/src/chat/plugins/inline-manifest-source.ts index 8ce502c8c2..44431393f4 100644 --- a/packages/junior/src/chat/plugins/inline-manifest-source.ts +++ b/packages/junior/src/chat/plugins/inline-manifest-source.ts @@ -79,6 +79,7 @@ function inlineOauthSource(oauth: PluginManifest["oauth"]): unknown { setDefined(result, "authorize-endpoint", oauth.authorizeEndpoint); setDefined(result, "token-endpoint", oauth.tokenEndpoint); setDefined(result, "scope", oauth.scope); + setDefined(result, "token-subject", oauth.tokenSubject); setDefined(result, "authorize-params", oauth.authorizeParams); setDefined(result, "token-auth-method", oauth.tokenAuthMethod); setDefined(result, "token-extra-headers", oauth.tokenExtraHeaders); diff --git a/packages/junior/src/chat/plugins/manifest.ts b/packages/junior/src/chat/plugins/manifest.ts index e70003fd91..23bbb52212 100644 --- a/packages/junior/src/chat/plugins/manifest.ts +++ b/packages/junior/src/chat/plugins/manifest.ts @@ -216,6 +216,7 @@ const oauthSourceSchema = z "authorize-endpoint": httpsUrlString, "token-endpoint": httpsUrlString, scope: nonEmptyTrimmedString.optional(), + "token-subject": z.enum(["installation", "user"]).optional(), "authorize-params": stringMapSchema.optional(), "token-extra-headers": stringMapSchema.optional(), "token-auth-method": nonEmptyTrimmedString @@ -375,6 +376,7 @@ function manifestConfigPatch( setDefined(oauth, "authorize-endpoint", config.oauth.authorizeEndpoint); setDefined(oauth, "token-endpoint", config.oauth.tokenEndpoint); setDefined(oauth, "scope", config.oauth.scope); + setDefined(oauth, "token-subject", config.oauth.tokenSubject); setDefined(oauth, "authorize-params", config.oauth.authorizeParams); setDefined(oauth, "token-auth-method", config.oauth.tokenAuthMethod); setDefined(oauth, "token-extra-headers", config.oauth.tokenExtraHeaders); @@ -1134,6 +1136,9 @@ function parseManifestSource( authorizeEndpoint: result.data["authorize-endpoint"], tokenEndpoint: result.data["token-endpoint"], ...(result.data.scope ? { scope: result.data.scope } : undefined), + ...(result.data["token-subject"] + ? { tokenSubject: result.data["token-subject"] } + : undefined), ...(authorizeParams ? { authorizeParams } : undefined), ...(result.data["token-auth-method"] ? { tokenAuthMethod: result.data["token-auth-method"] } diff --git a/packages/junior/src/chat/plugins/registry.ts b/packages/junior/src/chat/plugins/registry.ts index 8b030dc90d..e629e026f7 100644 --- a/packages/junior/src/chat/plugins/registry.ts +++ b/packages/junior/src/chat/plugins/registry.ts @@ -511,7 +511,9 @@ export function createPluginCatalogRuntime(): PluginCatalogRuntime { commands.push({ cmd: command.cmd, ...(command.args ? { args: [...command.args] } : undefined), - ...(command.sudo !== undefined ? { sudo: command.sudo } : undefined), + ...(command.sudo !== undefined + ? { sudo: command.sudo } + : undefined), }); } } @@ -528,6 +530,9 @@ export function createPluginCatalogRuntime(): PluginCatalogRuntime { authorizeEndpoint: oauth.authorizeEndpoint, tokenEndpoint: oauth.tokenEndpoint, ...(oauth.scope ? { scope: oauth.scope } : undefined), + ...(oauth.tokenSubject + ? { tokenSubject: oauth.tokenSubject } + : undefined), ...(oauth.authorizeParams ? { authorizeParams: { ...oauth.authorizeParams } } : undefined), diff --git a/packages/junior/src/chat/plugins/types.ts b/packages/junior/src/chat/plugins/types.ts index 515034b33b..6cd748e9fb 100644 --- a/packages/junior/src/chat/plugins/types.ts +++ b/packages/junior/src/chat/plugins/types.ts @@ -4,6 +4,7 @@ import type { PluginRuntimePostinstallCommand, } from "@sentry/junior-plugin-api"; import type { UserTokenStore } from "@/chat/credentials/user-token-store"; +import type { InstallationTokenStore } from "@/chat/credentials/installation-token-store"; export type { PluginNpmRuntimeDependency, @@ -20,6 +21,7 @@ export interface PluginOAuthConfig { authorizeEndpoint: string; tokenEndpoint: string; scope?: string; + tokenSubject?: "installation" | "user"; /** * Set true when the provider returns an empty scope string even for authorized * grants (e.g. GitHub App user-to-server tokens always return `scope: ""` @@ -143,6 +145,7 @@ export interface PluginCatalogConfig { } export interface PluginBrokerDeps { + installationTokenStore?: InstallationTokenStore; userTokenStore: UserTokenStore; } diff --git a/packages/junior/src/handlers/oauth-callback.ts b/packages/junior/src/handlers/oauth-callback.ts index eec5bcbd2f..2696bcf101 100644 --- a/packages/junior/src/handlers/oauth-callback.ts +++ b/packages/junior/src/handlers/oauth-callback.ts @@ -1,4 +1,7 @@ -import { createUserTokenStore } from "@/chat/capabilities/factory"; +import { + createInstallationTokenStore, + createUserTokenStore, +} from "@/chat/capabilities/factory"; import { hasRequiredOAuthScope } from "@/chat/credentials/oauth-scope"; import { coerceThreadConversationState } from "@/chat/state/conversation"; import { hydrateConversationMessages } from "@/chat/conversations/messages"; @@ -681,6 +684,7 @@ export async function GET( } const userTokenStore = createUserTokenStore(); + const installationTokenStore = createInstallationTokenStore(); let account: Awaited>; try { account = await resolvePluginOAuthAccount({ @@ -694,13 +698,18 @@ export async function GET( 500, ); } - await userTokenStore.set(stored.userId, provider, { + const storedTokens = { ...parsedTokenResponse, ...(account ? { account } : undefined), - }); + }; + if (providerConfig.tokenSubject === "installation") { + await installationTokenStore.set(provider, storedTokens); + } else { + await userTokenStore.set(stored.userId, provider, storedTokens); + } const slackActor = stored.actor?.platform === "slack" ? stored.actor : undefined; - if (account && slackActor) { + if (providerConfig.tokenSubject !== "installation" && account && slackActor) { await runBestEffort( async () => { const slackUserId = parseSlackUserId(slackActor.userId); diff --git a/packages/junior/tests/unit/plugins/plugin-manifest-config.test.ts b/packages/junior/tests/unit/plugins/plugin-manifest-config.test.ts index 244e4bc7a7..285239eab8 100644 --- a/packages/junior/tests/unit/plugins/plugin-manifest-config.test.ts +++ b/packages/junior/tests/unit/plugins/plugin-manifest-config.test.ts @@ -77,6 +77,30 @@ describe("plugin manifest config", () => { expect(manifest.oauth?.treatEmptyScopeAsUnreported).toBe(true); }); + it("parses installation OAuth token subjects", () => { + const manifest = parsePluginManifest( + [ + "name: linear", + "display-name: Linear", + "description: Linear", + "credentials:", + " type: oauth-bearer", + " domains:", + " - api.linear.app", + " auth-token-env: LINEAR_ACCESS_TOKEN", + "oauth:", + " client-id-env: LINEAR_CLIENT_ID", + " client-secret-env: LINEAR_CLIENT_SECRET", + " authorize-endpoint: https://linear.app/oauth/authorize", + " token-endpoint: https://api.linear.app/oauth/token", + " token-subject: installation", + ].join("\n"), + "/plugins/linear", + ); + + expect(manifest.oauth?.tokenSubject).toBe("installation"); + }); + it("parses treat-empty-scope-as-unreported from YAML oauth block", () => { const manifest = parsePluginManifest( [ diff --git a/packages/junior/tests/unit/plugins/sentry-broker.test.ts b/packages/junior/tests/unit/plugins/sentry-broker.test.ts index 0a11173b30..c64e5d9905 100644 --- a/packages/junior/tests/unit/plugins/sentry-broker.test.ts +++ b/packages/junior/tests/unit/plugins/sentry-broker.test.ts @@ -10,6 +10,7 @@ import type { StoredTokens, UserTokenStore, } from "@/chat/credentials/user-token-store"; +import type { InstallationTokenStore } from "@/chat/credentials/installation-token-store"; const ORIGINAL_ENV = { ...process.env }; const ORIGINAL_FETCH = globalThis.fetch; @@ -62,6 +63,22 @@ function createMockTokenStore( }; } +function createMockInstallationTokenStore( + token?: StoredTokens, +): InstallationTokenStore { + let stored = token; + return { + get: async () => stored, + set: async (_provider, tokens) => { + stored = tokens; + }, + delete: async () => { + stored = undefined; + }, + withRefresh: async (_provider, callback) => callback(), + }; +} + function createBroker(tokenStore?: UserTokenStore) { return createOAuthBearerBroker( SENTRY_MANIFEST, @@ -77,6 +94,45 @@ afterEach(() => { }); describe("sentry credential broker (oauth-bearer plugin)", () => { + it("issues a lease from an installation OAuth token for system runs", async () => { + const manifest: PluginManifest = { + ...SENTRY_MANIFEST, + oauth: { + ...SENTRY_MANIFEST.oauth!, + tokenSubject: "installation", + }, + }; + const broker = createOAuthBearerBroker( + manifest, + manifest.credentials as OAuthBearerCredentials, + { + installationTokenStore: createMockInstallationTokenStore({ + accessToken: "installation-access-token", + refreshToken: "installation-refresh-token", + expiresAt: Date.now() + 60 * 60 * 1000, + scope: SENTRY_SCOPE, + }), + userTokenStore: createMockTokenStore(), + }, + ); + + const lease = await broker.issue({ + context: SYSTEM_CREDENTIAL_CONTEXT, + reason: "test:installation-oauth", + }); + + expect(lease.headerTransforms).toEqual([ + { + domain: "us.sentry.io", + headers: { Authorization: "Bearer installation-access-token" }, + }, + { + domain: "de.sentry.io", + headers: { Authorization: "Bearer installation-access-token" }, + }, + ]); + }); + it("issues a lease from a per-user OAuth token", async () => { const tokenStore = createMockTokenStore({ "U123:sentry": { @@ -200,14 +256,14 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { }, }); - // @ts-expect-error non-overlapping boundary cast; rule forbids as-unknown-as chains - globalThis.fetch = (vi.fn(async () => ({ - ok: true, - json: async () => ({ - access_token: "new-access-token", - expires_in: 3600, - }), - }))) as typeof fetch; + // @ts-expect-error non-overlapping boundary cast; rule forbids as-unknown-as chains + globalThis.fetch = vi.fn(async () => ({ + ok: true, + json: async () => ({ + access_token: "new-access-token", + expires_in: 3600, + }), + })) as typeof fetch; const broker = createBroker(tokenStore); const lease = await broker.issue({ @@ -256,16 +312,16 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { }, }); - // @ts-expect-error non-overlapping boundary cast; rule forbids as-unknown-as chains - globalThis.fetch = (vi.fn(async () => ({ - ok: true, - json: async () => ({ - access_token: "new-access-token", - refresh_token: "new-refresh-token", - expires_in: 3600, - refresh_token_expires_in: 7200, - }), - }))) as typeof fetch; + // @ts-expect-error non-overlapping boundary cast; rule forbids as-unknown-as chains + globalThis.fetch = vi.fn(async () => ({ + ok: true, + json: async () => ({ + access_token: "new-access-token", + refresh_token: "new-refresh-token", + expires_in: 3600, + refresh_token_expires_in: 7200, + }), + })) as typeof fetch; const broker = createBroker(tokenStore); await broker.issue({ @@ -305,7 +361,7 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { delete: vi.fn(), withRefresh, }; - globalThis.fetch = (vi.fn()) as typeof fetch; + globalThis.fetch = vi.fn() as typeof fetch; const broker = createBroker(tokenStore); const lease = await broker.issue({ @@ -340,12 +396,12 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { }, }); - globalThis.fetch = (vi.fn( - async () => - new Response(JSON.stringify({ error: "invalid_grant" }), { - status: 400, - }), - )) as typeof fetch; + globalThis.fetch = vi.fn( + async () => + new Response(JSON.stringify({ error: "invalid_grant" }), { + status: 400, + }), + ) as typeof fetch; const broker = createBroker(tokenStore); await expect( @@ -370,12 +426,12 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { }); const providerText = "SENSITIVE_CANARY"; - globalThis.fetch = (vi.fn( - async () => - new Response(JSON.stringify({ error: providerText }), { - status: 500, - }), - )) as typeof fetch; + globalThis.fetch = vi.fn( + async () => + new Response(JSON.stringify({ error: providerText }), { + status: 500, + }), + ) as typeof fetch; const broker = createBroker(tokenStore); const error = await broker @@ -420,7 +476,9 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { cancelled = true; }, }); - globalThis.fetch = (vi.fn(async () => new Response(body, { status: 500 }))) as typeof fetch; + globalThis.fetch = vi.fn( + async () => new Response(body, { status: 500 }), + ) as typeof fetch; const broker = createBroker(tokenStore); await expect( @@ -451,7 +509,9 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { controller.error(new Error(providerText)); }, }); - globalThis.fetch = (vi.fn(async () => new Response(body, { status: 500 }))) as typeof fetch; + globalThis.fetch = vi.fn( + async () => new Response(body, { status: 500 }), + ) as typeof fetch; const broker = createBroker(tokenStore); const error = await broker diff --git a/packages/junior/tests/unit/state/state-adapter-token-store.test.ts b/packages/junior/tests/unit/state/state-adapter-token-store.test.ts index 090c0ccbfc..aec104a06c 100644 --- a/packages/junior/tests/unit/state/state-adapter-token-store.test.ts +++ b/packages/junior/tests/unit/state/state-adapter-token-store.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it, vi } from "vitest"; import type { StateAdapter } from "chat"; -import { StateAdapterTokenStore } from "@/chat/credentials/state-adapter-token-store"; +import { + StateAdapterInstallationTokenStore, + StateAdapterTokenStore, +} from "@/chat/credentials/state-adapter-token-store"; import { ACTIVE_LOCK_TTL_MS } from "@/chat/state/locks"; describe("StateAdapterTokenStore", () => { @@ -71,6 +74,25 @@ describe("StateAdapterTokenStore", () => { ); }); + it("stores installation tokens outside user slots", async () => { + const adapter = createAdapter(); + const store = new StateAdapterInstallationTokenStore(adapter); + + await store.set("linear", { + accessToken: "access-token", + refreshToken: "refresh-token", + }); + + expect(adapter.set).toHaveBeenCalledWith( + "oauth-installation-token:linear", + { + accessToken: "access-token", + refreshToken: "refresh-token", + }, + 365 * 24 * 60 * 60 * 1000, + ); + }); + it("waits for the refresh lock before running the callback", async () => { const lock = { key: "oauth-token:U123:github:refresh", lockId: "lock-id" }; const acquireLock = vi From 8491c42e17eb43fe06e6c28e318cf217046a334a Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:03:38 +0000 Subject: [PATCH 2/8] test(linear): Update native tool coverage --- .../component/linear-native-create.test.ts | 106 ++++++ .../integration/linear-native-create.test.ts | 310 ------------------ .../capabilities/capability-factory.test.ts | 2 + .../tests/unit/linear/after-mcp-tool.test.ts | 136 -------- 4 files changed, 108 insertions(+), 446 deletions(-) create mode 100644 packages/junior/tests/component/linear-native-create.test.ts delete mode 100644 packages/junior/tests/integration/linear-native-create.test.ts delete mode 100644 packages/junior/tests/unit/linear/after-mcp-tool.test.ts diff --git a/packages/junior/tests/component/linear-native-create.test.ts b/packages/junior/tests/component/linear-native-create.test.ts new file mode 100644 index 0000000000..ecde85da46 --- /dev/null +++ b/packages/junior/tests/component/linear-native-create.test.ts @@ -0,0 +1,106 @@ +import { createLocalSource } from "@sentry/junior-plugin-api"; +import { afterEach, describe, expect, it } from "vitest"; +import { linearPlugin } from "../../../junior-linear/src/index.js"; +import { getConversationStore, getDb } from "@/chat/db"; +import { + getPluginTools, + setPlugins, +} from "@/chat/plugins/agent-hooks"; +import { listConversationAnnotations } from "@/chat/plugins/annotations"; + +const conversationId = "local:test:linear-native-create"; +const destination = { platform: "local" as const, conversationId }; + +describe("Linear native create", () => { + let previousPlugins: ReturnType | undefined; + + afterEach(() => { + if (previousPlugins) { + setPlugins(previousPlugins); + previousPlugins = undefined; + } + }); + + it("creates and annotates an issue through plugin tool wiring", async () => { + previousPlugins = setPlugins([linearPlugin()]); + await getConversationStore().recordActivity({ + conversationId, + destination, + nowMs: Date.now(), + source: "local", + title: "Linear native create", + }); + const requests: Array<{ + operation: string; + provider: string; + request: Request; + }> = []; + const tools = getPluginTools({ + conversationId, + destination, + egress: { + async fetch(input) { + requests.push(input); + return Response.json({ + data: { + issueCreate: { + success: true, + issue: { + id: "issue-id", + identifier: "ENG-123", + title: "Native Linear issue", + description: null, + priority: 3, + url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", + state: { id: "state-id", name: "Todo" }, + team: { + id: "team-id", + key: "ENG", + name: "Engineering", + }, + project: null, + }, + }, + }, + }); + }, + }, + source: createLocalSource(conversationId), + workspace: {} as never, + }); + const createIssue = tools.linear_createIssue; + if (!createIssue?.execute) { + throw new Error("linear_createIssue tool is missing"); + } + + await expect( + createIssue.execute( + { teamId: "team-id", title: "Native Linear issue" }, + { toolCallId: "create-linear-issue" }, + ), + ).resolves.toMatchObject({ + issue: { + identifier: "ENG-123", + url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", + }, + }); + expect(requests).toHaveLength(1); + expect(requests[0]).toMatchObject({ + operation: "linear.issue.create", + provider: "linear", + }); + expect(requests[0]?.request.headers.has("authorization")).toBe(false); + await expect( + listConversationAnnotations(getDb(), conversationId), + ).resolves.toMatchObject([ + { + kind: "resource_link", + key: "ENG-123", + label: "ENG-123", + plugin: "linear", + status: "open", + url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", + }, + ]); + }); +}); diff --git a/packages/junior/tests/integration/linear-native-create.test.ts b/packages/junior/tests/integration/linear-native-create.test.ts deleted file mode 100644 index 7f59e691e5..0000000000 --- a/packages/junior/tests/integration/linear-native-create.test.ts +++ /dev/null @@ -1,310 +0,0 @@ -import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js"; -import { http } from "msw"; -import { afterEach, describe, expect, it } from "vitest"; -import { linearPlugin } from "../../../junior-linear/src/index.js"; -import { getConversationStore, getDb } from "@/chat/db"; -import { McpToolManager } from "@/chat/mcp/tool-manager"; -import { - createPluginHookRunner, - setPlugins, -} from "@/chat/plugins/agent-hooks"; -import { listConversationAnnotations } from "@/chat/plugins/annotations"; -import { parseInlinePluginManifest } from "@/chat/plugins/manifest"; -import { mswServer } from "../msw/server"; -import { z } from "zod"; - -describe("Linear MCP create annotations", () => { - let manager: McpToolManager | undefined; - let transport: WebStandardStreamableHTTPServerTransport | undefined; - - afterEach(async () => { - await manager?.close(); - await transport?.close(); - manager = undefined; - transport = undefined; - }); - - it("annotates created issues from live save_issue calls and skips updates", async () => { - const saveCalls: Array> = []; - const server = new McpServer({ name: "linear-test", version: "1.0.0" }); - server.registerTool( - "save_issue", - { - description: "Create or update a Linear issue", - inputSchema: { - id: z.string().optional(), - team: z.string().optional(), - title: z.string().optional(), - description: z.string().optional(), - priority: z.enum(["low", "medium", "high", "urgent"]).optional(), - project: z.string().optional(), - state: z.string().optional(), - }, - }, - (input) => { - saveCalls.push(input); - const action = input.id ? "Updated" : "Created"; - const issueUrl = - "https://linear.app/acme/issue/ENG-123/native-linear-issue"; - return { - content: [ - { - type: "text", - text: `${action} [ENG-123](${issueUrl})`, - }, - ], - // Model-visible MCP results prefer structuredContent over text. - structuredContent: { - issue: { - id: "issue-id", - identifier: "ENG-123", - title: input.title, - url: issueUrl, - }, - }, - }; - }, - ); - server.registerTool( - "get_issue", - { - description: "Get a Linear issue", - inputSchema: { identifier: z.string() }, - }, - ({ identifier }) => ({ - content: [{ type: "text", text: identifier }], - }), - ); - transport = new WebStandardStreamableHTTPServerTransport({ - enableJsonResponse: true, - sessionIdGenerator: () => "linear-test-session", - }); - await server.connect(transport); - mswServer.use( - http.all("https://mcp.linear.app/mcp", async ({ request }) => - transport!.handleRequest(request), - ), - ); - - const registration = linearPlugin(); - const previousPlugins = setPlugins([registration]); - const conversationId = "local:test:linear-mcp-create-annotations"; - const pluginHooks = createPluginHookRunner(); - const manifest = parseInlinePluginManifest( - { - ...registration.manifest, - configKeys: registration.manifest.configKeys ?? [], - }, - "/plugins/linear", - ); - manager = new McpToolManager( - [ - { - dir: "/plugins/linear", - skillsDir: "/plugins/linear/skills", - manifest, - }, - ], - { - onToolSuccess: async (input) => { - await pluginHooks.afterMcpTool({ - ...input, - conversationId, - }); - }, - }, - ); - - try { - await getConversationStore().recordActivity({ - destination: { platform: "local" as const, conversationId }, - conversationId, - nowMs: Date.now(), - source: "local", - title: "Linear MCP create annotations", - }); - - expect(await manager.activateProvider("linear")).toBe(true); - const catalog = manager.getActiveToolCatalog(); - expect(catalog.map((tool) => tool.rawName).sort()).toEqual([ - "get_issue", - "save_issue", - ]); - - const saveIssue = manager - .getResolvedActiveTools({ provider: "linear" }) - .find((tool) => tool.rawName === "save_issue"); - if (!saveIssue) { - throw new Error("save_issue is unavailable after activation"); - } - - const createInput = { - team: "Engineering", - title: "Linear MCP create issue", - description: "Create through the hosted MCP provider.", - priority: "high", - project: "Junior", - } as const; - const createResult = await saveIssue.execute(createInput); - expect(createResult).toMatchObject({ - structuredContent: { - issue: { - identifier: "ENG-123", - url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", - }, - }, - }); - expect(createResult.content).toEqual([ - { - type: "text", - text: expect.stringContaining('"identifier": "ENG-123"'), - }, - ]); - await expect( - listConversationAnnotations(getDb(), conversationId), - ).resolves.toMatchObject([ - { - kind: "resource_link", - key: "ENG-123", - label: "ENG-123", - plugin: "linear", - status: "open", - url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", - }, - ]); - - const updateResult = await saveIssue.execute({ - id: "ENG-123", - state: "In Progress", - }); - expect(updateResult).toMatchObject({ - structuredContent: { - issue: { - identifier: "ENG-123", - url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", - }, - }, - }); - await expect( - listConversationAnnotations(getDb(), conversationId), - ).resolves.toHaveLength(1); - - expect(saveCalls).toEqual([ - createInput, - { id: "ENG-123", state: "In Progress" }, - ]); - } finally { - setPlugins(previousPlugins); - await server.close(); - } - }); - - it("keeps the tool result when annotation processing fails", async () => { - const server = new McpServer({ name: "linear-test", version: "1.0.0" }); - server.registerTool( - "save_issue", - { - description: "Create or update a Linear issue", - inputSchema: { - team: z.string().optional(), - title: z.string().optional(), - }, - }, - () => ({ - content: [ - { - type: "text", - text: "Created [ENG-999](https://linear.app/acme/issue/ENG-999/hook-failure)", - }, - ], - }), - ); - transport = new WebStandardStreamableHTTPServerTransport({ - enableJsonResponse: true, - sessionIdGenerator: () => "linear-test-session-hook-failure", - }); - await server.connect(transport); - mswServer.use( - http.all("https://mcp.linear.app/mcp", async ({ request }) => - transport!.handleRequest(request), - ), - ); - - const registration = linearPlugin(); - const previousPlugins = setPlugins([ - { - ...registration, - hooks: { - ...registration.hooks, - afterMcpTool: async () => { - throw new Error("annotation failed"); - }, - }, - }, - ]); - const conversationId = "local:test:linear-mcp-create-hook-failure"; - const pluginHooks = createPluginHookRunner(); - const manifest = parseInlinePluginManifest( - { - ...registration.manifest, - configKeys: registration.manifest.configKeys ?? [], - }, - "/plugins/linear", - ); - manager = new McpToolManager( - [ - { - dir: "/plugins/linear", - skillsDir: "/plugins/linear/skills", - manifest, - }, - ], - { - onToolSuccess: async (input) => { - await pluginHooks.afterMcpTool({ - ...input, - conversationId, - }); - }, - }, - ); - - try { - await getConversationStore().recordActivity({ - destination: { platform: "local" as const, conversationId }, - conversationId, - nowMs: Date.now(), - source: "local", - title: "Linear MCP create hook failure", - }); - expect(await manager.activateProvider("linear")).toBe(true); - const saveIssue = manager - .getResolvedActiveTools({ provider: "linear" }) - .find((tool) => tool.rawName === "save_issue"); - if (!saveIssue) { - throw new Error("save_issue is unavailable after activation"); - } - - await expect( - saveIssue.execute({ - team: "Engineering", - title: "Hook failure should not break create", - }), - ).resolves.toMatchObject({ - content: [ - { - type: "text", - text: "Created [ENG-999](https://linear.app/acme/issue/ENG-999/hook-failure)", - }, - ], - }); - await expect( - listConversationAnnotations(getDb(), conversationId), - ).resolves.toEqual([]); - } finally { - setPlugins(previousPlugins); - await server.close(); - } - }); -}); diff --git a/packages/junior/tests/unit/capabilities/capability-factory.test.ts b/packages/junior/tests/unit/capabilities/capability-factory.test.ts index c54f41bd5c..189c7475d9 100644 --- a/packages/junior/tests/unit/capabilities/capability-factory.test.ts +++ b/packages/junior/tests/unit/capabilities/capability-factory.test.ts @@ -74,6 +74,7 @@ describe("capability factory", () => { }); expect(createBrokerMock).toHaveBeenCalledWith("example", { + installationTokenStore: expect.any(Object), userTokenStore: expect.any(Object), }); expect(broker.issue).toHaveBeenCalledWith({ @@ -133,6 +134,7 @@ describe("capability factory", () => { expect(createBrokerMock).toHaveBeenCalledTimes(1); expect(createBrokerMock).toHaveBeenCalledWith("sentry", { + installationTokenStore: expect.any(Object), userTokenStore: expect.any(Object), }); }); diff --git a/packages/junior/tests/unit/linear/after-mcp-tool.test.ts b/packages/junior/tests/unit/linear/after-mcp-tool.test.ts deleted file mode 100644 index a4d74ee1cb..0000000000 --- a/packages/junior/tests/unit/linear/after-mcp-tool.test.ts +++ /dev/null @@ -1,136 +0,0 @@ -import { describe, expect, it, vi } from "vitest"; -import { linearPlugin } from "../../../../junior-linear/src/index.js"; -import type { AfterMcpToolHookContext } from "@sentry/junior-plugin-api"; - -function baseContext( - overrides: Partial = {}, -): AfterMcpToolHookContext { - return { - db: {}, - log: { - error() {}, - info() {}, - warn() {}, - }, - plugin: { name: "linear" }, - result: { - structuredContent: { - issue: { - identifier: "ENG-123", - url: "https://linear.app/acme/issue/ENG-123/created", - }, - }, - }, - tool: { - arguments: { - team: "Engineering", - title: "Created issue", - }, - name: "save_issue", - }, - ...overrides, - }; -} - -describe("linear afterMcpTool annotations", () => { - it("annotates create-shaped save_issue successes", async () => { - const upsert = vi.fn(async () => undefined); - const plugin = linearPlugin(); - const hook = plugin.hooks?.afterMcpTool; - if (!hook) { - throw new Error("linear afterMcpTool hook is missing"); - } - - await hook( - baseContext({ - annotations: { - upsert, - async remove() {}, - async list() { - return []; - }, - }, - }), - ); - - expect(upsert).toHaveBeenCalledWith({ - kind: "resource_link", - key: "ENG-123", - label: "ENG-123", - url: "https://linear.app/acme/issue/ENG-123/created", - status: "open", - }); - }); - - it("logs and skips annotation when the response schema does not match", async () => { - const warn = vi.fn(); - const upsert = vi.fn(async () => undefined); - const plugin = linearPlugin(); - const hook = plugin.hooks?.afterMcpTool; - if (!hook) { - throw new Error("linear afterMcpTool hook is missing"); - } - - await hook( - baseContext({ - annotations: { - upsert, - async remove() {}, - async list() { - return []; - }, - }, - log: { - error() {}, - info() {}, - warn, - }, - result: { - structuredContent: { issue: { title: "Incomplete" } }, - }, - }), - ); - - expect(warn).toHaveBeenCalledWith("linear.issue_annotation.skipped", { - "app.reason": "unexpected_save_response", - }); - expect(upsert).not.toHaveBeenCalled(); - }); - - it("skips updates and non-save_issue tools", async () => { - const upsert = vi.fn(async () => undefined); - const plugin = linearPlugin(); - const hook = plugin.hooks?.afterMcpTool; - if (!hook) { - throw new Error("linear afterMcpTool hook is missing"); - } - const annotations = { - upsert, - async remove() {}, - async list() { - return []; - }, - }; - - await hook( - baseContext({ - annotations, - tool: { - arguments: { id: "ENG-123", state: "In Progress" }, - name: "save_issue", - }, - }), - ); - await hook( - baseContext({ - annotations, - tool: { - arguments: { query: "ENG-123" }, - name: "get_issue", - }, - }), - ); - - expect(upsert).not.toHaveBeenCalled(); - }); -}); From 0b4df0c6935419e7b658c313fc1ae072bf0efe29 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:13:20 +0000 Subject: [PATCH 3/8] docs(linear): Simplify OAuth guidance Co-Authored-By: David Cramer --- .../src/content/docs/extend/linear-plugin.md | 60 +++++++++++++------ packages/junior-linear/README.md | 14 ++--- packages/junior-linear/skills/linear/SKILL.md | 12 ++-- .../skills/linear/references/api-surface.md | 16 ++--- .../linear/references/common-use-cases.md | 8 +-- .../skills/linear/references/issue-writing.md | 3 +- .../references/troubleshooting-workarounds.md | 8 +-- packages/junior-linear/src/plugin.ts | 4 +- packages/junior-linear/src/tools.ts | 4 +- packages/junior-linear/tests/tools.test.ts | 12 ++-- ...e-create.test.ts => linear-create.test.ts} | 18 +++--- 11 files changed, 92 insertions(+), 67 deletions(-) rename packages/junior/tests/component/{linear-native-create.test.ts => linear-create.test.ts} (82%) diff --git a/packages/docs/src/content/docs/extend/linear-plugin.md b/packages/docs/src/content/docs/extend/linear-plugin.md index c2c0179759..62d3a1c9a8 100644 --- a/packages/docs/src/content/docs/extend/linear-plugin.md +++ b/packages/docs/src/content/docs/extend/linear-plugin.md @@ -11,9 +11,9 @@ related: - /operate/security-hardening/ --- -Use the Linear plugin to find, create, update, comment on, and triage Linear issues from Slack. Each user connects their own Linear account through Linear's hosted MCP server. +Use the Linear plugin to find, create, update, comment on, and triage Linear issues from Slack. A workspace admin installs one Linear OAuth app. Junior uses that app connection instead of asking each user to connect Linear. -Optional webhooks let Junior publish `issue.created` resource events for subscriptions and event tasks. User MCP OAuth and webhook ingress stay separate. +Optional webhooks let Junior publish `issue.created` resource events for watches and event tasks. OAuth and webhooks use separate secrets. ## Install @@ -28,16 +28,19 @@ import { linearPlugin } from "@sentry/junior-linear"; export const plugins = defineJuniorPlugins([linearPlugin()]); ``` -Register `linearPlugin()` so Junior loads the webhook route. +Register `linearPlugin()` so Junior loads the Linear tools and webhook route. -## Auth model +## Connect Linear -- No `LINEAR_API_KEY`, shared workspace token, or custom OAuth app is required for the default setup. -- Each user completes Linear's MCP OAuth flow the first time Junior calls a Linear MCP tool on their behalf. -- Junior sends the authorization link privately, then resumes the same thread automatically after the user authorizes. -- Webhooks use a separate Linear webhook secret. They do not use the user's MCP OAuth grant. +1. Create an OAuth app in Linear. +2. Set its callback URL to `https:///api/oauth/callback/linear`. +3. Give it the `read,write` scopes. +4. Set `LINEAR_CLIENT_ID` and `LINEAR_CLIENT_SECRET` in Junior. +5. Ask Junior to use Linear. A workspace admin must complete the install once. -Junior uses Linear's hosted MCP tools for reads and writes. When an issue is created through that path, Junior links it to the current conversation. +Junior requests `actor=app`. Linear records changes as made by the Junior app. The app can access every team available to it in the connected workspace. Junior uses the same app connection for requests from conversations, scheduled tasks, and event tasks. + +When Junior creates an issue, it links the issue to the current conversation. If Linear rejects the refresh token, an admin must install the app again. ## Config @@ -71,6 +74,28 @@ Default project for issue creation when a request does not name one. Use it only ### Environment variables +
+LINEAR_CLIENT_ID + +Client ID for the Linear OAuth app. + +- **Define:** Set `LINEAR_CLIENT_ID` in the deployment environment +- **Required:** Yes +- **Environment override:** `LINEAR_CLIENT_ID` + +
+ +
+LINEAR_CLIENT_SECRET + +Client secret for the Linear OAuth app. + +- **Define:** Set `LINEAR_CLIENT_SECRET` in the deployment environment +- **Required:** Yes +- **Environment override:** `LINEAR_CLIENT_SECRET` + +
+
LINEAR_WEBHOOK_SECRET @@ -143,22 +168,23 @@ Optional `match` values come from the resource type. For Linear issue and team e ## Verify -**OAuth:** Ask Junior to create or update a real Linear issue, complete the private authorization flow, and confirm the issue key or URL returns in the same thread. +**OAuth:** Ask Junior to create or update a real Linear issue. If Linear is not connected, have a workspace admin complete the install. Confirm that Junior returns the issue key or URL in the same thread. **Webhooks:** Create an event task for a team key, then create a test issue in that team. You can also create an issue-scoped task with `match.teamKey` set to the same team key and confirm non-matching teams do not fire. ## Security -- Junior stores user MCP grants and does not include them in model input. -- Webhooks use the Linear webhook signing secret, not user MCP OAuth. -- Issue title, description, and other payload text are untrusted event content. +- Junior stores the app tokens outside the model and sandbox. +- Webhooks use `LINEAR_WEBHOOK_SECRET`, not the OAuth app tokens. +- Issue titles, descriptions, and other webhook text are untrusted input. ## Failure modes -- **No auth prompt or no resume:** Retry the Linear request and complete the private authorization flow when prompted. -- **Wrong team or project target:** Include the team name, project name, or existing Linear issue key explicitly in the Slack request. -- **Duplicate or low-signal tickets:** Give Junior the core problem, impact, and any supporting URLs from the thread so it can create a grounded issue instead of a vague summary. -- **Permission failures after connect:** The user's Linear account may not have access to that team, project, or issue. Retry with a resource the user can access. +- **Linear is not connected:** Check `LINEAR_CLIENT_ID` and `LINEAR_CLIENT_SECRET`, then have a workspace admin install the app. +- **The connection expired:** Have a workspace admin install the app again. +- **Wrong team or project:** Name the team, project, or issue key in the Slack request. +- **Duplicate or vague tickets:** Give Junior the core problem, impact, and useful links from the thread. +- **Permission failure:** Confirm that the installed app can access the team, project, or issue. - **Webhooks are ignored:** Check `LINEAR_WEBHOOK_SECRET`, confirm the webhook points at `/api/webhooks/linear`, and confirm a matching subscription or event task exists. - **Event task stays unavailable:** Resource events stay disabled until `LINEAR_WEBHOOK_SECRET` is set and Junior is redeployed. diff --git a/packages/junior-linear/README.md b/packages/junior-linear/README.md index a72989b237..e2efa338b0 100644 --- a/packages/junior-linear/README.md +++ b/packages/junior-linear/README.md @@ -1,22 +1,22 @@ # @sentry/junior-linear -`@sentry/junior-linear` adds native Linear GraphQL tools and issue webhooks to Junior. +`@sentry/junior-linear` lets Junior read and update Linear through its GraphQL API. It also supports issue webhooks. Install it alongside `@sentry/junior`, then register `linearPlugin()` in `plugins.ts`. ## OAuth app -Create a Linear OAuth application and configure: +Create a Linear OAuth app with: - Callback: `https:///api/oauth/callback/linear` - Scopes: `read,write` -- Environment: `LINEAR_CLIENT_ID` and `LINEAR_CLIENT_SECRET` +- Environment variables: `LINEAR_CLIENT_ID` and `LINEAR_CLIENT_SECRET` -Junior sends `actor=app`. A workspace admin authorizes the app once. Junior stores the access and refresh tokens for the installation, refreshes them behind a provider lock, and uses the app credential for interactive, scheduled, and event work. +Junior requests `actor=app`. A workspace admin installs the app once. Junior then uses that app connection for requests from conversations, scheduled tasks, and event tasks. It does not ask each user to connect Linear. -The initial policy is a hard cutover from MCP: mutations use Junior app attribution, per-user OAuth is not retained, and the Linear workspace is the access boundary. There is no separate team allowlist in this version. A rejected refresh token is removed and requires an admin reinstall. +Linear records changes as made by the Junior app. The app can access every team available to it in the connected workspace. If Linear rejects the refresh token, an admin must install the app again. -The native tools cover issue get/search/create/update, comment creation, and team, project, and workflow-state lookup. Created issues are linked to the current Junior conversation. +The tools can read and search issues, create and update issues, add comments, and list teams, projects, and workflow states. Junior links created issues to the current conversation. ## Webhooks @@ -28,4 +28,4 @@ To run watches or event tasks when Linear issues are created: The plugin verifies `Linear-Signature` and publishes `issue.created` for the issue identifier and team key. -Optional conversation defaults remain available through `linear.team` and `linear.project`. Explicit user input wins over these defaults. +You can set conversation defaults with `linear.team` and `linear.project`. An explicit team or project in the request always wins. diff --git a/packages/junior-linear/skills/linear/SKILL.md b/packages/junior-linear/skills/linear/SKILL.md index 8bb6c3ce41..b4c92d9779 100644 --- a/packages/junior-linear/skills/linear/SKILL.md +++ b/packages/junior-linear/skills/linear/SKILL.md @@ -1,6 +1,6 @@ --- name: linear -description: Manage Linear issues through Junior's native Linear GraphQL tools and installed OAuth app. Use when users ask to create a Linear ticket, update a Linear issue, add a Linear comment, move work between states, assign work, or look up Linear issue, team, or project details from Slack context. +description: Manage Linear issues through Junior's Linear tools. Use when users ask to create a Linear ticket, update a Linear issue, add a Linear comment, move work between states, assign work, or look up Linear issue, team, or project details from Slack context. --- # Linear Operations @@ -32,7 +32,7 @@ Load references conditionally based on the request: 2. Prepare the Linear operation: -- Prefer a short read/search step before mutating when you need to confirm the existing issue, team, project, or workflow state. +- Read or search first when you need to confirm the issue, team, project, or workflow state. 3. Draft issue content (create or substantial rewrite): @@ -57,7 +57,7 @@ Attribute the reporter by name when clear from the thread (e.g. "Raised by Alice - Use only Linear's standard priority levels: `low`, `medium`, `high`, `urgent`. - Set project, labels, cycle, estimate, or assignee only when the user asked for them or the thread makes them clear. -5. Verify draft before mutating: +5. Check the draft before writing: - Title length ≤ 60 characters. - Delegated-action footer is the last line when applicable, using the action actor's real name, not the reporter's name unless they are the same person. @@ -69,13 +69,13 @@ If any gate fails, revise and re-check before calling the Linear create/update t 6. Execute: -- Use the native `linear_*` tools. Resolve team, project, workflow state, and issue IDs with read tools before writes. -- For updates, prefer partial changes over full rewrites. Fetch current issue state first if the mutation could overwrite structured fields or duplicate an existing comment. +- Use the `linear_*` tools. Look up team, project, workflow state, and issue IDs before writes. +- For updates, change only the requested fields. Fetch the issue first if an update could overwrite fields or repeat a comment. - Check for duplicates silently before creating a new issue when the request appears related to existing work. 7. Report the result: -- Return the canonical Linear issue URL or key and what changed. +- Return the Linear issue URL or key and what changed. - Report issue type when you created a new issue and it materially clarifies the outcome. ## Guardrails diff --git a/packages/junior-linear/skills/linear/references/api-surface.md b/packages/junior-linear/skills/linear/references/api-surface.md index 690380dcc1..35c50fafac 100644 --- a/packages/junior-linear/skills/linear/references/api-surface.md +++ b/packages/junior-linear/skills/linear/references/api-surface.md @@ -2,10 +2,10 @@ Use this reference for any Linear operation. -## Native tools +## Tools - `linear_getIssue`: get one issue by UUID or identifier. -- `linear_searchIssues`: find issues before creation or mutation. +- `linear_searchIssues`: find issues before a create or update. - `linear_createIssue`: create an issue as the installed Junior app. - `linear_updateIssue`: update selected issue fields as the installed Junior app. - `linear_createComment`: add a comment as the installed Junior app. @@ -13,7 +13,7 @@ Use this reference for any Linear operation. - `linear_listProjects`: resolve an active project UUID. - `linear_listWorkflowStates`: resolve one team's workflow state UUID. -The tools call Linear's GraphQL API through Junior's credential broker. They do not use hosted MCP and do not use the requesting user's Linear identity. +The tools call Linear's GraphQL API with the installed Junior app. They do not use the requesting user's Linear account. ## Linear issue model constraints @@ -31,10 +31,10 @@ The tools call Linear's GraphQL API through Junior's credential broker. They do - Comment: resolve the exact issue before `linear_createComment`. - Move state: list the team's states before updating `stateId`. -## Credential policy +## OAuth - One workspace admin installs the OAuth app with `actor=app`. -- Junior stores that OAuth grant for the installation and refreshes it behind a provider lock. -- Routine interactive, scheduled, and event work uses the same app credential. -- Mutations are attributed to the Junior Linear app. Do not claim that they were made as the Slack user. -- A rejected refresh requires an admin to reinstall the app. Do not fall back to per-user OAuth. +- Junior stores and refreshes the app tokens. +- Requests from conversations, scheduled tasks, and event tasks use the same app connection. +- Linear records changes as made by the Junior app, not the Slack user. +- If Linear rejects the refresh token, an admin must install the app again. diff --git a/packages/junior-linear/skills/linear/references/common-use-cases.md b/packages/junior-linear/skills/linear/references/common-use-cases.md index c5e7cf1be4..96895af85d 100644 --- a/packages/junior-linear/skills/linear/references/common-use-cases.md +++ b/packages/junior-linear/skills/linear/references/common-use-cases.md @@ -37,7 +37,7 @@ Use these patterns to shape concrete Linear requests. ## 6. Reassign work or change ownership - Resolve the issue and confirm the target assignee when names are ambiguous. -- Keep the mutation small. Do not rewrite unrelated fields. +- Change only the requested fields. - Preserve the current project, labels, and workflow state unless the user asked to change them too. ## 7. Tighten an existing issue description @@ -60,9 +60,9 @@ Use these patterns to shape concrete Linear requests. ## 10. Mark work as a duplicate -- Search for the canonical destination issue first. -- If the MCP tool supports duplicate relationships directly, use that instead of only posting a comment. -- If the workflow exposes a dedicated duplicate status, prefer it; otherwise expect duplicate handling to land in the team's canceled category. +- Search for the issue that should remain open. +- Add a comment that links the duplicate to that issue. +- Use the team's duplicate state if one exists. Otherwise use its canceled state. ## 11. When a user asks to set channel defaults for a Linear-heavy Slack thread diff --git a/packages/junior-linear/skills/linear/references/issue-writing.md b/packages/junior-linear/skills/linear/references/issue-writing.md index 21899bfb25..752cd87d98 100644 --- a/packages/junior-linear/skills/linear/references/issue-writing.md +++ b/packages/junior-linear/skills/linear/references/issue-writing.md @@ -21,12 +21,11 @@ Default to `task` when the request does not clearly describe a defect or a net-n ## Linear-specific field guidance - Every new issue must belong to a single team. Resolve that before creating the issue. -- If the request maps to a known team template and the active MCP tools expose template-based creation, prefer the template so the team's default properties are applied consistently. - Do not invent a custom status name. Read the team's actual workflow states first when a non-default status is needed. - Priority stays within Linear's standard levels: `low`, `medium`, `high`, `urgent`. - Estimates are team-configured. Set one only when the thread provides a clear value or the team context makes the scale unambiguous. - Labels may be workspace- or team-scoped. Reuse an existing matching label instead of introducing near-duplicates. -- If the tool exposes structured link attachments, attach important URLs there and keep the prose body focused on interpretation. +- Put important URLs in the issue body near the text they support. ## Duplicate handling diff --git a/packages/junior-linear/skills/linear/references/troubleshooting-workarounds.md b/packages/junior-linear/skills/linear/references/troubleshooting-workarounds.md index 44a202bdfd..9379e04d5a 100644 --- a/packages/junior-linear/skills/linear/references/troubleshooting-workarounds.md +++ b/packages/junior-linear/skills/linear/references/troubleshooting-workarounds.md @@ -1,17 +1,17 @@ # Troubleshooting and Workarounds -Use this reference when Linear MCP work fails or the request is ambiguous. +Use this reference when a Linear request fails or is ambiguous. | Symptom | Likely cause | Response | | --------------------------------------------------------- | ------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | -| Authorization completes but the write still fails | The connected Linear account lacks access to the target team, project, or issue | Retry only after confirming the target is visible to the user; otherwise explain the concrete access blocker. | +| App install completes but a write still fails | The installed app cannot access the target team, project, or issue | Confirm that the target is available to the app. Otherwise, explain what access is missing. | | Creating a new issue is blocked | No team was resolved for the issue | Ask for the team only when it is not already clear from the thread, project context, or existing issue references. | | Search returns too many matches | The request lacks a specific issue key, team, project, or title phrase | Narrow the search with the most specific identifier present in the thread, then ask one concise follow-up only if ambiguity remains. | | The request might create a duplicate issue | Similar work already exists in Linear | Search before creating; if a clear match exists, update or comment on that issue instead of opening a new one. | -| A mutation could overwrite good structured content | The request asks for broad edits without showing current issue state | Fetch the current issue first and prefer partial updates over full body rewrites. | +| An update could overwrite useful fields | The request asks for broad edits without showing the current issue | Fetch the issue first and update only the requested fields. | | A requested status name does not exist | Workflow states are team-specific | Read the team's actual states and map the user's intent to a real state instead of inventing one. | | Priority or estimate is unclear | Linear uses constrained priorities and team-configured estimate scales | Use only `low`, `medium`, `high`, or `urgent`; avoid setting an estimate unless the value is explicit or the team scale is already known. | | The draft issue reads like Slack chat instead of a ticket | The content copied conversational phrasing directly from the thread | Rewrite into product or engineering language, preserve only material links, and remove usernames, channel names, and slash-command noise. | | A comment or update repeats existing context | The thread already includes the same evidence in the issue history | Fetch current issue state first, then add only the new evidence or decision. | | The team or project is unclear | The Slack request does not say where the issue belongs | Inspect thread context for earlier mentions, then ask one direct question only if the write is still blocked. | -| The result lacks a durable handoff | The reply reports success without the canonical issue reference | Return the Linear issue key or URL and summarize the exact change that was made. | +| The reply does not identify the issue | The reply reports success without an issue key or URL | Return the Linear issue key or URL and summarize the change. | diff --git a/packages/junior-linear/src/plugin.ts b/packages/junior-linear/src/plugin.ts index 33f9de3ca1..6ab51d29f7 100644 --- a/packages/junior-linear/src/plugin.ts +++ b/packages/junior-linear/src/plugin.ts @@ -10,7 +10,7 @@ import { createLinearTools } from "./tools.js"; import { createLinearWebhookRoute } from "./webhooks/handler.js"; import { linearWebhookSecret } from "./webhooks/secret.js"; -/** Register Linear app OAuth, native GraphQL tools, and issue webhooks. */ +/** Register Linear OAuth, tools, and issue webhooks. */ export function linearPlugin(): PluginRegistration { return defineJuniorPlugin({ packageName: "@sentry/junior-linear", @@ -44,7 +44,7 @@ export function linearPlugin(): PluginRegistration { type: "oauth-bearer", }, description: - "Linear issue tracking via an installed OAuth app, native GraphQL tools, and issue webhooks", + "Read and update Linear through an installed OAuth app, with optional issue webhooks", displayName: "Linear", envVars: { LINEAR_CLIENT_ID: {}, diff --git a/packages/junior-linear/src/tools.ts b/packages/junior-linear/src/tools.ts index a06b4325d3..e689f2933f 100644 --- a/packages/junior-linear/src/tools.ts +++ b/packages/junior-linear/src/tools.ts @@ -80,7 +80,7 @@ function issueInput(input: Record) { ); } -/** Build Linear's native GraphQL tool surface. */ +/** Build the tools that read and update Linear. */ export function createLinearTools( ctx: ToolRegistrationHookContext, ): Record { @@ -116,7 +116,7 @@ export function createLinearTools( readOnlyHint: true, }, description: - "Search Linear issue titles and descriptions for duplicate or target resolution.", + "Search Linear issue titles and descriptions.", inputSchema: z .object({ query: z.string().min(1), diff --git a/packages/junior-linear/tests/tools.test.ts b/packages/junior-linear/tests/tools.test.ts index b810380942..100031e734 100644 --- a/packages/junior-linear/tests/tools.test.ts +++ b/packages/junior-linear/tests/tools.test.ts @@ -6,10 +6,10 @@ function issue() { return { id: "issue-id", identifier: "ENG-123", - title: "Native Linear tools", + title: "Linear tools", description: null, priority: 3, - url: "https://linear.app/acme/issue/ENG-123/native-linear-tools", + url: "https://linear.app/acme/issue/ENG-123/linear-tools", state: { id: "state-id", name: "Todo" }, team: { id: "team-id", key: "ENG", name: "Engineering" }, project: null, @@ -41,8 +41,8 @@ function toolContext(data: unknown) { return { requests, tools, upsert }; } -describe("Linear native tools", () => { - it("gets an issue through host-owned Linear egress", async () => { +describe("Linear tools", () => { + it("gets an issue without adding an authorization header", async () => { const { requests, tools } = toolContext({ issue: issue() }); await expect( @@ -64,7 +64,7 @@ describe("Linear native tools", () => { await expect( tools.createIssue?.execute?.( - { teamId: "team-id", title: "Native Linear tools" }, + { teamId: "team-id", title: "Linear tools" }, { toolCallId: "create" }, ), ).resolves.toEqual({ issue: issue() }); @@ -82,7 +82,7 @@ describe("Linear native tools", () => { }); }); - it("surfaces GraphQL errors as repairable tool errors", async () => { + it("returns Linear errors to the caller", async () => { const { tools } = toolContext(undefined); const ctx = { egress: { diff --git a/packages/junior/tests/component/linear-native-create.test.ts b/packages/junior/tests/component/linear-create.test.ts similarity index 82% rename from packages/junior/tests/component/linear-native-create.test.ts rename to packages/junior/tests/component/linear-create.test.ts index ecde85da46..bc51189bd3 100644 --- a/packages/junior/tests/component/linear-native-create.test.ts +++ b/packages/junior/tests/component/linear-create.test.ts @@ -8,10 +8,10 @@ import { } from "@/chat/plugins/agent-hooks"; import { listConversationAnnotations } from "@/chat/plugins/annotations"; -const conversationId = "local:test:linear-native-create"; +const conversationId = "local:test:linear-create"; const destination = { platform: "local" as const, conversationId }; -describe("Linear native create", () => { +describe("Linear create", () => { let previousPlugins: ReturnType | undefined; afterEach(() => { @@ -21,14 +21,14 @@ describe("Linear native create", () => { } }); - it("creates and annotates an issue through plugin tool wiring", async () => { + it("creates an issue and links it to the conversation", async () => { previousPlugins = setPlugins([linearPlugin()]); await getConversationStore().recordActivity({ conversationId, destination, nowMs: Date.now(), source: "local", - title: "Linear native create", + title: "Linear create", }); const requests: Array<{ operation: string; @@ -48,10 +48,10 @@ describe("Linear native create", () => { issue: { id: "issue-id", identifier: "ENG-123", - title: "Native Linear issue", + title: "Linear issue", description: null, priority: 3, - url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", + url: "https://linear.app/acme/issue/ENG-123/linear-issue", state: { id: "state-id", name: "Todo" }, team: { id: "team-id", @@ -75,13 +75,13 @@ describe("Linear native create", () => { await expect( createIssue.execute( - { teamId: "team-id", title: "Native Linear issue" }, + { teamId: "team-id", title: "Linear issue" }, { toolCallId: "create-linear-issue" }, ), ).resolves.toMatchObject({ issue: { identifier: "ENG-123", - url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", + url: "https://linear.app/acme/issue/ENG-123/linear-issue", }, }); expect(requests).toHaveLength(1); @@ -99,7 +99,7 @@ describe("Linear native create", () => { label: "ENG-123", plugin: "linear", status: "open", - url: "https://linear.app/acme/issue/ENG-123/native-linear-issue", + url: "https://linear.app/acme/issue/ENG-123/linear-issue", }, ]); }); From 871364065665cabb679b448bdf3d481742175318 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:28:07 +0000 Subject: [PATCH 4/8] fix(oauth): Protect installation connections --- .../src/chat/credentials/unlink-provider.ts | 14 ++ .../junior/src/chat/ingress/slack-webhook.ts | 6 +- .../junior/src/chat/ingress/slash-command.ts | 19 ++- packages/junior/src/chat/oauth-flow.ts | 22 +++ packages/junior/src/chat/slack/admin.ts | 14 ++ packages/junior/src/chat/slack/users.ts | 9 ++ .../junior/src/handlers/oauth-callback.ts | 39 +++++- .../credentials/unlink-provider.test.ts | 68 +++++++++ .../tests/fixtures/slack/factories/api.ts | 6 + .../unit/handlers/oauth-callback.test.ts | 131 +++++++++++++++++- 10 files changed, 319 insertions(+), 9 deletions(-) create mode 100644 packages/junior/src/chat/slack/admin.ts create mode 100644 packages/junior/tests/component/credentials/unlink-provider.test.ts diff --git a/packages/junior/src/chat/credentials/unlink-provider.ts b/packages/junior/src/chat/credentials/unlink-provider.ts index c475947402..a1b76909a6 100644 --- a/packages/junior/src/chat/credentials/unlink-provider.ts +++ b/packages/junior/src/chat/credentials/unlink-provider.ts @@ -1,6 +1,9 @@ import { getSqlExecutor } from "@/chat/db"; import { deleteProviderIdentityForSlackUser } from "@/chat/identities/sql"; import type { UserTokenStore } from "@/chat/credentials/user-token-store"; +import type { InstallationTokenStore } from "@/chat/credentials/installation-token-store"; +import { pluginCatalogRuntime } from "@/chat/plugins/catalog-runtime"; +import { isSlackWorkspaceAdmin } from "@/chat/slack/admin"; import { deleteMcpAuthSessionsForUserProvider, deleteMcpServerSessionId, @@ -12,8 +15,19 @@ export async function unlinkProvider( userId: string, provider: string, userTokenStore: UserTokenStore, + installationTokenStore: InstallationTokenStore, slackTeamId?: string, ): Promise { + if ( + pluginCatalogRuntime.getOAuthConfig(provider)?.tokenSubject === "installation" + ) { + if (!(await isSlackWorkspaceAdmin(userId))) { + throw new Error("Only a Slack workspace admin can disconnect this app"); + } + await installationTokenStore.delete(provider); + return; + } + const tokens = await userTokenStore.get(userId, provider); if (tokens?.account && slackTeamId) { await deleteProviderIdentityForSlackUser( diff --git a/packages/junior/src/chat/ingress/slack-webhook.ts b/packages/junior/src/chat/ingress/slack-webhook.ts index c7027a66ca..8b21f7f577 100644 --- a/packages/junior/src/chat/ingress/slack-webhook.ts +++ b/packages/junior/src/chat/ingress/slack-webhook.ts @@ -38,7 +38,10 @@ import { parseSlackThreadId } from "@/chat/slack/context"; import { getStateAdapter } from "@/chat/state/adapter"; import { handleSlashCommand } from "@/chat/ingress/slash-command"; import { createActor, parseActorUserId } from "@/chat/actor"; -import { createUserTokenStore } from "@/chat/capabilities/factory"; +import { + createInstallationTokenStore, + createUserTokenStore, +} from "@/chat/capabilities/factory"; import { unlinkProvider } from "@/chat/credentials/unlink-provider"; import type { UserTokenStore } from "@/chat/credentials/user-token-store"; import { publishAppHomeView } from "@/chat/slack/app-home"; @@ -605,6 +608,7 @@ async function handleInteractivePayload(args: { userId, provider, args.userTokenStore, + createInstallationTokenStore(), getWorkspaceTeamId(), ); } catch (error) { diff --git a/packages/junior/src/chat/ingress/slash-command.ts b/packages/junior/src/chat/ingress/slash-command.ts index 2f697a3816..bb7af67184 100644 --- a/packages/junior/src/chat/ingress/slash-command.ts +++ b/packages/junior/src/chat/ingress/slash-command.ts @@ -1,5 +1,8 @@ import type { SlashCommandEvent } from "chat"; -import { createUserTokenStore } from "@/chat/capabilities/factory"; +import { + createInstallationTokenStore, + createUserTokenStore, +} from "@/chat/capabilities/factory"; import { unlinkProvider } from "@/chat/credentials/unlink-provider"; import { formatProviderLabel, startOAuthFlow } from "@/chat/oauth-flow"; import { pluginCatalogRuntime } from "@/chat/plugins/catalog-runtime"; @@ -96,15 +99,25 @@ async function handleUnlink( const tokenStore = createUserTokenStore(); const teamId = (event.raw as { team_id?: string }).team_id; - await unlinkProvider(actorId, provider, tokenStore, teamId); + await unlinkProvider( + actorId, + provider, + tokenStore, + createInstallationTokenStore(), + teamId, + ); logInfo("slash_command.credential.unlinked", { "app.credential.provider": provider, }); + const installation = + pluginCatalogRuntime.getOAuthConfig(provider)?.tokenSubject === "installation"; await postEphemeral( event, - `Your ${formatProviderLabel(provider)} account has been unlinked.`, + installation + ? `${formatProviderLabel(provider)} has been disconnected.` + : `Your ${formatProviderLabel(provider)} account has been unlinked.`, ); } diff --git a/packages/junior/src/chat/oauth-flow.ts b/packages/junior/src/chat/oauth-flow.ts index b107e7dd31..e869945640 100644 --- a/packages/junior/src/chat/oauth-flow.ts +++ b/packages/junior/src/chat/oauth-flow.ts @@ -25,6 +25,8 @@ import type { import { formatOAuthAuthorizationMessage } from "@/chat/slack/oauth-authorization-message"; import { isRecord } from "@/chat/coerce"; import { getStateAdapter } from "@/chat/state/adapter"; +import { StateAdapterInstallationTokenStore } from "@/chat/credentials/state-adapter-token-store"; +import { isSlackWorkspaceAdmin } from "@/chat/slack/admin"; type PrivateDeliveryResult = "in_context" | "fallback_dm" | false; @@ -240,6 +242,26 @@ export async function startOAuthFlow( }; } + if (providerConfig.tokenSubject === "installation") { + if ( + input.actor?.platform !== "slack" || + !(await isSlackWorkspaceAdmin(input.actorId)) + ) { + return { + ok: false, + error: `Only a Slack workspace admin can install ${formatProviderLabel(provider)}`, + }; + } + if ( + await new StateAdapterInstallationTokenStore(getStateAdapter()).get(provider) + ) { + return { + ok: false, + error: `${formatProviderLabel(provider)} is already installed. Disconnect it before installing it again`, + }; + } + } + const clientId = process.env[providerConfig.clientIdEnv]?.trim(); if (!clientId) { return { diff --git a/packages/junior/src/chat/slack/admin.ts b/packages/junior/src/chat/slack/admin.ts new file mode 100644 index 0000000000..c753a1d588 --- /dev/null +++ b/packages/junior/src/chat/slack/admin.ts @@ -0,0 +1,14 @@ +import { parseSlackUserId } from "@/chat/slack/ids"; +import { lookupSlackUserProfile } from "@/chat/slack/users"; + +/** Return whether a Slack user can manage workspace-wide connections. */ +export async function isSlackWorkspaceAdmin(userId: string): Promise { + const parsedUserId = parseSlackUserId(userId); + if (!parsedUserId) { + return false; + } + const profile = await lookupSlackUserProfile(parsedUserId); + return Boolean( + profile.is_admin || profile.is_owner || profile.is_primary_owner, + ); +} diff --git a/packages/junior/src/chat/slack/users.ts b/packages/junior/src/chat/slack/users.ts index 5fa4152c52..b8e4dfc025 100644 --- a/packages/junior/src/chat/slack/users.ts +++ b/packages/junior/src/chat/slack/users.ts @@ -12,8 +12,11 @@ export interface SlackUserProfile { email?: string; status_text?: string; status_emoji?: string; + is_admin?: boolean; is_bot: boolean; is_deleted: boolean; + is_owner?: boolean; + is_primary_owner?: boolean; timezone?: string; profile_fields?: Array<{ id: string; @@ -35,7 +38,10 @@ interface SlackUserRaw { name?: string; real_name?: string; deleted?: boolean; + is_admin?: boolean; is_bot?: boolean; + is_owner?: boolean; + is_primary_owner?: boolean; tz?: string; profile?: { display_name?: string; @@ -74,8 +80,11 @@ function normalizeUser(raw: SlackUserRaw): SlackUserProfile { email: raw.profile?.email || undefined, status_text: raw.profile?.status_text ?? undefined, status_emoji: raw.profile?.status_emoji ?? undefined, + is_admin: raw.is_admin ?? false, is_bot: raw.is_bot ?? false, is_deleted: raw.deleted ?? false, + is_owner: raw.is_owner ?? false, + is_primary_owner: raw.is_primary_owner ?? false, timezone: raw.tz || undefined, ...(profileFields.length > 0 ? { profile_fields: profileFields } : undefined), }; diff --git a/packages/junior/src/handlers/oauth-callback.ts b/packages/junior/src/handlers/oauth-callback.ts index 2696bcf101..6582f78fc3 100644 --- a/packages/junior/src/handlers/oauth-callback.ts +++ b/packages/junior/src/handlers/oauth-callback.ts @@ -3,6 +3,7 @@ import { createUserTokenStore, } from "@/chat/capabilities/factory"; import { hasRequiredOAuthScope } from "@/chat/credentials/oauth-scope"; +import { isSlackWorkspaceAdmin } from "@/chat/slack/admin"; import { coerceThreadConversationState } from "@/chat/state/conversation"; import { hydrateConversationMessages } from "@/chat/conversations/messages"; import { @@ -585,6 +586,27 @@ export async function GET( await stateAdapter.delete(stateKey); + const installationTokenStore = createInstallationTokenStore(); + if (providerConfig.tokenSubject === "installation") { + if ( + stored.actor?.platform !== "slack" || + !(await isSlackWorkspaceAdmin(stored.userId)) + ) { + return htmlErrorResponse( + "Install blocked", + `Only a Slack workspace admin can install ${providerLabel}.`, + 403, + ); + } + if (await installationTokenStore.get(provider)) { + return htmlErrorResponse( + "Install blocked", + `${providerLabel} is already installed. Disconnect it before installing it again.`, + 409, + ); + } + } + const clientId = process.env[providerConfig.clientIdEnv]?.trim(); const clientSecret = process.env[providerConfig.clientSecretEnv]?.trim(); if (!clientId || !clientSecret) { @@ -684,7 +706,6 @@ export async function GET( } const userTokenStore = createUserTokenStore(); - const installationTokenStore = createInstallationTokenStore(); let account: Awaited>; try { account = await resolvePluginOAuthAccount({ @@ -703,7 +724,21 @@ export async function GET( ...(account ? { account } : undefined), }; if (providerConfig.tokenSubject === "installation") { - await installationTokenStore.set(provider, storedTokens); + let installed = false; + await installationTokenStore.withRefresh(provider, async () => { + if (await installationTokenStore.get(provider)) { + return; + } + await installationTokenStore.set(provider, storedTokens); + installed = true; + }); + if (!installed) { + return htmlErrorResponse( + "Install blocked", + `${providerLabel} is already installed. Disconnect it before installing it again.`, + 409, + ); + } } else { await userTokenStore.set(stored.userId, provider, storedTokens); } diff --git a/packages/junior/tests/component/credentials/unlink-provider.test.ts b/packages/junior/tests/component/credentials/unlink-provider.test.ts new file mode 100644 index 0000000000..d91c7e36a0 --- /dev/null +++ b/packages/junior/tests/component/credentials/unlink-provider.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it, vi } from "vitest"; +import type { InstallationTokenStore } from "@/chat/credentials/installation-token-store"; +import type { UserTokenStore } from "@/chat/credentials/user-token-store"; + +const { isSlackWorkspaceAdmin } = vi.hoisted(() => ({ + isSlackWorkspaceAdmin: vi.fn(), +})); + +vi.mock("@/chat/slack/admin", () => ({ isSlackWorkspaceAdmin })); +vi.mock("@/chat/plugins/catalog-runtime", () => ({ + pluginCatalogRuntime: { + getOAuthConfig: (provider: string) => + provider === "linear" ? { tokenSubject: "installation" } : {}, + }, +})); + +import { unlinkProvider } from "@/chat/credentials/unlink-provider"; + +function stores() { + const userTokenStore: UserTokenStore = { + get: vi.fn(async () => undefined), + set: vi.fn(async () => undefined), + delete: vi.fn(async () => undefined), + withRefresh: vi.fn(async (_userId, _provider, callback) => callback()), + }; + const installationTokenStore: InstallationTokenStore = { + get: vi.fn(async () => undefined), + set: vi.fn(async () => undefined), + delete: vi.fn(async () => undefined), + withRefresh: vi.fn(async (_provider, callback) => callback()), + }; + return { installationTokenStore, userTokenStore }; +} + +describe("unlinkProvider", () => { + it("lets a Slack admin disconnect an installation", async () => { + isSlackWorkspaceAdmin.mockResolvedValue(true); + const { installationTokenStore, userTokenStore } = stores(); + + await unlinkProvider( + "U123", + "linear", + userTokenStore, + installationTokenStore, + "T123", + ); + + expect(installationTokenStore.delete).toHaveBeenCalledWith("linear"); + expect(userTokenStore.delete).not.toHaveBeenCalled(); + }); + + it("does not let a non-admin disconnect an installation", async () => { + isSlackWorkspaceAdmin.mockResolvedValue(false); + const { installationTokenStore, userTokenStore } = stores(); + + await expect( + unlinkProvider( + "U123", + "linear", + userTokenStore, + installationTokenStore, + "T123", + ), + ).rejects.toThrow("Only a Slack workspace admin"); + + expect(installationTokenStore.delete).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/junior/tests/fixtures/slack/factories/api.ts b/packages/junior/tests/fixtures/slack/factories/api.ts index dd26a6490d..194fbd54c1 100644 --- a/packages/junior/tests/fixtures/slack/factories/api.ts +++ b/packages/junior/tests/fixtures/slack/factories/api.ts @@ -457,7 +457,10 @@ export function usersInfoOk( email?: string; statusText?: string; statusEmoji?: string; + isAdmin?: boolean; isBot?: boolean; + isOwner?: boolean; + isPrimaryOwner?: boolean; deleted?: boolean; tz?: string; fields?: Record; @@ -472,7 +475,10 @@ export function usersInfoOk( name: input.userName ?? "testuser", real_name: input.realName ?? "Test User", deleted: input.deleted ?? false, + is_admin: input.isAdmin ?? false, is_bot: input.isBot ?? false, + is_owner: input.isOwner ?? false, + is_primary_owner: input.isPrimaryOwner ?? false, tz: input.tz ?? "America/Los_Angeles", profile: { display_name: input.displayName ?? "Test User", diff --git a/packages/junior/tests/unit/handlers/oauth-callback.test.ts b/packages/junior/tests/unit/handlers/oauth-callback.test.ts index 647042d133..f82e510d57 100644 --- a/packages/junior/tests/unit/handlers/oauth-callback.test.ts +++ b/packages/junior/tests/unit/handlers/oauth-callback.test.ts @@ -2,12 +2,17 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createSlackSource } from "@sentry/junior-plugin-api"; import { http, HttpResponse } from "msw"; import { mswServer } from "../../msw/server"; -import { queueSlackApiError } from "../../msw/handlers/slack-api"; +import { + queueSlackApiError, + queueSlackApiResponse, +} from "../../msw/handlers/slack-api"; +import { usersInfoOk } from "../../fixtures/slack/factories/api"; const { BASE_URL, EXAMPLE_OAUTH_CONFIG, GITHUB_OAUTH_CONFIG, + LINEAR_OAUTH_CONFIG, SENTRY_OAUTH_CONFIG, lookupSlackActorMock, resolvePluginOAuthAccountMock, @@ -32,6 +37,15 @@ const { tokenExtraHeaders: { "Content-Type": "application/json" }, callbackPath: "/api/oauth/callback/example", }, + LINEAR_OAUTH_CONFIG: { + clientIdEnv: "LINEAR_CLIENT_ID", + clientSecretEnv: "LINEAR_CLIENT_SECRET", + authorizeEndpoint: "https://linear.app/oauth/authorize", + tokenEndpoint: "https://api.linear.app/oauth/token", + scope: "read,write", + tokenSubject: "installation" as const, + callbackPath: "/api/oauth/callback/linear", + }, GITHUB_OAUTH_CONFIG: { clientIdEnv: "GITHUB_APP_CLIENT_ID", clientSecretEnv: "GITHUB_APP_CLIENT_SECRET", @@ -57,6 +71,9 @@ vi.mock("@/chat/plugins/catalog-runtime", () => ({ if (provider === "github") { return "GitHub"; } + if (provider === "linear") { + return "Linear"; + } return undefined; }, getOAuthConfig: (provider: string) => { @@ -69,10 +86,16 @@ vi.mock("@/chat/plugins/catalog-runtime", () => ({ if (provider === "github") { return GITHUB_OAUTH_CONFIG; } + if (provider === "linear") { + return LINEAR_OAUTH_CONFIG; + } return undefined; }, isProvider: (provider: string) => - provider === "sentry" || provider === "example" || provider === "github", + provider === "sentry" || + provider === "example" || + provider === "github" || + provider === "linear", isCapability: () => false, isConfigKey: () => false, getProviders: () => [], @@ -112,7 +135,10 @@ vi.mock("@/chat/conversations/projection", async (importOriginal) => ({ recordAuthenticationLinked: vi.fn(async () => undefined), })); -import { createUserTokenStore } from "@/chat/capabilities/factory"; +import { + createInstallationTokenStore, + createUserTokenStore, +} from "@/chat/capabilities/factory"; import { disconnectStateAdapter, getStateAdapter } from "@/chat/state/adapter"; import { GET } from "@/handlers/oauth-callback"; import type { WaitUntilFn } from "@/handlers/types"; @@ -176,6 +202,12 @@ function configureExampleOAuthEnv() { process.env.JUNIOR_BASE_URL = BASE_URL; } +function configureLinearOAuthEnv() { + process.env.LINEAR_CLIENT_ID = "linear-client-id"; + process.env.LINEAR_CLIENT_SECRET = "linear-client-secret"; + process.env.JUNIOR_BASE_URL = BASE_URL; +} + function configureGitHubOAuthEnv() { process.env.GITHUB_APP_CLIENT_ID = "github-client-id"; process.env.GITHUB_APP_CLIENT_SECRET = "github-client-secret"; @@ -502,6 +534,99 @@ describe("oauth callback handler", () => { expect(stored.expiresAt).toBeUndefined(); }); + it("stores installation tokens after a Slack admin completes OAuth", async () => { + configureLinearOAuthEnv(); + await putStoredState("oauth-state:linear-install", { + userId: "U777", + provider: "linear", + actor: { platform: "slack", teamId: "T777", userId: "U777" }, + }); + queueSlackApiResponse("users.info", { + body: usersInfoOk({ userId: "U777", isAdmin: true }), + }); + mswServer.use( + http.post("https://api.linear.app/oauth/token", () => + HttpResponse.json({ + access_token: "linear-access-token", + refresh_token: "linear-refresh-token", + scope: "read,write", + }), + ), + ); + + const response = await GET( + makeRequest( + "https://example.com/api/oauth/callback/linear?code=valid-code&state=linear-install", + ), + "linear", + testWaitUntil, + { agentRunner: testAgentRunner }, + ); + + expect(response.status).toBe(200); + expect(await createInstallationTokenStore().get("linear")).toMatchObject({ + accessToken: "linear-access-token", + refreshToken: "linear-refresh-token", + }); + expect(await getStoredTokens("U777", "linear")).toBeUndefined(); + }); + + it("blocks installation OAuth callbacks from Slack non-admins", async () => { + configureLinearOAuthEnv(); + await putStoredState("oauth-state:linear-non-admin", { + userId: "U777", + provider: "linear", + actor: { platform: "slack", teamId: "T777", userId: "U777" }, + }); + queueSlackApiResponse("users.info", { + body: usersInfoOk({ userId: "U777", isAdmin: false }), + }); + + const response = await GET( + makeRequest( + "https://example.com/api/oauth/callback/linear?code=valid-code&state=linear-non-admin", + ), + "linear", + testWaitUntil, + { agentRunner: testAgentRunner }, + ); + + expect(response.status).toBe(403); + expect(await response.text()).toContain("Only a Slack workspace admin"); + expect(await createInstallationTokenStore().get("linear")).toBeUndefined(); + }); + + it("does not replace an existing installation token", async () => { + configureLinearOAuthEnv(); + await createInstallationTokenStore().set("linear", { + accessToken: "existing-access-token", + refreshToken: "existing-refresh-token", + scope: "read,write", + }); + await putStoredState("oauth-state:linear-existing", { + userId: "U777", + provider: "linear", + actor: { platform: "slack", teamId: "T777", userId: "U777" }, + }); + queueSlackApiResponse("users.info", { + body: usersInfoOk({ userId: "U777", isAdmin: true }), + }); + + const response = await GET( + makeRequest( + "https://example.com/api/oauth/callback/linear?code=valid-code&state=linear-existing", + ), + "linear", + testWaitUntil, + { agentRunner: testAgentRunner }, + ); + + expect(response.status).toBe(409); + expect(await createInstallationTokenStore().get("linear")).toMatchObject({ + accessToken: "existing-access-token", + }); + }); + it("stores GitHub App user tokens when GitHub returns an empty OAuth scope", async () => { const stateKey = "oauth-state:github-exchange"; await putStoredState(stateKey, { From 7a6e44e761096169f7c27a1250cdbff9510c602d Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:40:11 +0000 Subject: [PATCH 5/8] fix(slack): fail closed when admin lookup fails --- packages/junior/src/chat/slack/admin.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/junior/src/chat/slack/admin.ts b/packages/junior/src/chat/slack/admin.ts index c753a1d588..8cf8ea5c78 100644 --- a/packages/junior/src/chat/slack/admin.ts +++ b/packages/junior/src/chat/slack/admin.ts @@ -7,8 +7,12 @@ export async function isSlackWorkspaceAdmin(userId: string): Promise { if (!parsedUserId) { return false; } - const profile = await lookupSlackUserProfile(parsedUserId); - return Boolean( - profile.is_admin || profile.is_owner || profile.is_primary_owner, - ); + try { + const profile = await lookupSlackUserProfile(parsedUserId); + return Boolean( + profile.is_admin || profile.is_owner || profile.is_primary_owner, + ); + } catch { + return false; + } } From 9152c1a6aebeee596c1fe0a24821402e03830a03 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:10:04 +0000 Subject: [PATCH 6/8] fix(linear): scope app tokens by Slack workspace --- .../junior/src/chat/capabilities/factory.ts | 26 ++++++++++--- .../credentials/installation-token-store.ts | 2 +- .../credentials/state-adapter-token-store.ts | 18 +++++---- packages/junior/src/chat/oauth-flow.ts | 5 ++- .../junior/src/chat/slack/adapter-context.ts | 8 +++- .../junior/src/handlers/oauth-callback.ts | 6 ++- .../unit/handlers/oauth-callback.test.ts | 14 +++++-- .../state/state-adapter-token-store.test.ts | 38 +++++++++++++++++-- 8 files changed, 92 insertions(+), 25 deletions(-) diff --git a/packages/junior/src/chat/capabilities/factory.ts b/packages/junior/src/chat/capabilities/factory.ts index b2732a691c..9ebc50de59 100644 --- a/packages/junior/src/chat/capabilities/factory.ts +++ b/packages/junior/src/chat/capabilities/factory.ts @@ -13,10 +13,11 @@ import type { InstallationTokenStore } from "@/chat/credentials/installation-tok import type { UserTokenStore } from "@/chat/credentials/user-token-store"; import { pluginCatalogRuntime } from "@/chat/plugins/catalog-runtime"; import { getStateAdapter } from "@/chat/state/adapter"; +import { getWorkspaceTeamId } from "@/chat/slack/workspace-context"; const sandboxEgressRouters = new WeakMap< StateAdapter, - ProviderCredentialRouter + Map >(); /** Create the user token store used by OAuth-backed credential brokers. */ @@ -25,12 +26,18 @@ export function createUserTokenStore(): UserTokenStore { } /** Create the token store used by installation OAuth grants. */ -export function createInstallationTokenStore(): InstallationTokenStore { - return new StateAdapterInstallationTokenStore(getStateAdapter()); +export function createInstallationTokenStore( + workspaceId?: string, +): InstallationTokenStore { + return new StateAdapterInstallationTokenStore( + getStateAdapter(), + workspaceId ?? getWorkspaceTeamId(), + ); } function createProviderCredentialRouter( stateAdapter: StateAdapter, + workspaceId?: string, ): ProviderCredentialRouter { const brokersByProvider: Record = {}; @@ -42,6 +49,7 @@ function createProviderCredentialRouter( brokersByProvider[name] = pluginCatalogRuntime.createBroker(name, { installationTokenStore: new StateAdapterInstallationTokenStore( stateAdapter, + workspaceId, ), userTokenStore: new StateAdapterTokenStore(stateAdapter), }); @@ -52,10 +60,16 @@ function createProviderCredentialRouter( function getSandboxEgressRouter(): ProviderCredentialRouter { const stateAdapter = getStateAdapter(); - let router = sandboxEgressRouters.get(stateAdapter); + const workspaceId = getWorkspaceTeamId() ?? "local"; + let routers = sandboxEgressRouters.get(stateAdapter); + if (!routers) { + routers = new Map(); + sandboxEgressRouters.set(stateAdapter, routers); + } + let router = routers.get(workspaceId); if (!router) { - router = createProviderCredentialRouter(stateAdapter); - sandboxEgressRouters.set(stateAdapter, router); + router = createProviderCredentialRouter(stateAdapter, workspaceId); + routers.set(workspaceId, router); } return router; } diff --git a/packages/junior/src/chat/credentials/installation-token-store.ts b/packages/junior/src/chat/credentials/installation-token-store.ts index d5cf087a66..f63125fdb0 100644 --- a/packages/junior/src/chat/credentials/installation-token-store.ts +++ b/packages/junior/src/chat/credentials/installation-token-store.ts @@ -1,6 +1,6 @@ import type { StoredTokens } from "@/chat/credentials/user-token-store"; -/** Persistent OAuth token storage shared by one Junior installation. */ +/** Persistent OAuth token storage scoped to one Slack workspace or local app. */ export interface InstallationTokenStore { get(provider: string): Promise; set(provider: string, tokens: StoredTokens): Promise; diff --git a/packages/junior/src/chat/credentials/state-adapter-token-store.ts b/packages/junior/src/chat/credentials/state-adapter-token-store.ts index f4ff590bcf..22b04b7a8d 100644 --- a/packages/junior/src/chat/credentials/state-adapter-token-store.ts +++ b/packages/junior/src/chat/credentials/state-adapter-token-store.ts @@ -19,8 +19,8 @@ function tokenKey(userId: string, provider: string): string { return `${KEY_PREFIX}:${userId}:${provider}`; } -function installationTokenKey(provider: string): string { - return `${INSTALLATION_KEY_PREFIX}:${provider}`; +function installationTokenKey(provider: string, workspaceId?: string): string { + return `${INSTALLATION_KEY_PREFIX}:${provider}:${workspaceId ?? "local"}`; } function tokenTtlMs(tokens: StoredTokens): number { @@ -100,12 +100,16 @@ export class StateAdapterTokenStore implements UserTokenStore { } } +/** Store installation OAuth tokens in the current Slack workspace scope. */ export class StateAdapterInstallationTokenStore implements InstallationTokenStore { - constructor(private readonly state: StateAdapter) {} + constructor( + private readonly state: StateAdapter, + private readonly workspaceId?: string, + ) {} async get(provider: string): Promise { const stored = await this.state.get( - installationTokenKey(provider), + installationTokenKey(provider, this.workspaceId), ); return stored === null || stored === undefined ? undefined @@ -115,14 +119,14 @@ export class StateAdapterInstallationTokenStore implements InstallationTokenStor async set(provider: string, tokens: StoredTokens): Promise { const parsed = storedTokensSchema.parse(tokens); await this.state.set( - installationTokenKey(provider), + installationTokenKey(provider, this.workspaceId), parsed, tokenTtlMs(parsed), ); } async delete(provider: string): Promise { - await this.state.delete(installationTokenKey(provider)); + await this.state.delete(installationTokenKey(provider, this.workspaceId)); } async withRefresh( @@ -131,7 +135,7 @@ export class StateAdapterInstallationTokenStore implements InstallationTokenStor ): Promise { return await withTokenRefresh( this.state, - installationTokenKey(provider), + installationTokenKey(provider, this.workspaceId), callback, ); } diff --git a/packages/junior/src/chat/oauth-flow.ts b/packages/junior/src/chat/oauth-flow.ts index e869945640..8d9e7a0f06 100644 --- a/packages/junior/src/chat/oauth-flow.ts +++ b/packages/junior/src/chat/oauth-flow.ts @@ -253,7 +253,10 @@ export async function startOAuthFlow( }; } if ( - await new StateAdapterInstallationTokenStore(getStateAdapter()).get(provider) + await new StateAdapterInstallationTokenStore( + getStateAdapter(), + input.actor.teamId, + ).get(provider) ) { return { ok: false, diff --git a/packages/junior/src/chat/slack/adapter-context.ts b/packages/junior/src/chat/slack/adapter-context.ts index 27581da7f3..c80a28cf1f 100644 --- a/packages/junior/src/chat/slack/adapter-context.ts +++ b/packages/junior/src/chat/slack/adapter-context.ts @@ -2,6 +2,7 @@ import type { SlackAdapter } from "@chat-adapter/slack"; import type { ChatInstance, StateAdapter } from "chat"; import { runWithSlackInstallationToken } from "@/chat/slack/client"; import { getStateAdapter } from "@/chat/state/adapter"; +import { runWithWorkspaceTeamId } from "@/chat/slack/workspace-context"; interface SlackAdapterInternals { defaultBotTokenProvider?: () => string | Promise; @@ -101,7 +102,7 @@ export async function runWithSlackInstallation(args: { // @ts-expect-error non-overlapping boundary cast; rule forbids as-unknown-as chains const internals = args.adapter as SlackAdapterInternals; if (internals.defaultBotTokenProvider) { - return await args.task(); + return await runWithWorkspaceTeamId(args.installation.teamId, args.task); } const installationId = args.installation.isEnterpriseInstall @@ -131,6 +132,9 @@ export async function runWithSlackInstallation(args: { enterpriseId: args.installation.enterpriseId, isEnterpriseInstall: args.installation.isEnterpriseInstall, }, - () => runWithSlackInstallationToken(tokenContext.token, args.task), + () => + runWithWorkspaceTeamId(args.installation.teamId, () => + runWithSlackInstallationToken(tokenContext.token, args.task), + ), ); } diff --git a/packages/junior/src/handlers/oauth-callback.ts b/packages/junior/src/handlers/oauth-callback.ts index 6582f78fc3..9762c33199 100644 --- a/packages/junior/src/handlers/oauth-callback.ts +++ b/packages/junior/src/handlers/oauth-callback.ts @@ -586,7 +586,11 @@ export async function GET( await stateAdapter.delete(stateKey); - const installationTokenStore = createInstallationTokenStore(); + const installationWorkspaceId = + stored.actor?.platform === "slack" ? stored.actor.teamId : undefined; + const installationTokenStore = createInstallationTokenStore( + installationWorkspaceId, + ); if (providerConfig.tokenSubject === "installation") { if ( stored.actor?.platform !== "slack" || diff --git a/packages/junior/tests/unit/handlers/oauth-callback.test.ts b/packages/junior/tests/unit/handlers/oauth-callback.test.ts index f82e510d57..4cd499889e 100644 --- a/packages/junior/tests/unit/handlers/oauth-callback.test.ts +++ b/packages/junior/tests/unit/handlers/oauth-callback.test.ts @@ -564,7 +564,9 @@ describe("oauth callback handler", () => { ); expect(response.status).toBe(200); - expect(await createInstallationTokenStore().get("linear")).toMatchObject({ + expect( + await createInstallationTokenStore("T777").get("linear"), + ).toMatchObject({ accessToken: "linear-access-token", refreshToken: "linear-refresh-token", }); @@ -593,12 +595,14 @@ describe("oauth callback handler", () => { expect(response.status).toBe(403); expect(await response.text()).toContain("Only a Slack workspace admin"); - expect(await createInstallationTokenStore().get("linear")).toBeUndefined(); + expect( + await createInstallationTokenStore("T777").get("linear"), + ).toBeUndefined(); }); it("does not replace an existing installation token", async () => { configureLinearOAuthEnv(); - await createInstallationTokenStore().set("linear", { + await createInstallationTokenStore("T777").set("linear", { accessToken: "existing-access-token", refreshToken: "existing-refresh-token", scope: "read,write", @@ -622,7 +626,9 @@ describe("oauth callback handler", () => { ); expect(response.status).toBe(409); - expect(await createInstallationTokenStore().get("linear")).toMatchObject({ + expect( + await createInstallationTokenStore("T777").get("linear"), + ).toMatchObject({ accessToken: "existing-access-token", }); }); diff --git a/packages/junior/tests/unit/state/state-adapter-token-store.test.ts b/packages/junior/tests/unit/state/state-adapter-token-store.test.ts index aec104a06c..4694fa9539 100644 --- a/packages/junior/tests/unit/state/state-adapter-token-store.test.ts +++ b/packages/junior/tests/unit/state/state-adapter-token-store.test.ts @@ -74,9 +74,9 @@ describe("StateAdapterTokenStore", () => { ); }); - it("stores installation tokens outside user slots", async () => { + it("stores installation tokens in workspace slots", async () => { const adapter = createAdapter(); - const store = new StateAdapterInstallationTokenStore(adapter); + const store = new StateAdapterInstallationTokenStore(adapter, "T123"); await store.set("linear", { accessToken: "access-token", @@ -84,7 +84,7 @@ describe("StateAdapterTokenStore", () => { }); expect(adapter.set).toHaveBeenCalledWith( - "oauth-installation-token:linear", + "oauth-installation-token:linear:T123", { accessToken: "access-token", refreshToken: "refresh-token", @@ -93,6 +93,38 @@ describe("StateAdapterTokenStore", () => { ); }); + it("keeps installation token writes isolated by workspace", async () => { + const adapter = createAdapter(); + + await new StateAdapterInstallationTokenStore(adapter, "T123").set("linear", { + accessToken: "first-workspace-token", + refreshToken: "first-workspace-refresh-token", + }); + await new StateAdapterInstallationTokenStore(adapter, "T456").set("linear", { + accessToken: "second-workspace-token", + refreshToken: "second-workspace-refresh-token", + }); + + expect(adapter.set).toHaveBeenNthCalledWith( + 1, + "oauth-installation-token:linear:T123", + { + accessToken: "first-workspace-token", + refreshToken: "first-workspace-refresh-token", + }, + 365 * 24 * 60 * 60 * 1000, + ); + expect(adapter.set).toHaveBeenNthCalledWith( + 2, + "oauth-installation-token:linear:T456", + { + accessToken: "second-workspace-token", + refreshToken: "second-workspace-refresh-token", + }, + 365 * 24 * 60 * 60 * 1000, + ); + }); + it("waits for the refresh lock before running the callback", async () => { const lock = { key: "oauth-token:U123:github:refresh", lockId: "lock-id" }; const acquireLock = vi From ec3ceb5b3bc1d905edafd4951e0316189c87bf72 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:19:40 +0000 Subject: [PATCH 7/8] fix(linear): stamp workspace on egress credentials Sandbox egress issues installation tokens outside webhook ALS, so scope credential context and shared lease keys by Slack workspace id. --- .../junior/src/chat/agent-dispatch/work.ts | 3 + .../junior/src/chat/capabilities/factory.ts | 16 +++-- .../junior/src/chat/credentials/context.ts | 26 ++++++- .../junior/src/chat/sandbox/egress/session.ts | 29 +++++--- .../chat/task-execution/conversation-turn.ts | 10 ++- .../src/chat/task-execution/paused-turn.ts | 2 +- .../capabilities/capability-factory.test.ts | 67 +++++++++++++++++++ .../credentials/credential-context.test.ts | 32 +++++++++ 8 files changed, 167 insertions(+), 18 deletions(-) diff --git a/packages/junior/src/chat/agent-dispatch/work.ts b/packages/junior/src/chat/agent-dispatch/work.ts index 5c609dfbf2..d7003108fe 100644 --- a/packages/junior/src/chat/agent-dispatch/work.ts +++ b/packages/junior/src/chat/agent-dispatch/work.ts @@ -79,6 +79,9 @@ export function buildDispatchRoutingContext( credentialContext: credentialContextForActor( dispatch.actor, dispatch.credentialSubject, + dispatch.destination.platform === "slack" + ? dispatch.destination.teamId + : undefined, ), dispatch: { actor: dispatch.actor, diff --git a/packages/junior/src/chat/capabilities/factory.ts b/packages/junior/src/chat/capabilities/factory.ts index 9ebc50de59..eac202a215 100644 --- a/packages/junior/src/chat/capabilities/factory.ts +++ b/packages/junior/src/chat/capabilities/factory.ts @@ -5,6 +5,7 @@ import type { CredentialLease, } from "@/chat/credentials/broker"; import type { CredentialContext } from "@/chat/credentials/context"; +import { credentialWorkspaceId } from "@/chat/credentials/context"; import { StateAdapterInstallationTokenStore, StateAdapterTokenStore, @@ -58,18 +59,19 @@ function createProviderCredentialRouter( return new ProviderCredentialRouter({ brokersByProvider }); } -function getSandboxEgressRouter(): ProviderCredentialRouter { +function getSandboxEgressRouter(workspaceId?: string): ProviderCredentialRouter { const stateAdapter = getStateAdapter(); - const workspaceId = getWorkspaceTeamId() ?? "local"; + const scopedWorkspaceId = + workspaceId?.trim() || getWorkspaceTeamId() || "local"; let routers = sandboxEgressRouters.get(stateAdapter); if (!routers) { routers = new Map(); sandboxEgressRouters.set(stateAdapter, routers); } - let router = routers.get(workspaceId); + let router = routers.get(scopedWorkspaceId); if (!router) { - router = createProviderCredentialRouter(stateAdapter, workspaceId); - routers.set(workspaceId, router); + router = createProviderCredentialRouter(stateAdapter, scopedWorkspaceId); + routers.set(scopedWorkspaceId, router); } return router; } @@ -80,5 +82,7 @@ export async function issueProviderCredentialLease(input: { provider: string; reason: string; }): Promise { - return await getSandboxEgressRouter().issue(input); + return await getSandboxEgressRouter(credentialWorkspaceId(input.context)).issue( + input, + ); } diff --git a/packages/junior/src/chat/credentials/context.ts b/packages/junior/src/chat/credentials/context.ts index e3326398c3..dfc0e9ae4a 100644 --- a/packages/junior/src/chat/credentials/context.ts +++ b/packages/junior/src/chat/credentials/context.ts @@ -93,16 +93,20 @@ export const credentialSubjectSchema = z.discriminatedUnion("allowedWhen", [ }), ]); +const credentialWorkspaceIdSchema = exactNonBlankStringSchema.optional(); + export const credentialContextSchema = z.union([ z .object({ actor: credentialUserActorSchema, + workspaceId: credentialWorkspaceIdSchema, }) .strict(), z .object({ actor: credentialSystemActorSchema, subject: credentialSubjectSchema.optional(), + workspaceId: credentialWorkspaceIdSchema, }) .strict(), ]); @@ -131,14 +135,25 @@ export type CredentialContext = z.output; export function credentialContextForActor( actor: Actor, subject?: CredentialSubject, + workspaceId?: string, ): CredentialContext { + const scopedWorkspaceId = + workspaceId?.trim() || + (actor.platform === "slack" ? actor.teamId : undefined); if (actor.platform === "system") { - return subject ? { actor, subject } : { actor }; + return { + actor, + ...(subject ? { subject } : undefined), + ...(scopedWorkspaceId ? { workspaceId: scopedWorkspaceId } : undefined), + }; } if (subject) { throw new TypeError("Delegated credential subjects require a system actor"); } - return { actor: { type: "user", userId: actor.userId } }; + return { + actor: { type: "user", userId: actor.userId }, + ...(scopedWorkspaceId ? { workspaceId: scopedWorkspaceId } : undefined), + }; } /** Return the user whose OAuth token may satisfy this credential request. */ @@ -151,6 +166,13 @@ export function credentialUserSubjectId( return "subject" in context ? context.subject?.userId : undefined; } +/** Return the Slack workspace scope for installation OAuth grants. */ +export function credentialWorkspaceId( + context: CredentialContext, +): string | undefined { + return context.workspaceId; +} + /** Parse an untrusted credential context payload from sandbox egress state. */ export function parseCredentialContext( value: unknown, diff --git a/packages/junior/src/chat/sandbox/egress/session.ts b/packages/junior/src/chat/sandbox/egress/session.ts index 6d3bc97bb7..27ae658be0 100644 --- a/packages/junior/src/chat/sandbox/egress/session.ts +++ b/packages/junior/src/chat/sandbox/egress/session.ts @@ -52,13 +52,16 @@ function isSharedInstallationGrant( * Build the host key for a shared installation grant. * * Only installation grants are remembered. User and other grants issue live - * headers each time, so they never write a host key. + * headers each time, so they never write a host key. Keys stay scoped to the + * Slack workspace that owns the installation token. */ function leaseKey( provider: string, grant: SandboxEgressCredentialLease["grant"], + workspaceId?: string, ): string { - return `${SANDBOX_EGRESS_LEASE_PREFIX}:${provider}:${grant.name}:shared`; + const scopedWorkspaceId = workspaceId?.trim() || "local"; + return `${SANDBOX_EGRESS_LEASE_PREFIX}:${provider}:${grant.name}:${scopedWorkspaceId}`; } /** @@ -214,7 +217,7 @@ export function parseSandboxEgressCredentialToken( * it does not shorten a shared installation grant for other sandboxes. */ export async function setSandboxEgressCredentialLease( - _context: SandboxEgressCredentialContext, + context: SandboxEgressCredentialContext, lease: SandboxEgressCredentialLease, ): Promise { if (!isSharedInstallationGrant(lease.grant)) { @@ -227,35 +230,45 @@ export async function setSandboxEgressCredentialLease( const ttlMs = Math.max(1, leaseExpiresAtMs - Date.now()); const state = getStateAdapter(); await state.connect(); - await state.set(leaseKey(lease.provider, lease.grant), lease, ttlMs); + await state.set( + leaseKey(lease.provider, lease.grant, context.credentials.workspaceId), + lease, + ttlMs, + ); } /** Load remembered auth headers for a shared installation grant. */ export async function getSandboxEgressCredentialLease( provider: string, grant: SandboxEgressCredentialLease["grant"], - _context: SandboxEgressCredentialContext, + context: SandboxEgressCredentialContext, ): Promise { if (!isSharedInstallationGrant(grant)) { return undefined; } const state = getStateAdapter(); await state.connect(); - return parseLease(await state.get(leaseKey(provider, grant))); + return parseLease( + await state.get( + leaseKey(provider, grant, context.credentials.workspaceId), + ), + ); } /** Drop remembered installation headers after the upstream rejects them. */ export async function clearSandboxEgressCredentialLease( provider: string, grant: SandboxEgressCredentialLease["grant"], - _context: SandboxEgressCredentialContext, + context: SandboxEgressCredentialContext, ): Promise { if (!isSharedInstallationGrant(grant)) { return; } const state = getStateAdapter(); await state.connect(); - await state.delete(leaseKey(provider, grant)); + await state.delete( + leaseKey(provider, grant, context.credentials.workspaceId), + ); } /** diff --git a/packages/junior/src/chat/task-execution/conversation-turn.ts b/packages/junior/src/chat/task-execution/conversation-turn.ts index b714ee2545..5328917b8f 100644 --- a/packages/junior/src/chat/task-execution/conversation-turn.ts +++ b/packages/junior/src/chat/task-execution/conversation-turn.ts @@ -479,7 +479,15 @@ export function createConversationTurnWorker( }, history: piMessages, actor, - credentialContext: credentialContextForActor(actor), + credentialContext: credentialContextForActor( + actor, + undefined, + destination.platform === "slack" + ? destination.teamId + : conversationLocation?.provider === "slack" + ? conversationLocation.teamId + : undefined, + ), // TODO(dcramer): Remove AgentRun.destination after agent and tool // code reads AgentRun.location and no Run consumer needs it. destination, diff --git a/packages/junior/src/chat/task-execution/paused-turn.ts b/packages/junior/src/chat/task-execution/paused-turn.ts index a0bde08b8a..a1b336d1fd 100644 --- a/packages/junior/src/chat/task-execution/paused-turn.ts +++ b/packages/junior/src/chat/task-execution/paused-turn.ts @@ -306,7 +306,7 @@ async function resolveResumeExecutionIdentity(args: { } return { actor, - credentialContext: credentialContextForActor(actor), + credentialContext: credentialContextForActor(actor, undefined, args.teamId), }; } diff --git a/packages/junior/tests/unit/capabilities/capability-factory.test.ts b/packages/junior/tests/unit/capabilities/capability-factory.test.ts index 189c7475d9..f0597eb5cf 100644 --- a/packages/junior/tests/unit/capabilities/capability-factory.test.ts +++ b/packages/junior/tests/unit/capabilities/capability-factory.test.ts @@ -27,6 +27,10 @@ vi.mock("@/chat/state/adapter", () => ({ }), })); +vi.mock("@/chat/slack/workspace-context", () => ({ + getWorkspaceTeamId: () => undefined, +})); + describe("capability factory", () => { afterEach(() => { createBrokerMock.mockReset(); @@ -84,6 +88,69 @@ describe("capability factory", () => { expect(lease.provider).toBe("example"); }); + it("scopes installation brokers from credential workspace id without ALS", async () => { + const firstBroker = { + issue: vi.fn(async () => ({ + id: "lease-1", + provider: "linear", + env: {}, + expiresAt: new Date(Date.now() + 60_000).toISOString(), + })), + }; + const secondBroker = { + issue: vi.fn(async () => ({ + id: "lease-2", + provider: "linear", + env: {}, + expiresAt: new Date(Date.now() + 60_000).toISOString(), + })), + }; + createBrokerMock + .mockReturnValueOnce(firstBroker) + .mockReturnValueOnce(secondBroker); + getProvidersMock.mockReturnValue([ + { + manifest: { + name: "linear", + displayName: "Linear", + description: "Linear", + configKeys: [], + credentials: { + type: "oauth-bearer", + domains: ["api.linear.app"], + authTokenEnv: "LINEAR_ACCESS_TOKEN", + }, + }, + dir: "/tmp/linear", + skillsDir: "/tmp/linear/skills", + }, + ]); + + const { issueProviderCredentialLease } = + await import("@/chat/capabilities/factory"); + + await issueProviderCredentialLease({ + context: { + actor: { type: "user", userId: "U123" }, + workspaceId: "T111", + }, + provider: "linear", + reason: "test:workspace-a", + }); + await issueProviderCredentialLease({ + context: { + actor: { type: "user", userId: "U123" }, + workspaceId: "T222", + }, + provider: "linear", + reason: "test:workspace-b", + }); + + expect(createBrokerMock).toHaveBeenCalledTimes(2); + expect(firstBroker.issue).toHaveBeenCalledTimes(1); + expect(secondBroker.issue).toHaveBeenCalledTimes(1); + }); + it("skips domain-only providers in the generic credential router", async () => { const broker = { issue: vi.fn(async () => ({ diff --git a/packages/junior/tests/unit/credentials/credential-context.test.ts b/packages/junior/tests/unit/credentials/credential-context.test.ts index 314bf4bd89..d3ec61598e 100644 --- a/packages/junior/tests/unit/credentials/credential-context.test.ts +++ b/packages/junior/tests/unit/credentials/credential-context.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from "vitest"; import { + credentialContextForActor, credentialUserSubjectId, + credentialWorkspaceId, parseCredentialContext, } from "@/chat/credentials/context"; @@ -41,6 +43,36 @@ describe("credential context", () => { ).toBeUndefined(); }); + it("stamps Slack workspace scope onto credential context", () => { + expect( + credentialWorkspaceId( + credentialContextForActor({ + platform: "slack", + teamId: "T123", + userId: "U123", + }), + ), + ).toBe("T123"); + expect( + credentialWorkspaceId( + credentialContextForActor( + { platform: "system", name: "scheduler" }, + undefined, + "T777", + ), + ), + ).toBe("T777"); + expect( + parseCredentialContext({ + actor: { type: "user", userId: "U123" }, + workspaceId: "T123", + }), + ).toEqual({ + actor: { type: "user", userId: "U123" }, + workspaceId: "T123", + }); + }); + it("parses untrusted egress contexts with the same actor rules", () => { expect( parseCredentialContext({ From ea40a21b21f3659f7607e3f896f8ecc95b8cb1b1 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:27:16 +0000 Subject: [PATCH 8/8] fix(linear): align egress workspace fallbacks Stop pairing ALS-backed installation tokens with local lease cache slots when credential context omits workspaceId. --- .../junior/src/chat/capabilities/factory.ts | 11 ++- .../junior/src/chat/sandbox/egress/session.ts | 8 +- .../capabilities/capability-factory.test.ts | 75 +++++++++++++++++-- .../sandbox-egress-credentials.test.ts | 2 +- 4 files changed, 85 insertions(+), 11 deletions(-) diff --git a/packages/junior/src/chat/capabilities/factory.ts b/packages/junior/src/chat/capabilities/factory.ts index eac202a215..802d702dbb 100644 --- a/packages/junior/src/chat/capabilities/factory.ts +++ b/packages/junior/src/chat/capabilities/factory.ts @@ -59,10 +59,17 @@ function createProviderCredentialRouter( return new ProviderCredentialRouter({ brokersByProvider }); } +function installationWorkspaceScope(workspaceId?: string): string { + const scoped = workspaceId?.trim(); + return scoped && scoped.length > 0 ? scoped : "local"; +} + function getSandboxEgressRouter(workspaceId?: string): ProviderCredentialRouter { const stateAdapter = getStateAdapter(); - const scopedWorkspaceId = - workspaceId?.trim() || getWorkspaceTeamId() || "local"; + // Egress leases are issued from a signed credential context, not webhook ALS. + // Keep the router slot aligned with lease-key scope so a missing stamp cannot + // pair ALS-backed tokens with a shared "local" cache entry. + const scopedWorkspaceId = installationWorkspaceScope(workspaceId); let routers = sandboxEgressRouters.get(stateAdapter); if (!routers) { routers = new Map(); diff --git a/packages/junior/src/chat/sandbox/egress/session.ts b/packages/junior/src/chat/sandbox/egress/session.ts index 27ae658be0..12fb72efb8 100644 --- a/packages/junior/src/chat/sandbox/egress/session.ts +++ b/packages/junior/src/chat/sandbox/egress/session.ts @@ -55,13 +55,17 @@ function isSharedInstallationGrant( * headers each time, so they never write a host key. Keys stay scoped to the * Slack workspace that owns the installation token. */ +function installationWorkspaceScope(workspaceId?: string): string { + const scoped = workspaceId?.trim(); + return scoped && scoped.length > 0 ? scoped : "local"; +} + function leaseKey( provider: string, grant: SandboxEgressCredentialLease["grant"], workspaceId?: string, ): string { - const scopedWorkspaceId = workspaceId?.trim() || "local"; - return `${SANDBOX_EGRESS_LEASE_PREFIX}:${provider}:${grant.name}:${scopedWorkspaceId}`; + return `${SANDBOX_EGRESS_LEASE_PREFIX}:${provider}:${grant.name}:${installationWorkspaceScope(workspaceId)}`; } /** diff --git a/packages/junior/tests/unit/capabilities/capability-factory.test.ts b/packages/junior/tests/unit/capabilities/capability-factory.test.ts index f0597eb5cf..8738ad08b5 100644 --- a/packages/junior/tests/unit/capabilities/capability-factory.test.ts +++ b/packages/junior/tests/unit/capabilities/capability-factory.test.ts @@ -19,22 +19,28 @@ vi.mock("@/chat/plugins/catalog-runtime", () => ({ } satisfies Pick, })); +const stateAdapter = { + get: vi.fn(), + set: vi.fn(), + delete: vi.fn(), +}; + vi.mock("@/chat/state/adapter", () => ({ - getStateAdapter: () => ({ - get: vi.fn(), - set: vi.fn(), - delete: vi.fn(), - }), + getStateAdapter: () => stateAdapter, })); +const getWorkspaceTeamIdMock = vi.fn<() => string | undefined>(() => undefined); + vi.mock("@/chat/slack/workspace-context", () => ({ - getWorkspaceTeamId: () => undefined, + getWorkspaceTeamId: () => getWorkspaceTeamIdMock(), })); describe("capability factory", () => { afterEach(() => { createBrokerMock.mockReset(); getProvidersMock.mockReset(); + getWorkspaceTeamIdMock.mockReset(); + getWorkspaceTeamIdMock.mockReturnValue(undefined); vi.resetModules(); }); @@ -151,6 +157,63 @@ describe("capability factory", () => { expect(secondBroker.issue).toHaveBeenCalledTimes(1); }); + it("does not use ALS workspace when credential context omits workspaceId", async () => { + const broker = { + issue: vi.fn(async () => ({ + id: "lease-local", + provider: "linear", + env: {}, + expiresAt: new Date(Date.now() + 60_000).toISOString(), + })), + }; + createBrokerMock.mockReturnValue(broker); + getWorkspaceTeamIdMock.mockReturnValue("T-from-als"); + getProvidersMock.mockReturnValue([ + { + manifest: { + name: "linear", + displayName: "Linear", + description: "Linear", + configKeys: [], + credentials: { + type: "oauth-bearer", + domains: ["api.linear.app"], + authTokenEnv: "LINEAR_ACCESS_TOKEN", + }, + }, + dir: "/tmp/linear", + skillsDir: "/tmp/linear/skills", + }, + ]); + + const { issueProviderCredentialLease } = + await import("@/chat/capabilities/factory"); + + await issueProviderCredentialLease({ + context: { + actor: { type: "user", userId: "U123" }, + }, + provider: "linear", + reason: "test:missing-workspace", + }); + + expect(createBrokerMock).toHaveBeenCalledTimes(1); + expect(createBrokerMock.mock.calls[0]?.[1]).toMatchObject({ + installationTokenStore: expect.any(Object), + userTokenStore: expect.any(Object), + }); + // Router must stay on the local slot so lease cache and token store match. + await issueProviderCredentialLease({ + context: { + actor: { type: "user", userId: "U123" }, + }, + provider: "linear", + reason: "test:missing-workspace-again", + }); + expect(createBrokerMock).toHaveBeenCalledTimes(1); + expect(broker.issue).toHaveBeenCalledTimes(2); + }); + it("skips domain-only providers in the generic credential router", async () => { const broker = { issue: vi.fn(async () => ({ diff --git a/packages/junior/tests/unit/handlers/sandbox-egress-credentials.test.ts b/packages/junior/tests/unit/handlers/sandbox-egress-credentials.test.ts index 99a998d6f7..2887dcc8f1 100644 --- a/packages/junior/tests/unit/handlers/sandbox-egress-credentials.test.ts +++ b/packages/junior/tests/unit/handlers/sandbox-egress-credentials.test.ts @@ -234,7 +234,7 @@ describe("sandboxEgressCredentialLease — credential error normalization", () = expect(issuePluginCredential).toHaveBeenCalledTimes(1); expect(stateStub.set.mock.calls.map(([key]) => key)).toEqual([ - "sandbox-egress-lease:sentry:installation-write:shared", + "sandbox-egress-lease:sentry:installation-write:local", ]); });