fix: close auth asymmetry between folder and file downloads#332
Open
danielmeppiel wants to merge 2 commits intomainfrom
Open
fix: close auth asymmetry between folder and file downloads#332danielmeppiel wants to merge 2 commits intomainfrom
danielmeppiel wants to merge 2 commits intomainfrom
Conversation
Folder-level deps used git clone (which inherits OS credential helpers), but single-file deps used the GitHub API with only env var tokens. This meant private repos worked without a PAT for folders but failed for individual files. Changes: - Add GH_TOKEN to modules token precedence (GITHUB_APM_PAT → GITHUB_TOKEN → GH_TOKEN) - Add git credential fill as last-resort token resolver in GitHubTokenManager, cached per host, 5s timeout, no interactive prompts - Wire credential fallback into _download_github_file() and _setup_git_environment() - Improve auth error messages to suggest 'gh auth login' as zero-config fix - Update authentication docs and CHANGELOG Closes #331 Related: #319 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes the authentication asymmetry between folder-level (git clone) and single-file (GitHub API) dependency downloads by adding a git credential-helper fallback for file downloads, plus recognizing GH_TOKEN for module access.
Changes:
- Add
GH_TOKENto module token precedence and introducegit credential fillfallback with per-host caching inGitHubTokenManager. - Wire the credential fallback into
GitHubPackageDownloaderso_download_github_file()can authenticate without requiring env var tokens. - Add/adjust tests and update docs + changelog to reflect the new behavior.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Updates locked apm-cli version metadata to 0.8.0. |
src/apm_cli/core/token_manager.py |
Adds GH_TOKEN precedence and git credential fill fallback + caching. |
src/apm_cli/deps/github_downloader.py |
Uses credential fallback for token resolution and improves auth error messaging. |
tests/test_token_manager.py |
New unit tests covering precedence, credential fill parsing, and caching. |
tests/test_github_downloader.py |
Adds integration tests for downloader credential fallback behavior. |
tests/test_github_downloader_token_precedence.py |
Updates existing tests to account for credential fallback behavior. |
docs/src/content/docs/getting-started/authentication.md |
Documents GH_TOKEN precedence and single-file credential fallback via git credential fill. |
CHANGELOG.md |
Adds Unreleased entries describing the auth fix and token precedence changes. |
| if not token: | ||
| error_msg += ( | ||
| "This might be a private repository. " | ||
| "Set GITHUB_APM_PAT or GITHUB_TOKEN, or run 'gh auth login' " |
src/apm_cli/core/token_manager.py
Outdated
| capture_output=True, | ||
| text=True, | ||
| timeout=5, | ||
| env={**os.environ, 'GIT_TERMINAL_PROMPT': '0'}, |
CHANGELOG.md
Outdated
Comment on lines
+13
to
+18
| - `git credential fill` fallback for single-file downloads from private repos — APM now discovers credentials from git credential helpers (macOS Keychain, `gh auth login`, Windows Credential Manager, etc.) when no `GITHUB_APM_PAT`/`GITHUB_TOKEN` is set, closing the auth asymmetry between folder and file downloads (#331, #319) | ||
|
|
||
| ### Changed | ||
|
|
||
| - `GH_TOKEN` environment variable is now recognized for APM module access, matching the token used by the GitHub CLI (#331) | ||
| - Improved authentication error messages to suggest `gh auth login` as a zero-config solution (#331) |
Comment on lines
+1175
to
+1187
| def test_credential_fill_for_non_default_host(self): | ||
| """Non-default hosts should try credential fill on demand in _download_github_file.""" | ||
| with patch.dict(os.environ, {}, clear=True), \ | ||
| patch( | ||
| 'apm_cli.core.token_manager.GitHubTokenManager.resolve_credential_from_git', | ||
| ) as mock_cred: | ||
| # Return None for default host, enterprise token for custom host | ||
| mock_cred.side_effect = lambda host: ( | ||
| 'enterprise-token' if host == 'ghes.company.com' else None | ||
| ) | ||
| downloader = GitHubPackageDownloader() | ||
| # No token for default host | ||
| assert downloader.github_token is None |
Comment on lines
19
to
+21
| When APM has a token for a recognized host (GitHub.com, GitHub Enterprise under `*.ghe.com`, or Azure DevOps), it injects it directly and disables interactive prompts. When no token is available, or the host is treated as generic (including GitHub Enterprise on custom domains), APM relaxes the git environment so your existing credential helpers — `gh auth`, macOS Keychain, Windows Credential Manager, `git-credential-store`, etc. — can provide credentials transparently. | ||
|
|
||
| For single-file downloads from GitHub (which use the GitHub API rather than `git clone`), APM also queries `git credential fill` as a last-resort fallback when no token environment variable is set. This means credentials stored by `gh auth login` or your OS keychain work for both folder-level and file-level dependencies. |
Comment on lines
+711
to
+713
| # for non-default hosts, try credential fill on demand. | ||
| token = self.github_token | ||
| if not token and host != default_host(): |
… deps Combines CDN fast-path (#322) with git credential fill fallback (#332): - raw.githubusercontent.com CDN first for unauthenticated github.com downloads - git credential fill fallback when no env token is set - GH_TOKEN recognition in token precedence - GitHub API rate-limit 403 detection (not misdiagnosed as auth failure) - Per-host token resolution for enterprise/custom domains - URL-encode refs in CDN URLs for branches with slashes - GIT_ASKPASS=echo hardening for non-interactive credential fill - Improved error messages suggesting gh auth login Closes #319, #331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
7 tasks
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| assert result == b'# Agent content' | ||
| call_url = mock_get.call_args[0][0] | ||
| assert not call_url.startswith("https://raw.githubusercontent.com/") | ||
| assert "github.mycompany.com" in call_url |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High test
Copilot Autofix
AI about 3 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the authentication asymmetry between folder-level and file-level dependency downloads from private repositories.
Root cause: Folder deps used
git clone(which inherits OS credential helpers like macOS Keychain,gh auth login, Windows Credential Manager), but single-file deps used the GitHub API with only env var tokens (GITHUB_APM_PAT/GITHUB_TOKEN). Private repos worked without a PAT for folders but failed for individual files.Changes
git credential fillfallbackresolve_credential_from_git(host)toGitHubTokenManager— runsgit credential fill(the same mechanismgit cloneuses internally) as a last-resort token resolverGIT_TERMINAL_PROMPT=0to prevent interactive prompts_setup_git_environment()and_download_github_file()so single-file downloads benefit from credential helpersGH_TOKENrecognitionGH_TOKENtoTOKEN_PRECEDENCE['modules']:GITHUB_APM_PAT → GITHUB_TOKEN → GH_TOKENgh)Improved error messages
gh auth loginas a zero-config solutionTests
test_token_manager.py(precedence, credential fill, caching)test_github_downloader.py(credential fallback integration)88 tests pass, 0 failures.
Closes #331 | Related: #319