Pass environment-derived values into the resolvers their tests steer (#806)

[sehkone]

Summary

Test code in this crate mutated the process environment to steer production functions that read it, serialised behind shared mutexes. In Rust 2024 set_var/remove_var are unsafe, and that lock does not discharge the contract: it serialises only the code that takes the lock, while the tokio runtime, the HTTP client, the env-filtered subscriber and the libc resolver all read the environment from threads that hold nothing.

Every resolver a test steered now takes the value as a parameter, leaving one thin wrapper at the composition root that reads the environment. The variables consulted, their precedence and their fallbacks are unchanged; what moves is where they are read.

• POSTGRES_* — resolve_db_dsn_for_init and resolve_db_dsn receive the six values; init::steps::orchestrator reads them straight after load_dotenv_into_env promotes the compose .env, preserving the precedence the builders rely on.
• COMPOSE_PROJECT_NAME — compose_project_name_override takes the override, and both ComposeIdentity::resolve and ComposeIdentity::resolve_for_dir gain an override-taking sibling, so the compose-file resolver test is no longer steered by whatever the invoking shell exported (the E2E harness exports that variable for a whole scenario).
• OPENBAO_ROOT_TOKEN — resolve_root_token takes the token.
• *_HOST_PORT — host_port::resolve_host_port, db::resolve_postgres_host_port, the OpenBao URL resolver and rotate::db::resolve_db_admin_dsn take the port. resolve_db_admin_dsn took compose_dir only to derive that port, so the port replaces it; the ca.json fixture that existed to prove no fallthrough went with the directory, since the signature is now what rules it out.
• load_dotenv_into_env — split into a pure dotenv_pairs_to_apply, which decides which pairs apply (including the COMPOSE_PROJECT_NAME exclusion), and the thin wrapper that keeps the single production set_var with its // SAFETY: comment unchanged.

Deleted as their last users went: compose_project::test_env (ENV_LOCK, env_lock, ScopedEnv, ComposeProjectEnv), openbao_auth's ENV_LOCK/env_lock, init::steps::test_support's ENV_LOCK/env_lock, and the three anonymous OnceLock<Mutex<()>> guards in host_port.rs, openbao_url.rs and db.rs. The rest of init::steps::test_support (default_init_args, test_messages, test_cert_pem) and all of rotate::test_support and verify.rs's ENV_LOCK are untouched — they belong to other issues in this effort.

reinit's scope-check test no longer calls set_current_dir. The working directory is process-global exactly as the environment is, and the . it was reproducing is a property of compose_file_dir, so the test passes the directory in.

Three parameters ended up threaded but steering nothing — status_openbao_url_with_env, init_args_for_reinit's openbao_host_port_env, and every field of HostPortEnv bar postgres could each be replaced with None with the whole suite still green. Each now has an assertion that fails when the value does not reach its resolver.

No new unsafe block, no replacement lock, no --test-threads=1.

Closes #806

Test plan

• cargo test --bin bootroot passes (980 tests) — the one that actually runs the commands::* tests
• cargo test --lib passes (400 tests)
• grep -rn "set_var\|remove_var" over the ten scope-table files returns nothing
• The same grep over src/commands/dotenv.rs returns only the production call
• grep -rn "env_lock\|ENV_LOCK\|ScopedEnv\|ComposeProjectEnv\|set_current_dir" over all eleven files returns nothing
• grep -rn "env_lock" src/commands/init returns only imports and call sites resolving to rotate::test_support
• rotate::test_support and verify.rs's ENV_LOCK still present and compiling
• init::steps::test_support still exports default_init_args, test_messages and test_cert_pem to its six importers
• Converted tests were mutation-checked: passing the wrong value fails the assertion
• cargo clippy --all-targets -- -D warnings clean
• cargo fmt -- --check --config group_imports=StdExternalCrate clean
• Full CI green, including the Docker E2E matrix
• run-extended from e2e-extended.yml dispatched at the head commit; still running

[sehkone]

[Reviewer Round 1]

No blocking findings. The change follows the issue’s intended shape: the value-steered paths now have parameterized forms (PostgresEnv in src/commands/init/steps/database.rs, *_with_env host-port resolvers, and the OpenBao/compose siblings), while the production wrappers retain the existing precedence reads. The new tests exercise the supplied values rather than ambient state, including the per-service routing assertion in src/commands/infra.rs and the changed-port assertion in src/commands/rotate/db.rs.

I also confirmed the scoped test environment mutations and the associated lock/RAII helpers are gone, reinit no longer changes the process working directory, and the PR links Closes #806 with a test-plan checklist.

[sehkone]

[Review Verdict Round 1: APPROVED]

[sehkone]

Suggested squash commit

Title

Pass environment values into the resolvers

Body

Test code steered production functions by mutating the process
environment behind a shared mutex. In Rust 2024 `set_var` and
`remove_var` are `unsafe`, and that lock does not discharge the
contract: it serialises only the code that takes it, while the tokio
runtime, the HTTP client, the env-filtered subscriber and the libc
resolver all read the environment from threads holding nothing.

Every resolver a test steered now takes the value as a parameter,
leaving one thin wrapper at the composition root that reads the
environment. The variables consulted, their precedence and their
fallbacks are unchanged; what moves is where they are read. The
`POSTGRES_*` set reaches `resolve_db_dsn_for_init` and
`resolve_db_dsn` from `init::steps::orchestrator`, which reads it
straight after `load_dotenv_into_env` promotes the compose `.env` so
the builders keep the precedence they rely on. `COMPOSE_PROJECT_NAME`,
`OPENBAO_ROOT_TOKEN` and the `*_HOST_PORT` keys follow the same shape.

`load_dotenv_into_env` is the one function whose job is the mutation,
so it splits instead: a pure `dotenv_pairs_to_apply` decides which
pairs apply, including the `COMPOSE_PROJECT_NAME` exclusion that half
its tests were really about, and the wrapper keeps the single
production `set_var` with its `// SAFETY:` comment unchanged.

`resolve_db_admin_dsn` took `compose_dir` only to derive the host
port, so the port replaces it. That strands the `ca.json` the
no-source test wrote to prove no fallthrough — with no directory to
reach it through, the signature is what rules the fallthrough out.

With their last users gone, `compose_project::test_env`,
`openbao_auth`'s `ENV_LOCK`/`env_lock`, `init::steps::test_support`'s
`ENV_LOCK`/`env_lock` and the three anonymous `OnceLock<Mutex<()>>`
guards in `host_port.rs`, `openbao_url.rs` and `db.rs` are deleted;
clippy would fail on them otherwise. The rest of
`init::steps::test_support`, all of `rotate::test_support` and
`verify.rs`'s `ENV_LOCK` stay for the other issues in this effort.

`reinit`'s scope-check test no longer calls `set_current_dir`. The
working directory is process-global exactly as the environment is,
and the `.` it reproduced is a property of `compose_file_dir`, so the
test passes the directory in rather than moving the process.

Three parameters ended up threaded but steering nothing, each
replaceable with `None` with the suite still green; they now carry an
assertion that fails when the value does not reach its resolver.

No new `unsafe` block, no replacement lock, no `--test-threads=1`.

Closes #806