From ef160c975cb5816c98cf2292b6756e4ec96e9de1 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:48:06 +0000 Subject: [PATCH 1/2] feat(github): allow release-asset writes for image attachments Adds a scoped installation write grant for GitHub Releases (release create and release-asset upload only; release/asset edit, delete, and asset overwrite stay denied by the default fallthrough) so the github-issues and github-code skills can embed images inline via a shared, append-only _attachments release instead of ephemeral Slack file links. Refs getsentry/junior#101 Co-Authored-By: immutable dcramer --- packages/junior-github/SETUP.md | 1 + .../junior-github/skills/github-code/SKILL.md | 3 + .../references/image-attachments.md | 50 +++++++++ .../skills/github-issues/SKILL.md | 3 +- .../references/image-attachments.md | 50 +++++++++ packages/junior-github/src/index.ts | 75 ++++++++++++- .../junior-github/tests/github-plugin.test.ts | 105 ++++++++++++++++++ 7 files changed, 283 insertions(+), 4 deletions(-) create mode 100644 packages/junior-github/skills/github-code/references/image-attachments.md create mode 100644 packages/junior-github/skills/github-issues/references/image-attachments.md diff --git a/packages/junior-github/SETUP.md b/packages/junior-github/SETUP.md index df6517c2b..215a86da9 100644 --- a/packages/junior-github/SETUP.md +++ b/packages/junior-github/SETUP.md @@ -137,6 +137,7 @@ Committing and pushing code uses more than one GitHub surface: - The smart-HTTP classifier does not distinguish Junior-managed branches or independently detect force updates or ref deletion. Use GitHub branch protection and limit the App installation to repositories where Junior may push. - REST Git database and ref writes are denied by the current write allowlist. Use Git smart HTTP (`git push`) for branch updates instead. - Opening the PR after the branch exists is separate: `github_createPullRequest` needs pull-request write permission, but it should not create or push commits itself. +- Release create and release-asset upload (`uploads.github.com`) are allowlisted under the same `Contents: write` permission, scoped to the target repository. This backs the image-attachment technique used by the `github-issues` and `github-code` skills. Release edit/delete and asset delete/overwrite are not enabled; the workflow is append-only by policy. Fork creation is not part of the default PR path and is denied by the current write allowlist. Do not grant `Administration: write` for routine PR creation; push a branch explicitly and create the PR with `github_createPullRequest` instead. diff --git a/packages/junior-github/skills/github-code/SKILL.md b/packages/junior-github/skills/github-code/SKILL.md index a1981bdc7..e9e501a0a 100644 --- a/packages/junior-github/skills/github-code/SKILL.md +++ b/packages/junior-github/skills/github-code/SKILL.md @@ -12,6 +12,7 @@ Use `gh` and `git` for repository checkout, source investigation, code changes, | Need | Load | | ----------------------------------- | -------------------------------------------------------------------------------------- | | Command syntax, permissions, config | [references/api-surface.md](references/api-surface.md) | +| Embedding an image in a PR/comment | [references/image-attachments.md](references/image-attachments.md) | | Failed commands, permission errors | [references/troubleshooting-workarounds.md](references/troubleshooting-workarounds.md) | ## Core rules @@ -163,6 +164,8 @@ Before finishing, reconcile any plan or checklist stated earlier — mark items **PR mutation** — push before create. Use only the allowlisted REST endpoints in the API reference. Merge, close-with-delete, REST ref mutation, and admin operations are unsupported. Git smart HTTP does not independently prevent force updates or ref deletion; rely on GitHub rulesets and do not request destructive pushes. +**Embedding an image in a PR body/comment** — see [references/image-attachments.md](references/image-attachments.md) for the GitHub Release asset technique. + **Workflow dispatch** — `gh workflow run` is supported for workflows that declare `workflow_dispatch`. Workflow reruns, cancellations, and other Actions mutations remain unsupported. ## Guardrails diff --git a/packages/junior-github/skills/github-code/references/image-attachments.md b/packages/junior-github/skills/github-code/references/image-attachments.md new file mode 100644 index 000000000..dd23d9f30 --- /dev/null +++ b/packages/junior-github/skills/github-code/references/image-attachments.md @@ -0,0 +1,50 @@ +# Image attachments via GitHub Release assets + +Embed an image inline in an issue, PR, or comment body using only `gh` — no third-party image host, no extra token. + +## Mechanism + +GitHub serves release-asset download URLs directly to the browser (no camo proxy for `github.com` URLs). Upload the image as an asset on a shared, append-only release, then reference its stable download URL as `![]()` markdown. + +Visibility follows the host repo: a private host repo renders the image for readers with repo access and 404s for everyone else; a public host repo is world-visible. Always host in the repo the issue/PR/comment lives in so visibility matches the audience — never stage a private screenshot through a public repo. + +## Procedure + +Given an image at `$IMG` and target repo `$REPO` (`owner/repo`, resolved the same way as any other operation in this skill): + +```bash +TAG="_attachments" # one shared, append-only release per repo for all attachments + +# 1. Ensure the attachments release exists (idempotent). Create it WITHOUT +# passing files — passing files to `gh release create` also publishes the +# release via a PATCH that Junior's write allowlist denies. +gh release view "$TAG" --repo "$REPO" >/dev/null 2>&1 || \ + gh release create "$TAG" --repo "$REPO" --title attachments --notes "image attachments" --latest=false + +# 2. Upload with a collision-proof name (hash/random suffix avoids overwriting +# an already-linked image). Never use `--clobber` — asset overwrite/delete +# is not part of this workflow. +NAME="$(basename "${IMG%.*}")-$(head -c4 /dev/urandom | xxd -p).${IMG##*.}" +gh release upload "$TAG" "$IMG#$NAME" --repo "$REPO" + +# 3. Build the URL and embed it. +URL="https://github.com/$REPO/releases/download/$TAG/$NAME" +echo "![${NAME}]($URL)" +``` + +Then include `![]($URL)` in the issue/PR/comment body via the normal creation or edit path for that operation. + +## Rules + +- **Append-only.** Never delete the `_attachments` release or its assets — the download URL points at live storage, and deleting an asset turns every historical embed into a broken image. +- **Unique filenames, no `--clobber`.** Always suffix with a hash/random string instead of overwriting. Asset delete is not part of this workflow and is denied by Junior's write allowlist. +- **Never pass files directly to `gh release create`.** That publishes the release via a PATCH request, which is denied. Create the (draft, unpublished-is-fine) release first, then upload separately. +- **Match host visibility to audience.** Host in the repo the content lives in so private images stay private. +- **Only use this for images that need to render inline in GitHub markdown** (screenshots, diagrams, charts from a Slack thread or elsewhere). Don't use it as a general-purpose file store. + +## Requirements + +- GitHub App `Contents: write` on the target repository (this covers release create/read/update and asset upload, the same permission repository content pushes already use). +- Junior's GitHub plugin classifies release-create (`POST /repos/{owner}/{repo}/releases`) and asset-upload (`POST` to `uploads.github.com/repos/{owner}/{repo}/releases/{id}/assets`) as an allowed installation write scoped to the target repository, and the plugin manifest lists `uploads.github.com` as an egress domain (`packages/junior-github/src/index.ts`). Release edit/delete and asset delete are intentionally denied — this workflow is append-only by policy, not just by convention. If `gh release create`/`gh release upload` ever fails with an egress/policy denial, that's a regression in this classification — don't retry with a workaround; report the exact failure. + +Source technique: https://gist.github.com/CatalanCabbage/649aae8f9a7b813776b22340b0f07d05 diff --git a/packages/junior-github/skills/github-issues/SKILL.md b/packages/junior-github/skills/github-issues/SKILL.md index f20962bf0..db6daeb78 100644 --- a/packages/junior-github/skills/github-issues/SKILL.md +++ b/packages/junior-github/skills/github-issues/SKILL.md @@ -14,6 +14,7 @@ Use only for GitHub issues. For pull requests, branches, pushes, or PR creation | ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | Any operation | [references/api-surface.md](references/api-surface.md) | | `issue create`, `issue body rewrite` | [references/issue-examples.md](references/issue-examples.md), the matching type-specific guide ([issue-bug.md](references/issue-bug.md), [issue-feature.md](references/issue-feature.md), [issue-task.md](references/issue-task.md)), and [references/research-rules.md](references/research-rules.md) | +| Embedding an image from the thread | [references/image-attachments.md](references/image-attachments.md) | | On failure | [references/troubleshooting-workarounds.md](references/troubleshooting-workarounds.md) | ## Workflow @@ -64,7 +65,7 @@ Follow [references/research-rules.md](references/research-rules.md) for cross-ty - The runtime adds the verified `Requested by` block. Do not add or rewrite requester attribution in model-authored body text. - If the person who originally reported or observed the problem differs from the issue creator, capture that with durable body text such as `Reported by Alice.` or `Raised by Alice during incident triage.` -- Attach screenshots from the thread as image links when present. +- Attach screenshots from the thread as image links when present, using [references/image-attachments.md](references/image-attachments.md) to embed them so they render inline for anyone with repo access. - Include code snippets, related issues, and related PRs only when they materially improve the issue. ### 4. Verify draft diff --git a/packages/junior-github/skills/github-issues/references/image-attachments.md b/packages/junior-github/skills/github-issues/references/image-attachments.md new file mode 100644 index 000000000..dd23d9f30 --- /dev/null +++ b/packages/junior-github/skills/github-issues/references/image-attachments.md @@ -0,0 +1,50 @@ +# Image attachments via GitHub Release assets + +Embed an image inline in an issue, PR, or comment body using only `gh` — no third-party image host, no extra token. + +## Mechanism + +GitHub serves release-asset download URLs directly to the browser (no camo proxy for `github.com` URLs). Upload the image as an asset on a shared, append-only release, then reference its stable download URL as `![]()` markdown. + +Visibility follows the host repo: a private host repo renders the image for readers with repo access and 404s for everyone else; a public host repo is world-visible. Always host in the repo the issue/PR/comment lives in so visibility matches the audience — never stage a private screenshot through a public repo. + +## Procedure + +Given an image at `$IMG` and target repo `$REPO` (`owner/repo`, resolved the same way as any other operation in this skill): + +```bash +TAG="_attachments" # one shared, append-only release per repo for all attachments + +# 1. Ensure the attachments release exists (idempotent). Create it WITHOUT +# passing files — passing files to `gh release create` also publishes the +# release via a PATCH that Junior's write allowlist denies. +gh release view "$TAG" --repo "$REPO" >/dev/null 2>&1 || \ + gh release create "$TAG" --repo "$REPO" --title attachments --notes "image attachments" --latest=false + +# 2. Upload with a collision-proof name (hash/random suffix avoids overwriting +# an already-linked image). Never use `--clobber` — asset overwrite/delete +# is not part of this workflow. +NAME="$(basename "${IMG%.*}")-$(head -c4 /dev/urandom | xxd -p).${IMG##*.}" +gh release upload "$TAG" "$IMG#$NAME" --repo "$REPO" + +# 3. Build the URL and embed it. +URL="https://github.com/$REPO/releases/download/$TAG/$NAME" +echo "![${NAME}]($URL)" +``` + +Then include `![]($URL)` in the issue/PR/comment body via the normal creation or edit path for that operation. + +## Rules + +- **Append-only.** Never delete the `_attachments` release or its assets — the download URL points at live storage, and deleting an asset turns every historical embed into a broken image. +- **Unique filenames, no `--clobber`.** Always suffix with a hash/random string instead of overwriting. Asset delete is not part of this workflow and is denied by Junior's write allowlist. +- **Never pass files directly to `gh release create`.** That publishes the release via a PATCH request, which is denied. Create the (draft, unpublished-is-fine) release first, then upload separately. +- **Match host visibility to audience.** Host in the repo the content lives in so private images stay private. +- **Only use this for images that need to render inline in GitHub markdown** (screenshots, diagrams, charts from a Slack thread or elsewhere). Don't use it as a general-purpose file store. + +## Requirements + +- GitHub App `Contents: write` on the target repository (this covers release create/read/update and asset upload, the same permission repository content pushes already use). +- Junior's GitHub plugin classifies release-create (`POST /repos/{owner}/{repo}/releases`) and asset-upload (`POST` to `uploads.github.com/repos/{owner}/{repo}/releases/{id}/assets`) as an allowed installation write scoped to the target repository, and the plugin manifest lists `uploads.github.com` as an egress domain (`packages/junior-github/src/index.ts`). Release edit/delete and asset delete are intentionally denied — this workflow is append-only by policy, not just by convention. If `gh release create`/`gh release upload` ever fails with an egress/policy denial, that's a regression in this classification — don't retry with a workaround; report the exact failure. + +Source technique: https://gist.github.com/CatalanCabbage/649aae8f9a7b813776b22340b0f07d05 diff --git a/packages/junior-github/src/index.ts b/packages/junior-github/src/index.ts index 7fef72366..c2aeaa0b5 100644 --- a/packages/junior-github/src/index.ts +++ b/packages/junior-github/src/index.ts @@ -82,6 +82,7 @@ type GitHubGrantName = | "installation-pr-branch-write" | "installation-pull-requests-write" | "installation-read" + | "installation-releases-write" | "user-read" | "user-write"; type GitHubGrantReason = @@ -97,6 +98,7 @@ type GitHubGrantReason = | "github.pull-create" | "github.pull-review-write" | "github.pull-requests-write" + | "github.releases-write" | "github.user-read" | "github.workflows-write"; type GitHubGrant = PluginGrant & { @@ -190,6 +192,11 @@ const PULL_REVIEW_WRITE_REQUIREMENTS = [ ...PULL_REQUESTS_WRITE_REQUIREMENTS, "requesting GitHub user permission to review the pull request", ]; +// GitHub Apps expose release create/read/update and asset upload/delete under +// the same "Contents" permission used for repository file contents. +const RELEASES_WRITE_REQUIREMENTS = [ + "GitHub App Contents: write on the target repository", +]; class GitHubUserRefreshRejectedError extends Error { constructor(message: string) { @@ -770,7 +777,11 @@ function createCredentialLease( ...(input.account ? { account: input.account } : {}), ...(input.authorization ? { authorization: input.authorization } : {}), expiresAt: new Date(input.expiresAtMs).toISOString(), - headerTransforms: ["api.github.com", "github.com"].map((domain) => ({ + headerTransforms: [ + "api.github.com", + "github.com", + "uploads.github.com", + ].map((domain) => ({ domain, headers: { Authorization: authorizationFor(domain, input.token), @@ -855,7 +866,10 @@ function githubRepositoryFromUrl( upstreamUrl: URL, ): GitHubRepository | undefined { const segments = upstreamUrl.pathname.split("/").filter(Boolean); - if (isGitHubApiUrl(upstreamUrl) && segments[0]?.toLowerCase() === "repos") { + if ( + (isGitHubApiUrl(upstreamUrl) || isGitHubUploadsUrl(upstreamUrl)) && + segments[0]?.toLowerCase() === "repos" + ) { const owner = segments[1] ? decodeGitHubPathSegment(segments[1]) : undefined; @@ -1211,6 +1225,44 @@ function isGitHubApiUrl(upstreamUrl: URL): boolean { return upstreamUrl.hostname.toLowerCase() === "api.github.com"; } +// GitHub's release-asset upload endpoint (the `upload_url` returned by the +// create-release response) lives on a dedicated host, not api.github.com. +function isGitHubUploadsUrl(upstreamUrl: URL): boolean { + return upstreamUrl.hostname.toLowerCase() === "uploads.github.com"; +} + +// Release ids in GitHub's REST API are always numeric. +function isGitHubReleaseCreateRequest( + method: string, + upstreamUrl: URL, +): boolean { + return ( + method === "POST" && + isGitHubApiUrl(upstreamUrl) && + /^\/repos\/[^/]+\/[^/]+\/releases$/.test(upstreamUrl.pathname) + ); +} + +// Intentionally does not allow asset delete, release edit, or release +// delete: the image-attachment workflow this backs is append-only (unique +// filenames, never `--clobber`), so those stay denied by the default +// fallthrough. `gh release create TAG --latest=false` (no files argument) +// followed by a separate `gh release upload` is the supported sequence; +// passing files directly to `gh release create` additionally PATCHes the +// release to publish it, which remains denied. +function isGitHubReleaseAssetUploadRequest( + method: string, + upstreamUrl: URL, +): boolean { + return ( + method === "POST" && + isGitHubUploadsUrl(upstreamUrl) && + /^\/repos\/[^/]+\/[^/]+\/releases\/[0-9]+\/assets$/.test( + upstreamUrl.pathname, + ) + ); +} + function githubUserReadReason( method: string, upstreamUrl: URL, @@ -1381,9 +1433,15 @@ function githubApiWriteReason( upstreamUrl: URL, ): GitHubGrantReason | undefined { const pathname = upstreamUrl.pathname.toLowerCase(); + if (isGitHubReleaseAssetUploadRequest(method, upstreamUrl)) { + return "github.releases-write"; + } if (!isGitHubApiUrl(upstreamUrl)) { return undefined; } + if (isGitHubReleaseCreateRequest(method, upstreamUrl)) { + return "github.releases-write"; + } if ( method === "POST" && /^\/repos\/[^/]+\/[^/]+\/actions\/workflows\/[^/]+\/dispatches$/.test( @@ -1601,6 +1659,9 @@ function grantRequirements(reason: GitHubGrantReason): string[] | undefined { if (reason === "github.pull-review-write") { return PULL_REVIEW_WRITE_REQUIREMENTS; } + if (reason === "github.releases-write") { + return RELEASES_WRITE_REQUIREMENTS; + } if ( reason === "github.pull-create" || reason === "github.pull-requests-write" @@ -1671,6 +1732,14 @@ function installationGrantForWrite( if (reason === "github.pull-review-write") { return grantForAccess("write", reason, "user-write", leaseScope); } + if (reason === "github.releases-write") { + return grantForAccess( + "write", + reason, + "installation-releases-write", + leaseScope, + ); + } return undefined; } @@ -1821,7 +1890,7 @@ export function githubPlugin( "GitHub issue, pull request, and repository workflows via GitHub App", ...(appCapabilities ? { capabilities: appCapabilities } : {}), configKeys: ["org", "repo"], - domains: ["api.github.com", "github.com"], + domains: ["api.github.com", "github.com", "uploads.github.com"], envVars: { [appIdEnv]: {}, [privateKeyEnv]: {}, diff --git a/packages/junior-github/tests/github-plugin.test.ts b/packages/junior-github/tests/github-plugin.test.ts index 86a80530f..0b7236047 100644 --- a/packages/junior-github/tests/github-plugin.test.ts +++ b/packages/junior-github/tests/github-plugin.test.ts @@ -1567,6 +1567,91 @@ Conversation: \`local:test:old-conversation\` ).rejects.toThrow("github.contents-write is not enabled"); }); + it("grants scoped release create and asset upload writes for the image-attachment workflow", async () => { + await expect( + grantForEgress({ + method: "POST", + url: "https://api.github.com/repos/getsentry/junior/releases", + }), + ).resolves.toMatchObject({ + name: "installation-releases-write", + access: "write", + leaseScope: "repository:getsentry/junior", + reason: "github.releases-write", + requirements: expect.arrayContaining([ + "GitHub App Contents: write on the target repository", + ]), + }); + await expect( + grantForEgress({ + method: "POST", + url: "https://uploads.github.com/repos/getsentry/junior/releases/123/assets?name=screenshot-a1b2c3d4.png", + }), + ).resolves.toMatchObject({ + name: "installation-releases-write", + access: "write", + leaseScope: "repository:getsentry/junior", + reason: "github.releases-write", + }); + }); + + it("denies release-shaped requests on other hosts, non-numeric ids, and release/asset mutation", async () => { + await expect( + grantForEgress({ + method: "POST", + url: "https://evil.example.com/repos/getsentry/junior/releases/123/assets?name=x.png", + }), + ).rejects.toThrow("not an explicitly allowed Junior operation"); + await expect( + grantForEgress({ + method: "POST", + url: "https://uploads.github.com/repos/getsentry/junior/releases/not-a-number/assets?name=x.png", + }), + ).rejects.toThrow("not an explicitly allowed Junior operation"); + await expect( + grantForEgress({ + method: "DELETE", + url: "https://api.github.com/repos/getsentry/junior/releases/123", + }), + ).rejects.toThrow("not an explicitly allowed Junior operation"); + await expect( + grantForEgress({ + method: "PATCH", + url: "https://api.github.com/repos/getsentry/junior/releases/123", + }), + ).rejects.toThrow("not an explicitly allowed Junior operation"); + await expect( + grantForEgress({ + method: "DELETE", + url: "https://api.github.com/repos/getsentry/junior/releases/assets/456", + }), + ).rejects.toThrow("not an explicitly allowed Junior operation"); + await expect( + grantForEgress({ + method: "POST", + url: "https://api.github.com/repos/getsentry/junior/releases/generate-notes", + }), + ).rejects.toThrow("not an explicitly allowed Junior operation"); + }); + + it("scopes release writes to the repository named in the request URL, not an unrelated repository", async () => { + const createGrant = await grantForEgress({ + method: "POST", + url: "https://api.github.com/repos/getsentry/junior/releases", + }); + const uploadGrant = await grantForEgress({ + method: "POST", + url: "https://uploads.github.com/repos/getsentry/other-repo/releases/123/assets?name=x.png", + }); + expect(createGrant).toMatchObject({ + leaseScope: "repository:getsentry/junior", + }); + expect(uploadGrant).toMatchObject({ + leaseScope: "repository:getsentry/other-repo", + }); + expect(createGrant?.leaseScope).not.toEqual(uploadGrant?.leaseScope); + }); + it("separates pull request lifecycle writes from human review identity", async () => { await expect( grantForEgress({ @@ -1764,6 +1849,10 @@ Conversation: \`local:test:old-conversation\` { domain: "github.com", }, + { + domain: "uploads.github.com", + headers: { Authorization: "Bearer installation-token" }, + }, ], }, }); @@ -1914,6 +2003,10 @@ Conversation: \`local:test:old-conversation\` Authorization: expect.stringMatching(/^Basic /), }, }, + { + domain: "uploads.github.com", + headers: { Authorization: "Bearer user-token" }, + }, ], }, }); @@ -2009,6 +2102,10 @@ Conversation: \`local:test:old-conversation\` Authorization: expect.stringMatching(/^Basic /), }, }, + { + domain: "uploads.github.com", + headers: { Authorization: "Bearer delegated-token" }, + }, ], }, }); @@ -2096,6 +2193,10 @@ Conversation: \`local:test:old-conversation\` Authorization: expect.stringMatching(/^Basic /), }, }, + { + domain: "uploads.github.com", + headers: { Authorization: "Bearer fresh-token" }, + }, ], }, }); @@ -2171,6 +2272,10 @@ Conversation: \`local:test:old-conversation\` Authorization: expect.stringMatching(/^Basic /), }, }, + { + domain: "uploads.github.com", + headers: { Authorization: "Bearer fresh-token" }, + }, ], }, }); From 0c22153ab2882f73b2851ecb6fc6433f0a5cb3d8 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:54:28 +0000 Subject: [PATCH 2/2] test: update GitHub manifest domain assertion for uploads.github.com --- .../tests/unit/plugins/plugin-manifest-api-headers.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/junior/tests/unit/plugins/plugin-manifest-api-headers.test.ts b/packages/junior/tests/unit/plugins/plugin-manifest-api-headers.test.ts index 4a07687df..684dbd0d4 100644 --- a/packages/junior/tests/unit/plugins/plugin-manifest-api-headers.test.ts +++ b/packages/junior/tests/unit/plugins/plugin-manifest-api-headers.test.ts @@ -147,7 +147,11 @@ describe("plugin manifest API headers", () => { const { githubPlugin } = await import("@sentry/junior-github"); const manifest = githubPlugin().manifest!; - expect(manifest.domains).toEqual(["api.github.com", "github.com"]); + expect(manifest.domains).toEqual([ + "api.github.com", + "github.com", + "uploads.github.com", + ]); expect(manifest.credentials).toBeUndefined(); expect(manifest.envVars).toMatchObject({ GITHUB_APP_CLIENT_ID: {},