From f534d7afb499b2f2fba1219450d76851629e5f45 Mon Sep 17 00:00:00 2001 From: controlloop-security <344908699+controlloop-security@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:44:01 -0500 Subject: [PATCH] fix(mcp): read the uvx --from and pipx --spec requirement as the package `uvx --from pkg==1.2 tool` was recorded as package `tool`, unpinned: the `--from` value was skipped as a flag value and the command name read as the package. A different package behind a familiar command name was therefore invisible, and a version bump behind `--from` scored `medium` (changed arguments) instead of `high` in policy.mcp-server-identity-change. The `--from` (uvx) or `--spec` (pipx run) requirement, when it comes before the command, is now the package. `===` pins and Git sources with a revision are read as pinned. The requirement itself is no longer redacted as a KEY=value argument (`pkg==1.2` was shown as `pkg=`); other KEY=value arguments still are. Signed-off-by: controlloop-security <344908699+controlloop-security@users.noreply.github.com> --- CHANGELOG.md | 14 ++++ src/controlloop/adapters/mcp.py | 81 ++++++++++++++++--- tests/adapters/test_mcp_identity.py | 55 ++++++++++++- .../rules/test_mcp_server_identity_change.py | 22 +++++ 4 files changed, 162 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da00ab1..bc8ddd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,20 @@ project uses [Semantic Versioning](https://semver.org/). dependency. See [The local MCP server](docs/reference/mcp-server.md). +### Fixed + +- **`uvx --from` and `pipx run --spec` name the package that runs.** An + MCP server launched as `uvx --from pkg==1.2 tool` was recorded as + package `tool`, unpinned: the scan read the command name and skipped the + requirement. It now records the `--from` (or `--spec`) requirement, so + `--from other-pkg==1.0 tool` reports `other-pkg` at `1.0`, and a Git + source with a revision (`git+https://host/repo@v1.2`) counts as pinned. + The requirement also stays readable in the recorded arguments: `pkg==1.2` + was shown as `pkg=`. Other `KEY=value` arguments are still + redacted. `policy.mcp-server-identity-change` now rates a version bump or + a different package behind the same `uvx --from` command `high`; it was + `medium`, as changed arguments. + ## [0.6.2] - 2026-10-30 The first release listed on GitHub Marketplace. It carries 0.6.1's fix, diff --git a/src/controlloop/adapters/mcp.py b/src/controlloop/adapters/mcp.py index c6c3483..24607c5 100644 --- a/src/controlloop/adapters/mcp.py +++ b/src/controlloop/adapters/mcp.py @@ -106,11 +106,14 @@ class _Location: _NPM_LAUNCHERS = frozenset({"npx", "bunx"}) _NPM_SUBCOMMAND_LAUNCHERS = frozenset({"pnpm", "yarn"}) _PYPI_LAUNCHERS = frozenset({"uvx"}) +#: `uvx --from` and `pipx run --spec` name the requirement installed, which +#: then supplies the command; their value is the package, not the command. +_PYPI_SOURCE_FLAGS = frozenset({"--from", "--spec"}) _CONTAINER_LAUNCHERS = frozenset({"docker", "podman"}) #: Launcher flags that take a value, so the value is not the package. _FLAGS_WITH_VALUES = frozenset( { - "-p", "--package", "--from", "--with", "--python", "--index-url", + "-p", "--package", "--from", "--spec", "--with", "--python", "--index-url", "-e", "--env", "--env-file", "-v", "--volume", "--name", "--network", "-w", "--workdir", "-u", "--user", "--entrypoint", "--platform", "--mount", "-l", "--label", "--cidfile", "--registry", @@ -289,9 +292,20 @@ def _load(path: Path, dialect: str) -> Any: raise -def _redacted_args(args: list[str]) -> list[str]: +def _redacted_args(args: list[str], package_spec: str | None = None) -> list[str]: + """Keep only the key of `KEY=value` arguments, except the launched + package's own requirement (`pkg==1.2`, `--from=pkg==1.2`), whose version + is already recorded as `package_version`.""" + redacted: list[str] = [] for arg in args: + flag, equals, value = arg.partition("=") + if package_spec is not None and ( + arg == package_spec + or (equals and flag in _PYPI_SOURCE_FLAGS and value == package_spec) + ): + redacted.append(aa.authority_text(arg)) + continue match = _KEY_VALUE_ARG.match(arg) redacted.append( f"{match.group(1)}{aa.REDACTED_VALUE}" if match else aa.authority_text(arg) @@ -314,6 +328,59 @@ def _first_positional(args: list[str]) -> str | None: return None +def _pypi_requirement(args: list[str]) -> str | None: + """The requirement uvx or pipx installs: the `--from`/`--spec` value + when one comes before the command, else the first positional.""" + + skip = False + for index, arg in enumerate(args): + if skip: + skip = False + continue + if arg in _PYPI_SOURCE_FLAGS: + return args[index + 1] if index + 1 < len(args) else None + flag, equals, value = arg.partition("=") + if equals and flag in _PYPI_SOURCE_FLAGS: + return value or None + if arg in _FLAGS_WITH_VALUES: + skip = True + continue + if arg.startswith("-"): + continue + return arg + return None + + +def _pypi_identity(spec: str) -> tuple[str, str]: + """Split a requirement into `(package, version)`; a range, a bare name + or a VCS URL without a revision is `unpinned`.""" + + if "==" in spec: + name, version = spec.split("==", 1) + return name, version.lstrip("=") or "unpinned" + if "://" in spec: + # `git+https://host/repo@v1.2`; the `@` in `git@host` is not a pin. + name, at, revision = spec.rpartition("@") + if at and revision and "/" not in revision and "://" in name: + return name, revision + return spec, "unpinned" + name, at, version = spec.partition("@") + return (name, version) if at and version else (spec, "unpinned") + + +def _package_spec(command: str | None, args: list[str]) -> str | None: + """The requirement text a PyPI launcher installs, as written.""" + + if command is None: + return None + launcher = command.rsplit("/", 1)[-1] + if launcher == "pipx" and args[:1] == ["run"]: + return _pypi_requirement(args[1:]) + if launcher in _PYPI_LAUNCHERS: + return _pypi_requirement(args) + return None + + def _package_identity( command: str | None, args: list[str] ) -> tuple[str, str, str] | None: @@ -350,14 +417,10 @@ def _package_identity( name = f"{scope}/{name_part}" if scope else name_part return "npm", name, version if at and version else "unpinned" if launcher in _PYPI_LAUNCHERS: - spec = _first_positional(rest) + spec = _pypi_requirement(rest) if spec is None: return None - for separator in ("==", "@"): - if separator in spec: - name, version = spec.split(separator, 1) - return "pypi", name, version - return "pypi", spec, "unpinned" + return "pypi", *_pypi_identity(spec) return None @@ -397,7 +460,7 @@ def _tools_digest(context: ScanContext, server: _Server) -> str: def _attributes(context: ScanContext, server: _Server) -> dict[str, str]: - args = _redacted_args(server.args) + args = _redacted_args(server.args, _package_spec(server.command, server.args)) attributes: dict[str, str] = { "transport": "stdio" if server.command else "http", "declaration_digest": "", diff --git a/tests/adapters/test_mcp_identity.py b/tests/adapters/test_mcp_identity.py index 8c656b4..1c9c556 100644 --- a/tests/adapters/test_mcp_identity.py +++ b/tests/adapters/test_mcp_identity.py @@ -66,8 +66,30 @@ def _server(tmp_path: Path, entry: Mapping[str, object], name: str = "s") -> Nod ("bunx", ["pkg@0.1.0"], "npm", "pkg", "0.1.0"), ("pnpm", ["dlx", "pkg@3.0.0"], "npm", "pkg", "3.0.0"), ("uvx", ["mcp-server-fetch==1.2.3"], "pypi", "mcp-server-fetch", "1.2.3"), - ("uvx", ["--from", "git+x", "tool"], "pypi", "tool", "unpinned"), + ("uvx", ["--from", "git+x", "tool"], "pypi", "git+x", "unpinned"), + ("uvx", ["--from", "srv==0.6.1", "srv", "mcp"], "pypi", "srv", "0.6.1"), + ("uvx", ["--from=srv==0.6.1", "srv"], "pypi", "srv", "0.6.1"), + ("uvx", ["--from", "other==1.0", "srv"], "pypi", "other", "1.0"), + ("uvx", ["--from", "srv>=1", "srv"], "pypi", "srv>=1", "unpinned"), + ("uvx", ["srv", "--from", "x==1"], "pypi", "srv", "unpinned"), + ("uvx", ["srv@0.6.1", "mcp"], "pypi", "srv", "0.6.1"), + ("uvx", ["srv===2.0"], "pypi", "srv", "2.0"), + ( + "uvx", + ["--from", "git+https://github.com/acme/srv@v1.2", "srv"], + "pypi", + "git+https://github.com/acme/srv", + "v1.2", + ), + ( + "uvx", + ["--from", "git+ssh://git@github.com/acme/srv", "srv"], + "pypi", + "git+ssh://git@github.com/acme/srv", + "unpinned", + ), ("pipx", ["run", "srv@4.0"], "pypi", "srv", "4.0"), + ("pipx", ["run", "--spec", "srv==4.0", "srv"], "pypi", "srv", "4.0"), ( "docker", ["run", "-i", "--rm", "-e", "TOKEN", "ghcr.io/acme/mcp:1.4"], @@ -142,6 +164,37 @@ def test_key_value_arguments_keep_only_the_key(tmp_path: Path) -> None: assert "REGION=" in args +@pytest.mark.parametrize( + ("args", "shown"), + [ + (["srv==0.6.1", "mcp"], "srv==0.6.1 mcp"), + (["--from", "srv==0.6.1", "srv"], "--from srv==0.6.1 srv"), + (["--from=srv==0.6.1", "srv"], "--from=srv==0.6.1 srv"), + ], +) +def test_the_launched_requirement_is_not_redacted( + tmp_path: Path, args: list[str], shown: str +) -> None: + node = _server(tmp_path, {"command": "uvx", "args": args}) + + assert node.attributes["args"] == shown + + +def test_key_value_arguments_after_the_requirement_stay_redacted( + tmp_path: Path, +) -> None: + node = _server( + tmp_path, + { + "command": "uvx", + "args": ["--from", "srv==1.0", "srv", f"TOKEN=={_SECRET_ARG}"], + }, + ) + + assert _SECRET_ARG not in node.attributes["args"] + assert node.attributes["args"] == "--from srv==1.0 srv TOKEN=" + + def test_the_declaration_digest_changes_with_the_declaration_only( tmp_path: Path, ) -> None: diff --git a/tests/policy/rules/test_mcp_server_identity_change.py b/tests/policy/rules/test_mcp_server_identity_change.py index 0864e80..e5c18a0 100644 --- a/tests/policy/rules/test_mcp_server_identity_change.py +++ b/tests/policy/rules/test_mcp_server_identity_change.py @@ -44,6 +44,28 @@ def test_a_same_name_package_version_change_is_high(tmp_path: Path) -> None: assert finding.affected_agent is not None +def test_a_uvx_from_version_change_is_high(tmp_path: Path) -> None: + base, head = repos(tmp_path) + _server(base, ["--from", "postmark-mcp==1.0.15", "postmark-mcp"], "uvx") + _server(head, ["--from", "postmark-mcp==1.0.16", "postmark-mcp"], "uvx") + + (finding,) = mcp_server_identity_change_findings(context(head, base)) + + assert finding.severity is FindingSeverity.HIGH + assert "package version from '1.0.15' to '1.0.16'" in finding.message + + +def test_a_uvx_from_package_behind_the_same_command_is_high(tmp_path: Path) -> None: + base, head = repos(tmp_path) + _server(base, ["--from", "postmark-mcp==1.0.15", "postmark-mcp"], "uvx") + _server(head, ["--from", "postmark-mcpp==1.0.15", "postmark-mcp"], "uvx") + + (finding,) = mcp_server_identity_change_findings(context(head, base)) + + assert finding.severity is FindingSeverity.HIGH + assert "package from 'postmark-mcp' to 'postmark-mcpp'" in finding.message + + def test_a_changed_command_is_high(tmp_path: Path) -> None: base, head = repos(tmp_path) _server(base, ["server.js"], command="node")