Skip to content

Distrust repository-local Python interpreters (avoid executing .venv/bin/python) #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tachyon-beep wants to merge 1 commit into from
Closed

Distrust repository-local Python interpreters (avoid executing .venv/bin/python) #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions crates/loomweave-core/src/plugin/host.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1600,11 +1600,11 @@ ontology_version = "0.1.0"
std::fs::set_permissions(&venv, std::fs::Permissions::from_mode(0o755)).unwrap();
let empty = |_: &str| -> Option<OsString> { None };

// A pyright plugin is handed the project's own `.venv` interpreter.
// Repository-local interpreters are untrusted and are not exported.
assert_eq!(
exported_interpreter(&pyright_small_rss_manifest(), dir.path(), &empty),
Some(venv.clone()),
"a language-server plugin must be pinned to the project interpreter"
None,
"a language-server plugin must not execute a repository interpreter"
);
// A plugin that does not declare the pyright runtime never triggers
// discovery: the variable means nothing to it, and exporting it would
Expand All @@ -1625,18 +1625,14 @@ ontology_version = "0.1.0"
None,
"an operator override must survive untouched"
);
// ...but an EMPTY value is not an override. Both discoveries treat
// `""` as unset on the override rung (`if override:` in Python,
// `.filter(|v| !v.is_empty())` in Rust), so a bare `is_some()` guard
// here would suppress the export for a variable the plugin then also
// ignores — leaving the child with no interpreter at all, which is the
// launcher-dependent hole this export exists to close.
// An EMPTY value is not an override, and does not make repository
// interpreter discovery safe.
let empty_override =
|key: &str| -> Option<OsString> { (key == PYTHON_INTERPRETER_ENV).then(OsString::new) };
assert_eq!(
exported_interpreter(&pyright_small_rss_manifest(), dir.path(), &empty_override),
Some(venv.clone()),
"an empty override is unset: the host still exports its own pinned choice"
None,
"an empty override must not permit a repository interpreter"
);
// An UNPINNED (bare `PATH`) choice is not exported: presenting a guess
// to the plugin as an authoritative pin buys nothing over its own
Expand Down
129 changes: 22 additions & 107 deletions crates/loomweave-core/src/plugin/interpreter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ pub const PYTHON_INTERPRETER_ENV: &str = "LOOMWEAVE_PYTHON_INTERPRETER";
pub enum InterpreterSource {
/// [`PYTHON_INTERPRETER_ENV`] named an executable file.
Override,
/// `<project_root>/.venv/bin/python` — the project's own virtualenv.
DotVenv,
/// `$VIRTUAL_ENV/bin/python` — an activated virtualenv.
VirtualEnv,
/// `$CONDA_PREFIX/bin/python` — an activated conda environment.
Expand All @@ -60,7 +58,7 @@ pub struct ProjectInterpreter {
}

impl ProjectInterpreter {
/// Project-owned (override / `.venv` / `VIRTUAL_ENV` / `CONDA_PREFIX`).
/// Explicitly selected (override / `VIRTUAL_ENV` / `CONDA_PREFIX`).
#[must_use]
pub fn pinned(&self) -> bool {
!matches!(
Expand All @@ -72,7 +70,7 @@ impl ProjectInterpreter {
/// Stable string for `plugin_index_meta.resolver_environment`.
///
/// An unpinned choice is tagged so it can never compare equal to a pinned
/// path with the same bytes: acquiring a project venv at a location that
/// path with the same bytes: activating an environment whose interpreter
/// happened to be first on `PATH` still moves the marker.
#[must_use]
pub fn fingerprint(&self) -> String {
Expand Down Expand Up @@ -184,17 +182,13 @@ fn which(name: &str, path_var: Option<&OsString>) -> Option<PathBuf> {
/// Resolve the project's interpreter in the contract order (module docs).
/// `env` abstracts `std::env::var_os` so tests can inject an environment.
///
/// `project_root` MUST be canonicalised by the caller. Discovery joins
/// `.venv/bin/python` onto the root as given and normalises only lexically, so
/// a symlinked root yields a symlinked interpreter path and a different
/// [`ProjectInterpreter::fingerprint`]. `analyze` (which records the
/// fingerprint) and `PluginHost::spawn_unhandshaken` (which exports the
/// interpreter) both canonicalise first; dropping it at either site would skew
/// the marker against the exported interpreter and re-dispatch every run. See
/// `the_root_canonicalisation_at_both_call_sites_is_load_bearing`.
/// `project_root` remains part of the cross-language API but is deliberately
/// not inspected for interpreters. Pyright executes its configured Python, so
/// selecting `<project_root>/.venv/bin/python` would execute untrusted
/// repository content without an operator trust decision.
#[must_use]
pub fn discover_project_interpreter(
project_root: &Path,
_project_root: &Path,
env: &dyn Fn(&str) -> Option<OsString>,
Comment on lines 190 to 192

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the CLI test for removed dotvenv discovery

When no override or activated environment is present, ignoring project_root makes discovery return the unpinned PATH/none result, but analyze_interpreter_change_forces_full_reanalysis in crates/loomweave-cli/tests/analyze.rs still supplies only <project>/.venv/bin/python and asserts that this path is fingerprinted and exported. The verify.yml workspace-wide nextest job runs this test, so the suite will fail on Unix; update the integration scenario to transition between trusted interpreter sources while preserving its re-dispatch coverage.

Useful? React with 👍 / 👎.

) -> ProjectInterpreter {
if let Some(raw) = env(PYTHON_INTERPRETER_ENV).filter(|value| !value.is_empty()) {
Expand All @@ -209,12 +203,6 @@ pub fn discover_project_interpreter(
"{PYTHON_INTERPRETER_ENV} is not an executable file; ignoring the override"
);
}
if let Some(path) = usable(&project_root.join(".venv/bin/python")) {
return ProjectInterpreter {
path: Some(path),
source: InterpreterSource::DotVenv,
};
}
for (var, source) in [
("VIRTUAL_ENV", InterpreterSource::VirtualEnv),
("CONDA_PREFIX", InterpreterSource::Conda),
Expand Down Expand Up @@ -286,53 +274,6 @@ mod tests {
assert_eq!(PYTHON_INTERPRETER_ENV, "LOOMWEAVE_PYTHON_INTERPRETER");
}

#[test]
fn the_root_canonicalisation_at_both_call_sites_is_load_bearing() {
// `analyze` computes the fingerprint from its canonicalised
// `project_root`; `PluginHost::spawn_unhandshaken` re-canonicalises the
// root it is handed before running the SAME discovery to decide what to
// export. They agree only because BOTH canonicalise and canonicalise is
// idempotent.
//
// Discovery itself is deliberately NOT root-invariant: it joins
// `.venv/bin/python` onto the root as given and lexically normalises,
// so a symlinked root yields a symlinked interpreter path. That is
// correct for a venv (see the symlink test below) but it means dropping
// the canonicalisation at either call site would silently skew the
// recorded marker against the exported interpreter — an index that
// re-dispatches every run. This test pins the skew so that removal
// fails loudly here rather than quietly in production.
let dir = tempfile::tempdir().unwrap();
let real_root = dir.path().join("real");
let venv = make_python(&real_root.join(".venv/bin/python"));
let link_root = dir.path().join("link");
std::os::unix::fs::symlink(&real_root, &link_root).unwrap();
let canonical_root = link_root.canonicalize().unwrap();

assert_eq!(
discover_project_interpreter(&canonical_root, &env(&HashMap::new())).path,
Some(venv),
"a canonical root finds the project .venv at its real path"
);
// Idempotence — the property the two call sites actually rely on.
assert_eq!(
discover_project_interpreter(&canonical_root, &env(&HashMap::new())).fingerprint(),
discover_project_interpreter(
&canonical_root.canonicalize().unwrap(),
&env(&HashMap::new())
)
.fingerprint(),
"canonicalising twice must not move the fingerprint"
);
// And the skew a dropped canonicalisation would introduce.
assert_ne!(
discover_project_interpreter(&link_root, &env(&HashMap::new())).fingerprint(),
discover_project_interpreter(&canonical_root, &env(&HashMap::new())).fingerprint(),
"an UNcanonicalised root yields a different fingerprint — which is why both \
`analyze` and `spawn_unhandshaken` must canonicalise before discovering"
);
}

/// Sets the process CWD for the duration of a test and restores it on drop
/// (including on panic, so a failing assertion cannot leak a bad CWD into
/// another test in the same binary).
Expand Down Expand Up @@ -416,19 +357,6 @@ mod tests {
"empty values on every rung must discover nothing — NOT the CWD's python"
);

// An empty override falls through to `.venv` without taking the
// warning path (that branch is for an operator who set a BAD path, not
// for an unset variable), and an empty PATH cannot outrank it.
let dotvenv = make_python(&root.join(".venv/bin/python"));
assert_eq!(
discover_project_interpreter(&root, &env(&all_empty)),
ProjectInterpreter {
path: Some(dotvenv),
source: InterpreterSource::DotVenv
},
"an empty override falls through to .venv"
);

// Control: a NON-empty PATH naming a directory with no interpreter
// reaches the same `None`, the legitimate way.
let empty_dir = dir.path().join("empty");
Expand All @@ -441,33 +369,23 @@ mod tests {
}

#[test]
fn dotvenv_wins_over_virtual_env_and_path() {
fn repository_dotvenv_is_ignored() {
let dir = tempfile::tempdir().unwrap();
let dotvenv = make_python(&dir.path().join(".venv/bin/python"));
let other = make_python(&dir.path().join("elsewhere/bin/python"));
let map = HashMap::from([
(
"VIRTUAL_ENV",
dir.path().join("elsewhere").display().to_string(),
),
("PATH", other.parent().unwrap().display().to_string()),
]);
make_python(&dir.path().join(".venv/bin/python"));
let trusted = make_python(&dir.path().join("elsewhere/bin/python"));
let map = HashMap::from([(
"VIRTUAL_ENV",
dir.path().join("elsewhere").display().to_string(),
)]);
let found = discover_project_interpreter(dir.path(), &env(&map));
assert_eq!(
found,
ProjectInterpreter {
path: Some(dotvenv.clone()),
source: InterpreterSource::DotVenv
}
);
assert!(found.pinned());
assert_eq!(found.fingerprint(), dotvenv.display().to_string());
assert_eq!(found.path, Some(trusted));
assert_eq!(found.source, InterpreterSource::VirtualEnv);
}

#[test]
fn override_wins_and_an_unusable_override_falls_through() {
let dir = tempfile::tempdir().unwrap();
let dotvenv = make_python(&dir.path().join(".venv/bin/python"));
make_python(&dir.path().join(".venv/bin/python"));
let custom = make_python(&dir.path().join("custom/python"));
let map = HashMap::from([(PYTHON_INTERPRETER_ENV, custom.display().to_string())]);
assert_eq!(
Expand All @@ -479,8 +397,8 @@ mod tests {
dir.path().join("nope").display().to_string(),
)]);
let found = discover_project_interpreter(dir.path(), &env(&map));
assert_eq!(found.source, InterpreterSource::DotVenv);
assert_eq!(found.path, Some(dotvenv));
assert_eq!(found.source, InterpreterSource::None);
assert_eq!(found.path, None);
}

#[test]
Expand Down Expand Up @@ -544,12 +462,9 @@ mod tests {
discover_project_interpreter(dir.path(), &env(&map)).path,
Some(real.clone())
);
let link = dir.path().join(".venv/bin/python");
fs::create_dir_all(link.parent().unwrap()).unwrap();
std::os::unix::fs::symlink(&real, &link).unwrap();
// A repository-local symlink is not considered without an explicit override.
let found = discover_project_interpreter(dir.path(), &env(&HashMap::new()));
assert_eq!(found.path, Some(link), "the symlink path, not its target");
assert_eq!(found.source, InterpreterSource::DotVenv);
assert_eq!(found.source, InterpreterSource::None);
}

#[test]
Expand All @@ -558,7 +473,7 @@ mod tests {
// separator at construction, so `is_file()` succeeds and the plugin
// pins the override. Rust's `metadata`/`access(2)` on the raw
// `…/python/` fail with ENOTDIR, so without the strip the HOST would
// fall through to `.venv` (or export nothing) while the PLUGIN pinned
// export nothing while the PLUGIN pinned
// the operator's path — the two disagreeing on the same environment,
// which is the failure mode this module exists to prevent. The
// returned path must also be the stripped, normalised one, byte-equal
Expand Down
19 changes: 11 additions & 8 deletions plugins/python/src/loomweave_plugin_python/interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
``python.pythonPath``. Under ``loomweave analyze`` launched from an agent hook
that ``python`` is the system interpreter, which cannot import the project's
editable install, and every ``tests/`` -> ``src/`` call target came back empty
while the coverage claim still said ``complete``. This module picks the
project's own interpreter deterministically so the answer no longer depends
on who launched the run.
while the coverage claim still said ``complete``. This module picks an
explicitly trusted or activated interpreter deterministically so the answer
no longer depends on who launched the run. Repository-local interpreters are
deliberately not discovered: Pyright executes the selected interpreter, so an
executable committed at ``.venv/bin/python`` is untrusted repository content.

The order below is a CROSS-LANGUAGE CONTRACT with
``crates/loomweave-core/src/plugin/interpreter.rs`` (the host runs the same
Expand Down Expand Up @@ -43,14 +45,14 @@
# must carry the same literal.
INTERPRETER_OVERRIDE_ENV: Final = "LOOMWEAVE_PYTHON_INTERPRETER"

InterpreterSource = Literal["override", "dotvenv", "virtual_env", "conda", "path", "none"]
InterpreterSource = Literal["override", "virtual_env", "conda", "path", "none"]

_PREFIX_SOURCES: Final[tuple[tuple[str, InterpreterSource], ...]] = (
("VIRTUAL_ENV", "virtual_env"),
("CONDA_PREFIX", "conda"),
)
_PINNED_SOURCES: Final[frozenset[InterpreterSource]] = frozenset(
{"override", "dotvenv", "virtual_env", "conda"},
{"override", "virtual_env", "conda"},
)


Expand All @@ -63,7 +65,7 @@ class ProjectInterpreter:

@property
def pinned(self) -> bool:
"""True when the interpreter is project-owned (not a PATH guess)."""
"""True when the interpreter was explicitly selected (not a PATH guess)."""
return self.source in _PINNED_SOURCES


Expand All @@ -78,6 +80,9 @@ def discover_project_interpreter(
environ: Mapping[str, str] | None = None,
) -> ProjectInterpreter:
"""Resolve the project's interpreter in the contract order (see module doc)."""
# Keep the project root in the API because this function is mirrored by
# the host, but never use it to select an executable from the repository.
_ = project_root
Comment on lines +83 to +85

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove the obsolete create-.venv remediation

After this function deliberately stops inspecting the project root, merely creating <project>/.venv cannot resolve interpreter_unpinned; nevertheless, loomweave doctor still tells affected users to “set LOOMWEAVE_PYTHON_INTERPRETER or create .venv,” and the Python README and ADR document the same obsolete rung. Users following that advice remain degraded, so the user-facing remediation and discovery documentation should instead require an explicit override or activated environment.

Useful? React with 👍 / 👎.

env = os.environ if environ is None else environ
override = env.get(INTERPRETER_OVERRIDE_ENV)
if override:
Expand All @@ -87,8 +92,6 @@ def discover_project_interpreter(
f"loomweave-plugin-python: {INTERPRETER_OVERRIDE_ENV}={override!r} is not an "
"executable file; ignoring the override and discovering the interpreter\n",
)
if (hit := _usable(Path(project_root) / ".venv" / "bin" / "python")) is not None:
return ProjectInterpreter(path=str(hit), source="dotvenv")
for var, source in _PREFIX_SOURCES:
prefix = env.get(var)
if prefix and (hit := _usable(Path(prefix) / "bin" / "python")) is not None:
Expand Down
Loading
Loading