diff --git a/docs/run-spec.md b/docs/run-spec.md index 46fc553..6a44d6f 100644 --- a/docs/run-spec.md +++ b/docs/run-spec.md @@ -106,6 +106,20 @@ Rules the engine enforces: - **Modes** are `manual` and `auto-confirm`. Auto-confirm applies confirmation only; running the step's own command is a separate, later level ("auto-pilot with execution", RBT-WP-0006). +## Closed runs are locked and sealed + +Closing a run (`finalize`) makes its record final: + +- **Locked.** A closed record cannot be saved with changes: the engine refuses, naming what would have changed. The only later update it + accepts is `exercise_recorded`, written by the legacy "Record pack as exercised" step. This holds for runs closed before sealing existed too. +- **Feedback.** It locks with the run. Give it before closing, or with the closing form: `finalize(..., rating, comment)` records an optional + rating and comment together with the outcome (validated first, so a refused close changes nothing). This keeps abandoned and failed runs from + losing what the operator wanted to say. +- **Sealed.** The record carries a SHA-256 `seal` of its content (excluding `seal`, `updated` and `exercise_recorded`), and the file is made + read-only. `seal_state` reports `open`, `sealed`, `broken` (changed outside the app after closing) or `unsealed` (closed before sealing existed). + A broken seal is called out in the UI, and such a run no longer counts as maturity evidence. This is tamper-evidence, not prevention: someone + with file access can still edit a file, but not without being noticed. + ## Changing a value after steps used it A v0.1 run fixes its parameters when it starts. In a v0.2 run datapoints can be edited (guided flow questions, data view), but a value diff --git a/rtut/maturity.py b/rtut/maturity.py index b96577c..34555cd 100644 --- a/rtut/maturity.py +++ b/rtut/maturity.py @@ -53,7 +53,8 @@ def _env_key(ctx: dict) -> str: def _evidence(pack: Pack, dig: str, runs: list[R.Run]) -> dict: clean = [r for r in runs if r.data.get("pack_digest") == dig and r.data.get("outcome") == "succeeded" - and not r.data.get("deviations") and r.data.get("context", {}).get("environment")] + and not r.data.get("deviations") and r.data.get("context", {}).get("environment") + and R.seal_state(r) != "broken"] # a record changed after closing is not evidence reviewed = any(rv.get("digest") == dig for rv in _reviews(pack)) rehearsal = [r for r in clean if r.data["context"]["environment"] == "rehearsal"] envs = {_env_key(r.data["context"]) for r in clean} diff --git a/rtut/run.py b/rtut/run.py index 27d6b8d..c9ac4ef 100644 --- a/rtut/run.py +++ b/rtut/run.py @@ -44,10 +44,22 @@ class Run: def path(self) -> Path: return runs_dir(self.pack) / f"{self.data['run_id']}.json" - def save(self) -> None: + def save(self, allow: tuple = ()) -> None: + """Write the record. A closed run is locked: nothing may change except the keys named in `allow`.""" + disk = _read_json(self.path) + if disk and disk.get("outcome"): + keys = set(disk) | set(self.data) + changed = {k for k in keys if disk.get(k) != self.data.get(k)} - {"updated"} - set(allow) + if changed: + raise RunError(f"this run is closed and locked ({disk['outcome']}); it cannot be changed " + f"({', '.join(sorted(changed))})") self.data["updated"] = _now() self.path.parent.mkdir(parents=True, exist_ok=True) + if self.path.exists(): + self.path.chmod(0o644) # a sealed file is read-only; the allowed update reopens it briefly self.path.write_text(json.dumps(self.data, indent=2, sort_keys=True) + "\n") + if self.data.get("outcome"): + self.path.chmod(0o444) def step_def(self, step_id: str) -> dict: if self.pack.spec == "runbook-pack/v0.2": # steps live in stages; alternatives are applied @@ -280,12 +292,19 @@ def exercise_update(run: Run, today: date | None = None) -> tuple[str, str]: RATINGS = {"good": ":)", "neutral": ":|", "poor": ":("} -def add_feedback(run: Run, rating: str, comment: str = "") -> dict: - """Record how the runbook felt. Allowed at any time; the latest entry is the current one.""" +def _feedback_entry(rating: str, comment: str) -> dict: if rating not in RATINGS: raise RunError(f"rating must be one of {', '.join(RATINGS)}") - entry = {"rating": rating, "comment": _clean(comment.strip(), "comment"), - "at": datetime.now(timezone.utc).isoformat(timespec="microseconds")} # sortable even within one second + return {"rating": rating, "comment": _clean(comment.strip(), "comment"), + "at": datetime.now(timezone.utc).isoformat(timespec="microseconds")} # sortable even within one second + + +def add_feedback(run: Run, rating: str, comment: str = "") -> dict: + """Record how the runbook felt. Possible while the run is open (the latest entry counts); a closed run is locked, + so give the feedback before closing, or with the closing form.""" + if run.data.get("outcome"): + raise RunError("this run is closed and locked: its feedback can no longer be changed") + entry = _feedback_entry(rating, comment) run.data.setdefault("feedback", []).append(entry) run.save() return entry @@ -359,8 +378,37 @@ OUTCOMES = ("succeeded", "failed", "abandoned") STALE_DAYS = 7 -def finalize(run: Run, outcome: str, note: str = "") -> None: - """Close a run. 'succeeded' needs every step done; 'failed' needs a failed step.""" +def _seal_of(data: dict) -> str: + """Fingerprint of a closed record. Timestamps of allowed later updates and the exercise marker are not part of it.""" + import hashlib + body = {k: v for k, v in data.items() if k not in ("seal", "updated", "exercise_recorded")} + return hashlib.sha256(json.dumps(body, sort_keys=True).encode()).hexdigest() + + +def _read_json(path) -> dict | None: + try: + return json.loads(Path(path).read_text()) + except (OSError, ValueError): + return None + + +def seal_state(run: Run) -> str: + """'open' | 'sealed' | 'broken' (changed after closing, outside the app) | 'unsealed' (closed before sealing existed). + + Judged on what is on disk, not on the in-memory copy.""" + disk = _read_json(run.path) or run.data + if not disk.get("outcome"): + return "open" + if not disk.get("seal"): + return "unsealed" + return "sealed" if disk["seal"] == _seal_of(disk) else "broken" + + +def finalize(run: Run, outcome: str, note: str = "", rating: str | None = None, comment: str = "") -> None: + """Close a run: its outcome is recorded, its record sealed and locked. An optional rating and comment are + recorded with it, because a closed run can no longer take feedback. + + 'succeeded' needs every step done; 'failed' needs a failed or pushed-through step.""" if run.data.get("outcome"): raise RunError(f"run already finished ({run.data['outcome']})") if outcome not in OUTCOMES: @@ -373,10 +421,14 @@ def finalize(run: Run, outcome: str, note: str = "") -> None: gstates = [g["state"] for g in run.data.get("gates", {}).values()] if outcome == "failed" and not ({"failed", "pushed"} & set(states) or {"failed", "pushed"} & set(gstates)): raise RunError("a run can be finished as failed only when a step or gate has failed or was pushed through; otherwise abandon it") + entry = _feedback_entry(rating, comment) if rating else None # validated before anything changes run.data["outcome"] = outcome run.data["finished"] = _now() if note.strip(): run.data["outcome_note"] = _clean(note.strip(), "note") + if entry: + run.data.setdefault("feedback", []).append(entry) + run.data["seal"] = _seal_of(run.data) run.save() @@ -462,5 +514,5 @@ def apply_exercise(run: Run, today: date | None = None) -> str: status = STATUS_LINE.search(new).group(0).split(": ", 1)[1] run.pack.data["exercise_status"] = status run.data["exercise_recorded"] = status - run.save() + run.save(allow=("exercise_recorded",)) # the one thing a closed run may still receive return status diff --git a/rtut/ui.py b/rtut/ui.py index 47d6a92..e82ba04 100644 --- a/rtut/ui.py +++ b/rtut/ui.py @@ -75,6 +75,7 @@ a.chip:hover{background:var(--hi);color:#141414;border-color:var(--hi)}.chip.on{ .pill[data-s=failed],.pill[data-s=err],.pill[data-s=invalid]{color:var(--bad);border-color:var(--bad)} .pill[data-s=changed-after-use],.pill[data-s=pushed],.pill[data-s=waived],.pill[data-s=stale],.pill[data-s=override],.pill[data-s=push-through],.pill[data-s=waiver],.pill[data-s=modified],.pill[data-s=abandoned]{color:var(--warn);border-color:var(--warn)} .pill[data-s=nil],.pill[data-s=todo],.pill[data-s=open],.pill[data-s=planned],.pill[data-s="not checked"],.pill[data-s=not-applied]{color:var(--mut);border-style:dashed} +.hl a{text-decoration:none}.hl a::after{content:" โ";opacity:.35;font-weight:400}.hl a:hover::after{opacity:1} .feed{list-style:none;padding:0}.feed li{padding:.3rem 0;border-bottom:1px dashed var(--line)} .checks{list-style:none;padding:0;margin:.4rem 0}.checks li{padding:.35rem 0;border-bottom:1px dashed var(--line)}.checks li:last-child{border:0} #current{scroll-margin-top:1rem} @@ -143,6 +144,13 @@ def e(s) -> str: return html.escape(str(s)) +def heading(level: str, text: str, href: str | None = None, extra: str = "") -> str: + """A section headline. On the Home page it links to the matching sidebar section, as a shortcut.""" + if href: + return f"<{level} class=hl>{html.escape(text)}{extra}{level}>" + return f"<{level}>{html.escape(text)}{extra}{level}>" + + SMILEYS = (("good", "๐", "Went well"), ("neutral", "๐", "So-so"), ("poor", "๐", "Poor")) @@ -177,15 +185,15 @@ class App: return (f"" f"
Running as {label} ยท change
") - def activities_html(self, limit: int = 12, level: str = "h2") -> str: + def activities_html(self, limit: int = 12, level: str = "h2", link: bool = False) -> str: events = ACT.latest(self.packs, limit) if not events: - return f"<{level}>Latest activities{level}>Nothing has happened yet. Start a run or a project.
" + return f"{heading(level, 'Latest activities', '/activity' if link else None)}Nothing has happened yet. Start a run or a project.
" rows = "".join( f"Run finished: {e(outcome)}{note}
" + box = f"Run finished: {e(outcome)}{note}
{self.locked_note(r)}" if outcome == "succeeded": if r.data.get("exercise_recorded"): box += f"Pack recorded as {e(r.data['exercise_recorded'])}. Commit the change in the owning repository.
" @@ -315,7 +323,7 @@ class App: "Finishing records the outcome in the pack's score. A finished run is read-only. " "Unfinished runs count as stalled, and as abandoned after a week.
" f"") def feedback_form(self, i: int, rid: str, r) -> str: @@ -328,14 +336,41 @@ class App: "Never include secrets." f"{btns}
") - if fb: # collected: close the section, suggest the next step + if r.data.get("outcome"): # a closed run is locked: show what was said, offer no change + if not fb: + return "" + face = next(f for k, f, _ in SMILEYS if k == fb["rating"]) + return (f"{face} {e(fb['comment'] or '(no comment)')}
" + "The run is closed and locked, so its feedback can no longer be changed.
Thank you. Recorded: " f"{face} {e(fb['comment'] or '(no comment)')}
" - "" f"โ This closed run's record was changed outside the app after it was sealed. " + "It no longer counts towards maturity.
") + base = ("This run is closed and locked: its record is sealed and cannot be changed. " + "Start a new run to try again.") + if state == "unsealed": + base += " (It was closed before sealing existed, so its record carries no seal.)" + return f"{base}
" + def run_page(self, i: int, rid: str, msg: str = "", query: dict | None = None) -> bytes: p = self.packs[i]; r = R.load(p, rid); d = p.data if p.spec == "runbook-pack/v0.2": @@ -424,7 +459,7 @@ class App: if r.complete: out.append("All steps done. To record this run as exercising the pack, review and apply: "
f"python -m rtut run finish {e(p.path)} {e(rid)} --apply
{note}
" - "The outcome is part of this runbook's score and maturity evidence at this revision. " - "โ Back to the runbook list
{note}
{c.app.locked_note(run)}" + "The outcome is part of this runbook's score and maturity evidence at this revision.
Finishing records the outcome. A run with deviations can be finished but does not count towards maturity. " "A finished run is read-only.
" f"") + f"{c.app.close_feedback_fields(run)}{' '.join(btns)}
") # ---- actions ------------------------------------------------------------------------------------------------------------------- @@ -638,7 +637,7 @@ def handle_post(app, i: int, rid: str, parts: list[str], form: dict) -> str: R1.add_feedback(run, form.get("rating", ""), form.get("comment", "")) return f"{base}#feedback" if parts == ["finalize"]: - R1.finalize(run, form.get("outcome", ""), form.get("note", "")) + R1.finalize(run, form.get("outcome", ""), form.get("note", ""), rating=form.get("rating") or None, comment=form.get("fb_comment", "")) return f"{base}#close" if parts == ["mode"]: R.switch_mode(run, form.get("to", "")) diff --git a/tests/test_score.py b/tests/test_score.py index 36977ee..548974c 100644 --- a/tests/test_score.py +++ b/tests/test_score.py @@ -42,16 +42,17 @@ def test_finalize_rules(pk): R.finalize(r, "abandoned") # already finished -def test_finished_run_is_read_only_but_takes_feedback(pk): +def test_finished_run_is_read_only_and_feedback_is_locked_too(pk): r = R.create(pk, "bernd"); R.finalize(r, "abandoned") for call in (lambda: R.start(r, "read-the-sign"), lambda: R.add_capture(r, "read-the-sign", "x")): with pytest.raises(R.RunError, match="finished"): call() - R.add_feedback(r, "poor", "gave up: unclear") # feedback stays possible + with pytest.raises(R.RunError, match="closed and locked"): + R.add_feedback(r, "poor", "gave up: unclear") # feedback goes in before closing, or with the closing form def test_scorecard_counts_every_outcome_and_stale_open_runs(pk): - ok = R.create(pk, "a"); play_all(ok); R.finalize(ok, "succeeded"); R.add_feedback(ok, "good") + ok = R.create(pk, "a"); play_all(ok); R.finalize(ok, "succeeded", rating="good") bad = R.create(pk, "b"); R.start(bad, "read-the-sign"); R.fail(bad, "read-the-sign", "x"); R.finalize(bad, "failed") gone = R.create(pk, "c"); R.finalize(gone, "abandoned") stalled = R.create(pk, "d") @@ -151,3 +152,72 @@ def test_unfinished_run_elapsed_runs_to_now_and_no_average_without_succeeded_run t = R.run_times(r, now=datetime.now(timezone.utc) + timedelta(seconds=90)) assert not t["finished_run"] and 89 <= t["elapsed"] <= 91 assert R.scorecard(pk)["avg_seconds"] is None and R.scorecard(pk)["timed_runs"] == 0 + + +# ---- a closed run is locked and sealed -------------------------------------------------------------------------- + +def test_closing_records_feedback_with_the_outcome_and_validates_before_changing_anything(pk): + r = R.create(pk, "bernd") + with pytest.raises(R.RunError, match="rating must be"): + R.finalize(r, "abandoned", rating="great") + assert not r.data.get("outcome") and "seal" not in r.data # a refused close changed nothing + R.finalize(r, "abandoned", "gave up", rating="poor", comment="step 2 was unclear") + assert R.latest_feedback(r)["comment"] == "step 2 was unclear" and r.data["outcome"] == "abandoned" + assert R.load(pk, r.data["run_id"]).data["feedback"][0]["rating"] == "poor" + + +def test_a_closed_record_cannot_be_saved_with_changes_but_the_exercise_marker_is_allowed(pk): + r = R.create(pk, "bernd"); R.finalize(r, "abandoned") + r2 = R.load(pk, r.data["run_id"]) + r2.data["steps"]["read-the-sign"]["state"] = "done" + with pytest.raises(R.RunError, match="closed and locked"): + r2.save() + r3 = R.load(pk, r.data["run_id"]) + r3.data["operator"] = "someone-else" + with pytest.raises(R.RunError, match="operator"): + r3.save() + assert R.load(pk, r.data["run_id"]).data["operator"] == "bernd" # nothing reached the disk + r4 = R.load(pk, r.data["run_id"]) + r4.data["exercise_recorded"] = "x" + r4.save(allow=("exercise_recorded",)) # the one permitted later update + assert R.load(pk, r.data["run_id"]).data["exercise_recorded"] == "x" and R.seal_state(R.load(pk, r.data["run_id"])) == "sealed" + + +def test_closing_seals_the_file_read_only_and_tampering_is_detected(pk): + import json, stat + r = R.create(pk, "bernd") + assert R.seal_state(r) == "open" and r.path.stat().st_mode & stat.S_IWUSR + R.finalize(r, "abandoned") + assert R.seal_state(r) == "sealed" and not (r.path.stat().st_mode & (stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH)) + assert len(r.data["seal"]) == 64 + r.path.chmod(0o644) # someone edits the file by hand + data = json.loads(r.path.read_text()); data["operator"] = "forged" + r.path.write_text(json.dumps(data)) + assert R.seal_state(R.load(pk, r.data["run_id"])) == "broken" + + +def test_a_run_closed_before_sealing_existed_is_reported_as_unsealed_not_broken(pk): + import json + r = R.create(pk, "bernd"); R.finalize(r, "abandoned") + r.path.chmod(0o644) + data = json.loads(r.path.read_text()); data.pop("seal"); r.path.write_text(json.dumps(data)) + assert R.seal_state(R.load(pk, r.data["run_id"])) == "unsealed" + + +def test_a_tampered_run_no_longer_counts_as_maturity_evidence(pk): + import json + from rtut import maturity as M + M.add_review(pk, "alice") + r = R.create(pk, "bob", {"logdir": "/tmp/rtut-lock-mat"}, {"project": "lab", "environment": "rehearsal"}) + for s in pk.data["steps"]: + sid = s["id"]; R.start(r, sid) + cmd = R.command_for(r, sid) + if cmd: + subprocess.run(cmd, shell=True, check=True, capture_output=True) + R.verify(r, sid, confirm=True, manual_ok=True if s["verify"]["expect"] == "manual" else None) + R.complete(r, sid, {n: "1" for n in s.get("evidence", [])}) + R.finalize(r, "succeeded") + assert M.maturity(pk)["evidence"]["rehearsal_runs"] == 1 + r.path.chmod(0o644) + data = json.loads(r.path.read_text()); data["operator"] = "forged"; r.path.write_text(json.dumps(data)) + assert M.maturity(pk)["evidence"]["clean_runs"] == 0 # changed after closing: not evidence diff --git a/tests/test_ui.py b/tests/test_ui.py index 06b614f..88bd957 100644 --- a/tests/test_ui.py +++ b/tests/test_ui.py @@ -205,18 +205,18 @@ def test_feedback_form_after_all_steps_done_and_recording(srv): t, loc, _ = _showcase_run(srv) base = loc.split("#")[0] _, _, body = req(srv, "GET", base) - assert "How was this runbook?" not in body # not before the run is complete + assert "