Skip to content

fix: resolve GitHub release asset API URL for private repo bundle downloads#3136

Open
lselvar wants to merge 3 commits into
github:mainfrom
lselvar:fix/private-github-release-bundle-downloads
Open

fix: resolve GitHub release asset API URL for private repo bundle downloads#3136
lselvar wants to merge 3 commits into
github:mainfrom
lselvar:fix/private-github-release-bundle-downloads

Conversation

@lselvar

@lselvar lselvar commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix — one download path is now covered

_download_remote_manifest (catalog-based bundle manifest download):

  • Resolves browser release URLs to REST API asset URLs before downloading the bundle manifest (YAML or ZIP)
  • Direct REST API asset URLs (api.github.com/repos/.../releases/assets/<id>) are passed through directly with Accept: application/octet-stream
  • The original catalog URL is still used to determine artifact format (.zip vs YAML) since the resolved API URL does not carry the file extension
  • Other URLs: existing behavior unchanged

Implementation

Uses the existing shared resolve_github_release_asset_api_url(download_url, open_url_fn, timeout) function from _github_http.py — the same utility used by the extension, preset, and workflow fixes. No new helpers needed.

Test plan

  • Run the bundler contract test suite:
    UV_NATIVE_TLS=true SSL_CERT_FILE=/opt/homebrew/etc/openssl@3/cert.pem UV_DEFAULT_INDEX=https://pypi.org/simple PYTHONPATH=src uv run --extra test pytest tests/contract/test_bundle_cli.py -v
    Expected: 25 passed (includes 2 new tests)
  • Run full bundler test suite:
    UV_NATIVE_TLS=true SSL_CERT_FILE=/opt/homebrew/etc/openssl@3/cert.pem UV_DEFAULT_INDEX=https://pypi.org/simple PYTHONPATH=src uv run --extra test pytest tests/contract/ tests/unit/ tests/integration/ tests/test_github_http.py
    Expected: all passing
  • Verify specify bundle install <id> works when the catalog's download_url points at a GitHub release asset on a private repo
  • Verify specify bundle info <id> works for the same case

AI disclosure: This PR was developed with Claude Code assistance.

🤖 Generated with Claude Code

…nloads

For private/SSO-protected GitHub repos, browser release download URLs
(https://github.com/<owner>/<repo>/releases/download/<tag>/<asset>)
redirect to an HTML/SSO page instead of delivering the asset, causing
bundle manifest downloads to fail.

Extends the pattern from github#2855 (presets/workflows) to cover the bundle
manifest download path in _download_remote_manifest:

- Resolves browser release URLs to GitHub REST API asset URLs via
  resolve_github_release_asset_api_url before downloading
- Direct REST API asset URLs (api.github.com/repos/.../releases/assets/<id>)
  are passed through directly
- Both cases use Accept: application/octet-stream so the API returns the
  binary payload rather than JSON metadata
- The original catalog URL is used to determine artifact format (.zip vs
  YAML) since the resolved API URL does not carry the file extension

Adds two CLI-level contract tests:
- bundle info resolves browser release URL via GitHub tags API
- bundle info passes direct API asset URL through with octet-stream

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

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.

Pull request overview

This PR extends the existing “private GitHub release asset download URL” fix to the bundle manifest download path, ensuring bundle info / bundle install can successfully download manifests from private/SSO-protected GitHub release assets by resolving browser release URLs to GitHub REST asset URLs and downloading them with Accept: application/octet-stream.

Changes:

  • Apply resolve_github_release_asset_api_url() in _download_remote_manifest() to translate GitHub browser release download URLs into REST asset URLs prior to downloading.
  • Send Accept: application/octet-stream when downloading via GitHub REST asset URLs to retrieve the binary payload instead of JSON metadata.
  • Add contract tests validating browser-URL resolution and direct REST asset URL passthrough for bundle info.
Show a summary per file
File Description
src/specify_cli/commands/bundle/__init__.py Resolve GitHub release asset URLs before manifest download; download assets via REST with octet-stream header.
tests/contract/test_bundle_cli.py Add contract coverage for browser release URL resolution and direct REST asset URL behavior for bundle manifest downloads.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment thread tests/contract/test_bundle_cli.py
Comment thread src/specify_cli/commands/bundle/__init__.py Outdated

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address Copilot feedback

Address Copilot review feedback on PR github#3136:

1. Detect ZIP payloads by magic bytes (PK\x03\x04) in addition to the
   '.zip' URL suffix so that direct GitHub REST asset URLs — which carry
   no file extension — are correctly routed through the ZIP extraction
   path when the asset is a ZIP bundle artifact.

2. Add two new contract tests:
   - test_bundle_info_resolves_github_browser_release_url_zip: exercises
     the '.zip' browser release URL path end-to-end, verifying the tags
     API lookup fires, octet-stream header is used, and bundle.yml is
     successfully extracted from the ZIP payload.
   - test_bundle_info_api_asset_url_zip_detected_by_magic_bytes: verifies
     that a direct REST asset URL returning ZIP bytes is detected by magic
     and parsed correctly without a tags API call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

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.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 3
  • Review effort level: Low

Comment thread src/specify_cli/commands/bundle/__init__.py Outdated
Comment thread src/specify_cli/commands/bundle/__init__.py Outdated
Comment thread tests/contract/test_bundle_cli.py Outdated
@mnriem

mnriem commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback ;)

Address second-round Copilot review feedback on PR github#3136:

- Error message: when the download fails, report the original catalog
  download_url so the user knows which entry to fix; include the resolved
  REST API URL when it differs for easier debugging.
- ZIP detection: broaden the magic-bytes check from PK\x03\x04 to raw[:2]
  == b"PK", covering all valid ZIP variants (local-file header PK\x03\x04,
  empty-archive PK\x05\x06, spanned/split PK\x07\x08).
- Tests: remove the unused tmp_path parameter from
  test_bundle_info_resolves_github_browser_release_url_zip.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

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.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Low

# release URLs carry the filename), and falls back to the ZIP magic bytes
# (``PK`` prefix) for direct REST API asset URLs which have no file extension.
# All valid ZIP variants start with ``PK`` (local-file, empty-archive, split).
if url.lower().endswith(".zip") or raw[:2] == b"PK":
@mnriem

mnriem commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback. Almost there!

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.

3 participants