From 0c4bca0d4e4bf279ad9ca2b81cf5e26acf013c0d Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 22 Sep 2026 09:30:17 +0530 Subject: [PATCH 1/6] feat(vetted-ops): add stdlib http read backend (#1320) --- tools/vetted-ops/src/vetted_ops/cli.py | 97 +++++++++++++++++++---- tools/vetted-ops/src/vetted_ops/config.py | 18 +++++ tools/vetted-ops/src/vetted_ops/ops.py | 41 +++++++++- tools/vetted-ops/tests/test_vetted_ops.py | 66 +++++++++++---- 4 files changed, 190 insertions(+), 32 deletions(-) diff --git a/tools/vetted-ops/src/vetted_ops/cli.py b/tools/vetted-ops/src/vetted_ops/cli.py index 6d742a381..db48eeff4 100644 --- a/tools/vetted-ops/src/vetted_ops/cli.py +++ b/tools/vetted-ops/src/vetted_ops/cli.py @@ -45,8 +45,11 @@ from __future__ import annotations import argparse +import json import subprocess import sys +import urllib.error +import urllib.request from collections.abc import Sequence from pathlib import Path @@ -106,18 +109,32 @@ def _validate_params( resolved[name] = ops_mod.ghsa(raw) elif name in {"item_id", "content_id"}: resolved[name] = ops_mod.node_id(raw) + elif name == "vuln_id": + resolved[name] = ops_mod.vuln_id(raw) + elif name == "package_name": + resolved[name] = ops_mod.package_name(raw) + elif name == "version": + resolved[name] = ops_mod.version(raw) + elif name == "commit_hash": + resolved[name] = ops_mod.commit_hash(raw) + elif name == "cve_id": + resolved[name] = ops_mod.cve_id(raw) else: # pragma: no cover - guarded by the catalogue test raise ops_mod.ParamError(f"operation {op.name!r} declares unknown parameter {name!r}") return resolved, body -def build_argv(op: ops_mod.Op, params: dict[str, str], config: Config) -> list[str]: - argv = op.build(config.as_mapping(), **params) - if not isinstance(argv, list) or not all(isinstance(a, str) for a in argv): - raise ops_mod.ParamError(f"operation {op.name!r} produced a malformed argv") - if argv[0] != "gh": - raise ops_mod.ParamError(f"operation {op.name!r} tried to run {argv[0]!r}, not gh") - return argv +def build_argv(op: ops_mod.Op, params: dict[str, str], config: Config) -> list[str] | dict[str, object]: + result = op.build(config.as_mapping(), **params) + if op.backend == "gh": + if not isinstance(result, list) or not all(isinstance(a, str) for a in result): + raise ops_mod.ParamError(f"operation {op.name!r} produced a malformed argv") + if result[0] != "gh": + raise ops_mod.ParamError(f"operation {op.name!r} tried to run {result[0]!r}, not gh") + elif op.backend == "http-read": + if not isinstance(result, dict): + raise ops_mod.ParamError(f"operation {op.name!r} produced a malformed request descriptor") + return result def _reject_repeated_caller(argv: Sequence[str]) -> None: @@ -202,16 +219,68 @@ def main(argv: list[str] | None = None, *, read_only: bool = False) -> int: return EXIT_POLICY if args.dry_run: - print(" ".join(command)) + if op.backend == "gh": + assert isinstance(command, list) + print(" ".join(command)) + else: + assert isinstance(command, dict) + print(f"{command.get('method', 'GET')} {command.get('url')}") + if command.get("body"): + print("Body:", command["body"]) return EXIT_OK - # No shell. The argv list is passed through verbatim, and any body travels - # on stdin as bytes we already read — `gh` opens no file of ours. - completed = subprocess.run(command, check=False, input=body) - if completed.returncode != EXIT_OK: - print(f"vetted-op: {op.name} failed (gh exit {completed.returncode})", file=sys.stderr) + if op.backend == "gh": + assert isinstance(command, list) + # No shell. The argv list is passed through verbatim, and any body travels + # on stdin as bytes we already read — `gh` opens no file of ours. + completed = subprocess.run(command, check=False, input=body) + if completed.returncode != EXIT_OK: + print(f"vetted-op: {op.name} failed (gh exit {completed.returncode})", file=sys.stderr) + return EXIT_COMMAND + return EXIT_OK + elif op.backend == "http-read": + assert isinstance(command, dict) + return _run_http(command, body=body) + else: # pragma: no cover + raise ops_mod.ParamError(f"unknown backend {op.backend!r}") + + +def _run_http(request_desc: dict[str, object], *, body: bytes | None) -> int: + """Execute an HTTP read operation.""" + url = request_desc.get("url") + method = request_desc.get("method", "GET") + headers = request_desc.get("headers", {}) + if not isinstance(headers, dict): + print("vetted-op: internal error: http headers must be a dict", file=sys.stderr) + return EXIT_COMMAND + + if not isinstance(url, str): + print("vetted-op: internal error: http request missing url", file=sys.stderr) + return EXIT_COMMAND + + # Optional request body from the descriptor + desc_body = request_desc.get("body") + payload: bytes | None = None + if desc_body is not None: + payload = desc_body.encode("utf-8") if isinstance(desc_body, str) else desc_body # type: ignore[assignment] + elif body is not None: + payload = body + + req = urllib.request.Request(url, data=payload, method=str(method)) + for k, v in headers.items(): + req.add_header(str(k), str(v)) + + try: + with urllib.request.urlopen(req) as response: + result = response.read() + sys.stdout.buffer.write(result) + return EXIT_OK + except urllib.error.HTTPError as exc: + print(f"vetted-op: http request failed with {exc.code} {exc.reason}", file=sys.stderr) + return EXIT_COMMAND + except urllib.error.URLError as exc: + print(f"vetted-op: http request failed: {exc.reason}", file=sys.stderr) return EXIT_COMMAND - return EXIT_OK def main_read(argv: list[str] | None = None) -> int: diff --git a/tools/vetted-ops/src/vetted_ops/config.py b/tools/vetted-ops/src/vetted_ops/config.py index 497f602dd..d85e60639 100644 --- a/tools/vetted-ops/src/vetted_ops/config.py +++ b/tools/vetted-ops/src/vetted_ops/config.py @@ -56,12 +56,15 @@ class Config: workspace: Path #: Enum name -> permitted values. Board columns map name -> option id. values: dict[str, object] + #: Endpoint name -> URL. Defaults are provided for canonical OSV/CVE APIs. + endpoints: dict[str, str] #: Caller name -> permitted operation names. callers: dict[str, frozenset[str]] def as_mapping(self) -> dict[str, object]: """The mapping handed to an operation's ``build`` callable.""" merged: dict[str, object] = dict(self.values) + merged.update(self.endpoints) merged["tracker_repo"] = self.tracker_repo merged["upstream_repo"] = self.upstream_repo return merged @@ -140,11 +143,25 @@ def load(path: Path | None = None, *, cwd: Path | None = None) -> Config: raise ConfigError(f"callers.{name} must be a list of operation names") callers[name] = frozenset(ops) + endpoints_raw = raw.get("endpoints", {}) + if not isinstance(endpoints_raw, dict): + raise ConfigError("[endpoints] must be a table") + + endpoints = { + "osv_api": "https://api.osv.dev/v1", + "cve_services_api": "https://cveawg.mitre.org/api", + } + for name, url in endpoints_raw.items(): + if not isinstance(url, str): + raise ConfigError(f"endpoints.{name} must be a URL string") + endpoints[name] = url.rstrip("/") + return Config( tracker_repo=_optional_repo(repos, "tracker"), upstream_repo=_require_repo(repos, "upstream"), workspace=workspace, values=dict(values_raw), + endpoints=endpoints, callers=callers, ) @@ -157,6 +174,7 @@ def describe(config: Config) -> str: "upstream_repo": config.upstream_repo, "workspace": str(config.workspace), "values": {k: (sorted(v) if isinstance(v, dict) else v) for k, v in config.values.items()}, + "endpoints": dict(sorted(config.endpoints.items())), "callers": {k: sorted(v) for k, v in sorted(config.callers.items())}, }, indent=2, diff --git a/tools/vetted-ops/src/vetted_ops/ops.py b/tools/vetted-ops/src/vetted_ops/ops.py index 67132e7d8..9f4f34c01 100644 --- a/tools/vetted-ops/src/vetted_ops/ops.py +++ b/tools/vetted-ops/src/vetted_ops/ops.py @@ -71,6 +71,21 @@ #: else even before the containment check below. _QUERY_NAME = re.compile(r"^[a-z][a-z0-9-]{0,60}$") +#: An OSV vulnerability ID or similar identifier (CVE, GHSA, etc). +_VULN_ID = re.compile(r"^[A-Za-z0-9][-A-Za-z0-9]{2,60}$") + +#: An ecosystem package name. Supports scoped npm packages (@scope/name). +_PACKAGE_NAME = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._@/-]{0,200}$") + +#: A package version string. +_VERSION = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._+~-]{0,100}$") + +#: A full or short git commit hash. +_COMMIT_HASH = re.compile(r"^[0-9a-f]{7,40}$") + +#: A strict CVE identifier. +_CVE_ID = re.compile(r"^CVE-[0-9]{4}-[0-9]{4,}$") + #: Where the allowlisted GraphQL documents live. Shipping them as files inside #: the package — rather than accepting query text as a parameter — is what keeps #: the GraphQL surface closed: a caller selects a query, it never supplies one. @@ -176,6 +191,26 @@ def query_name(value: str) -> str: return str(path) +def vuln_id(value: str) -> str: + return _check(_VULN_ID, value, "vuln id") + + +def package_name(value: str) -> str: + return _check(_PACKAGE_NAME, value, "package name") + + +def version(value: str) -> str: + return _check(_VERSION, value, "version") + + +def commit_hash(value: str) -> str: + return _check(_COMMIT_HASH, value, "commit hash") + + +def cve_id(value: str) -> str: + return _check(_CVE_ID, value, "CVE id") + + def read_body(value: str, *, workspace: Path) -> bytes: """ Validate and read body text, returning its **content**. @@ -261,8 +296,10 @@ class Op: name: str #: Parameter names, in positional order. params: tuple[str, ...] - #: Builds the argv. Receives resolved config plus validated parameters. - build: Callable[..., list[str]] + #: Builds the argv or request descriptor. Receives resolved config plus validated parameters. + build: Callable[..., list[str] | dict[str, object]] + #: Execution backend. "gh" returns an argv list; "http-read" returns a request descriptor dict. + backend: str = "gh" #: True when the operation changes state visible outside the machine. writes: bool = False #: Human-readable one-liner for `list-ops`. diff --git a/tools/vetted-ops/tests/test_vetted_ops.py b/tools/vetted-ops/tests/test_vetted_ops.py index 01844e4f3..ebc15b1c2 100644 --- a/tools/vetted-ops/tests/test_vetted_ops.py +++ b/tools/vetted-ops/tests/test_vetted_ops.py @@ -24,11 +24,11 @@ from vetted_ops import cli, config, ops CONFIG_TOML = """ -workspace = "{workspace}" +workspace = '{workspace}' [repos] -tracker = "acme/tracker" upstream = "acme/product" +tracker = "acme/tracker" [values] labels = ["needs triage", "cve allocated"] @@ -57,7 +57,7 @@ def policy_path(tmp_path: Path) -> Path: workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) return cfg_path @@ -67,7 +67,7 @@ def policy(tmp_path: Path) -> config.Config: workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) return config.load(cfg_path) @@ -95,6 +95,11 @@ def test_every_op_declares_validators_for_all_its_params() -> None: "item_id", "content_id", "title", + "vuln_id", + "package_name", + "version", + "commit_hash", + "cve_id", } for op in ops.OPS.values(): for param in op.params: @@ -104,7 +109,7 @@ def test_every_op_declares_validators_for_all_its_params() -> None: def test_every_builder_produces_a_gh_argv(policy: config.Config) -> None: - """No operation may invoke anything other than gh.""" + """Every 'gh' operation must invoke nothing other than gh.""" sample = { "number": "1", "comment_id": "1", @@ -126,14 +131,26 @@ def test_every_builder_produces_a_gh_argv(policy: config.Config) -> None: "reason": "completed", "column": "Assessed", "body": "unused", + "vuln_id": "OSV-2020-111", + "package_name": "pytest", + "version": "1.0.0", + "commit_hash": "a1b2c3d", + "cve_id": "CVE-2023-1234", + "ecosystem": "PyPI", } body = policy.workspace / "body.md" body.write_text("x") for op in ops.OPS.values(): params = {p: (str(body) if p in op.body_files else sample[p]) for p in op.params} - argv = op.build(policy.as_mapping(), **params) - assert argv[0] == "gh", op.name - assert all(isinstance(a, str) for a in argv), op.name + result = op.build(policy.as_mapping(), **params) + + if op.backend == "gh": + assert isinstance(result, list) + assert result[0] == "gh", op.name + assert all(isinstance(a, str) for a in result), op.name + elif op.backend == "http-read": + assert isinstance(result, dict) + assert isinstance(result.get("url"), str), op.name # --- parameters can never become commands ------------------------------------ @@ -173,6 +190,7 @@ def test_configured_label_is_accepted(policy: config.Config) -> None: op = ops.resolve("issue-add-label") params, _body = cli._validate_params(op, ["7", "cve allocated"], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert argv == [ "gh", "issue", @@ -203,6 +221,7 @@ def test_body_file_content_may_contain_anything(policy: config.Config) -> None: op = ops.resolve("issue-comment") params, sent = cli._validate_params(op, ["7", str(body)], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) # The body reaches `gh` on stdin, not as a path it opens for itself. assert argv[-2:] == ["--body-file", "-"] assert sent == b"`id` $(whoami) && rm -rf / ; drop table\n" @@ -219,6 +238,7 @@ def test_issue_edit_body_sends_the_body_on_stdin(policy: config.Config) -> None: op = ops.resolve("issue-edit-body") params, sent = cli._validate_params(op, ["611", str(body)], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert argv[:5] == ["gh", "issue", "edit", "611", "--repo"] assert argv[-2:] == ["--body-file", "-"] assert sent == b"### Affected versions\n\napache-airflow `< NEXT VERSION`\n" @@ -242,6 +262,7 @@ def test_issue_edit_title_passes_the_title_as_one_argv_element(policy: config.Co title = 'Session cookie overrides `Authorization`; enables "session fixation"' params, sent = cli._validate_params(op, ["555", title], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert argv[:5] == ["gh", "issue", "edit", "555", "--repo"] assert argv[-2:] == ["--title", title] assert sent is None @@ -288,6 +309,7 @@ def test_milestone_create_is_gated_on_the_configured_milestones(policy: config.C params, _ = cli._validate_params(op, ["1.2.3"], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert argv == ["gh", "api", "repos/acme/tracker/milestones", "-f", "title=1.2.3"] @@ -303,6 +325,7 @@ def test_issue_remove_assignee_is_gated_on_the_roster(policy: config.Config) -> params, _ = cli._validate_params(op, ["611", "alice"], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert argv == [ "gh", "issue", @@ -352,7 +375,7 @@ def test_no_read_operation_sends_fields_without_an_explicit_get(policy: config.C body.write_text("x") for name, op in ops.OPS.items(): - if op.writes: + if op.writes or op.backend != "gh": continue params = {} for p in op.params: @@ -363,6 +386,7 @@ def test_no_read_operation_sends_fields_without_an_explicit_get(policy: config.C else: params[p] = sample[p] argv = op.build(policy.as_mapping(), **params) + assert isinstance(argv, list) if "graphql" in argv: continue if any(a in ("-f", "-F") for a in argv): @@ -387,6 +411,7 @@ def test_repo_tree_refuses_to_report_a_truncated_listing(policy: config.Config) op = ops.resolve("repo-tree") params, _ = cli._validate_params(op, ["main"], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) jq = argv[argv.index("--jq") + 1] assert ".truncated" in jq, "repo-tree drops the API's truncation flag" @@ -404,6 +429,7 @@ def test_board_add_item_takes_a_content_node_id(policy: config.Config) -> None: op = ops.resolve("board-add-item") params, _ = cli._validate_params(op, ["I_kwDOabc123"], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert argv[:3] == ["gh", "api", "graphql"] assert "content=I_kwDOabc123" in argv assert "project=PVT_proj" in argv @@ -420,6 +446,7 @@ def test_board_archive_item_targets_the_configured_project(policy: config.Config op = ops.resolve("board-archive-item") params, _ = cli._validate_params(op, ["PVTI_kwDOabc"], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert "item=PVTI_kwDOabc" in argv assert "project=PVT_proj" in argv assert "archiveProjectV2Item" in argv[-1] @@ -434,6 +461,7 @@ def test_milestone_close_is_by_number_and_hits_the_tracker(policy: config.Config op = ops.resolve("milestone-close") params, _ = cli._validate_params(op, ["64"], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert argv == [ "gh", "api", @@ -458,7 +486,7 @@ def test_caller_may_not_run_an_operation_outside_its_manifest( workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) rc = run(["--caller", "security-issue-triage", "issue-close", "7", "completed"], cfg_path) assert rc == cli.EXIT_POLICY @@ -470,7 +498,7 @@ def test_unknown_caller_is_refused(tmp_path: Path, capsys: pytest.CaptureFixture workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) rc = run(["--caller", "not-a-skill", "issue-view", "7"], cfg_path) assert rc == cli.EXIT_POLICY @@ -482,7 +510,7 @@ def test_permitted_caller_reaches_dry_run(tmp_path: Path, capsys: pytest.Capture workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) rc = run(["--caller", "security-issue-sync", "issue-view", "7", "--dry-run"], cfg_path) assert rc == cli.EXIT_OK @@ -494,7 +522,7 @@ def test_caller_is_required(tmp_path: Path, capsys: pytest.CaptureFixture[str]) workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) rc = run(["issue-view", "7"], cfg_path) assert rc == cli.EXIT_USAGE @@ -508,6 +536,7 @@ def test_repo_cannot_be_influenced_by_a_parameter(policy: config.Config) -> None op = ops.resolve("issue-view") params, _body = cli._validate_params(op, ["7"], policy) argv = cli.build_argv(op, params, policy) + assert isinstance(argv, list) assert argv[argv.index("--repo") + 1] == "acme/tracker" @@ -845,6 +874,7 @@ def test_pr_searches_are_a_fixed_qualifier_not_a_query(policy: config.Config) -> ("pr-search-reviewed-by", "--reviewed-by"), ): argv = ops.OPS[name].build(policy.as_mapping(), login="alice") + assert isinstance(argv, list) assert argv[:3] == ["gh", "search", "prs"], name assert argv[argv.index(qualifier) + 1] == "alice", name # The repo is pinned by policy and the state is fixed open. @@ -856,6 +886,7 @@ def test_pr_searches_are_a_fixed_qualifier_not_a_query(policy: config.Config) -> def test_team_search_cannot_leave_the_upstream_org(policy: config.Config) -> None: argv = ops.OPS["pr-search-team-review-requested"].build(policy.as_mapping(), team="reviewers") + assert isinstance(argv, list) assert argv[argv.index("--review-requested") + 1] == "acme/reviewers" @@ -927,7 +958,7 @@ def trackerless(tmp_path: Path) -> config.Config: workspace.mkdir(parents=True) workspace.chmod(0o700) cfg_path = root / "config.toml" - cfg_path.write_text(CONFIG_NO_TRACKER.format(workspace=workspace)) + cfg_path.write_text(CONFIG_NO_TRACKER.format(workspace=workspace.as_posix())) return config.load(cfg_path) @@ -940,6 +971,7 @@ def test_upstream_operations_work_without_a_tracker(trackerless: config.Config) """The regression: these died at load time over a value they never read.""" assert ops.OPS["viewer"].build(trackerless.as_mapping()) == ["gh", "api", "user", "--jq", ".login"] argv = ops.OPS["pr-diff"].build(trackerless.as_mapping(), number="1") + assert isinstance(argv, list) assert argv[argv.index("--repo") + 1] == "acme/product" @@ -1005,7 +1037,7 @@ def test_upstream_is_still_required(tmp_path: Path) -> None: workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" cfg_path.write_text( - CONFIG_NO_TRACKER.format(workspace=workspace).replace('upstream = "acme/product"', "") + CONFIG_NO_TRACKER.format(workspace=workspace.as_posix()).replace('upstream = "acme/product"', "") ) with pytest.raises(config.ConfigError): config.load(cfg_path) @@ -1018,7 +1050,9 @@ def test_a_malformed_tracker_is_still_refused(tmp_path: Path) -> None: workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" cfg_path.write_text( - CONFIG_NO_TRACKER.format(workspace=workspace).replace("[repos]", '[repos]\ntracker = "not-a-repo"') + CONFIG_NO_TRACKER.format(workspace=workspace.as_posix()).replace( + "[repos]", '[repos]\ntracker = "not-a-repo"' + ) ) with pytest.raises(config.ConfigError): config.load(cfg_path) From 4637d0d499bf67c51d9ef7e5fe4d933e7a45708b Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 22 Sep 2026 09:41:15 +0530 Subject: [PATCH 2/6] feat(vetted-ops): register OSV and CVE.org HTTP read operations (#1320) --- tools/vetted-ops/src/vetted_ops/cli.py | 1 - tools/vetted-ops/src/vetted_ops/ops.py | 87 ++++++- tools/vetted-ops/tests/test_vetted_ops.py | 291 +++++++++++++++++++++- 3 files changed, 372 insertions(+), 7 deletions(-) diff --git a/tools/vetted-ops/src/vetted_ops/cli.py b/tools/vetted-ops/src/vetted_ops/cli.py index db48eeff4..632307e6e 100644 --- a/tools/vetted-ops/src/vetted_ops/cli.py +++ b/tools/vetted-ops/src/vetted_ops/cli.py @@ -45,7 +45,6 @@ from __future__ import annotations import argparse -import json import subprocess import sys import urllib.error diff --git a/tools/vetted-ops/src/vetted_ops/ops.py b/tools/vetted-ops/src/vetted_ops/ops.py index 9f4f34c01..fb1257477 100644 --- a/tools/vetted-ops/src/vetted_ops/ops.py +++ b/tools/vetted-ops/src/vetted_ops/ops.py @@ -30,6 +30,7 @@ from __future__ import annotations +import json import os import re import stat as stat_mod @@ -75,7 +76,7 @@ _VULN_ID = re.compile(r"^[A-Za-z0-9][-A-Za-z0-9]{2,60}$") #: An ecosystem package name. Supports scoped npm packages (@scope/name). -_PACKAGE_NAME = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._@/-]{0,200}$") +_PACKAGE_NAME = re.compile(r"^[A-Za-z0-9@][A-Za-z0-9._@/-]{0,200}$") #: A package version string. _VERSION = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._+~-]{0,100}$") @@ -192,14 +193,20 @@ def query_name(value: str) -> str: def vuln_id(value: str) -> str: + if ".." in value: + raise ParamError(f"path traversal in vuln id: {value!r}") return _check(_VULN_ID, value, "vuln id") def package_name(value: str) -> str: + if ".." in value: + raise ParamError(f"path traversal in package name: {value!r}") return _check(_PACKAGE_NAME, value, "package name") def version(value: str) -> str: + if ".." in value: + raise ParamError(f"path traversal in version: {value!r}") return _check(_VERSION, value, "version") @@ -208,6 +215,8 @@ def commit_hash(value: str) -> str: def cve_id(value: str) -> str: + if ".." in value: + raise ParamError(f"path traversal in CVE id: {value!r}") return _check(_CVE_ID, value, "CVE id") @@ -1829,6 +1838,82 @@ def build(cfg: dict[str, str], **params: str) -> list[str]: ) ) +# ---- http reads ----------------------------------------------------------- + +_register( + Op( + name="osv-get-vuln", + params=("vuln_id",), + backend="http-read", + summary="Read one vulnerability record from OSV by ID.", + build=lambda cfg, vuln_id: { + "url": f"{cfg['osv_api']}/vulns/{vuln_id}", + "method": "GET", + }, + ) +) + +_register( + Op( + name="osv-query-package", + params=("package_name", "ecosystem", "version"), + backend="http-read", + summary="Query OSV for vulnerabilities affecting a package version.", + enums={"ecosystem": "ecosystems"}, + build=lambda cfg, package_name, ecosystem, version: { + "url": f"{cfg['osv_api']}/query", + "method": "POST", + "headers": {"Content-Type": "application/json"}, + "body": json.dumps( + {"package": {"name": package_name, "ecosystem": ecosystem}, "version": version} + ), + }, + ) +) + +_register( + Op( + name="osv-query-commit", + params=("commit_hash",), + backend="http-read", + summary="Query OSV for vulnerabilities affecting a commit hash.", + build=lambda cfg, commit_hash: { + "url": f"{cfg['osv_api']}/query", + "method": "POST", + "headers": {"Content-Type": "application/json"}, + "body": json.dumps({"commit": commit_hash}), + }, + ) +) + +_register( + Op( + name="osv-query-batch", + params=("body",), + backend="http-read", + summary="Batch query OSV for vulnerabilities affecting multiple packages or commits.", + body_files=("body",), + build=lambda cfg, body: { + "url": f"{cfg['osv_api']}/querybatch", + "method": "POST", + "headers": {"Content-Type": "application/json"}, + }, + ) +) + +_register( + Op( + name="cve-check-published", + params=("cve_id",), + backend="http-read", + summary="Fetch CVE publication state and record from CVE.org services API.", + build=lambda cfg, cve_id: { + "url": f"{cfg['cve_services_api']}/cve/{cve_id}", + "method": "GET", + }, + ) +) + def resolve(name: str) -> Op: try: diff --git a/tools/vetted-ops/tests/test_vetted_ops.py b/tools/vetted-ops/tests/test_vetted_ops.py index ebc15b1c2..f1f08c711 100644 --- a/tools/vetted-ops/tests/test_vetted_ops.py +++ b/tools/vetted-ops/tests/test_vetted_ops.py @@ -17,6 +17,8 @@ # under the License. from __future__ import annotations +import json +import os from pathlib import Path import pytest @@ -38,6 +40,7 @@ pr_states = ["open", "closed", "merged", "all"] upstream_labels = ["ready for maintainer review", "area:scheduler"] close_reasons = ["completed", "not planned"] +ecosystems = ["PyPI", "Maven", "npm"] board_project_id = "PVT_proj" board_status_field_id = "PVTSSF_field" @@ -45,8 +48,9 @@ "Assessed" = "opt_assessed" [callers] -"security-issue-sync" = ["issue-view", "issue-add-label", "issue-comment", "issue-close"] -"security-issue-triage" = ["issue-view"] +"security-issue-sync" = ["issue-view", "issue-add-label", "issue-comment", "issue-close", "cve-check-published"] +"security-issue-triage" = ["issue-view", "osv-get-vuln", "osv-query-package", "cve-check-published"] +"dependency-audit" = ["osv-query-package", "osv-query-commit", "osv-query-batch"] """ @@ -153,6 +157,233 @@ def test_every_builder_produces_a_gh_argv(policy: config.Config) -> None: assert isinstance(result.get("url"), str), op.name +def test_every_http_operation_is_read_only() -> None: + """HTTP operations are unprivileged reads by construction.""" + for op in ops.OPS.values(): + if op.backend == "http-read": + assert not op.writes, f"{op.name} has backend='http-read' but writes=True" + + +def test_osv_get_vuln_builder(policy: config.Config) -> None: + op = ops.resolve("osv-get-vuln") + params, _ = cli._validate_params(op, ["GHSA-7rjr-3q55-vv33"], policy) + req = cli.build_argv(op, params, policy) + assert isinstance(req, dict) + assert req["method"] == "GET" + assert req["url"] == "https://api.osv.dev/v1/vulns/GHSA-7rjr-3q55-vv33" + + +def test_osv_query_package_builder(policy: config.Config) -> None: + op = ops.resolve("osv-query-package") + params, _ = cli._validate_params(op, ["jinja2", "PyPI", "2.11.2"], policy) + req = cli.build_argv(op, params, policy) + assert isinstance(req, dict) + assert req["method"] == "POST" + assert req["url"] == "https://api.osv.dev/v1/query" + assert req["headers"] == {"Content-Type": "application/json"} + body_data = json.loads(str(req["body"])) + assert body_data == { + "package": {"name": "jinja2", "ecosystem": "PyPI"}, + "version": "2.11.2", + } + + +def test_osv_query_commit_builder(policy: config.Config) -> None: + op = ops.resolve("osv-query-commit") + params, _ = cli._validate_params(op, ["a1b2c3d4e5f67890"], policy) + req = cli.build_argv(op, params, policy) + assert isinstance(req, dict) + assert req["method"] == "POST" + assert req["url"] == "https://api.osv.dev/v1/query" + assert req["headers"] == {"Content-Type": "application/json"} + body_data = json.loads(str(req["body"])) + assert body_data == {"commit": "a1b2c3d4e5f67890"} + + +def test_osv_query_batch_builder(policy: config.Config, monkeypatch: pytest.MonkeyPatch) -> None: + body = policy.workspace / "batch.json" + body.write_text('{"queries": []}') + op = ops.resolve("osv-query-batch") + if not hasattr(os, "getuid"): + monkeypatch.setattr(ops, "read_body", lambda val, workspace: b'{"queries": []}') + params, sent = cli._validate_params(op, [str(body)], policy) + req = cli.build_argv(op, params, policy) + assert isinstance(req, dict) + assert req["method"] == "POST" + assert req["url"] == "https://api.osv.dev/v1/querybatch" + assert req["headers"] == {"Content-Type": "application/json"} + assert sent == b'{"queries": []}' + + +def test_cve_check_published_builder(policy: config.Config) -> None: + op = ops.resolve("cve-check-published") + params, _ = cli._validate_params(op, ["CVE-2023-1234"], policy) + req = cli.build_argv(op, params, policy) + assert isinstance(req, dict) + assert req["method"] == "GET" + assert req["url"] == "https://cveawg.mitre.org/api/cve/CVE-2023-1234" + + +@pytest.mark.parametrize( + "valid_id", + [ + "GHSA-7rjr-3q55-vv33", + "CVE-2021-45046", + "PYSEC-2021-123", + "RUSTSEC-2020-0001", + "GO-2022-0123", + "OSV-2020-111", + ], +) +def test_valid_vuln_ids_are_accepted(valid_id: str) -> None: + assert ops.vuln_id(valid_id) == valid_id + + +@pytest.mark.parametrize( + "hostile", + [ + "../../etc/passwd", + "GHSA; rm -rf /", + "GHSA $(whoami)", + "GHSA `id`", + "GHSA\nnewline", + "", + "x", + ], +) +def test_hostile_vuln_ids_are_refused(hostile: str) -> None: + with pytest.raises(ops.ParamError): + ops.vuln_id(hostile) + + +@pytest.mark.parametrize( + "valid_pkg", + [ + "jinja2", + "@scope/package", + "apache-airflow", + "github.com/gin-gonic/gin", + "osv.dev", + "pkg_name", + ], +) +def test_valid_package_names_are_accepted(valid_pkg: str) -> None: + assert ops.package_name(valid_pkg) == valid_pkg + + +@pytest.mark.parametrize( + "hostile", + [ + "../../etc/passwd", + "pkg; rm -rf /", + "pkg $(whoami)", + "pkg `id`", + "pkg\nnewline", + "", + ], +) +def test_hostile_package_names_are_refused(hostile: str) -> None: + with pytest.raises(ops.ParamError): + ops.package_name(hostile) + + +@pytest.mark.parametrize( + "valid_commit", + [ + "a1b2c3d", + "0123456789abcdef", + "a1b2c3d4e5f67890a1b2c3d4e5f67890a1b2c3d4", + ], +) +def test_valid_commits_are_accepted(valid_commit: str) -> None: + assert ops.commit_hash(valid_commit) == valid_commit + + +@pytest.mark.parametrize( + "invalid_commit", + [ + "a1b2c3", # too short (< 7) + "a1b2c3g", # non-hex + "A1B2C3D", # uppercase + "main", + "../../etc", + "", + ], +) +def test_invalid_commits_are_refused(invalid_commit: str) -> None: + with pytest.raises(ops.ParamError): + ops.commit_hash(invalid_commit) + + +@pytest.mark.parametrize( + "valid_cve", + [ + "CVE-2023-1234", + "CVE-1999-0001", + "CVE-2024-1234567", + ], +) +def test_valid_cve_ids_are_accepted(valid_cve: str) -> None: + assert ops.cve_id(valid_cve) == valid_cve + + +@pytest.mark.parametrize( + "invalid_cve", + [ + "cve-2023-1234", # lowercase + "CVE-23-1234", # 2-digit year + "CVE-2023-123", # 3-digit sequence + "GHSA-aaaa-bbbb-cccc", + "../../etc/passwd", + "", + ], +) +def test_invalid_cve_ids_are_refused(invalid_cve: str) -> None: + with pytest.raises(ops.ParamError): + ops.cve_id(invalid_cve) + + +def test_ecosystem_must_be_one_of_the_configured_values(policy: config.Config) -> None: + op = ops.resolve("osv-query-package") + with pytest.raises(ops.ParamError, match="not one of the configured values"): + cli._validate_params(op, ["jinja2", "UnknownEcosystem", "1.0.0"], policy) + + +def test_http_endpoints_can_be_customized(tmp_path: Path) -> None: + custom_toml = """ +workspace = '{workspace}' + +[repos] +upstream = "acme/product" + +[endpoints] +osv_api = "https://custom-osv.example.com/api" +cve_services_api = "https://custom-cve.example.com/api" + +[values] +ecosystems = ["PyPI"] + +[callers] +"security-issue-triage" = ["osv-get-vuln", "cve-check-published"] +""" + workspace = tmp_path / "scratch" + workspace.mkdir() + workspace.chmod(0o700) + cfg_path = tmp_path / "custom.toml" + cfg_path.write_text(custom_toml.format(workspace=workspace.as_posix())) + custom_cfg = config.load(cfg_path) + + op_osv = ops.resolve("osv-get-vuln") + req_osv = op_osv.build(custom_cfg.as_mapping(), vuln_id="OSV-1") + assert isinstance(req_osv, dict) + assert req_osv["url"] == "https://custom-osv.example.com/api/vulns/OSV-1" + + op_cve = ops.resolve("cve-check-published") + req_cve = op_cve.build(custom_cfg.as_mapping(), cve_id="CVE-2023-1234") + assert isinstance(req_cve, dict) + assert req_cve["url"] == "https://custom-cve.example.com/api/cve/CVE-2023-1234" + + # --- parameters can never become commands ------------------------------------ @@ -652,6 +883,8 @@ def test_tracker_and_upstream_operations_never_cross(policy: config.Config) -> N tracker, upstream = "acme/tracker", "acme/product" for name, op in ops.OPS.items(): + if op.backend != "gh": + continue params = {} for p in op.params: if p in op.body_files: @@ -660,7 +893,9 @@ def test_tracker_and_upstream_operations_never_cross(policy: config.Config) -> N params[p] = policy.enum_values(op.enums[p])[0] else: params[p] = sample[p] - argv = " ".join(op.build(policy.as_mapping(), **params)) + res = op.build(policy.as_mapping(), **params) + assert isinstance(res, list) + argv = " ".join(res) if name.startswith("repo-issue-") or name.startswith("pr-") or name.startswith("gql-"): assert tracker not in argv, f"{name} reached the tracker" elif name.startswith("issue-") or name in { @@ -722,6 +957,8 @@ def test_no_operation_interpolates_a_traversing_ref(policy: config.Config) -> No body = policy.workspace / "ref.md" body.write_text("x") for op in ops.OPS.values(): + if op.backend != "gh": + continue params = {} for p in op.params: if p in op.body_files: @@ -730,7 +967,9 @@ def test_no_operation_interpolates_a_traversing_ref(policy: config.Config) -> No params[p] = policy.enum_values(op.enums[p])[0] else: params[p] = sample[p] - for arg in op.build(policy.as_mapping(), **params): + res = op.build(policy.as_mapping(), **params) + assert isinstance(res, list) + for arg in res: assert "/../" not in arg and not arg.endswith("/.."), op.name @@ -821,6 +1060,44 @@ def test_read_dispatcher_still_runs_reads(policy_path: Path, capsys: pytest.Capt assert "gh issue view 7" in capsys.readouterr().out +def test_read_dispatcher_runs_http_reads(policy_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + rc = cli.main( + [ + "--caller", + "security-issue-triage", + "osv-get-vuln", + "GHSA-7rjr-3q55-vv33", + "--config", + str(policy_path), + "--dry-run", + ], + read_only=True, + ) + assert rc == cli.EXIT_OK + out = capsys.readouterr().out + assert "GET https://api.osv.dev/v1/vulns/GHSA-7rjr-3q55-vv33" in out + + rc_post = cli.main( + [ + "--caller", + "security-issue-triage", + "osv-query-package", + "jinja2", + "PyPI", + "2.11.2", + "--config", + str(policy_path), + "--dry-run", + ], + read_only=True, + ) + assert rc_post == cli.EXIT_OK + out_post = capsys.readouterr().out + assert "POST https://api.osv.dev/v1/query" in out_post + assert "Body:" in out_post + assert '"jinja2"' in out_post + + # -------------------------------------------------------------------------- # Body files: single open, owned workspace, no symlinks # -------------------------------------------------------------------------- @@ -1018,10 +1295,14 @@ def args_for(op: ops.Op) -> dict[str, str]: checked = 0 for name, op in ops.OPS.items(): + if op.backend != "gh": + continue args = args_for(op) if not args and op.params: continue - with_tracker = " ".join(op.build(policy.as_mapping(), **args)) + res = op.build(policy.as_mapping(), **args) + assert isinstance(res, list) + with_tracker = " ".join(res) if "acme/tracker" not in with_tracker: continue checked += 1 From ca6209fb4b826e4a9e71552abb490b5f955fcba4 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 22 Sep 2026 10:05:16 +0530 Subject: [PATCH 3/6] docs(tools): document HTTP backend and endpoints --- tools/cve-org/README.md | 6 +++--- tools/cve-org/tool.md | 4 ++-- tools/osv/README.md | 8 +++---- tools/osv/tool.md | 21 +++++------------- tools/vetted-ops/README.md | 44 +++++++++++++++++++++++++++----------- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/tools/cve-org/README.md b/tools/cve-org/README.md index 8fb06a842..6151ba91d 100644 --- a/tools/cve-org/README.md +++ b/tools/cve-org/README.md @@ -23,10 +23,10 @@ CVE.org publication client. Submits CVE records via the CVE.org REST API; consum ## Prerequisites -- **Runtime:** None of its own — this directory documents a read-only adapter; the publication-state check is a `curl` + `jq` one-liner (see `tool.md`). -- **CLIs:** `curl` and `jq`. +- **Runtime:** Python 3.11+ via `uv` (through `tools/vetted-ops` dispatcher). +- **CLIs:** `vetted-op-read` (from `tools/vetted-ops`) and `jq`. - **Credentials / auth:** None — cve.org is the public CVE registry and the CVE Services API has no auth. -- **Network:** `cveawg.mitre.org` (CVE Services API v2) and `www.cve.org` (the HTML record); optionally `nvd.nist.gov` for the alternative public registry. +- **Network:** `cveawg.mitre.org` (CVE Services API v2), routed through `vetted-ops` HTTP read backend. ## Configuration diff --git a/tools/cve-org/tool.md b/tools/cve-org/tool.md index 19f025e2c..f03597191 100644 --- a/tools/cve-org/tool.md +++ b/tools/cve-org/tool.md @@ -66,7 +66,7 @@ Recipe: ```bash # Read-only, no auth required. Returns JSON. -curl -sSf https://cveawg.mitre.org/api/cve/ \ +vetted-op-read --caller security-issue-sync cve-check-published \ | jq -r '.cveMetadata.state' ``` @@ -90,7 +90,7 @@ Extract the `datePublished` alongside the state when you need to print *"published on YYYY-MM-DD"* in the reporter email: ```bash -curl -sSf https://cveawg.mitre.org/api/cve/ \ +vetted-op-read --caller security-issue-sync cve-check-published \ | jq -r '{state: .cveMetadata.state, datePublished: .cveMetadata.datePublished}' ``` diff --git a/tools/osv/README.md b/tools/osv/README.md index 6418c270f..3249b6d95 100644 --- a/tools/osv/README.md +++ b/tools/osv/README.md @@ -26,10 +26,10 @@ See [`tool.md`](tool.md) for endpoint recipes, payload structures, and confident ## Prerequisites -- **Runtime:** None of its own — this directory documents a read-only adapter; queries are `curl` + `jq` recipes (see `tool.md`). -- **CLIs:** `curl` and `jq`. -- **Credentials / auth:** None — OSV.dev is a public vulnerability database with an open, unauthenticated REST API. -- **Network:** `api.osv.dev` (OSV.dev REST API v1) and `osv.dev` (public web UI). +- **Runtime:** Python 3.11+ via `uv` (through `tools/vetted-ops` dispatcher). +- **CLIs:** `vetted-op-read` (from `tools/vetted-ops`) and `jq`. +- **Credentials / auth:** None — open, unauthenticated REST API. +- **Network:** `api.osv.dev` (REST API v1), routed through `vetted-ops` HTTP read backend. ## Configuration diff --git a/tools/osv/tool.md b/tools/osv/tool.md index 0e193d7b5..0af106932 100644 --- a/tools/osv/tool.md +++ b/tools/osv/tool.md @@ -72,7 +72,7 @@ curl -sSf https://api.osv.dev/v1/vulns/ Extracting alias identifiers (e.g., resolving a GHSA ID to corresponding CVE IDs): ```bash -curl -sSf https://api.osv.dev/v1/vulns/GHSA-7rjr-3q55-vv33 \ +vetted-op-read --caller security-issue-triage osv-get-vuln GHSA-7rjr-3q55-vv33 \ | jq -r '{id: .id, aliases: .aliases, summary: .summary}' ``` @@ -90,7 +90,7 @@ Example JSON response: Extracting affected version ranges and fixed versions: ```bash -curl -sSf https://api.osv.dev/v1/vulns/ \ +vetted-op-read --caller security-issue-triage osv-get-vuln \ | jq -r '.affected[] | {package: .package.name, ecosystem: .package.ecosystem, fixed: [.ranges[].events[] | select(.fixed != null) | .fixed]}' ``` @@ -99,9 +99,7 @@ curl -sSf https://api.osv.dev/v1/vulns/ \ Check if a given package release is subject to any known advisories: ```bash -curl -sSf -X POST https://api.osv.dev/v1/query \ - -H "Content-Type: application/json" \ - -d '{"package": {"name": "jinja2", "ecosystem": "PyPI"}, "version": "2.11.2"}' \ +vetted-op-read --caller security-issue-triage osv-query-package jinja2 PyPI 2.11.2 \ | jq -r '.vulns[]? | {id: .id, aliases: .aliases, summary: .summary}' ``` @@ -112,9 +110,7 @@ Common ecosystems: `PyPI`, `Maven`, `npm`, `crates.io`, `Go`, `Packagist`, `NuGe Check if a public upstream commit SHA is indexed in OSV as a fix or vulnerability reference: ```bash -curl -sSf -X POST https://api.osv.dev/v1/query \ - -H "Content-Type: application/json" \ - -d '{"commit": ""}' \ +vetted-op-read --caller dependency-audit osv-query-commit \ | jq -r '.vulns[]? | {id: .id, aliases: .aliases, summary: .summary}' ``` @@ -123,14 +119,7 @@ curl -sSf -X POST https://api.osv.dev/v1/query \ Evaluate multiple dependencies in a single round-trip: ```bash -curl -sSf -X POST https://api.osv.dev/v1/querybatch \ - -H "Content-Type: application/json" \ - -d '{ - "queries": [ - {"package": {"name": "jinja2", "ecosystem": "PyPI"}, "version": "2.11.2"}, - {"package": {"name": "urllib3", "ecosystem": "PyPI"}, "version": "1.26.4"} - ] - }' \ +vetted-op-read --caller dependency-audit osv-query-batch /tmp/agent-scratch/batch.json \ | jq -r '.results | to_entries[] | {query: .key, vuln_count: ((.value.vulns // []) | length)}' ``` diff --git a/tools/vetted-ops/README.md b/tools/vetted-ops/README.md index 897b6534b..5c7eac44f 100644 --- a/tools/vetted-ops/README.md +++ b/tools/vetted-ops/README.md @@ -32,14 +32,16 @@ needs *one* allowlist entry instead of a dozen wildcard `ask` rules. ## Prerequisites -- **Runtime:** Python 3.11+ via `uv`. The package itself is stdlib-only. -- **CLIs:** the `gh` CLI on `PATH`, authenticated for the repositories the policy - names. Every operation shells out to it; the dispatcher runs nothing else. -- **Credentials:** whatever `gh` already uses (`~/.config/gh/`). This tool reads - no credential of its own and stores none. -- **Network:** only what `gh` needs — `github.com` / `api.github.com`. -- **Configuration:** a policy TOML (see *Configuration*). Without one, every - operation refuses. +- **Runtime:** Python 3.11+ via `uv`. +The package itself is stdlib-only. +- **CLIs:** the `gh` CLI on `PATH`, authenticated for the repositories the policy names. +HTTP operations do not use `gh` or `curl`; they use the `urllib.request` standard library module. +- **Credentials:** whatever `gh` already uses (`~/.config/gh/`). +This tool reads no credential of its own and stores none. +- **Network:** `github.com` / `api.github.com` for `gh` operations. +HTTP operations connect to the domains configured in the `[endpoints]` table (e.g., `api.osv.dev`, `cveawg.mitre.org`). +- **Configuration:** a policy TOML (see *Configuration*). +Without one, every operation refuses. ## Why @@ -98,9 +100,16 @@ Being precise, because a security tool that overstates itself is worse than none supplies the text, and `owner`/`name` come from policy. A caller can choose among the allowlisted queries; it cannot write one, and cannot re-aim one at another repository. -- **The catalogue is closed.** Widening the surface means editing - [`ops.py`](src/vetted_ops/ops.py) — a reviewed code change, not a runtime - decision. +- **A parameter can never become a URL path traversal.** +For HTTP operations, URLs are built from closed templates and injected parameters are strictly validated to refuse `..` and shell characters. +- **HTTP operations are read-only by construction.** +The backend `"http-read"` implies `writes=False`, enforced by the dispatcher. +- **No `curl` or `wget` is involved.** +The `urllib.request` implementation automatically obeys `HTTP_PROXY` and `HTTPS_PROXY` environment variables (egress gateway). +- **HTTP responses are streamed to stdout, never to files.** +There is no local filesystem exposure for downloaded data. +- **The catalogue is closed.** +Widening the surface means editing [`ops.py`](src/vetted_ops/ops.py) — a reviewed code change, not a runtime decision. ### The boundary is the entry point, not `--caller` @@ -157,6 +166,10 @@ Adopter-owned, at # Body files must resolve inside this directory. workspace = "/tmp/agent-scratch" +[endpoints] +osv_api = "https://api.osv.dev/v1" +cve_services_api = "https://cveawg.mitre.org/api" + [repos] tracker = "acme/tracker" # optional — see below upstream = "acme/product" @@ -169,6 +182,7 @@ assignees = ["alice", "bob"] issue_states = ["open", "closed", "all"] pr_states = ["open", "closed", "merged", "all"] close_reasons = ["completed", "not planned"] +ecosystems = ["PyPI", "Maven", "npm", "Go"] board_project_id = "PVT_kwDO…" # ProjectV2 node id board_status_field_id = "PVTSSF_…" # its Status field id @@ -179,8 +193,12 @@ board_status_field_id = "PVTSSF_…" # its Status field id [callers] # caller -> operations it may run "security-issue-sync" = ["issue-view", "issue-comments", "issue-add-label", - "issue-set-milestone", "issue-comment", "comment-update"] -"security-issue-triage" = ["issue-view", "issue-comments"] + "issue-set-milestone", "issue-comment", "comment-update", + "cve-check-published"] +"security-issue-triage" = ["issue-view", "issue-comments", + "osv-get-vuln", "osv-query-package"] +"security-issue-deduplicate" = ["osv-get-vuln"] +"dependency-audit" = ["osv-query-package", "osv-query-commit", "osv-query-batch"] "pr-management-triage" = ["pr-list", "pr-view", "pr-checks", "gql-pr-liveness", "pr-add-label", "pr-remove-label", "pr-draft", "pr-ready", "pr-comment", "pr-update-branch", "run-rerun-failed", From cf0ca7d8921d84d45726389265dafafe4ca941db Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 22 Sep 2026 21:43:35 +0530 Subject: [PATCH 4/6] fix(vetted-ops): address review feedback on HTTP backend and allowlists (#1320) --- .claude/settings.json | 1 + docs/setup/secure-agent-setup.md | 2 +- .../skills/issue-sync/gather.md | 4 +- tools/cve-org/README.md | 2 +- tools/cve-org/tool.md | 6 +- tools/osv/tool.md | 12 +- tools/sandbox-lint/expected.json | 1 + .../spec-loop/specs/vetted-command-surface.md | 6 +- tools/vetted-ops/README.md | 2 + tools/vetted-ops/src/vetted_ops/cli.py | 24 +++- tools/vetted-ops/src/vetted_ops/config.py | 2 + tools/vetted-ops/src/vetted_ops/ops.py | 14 +- tools/vetted-ops/tests/test_vetted_ops.py | 132 +++++++++++++++--- 13 files changed, 160 insertions(+), 48 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index d8015326c..67a59db90 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -48,6 +48,7 @@ "cve.org", "www.cve.org", "cveawg.mitre.org", + "api.osv.dev", "oauth2.googleapis.com", "gmail.googleapis.com", "*.crates.io", diff --git a/docs/setup/secure-agent-setup.md b/docs/setup/secure-agent-setup.md index 6648ae00a..3624c06f0 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -536,7 +536,7 @@ below, annotated. "objects.githubusercontent.com", "codeload.github.com", "uploads.github.com", "pypi.org", "files.pythonhosted.org", "lists.apache.org", "dist.apache.org", "downloads.apache.org", "archive.apache.org", - "cveprocess.apache.org", "cve.org", "www.cve.org", "cveawg.mitre.org", + "cveprocess.apache.org", "cve.org", "www.cve.org", "cveawg.mitre.org", "api.osv.dev", "oauth2.googleapis.com", "gmail.googleapis.com", // `*.crates.io` + `static.rust-lang.org` let the `lychee` rust // hook bootstrap a rustup toolchain and `cargo install` lychee diff --git a/plugins/magpie-security/skills/issue-sync/gather.md b/plugins/magpie-security/skills/issue-sync/gather.md index 72d3a5292..9d8b0aef9 100644 --- a/plugins/magpie-security/skills/issue-sync/gather.md +++ b/plugins/magpie-security/skills/issue-sync/gather.md @@ -767,7 +767,7 @@ Concretely, for each closed-`announced` tracker in this run: already read). 2. Call the API: ```bash - curl -sSf https://cveawg.mitre.org/api/cve/ \ + vetted-op-read --caller security-issue-sync cve-check-published \ | jq -r '{state: .cveMetadata.state, datePublished: .cveMetadata.datePublished}' ``` 3. Interpret: @@ -779,7 +779,7 @@ Concretely, for each closed-`announced` tracker in this run: - `state == "REJECTED"` → **surface as a blocker**. The record was withdrawn post-publication. Do not draft a reporter email; flag to the security team. - - `curl` error (404 / 5xx / DNS) → record *"cve.org lookup + - lookup error (non-zero exit from `vetted-op-read` — exit 4) → record *"cve.org lookup failed — — try again next sync"*. Do not propose notification on an absent response. diff --git a/tools/cve-org/README.md b/tools/cve-org/README.md index 6151ba91d..9a5db53ba 100644 --- a/tools/cve-org/README.md +++ b/tools/cve-org/README.md @@ -26,7 +26,7 @@ CVE.org publication client. Submits CVE records via the CVE.org REST API; consum - **Runtime:** Python 3.11+ via `uv` (through `tools/vetted-ops` dispatcher). - **CLIs:** `vetted-op-read` (from `tools/vetted-ops`) and `jq`. - **Credentials / auth:** None — cve.org is the public CVE registry and the CVE Services API has no auth. -- **Network:** `cveawg.mitre.org` (CVE Services API v2), routed through `vetted-ops` HTTP read backend. +- **Network:** `cveawg.mitre.org` (CVE Services API v2), routed through `vetted-ops` HTTP read backend; `www.cve.org` (the HTML record); optionally `nvd.nist.gov` for the alternative public registry. ## Configuration diff --git a/tools/cve-org/tool.md b/tools/cve-org/tool.md index f03597191..361dea319 100644 --- a/tools/cve-org/tool.md +++ b/tools/cve-org/tool.md @@ -66,7 +66,7 @@ Recipe: ```bash # Read-only, no auth required. Returns JSON. -vetted-op-read --caller security-issue-sync cve-check-published \ +vetted-op-read --caller cve-check-published \ | jq -r '.cveMetadata.state' ``` @@ -81,7 +81,7 @@ Interpretation: *CVE-published* email to the reporter. - `REJECTED` → something went wrong post-publication. Surface to the security team; do not notify the reporter on the happy path. -- Non-zero exit from `curl` (404, 5xx, DNS failure) → treat as +- Non-zero exit from `vetted-op-read` (exit 4, covering 404, 5xx, DNS failure, timeout) → treat as *"unknown — try again next sync"*. Do not propose notification on an absent response; cve.org sometimes returns transient 5xx during CNA-feed propagation. @@ -90,7 +90,7 @@ Extract the `datePublished` alongside the state when you need to print *"published on YYYY-MM-DD"* in the reporter email: ```bash -vetted-op-read --caller security-issue-sync cve-check-published \ +vetted-op-read --caller cve-check-published \ | jq -r '{state: .cveMetadata.state, datePublished: .cveMetadata.datePublished}' ``` diff --git a/tools/osv/tool.md b/tools/osv/tool.md index 0af106932..4cfe7716d 100644 --- a/tools/osv/tool.md +++ b/tools/osv/tool.md @@ -66,13 +66,13 @@ Fetch the OSV JSON record by its primary ID (or alias): ```bash # Read-only, unauthenticated. Returns complete OSV schema JSON. -curl -sSf https://api.osv.dev/v1/vulns/ +vetted-op-read --caller osv-get-vuln ``` Extracting alias identifiers (e.g., resolving a GHSA ID to corresponding CVE IDs): ```bash -vetted-op-read --caller security-issue-triage osv-get-vuln GHSA-7rjr-3q55-vv33 \ +vetted-op-read --caller osv-get-vuln GHSA-7rjr-3q55-vv33 \ | jq -r '{id: .id, aliases: .aliases, summary: .summary}' ``` @@ -90,7 +90,7 @@ Example JSON response: Extracting affected version ranges and fixed versions: ```bash -vetted-op-read --caller security-issue-triage osv-get-vuln \ +vetted-op-read --caller osv-get-vuln \ | jq -r '.affected[] | {package: .package.name, ecosystem: .package.ecosystem, fixed: [.ranges[].events[] | select(.fixed != null) | .fixed]}' ``` @@ -99,7 +99,7 @@ vetted-op-read --caller security-issue-triage osv-get-vuln \ Check if a given package release is subject to any known advisories: ```bash -vetted-op-read --caller security-issue-triage osv-query-package jinja2 PyPI 2.11.2 \ +vetted-op-read --caller osv-query-package jinja2 PyPI 2.11.2 \ | jq -r '.vulns[]? | {id: .id, aliases: .aliases, summary: .summary}' ``` @@ -110,7 +110,7 @@ Common ecosystems: `PyPI`, `Maven`, `npm`, `crates.io`, `Go`, `Packagist`, `NuGe Check if a public upstream commit SHA is indexed in OSV as a fix or vulnerability reference: ```bash -vetted-op-read --caller dependency-audit osv-query-commit \ +vetted-op-read --caller osv-query-commit \ | jq -r '.vulns[]? | {id: .id, aliases: .aliases, summary: .summary}' ``` @@ -119,7 +119,7 @@ vetted-op-read --caller dependency-audit osv-query-commit \ Evaluate multiple dependencies in a single round-trip: ```bash -vetted-op-read --caller dependency-audit osv-query-batch /tmp/agent-scratch/batch.json \ +vetted-op-read --caller osv-query-batch /tmp/agent-scratch/batch.json \ | jq -r '.results | to_entries[] | {query: .key, vuln_count: ((.value.vulns // []) | length)}' ``` diff --git a/tools/sandbox-lint/expected.json b/tools/sandbox-lint/expected.json index d8015326c..67a59db90 100644 --- a/tools/sandbox-lint/expected.json +++ b/tools/sandbox-lint/expected.json @@ -48,6 +48,7 @@ "cve.org", "www.cve.org", "cveawg.mitre.org", + "api.osv.dev", "oauth2.googleapis.com", "gmail.googleapis.com", "*.crates.io", diff --git a/tools/spec-loop/specs/vetted-command-surface.md b/tools/spec-loop/specs/vetted-command-surface.md index 1d3fba8ae..77d13aa5b 100644 --- a/tools/spec-loop/specs/vetted-command-surface.md +++ b/tools/spec-loop/specs/vetted-command-surface.md @@ -25,8 +25,7 @@ Replace the wildcard, not the confirmation. Route forge actions through a dispatcher whose operations are a **closed catalogue** of fixed shapes: - parameters are typed and validated; none may start with `-`; -- builders return `list[str]` executed without a shell, so no parameter can - become a command or a flag; +- builders return `list[str]` executed without a shell for forge operations, or a request descriptor executed via Python stdlib `urllib.request` for HTTP read operations; - the repository is policy, never a parameter; - value-bearing parameters (labels, milestones, assignees, columns, close reasons) must appear in adopter-declared enums; @@ -112,7 +111,8 @@ per [`docs/adapters/registry.md`](../../../docs/adapters/registry.md). capability the surrounding design withholds. **Widening the catalogue must not widen the posture**, and that constraint binds every family added next. -3. **Adapter parity.** Operations are `gh`-shaped today. The forge is already an +3. **Adapter parity.** Forge operations are `gh`-shaped today (HTTP read operations + now ship alongside them via a stdlib backend). The forge is already an adapter axis (`github`, `jira`, `bitbucket`, `sourcehut`, `fossil`), so the catalogue should eventually resolve its builder per configured forge rather than assuming one. diff --git a/tools/vetted-ops/README.md b/tools/vetted-ops/README.md index 5c7eac44f..2914f1ead 100644 --- a/tools/vetted-ops/README.md +++ b/tools/vetted-ops/README.md @@ -159,6 +159,8 @@ Layers 0–2 are unchanged and still carry the load. ## Configuration +> **Upgrade Note:** Existing adopters upgrading to use `osv-query-package` must add `ecosystems = ["PyPI", "Maven", "npm", ...]` to the `[values]` table in their policy TOML; without it, `vetted-op-read` refuses package queries during parameter validation. + Adopter-owned, at `.apache-magpie-overrides/tools/vetted-ops/config.toml` by default: diff --git a/tools/vetted-ops/src/vetted_ops/cli.py b/tools/vetted-ops/src/vetted_ops/cli.py index 632307e6e..6200b2a93 100644 --- a/tools/vetted-ops/src/vetted_ops/cli.py +++ b/tools/vetted-ops/src/vetted_ops/cli.py @@ -219,17 +219,20 @@ def main(argv: list[str] | None = None, *, read_only: bool = False) -> int: if args.dry_run: if op.backend == "gh": - assert isinstance(command, list) + if not isinstance(command, list): + raise ops_mod.ParamError(f"operation {op.name!r} produced invalid command type") print(" ".join(command)) else: - assert isinstance(command, dict) + if not isinstance(command, dict): + raise ops_mod.ParamError(f"operation {op.name!r} produced invalid request descriptor") print(f"{command.get('method', 'GET')} {command.get('url')}") if command.get("body"): print("Body:", command["body"]) return EXIT_OK if op.backend == "gh": - assert isinstance(command, list) + if not isinstance(command, list): + raise ops_mod.ParamError(f"operation {op.name!r} produced invalid command type") # No shell. The argv list is passed through verbatim, and any body travels # on stdin as bytes we already read — `gh` opens no file of ours. completed = subprocess.run(command, check=False, input=body) @@ -238,7 +241,8 @@ def main(argv: list[str] | None = None, *, read_only: bool = False) -> int: return EXIT_COMMAND return EXIT_OK elif op.backend == "http-read": - assert isinstance(command, dict) + if not isinstance(command, dict): + raise ops_mod.ParamError(f"operation {op.name!r} produced invalid request descriptor") return _run_http(command, body=body) else: # pragma: no cover raise ops_mod.ParamError(f"unknown backend {op.backend!r}") @@ -253,8 +257,8 @@ def _run_http(request_desc: dict[str, object], *, body: bytes | None) -> int: print("vetted-op: internal error: http headers must be a dict", file=sys.stderr) return EXIT_COMMAND - if not isinstance(url, str): - print("vetted-op: internal error: http request missing url", file=sys.stderr) + if not isinstance(url, str) or not url.startswith("https://"): + print("vetted-op: internal error: http request missing valid https:// url", file=sys.stderr) return EXIT_COMMAND # Optional request body from the descriptor @@ -266,13 +270,16 @@ def _run_http(request_desc: dict[str, object], *, body: bytes | None) -> int: payload = body req = urllib.request.Request(url, data=payload, method=str(method)) + if "User-Agent" not in headers: + req.add_header("User-Agent", "apache-magpie-vetted-ops/0.1.0") for k, v in headers.items(): req.add_header(str(k), str(v)) try: - with urllib.request.urlopen(req) as response: + with urllib.request.urlopen(req, timeout=30) as response: result = response.read() sys.stdout.buffer.write(result) + sys.stdout.buffer.flush() return EXIT_OK except urllib.error.HTTPError as exc: print(f"vetted-op: http request failed with {exc.code} {exc.reason}", file=sys.stderr) @@ -280,6 +287,9 @@ def _run_http(request_desc: dict[str, object], *, body: bytes | None) -> int: except urllib.error.URLError as exc: print(f"vetted-op: http request failed: {exc.reason}", file=sys.stderr) return EXIT_COMMAND + except TimeoutError as exc: + print(f"vetted-op: http request timed out: {exc}", file=sys.stderr) + return EXIT_COMMAND def main_read(argv: list[str] | None = None) -> int: diff --git a/tools/vetted-ops/src/vetted_ops/config.py b/tools/vetted-ops/src/vetted_ops/config.py index d85e60639..09439cdc7 100644 --- a/tools/vetted-ops/src/vetted_ops/config.py +++ b/tools/vetted-ops/src/vetted_ops/config.py @@ -154,6 +154,8 @@ def load(path: Path | None = None, *, cwd: Path | None = None) -> Config: for name, url in endpoints_raw.items(): if not isinstance(url, str): raise ConfigError(f"endpoints.{name} must be a URL string") + if not url.startswith("https://"): + raise ConfigError(f"endpoints.{name} must use the https:// scheme (got {url!r})") endpoints[name] = url.rstrip("/") return Config( diff --git a/tools/vetted-ops/src/vetted_ops/ops.py b/tools/vetted-ops/src/vetted_ops/ops.py index fb1257477..bbbb78b54 100644 --- a/tools/vetted-ops/src/vetted_ops/ops.py +++ b/tools/vetted-ops/src/vetted_ops/ops.py @@ -73,16 +73,16 @@ _QUERY_NAME = re.compile(r"^[a-z][a-z0-9-]{0,60}$") #: An OSV vulnerability ID or similar identifier (CVE, GHSA, etc). -_VULN_ID = re.compile(r"^[A-Za-z0-9][-A-Za-z0-9]{2,60}$") +_VULN_ID = re.compile(r"^[A-Za-z0-9][-A-Za-z0-9:]{2,60}$") #: An ecosystem package name. Supports scoped npm packages (@scope/name). -_PACKAGE_NAME = re.compile(r"^[A-Za-z0-9@][A-Za-z0-9._@/-]{0,200}$") +_PACKAGE_NAME = re.compile(r"^[A-Za-z0-9@][A-Za-z0-9._@/:-]{0,200}$") #: A package version string. _VERSION = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._+~-]{0,100}$") #: A full or short git commit hash. -_COMMIT_HASH = re.compile(r"^[0-9a-f]{7,40}$") +_COMMIT_HASH = re.compile(r"^[0-9a-f]{7,64}$") #: A strict CVE identifier. _CVE_ID = re.compile(r"^CVE-[0-9]{4}-[0-9]{4,}$") @@ -193,8 +193,6 @@ def query_name(value: str) -> str: def vuln_id(value: str) -> str: - if ".." in value: - raise ParamError(f"path traversal in vuln id: {value!r}") return _check(_VULN_ID, value, "vuln id") @@ -215,8 +213,6 @@ def commit_hash(value: str) -> str: def cve_id(value: str) -> str: - if ".." in value: - raise ParamError(f"path traversal in CVE id: {value!r}") return _check(_CVE_ID, value, "CVE id") @@ -357,6 +353,10 @@ def _owner_name(repo: str) -> tuple[str, str]: def _register(op: Op) -> None: + if op.backend == "http-read" and op.writes: + raise ValueError( + f"operation {op.name!r} has backend='http-read' but writes=True; HTTP operations must be read-only" + ) OPS[op.name] = op diff --git a/tools/vetted-ops/tests/test_vetted_ops.py b/tools/vetted-ops/tests/test_vetted_ops.py index f1f08c711..91673375a 100644 --- a/tools/vetted-ops/tests/test_vetted_ops.py +++ b/tools/vetted-ops/tests/test_vetted_ops.py @@ -18,7 +18,8 @@ from __future__ import annotations import json -import os +import urllib.error +import urllib.request from pathlib import Path import pytest @@ -29,8 +30,8 @@ workspace = '{workspace}' [repos] -upstream = "acme/product" tracker = "acme/tracker" +upstream = "acme/product" [values] labels = ["needs triage", "cve allocated"] @@ -61,7 +62,7 @@ def policy_path(tmp_path: Path) -> Path: workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) return cfg_path @@ -71,7 +72,7 @@ def policy(tmp_path: Path) -> config.Config: workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) return config.load(cfg_path) @@ -200,12 +201,10 @@ def test_osv_query_commit_builder(policy: config.Config) -> None: assert body_data == {"commit": "a1b2c3d4e5f67890"} -def test_osv_query_batch_builder(policy: config.Config, monkeypatch: pytest.MonkeyPatch) -> None: +def test_osv_query_batch_builder(policy: config.Config) -> None: body = policy.workspace / "batch.json" body.write_text('{"queries": []}') op = ops.resolve("osv-query-batch") - if not hasattr(os, "getuid"): - monkeypatch.setattr(ops, "read_body", lambda val, workspace: b'{"queries": []}') params, sent = cli._validate_params(op, [str(body)], policy) req = cli.build_argv(op, params, policy) assert isinstance(req, dict) @@ -228,6 +227,10 @@ def test_cve_check_published_builder(policy: config.Config) -> None: "valid_id", [ "GHSA-7rjr-3q55-vv33", + "RHSA-2021:4321", + "SUSE-SU-2021:1234-1", + "ALSA-2021:1234", + "RLSA-2021:1234", "CVE-2021-45046", "PYSEC-2021-123", "RUSTSEC-2020-0001", @@ -262,6 +265,7 @@ def test_hostile_vuln_ids_are_refused(hostile: str) -> None: "jinja2", "@scope/package", "apache-airflow", + "org.apache.logging.log4j:log4j-core", "github.com/gin-gonic/gin", "osv.dev", "pkg_name", @@ -370,7 +374,7 @@ def test_http_endpoints_can_be_customized(tmp_path: Path) -> None: workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "custom.toml" - cfg_path.write_text(custom_toml.format(workspace=workspace.as_posix())) + cfg_path.write_text(custom_toml.format(workspace=workspace)) custom_cfg = config.load(cfg_path) op_osv = ops.resolve("osv-get-vuln") @@ -717,7 +721,7 @@ def test_caller_may_not_run_an_operation_outside_its_manifest( workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) rc = run(["--caller", "security-issue-triage", "issue-close", "7", "completed"], cfg_path) assert rc == cli.EXIT_POLICY @@ -729,7 +733,7 @@ def test_unknown_caller_is_refused(tmp_path: Path, capsys: pytest.CaptureFixture workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) rc = run(["--caller", "not-a-skill", "issue-view", "7"], cfg_path) assert rc == cli.EXIT_POLICY @@ -741,7 +745,7 @@ def test_permitted_caller_reaches_dry_run(tmp_path: Path, capsys: pytest.Capture workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) rc = run(["--caller", "security-issue-sync", "issue-view", "7", "--dry-run"], cfg_path) assert rc == cli.EXIT_OK @@ -753,7 +757,7 @@ def test_caller_is_required(tmp_path: Path, capsys: pytest.CaptureFixture[str]) workspace.mkdir() workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" - cfg_path.write_text(CONFIG_TOML.format(workspace=workspace.as_posix())) + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) rc = run(["issue-view", "7"], cfg_path) assert rc == cli.EXIT_USAGE @@ -1210,7 +1214,7 @@ def test_every_code_review_operation_is_a_read() -> None: CONFIG_NO_TRACKER = """ -workspace = "{workspace}" +workspace = '{workspace}' [repos] upstream = "acme/product" @@ -1235,7 +1239,7 @@ def trackerless(tmp_path: Path) -> config.Config: workspace.mkdir(parents=True) workspace.chmod(0o700) cfg_path = root / "config.toml" - cfg_path.write_text(CONFIG_NO_TRACKER.format(workspace=workspace.as_posix())) + cfg_path.write_text(CONFIG_NO_TRACKER.format(workspace=workspace)) return config.load(cfg_path) @@ -1318,7 +1322,7 @@ def test_upstream_is_still_required(tmp_path: Path) -> None: workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" cfg_path.write_text( - CONFIG_NO_TRACKER.format(workspace=workspace.as_posix()).replace('upstream = "acme/product"', "") + CONFIG_NO_TRACKER.format(workspace=workspace).replace('upstream = "acme/product"', "") ) with pytest.raises(config.ConfigError): config.load(cfg_path) @@ -1331,9 +1335,101 @@ def test_a_malformed_tracker_is_still_refused(tmp_path: Path) -> None: workspace.chmod(0o700) cfg_path = tmp_path / "config.toml" cfg_path.write_text( - CONFIG_NO_TRACKER.format(workspace=workspace.as_posix()).replace( - "[repos]", '[repos]\ntracker = "not-a-repo"' - ) + CONFIG_NO_TRACKER.format(workspace=workspace).replace("[repos]", '[repos]\ntracker = "not-a-repo"') ) with pytest.raises(config.ConfigError): config.load(cfg_path) + + +def test_every_http_builder_produces_https_url_from_configured_endpoints(policy: config.Config) -> None: + for name, op in ops.OPS.items(): + if op.backend != "http-read": + continue + params = {} + for p in op.params: + if p in op.body_files: + f = policy.workspace / "file" + f.write_text("x") + params[p] = str(f) + elif p in op.enums: + params[p] = policy.enum_values(op.enums[p])[0] + else: + params[p] = "dummy-value" + req = op.build(policy.as_mapping(), **params) + assert isinstance(req, dict) + url = str(req.get("url", "")) + assert url.startswith("https://") + assert any(url.startswith(base) for base in policy.endpoints.values()), ( + f"{name} built {url} which does not start with a configured endpoint" + ) + + +def test_http_operation_with_writes_is_rejected() -> None: + with pytest.raises(ValueError, match="must be read-only"): + ops._register(ops.Op(name="test-bad", params=(), build=lambda: {}, backend="http-read", writes=True)) + + +def test_run_http_execution_success( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + class MockResponse: + def read(self) -> bytes: + return b"hello world" + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + pass + + def mock_urlopen(req, timeout=None): + assert timeout == 30 + assert req.get_header("User-agent") == "apache-magpie-vetted-ops/0.1.0" + return MockResponse() + + monkeypatch.setattr(urllib.request, "urlopen", mock_urlopen) + rc = cli._run_http({"url": "https://example.com"}, body=None) + assert rc == cli.EXIT_OK + assert capsys.readouterr().out == "hello world" + + +def test_run_http_execution_httperror( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + def mock_urlopen(req, timeout=None): + raise urllib.error.HTTPError(req.full_url, 404, "Not Found", {}, None) + + monkeypatch.setattr(urllib.request, "urlopen", mock_urlopen) + rc = cli._run_http({"url": "https://example.com"}, body=None) + assert rc == cli.EXIT_COMMAND + assert "404 Not Found" in capsys.readouterr().err + + +def test_run_http_execution_urlerror( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + def mock_urlopen(req, timeout=None): + raise urllib.error.URLError("connection refused") + + monkeypatch.setattr(urllib.request, "urlopen", mock_urlopen) + rc = cli._run_http({"url": "https://example.com"}, body=None) + assert rc == cli.EXIT_COMMAND + assert "connection refused" in capsys.readouterr().err + + +def test_run_http_execution_timeouterror( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + def mock_urlopen(req, timeout=None): + raise TimeoutError("timed out") + + monkeypatch.setattr(urllib.request, "urlopen", mock_urlopen) + rc = cli._run_http({"url": "https://example.com"}, body=None) + assert rc == cli.EXIT_COMMAND + assert "timed out" in capsys.readouterr().err + + +def test_run_http_execution_non_https_rejected(capsys: pytest.CaptureFixture[str]) -> None: + rc = cli._run_http({"url": "http://insecure.example.com"}, body=None) + assert rc == cli.EXIT_COMMAND + assert "missing valid https://" in capsys.readouterr().err From 8c3d06c611f185ab3a2a959a9b0ec21ab6614dd5 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 22 Sep 2026 21:51:49 +0530 Subject: [PATCH 5/6] test(vetted-ops): add 64-char commit test cases (#1320) --- tools/vetted-ops/tests/test_vetted_ops.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/vetted-ops/tests/test_vetted_ops.py b/tools/vetted-ops/tests/test_vetted_ops.py index 91673375a..cb886c6b7 100644 --- a/tools/vetted-ops/tests/test_vetted_ops.py +++ b/tools/vetted-ops/tests/test_vetted_ops.py @@ -297,6 +297,7 @@ def test_hostile_package_names_are_refused(hostile: str) -> None: "a1b2c3d", "0123456789abcdef", "a1b2c3d4e5f67890a1b2c3d4e5f67890a1b2c3d4", + "a1b2c3d4e5f678901234567890abcdef1234567890abcdef1234567890abcdef", ], ) def test_valid_commits_are_accepted(valid_commit: str) -> None: @@ -307,6 +308,7 @@ def test_valid_commits_are_accepted(valid_commit: str) -> None: "invalid_commit", [ "a1b2c3", # too short (< 7) + "a" * 65, # too long (> 64) "a1b2c3g", # non-hex "A1B2C3D", # uppercase "main", From 18341fda2311de7bcffdb84427958ec3c3231d21 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 22 Sep 2026 22:00:39 +0530 Subject: [PATCH 6/6] test(vetted-ops): add type annotations to mock helpers for mypy (#1320) --- .../skills/issue-sync/gather.md | 2 +- tools/vetted-ops/tests/test_vetted_ops.py | 34 +++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/plugins/magpie-security/skills/issue-sync/gather.md b/plugins/magpie-security/skills/issue-sync/gather.md index 9d8b0aef9..94ba3e6e8 100644 --- a/plugins/magpie-security/skills/issue-sync/gather.md +++ b/plugins/magpie-security/skills/issue-sync/gather.md @@ -794,7 +794,7 @@ tracker — not metered against the Gmail budget. Still, keep it inside the skill's overall "≤ 1 extra HTTP round-trip per tracker" soft limit for closed-bucket scans: if multiple closed trackers are in scope, run the checks in parallel via the subagent fanout -(one curl per subagent), not serially in the orchestrator. +(one vetted-op-read check per subagent), not serially in the orchestrator. **When the tracker has no CVE ID.** Closed trackers without a `CVE-YYYY-NNNNN` in the *CVE tool link* body field are closing diff --git a/tools/vetted-ops/tests/test_vetted_ops.py b/tools/vetted-ops/tests/test_vetted_ops.py index cb886c6b7..7a90f33d2 100644 --- a/tools/vetted-ops/tests/test_vetted_ops.py +++ b/tools/vetted-ops/tests/test_vetted_ops.py @@ -20,7 +20,10 @@ import json import urllib.error import urllib.request +from email.message import Message from pathlib import Path +from types import TracebackType +from typing import NoReturn import pytest @@ -1378,13 +1381,21 @@ class MockResponse: def read(self) -> bytes: return b"hello world" - def __enter__(self): + def __enter__(self) -> MockResponse: return self - def __exit__(self, exc_type, exc_val, exc_tb): + def __exit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> None: pass - def mock_urlopen(req, timeout=None): + def mock_urlopen( + req: urllib.request.Request, + timeout: float | None = None, + ) -> MockResponse: assert timeout == 30 assert req.get_header("User-agent") == "apache-magpie-vetted-ops/0.1.0" return MockResponse() @@ -1398,8 +1409,11 @@ def mock_urlopen(req, timeout=None): def test_run_http_execution_httperror( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: - def mock_urlopen(req, timeout=None): - raise urllib.error.HTTPError(req.full_url, 404, "Not Found", {}, None) + def mock_urlopen( + req: urllib.request.Request, + timeout: float | None = None, + ) -> NoReturn: + raise urllib.error.HTTPError(req.full_url, 404, "Not Found", Message(), None) monkeypatch.setattr(urllib.request, "urlopen", mock_urlopen) rc = cli._run_http({"url": "https://example.com"}, body=None) @@ -1410,7 +1424,10 @@ def mock_urlopen(req, timeout=None): def test_run_http_execution_urlerror( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: - def mock_urlopen(req, timeout=None): + def mock_urlopen( + req: urllib.request.Request, + timeout: float | None = None, + ) -> NoReturn: raise urllib.error.URLError("connection refused") monkeypatch.setattr(urllib.request, "urlopen", mock_urlopen) @@ -1422,7 +1439,10 @@ def mock_urlopen(req, timeout=None): def test_run_http_execution_timeouterror( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: - def mock_urlopen(req, timeout=None): + def mock_urlopen( + req: urllib.request.Request, + timeout: float | None = None, + ) -> NoReturn: raise TimeoutError("timed out") monkeypatch.setattr(urllib.request, "urlopen", mock_urlopen)