[architect] SSH transport policy restated at 20 call sites: tests/shared/ssh_config.py is a single source of truth that 18 of them bypass

[kubestellar-hive]

Architecture Finding

Type: coupling / interface-violation (single-source-of-truth bypass)
Affected area: tests/shared/ssh_config.py vs. suite-local step modules across tests/*/features/steps/

tests/shared/ssh_config.py declares itself, in its own module docstring, as
"One source of truth for the SSH connection parameters used by the shared SSH steps,
by suite-local helpers that run commands on the VM, and by suite environment.py hooks
that probe the VM directly." It resolves connection identity through a documented
4-level precedence chain: behave context attributes → behave userdata
(-D vm_ip=…) → environment variables → defaults.

That contract is not honoured. Twenty call sites across the repo build a raw ssh
argument vector inline, and most of them read os.environ directly — skipping levels
1 and 2 of the precedence chain entirely:

$ grep -rln StrictHostKeyChecking --include=*.py tests | grep -v tests/unit
tests/shared/gnome_shell_steps.py            tests/smoke/features/steps/steps.py
tests/shared/image_cache.py                  tests/smoke/features/steps/system_health_steps.py
tests/shared/screenshot.py                   tests/smoke/features/steps/offline_boot_steps.py
tests/shared/ssh_steps.py                    tests/smoke/features/steps/gnome_apps_steps.py
tests/dx/features/steps/steps.py             tests/smoke/features/steps/gnome_extensions_steps.py
tests/flatcar/features/steps/steps.py        tests/smoke/features/steps/gnome_notifications_steps.py
tests/kde-smoke/features/steps/steps.py      tests/smoke/features/steps/display_scaling_steps.py
tests/software/features/environment.py       tests/smoke/features/steps/app_support.py
tests/software/features/steps/steps.py       tests/vanilla-gnome/features/steps/steps.py

Only two modules import the resolver (tests/software/features/{environment,steps/steps}.py,
plus tests/shared/image_cache.py). Everything else restates the policy.

Concrete consequences

1. Split-brain destination inside a single scenario. tests/smoke/features/environment.py:179
calls populate_ssh_context(context), so the shared steps star-imported into the smoke
suite honour behave -D vm_ip=…. The eight smoke-local step modules re-read
os.environ.get("VM_IP", "127.0.0.1") and ignore both the context and the userdata.
A run driven by userdata therefore sends shared steps to one host and smoke-local
steps to another, in the same scenario, with no error.

2. Silent port loss. tests/dx/features/steps/steps.py:18 (_ssh) and
tests/flatcar/features/steps/steps.py:107 build the argv with no -p flag at all,
and neither suite's environment.py sets ssh_port. Under a port-forwarded QEMU lane
those suites connect to port 22 regardless of SSH_PORT.

3. Policy drift. The canonical run_ssh (tests/shared/ssh_steps.py:15) sets
LogLevel=ERROR; the ten smoke/kde/software copies do not, so ssh's own diagnostics
leak into captured output there but not elsewhere. tests/vanilla-gnome/features/steps/steps.py:144
passes ConnectTimeout={timeout}, conflating the connect deadline with the command
deadline — a 900s long-command timeout becomes a 900s connect timeout.

4. The intended contract is already enforced — for exactly one function.
tests/unit/test_ssh_config.py:173 asserts _flatpak() "must use tests.shared.ssh_config,
not private env reads". That guard was never generalised, so the other eighteen sites
regressed unchecked.

Impact

Connection identity has no single owner, so every new lane (non-default SSH port,
userdata-driven host, alternate user) has to be re-plumbed into ~20 places or it
silently half-works. Failures surface as connection refusals or wrong-host assertions
attributed to the image under test rather than to the harness — the most expensive
possible failure mode for a test suite whose whole job is to be trustworthy.

Recommendation

Extract the transport policy into ssh_config.ssh_argv() and migrate every raw builder
onto it, in reviewable phases:

• Phase 1 (PR below): add ssh_argv(); migrate the smoke suite (8 modules) and
  tests/shared/gnome_shell_steps.py; add a contract test that fails if any migrated
  module regrows a private copy. Byte-identical argv, no behaviour change.
• Phase 2: migrate tests/dx, tests/flatcar, tests/kde-smoke, tests/vanilla-gnome,
  tests/software, tests/shared/{image_cache,screenshot}.py, and fold run_ssh itself
  onto ssh_argv(). This phase does change behaviour (dx/flatcar gain -p,
  vanilla-gnome stops conflating the two timeouts) and needs a green e2e lane.
• Phase 3: widen the contract test's allowlist to the whole repo so StrictHostKeyChecking
  may appear only in tests/shared/ssh_config.py.

tests/smoke/features/steps/offline_boot_steps.py is deliberately excluded from phase 1
because PR #768 is adding unit coverage for it.

---

Filed by architect agent (ACMM L5 — hold-gated mode)

— hive: agent=architect backend=copilot model=claude-opus-5