diff --git a/docs/governance/workstreams.md b/docs/governance/workstreams.md index a2fbc89f..26903274 100644 --- a/docs/governance/workstreams.md +++ b/docs/governance/workstreams.md @@ -222,6 +222,7 @@ state to its index: [`docs/adr/`](../adr/README.md), [`docs/pdr/`](../pdr/README | D206 | **A spike's record refused by `deliver`** (#347). On the third project, cycle 06, D76 was a spike measured outside the shipped program, its result a line of the module's log issue, its deliverable naming `module: transcription`: `nstack deliver D76 --held '#129'` refused it, no merge, and the refusal pointed only at merging | **Done**: no new path — the playbook already had one: a spike outside the code has `module: null`, and the closure accepts it from its line (`framing.md` §4). §2 now says a measure whose product is a record is such a spike even about one module, and `deliver`'s refusal names that door. Test seen red first: the refusal names it | | D207 | **An attested project's screenshots had no route** (#341). On the third project, with `checks: attested`, two criteria asked for screenshots in the sheet: a verifier found them named only, paths in a scratch folder gone after the run. The playbook asked for a CI artefact — none with attested checks — or an image attached to the pull request — which an App cannot do through the API. The project committed small PNGs under the module's `docs/evidence/`, unguided | **Done**: the verification playbook names the route — the verifier commits the screenshots and videos proving a criterion on a branch of its own, `evidence/`, never on the change's branch: its head stays the one verified (T4) and the verifier writes no commit of the change (T2) —, links each file from the sheet at that branch's commit, made-up data only, each file read before it is pushed, bounded by the `check-added-large-files` hook as it is committed; the branch goes once the cycle is closed. `tests.md`, `verification.md`'s automated step and `.gitignore` say it. Not built: an upload by `nstack attest`, weight for no user yet. A fresh verifier's finding fixed: a first draft committed beside the module, which T2 and T4 refuse. Test seen red first: the playbook names the branch and the hook, and the skeleton runs it | | D208 | **A person named verifier was the author's word** (#363). On the third project, 2026-11-04, pull request #643, opened by an agent App, said `Verifier: @napkinstack-admin` over results, evidence and commits it had written itself; that account never touched the pull request. T2 compared the name with the change's authors only: the sheet passed it, and on a project that owes no approval the merge waited on T4 alone, which the author turns green by running its own tests at the head. The project's session caught it by reading | **Done**: `nstack pr-check` reads, in one GraphQL query, whose hand is on the pull request — its description's editors, its reviewers, its commenters — and T2 refuses a person named verifier who has none — each person named, so that a second name does not carry the first; a bot named with its `[bot]` suffix is the same account. A verifier session is out of scope: it acts under the same App as the author's. Unread — no token, no answer, a field refused, more than one page —, it is said as a warning, never failed; on a workstation and under `nstack attest`, with no pull request number or recording the head, it is not read. The skeleton's `pull-request.yml` gives it `PR_NUMBER` and the job's token, with `pull-requests: read`, and runs again on a submitted review, as it does on an edit of the description; a comment is read at the next run. `RULES.md`, `05-workflow.md` §7 and `verification.md` say it. A fresh verifier's findings fixed: a field refused by the forge crashed the check, a review did not start it again, a second name carried the first, and four mutants survived. Test seen red first: the case of #643 refused, each kind of hand accepted, the query's shape and its unread answers, and the workflow's wiring | +| D209 | **An unattested head read "compliant" on the forge** (#367). On a fourth project, 2026-11-07, `checks: attested`, a pull request was opened with no record on its head: the CI job "Test sheet and cycle" said `attested — judged where nstack attest runs, not in CI` and `Pull request rules: compliant.` — `check_attested` returned before reading anything under GitHub Actions, the checkout keeping no credential to fetch the notes; the status `nstack / local checks` is posted only by a session holding a token, and that one had none. Only an observer's local `pr-check` said A1 | **Done**: the record is read from the remote's notes in CI as on a workstation — the fetch carries the job's token in git's environment (`http.extraheader`), masked for the runner, never in an argument a log shows, only towards GitHub and only under GitHub Actions — on a workstation the remote's own credential answers, an expired token in the environment would refuse it; an unattested head is refused (A1), a failed or incomplete record too (A2). A remote that cannot be read — no token, a refusal — is a warning that names why, never the silent line. The A1 action says the check reads the record again at its next run, a push or an edit of the description. `07-governance.md` §7, `RULES.md` and the workflow's `contents: read` say it. Tests seen red first: a record pushed with the notes, read by a fresh checkout under CI; no record on the remote refused; a failed record refused; no remote, and a remote that refuses, warned with git's reason; notes listed but not fetched unread; the token reaching git in CI and never on a workstation. A fresh verifier's findings fixed: the token was sent on a workstation too (an expired one refused the fetch, seen on a private repository), the refusing remote was untested, git's reason could be a fragment, the secret was not masked; three mutants killed. Probed on a private repository: an installation token in the environment reads its notes | | v0.6.0 | **What is announced refuses, and the documents say what is true**: D51 to D54, and the documentary lot of the internal audit | **Done (2026-09-20)**: five workstreams merged (#83, #84, #85, #86, #87), **v0.6.0 published**, tag on `8ae544c`, verified on published artefacts — including an update of `tool-library` from v0.5.0, 9 files and no conflict, still compliant. Found doing it: D55. Next: the pilot, and M11 beside it | | Pilot | **`NapkinStack/flet`, the first real project**, run by a session that knew nothing of this repository, with the maintainer as decider and this repository's agent as observer — public on the Free plan, where the whole checklist is available, rather than private on Team where G5 is paid and G6 does not exist | **Set aside by the maintainer, 2026-10-15**, at its cycle boundary; the real product followed since is `summit`. **Ran from 2026-09-20**: discovery decided *go* after three rounds and two independent challenges, charter accepted; cycle 01 closed `stopped/reframed`, **cycle 02 closed** with two deliverables accepted and one deferred (its #27), **cycle 03 framed and approved** (#28, #30), which starts its first `critical` module; the framework taken to v0.6.1 by itself at the boundary (#29), v0.6.2 proposed (#31). It has produced D56 to D63, D68, D70 to D77, replicated D26 and D55, and measured D49 — the first figures on what an independent verifier finds (PDR-0003) | | M11 | **Ceremony follows consequence** (PDR-0011): the framework charges what a failure costs, per module and per project, and states each price before it is paid; with PDR-0004's routing, and PDR-0007, PDR-0008 and PDR-0010 — through to v0.7.0 | **Published as v0.7.0 (2026-10-12).** Planned 2026-09-23 (#98). Merged: **M11f** (#101), **M11a** (#102), **M11b** (#103), **M11d** (#104), **M11e** (#105), **M11c** (#106), **M11h** (#107), **M11g** (#108) closed by its probe — not clean, returned to M13; **M11m** (D68, D74, D75); **M11l** (PDR-0010, D59, D60); **M11n** (D79); **M11o** (PDR-0010 extended: every document has its moment); **M11p** (PDR-0002's three levels given a place: `docs/project/rules/`); **M11j** (PDR-0007: the unframed stage); **M11q** (no folder created empty; PDR-0012 proposed: a project's tree is the project's). **M11k** (PDR-0008's print mode: `nstack settings`; D77, D78). **M11s** (PDR-0012 accepted; the framework's files under `.nstack/`; an update carries what a project adapted); **M11u** (an update rewrites the old addresses a project wrote in its own files, D85). **M11t** (`nstack new-contract`: the contracts module is created by the first contract; PDR-0012 built in full); **M11v** (what the third project found: D81, D82, D83, D86, D87). **M11i**: v0.7.0 published 2026-10-12 (#125, D88). To do: **M11r** PDR-0008's apply mode and D57, after a probe on the real forge; the two earlier projects take v0.7.0 themselves, observed | diff --git a/platform/tests/test_attest.py b/platform/tests/test_attest.py index d4ab225a..a6820437 100644 --- a/platform/tests/test_attest.py +++ b/platform/tests/test_attest.py @@ -5,6 +5,7 @@ from __future__ import annotations +import base64 import json import os import subprocess @@ -264,10 +265,132 @@ def test_attested_with_a_complete_passed_record_is_compliant_and_says_attested(t assert "Pull request rules: compliant." in out -def test_in_ci_an_attested_project_is_not_judged_on_a_record_ci_cannot_fetch(tmp_path, monkeypatch, capsys): +def with_remote(root: Path) -> Path: + """The project's remote, a bare repository holding its branch — as the forge would.""" + remote = root.parent / f"{root.name}-remote.git" + git(root.parent, "init", "-q", "--bare", str(remote)) + git(root, "remote", "add", "origin", str(remote)) + git(root, "push", "-q", "origin", "main") + return remote + + +def fresh_checkout(root: Path) -> None: + """CI's checkout: the branch, and none of the notes a workstation holds.""" + git(root, "update-ref", "-d", f"refs/notes/{attest.NOTES}") + + +def test_in_ci_the_record_is_read_from_the_remote_and_judged(tmp_path, monkeypatch, capsys): + """#367: on a fourth project, CI said "compliant" for a head nobody had attested — it never read the + record. Where the remote answers, the record pushed with the notes is the one judged.""" + project = tmp_path / "p" + project.mkdir() + base, _ = change(project, "attested") + with_remote(project) + monkeypatch.setattr(attest, "CHECKS", PASSING) + attest.run(project, base, None, "") # pushes the record with the notes + fresh_checkout(project) + code, out = judged(project, base, monkeypatch, capsys, ci=True) + assert code == 0 and "attested on a workstation, by test " in out, out + + +def test_in_ci_a_head_with_no_record_on_the_remote_is_refused(tmp_path, monkeypatch, capsys): + project = tmp_path / "p" + project.mkdir() + base, head = change(project, "attested") + with_remote(project) + code, out = judged(project, base, monkeypatch, capsys, ci=True) + assert code == 1 and f"FAIL [A1] no record of the head {head[:7]}" in out, out + assert "Pull request rules: compliant." not in out + + +def test_in_ci_a_failed_record_on_the_remote_is_refused(tmp_path, monkeypatch, capsys): + project = tmp_path / "p" + project.mkdir() + base, _ = change(project, "attested") + with_remote(project) + monkeypatch.setattr(attest, "CHECKS", FAILING) + attest.run(project, base, None, "") + fresh_checkout(project) + code, out = judged(project, base, monkeypatch, capsys, ci=True) + assert code == 1 and "FAIL [A2]" in out, out + + +def test_in_ci_a_record_that_cannot_be_read_is_said_never_called_compliant_silently(tmp_path, monkeypatch, capsys): + """No remote, or one that refuses: the record is unread — a warning, not a failure, and never the + silent line that let an unattested head through (#367).""" base, _ = change(tmp_path, "attested") code, out = judged(tmp_path, base, monkeypatch, capsys, ci=True) - assert code == 0 and "attested — judged where nstack attest runs, not in CI" in out + assert code == 0 and "WARNING [A1] the record of the head is not read here" in out, out + assert "judged where nstack attest runs" not in out + + +def test_in_ci_a_remote_that_refuses_is_warned_with_git_s_reason(tmp_path, monkeypatch, capsys): + project = tmp_path / "p" + project.mkdir() + base, _ = change(project, "attested") + git(project, "remote", "add", "origin", str(tmp_path / "gone.git")) # a remote that does not answer + code, out = judged(project, base, monkeypatch, capsys, ci=True) + assert code == 0 and "WARNING [A1] the record of the head is not read here (origin did not answer: fatal:" in out, out + assert "FAIL [A1]" not in out + + +SECRET = base64.b64encode(b"x-access-token:tok123").decode() + + +def test_the_fetch_authenticates_with_the_job_token_in_the_environment_never_an_argument(monkeypatch, capsys): + monkeypatch.setenv("GITHUB_ACTIONS", "true") + env = attest.fetch_env("https://github.com/org/repo", "tok123") + assert env["GIT_CONFIG_COUNT"] == "1" + assert env["GIT_CONFIG_KEY_0"] == "http.https://github.com/.extraheader" + assert env["GIT_CONFIG_VALUE_0"] == f"AUTHORIZATION: basic {SECRET}" + assert f"::add-mask::{SECRET}" in capsys.readouterr().out # the runner never prints it in clear + assert attest.fetch_env("https://github.com/org/repo", None) == {} + assert attest.fetch_env("/some/path.git", "tok123") == {} # a token goes to GitHub only + monkeypatch.delenv("GITHUB_ACTIONS") + assert attest.fetch_env("https://github.com/org/repo", "tok123") == {} # never on a workstation + + +class Recorded: + """git as read_remote calls it: each call's arguments and environment, answered as asked.""" + + def __init__(self, answers): + self.calls, self.answers, self.real = [], answers, subprocess.run + + def __call__(self, args, **kwargs): + if args[1] not in self.answers: # the local reads go to git itself + return self.real(args, **kwargs) + self.calls.append((args, kwargs.get("env") or {})) + code, out, err = self.answers[args[1]] + return subprocess.CompletedProcess(args, code, out, err) + + +def github_project(root: Path) -> None: + git(root, "init", "-q", "--initial-branch=main") + git(root, "remote", "add", "origin", "https://github.com/org/repo.git") + + +@pytest.mark.parametrize("ci", [True, False]) +def test_the_token_reaches_git_in_ci_and_only_there(tmp_path, monkeypatch, ci): + """On a workstation a token in the environment, perhaps expired, would be sent before the + remote's own credential and refuse the fetch (#373's verifier).""" + github_project(tmp_path) + monkeypatch.setenv("GH_TOKEN", "tok123") + monkeypatch.setenv("GITHUB_ACTIONS", "true") if ci else monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + recorded = Recorded({"ls-remote": (0, "", "")}) + monkeypatch.setattr(attest.subprocess, "run", recorded) + assert attest.read_remote(tmp_path) == ("origin", None) + args, env = recorded.calls[0] + assert args[1] == "ls-remote" and "tok123" not in " ".join(args) and SECRET not in " ".join(args) + assert (env.get("GIT_CONFIG_VALUE_0") == f"AUTHORIZATION: basic {SECRET}") is ci + + +def test_notes_listed_but_not_fetched_are_unread_not_read(tmp_path, monkeypatch): + github_project(tmp_path) + monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + recorded = Recorded({"ls-remote": (0, "abc123\trefs/notes/nstack/checks\n", ""), + "fetch": (128, "", "remote: some detail\nfatal: unable to access 'x': refused\nmore\n")}) + monkeypatch.setattr(attest.subprocess, "run", recorded) + assert attest.read_remote(tmp_path) == ("origin", "origin's notes not fetched: fatal: unable to access 'x': refused") def test_attest_judges_the_pull_request_rules_without_asking_for_the_record_it_is_making(tmp_path, monkeypatch, capsys): diff --git a/skeleton/.github/workflows/pull-request.yml b/skeleton/.github/workflows/pull-request.yml index 4c469d3e..2a3a1c9a 100644 --- a/skeleton/.github/workflows/pull-request.yml +++ b/skeleton/.github/workflows/pull-request.yml @@ -14,7 +14,7 @@ on: types: [submitted] permissions: - contents: read + contents: read # the head, and the attested record in the remote's notes (A1, A2) pull-requests: read # whose hand is on the pull request: a person named verifier (T2) # Each event's run reads the description and the head as they were at that event: a later event diff --git a/skeleton/.nstack/handbook/07-governance.md b/skeleton/.nstack/handbook/07-governance.md index e3c40727..58628519 100644 --- a/skeleton/.nstack/handbook/07-governance.md +++ b/skeleton/.nstack/handbook/07-governance.md @@ -302,7 +302,10 @@ as `nstack / local checks`, a name no CI job has. Such a change is **attested**, *checked*: the record makes a claim traceable — who ran it, on which commit, with what output — not true. Whether a record stands in for the forge's CI is the decider's word, the project's answer `checks: attested` in `.copier-answers.yml`, changed in a pull request; `pr-check` then -requires a complete, passed record of the head (A1, A2). Attesting again once the test sheet is filled keeps the +requires a complete, passed record of the head (A1, A2) — on the workstation and in the forge's +CI alike, which reads the record from the remote's notes with the job's token: an unattested head +is red there too. A record CI cannot read — no token, a remote that refuses — is said as a +warning, never called compliant in silence. Attesting again once the test sheet is filled keeps the module checks the commit's record passed — they read the tree and the base, never the description — when the base commit, the engine and the person are the same, and the engine is published or named by a commit with no change of its own — tracked or not. The record says which run they come from diff --git a/skeleton/.nstack/handbook/RULES.md b/skeleton/.nstack/handbook/RULES.md index 2b63264d..60caa571 100644 --- a/skeleton/.nstack/handbook/RULES.md +++ b/skeleton/.nstack/handbook/RULES.md @@ -107,7 +107,7 @@ in git; elsewhere a record is shown, and the forge's CI judges. | Code | Refuses | Why | |---|---|---| -| `A1` | In a project whose checks are attested, a head commit with no record | `07-governance.md` §7 | +| `A1` | In a project whose checks are attested, a head commit with no record — read from the remote's notes, in CI too | `07-governance.md` §7 | | `A2` | A record of the head that misses a check the CI runs, or holds a failed one | `07-governance.md` §7 | ## K — the cycle a change belongs to diff --git a/src/napkinstack/attest.py b/src/napkinstack/attest.py index 2bbd4ab0..e661d488 100644 --- a/src/napkinstack/attest.py +++ b/src/napkinstack/attest.py @@ -15,6 +15,7 @@ from __future__ import annotations +import base64 import datetime import hashlib import json @@ -153,16 +154,55 @@ def _remote(root: Path) -> str | None: return "origin" if "origin" in remotes else None -def fetch(root: Path) -> str | None: - """Brings the remote's notes in, merged line by line with ours; returns the remote's name.""" +def fetch_env(url: str, token: str | None) -> dict[str, str]: + """In GitHub Actions only, the credential a fetch from GitHub carries — in git's environment, + never in an argument a log shows, and masked for the runner. CI's checkout keeps none + (`persist-credentials: false`); the job's token reads (#367). Never on a workstation: there the + remote's own credential answers, and a token in the environment, perhaps expired, would be sent + before it and refuse the fetch.""" + if os.environ.get("GITHUB_ACTIONS") != "true" or not token or not url.startswith("https://github.com/"): + return {} + secret = base64.b64encode(f"x-access-token:{token}".encode()).decode() + print(f"::add-mask::{secret}", flush=True) # as actions/checkout does, and hold's own fetch + return {"GIT_CONFIG_COUNT": "1", "GIT_CONFIG_KEY_0": "http.https://github.com/.extraheader", + "GIT_CONFIG_VALUE_0": f"AUTHORIZATION: basic {secret}"} + + +def _why(stderr: str) -> str: + """git's reason, in one line: its `fatal:` line when it says one, else its last.""" + lines = [line.strip() for line in stderr.strip().splitlines() if line.strip()] + fatal = [line for line in lines if line.startswith("fatal:")] + return (fatal or lines or ["no reason given"])[0 if fatal else -1] + + +def read_remote(root: Path) -> tuple[str | None, str | None]: + """Brings the remote's notes in, merged line by line with ours. Returns the remote's name, and + why its notes were not read — None when they were, or when it holds none.""" remote = _remote(root) if remote is None: - return None + return None, "no remote" + url = _git(root, "remote", "get-url", remote, check=False).stdout.strip() + env = dict(os.environ, **fetch_env(url, os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN"))) + + def git(*args: str) -> subprocess.CompletedProcess: + return subprocess.run(["git", *args], cwd=root, capture_output=True, text=True, env=env) + + listed = git("ls-remote", remote, f"refs/notes/{NOTES}") + if listed.returncode != 0: + return remote, f"{remote} did not answer: {_why(listed.stderr)}" + if not listed.stdout.strip(): + return remote, None # read: the remote holds no record at all theirs = f"refs/notes/nstack-remote/{NOTES.split('/')[-1]}" - got = _git(root, "fetch", "--quiet", remote, f"+refs/notes/{NOTES}:{theirs}", check=False) - if got.returncode == 0: - _git(root, "notes", f"--ref={NOTES}", "merge", "--quiet", "--strategy=cat_sort_uniq", theirs, check=False) - return remote + got = git("fetch", "--quiet", remote, f"+refs/notes/{NOTES}:{theirs}") + if got.returncode != 0: + return remote, f"{remote}'s notes not fetched: {_why(got.stderr)}" + _git(root, "notes", f"--ref={NOTES}", "merge", "--quiet", "--strategy=cat_sort_uniq", theirs, check=False) + return remote, None + + +def fetch(root: Path) -> str | None: + """Brings the remote's notes in, merged line by line with ours; returns the remote's name.""" + return read_remote(root)[0] def _session(root: Path, commit: str) -> str | None: diff --git a/src/napkinstack/pull_request.py b/src/napkinstack/pull_request.py index 3e468777..8c36a874 100644 --- a/src/napkinstack/pull_request.py +++ b/src/napkinstack/pull_request.py @@ -570,17 +570,18 @@ def charter_without_pdr(root: Path, base: str, files: list[str]) -> list[str]: "correction that decides nothing needs none (.nstack/playbooks/framing.md, Decisions)."] -def check_attested(root: Path, head: str, fail) -> str: +def check_attested(root: Path, head: str, fail, warn=lambda message: None) -> str: """Where the checks are judged, as the decider's answer `checks` says (A1, A2): on the forge's CI, or attested on a workstation by `nstack attest` — then a complete, passed record of the - head is required. Returns the line the summary prints.""" + head is required, read from the remote's notes wherever it runs, CI included (#367). Returns + the line the summary prints.""" from napkinstack import attest # the record's reader, and the one list of checks try: answers = yaml.safe_load((root / ANSWERS).read_text(encoding="utf-8")) or {} except (OSError, yaml.YAMLError): answers = {} - attest.fetch(root) + _, unread = attest.read_remote(root) record = attest.latest(root, head) if answers.get("checks", "forge") != "attested": shown = "" @@ -590,13 +591,17 @@ def check_attested(root: Path, head: str, fail) -> str: return "the forge's CI" + shown if os.environ.get("NSTACK_ATTESTING") == head: return "attested — this run is the record being made" - if os.environ.get("GITHUB_ACTIONS") == "true": - # CI's checkout keeps no credential to fetch the notes: where the checks are attested, the - # record is read where the merge is decided, never failed here for want of a fetch. - return "attested — judged where nstack attest runs, not in CI" + if record is None and unread and os.environ.get("GITHUB_ACTIONS") == "true": + # The record may well be there, out of reach: said, never failed for want of a fetch — and + # never the silent line that let an unattested head through (#367). + warn(f"[A1] the record of the head is not read here ({unread}): whether {head[:7]} was attested is " + "unknown to this run.\n Action: give the job a token that reads the repository " + "(GH_TOKEN, contents: read), or run nstack pr-check where the merge is decided.") + return "attested — the record of the head not read here" if record is None: fail("A1", f"no record of the head {head[:7]}: this project's checks are attested, and none ran on " - "this commit.\n Action: run nstack attest on the head, then push.") + "this commit.\n Action: run nstack attest on the head, then push — the forge's check reads the " + "record again at its next run: a push, or an edit of the description.") return "attested — no record of the head" names = [check["name"] for check in record["checks"]] missing = [check.name for check in attest.CHECKS if check.name not in names] @@ -662,7 +667,7 @@ def tree_of(commit: str) -> str: if modules: check_cycle(root, body, labels, datetime.date.today(), fail, warnings.append) check_hold(root, body, labels, authorship, fail, warnings.append, identities(root, base, head)) - checks = check_attested(root, head, fail) + checks = check_attested(root, head, fail, warnings.append) print(f"Modules touched : {len(modules)}" + "".join(f"\n - {folder}" for folder in modules)) print(f"Test sheet : {'required' if reasons else 'not required'}"