From 2076e0a877d077f3849a6ccf2fdec04f164009d0 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:29:34 +0000 Subject: [PATCH 1/3] refactor(github): Table-drive tool-owned writes Co-Authored-By: David Cramer --- packages/junior-github/src/plugin.ts | 207 ++++++++---------- .../junior-github/tests/github-plugin.test.ts | 10 +- .../junior-plugin-api/src/egress-policy.ts | 26 +++ packages/junior-plugin-api/src/index.ts | 1 + 4 files changed, 123 insertions(+), 121 deletions(-) create mode 100644 packages/junior-plugin-api/src/egress-policy.ts diff --git a/packages/junior-github/src/plugin.ts b/packages/junior-github/src/plugin.ts index 2e964e2e50..c16b7ccd44 100644 --- a/packages/junior-github/src/plugin.ts +++ b/packages/junior-github/src/plugin.ts @@ -6,6 +6,7 @@ import { defineJuniorPlugin, EgressPolicyDenied, + enforceToolOwnedEgress, type EgressHookContext, type EgressResponseHookContext, type PluginGrantAccess, @@ -332,6 +333,69 @@ function shouldInspectGitHubGraphqlResponse( return contentType ? /\bjson\b/i.test(contentType) : false; } +type GitHubToolOwnedWriteRule = { + graphqlField: + | "createIssue" + | "createPullRequest" + | "updateIssue" + | "updatePullRequest"; + grantName: "installation-write"; + message: string; + method: "PATCH" | "POST"; + operation: + | "github.issue.create" + | "github.issue.update" + | "github.pull.create" + | "github.pull.update"; + restPath: RegExp; +}; + +const GITHUB_TOOL_OWNED_WRITE_RULES = [ + { + graphqlField: "createIssue", + grantName: "installation-write", + message: `GitHub issue creation must use the github_createIssue tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + method: "POST", + operation: "github.issue.create", + restPath: /^\/repos\/[^/]+\/[^/]+\/issues$/, + }, + { + graphqlField: "createPullRequest", + grantName: "installation-write", + message: `GitHub pull request creation must use the github_createPullRequest tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + method: "POST", + operation: "github.pull.create", + restPath: /^\/repos\/[^/]+\/[^/]+\/pulls$/, + }, + { + graphqlField: "updateIssue", + grantName: "installation-write", + message: `GitHub issue updates must use the github_updateIssue tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + method: "PATCH", + operation: "github.issue.update", + restPath: /^\/repos\/[^/]+\/[^/]+\/issues\/[^/]+$/, + }, + { + graphqlField: "updatePullRequest", + grantName: "installation-write", + message: `GitHub pull request updates must use the github_updatePullRequest tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + method: "PATCH", + operation: "github.pull.update", + restPath: /^\/repos\/[^/]+\/[^/]+\/pulls\/[^/]+$/, + }, +] as const satisfies readonly GitHubToolOwnedWriteRule[]; + +function githubToolOwnedRestWriteRule( + method: string, + upstreamUrl: URL, +): GitHubToolOwnedWriteRule | undefined { + if (!isGitHubApiUrl(upstreamUrl)) return undefined; + const pathname = upstreamUrl.pathname.toLowerCase(); + return GITHUB_TOOL_OWNED_WRITE_RULES.find( + (rule) => rule.method === method && rule.restPath.test(pathname), + ); +} + function githubApiWriteGrantName( method: string, upstreamUrl: URL, @@ -358,21 +422,14 @@ function githubApiWriteGrantName( // Actions run control uses run-level cancel/rerun and job-level rerun endpoints. return "installation-write"; } - if (method === "POST" && /^\/repos\/[^/]+\/[^/]+\/issues$/.test(pathname)) { - return "installation-write"; - } + const toolOwnedRule = githubToolOwnedRestWriteRule(method, upstreamUrl); + if (toolOwnedRule) return toolOwnedRule.grantName; if ( method === "POST" && /^\/repos\/[^/]+\/[^/]+\/issues\/[^/]+\/comments$/.test(pathname) ) { return "installation-write"; } - if ( - method === "PATCH" && - /^\/repos\/[^/]+\/[^/]+\/issues\/[^/]+$/.test(pathname) - ) { - return "installation-write"; - } if ( (method === "POST" || method === "DELETE") && /^\/repos\/[^/]+\/[^/]+\/issues\/[^/]+\/(labels|assignees)(?:\/[^/]+)?$/.test( @@ -381,15 +438,6 @@ function githubApiWriteGrantName( ) { return "installation-write"; } - if (method === "POST" && /^\/repos\/[^/]+\/[^/]+\/pulls$/.test(pathname)) { - return "installation-write"; - } - if ( - method === "PATCH" && - /^\/repos\/[^/]+\/[^/]+\/pulls\/[^/]+$/.test(pathname) - ) { - return "installation-write"; - } if ( method === "POST" && /^\/repos\/[^/]+\/[^/]+\/pulls\/[^/]+\/ready_for_review$/.test(pathname) @@ -431,47 +479,15 @@ function githubApiWriteGrantName( return undefined; } -function isGitHubIssueCreateRestRequest( - method: string, - upstreamUrl: URL, -): boolean { - return ( - method === "POST" && - isGitHubApiUrl(upstreamUrl) && - /^\/repos\/[^/]+\/[^/]+\/issues$/.test(upstreamUrl.pathname.toLowerCase()) - ); -} - -function isGitHubPullCreateRestRequest( - method: string, - upstreamUrl: URL, -): boolean { - return ( - method === "POST" && - isGitHubApiUrl(upstreamUrl) && - /^\/repos\/[^/]+\/[^/]+\/pulls$/.test(upstreamUrl.pathname.toLowerCase()) - ); -} - -function isGitHubResourceUpdateRestRequest( - method: string, - upstreamUrl: URL, - resource: "issues" | "pulls", -): boolean { - return ( - method === "PATCH" && - isGitHubApiUrl(upstreamUrl) && - new RegExp(`^/repos/[^/]+/[^/]+/${resource}/[^/]+$`).test( - upstreamUrl.pathname.toLowerCase(), - ) - ); -} - function isGitHubGraphqlMutation( method: string, upstreamUrl: URL, bodyText: string | undefined, - field: "createIssue" | "createPullRequest" | "updateIssue" | "updatePullRequest", + field: + | "createIssue" + | "createPullRequest" + | "updateIssue" + | "updatePullRequest", ): boolean { if (method !== "POST" || !isGitHubGraphqlUrl(upstreamUrl)) return false; const parsed = parseGitHubGraphqlRequest(bodyText); @@ -495,70 +511,23 @@ function assertGitHubWriteAllowed(input: { method: input.method, upstreamUrl: input.upstreamUrl, }); - if (input.operation === "github.issue.create") return; - if (input.operation === "github.issue.update") return; - if (input.operation === "github.pull.create") return; - if (input.operation === "github.pull.update") return; - if ( - isGitHubIssueCreateRestRequest(input.method, input.upstreamUrl) || - isGitHubGraphqlMutation( - input.method, - input.upstreamUrl, - input.bodyText, - "createIssue", - ) - ) { - throw new EgressPolicyDenied( - `GitHub issue creation must use the github_createIssue tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, - ); - } - if ( - isGitHubPullCreateRestRequest(input.method, input.upstreamUrl) || - isGitHubGraphqlMutation( - input.method, - input.upstreamUrl, - input.bodyText, - "createPullRequest", - ) - ) { - throw new EgressPolicyDenied( - `GitHub pull request creation must use the github_createPullRequest tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, - ); - } - if ( - isGitHubResourceUpdateRestRequest( - input.method, - input.upstreamUrl, - "issues", - ) || - isGitHubGraphqlMutation( - input.method, - input.upstreamUrl, - input.bodyText, - "updateIssue", - ) - ) { - throw new EgressPolicyDenied( - `GitHub issue updates must use the github_updateIssue tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, - ); - } - if ( - isGitHubResourceUpdateRestRequest( - input.method, - input.upstreamUrl, - "pulls", - ) || - isGitHubGraphqlMutation( - input.method, - input.upstreamUrl, - input.bodyText, - "updatePullRequest", - ) - ) { - throw new EgressPolicyDenied( - `GitHub pull request updates must use the github_updatePullRequest tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, - ); - } + enforceToolOwnedEgress({ + matches(rule, request) { + return ( + (rule.method === request.method && + isGitHubApiUrl(request.upstreamUrl) && + rule.restPath.test(request.upstreamUrl.pathname.toLowerCase())) || + isGitHubGraphqlMutation( + request.method, + request.upstreamUrl, + request.bodyText, + rule.graphqlField, + ) + ); + }, + request: input, + rules: GITHUB_TOOL_OWNED_WRITE_RULES, + }); } function grantForAccess( diff --git a/packages/junior-github/tests/github-plugin.test.ts b/packages/junior-github/tests/github-plugin.test.ts index ef51fbf94a..7e67dd9892 100644 --- a/packages/junior-github/tests/github-plugin.test.ts +++ b/packages/junior-github/tests/github-plugin.test.ts @@ -355,8 +355,7 @@ function githubToolsContext(input?: { model: {}, resourceEvents: { canSubscribe: true }, users: { - resolveActor: - input?.resolveActor ?? (async () => undefined), + resolveActor: input?.resolveActor ?? (async () => undefined), }, state: { async delete(key: string) { @@ -1891,6 +1890,13 @@ Conversation: \`local:test:old-conversation\` url: "https://api.github.com/repos/getsentry/junior/issues/780", }), ).rejects.toThrow("must use the github_updateIssue tool"); + await expect( + grantForEgress({ + method: "PATCH", + operation: "github.pull.update", + url: "https://api.github.com/repos/getsentry/junior/issues/780", + }), + ).rejects.toThrow("must use the github_updateIssue tool"); await expect( grantForEgress({ method: "PATCH", diff --git a/packages/junior-plugin-api/src/egress-policy.ts b/packages/junior-plugin-api/src/egress-policy.ts new file mode 100644 index 0000000000..5f8abd76a7 --- /dev/null +++ b/packages/junior-plugin-api/src/egress-policy.ts @@ -0,0 +1,26 @@ +import { EgressPolicyDenied } from "./credentials"; + +/** One plugin-owned egress route that requires a specific operation. */ +export interface ToolOwnedEgressRule { + message: string; + operation: string; +} + +/** Match and enforce one plugin-owned route before credentials are issued. */ +export function enforceToolOwnedEgress< + Request extends { operation?: string }, + Rule extends ToolOwnedEgressRule, +>(input: { + matches(rule: Rule, request: Request): boolean; + request: Request; + rules: readonly Rule[]; +}): Rule | undefined { + const rule = input.rules.find((candidate) => + input.matches(candidate, input.request), + ); + if (!rule) return undefined; + if (input.request.operation !== rule.operation) { + throw new EgressPolicyDenied(rule.message); + } + return rule; +} diff --git a/packages/junior-plugin-api/src/index.ts b/packages/junior-plugin-api/src/index.ts index a76e21bd1b..015c8c70c6 100644 --- a/packages/junior-plugin-api/src/index.ts +++ b/packages/junior-plugin-api/src/index.ts @@ -20,6 +20,7 @@ export * from "./tasks"; export * from "./tools"; export * from "./operations"; export * from "./credentials"; +export * from "./egress-policy"; export * from "./hooks"; export * from "./cli"; export * from "./manifest"; From 9f3ce7abe2a320a9035351712ee6467f75d61819 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:59:21 +0000 Subject: [PATCH 2/3] refactor(plugins): Generalize egress policy API Co-Authored-By: David Cramer --- packages/junior-github/src/plugin.ts | 47 ++++++++++--------- .../junior-plugin-api/src/egress-policy.ts | 20 ++++---- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/packages/junior-github/src/plugin.ts b/packages/junior-github/src/plugin.ts index c16b7ccd44..f1544a9e2b 100644 --- a/packages/junior-github/src/plugin.ts +++ b/packages/junior-github/src/plugin.ts @@ -6,7 +6,7 @@ import { defineJuniorPlugin, EgressPolicyDenied, - enforceToolOwnedEgress, + enforceEgressPolicy, type EgressHookContext, type EgressResponseHookContext, type PluginGrantAccess, @@ -333,14 +333,14 @@ function shouldInspectGitHubGraphqlResponse( return contentType ? /\bjson\b/i.test(contentType) : false; } -type GitHubToolOwnedWriteRule = { +type GitHubBodyWritePolicy = { graphqlField: | "createIssue" | "createPullRequest" | "updateIssue" | "updatePullRequest"; grantName: "installation-write"; - message: string; + denialMessage: string; method: "PATCH" | "POST"; operation: | "github.issue.create" @@ -350,11 +350,11 @@ type GitHubToolOwnedWriteRule = { restPath: RegExp; }; -const GITHUB_TOOL_OWNED_WRITE_RULES = [ +const GITHUB_BODY_WRITE_POLICIES = [ { graphqlField: "createIssue", grantName: "installation-write", - message: `GitHub issue creation must use the github_createIssue tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + denialMessage: `GitHub issue creation must use the github_createIssue tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, method: "POST", operation: "github.issue.create", restPath: /^\/repos\/[^/]+\/[^/]+\/issues$/, @@ -362,7 +362,7 @@ const GITHUB_TOOL_OWNED_WRITE_RULES = [ { graphqlField: "createPullRequest", grantName: "installation-write", - message: `GitHub pull request creation must use the github_createPullRequest tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + denialMessage: `GitHub pull request creation must use the github_createPullRequest tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, method: "POST", operation: "github.pull.create", restPath: /^\/repos\/[^/]+\/[^/]+\/pulls$/, @@ -370,7 +370,7 @@ const GITHUB_TOOL_OWNED_WRITE_RULES = [ { graphqlField: "updateIssue", grantName: "installation-write", - message: `GitHub issue updates must use the github_updateIssue tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + denialMessage: `GitHub issue updates must use the github_updateIssue tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, method: "PATCH", operation: "github.issue.update", restPath: /^\/repos\/[^/]+\/[^/]+\/issues\/[^/]+$/, @@ -378,20 +378,20 @@ const GITHUB_TOOL_OWNED_WRITE_RULES = [ { graphqlField: "updatePullRequest", grantName: "installation-write", - message: `GitHub pull request updates must use the github_updatePullRequest tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + denialMessage: `GitHub pull request updates must use the github_updatePullRequest tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, method: "PATCH", operation: "github.pull.update", restPath: /^\/repos\/[^/]+\/[^/]+\/pulls\/[^/]+$/, }, -] as const satisfies readonly GitHubToolOwnedWriteRule[]; +] as const satisfies readonly GitHubBodyWritePolicy[]; -function githubToolOwnedRestWriteRule( +function githubBodyWritePolicy( method: string, upstreamUrl: URL, -): GitHubToolOwnedWriteRule | undefined { +): GitHubBodyWritePolicy | undefined { if (!isGitHubApiUrl(upstreamUrl)) return undefined; const pathname = upstreamUrl.pathname.toLowerCase(); - return GITHUB_TOOL_OWNED_WRITE_RULES.find( + return GITHUB_BODY_WRITE_POLICIES.find( (rule) => rule.method === method && rule.restPath.test(pathname), ); } @@ -422,8 +422,8 @@ function githubApiWriteGrantName( // Actions run control uses run-level cancel/rerun and job-level rerun endpoints. return "installation-write"; } - const toolOwnedRule = githubToolOwnedRestWriteRule(method, upstreamUrl); - if (toolOwnedRule) return toolOwnedRule.grantName; + const bodyWritePolicy = githubBodyWritePolicy(method, upstreamUrl); + if (bodyWritePolicy) return bodyWritePolicy.grantName; if ( method === "POST" && /^\/repos\/[^/]+\/[^/]+\/issues\/[^/]+\/comments$/.test(pathname) @@ -500,7 +500,7 @@ function isGitHubGraphqlMutation( ).test(parsed.normalized); } -function assertGitHubWriteAllowed(input: { +function applyGitHubEgressPolicy(input: { bodyText?: string; method: string; operation?: string; @@ -511,22 +511,25 @@ function assertGitHubWriteAllowed(input: { method: input.method, upstreamUrl: input.upstreamUrl, }); - enforceToolOwnedEgress({ - matches(rule, request) { + enforceEgressPolicy({ + allows(policy, request) { + return request.operation === policy.operation; + }, + matches(policy, request) { return ( - (rule.method === request.method && + (policy.method === request.method && isGitHubApiUrl(request.upstreamUrl) && - rule.restPath.test(request.upstreamUrl.pathname.toLowerCase())) || + policy.restPath.test(request.upstreamUrl.pathname.toLowerCase())) || isGitHubGraphqlMutation( request.method, request.upstreamUrl, request.bodyText, - rule.graphqlField, + policy.graphqlField, ) ); }, request: input, - rules: GITHUB_TOOL_OWNED_WRITE_RULES, + rules: GITHUB_BODY_WRITE_POLICIES, }); } @@ -560,7 +563,7 @@ async function githubGrantForEgress( ): Promise { const method = ctx.request.method.toUpperCase(); const upstreamUrl = new URL(ctx.request.url); - assertGitHubWriteAllowed({ + applyGitHubEgressPolicy({ ...(ctx.request.bodyText !== undefined ? { bodyText: ctx.request.bodyText } : {}), diff --git a/packages/junior-plugin-api/src/egress-policy.ts b/packages/junior-plugin-api/src/egress-policy.ts index 5f8abd76a7..0dc206e21a 100644 --- a/packages/junior-plugin-api/src/egress-policy.ts +++ b/packages/junior-plugin-api/src/egress-policy.ts @@ -1,16 +1,16 @@ import { EgressPolicyDenied } from "./credentials"; -/** One plugin-owned egress route that requires a specific operation. */ -export interface ToolOwnedEgressRule { - message: string; - operation: string; +/** One plugin-defined egress rule and its denial message. */ +export interface EgressPolicyRule { + denialMessage: string; } -/** Match and enforce one plugin-owned route before credentials are issued. */ -export function enforceToolOwnedEgress< - Request extends { operation?: string }, - Rule extends ToolOwnedEgressRule, +/** Apply the first plugin-defined rule that matches an egress request. */ +export function enforceEgressPolicy< + Request, + Rule extends EgressPolicyRule, >(input: { + allows(rule: Rule, request: Request): boolean; matches(rule: Rule, request: Request): boolean; request: Request; rules: readonly Rule[]; @@ -19,8 +19,8 @@ export function enforceToolOwnedEgress< input.matches(candidate, input.request), ); if (!rule) return undefined; - if (input.request.operation !== rule.operation) { - throw new EgressPolicyDenied(rule.message); + if (!input.allows(rule, input.request)) { + throw new EgressPolicyDenied(rule.denialMessage); } return rule; } From f2dcf742b98609b999ae3499111188ed61c0fde7 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 13 Aug 2026 02:04:12 +0000 Subject: [PATCH 3/3] refactor(plugins): Keep egress policy API thin Co-Authored-By: David Cramer --- packages/junior-github/src/plugin.ts | 78 ++++++++----------- .../junior-plugin-api/src/egress-policy.ts | 31 +++----- 2 files changed, 44 insertions(+), 65 deletions(-) diff --git a/packages/junior-github/src/plugin.ts b/packages/junior-github/src/plugin.ts index f1544a9e2b..1f4a774dda 100644 --- a/packages/junior-github/src/plugin.ts +++ b/packages/junior-github/src/plugin.ts @@ -333,14 +333,14 @@ function shouldInspectGitHubGraphqlResponse( return contentType ? /\bjson\b/i.test(contentType) : false; } -type GitHubBodyWritePolicy = { +/** GitHub body writes that must go through a typed tool. */ +type GitHubBodyWrite = { + denialMessage: string; graphqlField: | "createIssue" | "createPullRequest" | "updateIssue" | "updatePullRequest"; - grantName: "installation-write"; - denialMessage: string; method: "PATCH" | "POST"; operation: | "github.issue.create" @@ -350,51 +350,36 @@ type GitHubBodyWritePolicy = { restPath: RegExp; }; -const GITHUB_BODY_WRITE_POLICIES = [ +const GITHUB_BODY_WRITES = [ { - graphqlField: "createIssue", - grantName: "installation-write", denialMessage: `GitHub issue creation must use the github_createIssue tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + graphqlField: "createIssue", method: "POST", operation: "github.issue.create", restPath: /^\/repos\/[^/]+\/[^/]+\/issues$/, }, { - graphqlField: "createPullRequest", - grantName: "installation-write", denialMessage: `GitHub pull request creation must use the github_createPullRequest tool so Junior can own idempotency and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + graphqlField: "createPullRequest", method: "POST", operation: "github.pull.create", restPath: /^\/repos\/[^/]+\/[^/]+\/pulls$/, }, { - graphqlField: "updateIssue", - grantName: "installation-write", denialMessage: `GitHub issue updates must use the github_updateIssue tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + graphqlField: "updateIssue", method: "PATCH", operation: "github.issue.update", restPath: /^\/repos\/[^/]+\/[^/]+\/issues\/[^/]+$/, }, { - graphqlField: "updatePullRequest", - grantName: "installation-write", denialMessage: `GitHub pull request updates must use the github_updatePullRequest tool so Junior can own requester attribution and the conversation footer. ${CREATE_TOOL_ROUTING_GUIDANCE}`, + graphqlField: "updatePullRequest", method: "PATCH", operation: "github.pull.update", restPath: /^\/repos\/[^/]+\/[^/]+\/pulls\/[^/]+$/, }, -] as const satisfies readonly GitHubBodyWritePolicy[]; - -function githubBodyWritePolicy( - method: string, - upstreamUrl: URL, -): GitHubBodyWritePolicy | undefined { - if (!isGitHubApiUrl(upstreamUrl)) return undefined; - const pathname = upstreamUrl.pathname.toLowerCase(); - return GITHUB_BODY_WRITE_POLICIES.find( - (rule) => rule.method === method && rule.restPath.test(pathname), - ); -} +] as const satisfies readonly GitHubBodyWrite[]; function githubApiWriteGrantName( method: string, @@ -422,8 +407,13 @@ function githubApiWriteGrantName( // Actions run control uses run-level cancel/rerun and job-level rerun endpoints. return "installation-write"; } - const bodyWritePolicy = githubBodyWritePolicy(method, upstreamUrl); - if (bodyWritePolicy) return bodyWritePolicy.grantName; + if ( + GITHUB_BODY_WRITES.some( + (write) => write.method === method && write.restPath.test(pathname), + ) + ) { + return "installation-write"; + } if ( method === "POST" && /^\/repos\/[^/]+\/[^/]+\/issues\/[^/]+\/comments$/.test(pathname) @@ -511,25 +501,25 @@ function applyGitHubEgressPolicy(input: { method: input.method, upstreamUrl: input.upstreamUrl, }); + + const pathname = input.upstreamUrl.pathname.toLowerCase(); + const write = GITHUB_BODY_WRITES.find( + (candidate) => + (candidate.method === input.method && + isGitHubApiUrl(input.upstreamUrl) && + candidate.restPath.test(pathname)) || + isGitHubGraphqlMutation( + input.method, + input.upstreamUrl, + input.bodyText, + candidate.graphqlField, + ), + ); + if (!write) return; + enforceEgressPolicy({ - allows(policy, request) { - return request.operation === policy.operation; - }, - matches(policy, request) { - return ( - (policy.method === request.method && - isGitHubApiUrl(request.upstreamUrl) && - policy.restPath.test(request.upstreamUrl.pathname.toLowerCase())) || - isGitHubGraphqlMutation( - request.method, - request.upstreamUrl, - request.bodyText, - policy.graphqlField, - ) - ); - }, - request: input, - rules: GITHUB_BODY_WRITE_POLICIES, + allowed: input.operation === write.operation, + denialMessage: write.denialMessage, }); } diff --git a/packages/junior-plugin-api/src/egress-policy.ts b/packages/junior-plugin-api/src/egress-policy.ts index 0dc206e21a..9e8de37fc4 100644 --- a/packages/junior-plugin-api/src/egress-policy.ts +++ b/packages/junior-plugin-api/src/egress-policy.ts @@ -1,26 +1,15 @@ import { EgressPolicyDenied } from "./credentials"; -/** One plugin-defined egress rule and its denial message. */ -export interface EgressPolicyRule { +/** + * Deny provider egress when a plugin policy does not allow the request. + * + * Call from a plugin `grantForEgress` hook before returning a write grant. + */ +export function enforceEgressPolicy(input: { + allowed: boolean; denialMessage: string; -} - -/** Apply the first plugin-defined rule that matches an egress request. */ -export function enforceEgressPolicy< - Request, - Rule extends EgressPolicyRule, ->(input: { - allows(rule: Rule, request: Request): boolean; - matches(rule: Rule, request: Request): boolean; - request: Request; - rules: readonly Rule[]; -}): Rule | undefined { - const rule = input.rules.find((candidate) => - input.matches(candidate, input.request), - ); - if (!rule) return undefined; - if (!input.allows(rule, input.request)) { - throw new EgressPolicyDenied(rule.denialMessage); +}): void { + if (!input.allowed) { + throw new EgressPolicyDenied(input.denialMessage); } - return rule; }