refactor(auth): centralize indexed Git config cleanup

[Pybsama]

refactor(auth): centralize indexed Git config cleanup

TL;DR

Centralize ownership of indexed GIT_CONFIG_* authentication-header cleanup in
apm_cli.utils.git_env.

This removes two subtly different cleanup implementations from the authentication
resolver and the GitHub-host helper.

The shared owner now:

• identifies only http.*.extraheader entries whose value contains an actual
  Authorization: header field;
• preserves proxy, TLS, CA, and other unrelated Git configuration;
• handles LF, CRLF, and CR-separated header values;
• compacts retained entries into a stable contiguous sequence;
• removes orphaned indexed keys left by stale or malformed counts.

Fixes #2398.

Problem

APM has two consumers that need to remove stale Git authentication headers before
constructing a child-process environment:

• AuthResolver._clear_git_auth_env;
• set_authorization_header_git_env.

Each consumer previously implemented its own scan, filtering, and reindexing.

The two implementations did not agree on what counted as an authentication entry.
One checked whether the entire value started with authorization: while the other
removed any value containing the substring authorization.

That split ownership created two concrete risks:

1. a multiline extraheader value could retain a stale credential when its
   Authorization field was not first;
2. an unrelated value containing text such as x-authorization-mode could be
   discarded even though it was not an Authorization header.

Because the environment is inherited by Git subprocesses, both false negatives and
false positives matter: stale credentials are a security boundary, while removed
proxy or TLS configuration can make a valid clone fail.

Approach

Introduce one canonical helper that owns the indexed representation and make both
call sites delegate to it.

Concern	Decision
Header recognition	Parse line boundaries and match the exact Authorization field name
Line endings	Accept LF, CRLF, and CR
Whitespace	Allow optional horizontal whitespace around the field name and colon
Key scope	Restrict removal to http.*.extraheader entries
Retention	Preserve every non-authentication entry without modifying its value
Reindexing	Rewrite retained pairs from index zero in their original order
Stale metadata	Remove indexed keys that are not represented by a trustworthy count
Consumer behavior	Keep direct credential channels local to the resolver

The helper stages the new indexed state before mutating the environment. This
avoids leaving a partially rewritten sequence if the input contains malformed
count metadata.

Implementation

Canonical owner

src/apm_cli/utils/git_env.py
now provides:

• _is_git_auth_config_entry(key, value) for exact field recognition;
• strip_git_auth_config_entries(env) for filtering, cleanup, and stable
  compaction.

The function returns the next free index so a caller that needs to append a new
header can do so without recomputing the sequence.

It treats a missing, negative, non-integer, or otherwise stale count defensively
and still inspects actual indexed key slots present in the environment.

Consumers

AuthResolver._clear_git_auth_env
delegates indexed cleanup to the shared helper,
then continues to clear the resolver-owned direct credential channels.

set_authorization_header_git_env
rejects CR/LF in the new token, delegates stale
indexed cleanup, and appends exactly one fresh Authorization header at the returned
index.

Its contract now explicitly describes that sole indexed Git authentication header.

Ownership guard

scripts/check_git_auth_config_owner.py
establishes the helper as the only
production owner of indexed Git authentication cleanup.

The architecture lint invokes this checker as AC20.

The checker understands relevant syntax rather than relying on a broad text search,
so comments, fixtures, and approved helper implementation details do not create
false positives.

The owner declaration is mirrored in .apm, .github, and the lock file so the
published and repository-local architecture guidance remain aligned.

Tests

The change adds focused tests for:

• exact header-field matching;
• LF, CRLF, and CR multiline values;
• optional horizontal whitespace;
• safe values that merely contain authorization-related text;
• malformed and stale count metadata;
• orphan-key cleanup;
• stable reindexing and return values;
• resolver delegation and direct-channel cleanup;
• setter replacement and injection rejection;
• the architecture owner boundary.

Diagrams

flowchart LR
    subgraph Consumers["Consumers"]
        C1["AuthResolver._clear_git_auth_env"]
        C2["set_authorization_header_git_env"]
    end

    subgraph Owner["Canonical owner"]
        O1["strip_git_auth_config_entries"]:::new
        O2["_is_git_auth_config_entry"]:::new
    end

    subgraph Result["Indexed environment"]
        R1["Stable retained entries"]
        R2["Fresh header appended by setter"]
    end

    C1 --> O1
    C2 --> O1
    O1 --> O2
    O1 --> R1
    O1 --> C2
    C2 --> R2

    classDef new stroke-dasharray: 5 5

Loading

The dashed nodes are introduced by this change. Existing consumers now share the
same filtering and compaction path before continuing with their own responsibilities.

Trade-offs

• Exact header-field matching is deliberately narrower than a substring search.
  This preserves vendor-neutral configuration while still rejecting a real
  Authorization field anywhere in a multiline value.
• The helper scans the indexed slots actually present in the environment in
  addition to a valid declared range. This makes stale metadata recoverable without
  trusting an arbitrarily large count.
• Direct credential variables remain resolver-owned. Moving those channels into
  the indexed-config utility would broaden its contract beyond issue Extract shared retain/reindex predicate for git auth-config entries (follow-up to #2368) #2398.
• The architecture checker adds a small maintenance cost, but turns an otherwise
  informal ownership rule into an executable boundary.

Benefits

• One implementation defines the security-sensitive cleanup behavior.
• Both credential paths preserve unrelated proxy, TLS, CA, and hardening settings.
• Multiline stale Authorization fields cannot bypass cleanup.
• Safe values containing authorization-related words are no longer over-filtered.
• Future duplicate cleanup loops fail the architecture check before review.
• Focused regression tests describe the retained and removed cases independently.

Validation

Automated checks

• Focused Git-auth test selection:
  119 passed in 0.40s
• Full unit suite:
  19098 passed, 2 skipped, 21 xfailed, 19 warnings, 87 subtests passed in 162.63s
• Ruff check:
  All checks passed!
• Ruff format:
  1550 files already formatted
• Pylint:
  Your code has been rated at 10.00/10
• Authentication-signal lint:
  [+] auth-signal lint clean
• Architecture lint AC1-AC20:
  [+] architecture boundary lint clean
• Package build:
  source distribution and wheel both built successfully with uv build
• Patch hygiene:
  git diff --check completed with no output

Mutation evidence

Deliberate regression	Detection
Replace exact field matching with broad substring matching	5 focused tests fail
Change extraheader key scope from OR to AND	6 focused tests fail
Reintroduce a consumer-owned cleanup loop	AC20 fails

All mutations were temporary and the correct implementation was restored before
the full validation run.

Scenario Evidence

Scenario	Principle demonstrated	Test level	Regression trap
A bearer retry inherits corporate proxy and CA entries while stale auth is removed	Secure by default; vendor-neutral behavior	Unit	Resolver preservation test for #2398
The setter replaces an old indexed header and retains hardening entries	Secure by default; predictable developer experience	Unit	Setter replacement and ordering tests
An Authorization field follows another field in one multiline value	Secure by default	Unit	LF, CRLF, and CR multiline tests
A second production consumer attempts to own indexed cleanup	Community-maintainable architecture	Integration lint	AC20 mutation test

How to test

From the repository root:

1. Run the focused regression set:
   uv run pytest tests/unit/test_git_env.py tests/unit/test_auth.py tests/unit/test_github_host.py -q.
2. Run the full suite:
   uv run pytest tests/unit -q.
3. Run formatting and lint checks:
   uv run ruff check . and uv run ruff format --check ..
4. Run the repository authentication and architecture lint scripts.
5. Build both package artifacts with uv build.

Repository checklist

• Maintenance/refactor change
• Tested locally
• Existing tests pass
• New regression tests added
• OpenAPM specification change: N/A
• Architecture ownership metadata synchronized
• Changelog entry references refactor(auth): centralize indexed Git config cleanup #2411
• No new runtime dependency

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com