feat(cargo-aprz): discover tokens from GitHub CLI

[martin-kolinek]

🤖 Adds explicit, host-aware GitHub CLI credential discovery to cargo-aprz without invoking a developer's gh CLI by default.

• adds visible --github-token-from-gh to both crates and deps
• resolves credentials as --github-token, nonblank GITHUB_TOKEN, explicitly enabled gh auth token --hostname <host>, then anonymous access
• returns before hostname, PATH, or process discovery when the switch is absent
• uses the effective GitHub/GitHub Enterprise host when enabled
• resolves gh only through explicit PATH entries, accepts only direct executable images on Windows, suppresses stderr, redacts secrets, and enforces a ten-second timeout with child termination
• treats blank environment tokens as absent and removes them from the gh child environment
• verifies the production process/provider path against a mock Enterprise endpoint without mutating the parent process environment

This PR intentionally contains no cargo-anvil recipes, container integration, or generated Anvil changes. Adoption is deferred until a cargo-aprz release includes this capability. Codeberg credentials are unchanged.

[Copilot]

🟡 Changes recommended

Moderate issues remain around Windows batch-shim launching and production process-path test coverage.

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

Pull request overview

Adds host-aware GitHub CLI token discovery to cargo-aprz, preserving credential precedence and Codeberg behavior.

Changes:

• Adds asynchronous gh auth token fallback with Enterprise host support.
• Updates credential integration, redaction, documentation, and test fixtures.
• Resolves CLI credentials without logging or caching tokens.

File summaries

File	Summary
crates/cargo-aprz/src/main.rs	Updates CLI credential documentation.
crates/cargo-aprz/README.md	Documents credential discovery behavior.
crates/cargo-aprz/docs/DESIGN.md	Defines credential resolution and security guarantees.
crates/cargo-aprz-lib/tests/support/mod.rs	Supplies deterministic test credentials.
crates/cargo-aprz-lib/tests/process_globals_integration.rs	Updates process-global integration arguments.
crates/cargo-aprz-lib/tests/hosting_provider_integration.rs	Tests credential scoping and redaction.
crates/cargo-aprz-lib/src/commands/mod.rs	Registers the credentials module.
crates/cargo-aprz-lib/src/commands/github_credentials.rs	Implements token discovery and PATH resolution.
crates/cargo-aprz-lib/src/commands/common.rs	Integrates resolved GitHub credentials.

Review details

Suppressed comments (1)

crates/cargo-aprz-lib/src/commands/github_credentials.rs:201

• The resolver includes .BAT and .CMD from the default PATHEXT, but query_gh launches the resolved path directly with tokio::process::Command (lines 145-150) and deliberately does not use a shell. On Windows, a batch shim cannot be started by CreateProcess this way, so a PATH that selects gh.cmd/gh.bat will be treated as an unavailable GitHub CLI. Either exclude batch extensions from direct resolution or add a dedicated, safely handled cmd.exe path if batch shims are intended to work.

    const DEFAULT_PATH_EXT: &str = ".COM;.EXE;.BAT;.CMD";

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

---

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

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

❌ Patch coverage is 93.72822% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.7%. Comparing base (1910173) to head (a0cfb45).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
.../cargo-aprz-lib/src/commands/github_credentials.rs	93.5%	36 Missing ⚠️

❌ Your project status has failed because the head coverage (99.7%) is below the target coverage (100.0%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #175      +/-   ##
========================================
+ Coverage   97.6%   99.7%    +2.1%     
========================================
  Files        304      77     -227     
  Lines      69683   14571   -55112     
========================================
- Hits       68016   14533   -53483     
+ Misses      1667      38    -1629     

Flag	Coverage Δ
linux	99.7% <93.9%> (+2.1%)	⬆️
linux-arm	99.7% <93.9%> (+2.1%)	⬆️
scheduled	?
windows	99.7% <93.2%> (+1.9%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

🟡 Changes recommended

Container credential forwarding and invalid credential fallback issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

crates/cargo-aprz-lib/src/commands/github_credentials.rs:103

• When GITHUB_TOKEN is present but cannot be converted to UTF-8, this returns immediately and suppresses the gh fallback. That value cannot be used as an HTTP credential, so it should be treated like an unusable/missing environment value and continue to the host-aware lookup; otherwise an invalid environment entry prevents the documented credential precedence from reaching gh.

        let Ok(token) = token.into_string() else {
            log::trace!(
                target: LOG_TARGET,
                "GitHub credential source {GITHUB_TOKEN_ENV} is not valid UTF-8; using anonymous access"
            );
            return None;

crates/cargo-aprz-lib/src/commands/github_credentials.rs:95

• This early return makes a blank or whitespace-only --github-token authoritative, so the client receives an unusable token credential instead of continuing to GITHUB_TOKEN, gh, or anonymous access. The new CLI documentation says blank credentials are ignored, so apply the same trim-and-empty check used for the environment value here.

    if let Some(token) = explicit {
        log::trace!(target: LOG_TARGET, "GitHub credential source: --github-token");
        return Some(token.clone());
    }

• Files reviewed: 17/18 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Two moderate credential-discovery and fallback-warning issues remain unresolved.

Review details

Suppressed comments (2)

crates/cargo-anvil/templates/justfiles/anvil/checks/aprz.just:26

• With both credentials unavailable, this recipe now only performs the no-op and then runs anonymously, but it no longer emits the actionable warning that the previous recipe showed. CommonArgs defaults logging to none (crates/cargo-aprz-lib/src/commands/common.rs:107-109), while the new discovery diagnostics are trace-level, so a native full-workspace run can exhaust the 60-request quota described above and wait without telling the user to run gh auth login or set GITHUB_TOKEN. Preserve an equivalent visible warning for the anonymous fallback, outside the disabled logger.

    # Load-bearing no-op: the container driver scans the executable dry-run
    # plan for this variable before deriving a token for its gh-less image.
    $null = $env:GITHUB_TOKEN

crates/cargo-aprz-lib/src/commands/github_credentials.rs:102

• This branch makes an invalid-UTF-8 GITHUB_TOKEN a terminal anonymous decision. Such a value is not a usable nonblank token, so a valid gh login is skipped even though the documented precedence says the CLI fallback applies when no usable environment token is available. Treat this case like a missing/blank value and continue to hostname-aware discovery.

        let Ok(token) = token.into_string() else {
            log::trace!(
                target: LOG_TARGET,
                "GitHub credential source {GITHUB_TOKEN_ENV} is not valid UTF-8; using anonymous access"
            );

• Files reviewed: 17/18 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Two moderate credential-discovery issues remain in output validation and whitespace-only GITHUB_TOKEN handling.

Review details

Suppressed comments (2)

crates/cargo-aprz-lib/src/commands/github_credentials.rs:153

• Only non-empty UTF-8 is checked here, so a successful gh wrapper that emits a multi-line or control-character value is returned as a credential. Client::new then builds the Authorization header from it (crates/cargo-aprz-lib/src/facts/hosting/client.rs:96-108) and propagates the header-parse error through Common::new, making this best-effort fallback abort instead of continuing anonymously. Validate the candidate as a legal header value before returning it.

    let token = stdout.trim();
    if token.is_empty() {

crates/cargo-aprz-lib/src/commands/github_credentials.rs:225

• The whitespace-only GITHUB_TOKEN branch deliberately falls through to gh, but this Command inherits that original environment because it never removes the variable. GitHub CLI treats any non-empty GITHUB_TOKEN (including whitespace) as higher priority than its stored host credential, so gh auth token can echo the whitespace, which the later trim() turns into anonymous access instead of the saved login. Remove GITHUB_TOKEN from this child environment so blank values truly fall through to the CLI login.

    let child = Command::new(executable)
        .args(args)
        .stdin(stdin.into_stdio())
        .stdout(stdout.into_stdio())
        .stderr(stderr.into_stdio())

• Files reviewed: 17/18 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Invalid UTF-8 GITHUB_TOKEN values currently prevent the documented gh fallback.

Review details

Suppressed comments (1)

crates/cargo-aprz-lib/src/commands/github_credentials.rs:111

• When GITHUB_TOKEN is present as an invalid UTF-8 OsString, this branch returns immediately and skips the explicitly requested gh fallback. That contradicts the documented precedence of using gh when no usable/nonblank environment token is available; a valid gh login is silently replaced by anonymous access. Treat the invalid environment value as absent and continue to the opt-in discovery path.

    if let Some(token) = read_environment() {
        let Ok(token) = token.into_string() else {
            log::trace!(
                target: LOG_TARGET,
                "GitHub credential source {GITHUB_TOKEN_ENV} is not valid UTF-8; using anonymous access"
            );
            return None;

• Files reviewed: 21/22 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Critical host-selection and moderate process-tree termination issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

crates/cargo-aprz-lib/src/commands/github_credentials.rs:245

• The native timeout only drops wait_with_output() and relies on kill_on_drop(true), which terminates the gh child but does not apply the container implementation's recursive process-tree termination ($process.Kill($true)). A gh executable that has spawned a helper can therefore leave that descendant running after the ten-second deadline, contrary to the process-tree termination contract described for this feature. Use a platform-appropriate process-group/tree termination strategy for the native path, or narrow the contract and add coverage for the intended behavior.

• Files reviewed: 23/24 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Address the non-executable PATH candidate handling and terminate descendant processes after native timeouts.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

crates/cargo-aprz-lib/src/commands/github_credentials.rs:245

• The native timeout only drops Child after wait_with_output expires; .kill_on_drop(true) addresses that child, while this path has no process-group/job-object handling like the container implementation's Kill($true). A gh executable that spawns a descendant can therefore leave that descendant running (and holding inherited stdout/stderr resources) after the documented ten-second deadline. Terminate the complete process tree using a platform-appropriate process-group/job mechanism, and add a descendant-survival assertion to the native timeout test.

crates/cargo-anvil/templates/justfiles/anvil/container.just:982

• Resolve-AnvilGhExecutable returns the first regular gh file without checking whether it is executable. If an earlier explicit PATH entry contains a non-executable file and a later entry contains the real executable, Process.Start() fails for the first path and the wrapper falls through anonymously instead of continuing PATH lookup; the native resolver avoids this by filtering executability before selecting a candidate (crates/cargo-aprz-lib/src/commands/github_credentials.rs:277-289). Check executability before returning, or continue scanning after a failed candidate, so a shadowing non-executable file does not suppress a valid later gh.

            foreach ($name in $names) {
                $candidate = Join-Path $directory $name
                if (-not (Test-Path -LiteralPath $candidate -PathType Leaf)) { continue }
                # Process.Start below is the portable executable check. A Unix
                # file without execute permission fails there and falls through
                # anonymously without requiring an external `test` utility.
                return [IO.Path]::GetFullPath($candidate)

• Files reviewed: 23/24 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Handle invalid UTF-8 environment tokens before falling back to gh, and clarify deferred container behavior in the design documentation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

crates/cargo-aprz/docs/DESIGN.md:117

• This paragraph documents a container-side implementation that is not part of this PR: the repository has no container credential-derivation path for this new switch, and the PR description explicitly defers container integration. As written, the design contract claims forwarding APRZ_GITHUB_URL, token propagation, and process-tree termination already apply; remove this paragraph or clearly mark it as a future/deferred integration so the design does not promise behavior that this release does not provide.

Containerized opt-in applies the same process contract on the host because the
generated image does not contain `gh`: absolute resolution from explicit,
nonempty `PATH` entries; direct executable images only on Windows and regular
executable files on Unix; direct argument-vector launch with stdin and stderr
suppressed and strict UTF-8 stdout captured; the rejected blank environment
token removed; and a ten-second deadline that terminates the complete process
tree. Missing, unsuccessful, timed-out, blank, and invalid-output lookups all
continue anonymously. The container driver derives the same effective hostname
and forwards `APRZ_GITHUB_URL` with a discovered token so the inner provider
cannot target a different endpoint.

• Files reviewed: 11/11 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

No blocking issues were identified; the remaining documentation concern is a minor nit.

Review details

Suppressed comments (1)

crates/cargo-aprz/docs/DESIGN.md:119

• This section documents a containerized gh lookup contract, but this PR deliberately contains no container or cargo-anvil integration, and the changed code provides only the host-side cargo-aprz path. As written, the design doc claims behavior that users of the current release cannot obtain and conflicts with the PR's deferred-adoption scope; remove this paragraph or explicitly mark it as deferred until the container integration lands.

Containerized opt-in applies the same process contract on the host because the
generated image does not contain `gh`: absolute resolution from explicit,
nonempty `PATH` entries; direct executable images only on Windows and regular
executable files on Unix; direct argument-vector launch with stdin and stderr
suppressed and strict UTF-8 stdout captured; the rejected blank environment
token removed; and a ten-second deadline that terminates the complete process
tree. Missing, unsuccessful, timed-out, blank, and invalid-output lookups all
continue anonymously. The container driver derives the same effective hostname
and forwards `APRZ_GITHUB_URL` with a discovered token so the inner provider
cannot target a different endpoint.

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

No blocking issues were identified; one documentation nit remains.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

crates/cargo-aprz/docs/DESIGN.md:114

• This section documents a container-side implementation (host PATH resolution, process-tree termination, and forwarding APRZ_GITHUB_URL) that is not present in this PR; the PR explicitly defers container integration. In its current form the design document claims behavior users cannot get from this change. Move this contract to the deferred container-adoption change, or clearly label it as a future requirement rather than an implemented behavior.

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Absolute PATH entries should remain usable when the current directory is unavailable.

Review details

Suppressed comments (1)

crates/cargo-aprz-lib/src/commands/github_credentials.rs:266

• When the process current directory is unavailable (for example, the directory was removed after startup), this early return prevents resolution even if PATH contains an absolute executable directory. That contradicts the explicit-PATH behavior documented above and makes an otherwise valid gh fallback silently become anonymous; only require current_dir when resolving relative PATH entries, while allowing absolute entries to be checked.

    let current_dir = std::env::current_dir().ok()?;

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

No unresolved review issues were identified.

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite