Credential memo omits its own inputs from the cache key

[ModeSevenIndustrialSolutions]

Summary

derive_gerrit_credentials is memoised on a key that omits every input it actually reads. The cached answer can therefore be wrong, and the failure mode is that a developer's personal SSH username and git email are served where the code asked for the organisation service account.

Found while auditing process-wide caches for #385 (see #393).

Detail

src/github2gerrit/ssh_config_parser.py:443:

@lru_cache(maxsize=32)
def derive_gerrit_credentials(
    gerrit_host: str, organization: str, gerrit_port: int = 29418
) -> tuple[str | None, str | None]:

The key is (gerrit_host, organization, gerrit_port). The body reads three things, none of which appear in that key:

Read	Source	In key?
_get_respect_user_ssh_setting() (line 473)	G2G_RESPECT_USER_SSH env var	no
get_ssh_user_for_gerrit(...) (line 476)	~/.ssh/config on disk	no
_get_cached_git_user_email() (line 482)	git config user.email subprocess	no

respect_user_ssh selects between two entirely different credential sources — the user's SSH config and git identity, or the {organization}.gh2gerrit service-account fallback. Caching across a change in that flag returns credentials from the wrong source.

The two helpers are themselves @lru_cache(maxsize=1) over zero-argument functions. With no arguments there is a single slot and no key at all, so they are global variables with a confusing spelling: they encode "the environment never changes after first read".

Demonstration

os.environ["G2G_RESPECT_USER_SSH"] = "true"
r1 = derive_gerrit_credentials("gerrit.example.org", "myorg")

# Flip the real input AND clear both inner caches, so only the outer memo remains
os.environ["G2G_RESPECT_USER_SSH"] = "false"
_get_respect_user_ssh_setting.cache_clear()
_get_cached_git_user_email.cache_clear()
r2 = derive_gerrit_credentials("gerrit.example.org", "myorg")

respect=true  -> ('modesevenindustrialsolutions', 'mwatkins@linuxfoundation.org')
respect=false -> ('modesevenindustrialsolutions', 'mwatkins@linuxfoundation.org')
identical: True
outer cache_info: CacheInfo(hits=1, misses=1, maxsize=32, currsize=1)

With respect_user_ssh=false the correct result is the org fallback — ('myorg.gh2gerrit', 'releng+myorg-gh2gerrit@linuxfoundation.org'). The memo returns personal credentials instead. Clearing the inner caches does not help, because the outer memo never re-enters the body.

The same shape applies to the inner helper on its own:

os.environ["G2G_RESPECT_USER_SSH"] = "false"
_get_respect_user_ssh_setting()          # warms the single slot
os.environ["G2G_RESPECT_USER_SSH"] = "true"

env now: true
memo still says respect_user_ssh = False

Is it live today? No — it is latent

This wants stating plainly, because it changes the priority.

cli.py:2329-2331 mutates the flag at runtime:

if not github_mode and not os.getenv("G2G_RESPECT_USER_SSH"):
    log.debug("Local execution detected, enabling user SSH config respect")
    os.environ["G2G_RESPECT_USER_SSH"] = "true"

That sits at line 2330 of _load_effective_inputs, and the only chain reaching the cache starts at line 2344 in the same function:

apply_parameter_derivation → derive_gerrit_parameters (config.py:717) → derive_gerrit_credentials (config.py:639) → _get_respect_user_ssh_setting (ssh_config_parser.py:473)

_get_respect_user_ssh_setting and _get_cached_git_user_email have no other callers in src/, and derive_gerrit_credentials has exactly one. So the first read happens after the mutation. Verified by instrumenting the call:

env value observed at each call: ['true']

So the ordering is correct today by coincidence of layout, not by construction.

Why fix it anyway

• It is one edit away from being live. Any new caller of derive_gerrit_credentials or either helper earlier in the process, or any reordering of _load_effective_inputs, silently produces wrong credentials with no error.
• Wrong credentials fail confusingly. The symptom is a Gerrit SSH rejection or a change pushed under the wrong identity, with nothing pointing at a cache.
• The performance case is thin. The whole memo saves one environment read and one git config subprocess — the chain runs at most twice per process (cli.py:2344 and cli.py:767).
• clear_credential_cache() exists purely to paper over this. Fix: Resolve open cache-isolation and config-provenance issues #393 wires it into an autouse test fixture; that fixture should not need to exist.

Suggested direction

Prefer resolve-once-and-pass over injecting an object around the same unsound memo:

1. Resolve G2G_RESPECT_USER_SSH, the SSH config user and the git email once, into an explicit credentials context.
2. Pass that context down instead of having leaf functions reach for global state.
3. Delete the three lru_cache decorators, and clear_credential_cache() with them.

If the memo is kept instead, every real input must enter the key — which in practice means the env flag plus a fingerprint of ~/.ssh/config, at which point resolving once is simpler and cheaper.

Related: _ssh_config_cache (line 280) keys on the ~/.ssh/config path, never its contents, so rewriting that file under an unchanged HOME serves the previous parse.

Acceptance criteria

• Credential derivation cannot return a value computed under a different G2G_RESPECT_USER_SSH, SSH config or git identity
• A test that changes each input and asserts the derived credentials change with it
• clear_credential_cache() and the autouse fixture added in Fix: Resolve open cache-isolation and config-provenance issues #393 removed, or justified as still necessary
• No behaviour change for the ordinary path: local runs still prefer the user's SSH identity, CI still uses the org service account