fix(github): crafted owner/repo can hit the wrong GitHub API path

[SebTardif]

What Problem This Solves

Fixes an issue where Discord /github (and summary-emoji reactions on a GitHub issue or PR URL) would treat a crafted owner or repo string as extra GitHub API path segments. The bot then called that URL with the GitHub App installation token, so a slash or .. segment could change which API object was read. The issue body is also sent to OpenAI when a summary is generated.

Why This Change Was Made

fetchGitHubSummaryData now accepts only GitHub owner and repo names (A-Za-z0-9._-, and not . or ..) before it builds https://api.github.com/repos/{owner}/{repo}/issues/{number}. The installation token is attached only for openclaw/openclaw (case-insensitive), which is the command default and the App installation target. Other valid public repos still fetch without that token. Both /github and reaction summaries go through this one function.

User Impact

/github number user repo still summarizes public issues and PRs. Crafted owner or repo values no longer change the API path or reuse the App token. Summaries of openclaw/openclaw keep authenticated access for private or rate-limited reads.

Evidence

Terminal output from a live bun run of the patched helpers, plus a public GitHub API fetch with no installation token:

$ bun /tmp/hermit-f003-proof.ts
owner              repo                 valid   trusted  wouldFetch  auth
openclaw           openclaw             true    true     true        installation
OpenClaw           openclaw             true    true     true        installation
openclaw           hermit               true    false    true        public
octocat            hello-world          true    false    true        public
foo/../users       me                   false   false    false       public
openclaw           openclaw/../hermit   false   false    false       public
.                  openclaw             false   false    false       public
openclaw           ..                   false   false    false       public
foo%2fbar          repo                 false   false    false       public
public_issue_status 200
public_issue_number 1
public_issue_url https://github.com/openclaw/openclaw/pull/1

Before the patch, fetchGitHubSummaryData("foo/../users", "me", 1) built repoName: "foo/../users/me" and still called GitHub. After the patch that input returns null and no request is sent.

Real behavior proof

• Behavior or issue addressed: Crafted Discord /github owner or repo values could change the GitHub API path and were fetched with the App installation token.

• Real environment tested: Windows 11, Bun 1.4.1, hermit checkout at fix/github-owner-repo-sanitize on fd69205, live bun plus unauthenticated fetch to api.github.com.

• Exact steps or command run after this patch:

  bun /tmp/hermit-f003-proof.ts

• Evidence after fix: terminal output from the patched helpers (table above). Traversal names are rejected before fetch. octocat/hello-world would fetch without the installation token. openclaw/openclaw still uses the installation token. Unauthenticated GET https://api.github.com/repos/openclaw/openclaw/issues/1 returned HTTP 200 for PR 1.

• Observed result after fix: Unsafe names never reach api.github.com. Public repos stay readable without the App token. The trusted default repo still authenticates.

• What was not tested: A live Discord /github click in a production guild, and a private sibling repo under the same installation.

Related

• Introduced when summaries were added in bbfa77bb (2026-05-20) and authenticated in c16b98b7 (2026-05-21). Present for 111 days.
• GitHub owner and repo names are [A-Za-z0-9_.-] (desktop/desktop#3090, moby/moby#679).
• Same charset documented on integrations/terraform-provider-github#1215.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 8, 2026, 9:00 PM ET / September 9, 2026, 01:00 UTC.

ClawSweeper review

What this changes

Validates repository names before fetching GitHub summaries, restricts installation-token use to openclaw/openclaw, and adds regression tests.

Merge readiness

⛔ Blocked before merge - 6 items remain

The path-validation fix remains necessary on main. The branch contains useful hardening, but its authentication restriction introduces a compatibility regression and the supplied proof does not establish the changed production fetch behavior.

Priority: P1
Reviewed head: 89827a248f4ceefb71fd2b12f3aebb21c42ba89a
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	Focused hardening and regression tests are useful, but compatibility and production-path proof remain unresolved.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: The Windows/Bun transcript provides useful validator output and a real public GitHub response, but its wouldFetch table does not demonstrate requests through the changed fetchGitHubSummaryData owner. Add redacted production-path evidence of accepted authenticated and public requests plus rejected traversal input; Discord video is unnecessary for this network boundary. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The Windows/Bun transcript provides useful validator output and a real public GitHub response, but its wouldFetch table does not demonstrate requests through the changed fetchGitHubSummaryData owner. Add redacted production-path evidence of accepted authenticated and public requests plus rejected traversal input; Discord video is unnecessary for this network boundary. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	6 items	Current main still lacks the guard: The main implementation interpolates owner and repository directly into the issues URL and always obtains GitHub App headers. Both Discord entry points call this helper. Introduced authentication restriction: The verified base-to-head patch adds validation before fetching and makes every repository other than openclaw/openclaw anonymous. This is introduced branch behavior, not base drift. Installation is configurable: Token creation reads GITHUB_APP_INSTALLATION_ID from the environment; it does not establish that the installation belongs exclusively to openclaw/openclaw. The command also accepts explicit owner and repository options.
Findings	1 actionable finding	[P1] Preserve approved installation access instead of hardcoding the default
Security	None	None.

How this fits together

Hermit turns Discord commands and message reactions into GitHub issue or pull-request summaries. Its shared fetch helper retrieves issue data, optionally summarizes the body through OpenAI, and returns a Discord card.

flowchart TD
 A[Discord command or reaction] --> B[Validate repository names]
 B -->|Invalid| C[No GitHub request]
 B -->|Valid| D[Select public or authenticated access]
 D --> E[GitHub issue API]
 E --> F[Optional AI summary]
 F --> G[Discord summary card]

Loading

Decision needed

Question	Recommendation
Should authenticated summaries be limited permanently to openclaw/openclaw, or must explicitly approved repositories from the configured installation remain supported?	Support explicitly approved repositories: Retain validation and define a narrow repository policy that preserves approved authenticated use without exposing installation-wide access to arbitrary Discord requests.

Why: The command default does not establish the installation's authorization policy, and selecting that policy determines which existing summaries remain available.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The Windows/Bun transcript provides useful validator output and a real public GitHub response, but its wouldFetch table does not demonstrate requests through the changed fetchGitHubSummaryData owner. Add redacted production-path evidence of accepted authenticated and public requests plus rejected traversal input; Discord video is unnecessary for this network boundary. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Preserve approved installation access instead of hardcoding the default (P1) - For a deployment whose configured GitHub App installation can read a repository other than openclaw/openclaw, this branch now sends its summary request anonymously. Previously working private-repository summaries therefore become 404/null, producing a not-found command response or no reaction summary. GITHUB_APP_INSTALLATION_ID is configurable, and the command default does not constrain it. Retain the path guard, but resolve the approved repository policy and provide a compatibility path or explicit, documented approval of this breaking restriction.
• Resolve merge risk (P1) - Existing deployments using authenticated summaries for another installation-accessible repository will lose that access; affected deployment scope and acceptance of this restriction are unverified.
• Resolve merge risk (P1) - Non-default public repositories also lose authenticated rate-limit capacity, without upgrade evidence showing the operational effect.
• Complete next step (P2) - Resolve the authenticated-repository policy and compatibility finding, then add actual summary-fetch proof. Redact tokens, IP addresses, private endpoints, and issue contents. Terminal output or logs are sufficient; screenshots or recordings help when they show the behavior. Updating the PR body should trigger review automatically; otherwise ask a maintainer to comment @clawsweeper re-review.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [P1] Preserve approved installation access instead of hardcoding the default — src/utils/githubSummary.ts:247-249

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +28/-5; tests +133/-0	Production growth implements shared validation and header selection; the added coverage uses mocked transport.

Merge-risk options

Maintainer options:

1. Resolve repository policy before landing (recommended)
   Define approved authenticated repositories and cover an existing non-default installation before replacing the current behavior.
2. Accept the narrower supported scope
   Explicitly approve the loss of non-default authenticated summaries and supply operator-facing upgrade guidance and proof.

Technical review

Best possible solution:

Keep validation at the shared fetch boundary and use an explicitly approved repository-access policy with demonstrated fresh-install and upgrade behavior.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: the current command passes unrestricted strings into an authenticated URL interpolation, so slash and traversal segments can alter the request path. No live exploit or private-data retrieval was executed.

Is this the best way to solve the issue?

Partly: central validation is appropriately placed, but a command default is insufficient justification for replacing the configured installation's authentication behavior with a hardcoded repository policy.

Full review comments:

• [P1] Preserve approved installation access instead of hardcoding the default — src/utils/githubSummary.ts:247-249
  For a deployment whose configured GitHub App installation can read a repository other than openclaw/openclaw, this branch now sends its summary request anonymously. Previously working private-repository summaries therefore become 404/null, producing a not-found command response or no reaction summary. GITHUB_APP_INSTALLATION_ID is configurable, and the command default does not constrain it. Retain the path guard, but resolve the approved repository policy and provide a compatibility path or explicit, documented approval of this breaking restriction.
  Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against fd6920504d12.

Labels

Label changes:

• add P1: The owned summary path accepts crafted API-path input while using installation credentials.
• add merge-risk: 🚨 compatibility: The hardcoded repository restriction removes authenticated summary access supported by the configurable installation path.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The Windows/Bun transcript provides useful validator output and a real public GitHub response, but its wouldFetch table does not demonstrate requests through the changed fetchGitHubSummaryData owner. Add redacted production-path evidence of accepted authenticated and public requests plus rejected traversal input; Discord video is unnecessary for this network boundary. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

• P1: The owned summary path accepts crafted API-path input while using installation credentials.
• merge-risk: 🚨 compatibility: The hardcoded repository restriction removes authenticated summary access supported by the configurable installation path.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The Windows/Bun transcript provides useful validator output and a real public GitHub response, but its wouldFetch table does not demonstrate requests through the changed fetchGitHubSummaryData owner. Add redacted production-path evidence of accepted authenticated and public requests plus rejected traversal input; Discord video is unnecessary for this network boundary. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Current main still lacks the guard: The main implementation interpolates owner and repository directly into the issues URL and always obtains GitHub App headers. Both Discord entry points call this helper. (src/utils/githubSummary.ts:227, fd6920504d12)
• Introduced authentication restriction: The verified base-to-head patch adds validation before fetching and makes every repository other than openclaw/openclaw anonymous. This is introduced branch behavior, not base drift. (src/utils/githubSummary.ts:247, 89827a248f4c)
• Installation is configurable: Token creation reads GITHUB_APP_INSTALLATION_ID from the environment; it does not establish that the installation belongs exclusively to openclaw/openclaw. The command also accepts explicit owner and repository options. (src/utils/githubAuth.ts:62, 89827a248f4c)
• Captured proof and its coverage: The supplied complete PR body, in context sourceRevision 7233f99d25afacb72f027b85b287bff6086bcbd9d1f0fabc69ff6e465a987612, includes a Windows/Bun helper table labeled wouldFetch and a separate unauthenticated HTTP 200 result. It explicitly excludes a private sibling repository. The proof script is not supplied, so the transcript does not establish that the real fetchGitHubSummaryData path exercised authenticated and anonymous requests. The added tests replace fetch and mock the token. (tests/githubSummary.test.ts:18, 89827a248f4c)
• Feature-history routing: GitHub commit metadata identifies thewilloftheshadow as the author account for the authentication change. Its patch added githubAuth.ts and wired authentication into summaries. The raw commit records parent cf99519; inspection of that parent's tree found no githubAuth.ts. Some local follow/blame inspection failed on unavailable historical blobs; GitHub commit patches supplied the relevant history. (src/utils/githubAuth.ts:62, c16b98b7ce85)
• Canonical and release checks: The REST pull-request listing returned this PR as the only title match for GitHub, summary, token, or path. The main endpoint still identifies fd69205, and the releases endpoint returned no releases. No merged replacement was established. (fd6920504d12)

Likely related people:

• Shadow: Raw commit c16b98b adds src/utils/githubAuth.ts:62 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: c16b98b7ce85; files: src/utils/githubAuth.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Resolve the approved repository policy and demonstrate fresh-install and existing-installation behavior.
• Provide a redacted trace through the actual summary fetch helper showing authenticated success, anonymous public success, and rejection before I/O.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.