Skip to content

test: test_load_does_not_overwrite_existing_config mutates process cwd with no guard #238

Description

@bug-ops

Description

config/mod.rs::test_load_does_not_overwrite_existing_config calls std::env::set_current_dir to exercise ServerConfig::load()'s cwd-relative ./mcpls.toml lookup, then restores the original directory at the end of the test. set_current_dir mutates process-wide state (not thread-local), and the crate has no serial_test dependency or any other mechanism to serialize this test against others that might read the current directory concurrently.

The project mandates cargo nextest, whose per-test-process model happens to make this safe today. But cargo test (still a normal, undocumented-as-forbidden entry point, and how cargo test --doc / IDE "run test" buttons typically invoke tests) runs all unit tests as threads inside one process, where chdir affects every thread. Additionally, if this test panics between the set_current_dir(tmp_dir) call and the restore (e.g. any of the unwrap()/assert_eq! calls in between), the restore is skipped and the process cwd stays changed for the remainder of that run.

Reproduction Steps

  1. Run cargo test -p mcpls-core --lib config:: -- --test-threads=8 (plain cargo test, not nextest) alongside other tests that depend on relative paths or call ServerConfig::load().
  2. Observe that other tests running concurrently in the same process can transiently see the temp directory as cwd while this test holds it.
  3. Separately, temporarily break one of the assert_eq! calls between the two set_current_dir calls and note the test binary's cwd is never restored for the rest of that run.

Expected Behavior

The test should not depend on unguarded global process state, or should be protected so a panic mid-test cannot leave cwd changed and so it cannot race other tests under a threaded runner.

Actual Behavior

set_current_dir is called with no serialization primitive (no serial_test crate in the dependency tree) and no Drop guard to restore cwd on early return/panic.

Environment

  • Version: main @ 2c8ad7f
  • Features: default

Logs / Evidence

crates/mcpls-core/src/config/mod.rs:1057-1084. Verified via cargo tree -i serial_test that no serialization crate is present in the workspace, and rg set_current_dir shows this is the only occurrence in the codebase.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Low: cosmetic, edge case unlikely in practicebugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions