fix: gate untrusted project-local mcpls.toml and scope document_tracker lock per path

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **`--trust-project-config` flag / `MCPLS_TRUST_PROJECT_CONFIG` env var** — opt-in gate for loading a `./mcpls.toml` discovered relative to the current directory. New `config::ProjectConfigTrust` enum and `ServerConfig::load_with_trust(trust)`, which `ServerConfig::load()` now delegates to with `ProjectConfigTrust::Untrusted`. (#229)
 - **Explicit per-tool routing** — `[[lsp_servers]]` entries gain optional `name` (routing identity, defaults to `language_id`) and `handles` (list of tools this server serves; omitted means catch-all) fields, so two servers can share one `language_id` and each own a distinct subset of MCP tools (e.g. pyright for everything, pylsp for `diagnostics` only). See `docs/user-guide/configuration.md` for the full semantics, including what happens when the routed server fails to spawn. (#174)
 - **`config::ToolKind` / `config::ServerId` / `config::ToolRouter`** — new public types: `ToolKind` is the typed enum of every routable MCP tool; `ServerId` is a server's routing identity; `ToolRouter` resolves `(language, tool)` to a `ServerId`, enforces the workspace-scoped routing rules at startup, and rebinds dead routes to a live catch-all (never to a server that explicitly declined the tool) once server registration completes. (#174)
 - **Single source of truth for React language-ID variants** — new `config::react_variant_language_id` / `config::base_language_id`, consumed by both `language_id_for_pattern_extension` and the translator's per-file server routing, replacing two independent hardcoded match arms. (#165)
@@ -24,7 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
-- **Translator lock contention on concurrent tool calls** — `Translator` no longer sits behind a single `Arc<Mutex<Translator>>` held for the full duration of an LSP round trip. Read-only config (`lsp_clients`, `workspace_roots`, `extension_map`, `expected_languages`) is now `Arc`-shared with independent, short-lived locks, and the document tracker has its own lock acquired only for `ensure_open`. The actual LSP request/response round trip now runs with no lock held at all, so a slow in-flight call (e.g. `get_diagnostics` pulling diagnostics from a busy language server) no longer stalls unrelated calls like `get_hover` behind it for up to 30s. The `ensure_open` prepare step still serializes through one shared document-tracker lock (bounded by a 250ms disk-check debounce in the common case; see `TODO(critic-S2)` in `translator.rs` for the follow-up to scope it per path). (#108, #159)
+- **Untrusted project-local config could redirect the spawned LSP command (P1 security)** — `ServerConfig::load()` discovered and loaded `./mcpls.toml` relative to the process's current working directory unconditionally, and that file's `command`/`args` (and `[workspace]`, e.g. `roots`, `heuristics_max_depth`) fed directly into the LSP server mcpls spawns. Running `mcpls` against an untrusted checkout (e.g. `git clone && mcpls`) could execute an attacker-chosen command with no confirmation — not viable to gate interactively since stdio *is* the MCP transport. Breaking change (acceptable pre-1.0): a CWD-discovered `./mcpls.toml` is now ignored by default (a warning names the ignored path); pass `--trust-project-config` or set `MCPLS_TRUST_PROJECT_CONFIG=true` to opt in. An explicit `--config`/`MCPLS_CONFIG` path is unaffected — naming a path is itself consent. Built-in project-marker heuristics (e.g. `Cargo.toml` → rust-analyzer) still apply normally when the project-local file is ignored. Note: the original report's claim that the config's `env` field is attacker-controlled at spawn time does not hold today — `env` is parsed but never passed to the spawned process (no `.envs()` call exists), so this fix is scoped to `command`/`args`/`[workspace]`. (#229)
+- **Translator lock contention on concurrent tool calls** — `Translator` no longer sits behind a single `Arc<Mutex<Translator>>` held for the full duration of an LSP round trip. Read-only config (`lsp_clients`, `workspace_roots`, `extension_map`, `expected_languages`) is now `Arc`-shared with independent, short-lived locks, and the document tracker has its own lock acquired only for `ensure_open`. The actual LSP request/response round trip now runs with no lock held at all, so a slow in-flight call (e.g. `get_diagnostics` pulling diagnostics from a busy language server) no longer stalls unrelated calls like `get_hover` behind it for up to 30s. (#108, #159)
+- **`document_tracker` lock still spanned disk I/O and the `didOpen`/`didChange` notify in `ensure_open` (P1)** — the lock fixed above was a single lock shared across every language and path, held across `ensure_open`'s own awaits (a `stat`, optionally a full re-read of the file, and the LSP notify — a bounded channel send with no timeout). A wedged language server that stopped draining its stdin could stall `ensure_open` for every other file and language, not just the one talking to that server. `DocumentTracker` now locks its document map only for short, synchronous sections and serializes `ensure_open` per path via a keyed async lock, so calls for different paths never wait on each other; calls for the same path still collapse into exactly one `didOpen`. Breaking change (acceptable pre-1.0): every `DocumentTracker` method now takes `&self` instead of `&mut self`; `get` returns an owned `DocumentState` instead of `Option<&DocumentState>`; `open_paths` returns `Vec<PathBuf>` instead of an iterator; `Translator::document_tracker` is now a plain `Arc<DocumentTracker>` instead of `Arc<Mutex<DocumentTracker>>`, and `Translator::open_document_paths`/`is_document_open` are no longer `async`. (#227)
 - **Stale document tracker on external file changes** — `DocumentTracker::ensure_open` now stats the file on every call and resyncs the LSP server via a single full-replacement `textDocument/didChange` when the on-disk content changed. Previously a file was only ever read once per session, so external edits (git checkout/stash, formatters, the MCP host's own Edit/Write tools) went unnoticed by mcpls and produced stale hover/diagnostics/completion results until the process restarted. (#102)
 - **CodeQL fork PR checkout** — `actions/checkout@v7` refuses to check out a fork PR's head SHA in `pull_request_target` workflows unless explicitly opted in; set `allow-unsafe-pr-checkout: true` on the CodeQL workflow's checkout step, which was failing every fork-originated PR since the checkout v6 → v7 bump. Safe here because the job carries no secrets and permissions are scoped to `security-events: write` + `contents: read` only.
 - **Notification loss and cached-read stalls under translator-lock contention** — `NotificationCache` is now held behind its own `Arc<Mutex<NotificationCache>>`, independent of `Arc<Mutex<Translator>>`, and workspace-root path validation for cache-only reads uses a lock-free `Arc<[PathBuf]>` snapshot instead of locking the translator. Previously, `diagnostics_pump` wrote into the cache through the translator lock: while that lock was held elsewhere for the duration of a slow `textDocument/diagnostic` round-trip, incoming `publishDiagnostics`/log/message notifications were silently dropped (the LSP transport forwards them via a non-blocking `try_send`), and `get_cached_diagnostics`/`read_resource`/`subscribe` — which only ever needed a cached read or a path check — queued behind that same round-trip for as long as it took to complete. (#104)

README.md

@@ -212,6 +212,7 @@ project_markers = ["Cargo.toml", "rust-toolchain.toml", ".rust-version"]
 | Variable | Description | Default |
 |----------|-------------|---------|
 | `MCPLS_CONFIG` | Path to configuration file | Auto-detected |
+| `MCPLS_TRUST_PROJECT_CONFIG` | Load a `./mcpls.toml` found in the current directory | `false` |
 | `MCPLS_LOG` | Log level (trace, debug, info, warn, error) | `info` |
 | `MCPLS_LOG_JSON` | Output logs as JSON | `false` |
 
@@ -223,6 +224,13 @@ project_markers = ["Cargo.toml", "rust-toolchain.toml", ".rust-version"]
 | macOS | `~/.config/mcpls/mcpls.toml` or `~/Library/Application Support/mcpls/` |
 | Windows | `%APPDATA%\mcpls\mcpls.toml` |
 
+> [!WARNING]
+> A `./mcpls.toml` in the current directory is **not** loaded automatically: it
+> can control which command mcpls spawns as an LSP server, so running mcpls
+> against an untrusted checkout must not execute commands from that checkout
+> without explicit consent. Pass `--trust-project-config` (or set
+> `MCPLS_TRUST_PROJECT_CONFIG=true`) only for repositories you trust.
+
 </details>
 
 <details>

crates/mcpls-cli/README.md

@@ -28,9 +28,15 @@ mcpls --listen 127.0.0.1:3000             # HTTP transport (transport-http featu
 ## Configuration
 
 > [!NOTE]
-> Configuration auto-discovery order: `$MCPLS_CONFIG` → `./mcpls.toml` → platform config dir
+> Configuration auto-discovery order: `$MCPLS_CONFIG` → `./mcpls.toml` (requires
+> `--trust-project-config`) → platform config dir
 > Auto-creates default config with 30 language mappings on first run.
 
+> [!WARNING]
+> A `./mcpls.toml` in the current directory is ignored by default, since it can
+> control which command mcpls spawns as an LSP server. Pass `--trust-project-config`
+> (or set `MCPLS_TRUST_PROJECT_CONFIG=true`) only for repositories you trust.
+
 Create or edit `mcpls.toml` in the appropriate location:
 - **Linux/macOS:** `~/.config/mcpls/mcpls.toml`
 - **macOS (alternative):** `~/Library/Application Support/mcpls/mcpls.toml`
@@ -43,6 +49,7 @@ See the main [README](../../README.md) for configuration examples and custom ext
 | Flag | Env | Description |
 |------|-----|-------------|
 | `-c, --config <PATH>` | `MCPLS_CONFIG` | Configuration file path |
+| `--trust-project-config` | `MCPLS_TRUST_PROJECT_CONFIG` | Load a `./mcpls.toml` found in the current directory |
 | `-l, --log-level <LEVEL>` | `MCPLS_LOG` | trace, debug, info, warn, error (default: info) |
 | `--log-json` | `MCPLS_LOG_JSON` | JSON-formatted logs for tooling |
 | `--listen <ADDR>` | `MCPLS_LISTEN` | Bind address for HTTP transport (`transport-http` feature) |

crates/mcpls-cli/src/args.rs

@@ -17,11 +17,23 @@ pub struct Args {
     ///
     /// If not specified, searches for mcpls.toml in:
     /// 1. `$MCPLS_CONFIG` environment variable
-    /// 2. Current directory
+    /// 2. Current directory (only loaded with `--trust-project-config`)
     /// 3. ~/.config/mcpls/mcpls.toml
     #[arg(short, long, value_name = "FILE", env = "MCPLS_CONFIG")]
     pub config: Option<PathBuf>,
 
+    /// Trust and load a `mcpls.toml` found in the current directory.
+    ///
+    /// A project-local config discovered this way (as opposed to one passed
+    /// explicitly via `--config`/`MCPLS_CONFIG`) can control the LSP server
+    /// `command`/`args` mcpls spawns, so it is ignored by default to avoid
+    /// arbitrary code execution when running mcpls against an untrusted
+    /// checkout. Pass this flag only for repositories you trust. Via
+    /// `MCPLS_TRUST_PROJECT_CONFIG`, only the literal values `true`/`false`
+    /// are accepted; any other value is a parse error at startup.
+    #[arg(long, env = "MCPLS_TRUST_PROJECT_CONFIG")]
+    pub trust_project_config: bool,
+
     /// Logging level
     ///
     /// Valid values: trace, debug, info, warn, error
@@ -65,6 +77,18 @@ mod tests {
         assert!(!args.log_json);
     }
 
+    #[test]
+    fn test_trust_project_config_default_false() {
+        let args = Args::parse_from(["mcpls"]);
+        assert!(!args.trust_project_config);
+    }
+
+    #[test]
+    fn test_trust_project_config_flag() {
+        let args = Args::parse_from(["mcpls", "--trust-project-config"]);
+        assert!(args.trust_project_config);
+    }
+
     #[test]
     fn test_config_arg() {
         let args = Args::parse_from(["mcpls", "--config", "/path/to/config.toml"]);

crates/mcpls-cli/src/main.rs

@@ -5,6 +5,7 @@
 
 use anyhow::{Context, Result};
 use clap::Parser;
+use mcpls_core::ProjectConfigTrust;
 
 mod args;
 mod logging;
@@ -25,7 +26,12 @@ async fn main() -> Result<()> {
         mcpls_core::ServerConfig::load_from(config_path)
             .with_context(|| format!("failed to load config from {}", config_path.display()))?
     } else {
-        mcpls_core::ServerConfig::load().context("failed to load configuration")?
+        let trust = if args.trust_project_config {
+            ProjectConfigTrust::Trusted
+        } else {
+            ProjectConfigTrust::Untrusted
+        };
+        mcpls_core::ServerConfig::load_with_trust(trust).context("failed to load configuration")?
     };
 
     tracing::debug!(