From 199489c78471d8cb1d6647a244f1fcb56d956ec2 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 7 Jun 2026 22:18:00 -0700 Subject: [PATCH 1/4] fix(github): Classify GraphQL read queries as app egress Use the visible GitHub GraphQL request body to keep read-only POST queries on the installation-read grant while mutations, subscriptions, and unknown bodies continue to require user-write attribution. This prevents read-only gh issue list traffic from unnecessarily depending on requester OAuth credentials. Co-Authored-By: GPT-5 Codex --- packages/junior-github/SETUP.md | 2 +- packages/junior-github/index.js | 45 +++++- packages/junior-plugin-api/src/index.ts | 2 + .../src/chat/plugins/credential-hooks.ts | 2 + .../src/chat/sandbox/egress-credentials.ts | 2 + .../junior/src/chat/sandbox/egress-proxy.ts | 33 ++++- .../integration/sandbox-egress-proxy.test.ts | 129 +++++++++++++++++- .../tests/unit/plugins/github-plugin.test.ts | 100 +++++++++++++- specs/credential-injection.md | 2 +- 9 files changed, 304 insertions(+), 13 deletions(-) diff --git a/packages/junior-github/SETUP.md b/packages/junior-github/SETUP.md index fa159ea1d..4645b58c9 100644 --- a/packages/junior-github/SETUP.md +++ b/packages/junior-github/SETUP.md @@ -98,7 +98,7 @@ Use `additionalUserScopes` only when an integration flow requires specific GitHu ## 3) Runtime behavior - When either GitHub skill is active, authenticated `gh` and `git` commands cause the runtime to inject GitHub credentials automatically for the current turn. -- The plugin classifies GitHub traffic from the forwarded HTTP request. Safe app-readable API methods, GraphQL `GET`/`HEAD`/`OPTIONS` requests, and `git-upload-pack` use the `installation-read` grant. `GET /user` uses the `user-read` grant so account identity checks use the requester token. Write-specific REST URLs, non-read GraphQL methods, other non-read API methods, and `git-receive-pack` use the `user-write` grant. +- The plugin classifies GitHub traffic from the forwarded HTTP request. Safe app-readable API methods, GraphQL `GET`/`HEAD`/`OPTIONS` requests, GraphQL `POST` bodies that prove the operation is a query, and `git-upload-pack` use the `installation-read` grant. `GET /user` uses the `user-read` grant so account identity checks use the requester token. Write-specific REST URLs, GraphQL mutations/subscriptions, unknown GraphQL `POST` bodies, other non-read API methods, and `git-receive-pack` use the `user-write` grant. - `user-read` and `user-write` require the requester, or an explicitly delegated user subject from an allowed system run, to authorize the GitHub App through the private OAuth flow. Missing or expired user authorization pauses interactive turns, sends a private authorization link, and resumes after approval. - Git commits use the requester as the commit author, Junior as committer, and a Junior `Co-Authored-By` trailer. - Issued credentials are reused only within the current turn, credential leases are cached separately by plugin grant name, and upstream 403 permission denials clear the cached lease before the next retry. diff --git a/packages/junior-github/index.js b/packages/junior-github/index.js index 1a250a2cc..400531f89 100644 --- a/packages/junior-github/index.js +++ b/packages/junior-github/index.js @@ -701,17 +701,46 @@ function githubUserReadReason(method, upstreamUrl) { : undefined; } -function githubGraphqlAccess(method, upstreamUrl) { +function parseGitHubGraphqlOperation(bodyText) { + if (typeof bodyText !== "string" || bodyText.trim().length === 0) { + return undefined; + } + let parsed; + try { + parsed = JSON.parse(bodyText); + } catch { + return undefined; + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return undefined; + } + const query = parsed.query; + if (typeof query !== "string") { + return undefined; + } + const normalized = query.replace(/^\s*#[^\n\r]*(?:\r?\n|$)/gm, "").trim(); + if (/\b(mutation|subscription)\b/.test(normalized)) { + return "write"; + } + if (normalized.startsWith("{") || /\bquery\b/.test(normalized)) { + return "read"; + } + return undefined; +} + +function githubGraphqlAccess(method, upstreamUrl, bodyText) { if (!isGitHubGraphqlUrl(upstreamUrl)) { return undefined; } if (HTTP_READ_METHODS.has(method)) { return "read"; } - // GitHub GraphQL POST bodies can be read queries or write mutations. The - // egress hook intentionally does not read sandbox request bodies, so non-read - // methods require user-write attribution rather than risking an unattributed - // mutation through an installation-read token. + const operation = parseGitHubGraphqlOperation(bodyText); + if (operation) { + return operation; + } + // Unknown GraphQL POST bodies still require user-write attribution rather + // than risking an unattributed mutation through an installation-read token. return "write"; } @@ -830,7 +859,11 @@ async function githubGrantForEgress(ctx) { return grantForAccess("write", writeReason, "user-write"); } - const graphqlAccess = githubGraphqlAccess(method, upstreamUrl); + const graphqlAccess = githubGraphqlAccess( + method, + upstreamUrl, + ctx.request.bodyText, + ); if (graphqlAccess) { return grantForAccess( graphqlAccess, diff --git a/packages/junior-plugin-api/src/index.ts b/packages/junior-plugin-api/src/index.ts index 72c67a1f7..bc4f71030 100644 --- a/packages/junior-plugin-api/src/index.ts +++ b/packages/junior-plugin-api/src/index.ts @@ -453,6 +453,8 @@ export type AgentPluginGrant = z.output; /** Request details available while selecting the grant for sandbox egress. */ export interface AgentPluginEgressRequest { + /** Capped request body text when the host exposes it for provider-specific grant classification. */ + bodyText?: string; method: string; url: string; } diff --git a/packages/junior/src/chat/plugins/credential-hooks.ts b/packages/junior/src/chat/plugins/credential-hooks.ts index db60cd87b..b4863269d 100644 --- a/packages/junior/src/chat/plugins/credential-hooks.ts +++ b/packages/junior/src/chat/plugins/credential-hooks.ts @@ -91,6 +91,7 @@ function parseCredentialResult( } export interface EgressGrantInput { + bodyText?: string; method: string; provider: string; upstreamUrl: URL; @@ -109,6 +110,7 @@ export async function selectPluginGrant( plugin: { name: plugin.name }, log: createAgentPluginLogger(plugin.name), request: { + ...(input.bodyText !== undefined ? { bodyText: input.bodyText } : {}), method: input.method, url: input.upstreamUrl.toString(), }, diff --git a/packages/junior/src/chat/sandbox/egress-credentials.ts b/packages/junior/src/chat/sandbox/egress-credentials.ts index 88e5e506d..4bab89ae4 100644 --- a/packages/junior/src/chat/sandbox/egress-credentials.ts +++ b/packages/junior/src/chat/sandbox/egress-credentials.ts @@ -111,6 +111,7 @@ function assertLeaseTransformsOwnedByProvider( /** Select the plugin-defined or default grant needed for one outbound request. */ export async function selectSandboxEgressGrant(input: { + bodyText?: string; method: string; provider: string; upstreamUrl: URL; @@ -120,6 +121,7 @@ export async function selectSandboxEgressGrant(input: { } const pluginGrant = await selectPluginGrant({ + ...(input.bodyText !== undefined ? { bodyText: input.bodyText } : {}), provider: input.provider, method: input.method, upstreamUrl: input.upstreamUrl, diff --git a/packages/junior/src/chat/sandbox/egress-proxy.ts b/packages/junior/src/chat/sandbox/egress-proxy.ts index 4cee92fd8..c97d0a763 100644 --- a/packages/junior/src/chat/sandbox/egress-proxy.ts +++ b/packages/junior/src/chat/sandbox/egress-proxy.ts @@ -51,6 +51,7 @@ const DECODED_RESPONSE_HEADERS = new Set([ ]); const UPSTREAM_TOKEN_REJECTION_STATUS = 401; const UPSTREAM_PERMISSION_REJECTION_STATUS = 403; +const GRANT_SELECTION_BODY_TEXT_LIMIT_BYTES = 64 * 1024; /** Intercepts a credential-injected sandbox HTTP request before live forwarding. */ export type SandboxEgressHttpInterceptor = (input: { @@ -365,6 +366,27 @@ async function requestBodyBytes( return await request.arrayBuffer(); } +function isGrantSelectionBodyVisible(input: { + provider: string; + upstreamUrl: URL; +}): boolean { + return ( + input.provider === "github" && + input.upstreamUrl.hostname.toLowerCase() === "api.github.com" && + input.upstreamUrl.pathname.toLowerCase().endsWith("/graphql") + ); +} + +function requestBodyText(body: ArrayBuffer | undefined): string | undefined { + if ( + body === undefined || + body.byteLength > GRANT_SELECTION_BODY_TEXT_LIMIT_BYTES + ) { + return undefined; + } + return new TextDecoder().decode(body); +} + function requestHeaders( request: Request, lease: SandboxEgressCredentialLease, @@ -530,7 +552,14 @@ export async function proxySandboxEgressRequest( ); } + let body: ArrayBuffer | undefined; + let bodyRead = false; + if (isGrantSelectionBodyVisible({ provider, upstreamUrl })) { + body = await requestBodyBytes(request); + bodyRead = true; + } const grantSelection = await selectSandboxEgressGrant({ + bodyText: requestBodyText(body), provider, method: request.method, upstreamUrl, @@ -644,7 +673,9 @@ export async function proxySandboxEgressRequest( const fetchImpl = deps.fetch ?? fetch; const headers = requestHeaders(request, lease, upstreamUrl.hostname); - const body = await requestBodyBytes(request); + if (!bodyRead) { + body = await requestBodyBytes(request); + } const intercepted = await deps.interceptHttp?.({ provider, request: new Request(upstreamUrl, { diff --git a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts index 9d3a110d7..403688482 100644 --- a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts +++ b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts @@ -1,14 +1,17 @@ +import { generateKeyPairSync } from "node:crypto"; import path from "node:path"; import { defineJuniorPlugin, type AgentPluginHooks, } from "@sentry/junior-plugin-api"; +import { http, HttpResponse } from "msw"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createPluginAppFixture, type PluginAppFixture, } from "../fixtures/plugin-app"; import { githubPlugin } from "../../../junior-github/index.js"; +import { mswServer } from "../msw/server"; const ORIGINAL_ENV = { ...process.env }; const FIXTURE_PLUGIN_ROOT = path.resolve( @@ -143,9 +146,11 @@ async function registerManagedEgressPlugin(input?: { }); } -async function registerGitHubPlugin() { +async function registerGitHubPlugin( + options?: Parameters[0], +) { const { createApp, defineJuniorPlugins } = await import("@/app"); - const plugin = githubPlugin(); + const plugin = githubPlugin(options); await createApp({ plugins: defineJuniorPlugins([ { @@ -161,6 +166,33 @@ async function registerGitHubPlugin() { }); } +function configureGitHubAppEnv() { + process.env.GITHUB_APP_ID = "123"; + process.env.GITHUB_INSTALLATION_ID = "456"; + process.env.GITHUB_APP_PRIVATE_KEY = generateKeyPairSync("rsa", { + modulusLength: 2048, + }) + .privateKey.export({ type: "pkcs8", format: "pem" }) + .toString(); +} + +function mockGitHubInstallationToken() { + const requests: unknown[] = []; + mswServer.use( + http.post( + "https://api.github.com/app/installations/:installationId/access_tokens", + async ({ request }) => { + requests.push(await request.json()); + return HttpResponse.json({ + token: "installation-token", + expires_at: new Date(Date.now() + 60_000).toISOString(), + }); + }, + ), + ); + return requests; +} + describe("sandbox egress proxy integration", () => { let modules: LoadedModules; let pluginApp: PluginAppFixture | undefined; @@ -358,6 +390,99 @@ describe("sandbox egress proxy integration", () => { expect(upstreamFetch).toHaveBeenCalledTimes(2); }); + it("uses GitHub App credentials for GraphQL issue list queries", async () => { + configureGitHubAppEnv(); + const tokenRequests = mockGitHubInstallationToken(); + await registerGitHubPlugin({ + appPermissions: { + contents: "read", + issues: "write", + }, + }); + const credentialToken = modules.session.createSandboxEgressCredentialToken({ + credentials: { actor: { type: "user", userId: REQUESTER_ID } }, + egressId: EGRESS_ID, + ttlMs: 60_000, + }); + const networkPolicy = modules.policy.buildSandboxEgressNetworkPolicy({ + credentialToken, + }); + const forwardURL = forwardUrlFor(networkPolicy, GITHUB_API_HOST); + const upstreamFetch = vi.fn( + async (_url: URL | string, init?: RequestInit) => + new Response(new Headers(init?.headers).get("authorization")), + ); + + const response = await modules.proxy.proxySandboxEgressRequest( + proxiedRequest({ + body: JSON.stringify({ + query: + "fragment issue on Issue { number title state url } query IssueList($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { issues(first: 1) { nodes { ...issue } } } }", + variables: { owner: "getsentry", name: "junior-prod" }, + }), + forwardURL, + method: "POST", + upstreamHost: GITHUB_API_HOST, + upstreamPath: "/graphql", + }), + { + fetch: upstreamFetch as typeof fetch, + verifyOidc: async () => ({ sandbox_id: EGRESS_ID }), + }, + ); + + expect(response.status).toBe(200); + await expect(response.text()).resolves.toBe("Bearer installation-token"); + expect(upstreamFetch).toHaveBeenCalledTimes(1); + expect(tokenRequests).toEqual([ + { + permissions: { + contents: "read", + issues: "read", + metadata: "read", + }, + }, + ]); + }); + + it("keeps GraphQL mutations on GitHub user-write credentials", async () => { + await registerGitHubPlugin(); + const credentialToken = modules.session.createSandboxEgressCredentialToken({ + credentials: { actor: { type: "user", userId: REQUESTER_ID } }, + egressId: EGRESS_ID, + ttlMs: 60_000, + }); + const networkPolicy = modules.policy.buildSandboxEgressNetworkPolicy({ + credentialToken, + }); + const forwardURL = forwardUrlFor(networkPolicy, GITHUB_API_HOST); + const upstreamFetch = vi.fn(); + + const response = await modules.proxy.proxySandboxEgressRequest( + proxiedRequest({ + body: JSON.stringify({ + query: + "mutation CreateIssue($input: CreateIssueInput!) { createIssue(input: $input) { issue { number } } }", + variables: { input: { repositoryId: "repo", title: "test" } }, + }), + forwardURL, + method: "POST", + upstreamHost: GITHUB_API_HOST, + upstreamPath: "/graphql", + }), + { + fetch: upstreamFetch as typeof fetch, + verifyOidc: async () => ({ sandbox_id: EGRESS_ID }), + }, + ); + + expect(response.status).toBe(401); + await expect(response.text()).resolves.toContain( + "junior-auth-required provider=github grant=user-write access=write", + ); + expect(upstreamFetch).not.toHaveBeenCalled(); + }); + it("records plugin write auth needs over earlier read failures", async () => { await registerManagedEgressPlugin({ issueCredential(ctx) { diff --git a/packages/junior/tests/unit/plugins/github-plugin.test.ts b/packages/junior/tests/unit/plugins/github-plugin.test.ts index 52f2bcad6..3bc0a233a 100644 --- a/packages/junior/tests/unit/plugins/github-plugin.test.ts +++ b/packages/junior/tests/unit/plugins/github-plugin.test.ts @@ -152,12 +152,17 @@ function mockGitHubRefresh( return requests; } -async function grantForEgress(input: { method: string; url: string }) { +async function grantForEgress(input: { + bodyText?: string; + method: string; + url: string; +}) { const plugin = githubPlugin({ additionalUserScopes: ["repo"] }); return await plugin.hooks?.grantForEgress?.({ log: pluginLog, plugin: { name: "github" }, request: { + ...(input.bodyText !== undefined ? { bodyText: input.bodyText } : {}), method: input.method, url: input.url, }, @@ -383,7 +388,7 @@ describe("github plugin", () => { }); }); - it("treats GitHub GraphQL GET as read and POST as write", async () => { + it("treats GitHub GraphQL GET as read and ambiguous POST as write", async () => { expect( await grantForEgress({ method: "GET", @@ -406,6 +411,97 @@ describe("github plugin", () => { }); }); + it("selects installation-read for GitHub GraphQL read operations", async () => { + expect( + await grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: JSON.stringify({ + query: `query IssueList { + repository(owner: "getsentry", name: "junior-prod") { + issues(first: 20) { nodes { number title } } + } + }`, + }), + }), + ).toMatchObject({ + name: "installation-read", + access: "read", + reason: "github.graphql-read", + }); + expect( + await grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: JSON.stringify({ + query: "{ viewer { login } }", + }), + }), + ).toMatchObject({ + name: "installation-read", + access: "read", + reason: "github.graphql-read", + }); + }); + + it("keeps GitHub GraphQL mutations, subscriptions, and unparseable bodies on user-write", async () => { + await expect( + grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: JSON.stringify({ + query: `mutation AddIssueComment { + addComment(input: {subjectId: "I_kwDO", body: "test"}) { + clientMutationId + } + }`, + }), + }), + ).resolves.toMatchObject({ + name: "user-write", + access: "write", + reason: "github.graphql-write", + }); + await expect( + grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: JSON.stringify({ + query: "subscription IssueEvents { issueEvents { id } }", + }), + }), + ).resolves.toMatchObject({ + name: "user-write", + access: "write", + reason: "github.graphql-write", + }); + await expect( + grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: JSON.stringify({ + query: + 'fragment issueFields on Issue { number } mutation Search($query: String!) { createIssue(input: {repositoryId: "repo", title: $query}) { issue { ...issueFields } } }', + }), + }), + ).resolves.toMatchObject({ + name: "user-write", + access: "write", + reason: "github.graphql-write", + }); + await expect( + grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: "{", + }), + ).resolves.toMatchObject({ + name: "user-write", + access: "write", + reason: "github.graphql-write", + }); + }); + it("adds provider requirements to known GitHub write grants", async () => { await expect( grantForEgress({ diff --git a/specs/credential-injection.md b/specs/credential-injection.md index 09e25f8fb..70b908fc1 100644 --- a/specs/credential-injection.md +++ b/specs/credential-injection.md @@ -85,7 +85,7 @@ Define how Junior maps registered plugin provider domains to host-managed creden - The built-in GitHub plugin declares `api.github.com` for REST API calls and `github.com` for git smart-HTTP. - Runtime may reuse a short-lived sandbox egress lease for repeated GitHub commands in the same turn, but distinct plugin grant names are cached separately. Cached leases reuse issued headers only; logs and auth/permission signals must use the grant metadata selected for the current outbound request. -- GitHub read grants are derived by the GitHub plugin from runtime-visible HTTP evidence, including safe HTTP methods, GraphQL `GET`/`HEAD`/`OPTIONS` requests, `GET /user`, and `git-upload-pack`. GitHub write grants are derived from runtime-visible write evidence, including write-specific REST URLs, non-read GraphQL methods, other non-read HTTP methods, and `git-receive-pack`. +- GitHub read grants are derived by the GitHub plugin from runtime-visible HTTP evidence, including safe HTTP methods, GraphQL `GET`/`HEAD`/`OPTIONS` requests, GraphQL `POST` bodies that prove the operation is a query, `GET /user`, and `git-upload-pack`. GitHub write grants are derived from runtime-visible write evidence, including write-specific REST URLs, GraphQL mutations/subscriptions, unknown GraphQL `POST` bodies, other non-read HTTP methods, and `git-receive-pack`. - When a GitHub App installation lease is issued, the GitHub plugin sends an explicit read-only permissions body instead of inheriting the installation's default permissions. - If the plugin declares GitHub App permissions, each read-capable configured permission is requested at `read` level for installation-read leases. From dcd831795e1a0bff643adc940c489740d4e74410 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 7 Jun 2026 23:34:22 -0700 Subject: [PATCH 2/4] fix(github): Surface GraphQL access denials from egress Add a provider-owned egress response hook that preserves upstream responses by default and only rewrites when a plugin throws EgressAuthRequired. Use the hook in the GitHub plugin to annotate known GraphQL access errors returned in HTTP 200 response bodies, so failed commands receive a structured permission_denied signal without core GitHub special cases. Co-Authored-By: GPT-5 Codex --- packages/junior-github/index.js | 64 +++++ packages/junior-plugin-api/src/index.ts | 32 +++ .../src/chat/plugins/credential-hooks.ts | 50 ++++ .../junior/src/chat/sandbox/egress-proxy.ts | 168 +++++++++++++ .../junior/src/chat/sandbox/egress-schemas.ts | 3 +- .../chat/tools/execution/normalize-result.ts | 12 +- .../integration/sandbox-egress-proxy.test.ts | 230 +++++++++++++++++- .../tests/unit/plugins/github-plugin.test.ts | 15 +- specs/credential-injection.md | 9 +- 9 files changed, 557 insertions(+), 26 deletions(-) diff --git a/packages/junior-github/index.js b/packages/junior-github/index.js index 400531f89..98f06a55a 100644 --- a/packages/junior-github/index.js +++ b/packages/junior-github/index.js @@ -13,6 +13,7 @@ const GITHUB_AUTH_TOKEN_ENV = "GITHUB_TOKEN"; const GITHUB_AUTH_TOKEN_PLACEHOLDER = "ghp_host_managed_credential"; const MAX_LEASE_MS = 60 * 60 * 1000; const REFRESH_BUFFER_MS = 5 * 60 * 1000; +const GITHUB_GRAPHQL_RESPONSE_BODY_LIMIT_BYTES = 64 * 1024; const HTTP_READ_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); const USER_TOKEN_GRANTS = new Set(["user-read", "user-write"]); const CONTENTS_WRITE_REQUIREMENTS = [ @@ -744,6 +745,54 @@ function githubGraphqlAccess(method, upstreamUrl, bodyText) { return "write"; } +function githubGraphqlPermissionDeniedMessage(bodyText) { + let parsed; + try { + parsed = JSON.parse(bodyText); + } catch { + return undefined; + } + if (!isRecord(parsed) || !Array.isArray(parsed.errors)) { + return undefined; + } + for (const error of parsed.errors) { + if (!isRecord(error) || typeof error.message !== "string") { + continue; + } + const message = error.message; + if ( + error.type === "NOT_FOUND" && + /\bCould not resolve to a Repository with the name\b/.test(message) + ) { + return `GitHub GraphQL could not access the repository: ${message}`; + } + if (/\bResource not accessible by integration\b/.test(message)) { + return `GitHub GraphQL denied access: ${message}`; + } + } + return undefined; +} + +function shouldInspectGitHubGraphqlResponse(ctx) { + if ( + ctx.request.method.toUpperCase() !== "POST" || + ctx.response.status !== 200 + ) { + return false; + } + let upstreamUrl; + try { + upstreamUrl = new URL(ctx.request.url); + } catch { + return false; + } + if (!isGitHubGraphqlUrl(upstreamUrl)) { + return false; + } + const contentType = ctx.response.headers.get("content-type"); + return contentType ? /\bjson\b/i.test(contentType) : false; +} + function githubApiWriteReason(method, upstreamUrl) { const pathname = upstreamUrl.pathname.toLowerCase(); if (!isGitHubApiUrl(upstreamUrl)) { @@ -1002,6 +1051,21 @@ export function githubPlugin(options = {}) { grantForEgress(ctx) { return githubGrantForEgress(ctx); }, + async onEgressResponse(ctx) { + if (!shouldInspectGitHubGraphqlResponse(ctx)) { + return; + } + const bodyText = await ctx.response.readText( + GITHUB_GRAPHQL_RESPONSE_BODY_LIMIT_BYTES, + ); + if (!bodyText) { + return; + } + const message = githubGraphqlPermissionDeniedMessage(bodyText); + if (message) { + ctx.permissionDenied(message); + } + }, async resolveOAuthAccount(ctx) { return await resolveUserAccount(ctx.tokens); }, diff --git a/packages/junior-plugin-api/src/index.ts b/packages/junior-plugin-api/src/index.ts index bc4f71030..c4213bacf 100644 --- a/packages/junior-plugin-api/src/index.ts +++ b/packages/junior-plugin-api/src/index.ts @@ -443,6 +443,23 @@ export type AgentPluginAuthorization = z.output< typeof agentPluginAuthorizationSchema >; +/** Interrupt sandbox egress so Junior can start provider authorization. */ +export class EgressAuthRequired extends Error { + authorization?: AgentPluginAuthorization; + + constructor( + message: string, + options?: { + authorization?: AgentPluginAuthorization; + cause?: unknown; + }, + ) { + super(message, { cause: options?.cause }); + this.name = "EgressAuthRequired"; + this.authorization = options?.authorization; + } +} + /** Provider account identity resolved by a plugin OAuth hook. */ export type AgentPluginProviderAccount = z.output< typeof agentPluginProviderAccountSchema @@ -463,6 +480,20 @@ export interface EgressHookContext extends AgentPluginContext { request: AgentPluginEgressRequest; } +export interface AgentPluginEgressResponse { + /** Snapshot of upstream response headers; mutations do not affect pass-through. */ + headers: Headers; + readText(maxBytes: number): Promise; + status: number; +} + +export interface EgressResponseHookContext extends AgentPluginContext { + grant: AgentPluginGrant; + permissionDenied(message: string): void; + request: Omit; + response: AgentPluginEgressResponse; +} + /** Header mutations a plugin-issued credential lease may apply to owned domains. */ export type AgentPluginCredentialHeaderTransform = z.output< typeof agentPluginCredentialHeaderTransformSchema @@ -531,6 +562,7 @@ export interface AgentPluginHooks { issueCredential?( ctx: IssueCredentialHookContext, ): Promise | AgentPluginCredentialResult; + onEgressResponse?(ctx: EgressResponseHookContext): Promise | void; resolveOAuthAccount?( ctx: ResolveOAuthAccountHookContext, ): diff --git a/packages/junior/src/chat/plugins/credential-hooks.ts b/packages/junior/src/chat/plugins/credential-hooks.ts index b4863269d..a7686ecfc 100644 --- a/packages/junior/src/chat/plugins/credential-hooks.ts +++ b/packages/junior/src/chat/plugins/credential-hooks.ts @@ -118,6 +118,56 @@ export async function selectPluginGrant( return result === undefined ? undefined : parseGrant(result, plugin.name); } +export interface EgressResponseInput { + grant: AgentPluginGrant; + method: string; + provider: string; + response: { + headers: Headers; + readText(maxBytes: number): Promise; + status: number; + }; + upstreamUrl: URL; +} + +export interface EgressResponseEffects { + permissionDenied?: { + message: string; + }; +} + +/** Let the owning plugin inspect an upstream response without changing pass-through behavior. */ +export async function onPluginEgressResponse( + input: EgressResponseInput, +): Promise { + const plugin = agentPluginFor(input.provider); + const hook = plugin?.hooks?.onEgressResponse; + if (!plugin || !hook) { + return {}; + } + let permissionDenied: { message: string } | undefined; + await hook({ + plugin: { name: plugin.name }, + log: createAgentPluginLogger(plugin.name), + grant: input.grant, + permissionDenied(message) { + const trimmed = message.trim(); + if (!trimmed) { + throw new Error( + `Plugin "${plugin.name}" onEgressResponse permissionDenied message is empty`, + ); + } + permissionDenied = { message: trimmed }; + }, + request: { + method: input.method, + url: input.upstreamUrl.toString(), + }, + response: input.response, + }); + return permissionDenied ? { permissionDenied } : {}; +} + /** Return whether a plugin owns credential issuance for egress. */ export function hasEgressCredentialHooks(provider: string): boolean { const hooks = agentPluginFor(provider)?.hooks; diff --git a/packages/junior/src/chat/sandbox/egress-proxy.ts b/packages/junior/src/chat/sandbox/egress-proxy.ts index c97d0a763..6a5e81740 100644 --- a/packages/junior/src/chat/sandbox/egress-proxy.ts +++ b/packages/junior/src/chat/sandbox/egress-proxy.ts @@ -1,5 +1,6 @@ import { CredentialUnavailableError } from "@/chat/credentials/broker"; import { logInfo, logWarn } from "@/chat/logging"; +import { onPluginEgressResponse } from "@/chat/plugins/credential-hooks"; import { matchesSandboxEgressDomain, resolveSandboxEgressProviderForHost, @@ -20,6 +21,7 @@ import { setSandboxEgressPermissionDeniedSignal, type SandboxEgressCredentialLease, } from "@/chat/sandbox/egress-session"; +import { EgressAuthRequired } from "@sentry/junior-plugin-api"; import type { JWTPayload } from "jose"; const OIDC_TOKEN_HEADER = "vercel-sandbox-oidc-token"; @@ -52,6 +54,7 @@ const DECODED_RESPONSE_HEADERS = new Set([ const UPSTREAM_TOKEN_REJECTION_STATUS = 401; const UPSTREAM_PERMISSION_REJECTION_STATUS = 403; const GRANT_SELECTION_BODY_TEXT_LIMIT_BYTES = 64 * 1024; +const RESPONSE_BODY_TEXT_LIMIT_BYTES = 64 * 1024; /** Intercepts a credential-injected sandbox HTTP request before live forwarding. */ export type SandboxEgressHttpInterceptor = (input: { @@ -219,6 +222,13 @@ function permissionDeniedMessage( return `${provider} returned HTTP 403 after Junior injected the ${grant.name} grant. Junior forwarded the request; this is not a local runtime block.`; } +function isEgressAuthRequired(error: unknown): error is EgressAuthRequired { + return ( + error instanceof EgressAuthRequired || + (error instanceof Error && error.name === "EgressAuthRequired") + ); +} + function logSandboxEgressUpstreamRequest(input: { egressId: string; grantAccess?: "read" | "write"; @@ -387,6 +397,74 @@ function requestBodyText(body: ArrayBuffer | undefined): string | undefined { return new TextDecoder().decode(body); } +function responseContentLength(upstream: Response): number | undefined { + const raw = upstream.headers.get("content-length"); + if (!raw) { + return undefined; + } + const parsed = Number(raw); + return Number.isSafeInteger(parsed) && parsed >= 0 ? parsed : undefined; +} + +async function responseTextWithinLimit( + upstream: Response, + maxBytes: number, +): Promise { + const limit = Math.min( + Math.max(0, Math.floor(maxBytes)), + RESPONSE_BODY_TEXT_LIMIT_BYTES, + ); + if (limit <= 0) { + return undefined; + } + const contentLength = responseContentLength(upstream); + if (contentLength !== undefined && contentLength > limit) { + return undefined; + } + let clone: Response; + try { + clone = upstream.clone(); + } catch { + return undefined; + } + const body = clone.body; + if (!body) { + return ""; + } + const reader = body.getReader(); + const chunks: Uint8Array[] = []; + let bytes = 0; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) { + break; + } + if (!value) { + continue; + } + bytes += value.byteLength; + if (bytes > limit) { + await reader.cancel().catch(() => undefined); + return undefined; + } + chunks.push(value); + } + } catch { + await reader.cancel().catch(() => undefined); + return undefined; + } finally { + reader.releaseLock(); + } + const combined = new Uint8Array(bytes); + let offset = 0; + for (const chunk of chunks) { + combined.set(chunk, offset); + offset += chunk.byteLength; + } + return new TextDecoder().decode(combined); +} + function requestHeaders( request: Request, lease: SandboxEgressCredentialLease, @@ -695,6 +773,96 @@ export async function proxySandboxEgressRequest( ...(body !== undefined ? { body } : {}), redirect: "manual", }); + try { + const effects = await onPluginEgressResponse({ + provider, + grant: lease.grant, + method: request.method, + upstreamUrl, + response: { + headers: new Headers(upstream.headers), + readText: async (maxBytes) => + await responseTextWithinLimit(upstream, maxBytes), + status: upstream.status, + }, + }); + if (effects.permissionDenied) { + await setSandboxEgressPermissionDeniedSignal(credentialContext, { + provider, + grant: lease.grant, + ...(lease.account ? { account: lease.account } : {}), + message: effects.permissionDenied.message, + source: "upstream", + status: upstream.status, + upstreamHost: upstreamUrl.hostname, + upstreamPath: displayedUpstreamPath(upstreamUrl), + ...(provider === "github" ? githubPermissionHeaders(upstream) : {}), + }); + logWarn( + "sandbox_egress_upstream_permission_classified", + {}, + { + ...egressAttributes({ + egressId: activeEgressId, + grantAccess: lease.grant.access, + grantName: lease.grant.name, + grantReason: lease.grant.reason, + host: upstreamUrl.hostname, + method: request.method, + path: upstreamUrl.pathname, + provider, + status: upstream.status, + }), + ...routingAttributes(request, upstreamUrl), + ...upstreamPermissionAttributes(provider, upstream), + }, + "Sandbox egress plugin classified upstream response as permission denied", + ); + } + } catch (error) { + if (!isEgressAuthRequired(error)) { + throw error; + } + await clearSandboxEgressCredentialLease( + provider, + lease.grant.name, + credentialContext, + ); + await setSandboxEgressAuthRequiredSignal(credentialContext, { + provider, + grant: lease.grant, + ...((error.authorization ?? lease.authorization) + ? { authorization: error.authorization ?? lease.authorization } + : {}), + message: error.message, + }); + logWarn( + "sandbox_egress_upstream_auth_required_classified", + {}, + { + ...egressAttributes({ + egressId: activeEgressId, + grantAccess: lease.grant.access, + grantName: lease.grant.name, + grantReason: lease.grant.reason, + host: upstreamUrl.hostname, + method: request.method, + path: upstreamUrl.pathname, + provider, + status: upstream.status, + }), + ...routingAttributes(request, upstreamUrl), + ...upstreamPermissionAttributes(provider, upstream), + }, + "Sandbox egress plugin classified upstream response as auth required", + ); + await upstream.body?.cancel().catch(() => undefined); + return authRequiredResponse({ + provider, + grant: lease.grant, + message: error.message, + }); + } logSandboxEgressUpstreamRequest({ egressId: activeEgressId, grantAccess: lease.grant.access, diff --git a/packages/junior/src/chat/sandbox/egress-schemas.ts b/packages/junior/src/chat/sandbox/egress-schemas.ts index ec6e442ae..037b984ec 100644 --- a/packages/junior/src/chat/sandbox/egress-schemas.ts +++ b/packages/junior/src/chat/sandbox/egress-schemas.ts @@ -8,6 +8,7 @@ import { } from "@sentry/junior-plugin-api"; const finiteNumberSchema = z.number().refine(Number.isFinite); +const httpStatusSchema = z.number().int().min(100).max(599); const providerNameSchema = z.string().regex(/^[a-z][a-z0-9-]*$/); export const sandboxEgressGrantSchema = agentPluginGrantSchema; @@ -65,7 +66,7 @@ export const sandboxEgressPermissionDeniedSignalSchema = z provider: providerNameSchema, source: z.literal("upstream"), sso: z.string().optional(), - status: z.literal(403), + status: httpStatusSchema, upstreamHost: z.string().min(1), upstreamPath: z.string().min(1), createdAtMs: finiteNumberSchema, diff --git a/packages/junior/src/chat/tools/execution/normalize-result.ts b/packages/junior/src/chat/tools/execution/normalize-result.ts index b80abf362..3cb80f7f4 100644 --- a/packages/junior/src/chat/tools/execution/normalize-result.ts +++ b/packages/junior/src/chat/tools/execution/normalize-result.ts @@ -46,6 +46,11 @@ function stringField(record: Record, key: string): string { return typeof value === "string" ? value : ""; } +function numberField(record: Record, key: string): number { + const value = record[key]; + return typeof value === "number" && Number.isFinite(value) ? value : 0; +} + function stringListField( record: Record, key: string, @@ -73,14 +78,15 @@ function upstreamPermissionDeniedText(value: unknown): string | undefined { return undefined; } const signal = value.permission_denied; - if (signal.source !== "upstream" || signal.status !== 403) { + if (signal.source !== "upstream") { return undefined; } const provider = stringField(signal, "provider"); const message = stringField(signal, "message"); const upstreamHost = stringField(signal, "upstreamHost"); const upstreamPath = stringField(signal, "upstreamPath"); - if (!provider || !message || !upstreamHost || !upstreamPath) { + const status = numberField(signal, "status"); + if (!provider || !message || !upstreamHost || !upstreamPath || !status) { return undefined; } const grant = isRecord(signal.grant) ? signal.grant : {}; @@ -109,7 +115,7 @@ function upstreamPermissionDeniedText(value: unknown): string | undefined { ] : []), `Upstream: ${upstreamHost}${upstreamPath}`, - "Status: 403", + `Status: ${status}`, ...(acceptedPermissions ? [`Accepted provider permissions: ${acceptedPermissions}`] : []), diff --git a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts index 403688482..a281b389b 100644 --- a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts +++ b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts @@ -2,6 +2,7 @@ import { generateKeyPairSync } from "node:crypto"; import path from "node:path"; import { defineJuniorPlugin, + EgressAuthRequired, type AgentPluginHooks, } from "@sentry/junior-plugin-api"; import { http, HttpResponse } from "msw"; @@ -94,6 +95,7 @@ function proxiedRequest(input: { async function registerManagedEgressPlugin(input?: { issueCredential?: NonNullable; + onEgressResponse?: NonNullable; }) { const { createApp, defineJuniorPlugins } = await import("@/app"); await createApp({ @@ -135,6 +137,9 @@ async function registerManagedEgressPlugin(input?: { ], }, })), + ...(input?.onEgressResponse + ? { onEgressResponse: input.onEgressResponse } + : {}), }, }), ]), @@ -390,9 +395,97 @@ describe("sandbox egress proxy integration", () => { expect(upstreamFetch).toHaveBeenCalledTimes(2); }); + it("lets plugin response hooks interrupt egress for auth-required recovery", async () => { + await registerManagedEgressPlugin({ + onEgressResponse() { + throw new EgressAuthRequired("Managed provider needs reauthorization."); + }, + }); + const credentialToken = modules.session.createSandboxEgressCredentialToken({ + credentials: { actor: { type: "user", userId: REQUESTER_ID } }, + egressId: EGRESS_ID, + ttlMs: 60_000, + }); + const networkPolicy = modules.policy.buildSandboxEgressNetworkPolicy({ + credentialToken, + }); + const forwardURL = forwardUrlFor(networkPolicy, MANAGED_PROVIDER_HOST); + const upstreamFetch = vi.fn(async () => new Response("provider response")); + + const response = await modules.proxy.proxySandboxEgressRequest( + proxiedRequest({ + forwardURL, + upstreamHost: MANAGED_PROVIDER_HOST, + upstreamPath: "/v1/issues", + }), + { + fetch: upstreamFetch as typeof fetch, + verifyOidc: async () => ({ sandbox_id: EGRESS_ID }), + }, + ); + + expect(upstreamFetch).toHaveBeenCalledTimes(1); + expect(response.status).toBe(401); + await expect(response.text()).resolves.toContain( + "junior-auth-required provider=managed-egress grant=installation-read access=read", + ); + await expect( + modules.session.consumeSandboxEgressAuthRequiredSignal(EGRESS_ID), + ).resolves.toMatchObject({ + provider: "managed-egress", + grant: { + name: "installation-read", + access: "read", + }, + message: "Managed provider needs reauthorization.", + }); + }); + + it("keeps response hook header mutations isolated from upstream pass-through", async () => { + await registerManagedEgressPlugin({ + onEgressResponse(ctx) { + ctx.response.headers.delete("x-provider-result"); + ctx.response.headers.set("x-plugin-only", "changed"); + }, + }); + const credentialToken = modules.session.createSandboxEgressCredentialToken({ + credentials: { actor: { type: "user", userId: REQUESTER_ID } }, + egressId: EGRESS_ID, + ttlMs: 60_000, + }); + const networkPolicy = modules.policy.buildSandboxEgressNetworkPolicy({ + credentialToken, + }); + const forwardURL = forwardUrlFor(networkPolicy, MANAGED_PROVIDER_HOST); + + const response = await modules.proxy.proxySandboxEgressRequest( + proxiedRequest({ + forwardURL, + upstreamHost: MANAGED_PROVIDER_HOST, + upstreamPath: "/v1/issues", + }), + { + fetch: vi.fn( + async () => + new Response("provider response", { + headers: { + "x-provider-result": "kept", + }, + }), + ) as typeof fetch, + verifyOidc: async () => ({ sandbox_id: EGRESS_ID }), + }, + ); + + expect(response.status).toBe(200); + expect(response.headers.get("x-provider-result")).toBe("kept"); + expect(response.headers.get("x-plugin-only")).toBeNull(); + await expect(response.text()).resolves.toBe("provider response"); + }); + it("uses GitHub App credentials for GraphQL issue list queries", async () => { configureGitHubAppEnv(); - const tokenRequests = mockGitHubInstallationToken(); + mockGitHubInstallationToken(); await registerGitHubPlugin({ appPermissions: { contents: "read", @@ -434,15 +527,138 @@ describe("sandbox egress proxy integration", () => { expect(response.status).toBe(200); await expect(response.text()).resolves.toBe("Bearer installation-token"); expect(upstreamFetch).toHaveBeenCalledTimes(1); - expect(tokenRequests).toEqual([ + }); + + it("records GitHub GraphQL repository access errors without rewriting the response", async () => { + configureGitHubAppEnv(); + mockGitHubInstallationToken(); + await registerGitHubPlugin({ + appPermissions: { + contents: "read", + issues: "write", + }, + }); + const credentialToken = modules.session.createSandboxEgressCredentialToken({ + credentials: { actor: { type: "user", userId: REQUESTER_ID } }, + egressId: EGRESS_ID, + ttlMs: 60_000, + }); + const networkPolicy = modules.policy.buildSandboxEgressNetworkPolicy({ + credentialToken, + }); + const forwardURL = forwardUrlFor(networkPolicy, GITHUB_API_HOST); + const graphqlBody = { + data: { + repository: null, + }, + errors: [ + { + type: "NOT_FOUND", + path: ["repository"], + message: + "Could not resolve to a Repository with the name 'getsentry/junior-prod'.", + }, + ], + }; + const upstreamFetch = vi.fn( + async (_url: URL | string, init?: RequestInit) => { + expect(new Headers(init?.headers).get("authorization")).toBe( + "Bearer installation-token", + ); + return HttpResponse.json(graphqlBody); + }, + ); + + const response = await modules.proxy.proxySandboxEgressRequest( + proxiedRequest({ + body: JSON.stringify({ + query: + "query IssueList($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { issues(first: 1) { nodes { number } } } }", + variables: { owner: "getsentry", name: "junior-prod" }, + }), + forwardURL, + method: "POST", + upstreamHost: GITHUB_API_HOST, + upstreamPath: "/graphql", + }), { - permissions: { - contents: "read", - issues: "read", - metadata: "read", + fetch: upstreamFetch as typeof fetch, + verifyOidc: async () => ({ sandbox_id: EGRESS_ID }), + }, + ); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toEqual(graphqlBody); + await expect( + modules.session.consumeSandboxEgressPermissionDeniedSignal(EGRESS_ID), + ).resolves.toMatchObject({ + provider: "github", + grant: { + name: "installation-read", + access: "read", + }, + message: + "GitHub GraphQL could not access the repository: Could not resolve to a Repository with the name 'getsentry/junior-prod'.", + source: "upstream", + status: 200, + upstreamHost: GITHUB_API_HOST, + upstreamPath: "/graphql", + }); + }); + + it("passes through successful GitHub GraphQL responses without permission signals", async () => { + configureGitHubAppEnv(); + mockGitHubInstallationToken(); + await registerGitHubPlugin({ + appPermissions: { + contents: "read", + issues: "write", + }, + }); + const credentialToken = modules.session.createSandboxEgressCredentialToken({ + credentials: { actor: { type: "user", userId: REQUESTER_ID } }, + egressId: EGRESS_ID, + ttlMs: 60_000, + }); + const networkPolicy = modules.policy.buildSandboxEgressNetworkPolicy({ + credentialToken, + }); + const forwardURL = forwardUrlFor(networkPolicy, GITHUB_API_HOST); + const graphqlBody = { + data: { + repository: { + issues: { + nodes: [], + }, }, }, - ]); + }; + + const response = await modules.proxy.proxySandboxEgressRequest( + proxiedRequest({ + body: JSON.stringify({ + query: + "query IssueList($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { issues(first: 1) { nodes { number } } } }", + variables: { owner: "getsentry", name: "junior-prod" }, + }), + forwardURL, + method: "POST", + upstreamHost: GITHUB_API_HOST, + upstreamPath: "/graphql", + }), + { + fetch: vi.fn(async () => + HttpResponse.json(graphqlBody), + ) as typeof fetch, + verifyOidc: async () => ({ sandbox_id: EGRESS_ID }), + }, + ); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toEqual(graphqlBody); + await expect( + modules.session.consumeSandboxEgressPermissionDeniedSignal(EGRESS_ID), + ).resolves.toBeUndefined(); }); it("keeps GraphQL mutations on GitHub user-write credentials", async () => { diff --git a/packages/junior/tests/unit/plugins/github-plugin.test.ts b/packages/junior/tests/unit/plugins/github-plugin.test.ts index 3bc0a233a..690290fc8 100644 --- a/packages/junior/tests/unit/plugins/github-plugin.test.ts +++ b/packages/junior/tests/unit/plugins/github-plugin.test.ts @@ -444,7 +444,7 @@ describe("github plugin", () => { }); }); - it("keeps GitHub GraphQL mutations, subscriptions, and unparseable bodies on user-write", async () => { + it("keeps GitHub GraphQL mutations and unparseable bodies on user-write", async () => { await expect( grantForEgress({ method: "POST", @@ -462,19 +462,6 @@ describe("github plugin", () => { access: "write", reason: "github.graphql-write", }); - await expect( - grantForEgress({ - method: "POST", - url: "https://api.github.com/graphql", - bodyText: JSON.stringify({ - query: "subscription IssueEvents { issueEvents { id } }", - }), - }), - ).resolves.toMatchObject({ - name: "user-write", - access: "write", - reason: "github.graphql-write", - }); await expect( grantForEgress({ method: "POST", diff --git a/specs/credential-injection.md b/specs/credential-injection.md index 70b908fc1..b2497bb67 100644 --- a/specs/credential-injection.md +++ b/specs/credential-injection.md @@ -3,7 +3,7 @@ ## Metadata - Created: 2026-02-26 -- Last Edited: 2026-06-05 +- Last Edited: 2026-06-08 ## Related @@ -59,6 +59,10 @@ Define how Junior maps registered plugin provider domains to host-managed creden - The proxy must not use method/URL/body-only replay fingerprints as an authorization boundary because duplicate request shapes can be legitimate client retries. - The proxy must strip hop-by-hop and proxy-control headers before sending the upstream request. - Sandbox-supplied request headers and upstream response state may pass through once Vercel OIDC, credential context, and provider-domain ownership have been verified. +- Provider-owned egress response hooks may inspect upstream response metadata after a credentialed request is forwarded. The proxy must not read upstream response bodies by default. +- Response hooks may inspect a body only through the proxy's bounded lazy reader, which clones the response, enforces a hard byte cap, and leaves the original upstream response body available for pass-through. +- A response hook that returns normally must preserve the original upstream response. Throwing `EgressAuthRequired` is the only response-hook outcome that rewrites the upstream response to Junior's auth-required sentinel. +- A response hook may record a provider permission denial as a host-side signal while still preserving the original upstream response. ### Security goals @@ -95,6 +99,7 @@ Define how Junior maps registered plugin provider domains to host-managed creden - Provider `401` responses discard the cached sandbox egress lease so the next request re-issues from current provider state. - When an upstream `401` is received for a request where Junior injected a provider credential, the proxy replaces the raw provider response body with the command-readable `junior-auth-required provider= grant= access= 401 unauthorized` sentinel and records a host-side auth-required signal for the active sandbox egress session. Plugin auth orchestration must trust only the host-side signal for provider grant requirements; raw command stdout/stderr is attacker-influenceable and must not prove GitHub write access or trigger user-token unlink. - Upstream `403` responses are permission denials for an issued lease, not missing authorization. They pass through raw, clear the cached lease, and record a host-side `permission_denied` signal on failed bash results with `source: "upstream"`, a message that states the request was forwarded, provider, grant, upstream target, connected provider account when known, plugin-declared requirements when known, and provider permission headers such as GitHub's `X-Accepted-GitHub-Permissions` when present. +- The GitHub plugin may also record `permission_denied` for GitHub GraphQL HTTP `200` responses whose JSON `errors[]` carry known access-denial semantics, such as repository `NOT_FOUND` or `Resource not accessible by integration`. These responses must pass through unchanged, and the signal status must preserve the real upstream HTTP status. - GitHub `user-read` and `user-write` grants require a stored GitHub user-to-server OAuth token. Missing or stale user authorization returns the auth-required sentinel with the selected grant and access, which starts the private OAuth flow and resumes after authorization. - For GitHub App user-to-server tokens, an empty provider `scope` response is treated as unreported scope information. Junior persists the requested scope string so future broker checks can detect local reauthorization-contract changes. Provider authorization is enforced by GitHub permissions and upstream `401`/`403` responses, not Junior scope checks. - GitHub `installation-read` grants continue to use GitHub App installation tokens. Read-grant GitHub credential failures are operational app/installation failures and must not trigger user OAuth. @@ -120,6 +125,8 @@ Emit events without secret material: - `sandbox_egress_credential_needed` - `sandbox_egress_credential_unavailable` - `sandbox_egress_upstream_auth_rejected` +- `sandbox_egress_upstream_auth_required_classified` +- `sandbox_egress_upstream_permission_classified` ## Non-goals From 6e61b5c5102c31b56aab903a588c0488ea06d17d Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 8 Jun 2026 00:22:45 -0700 Subject: [PATCH 3/4] fix(github): Ignore GraphQL operation words in strings Mask GraphQL string literals before classifying request operations so read queries with search text like mutation or subscription stay on installation-read credentials. Co-Authored-By: GPT-5 Codex --- packages/junior-github/index.js | 15 ++++++++++++--- .../tests/unit/plugins/github-plugin.test.ts | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/junior-github/index.js b/packages/junior-github/index.js index 98f06a55a..1c808e24d 100644 --- a/packages/junior-github/index.js +++ b/packages/junior-github/index.js @@ -719,16 +719,25 @@ function parseGitHubGraphqlOperation(bodyText) { if (typeof query !== "string") { return undefined; } - const normalized = query.replace(/^\s*#[^\n\r]*(?:\r?\n|$)/gm, "").trim(); - if (/\b(mutation|subscription)\b/.test(normalized)) { + const normalized = maskGraphqlStringLiterals( + query.replace(/^\s*#[^\n\r]*(?:\r?\n|$)/gm, ""), + ).trim(); + const operation = normalized.match(/\b(query|mutation|subscription)\b/)?.[1]; + if (operation === "mutation" || operation === "subscription") { return "write"; } - if (normalized.startsWith("{") || /\bquery\b/.test(normalized)) { + if (normalized.startsWith("{") || operation === "query") { return "read"; } return undefined; } +function maskGraphqlStringLiterals(query) { + return query.replace(/"""[\s\S]*?"""|"(?:\\.|[^"\\])*"/g, (match) => + " ".repeat(match.length), + ); +} + function githubGraphqlAccess(method, upstreamUrl, bodyText) { if (!isGitHubGraphqlUrl(upstreamUrl)) { return undefined; diff --git a/packages/junior/tests/unit/plugins/github-plugin.test.ts b/packages/junior/tests/unit/plugins/github-plugin.test.ts index 690290fc8..47cb69b91 100644 --- a/packages/junior/tests/unit/plugins/github-plugin.test.ts +++ b/packages/junior/tests/unit/plugins/github-plugin.test.ts @@ -442,6 +442,20 @@ describe("github plugin", () => { access: "read", reason: "github.graphql-read", }); + expect( + await grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: JSON.stringify({ + query: + 'query SearchIssues { search(query: "mutation subscription", type: ISSUE, first: 1) { nodes { ... on Issue { number } } } }', + }), + }), + ).toMatchObject({ + name: "installation-read", + access: "read", + reason: "github.graphql-read", + }); }); it("keeps GitHub GraphQL mutations and unparseable bodies on user-write", async () => { From 216d9302c838b1ddaf4e1d3ee49125a0ea29272c Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 8 Jun 2026 05:13:50 -0700 Subject: [PATCH 4/4] fix(github): Respect GraphQL operationName for egress Classify the named GraphQL operation when operationName is provided so multi-operation documents cannot route a selected mutation through installation-read credentials. Co-Authored-By: GPT-5 Codex --- packages/junior-github/index.js | 29 +++++++++++++++++- .../tests/unit/plugins/github-plugin.test.ts | 30 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/packages/junior-github/index.js b/packages/junior-github/index.js index 1c808e24d..0b815bd15 100644 --- a/packages/junior-github/index.js +++ b/packages/junior-github/index.js @@ -719,14 +719,41 @@ function parseGitHubGraphqlOperation(bodyText) { if (typeof query !== "string") { return undefined; } + const operationName = + typeof parsed.operationName === "string" + ? parsed.operationName.trim() + : undefined; const normalized = maskGraphqlStringLiterals( query.replace(/^\s*#[^\n\r]*(?:\r?\n|$)/gm, ""), ).trim(); + if (operationName) { + const namedOperation = normalized.match( + new RegExp( + `\\b(query|mutation|subscription)\\s+${escapeRegExp(operationName)}\\b`, + ), + )?.[1]; + return namedOperation ? graphqlOperationAccess(namedOperation) : undefined; + } const operation = normalized.match(/\b(query|mutation|subscription)\b/)?.[1]; + const operationAccess = graphqlOperationAccess(operation); + if (operationAccess) { + return operationAccess; + } + if (normalized.startsWith("{")) { + return "read"; + } + return undefined; +} + +function escapeRegExp(value) { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +function graphqlOperationAccess(operation) { if (operation === "mutation" || operation === "subscription") { return "write"; } - if (normalized.startsWith("{") || operation === "query") { + if (operation === "query") { return "read"; } return undefined; diff --git a/packages/junior/tests/unit/plugins/github-plugin.test.ts b/packages/junior/tests/unit/plugins/github-plugin.test.ts index 47cb69b91..da46b9de0 100644 --- a/packages/junior/tests/unit/plugins/github-plugin.test.ts +++ b/packages/junior/tests/unit/plugins/github-plugin.test.ts @@ -456,6 +456,21 @@ describe("github plugin", () => { access: "read", reason: "github.graphql-read", }); + expect( + await grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: JSON.stringify({ + operationName: "ReadIssues", + query: + 'query ReadIssues { repository(owner: "getsentry", name: "junior-prod") { issues(first: 1) { nodes { number } } } } mutation CreateIssue { createIssue(input: {repositoryId: "repo", title: "test"}) { issue { number } } }', + }), + }), + ).toMatchObject({ + name: "installation-read", + access: "read", + reason: "github.graphql-read", + }); }); it("keeps GitHub GraphQL mutations and unparseable bodies on user-write", async () => { @@ -476,6 +491,21 @@ describe("github plugin", () => { access: "write", reason: "github.graphql-write", }); + await expect( + grantForEgress({ + method: "POST", + url: "https://api.github.com/graphql", + bodyText: JSON.stringify({ + operationName: "CreateIssue", + query: + 'query ReadIssues { repository(owner: "getsentry", name: "junior-prod") { issues(first: 1) { nodes { number } } } } mutation CreateIssue { createIssue(input: {repositoryId: "repo", title: "test"}) { issue { number } } }', + }), + }), + ).resolves.toMatchObject({ + name: "user-write", + access: "write", + reason: "github.graphql-write", + }); await expect( grantForEgress({ method: "POST",