feat(cli): add apm auth for git host credentials (#2788)

[Joni Oliveira (jonioliveira)]

Draft implementation of the apm auth command proposed in #2788.

This is a draft, not a review request. The questions in #2788 are still open — in particular whether this belongs as its own verb or folded into apm doctor, and whether the gap is one worth closing at all. I opened this so there is running code to judge rather than a description of it. If the answer is apm doctor, or no, closing this costs nothing.

What it does

Answers three questions about one host, and nothing else:

$ apm auth github.com
[+] github.com: using credential from GH_TOKEN.
[i] Add --check to validate it against the API.

$ apm auth gitlab.com --check
[!] gitlab.com: the credential from GITLAB_TOKEN was rejected (HTTP 401).
    An OAuth session token (e.g. from 'glab auth login') works for git but
    not for the REST API -- you need a personal access token.

$ eval "$(apm auth gitlab.com --export)"

1. Which credential does APM resolve, and from where — names the source, so TOKEN_PRECEDENCE becomes observable rather than documented.
2. Does it work (--check) — validates against the REST API, since a token git clone accepts is not automatically one the API accepts.
3. If there is none — opens the host's token page with name and scopes prefilled, accepts a paste.

Scope

Additive. No changes to AuthResolver, classify_host, or token_manager, so it should not interact with the auth work in flight (#2681, #2673, #2610).

File
src/apm_cli/commands/auth.py	the command
src/apm_cli/cli.py	+3 lines, registration
docs/src/content/docs/reference/cli/auth.md	reference page
docs/src/content/docs/reference/index.md	+1 line, index entry
tests/unit/test_auth_command.py	37 tests

Design notes

It does not save the token. AuthResolver reads only env vars and credential helpers, so writing to ~/.apm/config.json would be inert — APM would never read it back. It prints the export line instead.

--export reserves stdout for the export line alone, so eval "$(...)" is safe. That needs two layers: sys.stdout redirected to stderr for narration, and fd 1 redirected, because subprocesses (webbrowser.open, gh auth token, git credential fill) inherit fd 1 and would otherwise write into the caller's eval. There is a regression test using a real subprocess — CliRunner captures sys.stdout and structurally cannot see fd 1.

--check is tri-state, not pass/fail. Rejected (401, or 403 on a normal PAT) advises minting a new token. Unreachable, 5xx, or a GitHub App installation token keeps the credential and exits 0. This matters in CI: Actions' GITHUB_TOKEN is a ghs_ token with no user context, so GET /user answers 403 even though it reads repository contents fine — a pre-flight must not fail that build.

It never mutates a credential store. A shadowing macOS keychain entry is reported, never erased, and only when a credential helper was actually consulted — env vars are resolved first, so a keychain entry cannot be shadowing anything when the token came from GITHUB_APM_PAT.

Hosts that classify as generic exit non-zero, with an error naming GITHUB_HOST / GITLAB_HOST as the fix. I deliberately did not teach classify_host to infer GHES from a bare hostname — it is a shared authority consumed by install, marketplace, and deps, and #2594's transport policy suggests routing generic hosts through native helpers without platform tokens is intended behavior rather than a gap.

Testing

37 unit tests. Full unit suite passes on this branch (21081 passed); lint, format, and scripts/lint-architecture-boundaries.sh are clean. Rebased on current main.

One thing that stands alone

GH_TOKEN is in TOKEN_PRECEDENCE["modules"] but was missing from the documented resolution order, so a user with only GH_TOKEN set reads the docs as saying it is unused. Fixed here in all three places it is stated, with a test deriving the expected vars from TOKEN_PRECEDENCE so they cannot drift again. Happy to split that into its own PR regardless of what happens to the rest.

Closes #2788 if merged.

[Joni Oliveira (jonioliveira)]

@microsoft-github-policy-service agree company="supermodular.ai"

[Copilot]

🟡 Changes recommended

There are CI-relevant test URL assertion patterns and a shell-snippet quoting issue that should be fixed to satisfy repository security/testing conventions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a new apm auth CLI command to surface which credential APM resolves for a given git host, optionally validate it against the host REST API, and support a shell-safe --export mode for eval.

Changes:

• Introduces src/apm_cli/commands/auth.py implementing apm auth with --check, --export, and interactive remediation.
• Wires the new command into the CLI entrypoint and adds a reference docs page + index entry.
• Adds a comprehensive unit test suite covering resolution, tri-state checking behavior, interactive prompting, and export-mode stdout isolation.

File summaries

File	Description
src/apm_cli/commands/auth.py	New apm auth command implementation, including token checking and export-safe output handling.
src/apm_cli/cli.py	Registers the auth command and adds it to the CLI epilog workflow list.
docs/src/content/docs/reference/cli/auth.md	New CLI reference page documenting usage, options, and behavior.
docs/src/content/docs/reference/index.md	Adds auth to the reference index table.
tests/unit/test_auth_command.py	New unit tests covering host mapping, check semantics, export-mode behavior, and doc/help drift prevention.

Review details

Suppressed comments (2)

tests/unit/test_auth_command.py:90

• This test currently asserts the request URL via raw substring checks. Parse the URL and assert on hostname/path instead to align with the repo’s URL-assertion convention.

    def test_ghes_uses_the_enterprise_api_base(self):
        with patch("requests.get") as get:
            get.return_value = MagicMock(status_code=200)
            check_token("ghp_x", "ghe.corp.example", "ghes")
        assert "api.github.com" not in get.call_args[0][0]

tests/unit/test_auth_command.py:70

• Avoid substring assertions against request URLs in tests; parse the called URL and assert on components (hostname/path) to satisfy the repo’s URL-assertion contract and avoid CodeQL substring-url findings.

        assert get.call_args.kwargs["headers"]["Authorization"] == "token ghp_x"
        assert "api.github.com/user" in get.call_args[0][0]

• Files reviewed: 5/5 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Joni Oliveira (jonioliveira)]

Fixed in 72f78bf — the host is now a printf argument via shlex.quote rather than interpolated into the format string.

Worth noting for the record that a quote-bearing host can't actually reach this line: AuthResolver.classify_host matches self-managed hosts exactly against GITHUB_HOST / GITLAB_HOST / APM_GITLAB_HOSTS, so git.corp'x.com lands in generic and run_auth returns 1 at the if not env_var guard, well before the shadowing warning. So this wasn't reachable in practice.

Fixing it anyway, because the shape you're pointing at is the right one for a second independent reason: % and \ are interpreted only in printf's format, not in its arguments. Moving the host out of the format removes a caller-influenced value from a position where escape sequences are live. And shlex.quote("github.com") returns the string bare, so the common-case snippet a user copies is unchanged:

printf 'protocol=https\nhost=%s\n\n' github.com | git credential-osxkeychain erase

Verified by execution that both a plain and a quote-bearing host produce exactly the protocol=https\nhost=<host>\n\n payload git credential-osxkeychain erase expects.

[Joni Oliveira (jonioliveira)]

Fixed in 72f78bf, and swept wider than the three lines flagged here.

.github/instructions/tests.instructions.md is explicit that this fails CI via py/incomplete-url-substring-sanitization regardless of whether the asserted value is bounded, so I converted every URL assertion in the file rather than just the inline-flagged ones — including the two listed as suppressed comments (lines 70 and 90), which would otherwise have come back on the next review.

The parsed assertions are also strictly stronger than the substrings they replace, which is the part I'd flag as the real win:

• assert "scopes=repo" in url passes for scopes=repository. parse_qs(url.query)["scopes"] == ["repo"] does not.
• assert "github.com" not in url passes for a look-alike such as github.com.attacker.example. url.hostname == "ghe.corp.example" does not.
• assert "api.github.com" not in get.call_args[0][0] only asserted an absence; it now pins /api/v3/user on the enterprise host positively, so a GHES request silently routed to a third host would fail the test.

I used direct urlparse component assertions rather than the _printed_urls helper from tests/unit/test_mcp_command.py: that helper extracts URLs from a printed text blob, whereas these tests already hold a single bare URL, and the instructions doc sanctions component-level checks for exactly that case. Happy to switch to the helper if you'd rather have one idiom across the suite.

[Joni Oliveira (jonioliveira)]

Fixed in 72f78bf — confirmed as a real gap, and reproduced before fixing.

ghe.corp.example is the exact value the docs tell users to export as GITHUB_HOST, so this was well-placed. With it set, the test fails on the assertion rather than on anything to do with the code:

$ GITHUB_HOST=ghe.corp.example uv run --extra dev pytest tests/unit/test_auth_command.py -q
FAILED test_generic_host_error_names_the_env_var_to_set
E  assert 'GITHUB_HOST=ghe.corp.example' in "... Set GITHUB_APM_PAT to a token with scopes 'repo'."

The host classifies as ghes instead of generic, so the command takes the success path and never emits the error the test is about.

The vars are now cleared for that one invocation via CliRunner.invoke(env={...: None}), matching the existing use in tests/unit/test_outdated_phase3w5.py. Scoping it to the invocation keeps the reason legible at the call site — the test asserts a generic classification, which only holds while no env hint claims the host.

I checked whether the file's other runner.invoke calls share the exposure and they don't: github.com / gitlab.com resolve by built-in rules that take precedence over the env hints, so the suite still passes under GITHUB_HOST=gitlab.com APM_GITLAB_HOSTS=github.com. Only the generic case was environment-dependent, so I left the rest alone rather than adding blanket clearing.