feat: add builder pattern for customizable GitHub API URL#36
Conversation
Allow customizing the GitHub API URL via `AttestationClient::builder().base_url(...)`, enabling use behind reverse proxies or custom GitHub API endpoints. Closes #32 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a builder pattern for the AttestationClient, allowing for configurable base URLs and GitHub tokens, and re-exports the new builder. A security vulnerability was identified in the URL validation logic within the github_headers method, where the use of starts_with could lead to credential leakage via domain shadowing. A suggestion was provided to ensure the URL match occurs at a proper path boundary.
| .map_err(|e| AttestationError::Api(e.to_string()))?, | ||
| ); | ||
| } | ||
| if url.starts_with(&self.base_url) { |
There was a problem hiding this comment.
The use of starts_with for URL validation is insecure as it can lead to credential leakage via domain shadowing. For example, if base_url is https://api.github.com, it would match https://api.github.com.attacker.com, sending the AUTHORIZATION header to an untrusted domain.
You should ensure that the match occurs at a path boundary (i.e., the URL is either exactly the base URL or starts with the base URL followed by a /).
| if url.starts_with(&self.base_url) { | |
| if url == self.base_url || url.starts_with(&format!("{}/", self.base_url)) { |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 34ac4e5. Configure here.
Greptile SummaryThis PR introduces
Confidence Score: 5/5Safe to merge; no P0/P1 issues remain — the prior token-leakage vulnerability is fixed and both remaining findings are P2 hardening suggestions. The primary security concern from the previous review (unbounded starts_with allowing subdomain-spoofed bundle URLs to receive auth headers) is correctly addressed by appending '/' before the prefix check. The new tests directly exercise the header-isolation invariant with two separate mockito servers. The two remaining findings (Debug leaking token, no base_url validation) are quality-of-life improvements that do not affect correctness in normal usage. src/api.rs deserves a follow-up on the Debug impl and base_url validation; no other files require attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Builder as AttestationClientBuilder
participant Client as AttestationClient
participant GitHubAPI as GitHub API (base_url)
participant BundleHost as Bundle Host (other origin)
Caller->>Builder: base_url("https://proxy/github")
Caller->>Builder: github_token("ghp_xxx")
Caller->>Builder: build()
Builder-->>Caller: AttestationClient { base_url, github_token }
Caller->>Client: fetch_attestations(params)
Client->>Client: github_headers(url)<br/>url starts with base_url+"/"
Note right of Client: Authorization +<br/>x-github-api-version added
Client->>GitHubAPI: GET /repos/.../attestations/{digest} + auth headers
GitHubAPI-->>Client: {attestations: [{bundle_url: "https://other/bundle"}]}
Client->>Client: github_headers(bundle_url)<br/>bundle_url ≠ base_url prefix
Note right of Client: No auth headers added
Client->>BundleHost: GET /bundle (no Authorization)
BundleHost-->>Client: SigstoreBundle JSON
Client-->>Caller: Vec<Attestation>
Greploops — Automatically fix all review issues by running Reviews (2): Last reviewed commit: "fix: prevent token leakage via URL prefi..." | Re-trigger Greptile |
| #[derive(Debug, Clone, Default)] | ||
| pub struct AttestationClientBuilder { | ||
| base_url: Option<String>, | ||
| github_token: Option<String>, | ||
| } | ||
|
|
||
| impl AttestationClientBuilder { | ||
| pub fn base_url(mut self, url: &str) -> Self { | ||
| self.base_url = Some(url.trim_end_matches('/').to_string()); | ||
| self | ||
| } | ||
|
|
||
| pub fn github_token(mut self, token: &str) -> Self { | ||
| self.github_token = Some(token.to_string()); | ||
| self | ||
| } | ||
|
|
||
| pub fn build(self) -> Result<AttestationClient> { | ||
| let mut headers = HeaderMap::new(); | ||
| headers.insert(USER_AGENT, HeaderValue::from_static(USER_AGENT_VALUE)); | ||
|
|
||
| let client = reqwest::Client::builder() | ||
| .default_headers(headers) | ||
| .build()?; | ||
|
|
||
| Ok(AttestationClient { | ||
| client, | ||
| base_url: self.base_url.unwrap_or_else(|| GITHUB_API_URL.to_string()), | ||
| github_token: self.github_token, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
AttestationClientBuilder is new public API but the PR adds no tests for it. mockito is already a dev-dependency, so it would be straightforward to add tests covering:
- That
base_url()is used as the host for constructed request URLs - That auth headers are only sent to the configured host (directly exercises the security invariant in
github_headers) - That
AttestationClient::new()still produces a working client via its delegation to the builder
Use path-boundary check (trailing slash) in github_headers to prevent auth tokens being sent to look-alike domains (e.g. api.github.com.evil.com). Add tests covering the builder API, custom base URL routing, and the security invariant that auth headers are only sent to the configured host. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@greptileai check |
## 🤖 New release * `sigstore-verification`: 0.2.1 -> 0.2.2 (✓ API compatible changes) <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.2.2](v0.2.1...v0.2.2) - 2026-04-04 ### Added - add builder pattern for customizable GitHub API URL ([#36](#36)) ### Fixed - generate Cargo.lock before security audit ([#24](#24)) ### Other - *(deps)* pin dtolnay/rust-toolchain action to 29eef33 ([#33](#33)) - *(deps)* update jdx/mise-action digest to 1648a78 ([#34](#34)) - *(deps)* update jdx/mise-action action to v4 ([#31](#31)) - *(deps)* update swatinem/rust-cache digest to e18b497 ([#30](#30)) - *(deps)* update release-plz/action digest to 1528104 ([#29](#29)) - *(deps)* update jdx/mise-action digest to 5228313 ([#28](#28)) - *(deps)* update jdx/mise-action digest to e79ddf6 ([#27](#27)) - *(deps)* pin dependencies ([#26](#26)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: this PR only updates release metadata (crate version and changelog) without changing library code or behavior. > > **Overview** > Prepares the `v0.2.2` release by bumping `Cargo.toml` from `0.2.1` to `0.2.2` and adding the corresponding `CHANGELOG.md` section. > > The changelog notes the builder pattern for a customizable GitHub API URL, a fix to generate `Cargo.lock` before security audit, and several CI/dependency pin updates. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9b5a822. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## Summary - Adds a `base_url` option to `GitHubSource` and `verify_github_attestation`, so callers can verify attestations published on GitHub Enterprise Server instances (e.g. `https://github.enterprise.com/api/v3`) instead of always targeting `api.github.com`. - New surface (all additive, backward-compatible): - `GitHubSource::with_base_url(owner, repo, token, base_url)` - `GitHubSource::with_client(owner, repo, client)` — hand in a pre-built `AttestationClient` - `GitHubSource::builder()` fluent builder - `verify_github_attestation_with_base_url(...)` convenience wrapper mirroring `verify_github_attestation` - Existing `GitHubSource::new` and `verify_github_attestation` keep their current behavior (default to `api.github.com`). ## Motivation mise users who point the github backend at a GHES instance via `api_url = \"https://github.enterprise.com/api/v3\"` had their downloads succeed against GHES, but attestation verification then fell back to `api.github.com` and failed with a 401 because the GHES token isn't valid there. Details: jdx/mise#9176 The underlying `AttestationClient::builder().base_url(...)` already existed (#36); this PR plumbs it through the public `GitHubSource` and `verify_github_attestation` surfaces that callers actually use. ## Test plan - [x] `cargo test` — all existing tests still pass (10+6+3 in existing suites). - [x] New `tests/github_source_base_url.rs`: `GitHubSource::with_base_url`, `::with_client`, and `::builder()` all route requests to the configured host and send the auth token; `builder()` surfaces missing-field errors. - [x] New `tests/verify_github_attestation_base_url.rs`: end-to-end test of `verify_github_attestation_with_base_url` against a mockito server, asserting the request hits the custom host with the right auth header. - [x] `cargo clippy --all-targets -- -D warnings` clean. - [x] `cargo fmt --all --check` clean. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Moderate risk: changes how GitHub API clients are constructed and where auth tokens are sent, which can affect attestation fetching/verification behavior across hosts (dotcom vs GHES). Additive APIs and new tests reduce regression risk, but misconfiguration could lead to unexpected unauthenticated requests. > > **Overview** > Enables attestation fetching and `verify_github_attestation` to target a *custom GitHub API base URL* (e.g. GitHub Enterprise Server) instead of always using `api.github.com`, by introducing `verify_github_attestation_with_base_url` and refactoring verification to build an `AttestationClient` with optional token + base URL. > > Updates `GitHubSource` to support `with_base_url`, `with_client`, and a fluent `builder()` (with required-field validation), and adds mock-based tests to assert requests route to the configured host and include the expected auth/version headers. The changelog is updated to document the new capability. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit dbe7653. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Summary
AttestationClientBuilderwith a builder pattern for constructingAttestationClientbase_urlto override the defaulthttps://api.github.com, enabling use behind reverse proxiesgithub_headersto send auth tokens and API version headers to the configured base URL (not justapi.github.com)Usage
The existing
AttestationClient::new()API is unchanged.Closes #32
Test plan
cargo checkpasses🤖 Generated with Claude Code
Note
Medium Risk
Medium risk because it changes how auth and API-version headers are conditionally attached to outbound HTTP requests; mistakes could break attestation fetching or leak tokens to unintended endpoints.
Overview
Adds an
AttestationClientBuilderto constructAttestationClientwith an optional custombase_url(defaulting tohttps://api.github.com) and optional GitHub token, while keepingAttestationClient::new()by delegating to the builder.Updates
github_headersto applyAuthorizationandx-github-api-versionheaders based on whether a request URL matches the configuredbase_url(instead of hard-codingapi.github.com), preventing headers from being sent to non-matching hosts (e.g., externalbundle_urls).Re-exports
AttestationClientBuilderfromlib.rsand addsmockitotests covering default/custom base URLs, trailing-slash normalization, header injection for custom hosts, and ensuring headers are not sent to different hosts.Reviewed by Cursor Bugbot for commit 1406408. Bugbot is set up for automated code reviews on this repo. Configure here.