diff --git a/CHANGELOG.md b/CHANGELOG.md index 58aae6d..c193285 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -465,6 +465,17 @@ Notable changes to Cloudfall. The format follows `--gateway-url`, `--gateway-ca`, `--gateway-cert` and `--gateway-key` flags of `cloudfall mcp serve`, which only that tool used, are removed (#35) +- `cloudfall operator approve ` asks the person to type the proposal id + at a terminal before it runs the playbook or the verify step, and no flag + answers that, `--yes` or `"yes": true` in `--raw-payload` included. Off a + terminal it exits 4 with `PERSON_REQUIRED`; a mistyped id exits 4 with + `ATTESTATION_MISMATCH`; either way the proposal stays `proposed`. Before, + any shell call ran it, so an agent could approve its own proposal. + `operator show` previews it. The receipt's approval records + `via: terminal`; a person's approval written before reads as + `via: unknown`, and a policy's has none. The field is optional in + `operator-proposal.schema.json`. Scripted approvals stop working; a + declared `OperatorPolicy` still licenses autonomous runs (#35) ## [0.5.1] - 2026-09-22 diff --git a/config/schemas/v1/operator-proposal.schema.json b/config/schemas/v1/operator-proposal.schema.json index 570a4dd..5fe2ce5 100644 --- a/config/schemas/v1/operator-proposal.schema.json +++ b/config/schemas/v1/operator-proposal.schema.json @@ -192,6 +192,14 @@ }, "policy": { "$ref": "common.schema.json#/$defs/resourceId" + }, + "via": { + "description": "How a person's approval arrived: terminal when a person typed the proposal id at a terminal. A human approval written before this field reads as unknown; an autonomous one has none.", + "type": "string", + "enum": [ + "terminal", + "unknown" + ] } }, "required": [ diff --git a/docs/operator-guide.md b/docs/operator-guide.md index 7bc0cde..0223288 100644 --- a/docs/operator-guide.md +++ b/docs/operator-guide.md @@ -115,6 +115,16 @@ uv run cloudfall operator approve op-... \ --gateway-key certs/operator.key ``` +`operator show` is the preview. `operator approve` asks you to type the +proposal id at your terminal before it runs anything, and no flag answers +that, `--yes` included. Off a terminal, as from an agent's shell, a script +or a pipe, it exits 4 with `PERSON_REQUIRED`; a mistyped id exits 4 with +`ATTESTATION_MISMATCH`. Either way the proposal stays `proposed`. The +receipt records the approval as `approval.via: terminal`; a person's +approval written before reads as `via: unknown`. This is a speed bump +against an agent with a shell, not a boundary: a process running as the +same OS user can fake a terminal. + Approval executes the proposal's engine playbook and then verifies the trigger actually resolved: an alert-triggered proposal must see its alert stop firing, a drift-triggered proposal must see its drifted checks audit diff --git a/sdk/README.md b/sdk/README.md index dedbbe7..1a2bdb9 100644 --- a/sdk/README.md +++ b/sdk/README.md @@ -94,8 +94,9 @@ project and never overwrite an existing resource. `cloudfall init`, `dashboard serve` are never tools: the server starts inside an existing project, approving a recorded operation or an operator proposal is a command a person runs from a shell, and the loops run until stopped. -`decisions approve` asks the person to type the decision id at a terminal; -off a terminal it exits 4 with `PERSON_REQUIRED`, `--yes` or not. That is a +`decisions approve` and `operator approve` ask the person to type the +decision or proposal id at a terminal; off a terminal they exit 4 with +`PERSON_REQUIRED`, `--yes` or not. That is a speed bump against an agent with a shell, not a boundary: a process running as the same OS user can fake a terminal. diff --git a/sdk/src/cloudfall/app.py b/sdk/src/cloudfall/app.py index 11abd98..87772d6 100644 --- a/sdk/src/cloudfall/app.py +++ b/sdk/src/cloudfall/app.py @@ -2619,9 +2619,16 @@ class Approved: @operator.command( "approve", - # A person approves; no agent tool stands in for it. + # A person approves; no agent tool stands in for it. As with decisions + # approve, they type the proposal id at a terminal, which no flag + # answers, --yes included. requires_person keeps it off MCP too; mcp=False + # says so here as well. mcp=False, - description="Execute a proposal and verify its trigger resolves", + requires_person=True, + description=( + "Execute a proposal and verify its trigger resolves; a person types " + "the proposal id at a terminal, and `operator show` previews it" + ), danger_level="mutating", exit_codes=[ "PROJECT_INVALID", @@ -2638,22 +2645,36 @@ class Approved: subprocess=Subprocess("ansible-playbook"), required_tools={"ansible-playbook": "2.21.0"}, examples=[ - ("Approve one proposal", "cloudfall operator approve nginx-down-20260101"), + ( + "Approve and run one proposal, typing its id at a terminal", + "cloudfall operator approve nginx-down-20260101", + ), ], ) def operator_approve( args: OperatorApproveArgs, ctx: Ctx, fleet: Fleet ) -> Approved: - """Run the proposal's operation, then wait for its trigger to resolve.""" + """Run the proposal's operation once a person typed its id, then verify it.""" store = args.store(fleet) verifier_for = args.verifier_for(fleet, ctx, partial(args.feed, fleet)) try: + recorded = store.load(args.proposal) + verifier = verifier_for(recorded) + # Off a terminal this ends with PERSON_REQUIRED, and a mistyped id with + # ATTESTATION_MISMATCH, before anything runs: --yes never approves. + attestation = ctx.attest( + f"{recorded.diagnosis_summary}. It runs " + f"{' '.join(recorded.command)}. Type the proposal id to approve " + f"{recorded.resource_id.value} and run it", + expected=recorded.resource_id.value, + ) proposal = approve_proposal( store, args.proposal, engine_executor(args.context(fleet, ctx)), - verifier_for(store.load(args.proposal)), + verifier, ApproveOptions(verify_timeout_seconds=args.verify_timeout), + via=ApprovalChannel(attestation.channel), ) except OperatorError as error: raise _operator_failed(error) from error diff --git a/sdk/src/cloudfall/commands.py b/sdk/src/cloudfall/commands.py index 6a7142a..83eb43a 100644 --- a/sdk/src/cloudfall/commands.py +++ b/sdk/src/cloudfall/commands.py @@ -554,7 +554,11 @@ def _plan(subject: str, *extra: str) -> OutputShape: "operator approve", CommandEffect.SERVERS, "execute one proposal and verify that its trigger resolves", - gate="the proposal id names a receipt a human has reviewed", + gate=( + "a person types the proposal id at a terminal, which no flag answers " + "(`--yes` included); off a terminal it exits 4 with `PERSON_REQUIRED`, " + "and `operator show` previews the proposal" + ), output=_PROPOSAL, ), ) diff --git a/sdk/src/cloudfall/operator.py b/sdk/src/cloudfall/operator.py index 5d86cb6..a5c60da 100644 --- a/sdk/src/cloudfall/operator.py +++ b/sdk/src/cloudfall/operator.py @@ -21,6 +21,7 @@ from typing import TYPE_CHECKING, Protocol, cast from cloudfall.audit import AuditStatus, audit_inventory +from cloudfall.decision import ApprovalChannel from cloudfall.domain import AlertSeverity, ResourceId from cloudfall.lifecycle import LifecycleError, run_engine_playbook from cloudfall.observation import load_observations @@ -43,6 +44,7 @@ ERROR_PROPOSAL_MISSING = "operator_proposal_missing" """Code of the error ``ProposalStore`` raises for an id it holds no receipt for.""" _ERROR_PROPOSAL_NOT_OPEN = "operator_proposal_not_open" +_ERROR_CHANNEL_UNKNOWN = "operator_approval_channel_unknown" _ERROR_OPERATION_UNSUPPORTED = "operator_operation_unsupported" ERROR_GATEWAY_UNDECLARED = "operator_gateway_undeclared" _SERVICE_CHECK_PREFIXES = ("services.bind[", "alerting.rules[") @@ -144,16 +146,33 @@ def as_dict(self) -> dict[str, object]: @dataclass(frozen=True, slots=True) class ApprovalRecord: - """Who licensed an execution: a human confirm or a declared policy.""" + """Who licensed an execution: a person or a declared policy. + + A person's approval says how it arrived (``via``); a policy's has no + channel, since no one typed anything. + """ mode: str policy: ResourceId | None + via: ApprovalChannel | None = None + + def __post_init__(self) -> None: + """Reject a person's approval without a channel, or a policy's with one.""" + if (self.via is None) is (self.mode == "human"): + message = ( + f"a {self.mode} approval " + f"{'names' if self.mode == 'human' else 'has no'} the channel it " + "arrived through" + ) + raise ValueError(message) def as_dict(self) -> dict[str, object]: """Serialize the approval for the proposal receipt.""" result: dict[str, object] = {"mode": self.mode} if self.policy is not None: result["policy"] = self.policy.value + if self.via is not None: + result["via"] = self.via.value return result @@ -567,13 +586,20 @@ def _proposal_from_document(document: Mapping[str, object]) -> OperatorProposal: approval = None if isinstance(raw_approval, dict): raw_policy = raw_approval.get("policy") + mode = cast("str", raw_approval["mode"]) approval = ApprovalRecord( - mode=cast("str", raw_approval["mode"]), + mode=mode, policy=( ResourceId.from_boundary(raw_policy) if raw_policy is not None else None ), + # A person's approval written before the channel reads as unknown. + via=( + ApprovalChannel.from_record(raw_approval.get("via")) + if mode == "human" + else None + ), ) raw_outcome = spec.get("outcome") outcome = None @@ -803,14 +829,26 @@ class ApproveOptions: sleep: Callable[[float], None] = field(default=time.sleep) -def approve( +def approve( # noqa: PLR0913 - the run's contract plus how the approval came. store: ProposalStore, proposal_id: ResourceId, executor: Callable[[OperatorProposal], None], verifier: Callable[[OperatorProposal], bool], options: ApproveOptions | None = None, + *, + via: ApprovalChannel, ) -> OperatorProposal: - """Execute a human-approved proposal and verify its trigger resolves.""" + """Execute a human-approved proposal and verify its trigger resolves. + + ``via`` is how the person's approval arrived; a new approval cannot be + of an unknown channel. + """ + if via is ApprovalChannel.UNKNOWN: + message = ( + "an approval records how it arrived; a new one cannot be of an " + "unknown channel" + ) + raise OperatorError(_ERROR_CHANNEL_UNKNOWN, message) proposal = store.load(proposal_id) if proposal.status is not ProposalStatus.PROPOSED: message = ( @@ -823,7 +861,7 @@ def approve( proposal, executor, verifier, - ApprovalRecord(mode="human", policy=None), + ApprovalRecord(mode="human", policy=None, via=via), options, ) diff --git a/sdk/tests/test_operator.py b/sdk/tests/test_operator.py index f7b560e..a4aa78c 100644 --- a/sdk/tests/test_operator.py +++ b/sdk/tests/test_operator.py @@ -2,14 +2,17 @@ from __future__ import annotations +import io import itertools import json from dataclasses import dataclass, field from pathlib import Path import pytest +from cloudfall.app import app from cloudfall.audit import AuditCheck, AuditReport, AuditStatus, ServerAudit from cloudfall.cli import main +from cloudfall.decision import ApprovalChannel from cloudfall.domain import ResourceId from cloudfall.inventory import PlatformInventory from cloudfall.lifecycle import LifecycleError @@ -36,6 +39,8 @@ watch, ) from cloudfall.validation import SchemaCatalog, validate_config +from test_decision import _Terminal +from treaty import CommandPath _FAST_APPROVE = ApproveOptions( verify_timeout_seconds=2.0, @@ -43,6 +48,7 @@ sleep=lambda _: None, ) +TERMINAL = ApprovalChannel.TERMINAL ROOT = Path(__file__).parents[2] SCHEMAS = ROOT / "config" / "schemas" / "v1" EXAMPLES = ROOT / "config" / "examples" @@ -208,6 +214,7 @@ def test_approve_executes_and_verifies_resolution(tmp_path: Path) -> None: lambda proposal: executed.append(proposal.resource_id.value), alert_resolution_verifier(feed), _FAST_APPROVE, + via=TERMINAL, ) assert executed == [report.proposed[0]] @@ -229,6 +236,7 @@ def test_approve_marks_unresolved_alerts_failed(tmp_path: Path) -> None: lambda _proposal: None, alert_resolution_verifier(feed), _FAST_APPROVE, + via=TERMINAL, ) assert result.status is ProposalStatus.FAILED @@ -254,6 +262,7 @@ def _boom(_proposal: object) -> None: _boom, alert_resolution_verifier(feed), _FAST_APPROVE, + via=TERMINAL, ) failed = store.load(proposal_id) @@ -281,6 +290,7 @@ def _boom(_proposal: object) -> None: _boom, alert_resolution_verifier(feed), _FAST_APPROVE, + via=TERMINAL, ) failed = store.load(proposal_id) @@ -304,6 +314,7 @@ def test_approve_refuses_non_open_proposals(tmp_path: Path) -> None: lambda _proposal: None, alert_resolution_verifier(feed), _FAST_APPROVE, + via=TERMINAL, ) with pytest.raises(OperatorError) as caught: @@ -313,6 +324,7 @@ def test_approve_refuses_non_open_proposals(tmp_path: Path) -> None: lambda _proposal: None, alert_resolution_verifier(feed), _FAST_APPROVE, + via=TERMINAL, ) assert caught.value.code == "operator_proposal_not_open" @@ -409,6 +421,7 @@ def test_approve_drift_proposal_verifies_through_the_audit( ) ), _FAST_APPROVE, + via=TERMINAL, ) assert executed == ["converge-baseline"] @@ -430,6 +443,7 @@ def test_approve_drift_proposal_fails_when_drift_persists( lambda _proposal: None, drift_resolution_verifier(auditor), _FAST_APPROVE, + via=TERMINAL, ) assert result.status is ProposalStatus.FAILED @@ -638,6 +652,7 @@ def _seed_verified_drift( lambda _proposal: None, lambda _proposal: True, _FAST_APPROVE, + via=TERMINAL, ) @@ -751,6 +766,7 @@ def test_autonomy_suspends_after_a_failed_receipt(tmp_path: Path) -> None: lambda _proposal: None, lambda _proposal: False, _FAST_APPROVE, + via=TERMINAL, ) drift_pass( lambda: _audit_report(("packages.required[curl]", AuditStatus.DRIFT)), @@ -824,3 +840,242 @@ def test_autonomy_enforces_the_rate_limit(tmp_path: Path) -> None: assert len(first.executed) == 1 assert second.executed == () assert "rate limit reached" in second.withheld[0][1] + + +def _drift_proposal(tmp_path: Path) -> ResourceId: + report = drift_pass( + lambda: _audit_report(("packages.required[curl]", AuditStatus.DRIFT)), + _store(tmp_path), + ) + return ResourceId.from_boundary(report.proposed[0]) + + +def _operator_approve(tmp_path: Path, proposal: str, *flags: str) -> list[str]: + return [ + *("operator", "approve", proposal), + *("--project", str(EXAMPLES), "--schemas", str(SCHEMAS)), + *("--proposals", str(tmp_path / "proposals")), + *flags, + ] + + +def _approve_at( + tmp_path: Path, proposal: str, typed: str, *, stdin_tty: bool, stdout_tty: bool +) -> tuple[int, dict[str, object]]: + """Run ``operator approve`` with stdin and stdout each a terminal or not.""" + stdout = _Terminal() if stdout_tty else io.StringIO() + stdin = _Terminal(f"{typed}\n") if stdin_tty else io.StringIO(f"{typed}\n") + exit_code = app.run( + _operator_approve(tmp_path, proposal, "--format", "json"), + stdin=stdin, + stdout=stdout, + stderr=io.StringIO(), + ) + return exit_code, json.loads(stdout.getvalue()) + + +def _still_proposed(tmp_path: Path, proposal_id: ResourceId) -> None: + loaded = _store(tmp_path).load(proposal_id) + assert loaded.status is ProposalStatus.PROPOSED + assert loaded.approval is None + assert loaded.outcome is None + + +@pytest.mark.parametrize("flags", [["--yes"], []], ids=["yes", "bare"]) +def test_cli_operator_approve_off_a_terminal_is_refused( + tmp_path: Path, capsys: pytest.CaptureFixture[str], flags: list[str] +) -> None: + """An agent's shell call cannot approve its own proposal, ``--yes`` or not (#35).""" + proposal_id = _drift_proposal(tmp_path) + + exit_code = main(_operator_approve(tmp_path, proposal_id.value, *flags)) + + payload = json.loads(capsys.readouterr().out) + assert exit_code == 4 + assert payload["error"]["code"] == "PERSON_REQUIRED" + assert payload["error"]["retryable"] is False + _still_proposed(tmp_path, proposal_id) + + +def test_cli_operator_approve_raw_payload_cannot_approve_off_a_terminal( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + proposal_id = _drift_proposal(tmp_path) + payload_in = { + "proposal": proposal_id.value, + "project": str(EXAMPLES), + "schemas": str(SCHEMAS), + "proposals": str(tmp_path / "proposals"), + "yes": True, + } + + exit_code = main(["operator", "approve", "--raw-payload", json.dumps(payload_in)]) + + payload = json.loads(capsys.readouterr().out) + assert exit_code == 4 + assert payload["error"]["code"] == "PERSON_REQUIRED" + _still_proposed(tmp_path, proposal_id) + + +@pytest.mark.parametrize( + ("stdin_tty", "stdout_tty"), + [(True, False), (False, True)], + ids=["stdin-only", "stdout-only"], +) +def test_cli_operator_approve_needs_a_whole_terminal( + tmp_path: Path, *, stdin_tty: bool, stdout_tty: bool +) -> None: + proposal_id = _drift_proposal(tmp_path) + + exit_code, payload = _approve_at( + tmp_path, + proposal_id.value, + proposal_id.value, + stdin_tty=stdin_tty, + stdout_tty=stdout_tty, + ) + + error = payload["error"] + assert isinstance(error, dict) + assert exit_code == 4 + assert error["code"] == "PERSON_REQUIRED" + _still_proposed(tmp_path, proposal_id) + + +def test_cli_operator_approve_with_a_mistyped_id_runs_nothing(tmp_path: Path) -> None: + proposal_id = _drift_proposal(tmp_path) + + exit_code, payload = _approve_at( + tmp_path, proposal_id.value, "yes", stdin_tty=True, stdout_tty=True + ) + + error = payload["error"] + assert isinstance(error, dict) + assert exit_code == 4 + assert error["code"] == "ATTESTATION_MISMATCH" + _still_proposed(tmp_path, proposal_id) + + +def test_cli_operator_approve_runs_once_a_person_typed_the_id(tmp_path: Path) -> None: + """The typed id lets the playbook run, and the receipt says it came by terminal. + + The example hosts do not resolve, so the run fails fast at the engine step; + what matters is that it ran and that the approval was recorded. + """ + proposal_id = _drift_proposal(tmp_path) + + exit_code, payload = _approve_at( + tmp_path, proposal_id.value, proposal_id.value, stdin_tty=True, stdout_tty=True + ) + + error = payload["error"] + assert isinstance(error, dict) + assert exit_code != 4 + assert error["code"] == "ENGINE_STEP_FAILED" + loaded = _store(tmp_path).load(proposal_id) + assert loaded.status is ProposalStatus.FAILED + assert loaded.approval is not None + assert (loaded.approval.mode, loaded.approval.via) == ("human", TERMINAL) + + +def test_cli_operator_approve_keeps_its_gateway_flags( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """An alert proposal still names the gateway flags it needs, and takes them.""" + run_once(FakeFeed(payloads=[_alerts_payload()]), _inventory(), _store(tmp_path)) + (proposal,) = _store(tmp_path).list() + material = tmp_path / "material.pem" + material.write_text("not a certificate\n", encoding="utf-8") + + missing = main(_operator_approve(tmp_path, proposal.resource_id.value)) + refused = json.loads(capsys.readouterr().out) + given = main( + _operator_approve( + tmp_path, + proposal.resource_id.value, + *("--gateway-url", "https://127.0.0.1:9/api/v1/alerts"), + *("--gateway-ca", str(material), "--gateway-cert", str(material)), + *("--gateway-key", str(material)), + ) + ) + asked = json.loads(capsys.readouterr().out) + + assert missing == 4 + assert refused["error"]["context"]["code"] == "operator_gateway_material_missing" + assert given == 4 + assert asked["error"]["code"] == "PERSON_REQUIRED" + _still_proposed(tmp_path, proposal.resource_id) + + +def test_the_operator_approval_command_is_a_persons() -> None: + command = app.commands[CommandPath("operator.approve")] + + assert command.requires_person is True + assert command.mcp is False + + +def test_a_human_approval_records_that_it_came_through_a_terminal( + tmp_path: Path, +) -> None: + store = _store(tmp_path) + proposal_id = _drift_proposal(tmp_path) + + approve( + store, + proposal_id, + lambda _proposal: None, + lambda _proposal: True, + _FAST_APPROVE, + via=TERMINAL, + ) + + loaded = store.load(proposal_id) + assert loaded.approval is not None + assert (loaded.approval.mode, loaded.approval.via) == ("human", TERMINAL) + on_disk = json.loads( + (tmp_path / "proposals" / f"{proposal_id.value}.json").read_text( + encoding="utf-8" + ) + ) + assert on_disk["spec"]["approval"] == {"mode": "human", "via": "terminal"} + + +def test_a_human_approval_written_before_the_channel_reads_as_unknown( + tmp_path: Path, +) -> None: + store = _store(tmp_path) + proposal_id = _drift_proposal(tmp_path) + approve( + store, + proposal_id, + lambda _proposal: None, + lambda _proposal: True, + _FAST_APPROVE, + via=TERMINAL, + ) + path = tmp_path / "proposals" / f"{proposal_id.value}.json" + document = json.loads(path.read_text(encoding="utf-8")) + del document["spec"]["approval"]["via"] + path.write_text(json.dumps(document), encoding="utf-8") + + loaded = store.load(proposal_id) + + assert loaded.approval is not None + assert loaded.approval.via is ApprovalChannel.UNKNOWN + + +def test_a_new_operator_approval_names_how_it_arrived(tmp_path: Path) -> None: + proposal_id = _drift_proposal(tmp_path) + + with pytest.raises(OperatorError) as caught: + approve( + _store(tmp_path), + proposal_id, + lambda _proposal: None, + lambda _proposal: True, + _FAST_APPROVE, + via=ApprovalChannel.UNKNOWN, + ) + + assert caught.value.code == "operator_approval_channel_unknown" + _still_proposed(tmp_path, proposal_id)