core: environment variables are read directly from library code instead of being injected, and tests mutate real process env unsafely

[MaximeGaudin]

Summary

model_failure (engine.rs) and GenAiChat::new (llm/provider.rs) call std::env::var(variable) directly rather than through an injected accessor, which is why the crate's own tests resort to unsafe { std::env::set_var(...) } / remove_var(...) (engine.rs:613,626) to exercise both branches — a process-global mutation that is a data race under cargo test's default parallel test execution if any other test reads the same variable concurrently.

Scope

• Severity (reporter's assessment): low
• Area: arch
• Code: packages/core/src/engine.rs:62-77, packages/core/src/llm/provider.rs:142-151, packages/core/src/engine.rs:611-627
• Reproduced empirically: yes

Minimal reproduction

grep -n "std::env::var\|set_var\|remove_var" packages/core/src/engine.rs packages/core/src/llm/provider.rs shows direct std::env::var calls in production code (engine.rs:63,143; provider.rs:143) and unsafe { std::env::set_var("SLINT_TEST_PRESENT_KEY", ...) } / remove_var in the test module (engine.rs:613,626) — note the test even names the variable to reduce collision risk, an implicit admission that a real name would be unsafe to use.

Expected

Environment lookups used for user-facing messages/credential resolution should go through a small trait or closure parameter injected by the caller, so tests can supply a fake environment instead of mutating the real process environment, and so a library embedder can control where credentials are read from (e.g. a secrets manager) without shelling through std::env.

Sources

• https://doc.rust-lang.org/std/env/fn.set_var.html — as of Rust 2024, std::env::set_var is unsafe specifically because of the underlying process-wide, non-thread-safe C API it wraps

Notes

Low severity because the affected tests already guard against the most obvious collision by using distinctively-named variables, but the pattern remains a latent flaky-test / testability smell that a typed environment-provider abstraction would remove entirely.

---

Filed as part of a systematic pre-release audit. Triage and de-duplication pending.