Skip to content

fix(authentication): support GHE.com release asset downloads - #4807

Merged
mnriem merged 3 commits into
github:mainfrom
mnriem:mnriem-fix-ghecom-release-assets
Oct 1, 2026
Merged

mnriem merged 3 commits into
github:mainfrom
mnriem:mnriem-fix-ghecom-release-assets

Conversation

@mnriem

@mnriem mnriem commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Description

Fix private release-asset downloads for GitHub Enterprise Cloud with data residency (GHE.com) without reverting the URL-validation hardening introduced by #4438.

The shared resolver previously classified a trusted tenant.ghe.com host as GitHub Enterprise Server and looked up releases through https://tenant.ghe.com/api/v3. It then rejected the actual asset URL on https://api.tenant.ghe.com/repos/... because the API origin and path differed. Bundle downloads fell back to the browser release URL, which can serve SSO HTML instead of the ZIP.

  • Derive https://api.tenant.ghe.com for GHE.com release metadata.
  • Require the tenant web host and its paired API host to match trusted GitHub-provider hosts in auth.json.
  • Preserve exact origin, owner/repository, numeric asset endpoint, and malformed-URL validation.
  • Recognize direct trusted GHE.com API asset URLs so callers request Accept: application/octet-stream.
  • Preserve GitHub.com and GHES resolution behavior and document GHE.com authentication configuration.

The fix lives in the shared release-asset resolver used by bundles, extensions, presets, and workflows. It does not change workflow requires.speckit_version enforcement.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample project (if applicable)

Tests used this worktree's own interpreter instead of bare uv run pytest, per CONTRIBUTING.md and AGENTS.md. The sample-project checkbox is left unchecked because no live private GHE.com release was available; bundle coverage uses a temporary project and mocked release metadata/ZIP responses.

Exact commands and results:

  • uv sync --extra test --quiet -- passed; installed this worktree's test environment.
  • .venv/bin/python -m pytest tests/specify_cli/authentication/test_github_http.py tests/specify_cli/bundles/test_command_info.py -q -- before the implementation, the new regression cases produced 5 failed, 101 passed; after the fix and an additional trust test, 107 passed.
  • .venv/bin/python -m pytest tests/specify_cli/authentication tests/specify_cli/bundles tests/specify_cli/presets/test_catalog.py tests/specify_cli/presets/test_command_add.py tests/specify_cli/workflows/test_command_add.py tests/specify_cli/extensions/test_command_add.py -q -- 972 passed.
  • .venv/bin/python -m pytest tests -q -- 9,232 passed, 18 skipped; 9,250 collected, with no failures.
  • uv run specify --help -- passed.
  • uvx ruff@0.15.0 check src/specify_cli/authentication/github_http.py src/specify_cli/authentication/http.py tests/specify_cli/authentication/test_github_http.py tests/specify_cli/bundles/test_command_info.py -- passed.
  • git diff --check -- passed.

Positive coverage includes the exact GHE.com metadata URL, direct trusted asset API passthrough, and bundle ZIP extraction with the octet-stream header. Negative coverage includes missing trusted tenant hosts, another tenant's API origin, wrong owner/repository, the GHES /api/v3 path, HTTP, nonnumeric asset IDs, and query parameters. Existing malformed-URL and GitHub.com/GHES cases remain intact.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: GitHub Copilot acted on behalf of @mnriem using GPT-5.6 Sol and GPT-6.1 Sol in autonomous agent mode; the runtime reasoning-effort setting was not exposed. AI assistance covered investigation, code and documentation generation, regression tests, automated validation, the commit, and this PR draft. The contributor approved the approach and submission; no human line-by-line review is claimed.

Resolve data-resident GitHub Enterprise Cloud releases through their paired API subdomain while preserving strict asset URL validation. Require trusted web/API tenant hosts, recognize direct asset API URLs, and document configuration with resolver and bundle regression coverage.

Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 11:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The legacy GHES passthrough can bypass the new GHE.com tenant-pair trust checks.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Adds authenticated GHE.com release-asset resolution while retaining existing GitHub.com and GHES behavior.

Changes:

  • Derives paired GHE.com API hosts with trust validation.
  • Adds resolver and bundle regression coverage.
  • Documents GHE.com authentication configuration.
File Description
src/​specify_cli/​authentication/​github_http.py Implements GHE.com URL resolution and validation.
src/​specify_cli/​authentication/​http.py Updates provider-host documentation.
tests/​specify_cli/​authentication/​test_github_http.py Tests GHE.com resolution and rejection cases.
tests/​specify_cli/​bundles/​test_command_info.py Tests authenticated GHE.com bundle extraction.
docs/​reference/​authentication.md Documents required GHE.com hosts.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/authentication/github_http.py Outdated
Keep GHE.com API hosts out of the legacy GHES direct-asset passthrough so the tenant-pair trust checks cannot be bypassed. Add regression coverage for trusted and untrusted host configurations.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 11:44
@mnriem

mnriem commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the review finding in commit 5899a2f987c0af42321f6b0466a6df700ce09cc4 by excluding .ghe.com hosts from the legacy GHES /api/v3 direct-asset passthrough. Added regression coverage proving the invalid path is rejected with both trusted and untrusted host configurations.

Validation: .venv/bin/python -m pytest tests/specify_cli/authentication/test_github_http.py tests/specify_cli/bundles/test_command_info.py -q — 109 passed; shared authentication/bundle/preset/workflow/extension resolver suite — 974 passed; Ruff and git diff --check passed.

Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol, autonomous agent mode; reasoning-effort setting not exposed). AI assistance covered the code change, regression tests, validation, commit, and this review-round summary.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Direct GHE.com asset passthrough currently accepts malformed or noncanonical URLs.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Validate raw GHE asset URLs before passthrough

src/​specify_cli/​authentication/​github_http.py:218

The new direct GHE.com passthrough validates decoded, slash-stripped path parts, so noncanonical URLs such as .../assets/42?, .../assets/42/, .../assets/%34%32, or paths containing raw whitespace still pass and receive API-asset treatment. Empty query/fragment delimiters are not reflected in parsed.query/fragment, and strip("/") erases extra slashes. Validate the original URL spelling and the exact raw path before returning it, matching the metadata URL hardening.

Reject ambiguous raw spellings and noncanonical paths before treating direct GHE.com URLs as release assets. Share the existing metadata URL spelling validation and cover empty delimiters, trailing slashes, encoded IDs, whitespace, semicolons, and malformed escapes.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 11:53
@mnriem

mnriem commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the latest review in commit ac64adecc82b52ac72665e2d96610ee4e5f45f86. Direct GHE.com asset passthrough now validates the original URL spelling and exact raw /repos/{owner}/{repo}/releases/assets/{numeric-id} path before applying asset treatment. The shared spelling guard preserves the existing metadata hardening and rejects empty query/fragment delimiters, trailing slashes, percent-encoded asset IDs, whitespace/control characters, semicolons, and malformed percent escapes.

Validation: focused resolver/bundle suite — 116 passed; shared authentication/bundle/preset/workflow/extension resolver suite — 981 passed; Ruff and git diff --check passed.

Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol, autonomous agent mode; reasoning-effort setting not exposed). AI assistance covered the code change, regression tests, validation, commit, and this review-round summary.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation preserves existing paths and includes comprehensive positive, negative, and regression coverage.

Review effort: Balanced
Findings: None

@mnriem
mnriem merged commit 6244a2e into github:main Oct 1, 2026
15 checks passed
@mnriem
mnriem deleted the mnriem-fix-ghecom-release-assets branch October 1, 2026 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants