diff --git a/CHANGELOG.md b/CHANGELOG.md index e9957ca..b8d61ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ Notable changes to Cloudfall. The format follows ### Added +- A decision record's approval says how it arrived, as `approval.via`: + `terminal` when a person typed the decision id at a terminal. A record + written before reads as `via: unknown`. `cloudfall why` tells it in the + approval sentence. The field is optional in + `operation-decision.schema.json` (#25) - `agent investigate --replay DIR` plays a real run back instead of running Ansible: DIR is the decisions directory of a run on real hosts, and each check-mode call the model makes answers with the output and per-host @@ -130,6 +135,12 @@ Notable changes to Cloudfall. The format follows ### Changed +- treaty 1.0.0rc38 instead of 1.0.0rc29. `dashboard serve` and + `operator run` write each event in `json` and `jsonl` as a bare JSON line + with `_seq` instead of an envelope with `data` and `meta.seq`, and end + with one `"_summary": true` line or the error envelope. `mcp serve` + stopped by `SIGINT` or `SIGTERM` exits 130 or 143 with the `CANCELLED` + envelope instead of 0, and writes no envelope when it stops cleanly (#25) - The package is classified as Beta on PyPI instead of Alpha - The site's landing page is redesigned for technical founders leaving the cloud and for teams running servers for clients: a light, illustrated @@ -420,6 +431,19 @@ Notable changes to Cloudfall. The format follows `failed` receipt, left the proposal `proposed` with no trace that it ran, and showed the caller a traceback +### Security + +- `cloudfall decisions approve ` asks the person to type the decision id + at a terminal before it runs anything, and no flag answers that, `--yes` + included. Off a terminal, as from an agent's shell or a script, it exits 4 + with `PERSON_REQUIRED` and runs nothing; a mistyped id exits 4 with + `ATTESTATION_MISMATCH`. Before, `--yes` ran it headless and `--approver` + fell back to `$USER`, so an agent could approve its own proposal in the + person's name. Without `--yes` the command no longer previews the + proposal: `decisions show` does. Scripted or CI approvals stop working. + This is a speed bump, not a boundary: a process running as the same OS + user can fake a terminal (#25) + ## [0.5.1] - 2026-09-22 ### Changed diff --git a/config/schemas/v1/operation-decision.schema.json b/config/schemas/v1/operation-decision.schema.json index 04102ea..a8de463 100644 --- a/config/schemas/v1/operation-decision.schema.json +++ b/config/schemas/v1/operation-decision.schema.json @@ -206,6 +206,14 @@ "approvedAt": { "type": "string", "format": "date-time" + }, + "via": { + "description": "How the approval arrived: terminal when a person typed the decision id at a terminal. A record written before this field reads as unknown.", + "type": "string", + "enum": [ + "terminal", + "unknown" + ] } }, "required": [ diff --git a/examples/disk-full-incident/README.md b/examples/disk-full-incident/README.md index f278a6d..f1fcf0f 100644 --- a/examples/disk-full-incident/README.md +++ b/examples/disk-full-incident/README.md @@ -61,12 +61,14 @@ cloudfall agent investigate \ ``` The answer names the root cause and the decisions it recorded, in the order -to approve them. Nothing on the host has changed yet. Review and approve: +to approve them. Nothing on the host has changed yet. Review, then approve +at your terminal, where each approval asks you to type the decision id (an +agent's shell call is refused with `PERSON_REQUIRED`): ```console -cloudfall decisions approve shop-logrotate- # shows the proposal -cloudfall decisions approve shop-logrotate- --yes # runs it, then verifies -cloudfall decisions approve postgresql-restart- --yes +cloudfall decisions show shop-logrotate- # shows the proposal +cloudfall decisions approve shop-logrotate- # type the id; runs it, then verifies +cloudfall decisions approve postgresql-restart- cloudfall why --host hz1 --format html > why.html ``` diff --git a/pyproject.toml b/pyproject.toml index 8e70b0e..7c156de 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,7 @@ dependencies = [ "ansible-core>=2.21,<2.22", "jsonschema[format]>=4.23,<5", "pyyaml>=6,<7", - "treaty==1.0.0rc29", + "treaty==1.0.0rc38", ] [project.urls] diff --git a/sdk/README.md b/sdk/README.md index a212bd2..644c573 100644 --- a/sdk/README.md +++ b/sdk/README.md @@ -93,6 +93,10 @@ project and never overwrite an existing resource. `cloudfall init`, `decisions approve`, `operator run` and `dashboard serve` are never tools: the server starts inside an existing project, approving a recorded operation is a person's command, 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 +speed bump against an agent with a shell, not a boundary: a process running +as the same OS user can fake a terminal. ```console uv run cloudfall mcp serve --project config/examples diff --git a/sdk/src/cloudfall/app.py b/sdk/src/cloudfall/app.py index ddbf60f..5ac8378 100644 --- a/sdk/src/cloudfall/app.py +++ b/sdk/src/cloudfall/app.py @@ -100,6 +100,7 @@ CHECK_TIMEOUT_SECONDS, DECISION_DIRECTORY, ERROR_DECISION_MISSING, + ApprovalChannel, ApprovalRequest, CheckRunner, DecisionError, @@ -1957,14 +1958,6 @@ class ApproveDecisionArgs(DecisionsArgs): default=None, description="Who is approving (default: the USER environment variable)", ) - yes: bool = Flag( - default=False, - confirm=True, - description=( - "Change the servers; without it the command shows the recorded " - "proposal and runs nothing" - ), - ) @dataclass(frozen=True, slots=True) @@ -2058,11 +2051,14 @@ def _propose(args: ProposeArgs, run: CheckRunner) -> Decided: @decisions.command( "approve", - # A person approves; no agent tool stands in for it. + # A person approves; no agent tool stands in for it. The person types the + # decision 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, + requires_person=True, description=( - "Approve one recorded proposal, run it, and verify it; without --yes it " - "shows the proposal and runs nothing" + "Approve one recorded proposal, run it, and verify it; a person types " + "the decision id at a terminal, and `decisions show` previews it" ), danger_level="mutating", exit_codes=["NOT_FOUND", "PRECONDITION", "RECORD_INVALID", "NOT_VERIFIED"], @@ -2071,12 +2067,14 @@ def _propose(args: ProposeArgs, run: CheckRunner) -> Decided: subprocess=Subprocess("ansible-playbook"), required_tools={"ansible-playbook": "2.21.0"}, examples=[ - ("Review a proposal", "cloudfall decisions approve restart-nginx-20260101"), - ("Run it", "cloudfall decisions approve restart-nginx-20260101 --yes"), + ( + "Approve and run a proposal, typing its id at a terminal", + "cloudfall decisions approve restart-nginx-20260101", + ), ], ) def decisions_approve(args: ApproveDecisionArgs, ctx: Ctx) -> Decided: - """Run what was proposed, as recorded, and record how it ended.""" + """Run what was proposed, as recorded, once a person typed its id.""" store = DecisionStore( directory=args.root / args.decisions, catalog=SchemaCatalog(args.schemas) ) @@ -2086,21 +2084,22 @@ def decisions_approve(args: ApproveDecisionArgs, ctx: Ctx) -> Decided: if error.code == ERROR_DECISION_MISSING: raise Exit.NOT_FOUND(error.detail, context={"code": error.code}) from error raise _record_invalid(error) from error - if not args.yes: - # Without --yes the run is treaty's dry run (Flag(confirm=True)). - return Decided( - effect="would_update", - status="pending", - decision=decision.as_document(), - next=[ - f"review the recorded diff at {decision.check.diff.path}", - "approve with --yes to run it", - ], - ) + # 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"The diff is at {decision.check.diff.path}. Type the decision id to " + f"approve {decision.decision_id.value} and run it", + expected=decision.decision_id.value, + ) approver = args.approver if args.approver is not None else ctx.env.get("USER", "") try: approved = approve( - ApprovalRequest(decision=decision, approver=approver, repository=args.root), + ApprovalRequest( + decision=decision, + approver=approver, + repository=args.root, + via=ApprovalChannel(attestation.channel), + ), store, run=_check_runner(ctx, args.root), ) @@ -3817,7 +3816,10 @@ def propose_operation(arguments: Mapping[str, object], ctx: Ctx) -> Decided: if operation.risk is RiskLevel.READ: return decided record = cast("Mapping[str, Mapping[str, object]]", decided.decision) - approve = f"{APPROVAL_COMMAND} {record['metadata']['id']} --yes" + approve = ( + "a person approves it at a terminal, typing the decision id: " + f"{APPROVAL_COMMAND} {record['metadata']['id']}" + ) return dataclasses.replace(decided, next=[approve]) return McpTool( diff --git a/sdk/src/cloudfall/commands.py b/sdk/src/cloudfall/commands.py index dcdfe94..6a7142a 100644 --- a/sdk/src/cloudfall/commands.py +++ b/sdk/src/cloudfall/commands.py @@ -267,11 +267,12 @@ def _plan(subject: str, *extra: str) -> OutputShape: "decisions approve", CommandEffect.SERVERS, "approve one recorded proposal, run it for real, and verify it", - gate=_YES, - output=( - _shape("status", "decision", "next", when="without `--yes`"), - _shape("status", "decision", when="with `--yes`"), + gate=( + "a person types the decision id at a terminal, which no flag answers " + "(`--yes` included); off a terminal it exits 4 with `PERSON_REQUIRED`, " + "and `decisions show` previews the proposal" ), + output=(_shape("status", "decision"),), ), CommandContract( "cloudfall", diff --git a/sdk/src/cloudfall/decision.py b/sdk/src/cloudfall/decision.py index c24410a..d292846 100644 --- a/sdk/src/cloudfall/decision.py +++ b/sdk/src/cloudfall/decision.py @@ -68,6 +68,7 @@ ERROR_DECISION_MISSING = "decision_missing" _ERROR_NOT_PROPOSED = "decision_not_proposed" _ERROR_APPROVER_UNKNOWN = "decision_approver_unknown" +_ERROR_CHANNEL_UNKNOWN = "decision_approval_channel_unknown" class DecisionError(RuntimeError): @@ -223,16 +224,43 @@ def as_dict(self) -> dict[str, object]: } +class ApprovalChannel(StrEnum): + """How an approval reached the record. + + A person typing the decision id back at a terminal is the only channel + an approval is given through. A record written before the channel was + recorded says nothing about it, and reads as unknown. + """ + + TERMINAL = "terminal" + """A person typed the decision id at a terminal.""" + + UNKNOWN = "unknown" + """The record predates the channel; it cannot say how the approval came.""" + + @classmethod + def from_record(cls, value: object) -> ApprovalChannel: + """Read the channel a record names; a record that names none is unknown.""" + if value is None: + return cls.UNKNOWN + return cls(str(value)) + + @dataclass(frozen=True, slots=True) class Approval: - """Who let one operation through, and when.""" + """Who let one operation through, when, and how it arrived.""" approver: str approved_at: str + via: ApprovalChannel def as_dict(self) -> dict[str, object]: """Serialize the approval for the record.""" - return {"approver": self.approver, "approvedAt": self.approved_at} + return { + "approver": self.approver, + "approvedAt": self.approved_at, + "via": self.via.value, + } @dataclass(frozen=True, slots=True) @@ -435,6 +463,7 @@ class ApprovalRequest: decision: Decision approver: str repository: Path + via: ApprovalChannel @dataclass(frozen=True, slots=True) @@ -469,6 +498,12 @@ def approve( if not request.approver.strip(): message = "an approval records who gave it; pass --approver or set USER" raise DecisionError(_ERROR_APPROVER_UNKNOWN, message) + if request.via is ApprovalChannel.UNKNOWN: + message = ( + "an approval records how it arrived; a new one cannot be of an " + "unknown channel" + ) + raise DecisionError(_ERROR_CHANNEL_UNKNOWN, message) clock = now or _utc_now stage = _Stage(request=request, store=store, runner=run or _run_check) moment = clock() @@ -482,7 +517,9 @@ def approve( approved = replace( decision, approval=Approval( - approver=request.approver.strip(), approved_at=_timestamp(moment) + approver=request.approver.strip(), + approved_at=_timestamp(moment), + via=request.via, ), execution=execution, verification=verification, @@ -837,14 +874,16 @@ def _decision_from_document(document: Mapping[str, object]) -> Decision: verdict=str(spec["verdict"]) if "verdict" in spec else None, execution=_run_from_document(spec.get("execution")), verification=_run_from_document(spec.get("verify")), - approval=( - Approval( - approver=str(cast("Mapping[str, object]", approval)["approver"]), - approved_at=str( - cast("Mapping[str, object]", approval)["approvedAt"] - ), - ) - if isinstance(approval, dict) - else None - ), + approval=_approval_from_document(approval), + ) + + +def _approval_from_document(approval: object) -> Approval | None: + if not isinstance(approval, dict): + return None + record = cast("Mapping[str, object]", approval) + return Approval( + approver=str(record["approver"]), + approved_at=str(record["approvedAt"]), + via=ApprovalChannel.from_record(record.get("via")), ) diff --git a/sdk/src/cloudfall/why.py b/sdk/src/cloudfall/why.py index f1da1a9..d3d7ce0 100644 --- a/sdk/src/cloudfall/why.py +++ b/sdk/src/cloudfall/why.py @@ -20,13 +20,13 @@ from html import escape from typing import TYPE_CHECKING, cast -from cloudfall.decision import DecisionStatus, Requirement +from cloudfall.decision import ApprovalChannel, DecisionStatus, Requirement from cloudfall.domain import Hostname, ResourceId if TYPE_CHECKING: from collections.abc import Iterable, Mapping, Sequence - from cloudfall.decision import Decision, DecisionStore, RunRecord + from cloudfall.decision import Approval, Decision, DecisionStore, RunRecord _ERROR_INSTANT_INVALID = "why_instant_invalid" _ERROR_WINDOW_INVERTED = "why_window_inverted" @@ -348,10 +348,7 @@ def _story(decision: Decision) -> Iterable[str]: yield _checked(decision) yield _gated(decision) if decision.approval is not None: - yield ( - f"{decision.approval.approver} approved it at " - f"{decision.approval.approved_at}." - ) + yield _approved(decision.approval) if decision.execution is not None: yield _ran("The run", decision.execution, decision.execution.log.path) if decision.verification is not None: @@ -361,6 +358,16 @@ def _story(decision: Decision) -> Iterable[str]: yield _ended(decision) +def _approved(approval: Approval) -> str: + said = ( + f"{approval.approver} approved it at {approval.approved_at} " + f"(via {approval.via.value})" + ) + if approval.via is ApprovalChannel.UNKNOWN: + return f"{said}: the record predates noting how an approval arrived." + return f"{said}: a person typed its id at a terminal." + + def _saw(decision: Decision) -> str: observed = decision.basis.observed_at if not observed: diff --git a/sdk/tests/test_agent_server.py b/sdk/tests/test_agent_server.py index 4a9f04f..4a76c9d 100644 --- a/sdk/tests/test_agent_server.py +++ b/sdk/tests/test_agent_server.py @@ -207,7 +207,8 @@ def test_an_operation_tool_records_a_proposal_and_changes_nothing( assert decision["spec"]["status"] == "proposed" assert decision["spec"]["inputs"] == {"version": "2.0.0"} assert proposed["data"]["next"] == [ - f"cloudfall decisions approve {decision['metadata']['id']} --yes" + "a person approves it at a terminal, typing the decision id: " + f"cloudfall decisions approve {decision['metadata']['id']}" ] assert not (repository / "marker.txt").exists() recorded = decisions["data"]["decisions"] diff --git a/sdk/tests/test_app.py b/sdk/tests/test_app.py index 00225c6..b176734 100644 --- a/sdk/tests/test_app.py +++ b/sdk/tests/test_app.py @@ -118,7 +118,7 @@ def test_dashboard_serve_streams_where_it_listens_until_stopped( readable, _, _ = select.select([process.stdout], [], [], 30) assert readable, "the listening event never arrived" listening = json.loads(process.stdout.readline()) - url = listening["data"]["dashboard"]["url"] + url = listening["dashboard"]["url"] with urllib.request.urlopen(f"{url}operations.json", timeout=10) as response: # noqa: S310 - local server assert response.status == 200 process.send_signal(signal.SIGTERM) @@ -128,8 +128,8 @@ def test_dashboard_serve_streams_where_it_listens_until_stopped( process.kill() process.wait() - assert listening["meta"]["seq"] == 1 - assert listening["data"]["status"] == "ok" + assert listening["_seq"] == 1 + assert listening["status"] == "ok" assert closing["error"]["code"] == "CANCELLED" diff --git a/sdk/tests/test_commands.py b/sdk/tests/test_commands.py index c91f7d5..95bdacb 100644 --- a/sdk/tests/test_commands.py +++ b/sdk/tests/test_commands.py @@ -65,7 +65,6 @@ def test_every_yes_gated_command_is_classified_as_changing_servers() -> None: "restart", "data copy", "migrate", - "decisions approve", } assert gated <= changing for command in commands_with_effect(CommandEffect.SERVERS): diff --git a/sdk/tests/test_decision.py b/sdk/tests/test_decision.py index bc508d3..9a227f6 100644 --- a/sdk/tests/test_decision.py +++ b/sdk/tests/test_decision.py @@ -2,16 +2,19 @@ from __future__ import annotations +import io import json from datetime import UTC, datetime from pathlib import Path from typing import TYPE_CHECKING import pytest +from cloudfall.app import app from cloudfall.catalog import RiskLevel, TargetScope, load_catalog from cloudfall.cli import main from cloudfall.decision import ( DECISION_DIRECTORY, + ApprovalChannel, ApprovalRequest, DecisionError, DecisionStatus, @@ -30,12 +33,14 @@ from cloudfall.resources import default_schema_directory from cloudfall.validation import SchemaCatalog from test_catalog import DEPLOY, FACTS, _repository +from treaty import CommandPath if TYPE_CHECKING: from collections.abc import Mapping, Sequence SCHEMAS = default_schema_directory() MOMENT = datetime(2026, 9, 21, 14, 30, 12, tzinfo=UTC) +TERMINAL = ApprovalChannel.TERMINAL WIPE = """\ id: wipe @@ -499,7 +504,7 @@ def test_an_approval_runs_the_recorded_proposal_and_verifies_it( approved = approve( ApprovalRequest( - decision=proposed, approver="roman", repository=repository + decision=proposed, approver="roman", repository=repository, via=TERMINAL ), store, lambda: MOMENT, @@ -530,7 +535,7 @@ def test_an_approved_run_is_not_check_mode(tmp_path: Path) -> None: approve( ApprovalRequest( - decision=proposed, approver="roman", repository=repository + decision=proposed, approver="roman", repository=repository, via=TERMINAL ), store, lambda: MOMENT, @@ -550,7 +555,7 @@ def test_a_failed_run_is_recorded_as_failed_and_skips_verify( approved = approve( ApprovalRequest( - decision=proposed, approver="roman", repository=repository + decision=proposed, approver="roman", repository=repository, via=TERMINAL ), store, lambda: MOMENT, @@ -581,7 +586,7 @@ def __call__( approved = approve( ApprovalRequest( - decision=proposed, approver="roman", repository=repository + decision=proposed, approver="roman", repository=repository, via=TERMINAL ), store, lambda: MOMENT, @@ -609,7 +614,7 @@ def test_a_read_operation_without_verify_is_executed_not_verified( approved = approve( ApprovalRequest( - decision=proposed, approver="roman", repository=repository + decision=proposed, approver="roman", repository=repository, via=TERMINAL ), store, lambda: MOMENT, @@ -624,14 +629,14 @@ def test_one_proposal_is_approved_once(tmp_path: Path) -> None: store = _store(repository) proposed = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) request = ApprovalRequest( - decision=proposed, approver="roman", repository=repository + decision=proposed, approver="roman", repository=repository, via=TERMINAL ) approved = approve(request, store, lambda: MOMENT, _Runner()) with pytest.raises(DecisionError) as error: approve( ApprovalRequest( - decision=approved, approver="roman", repository=repository + decision=approved, approver="roman", repository=repository, via=TERMINAL ), store, lambda: MOMENT, @@ -648,7 +653,9 @@ def test_an_approval_records_who_gave_it(tmp_path: Path) -> None: with pytest.raises(DecisionError) as error: approve( - ApprovalRequest(decision=proposed, approver=" ", repository=repository), + ApprovalRequest( + decision=proposed, approver=" ", repository=repository, via=TERMINAL + ), store, lambda: MOMENT, _Runner(), @@ -657,30 +664,194 @@ def test_an_approval_records_who_gave_it(tmp_path: Path) -> None: assert error.value.code == "decision_approver_unknown" -def test_the_cli_shows_the_proposal_before_it_runs_anything( +class _Terminal(io.StringIO): + """A stream that says it is a terminal, as a person's shell is.""" + + def isatty(self) -> bool: + return True + + +LOCAL_PLAY = ( + "---\n- hosts: localhost\n connection: local\n gather_facts: false\n" + " tasks:\n - name: Nothing\n ansible.builtin.debug:\n" + " msg: nothing\n" +) + + +def _proposed_locally(repository: Path, capsys: pytest.CaptureFixture[str]) -> str: + """Propose deploy against localhost through the CLI; return the decision id.""" + _observations(repository, "web-1") + for name in ("deploy.yml", "health.yml"): + (repository / "playbooks" / name).write_text(LOCAL_PLAY, encoding="utf-8") + exit_code = main( + [ + *("operations", "propose", "deploy"), + *("--repository", str(repository), "--target", "localhost"), + *("--input", "version=1.4.0"), + ] + ) + proposed = json.loads(capsys.readouterr().out) + assert exit_code == 0 + return str(proposed["data"]["decision"]["metadata"]["id"]) + + +def _approve_at_terminal( + repository: Path, decision_id: str, typed: str +) -> tuple[int, dict[str, object]]: + """Run ``decisions approve`` as a person at a terminal who types ``typed``.""" + stdout = _Terminal() + exit_code = app.run( + [ + *("decisions", "approve", decision_id), + *("--repository", str(repository), "--approver", "roman"), + *("--format", "json"), + ], + stdin=_Terminal(f"{typed}\n"), + stdout=stdout, + stderr=io.StringIO(), + ) + return exit_code, json.loads(stdout.getvalue()) + + +def test_a_person_at_a_terminal_approves_by_typing_the_decision_id( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + repository = _repository(tmp_path) + decision_id = _proposed_locally(repository, capsys) + + exit_code, payload = _approve_at_terminal(repository, decision_id, decision_id) + + assert exit_code == 0, payload + loaded = _store(repository).load(ResourceId.from_boundary(decision_id)) + assert loaded.status is DecisionStatus.VERIFIED + assert loaded.approval is not None + assert loaded.approval.approver == "roman" + assert loaded.approval.via is ApprovalChannel.TERMINAL + on_disk = json.loads( + (repository / DECISION_DIRECTORY / f"{decision_id}.json").read_text( + encoding="utf-8" + ) + ) + assert on_disk["spec"]["approval"]["via"] == "terminal" + + +def test_a_mistyped_decision_id_approves_nothing( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: - """Without --yes the approval command changes nothing.""" + repository = _repository(tmp_path) + decision_id = _proposed_locally(repository, capsys) + + exit_code, payload = _approve_at_terminal(repository, decision_id, "yes") + + error = payload["error"] + assert isinstance(error, dict) + assert exit_code == 4 + assert error["code"] == "ATTESTATION_MISMATCH" + loaded = _store(repository).load(ResourceId.from_boundary(decision_id)) + assert loaded.status is DecisionStatus.PROPOSED + assert loaded.approval is None + + +@pytest.mark.parametrize("flags", [["--yes"], []], ids=["yes", "bare"]) +def test_an_approval_off_a_terminal_is_refused( + tmp_path: Path, capsys: pytest.CaptureFixture[str], flags: list[str] +) -> None: + """An agent's shell call cannot approve, ``--yes`` or not (#25).""" repository = _repository(tmp_path) store = _store(repository) proposed = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) exit_code = main( [ - "decisions", - "approve", - proposed.decision_id.value, - "--repository", - str(repository), + *("decisions", "approve", proposed.decision_id.value), + *("--repository", str(repository), "--approver", "roman"), + *flags, ] ) payload = json.loads(capsys.readouterr().out) - assert exit_code == 0 - assert payload["data"]["status"] == "pending" - assert payload["data"]["effect"] == "would_update" - assert payload["meta"]["dry_run"] is True - assert payload["data"]["decision"]["spec"]["status"] == "proposed" + assert exit_code == 4 + assert payload["error"]["code"] == "PERSON_REQUIRED" + assert payload["error"]["retryable"] is False + assert store.load(proposed.decision_id).status is DecisionStatus.PROPOSED + + +def test_a_raw_payload_cannot_approve_off_a_terminal( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + repository = _repository(tmp_path) + store = _store(repository) + proposed = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + payload_in = { + "decision": proposed.decision_id.value, + "repository": str(repository), + "approver": "roman", + "yes": True, + } + + exit_code = main( + ["decisions", "approve", "--raw-payload", json.dumps(payload_in)] + ) + + payload = json.loads(capsys.readouterr().out) + assert exit_code == 4 + assert payload["error"]["code"] == "PERSON_REQUIRED" + assert store.load(proposed.decision_id).status is DecisionStatus.PROPOSED + + +def test_the_approval_command_is_a_persons_and_never_a_tool() -> None: + command = app.commands[CommandPath("decisions.approve")] + + assert command.requires_person is True + assert command.mcp is False + + +def test_a_record_written_before_the_channel_reads_as_unknown( + tmp_path: Path, +) -> None: + repository = _repository(tmp_path) + store = _store(repository) + proposed = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + approved = approve( + ApprovalRequest( + decision=proposed, approver="roman", repository=repository, via=TERMINAL + ), + store, + lambda: MOMENT, + _Runner(), + ) + path = repository / DECISION_DIRECTORY / f"{approved.decision_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(approved.decision_id) + + assert loaded.approval is not None + assert loaded.approval.via is ApprovalChannel.UNKNOWN + shown = json.loads(json.dumps(loaded.as_document())) + assert shown["spec"]["approval"]["via"] == "unknown" + + +def test_a_new_approval_names_how_it_arrived(tmp_path: Path) -> None: + repository = _repository(tmp_path) + store = _store(repository) + proposed = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + + with pytest.raises(DecisionError) as error: + approve( + ApprovalRequest( + decision=proposed, + approver="roman", + repository=repository, + via=ApprovalChannel.UNKNOWN, + ), + store, + lambda: MOMENT, + _Runner(), + ) + + assert error.value.code == "decision_approval_channel_unknown" assert store.load(proposed.decision_id).status is DecisionStatus.PROPOSED @@ -693,7 +864,7 @@ def test_an_approved_decision_round_trips_through_the_store( proposed = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) approved = approve( ApprovalRequest( - decision=proposed, approver="roman", repository=repository + decision=proposed, approver="roman", repository=repository, via=TERMINAL ), store, lambda: MOMENT, @@ -719,7 +890,7 @@ def test_a_verify_step_that_changes_the_fleet_did_not_verify_it( approved = approve( ApprovalRequest( - decision=proposed, approver="roman", repository=repository + decision=proposed, approver="roman", repository=repository, via=TERMINAL ), store, lambda: MOMENT, @@ -745,6 +916,7 @@ def test_every_outcome_says_why_in_the_record(tmp_path: Path) -> None: ), approver="roman", repository=repository, + via=TERMINAL, ), store, lambda: MOMENT, @@ -760,6 +932,7 @@ def test_every_outcome_says_why_in_the_record(tmp_path: Path) -> None: ), approver="roman", repository=repository, + via=TERMINAL, ), store, lambda: MOMENT, diff --git a/sdk/tests/test_why.py b/sdk/tests/test_why.py index 5b75630..8977f4e 100644 --- a/sdk/tests/test_why.py +++ b/sdk/tests/test_why.py @@ -8,7 +8,14 @@ import pytest from cloudfall.cli import main -from cloudfall.decision import ApprovalRequest, DecisionStatus, approve, propose +from cloudfall.decision import ( + DECISION_DIRECTORY, + ApprovalChannel, + ApprovalRequest, + DecisionStatus, + approve, + propose, +) from cloudfall.why import Instant, WhyError, WhyQuery, answer, render_why_html from test_catalog import _repository from test_decision import MOMENT, _proposal, _Runner, _store @@ -26,7 +33,12 @@ def _record(repository: Path) -> tuple[Decision, Decision]: store = _store(repository) deploy = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) deploy = approve( - ApprovalRequest(decision=deploy, approver="roman", repository=repository), + ApprovalRequest( + decision=deploy, + approver="roman", + repository=repository, + via=ApprovalChannel.TERMINAL, + ), store, lambda: MOMENT, _Runner(), @@ -142,7 +154,10 @@ def test_every_sentence_of_the_story_cites_the_record(tmp_path: Path) -> None: assert story[2].startswith("Check mode would have changed web-1 and left 1 host") assert deploy.check.diff.sha256[:12] in story[2] assert story[3].startswith("It waited for an approval:") - assert story[4] == "roman approved it at 2026-09-21T14:30:12Z." + assert story[4] == ( + "roman approved it at 2026-09-21T14:30:12Z (via terminal): a person " + "typed its id at a terminal." + ) assert story[5].startswith( "The run at 2026-09-21T14:30:12Z exited 0 and changed web-1" ) @@ -180,6 +195,7 @@ def test_the_cli_answers_in_json_without_a_catalog( assert entry["id"] == "deploy-20260921143012" assert entry["hosts"] == ["web-1", "web-2"] assert entry["decision"]["spec"]["approval"]["approver"] == "roman" + assert entry["decision"]["spec"]["approval"]["via"] == "terminal" assert entry["decision"]["spec"]["verify"]["changed"] == [] assert entry["decision"]["spec"]["check"]["diff"]["sha256"] assert entry["decision"]["spec"]["basis"]["observedAt"] == { @@ -202,7 +218,30 @@ def test_the_cli_answers_as_a_page( assert "2 decisions" in page assert 'class="badge healthy">verified' in page assert 'class="badge warning">proposed' in page - assert "roman approved it at 2026-09-21T14:30:12Z." in page + assert "roman approved it at 2026-09-21T14:30:12Z (via terminal)" in page + + +def test_an_approval_recorded_before_the_channel_says_via_unknown( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + repository = _repository(tmp_path) + deploy, _ = _record(repository) + path = repository / DECISION_DIRECTORY / f"{deploy.decision_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") + + told = answer(_store(repository), WhyQuery.from_boundary(operation="deploy")) + exit_code = main(["why", "--repository", str(repository), "--host", "web-1"]) + + assert told.explanations[0].story[4] == ( + "roman approved it at 2026-09-21T14:30:12Z (via unknown): the record " + "predates noting how an approval arrived." + ) + payload = json.loads(capsys.readouterr().out) + assert exit_code == 0 + entry = payload["data"]["answers"][0] + assert entry["decision"]["spec"]["approval"]["via"] == "unknown" def test_the_cli_reports_a_bad_window_as_json( diff --git a/uv.lock b/uv.lock index 060090b..35ea2b9 100644 --- a/uv.lock +++ b/uv.lock @@ -285,7 +285,7 @@ requires-dist = [ { name = "jsonschema", extras = ["format"], specifier = ">=4.23,<5" }, { name = "mcp", marker = "extra == 'mcp'", specifier = ">=2,<3" }, { name = "pyyaml", specifier = ">=6,<7" }, - { name = "treaty", specifier = "==1.0.0rc29" }, + { name = "treaty", specifier = "==1.0.0rc38" }, ] provides-extras = ["mcp"] @@ -1085,11 +1085,11 @@ wheels = [ [[package]] name = "treaty" -version = "1.0.0rc29" +version = "1.0.0rc38" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/ec/50/e564b3e75431d5df24f170cf8726968a536bceb0ff7468da2308afac92de/treaty-1.0.0rc29.tar.gz", hash = "sha256:11f30f51f051d7da2008a2b15970edcb708a7d9becae8f1a0ecdd9631bb70a2f", size = 562870, upload-time = "2026-10-03T10:03:37.738Z" } +sdist = { url = "https://files.pythonhosted.org/packages/e8/b9/e20c42a64380f69eeff8ccb8aa04ca28dda5f8d00505f074757f20c0eba7/treaty-1.0.0rc38.tar.gz", hash = "sha256:580437e2d0f70f13340d5201639d9fba0ffec853980f9819fb0d365f82b76927", size = 515401, upload-time = "2026-10-08T17:50:35.324Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/02/b0/80f2a44a6151ddbc295f769dbf63542c88548f170f90fabb8c16d338e618/treaty-1.0.0rc29-py3-none-any.whl", hash = "sha256:1c7314db7ee78a7dc16199444c6486ece85a4ed1b24b622aedccf23b152d73c4", size = 561162, upload-time = "2026-10-03T10:03:35.85Z" }, + { url = "https://files.pythonhosted.org/packages/75/ee/c53221910616676275225137eaaa890d500f4d8ed7f82a0428059b994c56/treaty-1.0.0rc38-py3-none-any.whl", hash = "sha256:670573b0ed1b311531170abd2f3dc74f9443b757c1e60cf64ad4617a2d765770", size = 565262, upload-time = "2026-10-08T17:50:33.528Z" }, ] [[package]]