From 1b321d1ac50508040dcc78157869a1cced8d6079 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Fri, 16 Oct 2026 02:59:15 -0500 Subject: [PATCH 1/4] Kernel: a Lens run by GitHub Actions is read only from its own workflow file GitHub Actions is one App, 15368, for every job in the repo, one a pull request adds on its own branch among them. A Lens Actions runs is now pinned to its workflow file as well as its App. - The facts carry, for each check, `workflow`: the path of its check suite's workflow run, or null for a check no workflow ran. The configuration names each deterministic Lens's file: verify is .github/workflows/verify.yml, the secret scan .github/workflows/ci.yml, code-scanning null. - A check under a Lens's check name from its own App and another file, or with no file read, leaves the Lens unreadable as another App's check does (check_unreadable, evidence `:from_workflow:`, the path `unreadable` when none was read), on a queued item too, and moves nothing. A Lens that names no file is read only from a check that names none. - tools/check_workflows.py holds the configuration to the real files: each file a Lens names exists and runs a job under its check name. - The simulator's impostor fault posts a passing check from another workflow file of the Lens's own App, beside the one from another App. - Platform facts, unverified: the workflow file is readable (P-35), and an upload to code scanning under CodeQL's tool and category cannot replace the CodeQL verdict (P-36), in sim/grounding.py and design section 6. - Decision 0017 replaces 0015 and adds the ruling. Pinning each required check's App in the main ruleset stays the Maintainer's, with "The Factory repo's rulesets hold the Release tags and each required check's App" (issue 62). Co-Authored-By: Claude Opus 5.5 --- config/defaults.json | 9 +- docs/architecture/software-factory-design.md | 37 +++- ...05-each-role-writes-through-its-own-app.md | 2 +- ...n-app-and-only-what-gates-holds-a-merge.md | 7 +- ...kflow-and-only-what-gates-holds-a-merge.md | 111 +++++++++++ examples/facts.json | 3 + ...ge-by-a-person-skips-the-model-review.json | 9 +- .../config.a-neutral-check-passes.json | 9 +- .../config.a-required-lens-with-no-app.json | 9 +- .../config.a-required-lens-with-no-check.json | 9 +- ...nfig.a-required-lens-with-no-workflow.json | 60 ++++++ ...eness-limit-copied-from-the-lifecycle.json | 9 +- .../config.conclusion-mapped-to-nothing.json | 9 +- .../invalid/facts.a-category-nobody-owns.json | 3 + .../facts.a-check-without-its-app.json | 3 + ...s.a-check-without-its-completion-time.json | 3 + .../facts.a-check-without-its-workflow.json | 179 ++++++++++++++++++ .../invalid/facts.a-conclusion-in-words.json | 3 + ...cts.a-label-event-without-who-made-it.json | 3 + ...cts.a-thread-resolved-by-a-name-alone.json | 3 + .../facts.a-thread-without-its-category.json | 3 + .../facts.a-time-with-an-impossible-day.json | 3 + ...facts.a-time-with-an-impossible-month.json | 3 + .../facts.acceptance-carries-prose.json | 3 + ...-acceptance-contract-with-no-criteria.json | 3 + ...s.an-acceptance-criterion-named-twice.json | 3 + .../facts.an-acceptance-result-in-words.json | 3 + ...acceptance-result-without-its-content.json | 3 + ...or-of-a-kind-the-kernel-does-not-know.json | 3 + .../facts.an-actor-without-its-kind.json | 3 + examples/invalid/facts.carries-prose.json | 3 + ...ssed-review-without-its-earlier-state.json | 3 + .../facts.no-completeness-for-acceptance.json | 3 + .../facts.no-completeness-for-runs.json | 3 + .../facts.no-completeness-for-timeline.json | 3 + examples/invalid/facts.no-completeness.json | 3 + examples/invalid/facts.no-count-of-acts.json | 3 + .../invalid/facts.no-word-on-acceptance.json | 3 + .../facts.review-without-reviewer.json | 3 + .../invalid/facts.time-in-another-format.json | 3 + kernel/derive.py | 70 ++++--- schemas/config.schema.json | 11 +- schemas/facts.schema.json | 9 + sim/grounding.py | 9 + sim/world.py | 37 +++- tests/support.py | 6 +- tests/test_decide.py | 97 +++++++++- tests/test_simulator.py | 7 +- tests/test_tools.py | 23 +++ tools/check_workflows.py | 29 ++- tools/mutants.py | 37 +++- 51 files changed, 799 insertions(+), 76 deletions(-) create mode 100644 docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md create mode 100644 examples/invalid/config.a-required-lens-with-no-workflow.json create mode 100644 examples/invalid/facts.a-check-without-its-workflow.json diff --git a/config/defaults.json b/config/defaults.json index 3860b013..2b719d31 100644 --- a/config/defaults.json +++ b/config/defaults.json @@ -7,17 +7,20 @@ { "lens": "verify", "check": "verify", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/verify.yml" }, { "lens": "secret-scan", "check": "Secret scan (gitleaks)", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/ci.yml" }, { "lens": "code-scanning", "check": "CodeQL", - "app_id": 57789 + "app_id": 57789, + "workflow": null } ], "model": [ diff --git a/docs/architecture/software-factory-design.md b/docs/architecture/software-factory-design.md index 0826b056..755f61f6 100644 --- a/docs/architecture/software-factory-design.md +++ b/docs/architecture/software-factory-design.md @@ -323,7 +323,7 @@ for contract tests, so one schema has three uses. | `label_events` | Every `labeled` and `unlabeled` event on the timeline: the label, who added or removed it (kind, name and write access), and when | | `reviews` | Who submitted it (`by`: kind, name and whether that actor has write access, as a thread's opening actor), state (`approved`, `changes_requested`, `commented` or `dismissed`, as GitHub holds it now), commit, `content_id`, `submitted_at`; and, from the timeline, `dismissed_by`: who dismissed it, of what kind and with what access, and `dismissed_from`: what it was before (`approved`, `changes_requested` or `commented`). Both are null unless it was dismissed | | `threads` | Thread id, path, line, when it was opened, resolved, outdated, opening actor and resolving actor, each with its kind and whether it has write access (the resolving actor null unless resolved); and the Finding's `category` and `severity`. For a required Lens's thread both come from that Lens's own Finding record, in its Run's `result.json` (§3.10), joined by `surface.key`, never from the comment text, which anyone with write access can edit. A person's thread is `human`, with no severity; an Advisory reviewer's, which no Finding record names, has neither | -| `checks` | Name exactly as GitHub gives it, the id of the App that reported it (`app_id`, the check run's `app.id`), conclusion as GitHub gives it (or the check's status while it has not concluded), completion time, annotation count | +| `checks` | Name exactly as GitHub gives it, the id of the App that reported it (`app_id`, the check run's `app.id`), the workflow file that ran it (`workflow`, the `path` of its check suite's workflow run, such as `.github/workflows/verify.yml`; null for a check no workflow ran), conclusion as GitHub gives it (or the check's status while it has not concluded), completion time, annotation count | | `alerts` | Tool, rule, alert number, state, severity, path | | `queue` | Merge-queue membership | | `acts_since_writer` | Count of acts that landed since the last action by a person with write access, from GitHub's timeline: each approval, enqueue, dequeue and label a Factory identity made with no model Run | @@ -391,12 +391,33 @@ check suite replaces an earlier run: GitHub's `filter=latest` is per check suite runs on one commit both stay, and the Kernel reads both, which fails closed. Both rules wait on `adapters/`. +GitHub Actions is one App, 15368, for every job in the repo, one a pull request adds in a workflow +file of its own under a required Lens's check name among them. So a Lens that Actions runs is also +pinned to its workflow file: the configuration's `workflow` names it, `.github/workflows/verify.yml` +for `verify` and `.github/workflows/ci.yml` for `secret-scan`, the files whose jobs carry those +names, and null for `code-scanning`, which no workflow runs. The port gives each check run the `path` +of its check suite's workflow run as `workflow`, and null for a check no workflow ran (trial P-35). +The Kernel matches the file exactly. A check under a Lens's check name from its own App and another +file, or with no file read where the Lens names one, is read as one from another App is, with the +evidence `:from_workflow:`, the path `unreadable` when none was read; and a check that +names a file, where the Lens names none, is not that Lens's either. `tools/check_workflows.py` holds +the configuration to the files: each file it names exists and runs a job under the Lens's check name. +A change to a Lens's own file is a change under `.github/`, which only the Maintainer authors +(decisions 0004, 0005). The `main` ruleset pins `verify` to its App; pinning each required check is +the Maintainer's, in the repo's settings, with "The Factory repo's rulesets hold the Release tags and each required check's App" +(issue 62), and the Kernel's pin does not wait on it. A Lens no workflow runs has a gap of its own: +whether an upload to code scanning under CodeQL's tool and category, by an identity holding +`security_events: write`, can replace the CodeQL verdict is unverified (trial P-36). Such an upload +would be reported by the Lens's own App under its own check, so the Kernel could not tell it from +the Lens; if it can, the answer is that no identity but CodeQL's own analysis holds that permission. + A check under a required Lens's check name from any other App, or with an App id that is not a whole number, makes that Lens unreadable whatever it concludes, and whatever the Lens's own App says beside it. It is read before the failure of any Lens and before a conflict, as `review_unreadable` is, because a fix Run is an action: reason `check_unreadable`, for the Maintainer to see, since an App posting under a Lens's name is a person's to look into (I-13). Its evidence is -`:from_app:`, so it is not taken for a check that was cancelled. A check run cannot be +`:from_app:`, or `:from_workflow:` for another workflow file of the Lens's own +App, so it is not taken for a check that was cancelled. A check run cannot be deleted, so it stays on that head, and only a push clears it. Every check from the Lens's own App is read, never the last alone: any that fails fails the Lens; failing none, any that gave no verdict leaves it unreadable; then any still running is a wait. The Lens passes only when every one passes. @@ -1000,7 +1021,7 @@ goes stale as usual. State is never stored (decision 0002), so the time an item entered its State is read as the newest time a writer, a Lens or the Factory moved it: its last push, a required check's completion -(`completed_at`) as its Lens's own App reports it, a review or a thread by a person with write access or by a Lens's App (a gating +(`completed_at`) as its Lens's own App and workflow file report it, a review or a thread by a person with write access or by a Lens's App (a gating thread is always one), a writer's action, or a Run on it. A stranger's comment and a check that is not required move nothing, as for attempts (I-28): otherwise anyone could keep a stalled item from showing by commenting. A time is read only when it is a real moment no later than now: the month 1 to 12, a @@ -1029,7 +1050,7 @@ with its own reason: `human_hold`, `outside_not_admitted`, `too_large`, `empty_d `approver_is_author_or_last_pusher`, since GitHub does not count that approval (I-26). A Hold stops every Effect, and a merge is the last, so the dequeue is the one change the Factory makes to a held item (FR-23, I-21). So does a required check that now fails or gives no verdict, or one posted under a -required Lens's name by another App, reason `check_failed` or `check_unreadable` (I-9, I-27), a conflict, and a draft. So does a read that fails: +required Lens's name by another App or workflow file, reason `check_failed` or `check_unreadable` (I-9, I-27), a conflict, and a draft. So does a read that fails: `facts_incomplete` or `review_unreadable`. A dequeue is a stop, not work, so a fact that cannot be read takes the item out rather than leaving it to merge (I-13). @@ -1572,7 +1593,7 @@ A sweep on 2026-10-10 ran the Lifecycle against awkward inputs. Each case below | The approver is the author or the last pusher | GitHub rejects or discards the approval. The Kernel reads `author` and `last_pusher`, attempts no approval, and holds the item with that reason (I-26). The machine user therefore never pushes (§5.1). The Hold is a reading: after a conflict, the Factory's fix Run pushes, the Role's App becomes the last pusher, and the Hold ends with no person, since GitHub would then count the approval (§4.1) | | A branch pushed every few minutes | Deterministic Lenses first; a model review starts only on content they have passed (I-27) | | A required check that concludes `neutral`, `skipped`, `cancelled` or `stale`, or with a word the Kernel does not know | It gave no verdict, so it has not passed. The Kernel defers, reason `check_unreadable`, admits no Run, and shows the item as needing the Maintainer until the check reports again (I-13, §3.3). A `timed_out` check is a failure, and goes back to the author. A failure beside it is read first, so `fail-fast`'s cancelled siblings and a skipped dependent job do not hold an ordinary failure | -| A check under a required Lens's check name from another App, or two from the Lens's own App that disagree | The first leaves the Lens unreadable, reason `check_unreadable` with the evidence `:from_app:`, for the Maintainer to see, whatever any Lens's own check says, a failure among them; it moves nothing either. A check run cannot be deleted, so it stays on that head for good, and only a push clears it. Of the second, every one is read: a failure fails the Lens, and it passes only when every one passes (§3.3) | +| A check under a required Lens's check name from another App or another workflow file, or two from the Lens's own App and file that disagree | The first leaves the Lens unreadable, reason `check_unreadable` with the evidence `:from_app:` or `:from_workflow:`, for the Maintainer to see, whatever any Lens's own check says, a failure among them; it moves nothing either. A check run cannot be deleted, so it stays on that head for good, and only a push clears it. Of the second, every one is read: a failure fails the Lens, and it passes only when every one passes (§3.3) | | A Hold released, or a pull request reopened, after attempts ran out | Attempts count since the last action by a person with write access, which grants a fresh allowance. No agent's action does, and nor does a stranger's comment: on a public repo that would let anyone keep an agent running (I-28) | | An approval that does not land | The approval is a Run with no Engine. A failed one whose fault is the item's is an attempt, and at `max_attempts` the item is held. One that fails for any other fault, such as an expired token, counts toward the Run ceiling (§3.10) | | A merge queue that ejects the pull request every time | Each enqueue is an act. At `max_acts_per_item` acts since a person with write access acted, the item is held, reason `act_ceiling` (§3.3) | @@ -1714,7 +1735,7 @@ room. That shapes every row. | Get a harmless fork commit admitted, then push the real one | Admission lapses on the next push (I-36) | | From a fork, open pull requests that say they close an issue, to park it, hold it or spend its one retry | Only a pull request opened by a Factory App or a person with write access, or one the Maintainer has admitted, is linked to an issue (I-39) | | Get a harmless issue admitted, then edit it; or write an issue that tells the model to call itself ready, perhaps in a comment the rendered page hides | An edit ends acceptance at the next pass (I-36, I-42), and `apply` refuses a Run admitted for other content (I-21, I-40). Admission never changes the class of the text: a Run that reads it has no shell and no network tool (I-35), and the Runs that have them are given the investigating Role's statement instead. What remains is an instruction that survives that restatement. Against it: the readiness Intent is promoted only on an eval set, the model holds no credential that can write, and no implementing Run starts before the Maintainer has approved the contract (I-29). The opening Run can still push a contract and open a draft | -| With any App that can write checks, the Controller's among them, post a passing check under a required Lens's name, or a passing one after that Lens's failure | A required Lens is read only from its own App, by id, and every check it reports there is read: another App's check leaves the Lens unreadable, before any failure, and any failure fails it. The status on the item has one name, which can never be a Lens's or its check's (§3.3). The pin is to an App, so `verify` and `secret-scan` are bound to any GitHub Actions job, one a pull request adds with the same name among them. Reading every check from the Lens's App, all of which must pass, is what makes an extra passing one harmless; replacing the real job needs a change to `.github/`, a protected path that only the Maintainer authors (decisions 0004, 0005). A remaining risk, unverified: an upload to code scanning with CodeQL's tool and category, by anyone with `security_events: write`, might replace the CodeQL verdict. A follow-up story pins each Lens to its workflow file | +| With any App that can write checks, the Controller's among them, post a passing check under a required Lens's name, or a passing one after that Lens's failure | A required Lens is read only from its own App, by id, and every check it reports there is read: another App's check leaves the Lens unreadable, before any failure, and any failure fails it. The status on the item has one name, which can never be a Lens's or its check's (§3.3). A Lens GitHub Actions runs is pinned to its workflow file as well, since Actions is one App for every job: a job a pull request adds in a file of its own under `verify`'s or the secret scan's name leaves the Lens unreadable, and changing the Lens's own file needs a change to `.github/`, a protected path that only the Maintainer authors (decisions 0004, 0005). A remaining risk, unverified: an upload to code scanning with CodeQL's tool and category, by anyone with `security_events: write`, might replace the CodeQL verdict (P-36) | | Steal what the relay holds | It holds a webhook secret and nothing that can act on the Factory repo. The worst a thief can do is cause passes (§7.3) | | Forge a fault report from inside a Run, to pause the Factory | A pause needs agreeing faults from Runs on different items, and `apply` and the poller class the faults they see themselves. The worst case is delay, bounded by the probe Run | | Replace the commit bundle from inside `verify` | The bundle is taken by identifier and digest (§3.6) | @@ -1905,6 +1926,8 @@ it observed as a recorded fixture, which is what the simulator's fake GitHub is | P-22 | How soon after it is started does a Run appear in the list of run records? | Soon enough that a pass never admits the same Run twice; the item's lane and I-21 cover the gap | | P-32 | Can an issue's author's permission be read in the listing of open issues, and who added and who removed each label, with the time, in the full read? | Yes to both, within the budget P-8 measures | | P-34 | Can the time an issue's title or body last changed be read, apart from the time anything on it changed; and each reopening, with who did it? | Yes to both | +| P-35 | Can the workflow file of each check run on a head be read, as the `path` of its check suite's workflow run, with the permissions the reader and the Controller hold, in the one read of the facts; and does a job a pull request adds in a file of its own, under a required Lens's check name, report under GitHub Actions with that file's path? | Yes to both, within the budget P-8 measures | +| P-36 | Can an upload to code scanning under CodeQL's tool and category, on a pull request's head, by an identity holding `security_events: write` but not CodeQL's own analysis, change the conclusion of the check run `CodeQL` that GitHub Advanced Security reports? | It cannot. If it can, no identity but CodeQL's own analysis holds `security_events: write`, and the permission tables of the Apps and the workflows are checked to say so | | P-33 | Can the issue a pull request closes be read from a draft, an open, a merged and a closed-unmerged pull request alike, with its opening actor; and does a pull request to a branch that is not the default one carry it? Can who last changed the draft flag be read? | Yes to the first and last. If a closed pull request loses the reference, closes are counted by the work's branch name. If a non-default base carries none, the issue Lifecycle serves the default branch only. If who changed the draft flag cannot be read, a linked pull request that has once left draft and is a draft again is treated as taken in hand | ### 6.1 The order to run them in @@ -1914,7 +1937,7 @@ An assumption audit on 2026-10-10 ranked each trial by what breaks if its answer | Batch | Trials | If the answer is no | |---|---|---| | **1. Before any code** | P-13 (the model CLI returns structured output on a subscription); P-1, P-7, P-24 (the approval mechanics); P-17, P-22 (run records can be identified); P-3 (secrets are isolated by environment); P-28, P-29 (last pusher and content identity are readable); P-8 (a pass fits its budget) | The design does not work as written. Each of these is a foundation: no Run, no approval, no derived State, no credential boundary, or no poller | -| **2. Before the first approval** | P-9, P-20 (chaining and the outside tick); P-5, P-23, P-25 (resolving others' threads); P-2 (an App in the merge queue); P-30 (cutting a Release); P-27, P-14, P-4 (forks, caches, narrowed tokens); P-26 (the sandbox) | One mechanism is replaced and the design stands. If threads cannot be resolved by an App, required thread resolution is dropped in repos the Factory serves | +| **2. Before the first approval** | P-9, P-20 (chaining and the outside tick); P-5, P-23, P-25 (resolving others' threads); P-2 (an App in the merge queue); P-30 (cutting a Release); P-27, P-14, P-4 (forks, caches, narrowed tokens); P-26 (the sandbox); P-35, P-36 (what a Lens's check is read from) | One mechanism is replaced and the design stands. If threads cannot be resolved by an App, required thread resolution is dropped in repos the Factory serves | | **3. Before the control room** | P-11, P-19, P-21 (publishing the page); P-16, P-18 (retention and final steps) | The page is published another way, or history is shorter | | **4. As they come up** | P-6, P-10, P-12, P-15 | A detail changes | diff --git a/docs/decisions/0005-each-role-writes-through-its-own-app.md b/docs/decisions/0005-each-role-writes-through-its-own-app.md index 009d23be..22cd2a7b 100644 --- a/docs/decisions/0005-each-role-writes-through-its-own-app.md +++ b/docs/decisions/0005-each-role-writes-through-its-own-app.md @@ -36,7 +36,7 @@ GitHub does not count an approval from an author or a last pusher, so the approv | Until a Consumer repo is served (slice 3), each App is installed on named repos only; from then, as decided at that cutover, against UJ-6 | `tools/check_apps.py` | | Every permitted Effect of every Intent is within the ceiling of that Role's App | waiting on `roles/` | | The Controller's `checks: write` cannot pass a Lens: a required Lens is read only from its own App, and the status on the item has one name that is no Lens's or its check's | `tests/test_decide.py::test_a_check_from_another_app_is_unreadable_for_its_lens`, `tests/test_tools.py::test_the_status_on_the_item_never_carries_a_required_lens_name` | -| A Lens pinned to GitHub Actions is bound to any Actions job of its check's name, one a pull request adds among them: every check from the Lens's App is read and all must pass, so an extra passing one is harmless, and replacing the real job is a change to `.github/`, which only the Maintainer authors (0004). An upload to code scanning with CodeQL's tool and category, by anyone with `security_events: write`, might replace the CodeQL verdict; a follow-up story pins each Lens to its workflow file | `tests/test_decide.py::test_every_check_a_lens_reports_from_its_own_app_is_read`; the upload unverified | +| A Lens pinned to GitHub Actions is bound to any Actions job of its check's name, one a pull request adds among them: every check from the Lens's App is read and all must pass, so an extra passing one is harmless, and replacing the real job is a change to `.github/`, which only the Maintainer authors (0004). An upload to code scanning with CodeQL's tool and category, by anyone with `security_events: write`, might replace the CodeQL verdict; a follow-up story pins each Lens to its workflow file | `tests/test_decide.py::test_every_check_a_lens_reports_from_its_own_app_is_read`, `tests/test_decide.py::test_a_check_from_another_workflow_is_unreadable_for_its_lens`; the upload unverified, `sim/grounding.py` | | Every write by a Factory identity matches a recorded Effect; one that does not raises an alert | waiting on `controller/` | | No Factory secret is readable by a workflow that runs on a Work item's branch | waiting on `harness/` | | The code owners file is the org's, word for word | `tests/test_tools.py::test_code_owners_are_the_orgs_word_for_word` | diff --git a/docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md b/docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md index 9646f9f8..8a11ff73 100644 --- a/docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md +++ b/docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md @@ -1,10 +1,7 @@ # 0015. A Lens is read from its own App, and only what gates holds a merge -Status: accepted. Replaces `docs/decisions/0012-lenses-report-findings-and-the-kernel-decides-convergence.md`, -and keeps all of it. It adds the Maintainer's ruling of 2026-10-14 on a Finding that does not gate -("Kernel: what happens to a Finding that does not gate", issue 55), and the ruling on -"The Apps' permissions as data, before any is registered" (issue 20) that a Lens is read only from -the App that reports it ("Kernel: a Lens is read only from the App that reports it", issue 60). +Status: replaced by `docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md`, +which holds the rule in force and its boundaries. ## Decision diff --git a/docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md b/docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md new file mode 100644 index 00000000..62520d4b --- /dev/null +++ b/docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md @@ -0,0 +1,111 @@ +# 0017. A Lens is read from its own App and its own workflow file, and only what gates holds a merge + +Status: accepted. Replaces `docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md`, +and keeps all of it: the Maintainer's ruling of 2026-10-14 on a Finding that does not gate +("Kernel: what happens to a Finding that does not gate", issue 55), and the ruling on +"The Apps' permissions as data, before any is registered" (issue 20) that a Lens is read only from +the App that reports it ("Kernel: a Lens is read only from the App that reports it", issue 60). It +adds the ruling on "A Lens run by GitHub Actions is read only from its own workflow file" (issue 97): +a Lens GitHub Actions runs is read only from the workflow file the configuration names for it. + +## Decision + +Every source of judgement on a Work item, whether a Role, a third-party reviewer, a tool or a +person, is a Lens that reports Findings in one schema. Convergence is a Kernel function: every +required Lens has passed on the current content, and no gating Finding is open. Lenses do not +address each other. A Finding from a Lens outside the required set gates only when a required Lens +adopts it. A required model Lens's Finding gates when it is `blocking` or `major`, and a `security` +one at any severity. One that does not gate, a `minor` or `info` Finding of any category but +`security`, is an annotation on its Lens's check run and has no Disposition: it closes when its +Lens stops reporting it, as a `mechanical` Finding does. A required deterministic Lens gates through +its check's conclusion. It is read only from the App the configuration names for it, by the exact +name of its check run, and, where GitHub Actions runs it, only from the workflow file the +configuration names: a check under its name from any other App, from another workflow file, or with +no file read, leaves it unreadable, for the Maintainer, before any Lens's failure. A Lens no workflow +runs names none, and is read only from a check that names none. Every check its own App and file +report under that name is read, and it passes only when every one passes. A reviewing Role +declares a rubric, which is a floor. A change that has not converged within its bound is placed on +Hold. + +## Why + +A council of independent Lenses finds more than one reviewer, but only if they stay independent +and something other than their conversation decides. The proof of concept decided by parsing +comments, and waited for reviewers that never answered. + +A Disposition for every small Finding would cost Runs and bring back the gate built on every +comment, and closing one as deferred at the merge cannot work: the ruleset requires resolved +threads, so an open one would hold a merge the Kernel approved. An annotation's report still gives +each Finding one outcome, which is counted. Severity is never how a security defect gets merged. + +Any App that can write checks can post one under any name, the Controller's among them, and the +ruleset pins only `verify` to its App. Read by name alone, a passing copy could pass a Lens that +failed, or the last of several checks could hide a failure. Once a dependency update is judged by +the deterministic Lenses alone, they are its only judge. + +GitHub Actions is one App for every job in the repo, so a pin to the App alone binds `verify` and the +secret scan to any Actions job under their check's name, one a pull request adds in a workflow file +of its own among them. The workflow file is the narrowest thing GitHub records of what ran a check, +and only the Maintainer authors a change under `.github/` (decisions 0004, 0005), so a job in another +file is not the Lens, and a change to the Lens's own file is the Maintainer's. Pinning each required +check to its App in the `main` ruleset, not only `verify`, is the Maintainer's, in the repo's settings, +with "The Factory repo's rulesets hold the Release tags and each required check's App" (issue 62); the +Kernel's pin does not wait on it. Whether an upload to code scanning under CodeQL's tool and category +can replace the CodeQL verdict is a platform fact not yet tried: the fake GitHub assumes it cannot. + +## Boundaries + +| Boundary | Check | +|---|---| +| A Finding is a structured record | `schemas/finding.schema.json`, `tools/check_examples.py` | +| A reason is a code, never a sentence | `examples/invalid/decision.sentence-as-reason.json` | +| A required Lens's blocking or major Finding gates; a minor one of any category but `security`, or one from a Lens that is not required, does not | `tests/test_decide.py::test_required_lens_major_finding_gates`, `tests/test_decide.py::test_minor_finding_and_unrequired_lens_do_not_gate` | +| A required Lens's `security` Finding gates at any severity, in the merge queue too; one from a Lens that is not required gates only once a required Lens adopts it | `tests/test_decide.py::test_a_minor_security_finding_gates`, `tests/test_decide.py::test_a_minor_security_finding_opened_while_queued_takes_it_out_of_the_queue`, `tests/test_decide.py::test_a_security_finding_from_a_lens_that_is_not_required_does_not_gate` | +| A required Lens's open thread whose category or severity is missing, null or not in the schema defers for the Maintainer, and takes a queued item out; a thread from a Lens that is not required may carry neither | `tests/test_decide.py::test_a_required_lens_thread_that_cannot_be_read_defers`, `tests/test_decide.py::test_a_read_that_fails_takes_a_pull_request_out_of_the_queue` | +| A thread's category and severity are read from the required Lens's own Finding record, joined by its surface key, never from the comment text | waiting on `adapters/` | +| A thread carries its Finding's category, in the authority table's terms, and the Kernel reads the Finding's categories and severities | `tests/test_tools.py::test_the_kernel_reads_the_categories_and_severities_a_finding_carries`, `examples/invalid/facts.a-thread-without-its-category.json`, `examples/invalid/facts.a-category-nobody-owns.json`, `tests/test_tools.py::test_a_thread_carries_the_categories_a_finding_does` | +| A Finding that does not gate is an annotation on its Lens's check run; a Result that posts one on a thread is refused | waiting on `harness/` | +| A required deterministic Lens names the check GitHub reports for it, and that check reports on every head and in the merge queue | waiting on `adapters/` | +| No model Lens runs on content the deterministic Lenses have not passed | `tests/test_decide.py::test_no_model_review_before_deterministic_checks_pass` | +| Only `success` passes a required check. `failure` and `timed_out` go back to the author, and are read first; queued, running, waiting and absent checks are waited for; a check that gave no verdict, or whose conclusion the map does not name, defers and needs the Maintainer | `tests/test_decide.py::test_a_neutral_required_check_defers`, `tests/test_decide.py::test_a_timed_out_required_check_goes_back_to_the_author`, `tests/test_decide.py::test_a_queued_running_or_absent_required_check_is_waited_for`, `tests/test_decide.py::test_a_failed_check_is_read_before_an_unreadable_or_waiting_one`, `tests/test_decide.py::test_a_failure_with_cancelled_or_skipped_siblings_goes_back_to_the_author` | +| The configuration cannot make a check with no verdict pass | `tests/test_tools.py::test_only_success_can_pass_a_check`, `examples/invalid/config.a-neutral-check-passes.json`, `examples/invalid/config.conclusion-mapped-to-nothing.json` | +| A required deterministic Lens is read only from the App the configuration names for it, by an id that is a whole number: a check with its name from another App, or with no such id, leaves it unreadable, before any Lens's failure or a conflict, on a queued item too, says which App, and moves nothing | `tests/test_decide.py::test_a_check_from_another_app_is_unreadable_for_its_lens`, `tests/test_decide.py::test_a_check_from_another_app_or_a_failure_beside_a_pass_takes_it_out_of_the_queue`, `tests/test_decide.py::test_only_a_check_from_its_lens_app_moves_an_item`, `examples/invalid/facts.a-check-without-its-app.json`, `examples/invalid/config.a-required-lens-with-no-app.json` | +| Every check a Lens's own App and workflow file report under its name is read, never the last alone: any failure fails it, and it passes only when every one passes | `tests/test_decide.py::test_every_check_a_lens_reports_from_its_own_app_is_read` | +| A required deterministic Lens that GitHub Actions runs is read only from the workflow file the configuration names for it, exactly: a check with its name from another file of its App, or with no file read, leaves it unreadable, before any Lens's failure or a conflict, on a queued item too, says which file, and moves nothing. One from another App is shown as another App's, whatever file it names | `tests/test_decide.py::test_a_check_from_another_workflow_is_unreadable_for_its_lens`, `tests/test_decide.py::test_a_check_from_another_workflow_takes_it_out_of_the_queue`, `tests/test_decide.py::test_only_a_check_from_its_lens_workflow_moves_an_item`, `examples/invalid/facts.a-check-without-its-workflow.json` | +| A Lens no workflow runs names none, and is read only from a check that names none | `tests/test_decide.py::test_a_check_from_another_workflow_is_unreadable_for_its_lens`, `tests/test_decide.py::test_only_a_check_from_its_lens_workflow_moves_an_item` | +| Each deterministic Lens names its workflow file, or null where no workflow runs it, and the file it names exists and runs a job under the Lens's check name | `examples/invalid/config.a-required-lens-with-no-workflow.json`, `tests/test_tools.py::test_a_lens_read_from_a_workflow_that_does_not_run_its_check_is_caught` | +| The port gives each check run GitHub Actions reports with the path of its check suite's workflow run, and null only for a check no workflow ran | waiting on `adapters/` | +| A Role's change to a Lens's own workflow file, a change under `.github/`, is refused unless a person approved it | waiting on `harness/` | +| Whether an upload to code scanning under CodeQL's tool and category, by an identity holding `security_events: write`, can replace the CodeQL verdict is a platform fact, unverified until trial P-36 grounds it | `sim/grounding.py`, `tools/check_trials.py` | +| Each deterministic Lens names its check run's exact name and its App, and no Lens or check is named twice | `examples/invalid/config.a-required-lens-with-no-check.json`, `tests/test_tools.py::test_a_lens_or_a_check_named_twice_is_refused` | +| The status on the item has one name, and it is no required Lens's or its check's, in any case | `tests/test_tools.py::test_the_status_on_the_item_never_carries_a_required_lens_name` | +| The port gives every check run with its exact name and the real id of its App, from every App, never dropped, merged or renamed | waiting on `adapters/` | +| A check is a Lens's only under the exact name of the Lens's check: a job its own App runs under the Lens's name, or under the check's name in another case, passes nothing | `tests/test_decide.py::test_a_check_is_matched_to_a_lens_by_its_check_name_alone` | +| Only a re-run of the same job in the same check suite replaces an earlier check run; two workflow runs on one commit both stay, and both are read | waiting on `adapters/` | +| A dependency update is judged by the deterministic Lenses alone, and needs no model review | `tests/test_decide.py::test_a_dependency_update_needs_no_model_review` | +| A Finding a model Lens has raised gates whatever the class, a dependency update's too, and after the bot recreates the branch | `tests/test_decide.py::test_a_model_lens_thread_gates_a_dependency_update` | +| A dependency update is a dependency bot's pull request whose head that bot pushed; anyone else's push, and every other class, needs the model review | `tests/test_decide.py::test_a_dependency_update_someone_else_pushed_to_needs_a_model_review`, `tests/test_decide.py::test_every_other_class_needs_a_model_review`, `examples/invalid/config.a-change-by-a-person-skips-the-model-review.json` | +| A dependency update skips the model review and nothing else: every deterministic Lens, a person's thread, a Hold and a Pause still bind, and a `neutral` check does not pass it | `tests/test_decide.py::test_a_dependency_update_is_judged_by_every_deterministic_lens`, `tests/test_decide.py::test_a_neutral_check_does_not_pass_a_dependency_update`, `tests/test_decide.py::test_a_writers_thread_still_gates_a_dependency_update`, `tests/test_decide.py::test_a_dependency_update_skips_the_model_review_and_nothing_else` | +| The configuration names at least one deterministic Lens, so no update is approved with no check at all | `examples/invalid/config.no-deterministic-lens.json` | +| `last_pusher` is the identity GitHub records as having pushed the head, never a commit's author or committer; class `dependency` is given only for an exact App identity, and a fork is always `outside` | waiting on `adapters/` | +| An empty diff never converges | `tests/test_decide.py::test_empty_diff_never_converges` | +| Rounds between author and owner are bounded, and the bound ends in a Hold | `tests/test_decide.py::test_rounds_are_bounded` | +| Every category of Finding has exactly one owner | `tools/check_authority.py` | +| A rubric is data with a schema | `schemas/rubric.schema.json`, `tools/check_examples.py` | +| A reviewing Result reports on every criterion that applies | waiting on `harness/` | +| With an acceptance contract, every criterion has passing evidence on the current content | `tests/test_decide.py::test_with_a_contract_it_converges_only_when_every_criterion_has_passing_evidence` | +| Evidence given on other content does not count, passing or failing | `tests/test_decide.py::test_evidence_for_other_content_does_not_count` | +| A failing result for a criterion on this content goes back to the author, as a failed check does, and is read before a criterion still waiting for its evidence | `tests/test_decide.py::test_a_failing_criterion_goes_back_to_the_author` | +| An item with no acceptance contract converges as it did before contracts were read | `tests/test_decide.py::test_an_item_with_no_contract_converges_as_it_did` | +| A contract, or a result on this content, that cannot be read, or facts that say nothing of a contract, never converge: the Kernel defers for the Maintainer. A criterion's id and a result's content id are read only in the schema's form | `tests/test_decide.py::test_unreadable_acceptance_never_converges`, `examples/invalid/facts.no-word-on-acceptance.json`, `examples/invalid/facts.an-acceptance-result-in-words.json`, `examples/invalid/facts.an-acceptance-result-without-its-content.json`, `examples/invalid/facts.an-acceptance-contract-with-no-criteria.json`, `examples/invalid/facts.an-acceptance-criterion-named-twice.json` | +| Only this content's results are judged: one on earlier content, for a criterion the contract no longer lists or in any word, makes nothing unreadable | `tests/test_decide.py::test_only_this_contents_results_are_judged` | +| Whether the contract, or that there is none, was read in full is `complete.acceptance`: a read short defers as incomplete facts, and no contract converges only on a full read | `tests/test_decide.py::test_a_contract_read_short_is_incomplete`, `examples/invalid/facts.no-completeness-for-acceptance.json` | +| The facts carry a contract's criteria by id, never their statements | `examples/invalid/facts.acceptance-carries-prose.json` | +| The contract is read after the other three conditions of Convergence and before the approval | `tests/test_decide.py::test_acceptance_is_read_after_every_other_condition_of_convergence` | +| A queued item whose evidence is gone, is for other content, fails or cannot be read leaves the merge queue | `tests/test_decide.py::test_a_queued_item_whose_evidence_goes_missing_or_unreadable_leaves_the_queue`, `tests/test_simulator.py::test_a_queued_item_whose_evidence_is_gone_is_taken_out_before_it_merges` | +| A wait for evidence is a wait in `verifying`, and goes stale past its limit | `tests/test_decide.py::test_a_wait_for_acceptance_evidence_goes_stale` | +| The port reads the contract at the head, and each result for a criterion with the content it ran on, from the check that gave it | waiting on `adapters/` | +| The port reports the latest result per criterion per check suite | waiting on `adapters/` | +| A result counts only from the App the configuration allows for its criterion's kind; never from the author's Role. Issue 108, "Kernel: acceptance evidence from the right reporter, against the approved contract" | waiting on `adapters/` | +| The contract read is the one the Maintainer approved, unchanged since; no fix Run on a contract not yet approved (I-29), in the same story | waiting on `harness/` | +| No manifest names another Role's output as a trigger | waiting on `roles/` | diff --git a/examples/facts.json b/examples/facts.json index 2bce6050..225bb3a6 100644 --- a/examples/facts.json +++ b/examples/facts.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/config.a-change-by-a-person-skips-the-model-review.json b/examples/invalid/config.a-change-by-a-person-skips-the-model-review.json index 2806e9a9..9bda4f38 100644 --- a/examples/invalid/config.a-change-by-a-person-skips-the-model-review.json +++ b/examples/invalid/config.a-change-by-a-person-skips-the-model-review.json @@ -7,17 +7,20 @@ { "lens": "verify", "check": "verify", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/verify.yml" }, { "lens": "secret-scan", "check": "Secret scan (gitleaks)", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/ci.yml" }, { "lens": "code-scanning", "check": "CodeQL", - "app_id": 57789 + "app_id": 57789, + "workflow": null } ], "model": [ diff --git a/examples/invalid/config.a-neutral-check-passes.json b/examples/invalid/config.a-neutral-check-passes.json index 641ed3f5..099be17a 100644 --- a/examples/invalid/config.a-neutral-check-passes.json +++ b/examples/invalid/config.a-neutral-check-passes.json @@ -7,17 +7,20 @@ { "lens": "verify", "check": "verify", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/verify.yml" }, { "lens": "secret-scan", "check": "Secret scan (gitleaks)", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/ci.yml" }, { "lens": "code-scanning", "check": "CodeQL", - "app_id": 57789 + "app_id": 57789, + "workflow": null } ], "model": [ diff --git a/examples/invalid/config.a-required-lens-with-no-app.json b/examples/invalid/config.a-required-lens-with-no-app.json index 398588b4..4e4f12c2 100644 --- a/examples/invalid/config.a-required-lens-with-no-app.json +++ b/examples/invalid/config.a-required-lens-with-no-app.json @@ -7,16 +7,19 @@ { "lens": "verify", "check": "verify", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/verify.yml" }, { "lens": "secret-scan", "check": "Secret scan (gitleaks)", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/ci.yml" }, { "lens": "code-scanning", - "check": "CodeQL" + "check": "CodeQL", + "workflow": null } ], "model": [ diff --git a/examples/invalid/config.a-required-lens-with-no-check.json b/examples/invalid/config.a-required-lens-with-no-check.json index 576fb598..02aaaa1e 100644 --- a/examples/invalid/config.a-required-lens-with-no-check.json +++ b/examples/invalid/config.a-required-lens-with-no-check.json @@ -7,16 +7,19 @@ { "lens": "verify", "check": "verify", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/verify.yml" }, { "lens": "secret-scan", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/ci.yml" }, { "lens": "code-scanning", "check": "CodeQL", - "app_id": 57789 + "app_id": 57789, + "workflow": null } ], "model": [ diff --git a/examples/invalid/config.a-required-lens-with-no-workflow.json b/examples/invalid/config.a-required-lens-with-no-workflow.json new file mode 100644 index 00000000..5b89cf7b --- /dev/null +++ b/examples/invalid/config.a-required-lens-with-no-workflow.json @@ -0,0 +1,60 @@ +{ + "approver": "donpetry-bot", + "author_role": "dev-lead", + "owner_role": "pr-review", + "required_lenses": { + "deterministic": [ + { + "lens": "verify", + "check": "verify", + "app_id": 15368 + }, + { + "lens": "secret-scan", + "check": "Secret scan (gitleaks)", + "app_id": 15368, + "workflow": ".github/workflows/ci.yml" + }, + { + "lens": "code-scanning", + "check": "CodeQL", + "app_id": 57789, + "workflow": null + } + ], + "model": [ + "pr-review" + ], + "model_not_required_for": [ + "dependency" + ] + }, + "status_on_item": "factory", + "check_conclusions": { + "success": "passing", + "failure": "failing", + "timed_out": "failing", + "queued": "pending", + "in_progress": "pending", + "pending": "pending", + "waiting": "pending", + "requested": "pending", + "expected": "pending", + "neutral": "unreadable", + "skipped": "unreadable", + "cancelled": "unreadable", + "stale": "unreadable", + "action_required": "unreadable", + "startup_failure": "unreadable", + "error": "unreadable" + }, + "limits": { + "max_attempts": 3, + "max_rounds": 3, + "max_runs_per_item": 10, + "max_unmerged_closes": 1, + "org_max_active_runs": 4, + "max_diff_lines": 4000, + "max_acts_per_item": 6 + } +} diff --git a/examples/invalid/config.a-staleness-limit-copied-from-the-lifecycle.json b/examples/invalid/config.a-staleness-limit-copied-from-the-lifecycle.json index a943fbfc..ecdbf49c 100644 --- a/examples/invalid/config.a-staleness-limit-copied-from-the-lifecycle.json +++ b/examples/invalid/config.a-staleness-limit-copied-from-the-lifecycle.json @@ -7,17 +7,20 @@ { "lens": "verify", "check": "verify", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/verify.yml" }, { "lens": "secret-scan", "check": "Secret scan (gitleaks)", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/ci.yml" }, { "lens": "code-scanning", "check": "CodeQL", - "app_id": 57789 + "app_id": 57789, + "workflow": null } ], "model": [ diff --git a/examples/invalid/config.conclusion-mapped-to-nothing.json b/examples/invalid/config.conclusion-mapped-to-nothing.json index e1e2c5a7..3e1e6dbb 100644 --- a/examples/invalid/config.conclusion-mapped-to-nothing.json +++ b/examples/invalid/config.conclusion-mapped-to-nothing.json @@ -7,17 +7,20 @@ { "lens": "verify", "check": "verify", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/verify.yml" }, { "lens": "secret-scan", "check": "Secret scan (gitleaks)", - "app_id": 15368 + "app_id": 15368, + "workflow": ".github/workflows/ci.yml" }, { "lens": "code-scanning", "check": "CodeQL", - "app_id": 57789 + "app_id": 57789, + "workflow": null } ], "model": [ diff --git a/examples/invalid/facts.a-category-nobody-owns.json b/examples/invalid/facts.a-category-nobody-owns.json index 3a13d32a..d08e90f4 100644 --- a/examples/invalid/facts.a-category-nobody-owns.json +++ b/examples/invalid/facts.a-category-nobody-owns.json @@ -85,18 +85,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.a-check-without-its-app.json b/examples/invalid/facts.a-check-without-its-app.json index 9e269fba..b73461ec 100644 --- a/examples/invalid/facts.a-check-without-its-app.json +++ b/examples/invalid/facts.a-check-without-its-app.json @@ -85,17 +85,20 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.a-check-without-its-completion-time.json b/examples/invalid/facts.a-check-without-its-completion-time.json index fbacb0da..0650e72e 100644 --- a/examples/invalid/facts.a-check-without-its-completion-time.json +++ b/examples/invalid/facts.a-check-without-its-completion-time.json @@ -85,17 +85,20 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.a-check-without-its-workflow.json b/examples/invalid/facts.a-check-without-its-workflow.json new file mode 100644 index 00000000..81b96ee3 --- /dev/null +++ b/examples/invalid/facts.a-check-without-its-workflow.json @@ -0,0 +1,179 @@ +{ + "schema": "facts/1", + "item": { + "repo": "combsmith/software-factory", + "type": "pull_request", + "number": 42 + }, + "state": "open", + "is_draft": false, + "class": "maintainer", + "author": "don-petry", + "last_pusher": "don-petry", + "targets_protected": true, + "mergeable": true, + "content_id": "sha256:5d0c", + "diff": { + "files": 3, + "lines": 120, + "truncated": false + }, + "label_events": [ + { + "label": "factory:hold", + "action": "labeled", + "by": { + "kind": "human", + "name": "don-petry", + "can_write": true + }, + "at": "2026-10-11T11:40:00Z" + }, + { + "label": "factory:hold", + "action": "unlabeled", + "by": { + "kind": "human", + "name": "don-petry", + "can_write": true + }, + "at": "2026-10-11T11:45:00Z" + } + ], + "last_push_at": "2026-10-11T12:00:00Z", + "last_writer_action_at": "2026-10-11T12:00:00Z", + "reviews": [ + { + "by": { + "kind": "machine_user", + "name": "donpetry-bot", + "can_write": true + }, + "state": "dismissed", + "commit": "4b1a9c0", + "content_id": "sha256:4b1a", + "submitted_at": "2026-10-11T11:50:00Z", + "dismissed_by": { + "kind": "human", + "name": "don-petry", + "can_write": true + }, + "dismissed_from": "approved" + }, + { + "by": { + "kind": "human", + "name": "a-colleague", + "can_write": true + }, + "state": "changes_requested", + "commit": "5d0c7e2", + "content_id": "sha256:5d0c", + "submitted_at": "2026-10-11T12:18:00Z", + "dismissed_by": null, + "dismissed_from": null + } + ], + "threads": [ + { + "id": "PRRT_kw7", + "resolved": false, + "opened_by": { + "kind": "lens", + "name": "pr-review", + "can_write": true + }, + "category": "functional", + "severity": "major", + "resolved_by": null, + "opened_at": "2026-10-11T12:15:00Z" + }, + { + "id": "PRRT_kw1", + "resolved": false, + "opened_by": { + "kind": "lens", + "name": "coderabbit", + "can_write": false + }, + "category": null, + "severity": null, + "resolved_by": null, + "opened_at": "2026-10-11T12:15:00Z" + }, + { + "id": "PRRT_kw3", + "resolved": true, + "opened_by": { + "kind": "human", + "name": "a-colleague", + "can_write": true + }, + "category": "human", + "severity": null, + "resolved_by": { + "kind": "human", + "name": "a-colleague", + "can_write": true + }, + "opened_at": "2026-10-11T12:16:00Z" + } + ], + "checks": [ + { + "name": "verify", + "app_id": 15368, + "conclusion": "success", + "completed_at": "2026-10-11T12:06:00Z" + }, + { + "name": "Secret scan (gitleaks)", + "app_id": 15368, + "workflow": ".github/workflows/ci.yml", + "conclusion": "success", + "completed_at": "2026-10-11T12:06:00Z" + }, + { + "name": "CodeQL", + "app_id": 57789, + "workflow": null, + "conclusion": "success", + "completed_at": "2026-10-11T12:06:00Z" + } + ], + "queue": false, + "acts_since_writer": 2, + "acceptance": { + "criteria": [ + "AC-1", + "AC-2" + ], + "results": [ + { + "criterion": "AC-1", + "content_id": "sha256:4b1a", + "result": "failing" + }, + { + "criterion": "AC-1", + "content_id": "sha256:5d0c", + "result": "passing" + }, + { + "criterion": "AC-2", + "content_id": "sha256:5d0c", + "result": "passing" + } + ] + }, + "complete": { + "acceptance": true, + "reviews": true, + "threads": true, + "checks": true, + "diff": true, + "runs": true, + "timeline": true + }, + "read_at": "2026-10-11T12:20:00Z" +} diff --git a/examples/invalid/facts.a-conclusion-in-words.json b/examples/invalid/facts.a-conclusion-in-words.json index 91d51488..655541ba 100644 --- a/examples/invalid/facts.a-conclusion-in-words.json +++ b/examples/invalid/facts.a-conclusion-in-words.json @@ -54,18 +54,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "Completed with warnings", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.a-label-event-without-who-made-it.json b/examples/invalid/facts.a-label-event-without-who-made-it.json index 4b44887f..dcb17683 100644 --- a/examples/invalid/facts.a-label-event-without-who-made-it.json +++ b/examples/invalid/facts.a-label-event-without-who-made-it.json @@ -118,18 +118,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.a-thread-resolved-by-a-name-alone.json b/examples/invalid/facts.a-thread-resolved-by-a-name-alone.json index a95b0dac..99929ae6 100644 --- a/examples/invalid/facts.a-thread-resolved-by-a-name-alone.json +++ b/examples/invalid/facts.a-thread-resolved-by-a-name-alone.json @@ -119,18 +119,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.a-thread-without-its-category.json b/examples/invalid/facts.a-thread-without-its-category.json index 1da214e6..f031c89b 100644 --- a/examples/invalid/facts.a-thread-without-its-category.json +++ b/examples/invalid/facts.a-thread-without-its-category.json @@ -84,18 +84,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.a-time-with-an-impossible-day.json b/examples/invalid/facts.a-time-with-an-impossible-day.json index 7113aa9e..6c084ce7 100644 --- a/examples/invalid/facts.a-time-with-an-impossible-day.json +++ b/examples/invalid/facts.a-time-with-an-impossible-day.json @@ -85,18 +85,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.a-time-with-an-impossible-month.json b/examples/invalid/facts.a-time-with-an-impossible-month.json index 66e1ed66..d0a457ab 100644 --- a/examples/invalid/facts.a-time-with-an-impossible-month.json +++ b/examples/invalid/facts.a-time-with-an-impossible-month.json @@ -85,18 +85,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.acceptance-carries-prose.json b/examples/invalid/facts.acceptance-carries-prose.json index c46905ff..f5d9c0e4 100644 --- a/examples/invalid/facts.acceptance-carries-prose.json +++ b/examples/invalid/facts.acceptance-carries-prose.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.an-acceptance-contract-with-no-criteria.json b/examples/invalid/facts.an-acceptance-contract-with-no-criteria.json index a6b16bc1..9341fc9b 100644 --- a/examples/invalid/facts.an-acceptance-contract-with-no-criteria.json +++ b/examples/invalid/facts.an-acceptance-contract-with-no-criteria.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.an-acceptance-criterion-named-twice.json b/examples/invalid/facts.an-acceptance-criterion-named-twice.json index 31e96e8a..de5e1dde 100644 --- a/examples/invalid/facts.an-acceptance-criterion-named-twice.json +++ b/examples/invalid/facts.an-acceptance-criterion-named-twice.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.an-acceptance-result-in-words.json b/examples/invalid/facts.an-acceptance-result-in-words.json index e99c09e7..db5f5a26 100644 --- a/examples/invalid/facts.an-acceptance-result-in-words.json +++ b/examples/invalid/facts.an-acceptance-result-in-words.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.an-acceptance-result-without-its-content.json b/examples/invalid/facts.an-acceptance-result-without-its-content.json index 78e14064..e9eff50d 100644 --- a/examples/invalid/facts.an-acceptance-result-without-its-content.json +++ b/examples/invalid/facts.an-acceptance-result-without-its-content.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.an-actor-of-a-kind-the-kernel-does-not-know.json b/examples/invalid/facts.an-actor-of-a-kind-the-kernel-does-not-know.json index def74d78..e4fe0574 100644 --- a/examples/invalid/facts.an-actor-of-a-kind-the-kernel-does-not-know.json +++ b/examples/invalid/facts.an-actor-of-a-kind-the-kernel-does-not-know.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.an-actor-without-its-kind.json b/examples/invalid/facts.an-actor-without-its-kind.json index 300cbbb0..ec25d65f 100644 --- a/examples/invalid/facts.an-actor-without-its-kind.json +++ b/examples/invalid/facts.an-actor-without-its-kind.json @@ -122,18 +122,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.carries-prose.json b/examples/invalid/facts.carries-prose.json index 44a30bc0..1037728c 100644 --- a/examples/invalid/facts.carries-prose.json +++ b/examples/invalid/facts.carries-prose.json @@ -55,18 +55,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.dismissed-review-without-its-earlier-state.json b/examples/invalid/facts.dismissed-review-without-its-earlier-state.json index 39e5cca7..0fcefb92 100644 --- a/examples/invalid/facts.dismissed-review-without-its-earlier-state.json +++ b/examples/invalid/facts.dismissed-review-without-its-earlier-state.json @@ -71,18 +71,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.no-completeness-for-acceptance.json b/examples/invalid/facts.no-completeness-for-acceptance.json index 9d34cc2b..338c1e96 100644 --- a/examples/invalid/facts.no-completeness-for-acceptance.json +++ b/examples/invalid/facts.no-completeness-for-acceptance.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.no-completeness-for-runs.json b/examples/invalid/facts.no-completeness-for-runs.json index 6c30c346..4696c88a 100644 --- a/examples/invalid/facts.no-completeness-for-runs.json +++ b/examples/invalid/facts.no-completeness-for-runs.json @@ -54,18 +54,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.no-completeness-for-timeline.json b/examples/invalid/facts.no-completeness-for-timeline.json index 27daaff7..d0ae0b17 100644 --- a/examples/invalid/facts.no-completeness-for-timeline.json +++ b/examples/invalid/facts.no-completeness-for-timeline.json @@ -54,18 +54,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.no-completeness.json b/examples/invalid/facts.no-completeness.json index 5d893dd1..89e504eb 100644 --- a/examples/invalid/facts.no-completeness.json +++ b/examples/invalid/facts.no-completeness.json @@ -54,18 +54,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.no-count-of-acts.json b/examples/invalid/facts.no-count-of-acts.json index df0d7e6b..311d896a 100644 --- a/examples/invalid/facts.no-count-of-acts.json +++ b/examples/invalid/facts.no-count-of-acts.json @@ -54,18 +54,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.no-word-on-acceptance.json b/examples/invalid/facts.no-word-on-acceptance.json index 01ca0f00..41d0d7bb 100644 --- a/examples/invalid/facts.no-word-on-acceptance.json +++ b/examples/invalid/facts.no-word-on-acceptance.json @@ -123,18 +123,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.review-without-reviewer.json b/examples/invalid/facts.review-without-reviewer.json index 639243cb..4be2a2a9 100644 --- a/examples/invalid/facts.review-without-reviewer.json +++ b/examples/invalid/facts.review-without-reviewer.json @@ -63,18 +63,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/examples/invalid/facts.time-in-another-format.json b/examples/invalid/facts.time-in-another-format.json index 6845c905..33e540dc 100644 --- a/examples/invalid/facts.time-in-another-format.json +++ b/examples/invalid/facts.time-in-another-format.json @@ -54,18 +54,21 @@ { "name": "verify", "app_id": 15368, + "workflow": ".github/workflows/verify.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "Secret scan (gitleaks)", "app_id": 15368, + "workflow": ".github/workflows/ci.yml", "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" }, { "name": "CodeQL", "app_id": 57789, + "workflow": null, "conclusion": "success", "completed_at": "2026-10-11T12:06:00Z" } diff --git a/kernel/derive.py b/kernel/derive.py index 6a25097e..2a4a7947 100644 --- a/kernel/derive.py +++ b/kernel/derive.py @@ -70,41 +70,66 @@ def named_for(check: dict, lens: dict) -> bool: return check.get("name") == lens["check"] -def reports_for(check: dict, lens: dict) -> bool: - """Whether `check` reports for the required Lens `lens`: its check's name, from the App the configuration names. +def from_its_app(check: dict, lens: dict) -> bool: + """Whether `check` was reported by the App the configuration names for the required Lens `lens`. An App is an id that is a whole number, in the facts and in the configuration: never true, nor 15368.0. """ app = check.get("app_id") - return named_for(check, lens) and type(app) is int and type(lens["app_id"]) is int and app == lens["app_id"] + return type(app) is int and type(lens["app_id"]) is int and app == lens["app_id"] -def foreign_checks(checks: list, config: dict) -> list: - """Each check under a required deterministic Lens's check name that the Lens's own App did not report, as evidence. +def from_its_workflow(check: dict, lens: dict) -> bool: + """Whether `check` ran from the workflow file the configuration names for the required Lens `lens`, exactly. + + GitHub Actions is one App for every job in the repo, one a pull request adds on its own branch among them, so a + Lens it runs is read only from its own file. A Lens no workflow runs names none, and its check must name none + too. A check that carries no workflow at all was not read whole, and is never given one. + """ + return "workflow" in check and check["workflow"] == lens["workflow"] + - Any App with checks: write can post a check under any name; the ruleset pins `verify` to its App, and the - Kernel pins every required Lens. Such a check makes its Lens unreadable whatever it, or the Lens's own checks, - conclude (I-13). Each is given as `:from_app:`, the id `unreadable` when it is not a whole number, - so that the Maintainer can tell it from a check that gave no verdict. +def reports_for(check: dict, lens: dict) -> bool: + """Whether `check` reports for the required Lens `lens`: its check's name, from the App and the workflow file the + configuration names.""" + return named_for(check, lens) and from_its_app(check, lens) and from_its_workflow(check, lens) + + +def foreign_checks(checks: list, config: dict) -> list: + """Each check under a required deterministic Lens's check name that the Lens's own App, or its own workflow file, + did not report, as evidence. + + Any App with checks: write can post a check under any name, and GitHub Actions runs any job a pull request adds; the + ruleset pins `verify` to its App, and the Kernel pins every required Lens to its App and its file. Such a check + makes its Lens unreadable whatever it, or the Lens's own checks, conclude (I-13). One from another App is given as + `:from_app:`, the id `unreadable` when it is not a whole number; one from the Lens's App and another + file as `:from_workflow:`, the path `unreadable` when no file was read. So the Maintainer can tell it + from a check that gave no verdict. """ out = [] for lens in config["required_lenses"]["deterministic"]: for c in checks: - if named_for(c, lens) and not reports_for(c, lens): + if not named_for(c, lens) or reports_for(c, lens): + continue + if not from_its_app(c, lens): app = c.get("app_id") said = "%s:from_app:%s" % (lens["lens"], app if type(app) is int else "unreadable") - if said not in out: - out.append(said) + else: + path = c.get("workflow") + said = "%s:from_workflow:%s" % (lens["lens"], path if isinstance(path, str) else "unreadable") + if said not in out: + out.append(said) return out def lens_reading(checks: list, lens: dict, config: dict) -> str: - """What a required deterministic Lens says, from every check its own App reports: as `check_reading` gives it. + """What a required deterministic Lens says, from every check its own App and workflow file report: as + `check_reading` gives it. - Another App's check under its check's name is read before any Lens (foreign_checks). Every check from its own App is - read, never the last alone: any failing one fails the Lens; then any with no verdict leaves it unreadable; - then any still running is a wait. It passes only when every one of them passes. A Lens that has not reported - is pending. + Another App's check, or another file's, under its check's name is read before any Lens (foreign_checks). Every check + from its own App and file is read, never the last alone: any failing one fails the Lens; then any with no verdict + leaves it unreadable; then any still running is a wait. It passes only when every one of them passes. A Lens that + has not reported is pending. """ mine = [c for c in checks if reports_for(c, lens)] if not mine: @@ -218,9 +243,9 @@ def moved_at(facts: dict, runs: list, config: dict, now: str): a Lens or the Factory moved it: a push, a required check's completion, a review or a thread by a writer, a Lens's App or the machine user (a gating thread is always one; moves), a writer's action and a Run on this item. A stranger's comment, another App's, or a check that is not required, moves nothing, or anyone could keep a stalled - item from showing; nor does a check under a - required Lens's check name from any App but that Lens's own (reports_for). A time that is not a real - moment no later than `now` is passed over, so the wait counts from earlier: that shows more, never less. + item from showing; nor does a check under a required Lens's check name from any App or workflow file but that + Lens's own (reports_for). A time that is not a real moment no later than `now` is passed over, so the wait counts + from earlier: that shows more, never less. """ required = config["required_lenses"]["deterministic"] times = [facts["last_push_at"], facts.get("last_writer_action_at")] @@ -540,8 +565,9 @@ def reading(facts: dict, runs: list, config: dict, past: tuple) -> dict: unread = unreadable_threads(facts, config) if unread: return {"state": None, "reason": "thread_unreadable", "evidence": unread} - # So could a check that another App posted under a required Lens's check name: it is read before any Lens's failure - # and before a conflict, as a fix Run is an action. It stays on this head, so only a push clears it. + # So could a check that another App, or another workflow file, posted under a required Lens's check name: it is read + # before any Lens's failure and before a conflict, as a fix Run is an action. It stays on this head, so only a push + # clears it. foreign = foreign_checks(facts["checks"], config) if foreign: return {"state": t.VERIFYING, "reason": "check_unreadable", "evidence": foreign} diff --git a/schemas/config.schema.json b/schemas/config.schema.json index 57f8eadf..6df6413c 100644 --- a/schemas/config.schema.json +++ b/schemas/config.schema.json @@ -44,7 +44,8 @@ "required": [ "lens", "check", - "app_id" + "app_id", + "workflow" ], "properties": { "lens": { @@ -59,6 +60,14 @@ "app_id": { "type": "integer", "minimum": 1 + }, + "workflow": { + "description": "For a Lens GitHub Actions runs, the path of its workflow file, which only the Maintainer changes; null for a Lens no workflow runs. GitHub Actions is one App for every job in the repo, one a pull request adds among them, so a check under the Lens's name from any other file is not the Lens's", + "type": [ + "string", + "null" + ], + "pattern": "^\\.github/workflows/[^/]+\\.ya?ml$" } } } diff --git a/schemas/facts.schema.json b/schemas/facts.schema.json index 7cbf9345..da0c0285 100644 --- a/schemas/facts.schema.json +++ b/schemas/facts.schema.json @@ -296,6 +296,7 @@ "required": [ "name", "app_id", + "workflow", "conclusion", "completed_at" ], @@ -309,6 +310,14 @@ "type": "integer", "minimum": 1 }, + "workflow": { + "description": "The path of the workflow file that ran the check, as GitHub gives it for the workflow run of the check run's check suite (its `path`, such as .github/workflows/verify.yml); null for a check no workflow ran, as one GitHub Advanced Security reports. GitHub Actions is one App for every job in the repo, so a required Lens that it runs is read only from the file the configuration names for it", + "type": [ + "string", + "null" + ], + "minLength": 1 + }, "conclusion": { "description": "GitHub's own conclusion, or the check's status while it has not concluded. The port maps nothing; config check_conclusions says what each means.", "type": "string", diff --git a/sim/grounding.py b/sim/grounding.py index 66d49526..17b4f982 100644 --- a/sim/grounding.py +++ b/sim/grounding.py @@ -27,6 +27,15 @@ "check run: filter=latest is per check suite, so two workflow runs on one commit both stay", (), None), ("any_app_posts_a_check_under_any_name", "Any App with checks: write can post a check run under any name, a " "required Lens's too, and it stays on that head: a check run cannot be deleted", (), None), + ("a_check_names_the_workflow_that_ran_it", "Every check run GitHub Actions reports is read with the path of the " + "workflow file that ran it, the path of its check suite's workflow run, with the permissions the reader and the " + "Controller hold; a check run no workflow ran, as GitHub Advanced Security's, is read with none", ("P-35",), None), + ("actions_runs_a_job_under_any_name", "GitHub Actions reports every job in the repo as one App, 15368, one a pull " + "request adds in a workflow file of its own under a required Lens's check name among them, and its check run " + "names that file", ("P-35",), None), + ("only_codeql_gives_the_code_scanning_verdict", "Only CodeQL's own analysis gives the conclusion of the check run " + "CodeQL that GitHub Advanced Security, 57789, reports: an upload to code scanning under CodeQL's tool and category, " + "by an identity holding security_events: write, cannot replace that verdict on the head", ("P-36",), None), ("a_push_leaves_the_queue", "A push takes the pull request out of the merge queue", (), None), ("an_app_enters_the_queue", "An App can put an approved pull request in the merge queue", ("P-2",), None), ("the_queue_merges", "A pull request in the merge queue merges, unless a push takes it out first", (), None), diff --git a/sim/world.py b/sim/world.py index 5ed302ee..537ac909 100644 --- a/sim/world.py +++ b/sim/world.py @@ -28,8 +28,13 @@ APPS = {lens["lens"]: lens["app_id"] for lens in CONFIG["required_lenses"]["deterministic"]} # The name GitHub gives each required check run, by its Lens's name. CHECKS = {lens["lens"]: lens["check"] for lens in CONFIG["required_lenses"]["deterministic"]} +# The workflow file that runs each required check, by its Lens's name: null for a Lens no workflow runs. +WORKFLOWS = {lens["lens"]: lens["workflow"] for lens in CONFIG["required_lenses"]["deterministic"]} # An App no Lens is read from, which can write checks: the Controller's own can post a check under any name. IMPOSTOR_APP = 1000001 +# A workflow file no Lens is read from: GitHub Actions, a Lens's own App, runs any job in the repo under any name, one a +# pull request adds on its own branch among them. +IMPOSTOR_WORKFLOW = ".github/workflows/impostor.yml" # Who posts it, in the Lifecycle's words: the facts give only its App's id, and no Run or act of the Factory's posts a # check under a Lens's name, so the change is the platform's (by_of). IMPOSTOR = {"kind": "app", "name": "another-app[bot]", "can_write": True} @@ -155,7 +160,9 @@ def facts(self) -> dict: # GitHub: an_actor_has_a_kind "reviews": copy.deepcopy(self.reviews), "threads": copy.deepcopy(self.threads), # GitHub: a_check_is_read_by_name_from_every_app - "checks": [{"name": CHECKS[k], "app_id": APPS[k], "conclusion": v, "completed_at": self.completed.get(k)} + # GitHub: a_check_names_the_workflow_that_ran_it + "checks": [{"name": CHECKS[k], "app_id": APPS[k], "workflow": WORKFLOWS[k], "conclusion": v, + "completed_at": self.completed.get(k)} for k, v in sorted(self.checks.items())] + copy.deepcopy(self.impostors), "queue": self.queue, # GitHub: the_timeline_shows_each_act @@ -400,12 +407,12 @@ def poll(self): reviews = d["outcome"] == "admit" and d["admit"]["intent"] == "review-change" self.check(not no_verdict or not (reviews or (d["outcome"] == "act" and not dequeue)), "I-13 a review or an act while a check has no verdict") - # Another App's check under a required Lens's name lets no Run start and no act be made but a dequeue, not - # even a fix for a check that failed. - apps = {CHECKS[lens]: app for lens, app in APPS.items()} - foreign = [c for c in facts["checks"] if c["app_id"] != apps[c["name"]]] + # Another App's check, or another workflow file's, under a required Lens's name lets no Run start and no act be + # made but a dequeue, not even a fix for a check that failed. + own = {CHECKS[lens]: (APPS[lens], WORKFLOWS[lens]) for lens in APPS} + foreign = [c for c in facts["checks"] if (c["app_id"], c["workflow"]) != own[c["name"]]] self.check(not foreign or not (d["outcome"] == t.ADMIT or (d["outcome"] == t.ACT and not dequeue)), - "I-13 something done while another App's check stands under a Lens's name") + "I-13 something done while another App's or workflow's check stands under a Lens's name") waits = d["reason"] == "check_unreadable" and d["outcome"] == t.DEFER self.check(not waits or d["attention"] == t.NEEDS_YOU, "a check with no verdict is not shown as needing a person") @@ -711,10 +718,20 @@ def platform(self): self.caused("human" if lens in self.rerun else "lens") self.rerun.discard(lens) if self.faults and roll < 0.87 and self.checks[lens] == "failure": - # GitHub: any_app_posts_a_check_under_any_name - self.impostors.append({"name": CHECKS[lens], "app_id": IMPOSTOR_APP, "conclusion": "success", - "completed_at": self.now()}) - self.note("check %s: failure, and another App posted a passing one under its name" % lens) + if WORKFLOWS[lens] is not None and roll < 0.86: + # GitHub: actions_runs_a_job_under_any_name + self.impostors.append({"name": CHECKS[lens], "app_id": APPS[lens], "workflow": IMPOSTOR_WORKFLOW, + "conclusion": "success", "completed_at": self.now()}) + self.note("check %s: failure, and another workflow posted a passing one under its name" % lens) + else: + # GitHub: any_app_posts_a_check_under_any_name + # Never one from the Lens's own App and file, which the Kernel could not tell from the Lens's: for + # code scanning, that would be an upload under CodeQL's tool and category. + # GitHub: only_codeql_gives_the_code_scanning_verdict + self.impostors.append({"name": CHECKS[lens], "app_id": IMPOSTOR_APP, "workflow": None, + "conclusion": "success", "completed_at": self.now()}) + self.note("check %s: failure, and another App posted a passing one under its name" % lens) + # Neither is a Run or an act of the Factory's, nor the Lens's own: the change is the platform's. self.caused(self.by_of(IMPOSTOR)) def person(self): diff --git a/tests/support.py b/tests/support.py index 88b76e52..fbe6c5ec 100644 --- a/tests/support.py +++ b/tests/support.py @@ -14,12 +14,16 @@ ITEM = {"repo": "combsmith/software-factory", "type": "pull_request", "number": 42} NOW = "2026-10-11T13:00:00Z" # Every required check passing, each under its check run's name as GitHub gives it, from its Lens's own App. -GREEN = [{"name": lens["check"], "app_id": lens["app_id"], "conclusion": "success", +# A check GitHub Actions runs carries the workflow file that ran it; one no workflow ran carries null. +GREEN = [{"name": lens["check"], "app_id": lens["app_id"], "workflow": lens["workflow"], "conclusion": "success", "completed_at": "2026-10-11T12:05:00Z"} for lens in CONFIG["required_lenses"]["deterministic"]] # The Lens each required check reports for, by the check run's name: the name a Decision gives as evidence. LENS_OF = {lens["check"]: lens["lens"] for lens in CONFIG["required_lenses"]["deterministic"]} # An App no required Lens is read from: any App with checks: write, the Controller's among them, can post a check. OTHER_APP = 1000001 +# A workflow file no required Lens is read from: GitHub Actions runs every job in the repo, under any name, one a pull +# request adds on its own branch among them. +OTHER_WORKFLOW = ".github/workflows/impostor.yml" # The Maintainer: a person, which is an actor of kind human with write access, and nobody else is. MAINTAINER = {"kind": "human", "name": "don-petry", "can_write": True} diff --git a/tests/test_decide.py b/tests/test_decide.py index 710fd8c9..90b26f99 100644 --- a/tests/test_decide.py +++ b/tests/test_decide.py @@ -8,8 +8,8 @@ from kernel import derive from kernel.decide import decide -from tests.support import (CONFIG, GREEN, LENS_OF, MAINTAINER, NOW, OTHER_APP, ROOT, config, context, contract, - evidence, facts, labelled, review, run, thread, unlabelled) +from tests.support import (CONFIG, GREEN, LENS_OF, MAINTAINER, NOW, OTHER_APP, OTHER_WORKFLOW, ROOT, config, context, + contract, evidence, facts, labelled, review, run, thread, unlabelled) from tools.jsonschema_lite import validate DECISION_SCHEMA = json.loads((ROOT / "schemas" / "decision.schema.json").read_text()) @@ -1133,6 +1133,65 @@ def test_a_check_from_another_app_is_unreadable_for_its_lens(self): # I-13, I-2 other = {"name": "template-drift", "app_id": OTHER_APP, "conclusion": "failure", "completed_at": None} self.assertEqual(go(facts(checks=GREEN + [other]), REVIEWED)["action"], "issue_approval") + def test_a_check_from_another_workflow_is_unreadable_for_its_lens(self): # I-13, I-27 + # GitHub Actions is one App for every job in the repo, one a pull request adds on its own branch among them, so a + # Lens it runs is read only from the workflow file the configuration names for it: a check under the Lens's + # check name from another file of its own App, or with no file read, leaves it unreadable, as one from another + # App does. A Lens no workflow runs names none, and so must its check. The evidence names the Lens and the file. + for i, check in enumerate(GREEN): + lens = CONFIG["required_lenses"]["deterministic"][i] + self.assertEqual(lens["check"], check["name"]) + others = [OTHER_WORKFLOW] + sorted({c["workflow"] for c in GREEN if isinstance(c["workflow"], str)} + - {check["workflow"]}) + if isinstance(check["workflow"], str): + # The same file in another case, or from another directory, is another file; and null is no file read. + others += [check["workflow"].upper(), "x/" + check["workflow"], None] + for path in others + ["absent"]: + said = "%s:from_workflow:%s" % (lens["lens"], path if isinstance(path, str) and path != "absent" + else "unreadable") + for conclusion in ("success", "failure"): + impostor = dict(check, workflow=path, conclusion=conclusion) + if path == "absent": + del impostor["workflow"] + for entries in ([impostor], [check, impostor], [impostor, check], + [dict(check, conclusion="failure"), impostor]): + checks = GREEN[:i] + entries + GREEN[i + 1:] + for runs, reviews in (((), []), (REVIEWED, []), (REVIEWED, APPROVED)): + d = go(facts(checks=checks, reviews=reviews), runs) + self.assertEqual((d["state"], d["reason"], d["evidence"], d["outcome"], d["attention"]), + ("verifying", "check_unreadable", [said], "defer", "needs_you"), + (lens["lens"], path, conclusion, len(checks))) + self.assertNotIn("admit", d) + self.assertNotIn("action", d) + # It is read before any Lens's failure, and before a conflict, as another App's check is. + actions = next(c for c in GREEN if isinstance(c["workflow"], str)) + i = GREEN.index(actions) + impostor = dict(actions, workflow=OTHER_WORKFLOW) + said = ["%s:from_workflow:%s" % (LENS_OF[actions["name"]], OTHER_WORKFLOW)] + rest = GREEN[:i] + GREEN[i + 1:] + failing = [dict(rest[0], conclusion="failure")] + rest[1:] + for f in (facts(checks=[impostor] + failing), facts(checks=[impostor, actions] + failing), + facts(checks=[impostor] + GREEN, mergeable=False)): + d = go(f, REVIEWED) + self.assertEqual((d["state"], d["reason"], d["evidence"], d["outcome"], d["attention"]), + ("verifying", "check_unreadable", said, "defer", "needs_you")) + # From another App, it is shown as another App's, whatever file it names. + d = go(facts(checks=[dict(actions, app_id=OTHER_APP, workflow=OTHER_WORKFLOW)] + GREEN), REVIEWED) + self.assertEqual(d["evidence"], ["%s:from_app:%d" % (LENS_OF[actions["name"]], OTHER_APP)]) + # A configuration that names another file reads none of these checks as its Lens's. + lenses = [dict(lens, workflow=OTHER_WORKFLOW) if lens["check"] == actions["name"] else lens + for lens in CONFIG["required_lenses"]["deterministic"]] + cfg = config(required_lenses=dict(CONFIG["required_lenses"], deterministic=lenses)) + d = go(facts(), REVIEWED, cfg=cfg) + self.assertEqual((d["reason"], d["evidence"]), + ("check_unreadable", ["%s:from_workflow:%s" % (LENS_OF[actions["name"]], actions["workflow"])])) + # The other side: from its own App and its own file, the Lens is read, every one of its checks; and a job under a + # name no Lens has, from any file, is not read at all. + self.assertEqual(go(facts(), REVIEWED)["action"], "issue_approval") + self.assertEqual(go(facts(checks=[dict(actions)] + GREEN), REVIEWED)["action"], "issue_approval") + other = dict(actions, name="template-drift", workflow=OTHER_WORKFLOW, conclusion="failure") + self.assertEqual(go(facts(checks=GREEN + [other]), REVIEWED)["action"], "issue_approval") + def test_every_check_a_lens_reports_from_its_own_app_is_read(self): # I-13, I-27 # GitHub can hold several checks with one name from one App: two workflows with the same job name, say. Each # is read, never the last alone. Any that fails fails the Lens; failing none, any that gave no verdict leaves @@ -1781,6 +1840,20 @@ def test_a_check_from_another_app_or_a_failure_beside_a_pass_takes_it_out_of_the d = go(facts(queue=True, reviews=APPROVED, checks=entries + GREEN[1:]), REVIEWED) self.assertEqual((d["state"], d["reason"], d["outcome"]), ("queued", "in_merge_queue", "nothing"), entries) + def test_a_check_from_another_workflow_takes_it_out_of_the_queue(self): # I-9, I-27 + # A Lens GitHub Actions runs is read from its own workflow file alone, on a queued item too. + actions = next(c for c in GREEN if isinstance(c["workflow"], str)) + lens = LENS_OF[actions["name"]] + for path, shown in ((OTHER_WORKFLOW, OTHER_WORKFLOW), (None, "unreadable")): + for entries in ([dict(actions, workflow=path)] + GREEN, GREEN + [dict(actions, workflow=path)]): + d = go(facts(queue=True, reviews=APPROVED, checks=entries), REVIEWED) + self.assertEqual((d["state"], d["reason"], d["evidence"], d["outcome"], d["action"]), + ("verifying", "check_unreadable", ["%s:from_workflow:%s" % (lens, shown)], "act", + "dequeue"), path) + # The other side: two checks from its own file, both passing, take nothing out. + d = go(facts(queue=True, reviews=APPROVED, checks=GREEN + [dict(actions)]), REVIEWED) + self.assertEqual((d["state"], d["reason"], d["outcome"]), ("queued", "in_merge_queue", "nothing")) + def test_a_queued_item_whose_evidence_goes_missing_or_unreadable_leaves_the_queue(self): # I-31, I-13 # GitHub's merge queue knows nothing of an acceptance contract, so a queued item stays only while every # criterion has passing evidence on its content. Evidence that is gone, as an artifact is after its retention, @@ -1982,6 +2055,26 @@ def test_only_a_check_from_its_lens_app_moves_an_item(self): # I-1, I-28 del missing["app_id"] self.assertEqual(derive.moved_at(facts(checks=self.SILENT + [missing]), [], CONFIG, NOW), at(5)) + def test_only_a_check_from_its_lens_workflow_moves_an_item(self): # I-1, I-28 + # GitHub Actions runs any job a pull request adds, under any name, so only the check from a Lens's own workflow + # file is its completion; a Lens no workflow runs is read only from a check that names none. + own = dict(GREEN[1], completed_at=at(20)) + self.assertIsInstance(own["workflow"], str) + self.assertEqual(derive.moved_at(facts(checks=self.SILENT + [own]), [], CONFIG, NOW), at(20)) + for path in (OTHER_WORKFLOW, GREEN[0]["workflow"], None, own["workflow"].upper(), "absent"): + impostor = dict(own, workflow=path) + if path == "absent": + del impostor["workflow"] + self.assertEqual(derive.moved_at(facts(checks=self.SILENT + [impostor]), [], CONFIG, NOW), at(5), path) + unrun = next(c for c in GREEN if c["workflow"] is None) + self.assertEqual(derive.moved_at(facts(checks=self.SILENT + [dict(unrun, completed_at=at(20))]), [], CONFIG, + NOW), at(20)) + for path in (OTHER_WORKFLOW, "absent"): + impostor = dict(unrun, workflow=path, completed_at=at(20)) + if path == "absent": + del impostor["workflow"] + self.assertEqual(derive.moved_at(facts(checks=self.SILENT + [impostor]), [], CONFIG, NOW), at(5), path) + def test_a_time_that_is_no_moment_or_still_to_come_moves_nothing(self): # I-1, I-13 # A time is read only when it is a real moment no later than now. Any other is passed over, so a time in the # future, or one with a field no calendar has, never hides a stalled item: that shows more, never less. Each diff --git a/tests/test_simulator.py b/tests/test_simulator.py index f619daa0..69d6602e 100644 --- a/tests/test_simulator.py +++ b/tests/test_simulator.py @@ -285,16 +285,19 @@ def test_a_dequeue_at_the_act_ceiling_is_the_one_act_past_it(self): # I-8 def test_the_seeds_take_a_pull_request_out_of_the_queue_for_each_cause(self): # The faults while queued are reached: a dismissed approval, an open gate and a Hold each take one out. - # So is another App's passing check under the name of a Lens that failed, which no Run or act may follow. - causes, impostors = set(), 0 + # So is another App's passing check under the name of a Lens that failed, which no Run or act may follow, and + # one from another workflow file of the Lens's own App. + causes, impostors, workflows = set(), 0, 0 for seed in SEEDS: w = World(seed) for _ in range(250): w.step() causes |= {line.split("(")[-1].rstrip(")") for line in w.trace if "apply: dequeued" in line} impostors += sum("another App posted" in line for line in w.trace) + workflows += sum("another workflow posted" in line for line in w.trace) self.assertLessEqual({"approval_lost", "gate_open", "human_hold"}, causes) self.assertTrue(impostors) + self.assertTrue(workflows) def test_the_seeds_serve_items_with_an_acceptance_contract_and_without(self): # I-31 # Some worlds carry a contract, and its evidence goes missing for a while, fails, is given twice and disagrees, diff --git a/tests/test_tools.py b/tests/test_tools.py index abbbdc96..b60c9bba 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -912,6 +912,29 @@ def test_the_required_check_that_has_drifted_from_its_example_is_caught(self): root = tree({"examples/workflows/verify.yml": GOOD, ".github/workflows/verify.yml": bumped}) self.assertFalse(caught(check_workflows.run(root), "change both together")) + def test_a_lens_read_from_a_workflow_that_does_not_run_its_check_is_caught(self): + # Each required Lens GitHub Actions runs is read only from the workflow file the configuration names for it. A + # file that is gone, or that runs no job under the Lens's check name, would leave every pull request's check + # another file's, and the Lens unreadable on all of them. + defaults = json.loads((ROOT / "config" / "defaults.json").read_text()) + lenses = defaults["required_lenses"]["deterministic"] + run_by_actions = [lens for lens in lenses if isinstance(lens["workflow"], str)] + self.assertTrue(run_by_actions) + for lens in run_by_actions: + other = next(x["workflow"] for x in run_by_actions if x["workflow"] != lens["workflow"]) + for path, said in ((".github/workflows/gone.yml", "which does not exist"), + (other, "runs no job named %r" % lens["check"])): + moved = [dict(x, workflow=path) if x is lens else x for x in lenses] + cfg = dict(defaults, required_lenses=dict(defaults["required_lenses"], deterministic=moved)) + root = clone({"config/defaults.json": json.dumps(cfg, indent=2) + "\n"}) + self.assertTrue(caught(check_workflows.run(root), said), (lens["lens"], path)) + # The job renamed in its own file. + text = (ROOT / lens["workflow"]).read_text() + renamed = text.replace(" name: %s\n" % lens["check"], " name: %s-renamed\n" % lens["check"]) + self.assertNotEqual(renamed, text, lens["lens"]) + root = clone({lens["workflow"]: renamed}) + self.assertTrue(caught(check_workflows.run(root), "runs no job named %r" % lens["check"]), lens["lens"]) + def test_a_live_workflow_is_held_to_the_rules(self): bad = (ROOT / "examples" / "workflows" / "invalid" / "pin.a-tag-can-move.yml").read_text() problems = check_workflows.run(tree({".github/workflows/x.yml": bad, "examples/workflows/verify.yml": GOOD})) diff --git a/tools/check_workflows.py b/tools/check_workflows.py index ce6cd0dc..5cbfa434 100644 --- a/tools/check_workflows.py +++ b/tools/check_workflows.py @@ -11,6 +11,7 @@ from __future__ import annotations import hashlib +import json import pathlib import re @@ -192,8 +193,34 @@ def under(*path): return sorted(set(found)) -def run(root: pathlib.Path = ROOT) -> list: +def lens_workflows(root: pathlib.Path) -> list: + """Each required Lens GitHub Actions runs names a workflow file that exists and runs a job under its check's name. + + The Kernel reads such a Lens only from that file (`config/defaults.json`). Gone, moved, or with the job renamed, + every pull request's check would be another file's or none, and the Lens unreadable on all of them. + """ + config = root / "config" / "defaults.json" + if not config.is_file(): + return [] problems = [] + for lens in json.loads(config.read_text())["required_lenses"]["deterministic"]: + rel = lens.get("workflow") + if not isinstance(rel, str): + continue + path = root / rel + if not path.is_file(): + problems.append("config/defaults.json: the Lens %s is read from %s, which does not exist" % (lens["lens"], rel)) + continue + entries, _ = read(path.read_text(errors="replace")) + names = [e["value"] for e in entries if e["key"] == "name" and len(e["path"]) == 2 and e["path"][0] == "jobs"] + if lens["check"] not in names: + problems.append("config/defaults.json: the Lens %s is read from %s, which runs no job named %r" + % (lens["lens"], rel, lens["check"])) + return problems + + +def run(root: pathlib.Path = ROOT) -> list: + problems = lens_workflows(root) live = sorted((root / ".github" / "workflows").glob("*.y*ml")) example = root / "examples" / "workflows" / "verify.yml" for path in live + sorted((root / "examples" / "workflows").glob("*.y*ml")): diff --git a/tools/mutants.py b/tools/mutants.py index 23c3fdd5..d521e352 100644 --- a/tools/mutants.py +++ b/tools/mutants.py @@ -222,6 +222,13 @@ def moved_after(rel: str, block: str, anchor: str) -> tuple: ' lenses = config["required_lenses"]\n' ' required = set() if facts["class"] in lenses["model_not_required_for"] and facts["last_pusher"] == ' 'facts["author"] else set(lenses["model"])\n'), + ("a Lens is read from another workflow file", "config/defaults.json", + '"workflow": ".github/workflows/verify.yml"', '"workflow": ".github/workflows/ci.yml"'), + ("a Lens's workflow file need not exist", "tools/check_workflows.py", + ' problems.append("config/defaults.json: the Lens %s is read from %s, which does not exist"' + ' % (lens["lens"], rel))\n', " pass\n"), + ("a Lens's workflow file need not run its check", "tools/check_workflows.py", + ' if lens["check"] not in names:\n', " if False:\n"), ("a neutral check is read as passing", "config/defaults.json", '"neutral": "unreadable"', '"neutral": "passing"'), ("a neutral check is read as failing", "config/defaults.json", @@ -238,11 +245,25 @@ def moved_after(rel: str, block: str, anchor: str) -> tuple: ("an absent required check passes", "kernel/derive.py", ' if not mine:\n return "pending"\n', ' if not mine:\n return "passing"\n'), ("another App's check is read for a Lens", "kernel/derive.py", - ' and type(app) is int and type(lens["app_id"]) is int and app == lens["app_id"]\n', "\n"), + " return named_for(check, lens) and from_its_app(check, lens) and from_its_workflow(check, lens)\n", + " return named_for(check, lens) and from_its_workflow(check, lens)\n"), ("an App id in the facts that is not a whole number is read", "kernel/derive.py", - ' and type(app) is int and type(lens["app_id"]) is int and', ' and type(lens["app_id"]) is int and'), + ' return type(app) is int and type(lens["app_id"]) is int and', ' return type(lens["app_id"]) is int and'), ("an App id in the configuration that is not a whole number is read", "kernel/derive.py", - ' and type(app) is int and type(lens["app_id"]) is int and', " and type(app) is int and"), + ' return type(app) is int and type(lens["app_id"]) is int and', " return type(app) is int and"), + ("another workflow file's check is read for a Lens", "kernel/derive.py", + " return named_for(check, lens) and from_its_app(check, lens) and from_its_workflow(check, lens)\n", + " return named_for(check, lens) and from_its_app(check, lens)\n"), + ("a check that names no workflow is read for a Lens that names none", "kernel/derive.py", + ' return "workflow" in check and check["workflow"] == lens["workflow"]\n', + ' return check.get("workflow") == lens["workflow"]\n'), + ("a workflow file is matched in any case", "kernel/derive.py", + ' return "workflow" in check and check["workflow"] == lens["workflow"]\n', + ' return "workflow" in check and str(check["workflow"]).casefold() == str(lens["workflow"]).casefold()\n'), + ("a workflow file is matched by its name alone", "kernel/derive.py", + ' return "workflow" in check and check["workflow"] == lens["workflow"]\n', + ' return "workflow" in check and str(check["workflow"]).rsplit("/", 1)[-1] == str(lens["workflow"]).rsplit("/",' + ' 1)[-1]\n'), ("a check is matched to a Lens by the Lens's own name", "kernel/derive.py", ' return check.get("name") == lens["check"]\n', ' return check.get("name") in (lens["check"], lens["lens"])\n'), @@ -266,6 +287,13 @@ def moved_after(rel: str, block: str, anchor: str) -> tuple: ' said = lens["lens"]\n'), ("an App id that is no whole number is shown as it is", "kernel/derive.py", 'app if type(app) is int else "unreadable")', "app)"), + ("another workflow file's check is shown as another App's", "kernel/derive.py", + ' said = "%s:from_workflow:%s" % (lens["lens"], path if isinstance(path, str) else "unreadable")\n', + ' said = "%s:from_app:%s" % (lens["lens"], c.get("app_id"))\n'), + ("a workflow that was not read is shown as it is", "kernel/derive.py", + 'path if isinstance(path, str) else "unreadable")', "path)"), + ("a check from another App and another file is shown as another file's", "kernel/derive.py", + " if not from_its_app(c, lens):\n", " if from_its_workflow(c, lens):\n"), ("the last check with a Lens's name wins", "kernel/derive.py", " readings = {check_reading(c, config) for c in mine}\n", " readings = {check_reading(c, config) for c in mine[-1:]}\n"), @@ -638,6 +666,9 @@ def moved_after(rel: str, block: str, anchor: str) -> tuple: ("a check from another App moves an item", "kernel/derive.py", " if any(reports_for(c, lens) for lens in required)]", " if any(named_for(c, lens) for lens in required)]"), + ("a check from another workflow file moves an item", "kernel/derive.py", + " if any(reports_for(c, lens) for lens in required)]", + " if any(named_for(c, lens) and from_its_app(c, lens) for lens in required)]"), ("an App that is no Lens's moves an item", "kernel/derive.py", 'actor.get("kind") in ("lens", t.MACHINE_USER)', 'actor.get("kind") in ("lens", t.MACHINE_USER, t.APP)'), ("the machine user's approval does not move an item", "kernel/derive.py", From a685e7c332f9ce8a86c0399eed901746cfe43dce Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:10:51 -0500 Subject: [PATCH 2/4] Decision 0018, not 0017: a Lens is read from its own App and its own workflow file "Session hooks" (pull request 109) adds its own record 0017, so this one is numbered 0018: its file name and title, and 0015's "replaced by" Status. Co-Authored-By: Claude Opus 5.5 --- ...s-read-from-its-own-app-and-only-what-gates-holds-a-merge.md | 2 +- ...s-own-app-and-workflow-and-only-what-gates-holds-a-merge.md} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename docs/decisions/{0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md => 0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md} (99%) diff --git a/docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md b/docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md index 8a11ff73..d001cf91 100644 --- a/docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md +++ b/docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md @@ -1,6 +1,6 @@ # 0015. A Lens is read from its own App, and only what gates holds a merge -Status: replaced by `docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md`, +Status: replaced by `docs/decisions/0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md`, which holds the rule in force and its boundaries. ## Decision diff --git a/docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md b/docs/decisions/0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md similarity index 99% rename from docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md rename to docs/decisions/0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md index 62520d4b..064ef860 100644 --- a/docs/decisions/0017-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md +++ b/docs/decisions/0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md @@ -1,4 +1,4 @@ -# 0017. A Lens is read from its own App and its own workflow file, and only what gates holds a merge +# 0018. A Lens is read from its own App and its own workflow file, and only what gates holds a merge Status: accepted. Replaces `docs/decisions/0015-a-lens-is-read-from-its-own-app-and-only-what-gates-holds-a-merge.md`, and keeps all of it: the Maintainer's ruling of 2026-10-14 on a Finding that does not gate From 6e94d0407b4217c9ad7e44634e6b582e4fc2645e Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Fri, 16 Oct 2026 06:45:03 -0500 Subject: [PATCH 3/4] Lens workflow pin: honest about what it holds, and stricter where it reads The review and security passes on the pin to a Lens's workflow file. - Decision 0018 and the design say the pin holds the workflow file's path, not its content or what it runs, and name what does hold .github/ today, and the gap: "Kernel: a change to the Factory's own rules or a Lens's inputs needs a person" (issue 113). - A Decision shows a workflow path only in the configuration's form (.github/workflows/, ASCII letters, digits, . _ -, .yml or .yaml) and no longer than the new limit max_path_length; any other, dynamic/ paths among them, is shown unreadable. - tools/check_workflows.py: a Lens of GitHub Actions names a workflow file and no other Lens names one; only jobs..name, or the id, names the check; a Lens job with a matrix, a job-level if: or uses:, or a workflow not on both pull_request and merge_group, is refused. No job is granted security-events: write. - tools/check_apps.py: no Factory App holds workflows or security_events. - The simulator asserts the Kernel showed from_workflow evidence. - Trial P-35 asks where a check's workflow path comes from and in what form; P-36 says how its fallback is held. Co-Authored-By: Claude Opus 5.5 --- config/defaults.json | 3 +- docs/architecture/software-factory-design.md | 48 ++++++++---- ...kflow-and-only-what-gates-holds-a-merge.md | 42 ++++++---- docs/standards/workflows.md | 4 +- ...ge-by-a-person-skips-the-model-review.json | 3 +- .../config.a-neutral-check-passes.json | 3 +- .../config.a-required-lens-with-no-app.json | 3 +- .../config.a-required-lens-with-no-check.json | 3 +- ...nfig.a-required-lens-with-no-workflow.json | 3 +- ...eness-limit-copied-from-the-lifecycle.json | 3 +- .../config.conclusion-mapped-to-nothing.json | 3 +- .../invalid/config.no-deterministic-lens.json | 3 +- .../permissions.security-events-on-a-job.yml | 22 ++++++ kernel/derive.py | 22 +++++- kernel/terms.py | 5 ++ schemas/config.schema.json | 12 ++- schemas/facts.schema.json | 2 +- sim/grounding.py | 5 +- tests/test_decide.py | 42 +++++++++- tests/test_simulator.py | 26 ++++--- tests/test_tools.py | 64 +++++++++++++++ tools/check_apps.py | 7 ++ tools/check_workflows.py | 77 ++++++++++++++++--- tools/mutants.py | 50 ++++++++++-- 24 files changed, 376 insertions(+), 79 deletions(-) create mode 100644 examples/workflows/invalid/permissions.security-events-on-a-job.yml diff --git a/config/defaults.json b/config/defaults.json index 2b719d31..d75a4bfa 100644 --- a/config/defaults.json +++ b/config/defaults.json @@ -56,6 +56,7 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 } } diff --git a/docs/architecture/software-factory-design.md b/docs/architecture/software-factory-design.md index 755f61f6..bec2f71c 100644 --- a/docs/architecture/software-factory-design.md +++ b/docs/architecture/software-factory-design.md @@ -397,19 +397,35 @@ pinned to its workflow file: the configuration's `workflow` names it, `.github/w for `verify` and `.github/workflows/ci.yml` for `secret-scan`, the files whose jobs carry those names, and null for `code-scanning`, which no workflow runs. The port gives each check run the `path` of its check suite's workflow run as `workflow`, and null for a check no workflow ran (trial P-35). -The Kernel matches the file exactly. A check under a Lens's check name from its own App and another -file, or with no file read where the Lens names one, is read as one from another App is, with the -evidence `:from_workflow:`, the path `unreadable` when none was read; and a check that -names a file, where the Lens names none, is not that Lens's either. `tools/check_workflows.py` holds -the configuration to the files: each file it names exists and runs a job under the Lens's check name. -A change to a Lens's own file is a change under `.github/`, which only the Maintainer authors -(decisions 0004, 0005). The `main` ruleset pins `verify` to its App; pinning each required check is -the Maintainer's, in the repo's settings, with "The Factory repo's rulesets hold the Release tags and each required check's App" -(issue 62), and the Kernel's pin does not wait on it. A Lens no workflow runs has a gap of its own: -whether an upload to code scanning under CodeQL's tool and category, by an identity holding -`security_events: write`, can replace the CodeQL verdict is unverified (trial P-36). Such an upload -would be reported by the Lens's own App under its own check, so the Kernel could not tell it from -the Lens; if it can, the answer is that no identity but CodeQL's own analysis holds that permission. +The Kernel matches the path exactly. A check under a Lens's check name from its own App and another +path, or with no path read where the Lens names one, is read as one from another App is, with the +evidence `:from_workflow:`; and a check that names a path, where the Lens names none, is +not that Lens's either. The evidence shows the path only in the form a Lens's workflow takes in the +configuration, `.github/workflows/` and a name of ASCII letters, digits, `.`, `_` and `-` ending +`.yml` or `.yaml`, no longer than `max_path_length`; any other, a `dynamic/` path, a look-alike or a +control character among them, is shown `unreadable`, so text from outside never reaches a Decision. +`tools/check_workflows.py` holds the configuration to the files: a Lens whose App is GitHub Actions +names a file, no other Lens names one, and the file exists, runs on both `pull_request` and +`merge_group`, and has a job whose own `name:`, or whose id where it has none, is the Lens's check, +with no `strategy.matrix`, job-level `if:` or `uses:`, any of which would leave the Lens waiting. + +The pin is to the path, not to what the file says, nor to what it runs. A pull request from a +branch of this repo can change the file at that path in place, and its check still reports from +it; so can a dependency bot's security update, which edits the pins in workflow files and, as class +`dependency`, skips the model review. `verify` runs the pull request's own Makefile, tools and +tests, and the secret scan reads its own `.gitleaks.toml`. What holds `.github/` today is GitHub +refusing an App's push there without the `workflows` permission, which no Factory App holds +(`config/apps.json`, held by `tools/check_apps.py`); a person's admission of a fork's pull request +after its last push (I-36); a Role's `apply`, which waits on `harness/`; and the session hooks, +locally. The `main` ruleset requires no review and pins only `verify` to its App; pinning each +required check is the Maintainer's, in the repo's settings, with +"The Factory repo's rulesets hold the Release tags and each required check's App" (issue 62), and +the Kernel's pin does not wait on it. That a change to a Lens's inputs needs a person is +"Kernel: a change to the Factory's own rules or a Lens's inputs needs a person" (issue 113). A Lens +no workflow runs has a gap of its own: whether an upload to code scanning under CodeQL's tool and +category, by an identity holding `security_events: write`, can replace the CodeQL verdict is +unverified (trial P-36). Such an upload would be reported by the Lens's own App under its own +check, so the Kernel could not tell it from the Lens. A check under a required Lens's check name from any other App, or with an App id that is not a whole number, makes that Lens unreadable whatever it concludes, and whatever the Lens's own App says @@ -1735,7 +1751,7 @@ room. That shapes every row. | Get a harmless fork commit admitted, then push the real one | Admission lapses on the next push (I-36) | | From a fork, open pull requests that say they close an issue, to park it, hold it or spend its one retry | Only a pull request opened by a Factory App or a person with write access, or one the Maintainer has admitted, is linked to an issue (I-39) | | Get a harmless issue admitted, then edit it; or write an issue that tells the model to call itself ready, perhaps in a comment the rendered page hides | An edit ends acceptance at the next pass (I-36, I-42), and `apply` refuses a Run admitted for other content (I-21, I-40). Admission never changes the class of the text: a Run that reads it has no shell and no network tool (I-35), and the Runs that have them are given the investigating Role's statement instead. What remains is an instruction that survives that restatement. Against it: the readiness Intent is promoted only on an eval set, the model holds no credential that can write, and no implementing Run starts before the Maintainer has approved the contract (I-29). The opening Run can still push a contract and open a draft | -| With any App that can write checks, the Controller's among them, post a passing check under a required Lens's name, or a passing one after that Lens's failure | A required Lens is read only from its own App, by id, and every check it reports there is read: another App's check leaves the Lens unreadable, before any failure, and any failure fails it. The status on the item has one name, which can never be a Lens's or its check's (§3.3). A Lens GitHub Actions runs is pinned to its workflow file as well, since Actions is one App for every job: a job a pull request adds in a file of its own under `verify`'s or the secret scan's name leaves the Lens unreadable, and changing the Lens's own file needs a change to `.github/`, a protected path that only the Maintainer authors (decisions 0004, 0005). A remaining risk, unverified: an upload to code scanning with CodeQL's tool and category, by anyone with `security_events: write`, might replace the CodeQL verdict (P-36) | +| With any App that can write checks, the Controller's among them, post a passing check under a required Lens's name, or a passing one after that Lens's failure | A required Lens is read only from its own App, by id, and every check it reports there is read: another App's check leaves the Lens unreadable, before any failure, and any failure fails it. The status on the item has one name, which can never be a Lens's or its check's (§3.3). A Lens GitHub Actions runs is pinned to the path of its workflow file as well, since Actions is one App for every job: a job a pull request adds in a file of its own under `verify`'s or the secret scan's name leaves the Lens unreadable. The pin holds the path, not the file's content or what it runs: a pull request from a branch of this repo, or a dependency bot's security update, can change the file in place, or the Makefile, tools, tests and `.gitleaks.toml` the Lenses run, and nothing in the Kernel stops it yet: "Kernel: a change to the Factory's own rules or a Lens's inputs needs a person" (issue 113). No Factory App holds `workflows`, so none can push there. A remaining risk, unverified: an upload to code scanning with CodeQL's tool and category, by anyone with `security_events: write`, might replace the CodeQL verdict (P-36) | | Steal what the relay holds | It holds a webhook secret and nothing that can act on the Factory repo. The worst a thief can do is cause passes (§7.3) | | Forge a fault report from inside a Run, to pause the Factory | A pause needs agreeing faults from Runs on different items, and `apply` and the poller class the faults they see themselves. The worst case is delay, bounded by the probe Run | | Replace the commit bundle from inside `verify` | The bundle is taken by identifier and digest (§3.6) | @@ -1926,9 +1942,9 @@ it observed as a recorded fixture, which is what the simulator's fake GitHub is | P-22 | How soon after it is started does a Run appear in the list of run records? | Soon enough that a pass never admits the same Run twice; the item's lane and I-21 cover the gap | | P-32 | Can an issue's author's permission be read in the listing of open issues, and who added and who removed each label, with the time, in the full read? | Yes to both, within the budget P-8 measures | | P-34 | Can the time an issue's title or body last changed be read, apart from the time anything on it changed; and each reopening, with who did it? | Yes to both | -| P-35 | Can the workflow file of each check run on a head be read, as the `path` of its check suite's workflow run, with the permissions the reader and the Controller hold, in the one read of the facts; and does a job a pull request adds in a file of its own, under a required Lens's check name, report under GitHub Actions with that file's path? | Yes to both, within the budget P-8 measures | -| P-36 | Can an upload to code scanning under CodeQL's tool and category, on a pull request's head, by an identity holding `security_events: write` but not CodeQL's own analysis, change the conclusion of the check run `CodeQL` that GitHub Advanced Security reports? | It cannot. If it can, no identity but CodeQL's own analysis holds `security_events: write`, and the permission tables of the Apps and the workflows are checked to say so | | P-33 | Can the issue a pull request closes be read from a draft, an open, a merged and a closed-unmerged pull request alike, with its opening actor; and does a pull request to a branch that is not the default one carry it? Can who last changed the draft flag be read? | Yes to the first and last. If a closed pull request loses the reference, closes are counted by the work's branch name. If a non-default base carries none, the issue Lifecycle serves the default branch only. If who changed the draft flag cannot be read, a linked pull request that has once left draft and is a draft again is treated as taken in hand | +| P-35 | Where does each check run's workflow file come from, and in what form? Can it be read as the `path` of the check run's check suite's workflow run, with the permissions the reader and the Controller hold, in the one read of the facts, and does that `path` ever carry a ref suffix or take a `dynamic/` form, which the port passes on verbatim, never as null? Does the check run `CodeQL` that GitHub Advanced Security reports have a workflow run at all? Where does a check run created through the API with `GITHUB_TOKEN` and `checks: write` land, in which check suite and under which workflow path, and can that token update a check run of another workflow's run? And does a job a pull request adds in a file of its own, under a required Lens's check name, report under GitHub Actions with that file's path? | Each answered and recorded; the path readable within the budget P-8 measures; CodeQL's check with no workflow run; a check created with `GITHUB_TOKEN` under its own workflow's path and unable to change another's | +| P-36 | Can an upload to code scanning under CodeQL's tool and category, on a pull request's head, by an identity holding `security_events: write` but not CodeQL's own analysis, change the conclusion of the check run `CodeQL` that GitHub Advanced Security reports? | It cannot. If it can, no identity but CodeQL's own analysis may hold `security_events: write`: for the Factory's Apps `tools/check_apps.py` holds that, and for the live workflows the workflow lint, which refuses `security-events: write` on any job; for a third-party App installed on a served repo it waits on the readiness check (§3.12) | ### 6.1 The order to run them in diff --git a/docs/decisions/0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md b/docs/decisions/0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md index 064ef860..228b193f 100644 --- a/docs/decisions/0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md +++ b/docs/decisions/0018-a-lens-is-read-from-its-own-app-and-workflow-and-only-what-gates-holds-a-merge.md @@ -19,9 +19,10 @@ one at any severity. One that does not gate, a `minor` or `info` Finding of any `security`, is an annotation on its Lens's check run and has no Disposition: it closes when its Lens stops reporting it, as a `mechanical` Finding does. A required deterministic Lens gates through its check's conclusion. It is read only from the App the configuration names for it, by the exact -name of its check run, and, where GitHub Actions runs it, only from the workflow file the -configuration names: a check under its name from any other App, from another workflow file, or with -no file read, leaves it unreadable, for the Maintainer, before any Lens's failure. A Lens no workflow +name of its check run, and, where GitHub Actions runs it, only from the path of the workflow file +the configuration names: a check under its name from any other App, from another path, or with no +path read, leaves it unreadable, for the Maintainer, before any Lens's failure. The pin is to the +path, not to what the file at that path says. A Lens no workflow runs names none, and is read only from a check that names none. Every check its own App and file report under that name is read, and it passes only when every one passes. A reviewing Role declares a rubric, which is a floor. A change that has not converged within its bound is placed on @@ -45,13 +46,23 @@ the deterministic Lenses alone, they are its only judge. GitHub Actions is one App for every job in the repo, so a pin to the App alone binds `verify` and the secret scan to any Actions job under their check's name, one a pull request adds in a workflow file -of its own among them. The workflow file is the narrowest thing GitHub records of what ran a check, -and only the Maintainer authors a change under `.github/` (decisions 0004, 0005), so a job in another -file is not the Lens, and a change to the Lens's own file is the Maintainer's. Pinning each required -check to its App in the `main` ruleset, not only `verify`, is the Maintainer's, in the repo's settings, -with "The Factory repo's rulesets hold the Release tags and each required check's App" (issue 62); the -Kernel's pin does not wait on it. Whether an upload to code scanning under CodeQL's tool and category -can replace the CodeQL verdict is a platform fact not yet tried: the fake GitHub assumes it cannot. +of its own among them. The path of the workflow file is the narrowest thing GitHub records of what +ran a check, so a job from another path is not the Lens. + +The pin holds the path and nothing more. A pull request from a branch of this repo can change the +file at that path, and its check still reports from it; so can a dependency bot's security update, +which edits the pins in workflow files and, as class `dependency`, has no model review. `verify` also +runs the pull request's own Makefile, tools and tests, and the secret scan reads its own +`.gitleaks.toml`. What holds `.github/` today is: GitHub refusing an App's push there without the +`workflows` permission, which no Factory App holds (`config/apps.json`, `tools/check_apps.py`); the +admission of a fork's pull request by a person, after its last push (I-36); a Role's `apply`, waiting +on `harness/`; and the session hooks, locally. The `main` ruleset requires no review, and pins only +`verify` to its App; pinning each required check is the Maintainer's, in the repo's settings, with +"The Factory repo's rulesets hold the Release tags and each required check's App" (issue 62). That a +change to a Lens's inputs, or to the Factory's own rules, needs a person is +"Kernel: a change to the Factory's own rules or a Lens's inputs needs a person" (issue 113). Whether +an upload to code scanning under CodeQL's tool and category can replace the CodeQL verdict is a +platform fact not yet tried: the fake GitHub assumes it cannot. ## Boundaries @@ -71,11 +82,14 @@ can replace the CodeQL verdict is a platform fact not yet tried: the fake GitHub | The configuration cannot make a check with no verdict pass | `tests/test_tools.py::test_only_success_can_pass_a_check`, `examples/invalid/config.a-neutral-check-passes.json`, `examples/invalid/config.conclusion-mapped-to-nothing.json` | | A required deterministic Lens is read only from the App the configuration names for it, by an id that is a whole number: a check with its name from another App, or with no such id, leaves it unreadable, before any Lens's failure or a conflict, on a queued item too, says which App, and moves nothing | `tests/test_decide.py::test_a_check_from_another_app_is_unreadable_for_its_lens`, `tests/test_decide.py::test_a_check_from_another_app_or_a_failure_beside_a_pass_takes_it_out_of_the_queue`, `tests/test_decide.py::test_only_a_check_from_its_lens_app_moves_an_item`, `examples/invalid/facts.a-check-without-its-app.json`, `examples/invalid/config.a-required-lens-with-no-app.json` | | Every check a Lens's own App and workflow file report under its name is read, never the last alone: any failure fails it, and it passes only when every one passes | `tests/test_decide.py::test_every_check_a_lens_reports_from_its_own_app_is_read` | -| A required deterministic Lens that GitHub Actions runs is read only from the workflow file the configuration names for it, exactly: a check with its name from another file of its App, or with no file read, leaves it unreadable, before any Lens's failure or a conflict, on a queued item too, says which file, and moves nothing. One from another App is shown as another App's, whatever file it names | `tests/test_decide.py::test_a_check_from_another_workflow_is_unreadable_for_its_lens`, `tests/test_decide.py::test_a_check_from_another_workflow_takes_it_out_of_the_queue`, `tests/test_decide.py::test_only_a_check_from_its_lens_workflow_moves_an_item`, `examples/invalid/facts.a-check-without-its-workflow.json` | +| A required deterministic Lens that GitHub Actions runs is read only from the path of the workflow file the configuration names for it, exactly: a check with its name from another path of its App, or with no path read, leaves it unreadable, before any Lens's failure or a conflict, on a queued item too, says which path, and moves nothing. One from another App is shown as another App's, whatever path it names | `tests/test_decide.py::test_a_check_from_another_workflow_is_unreadable_for_its_lens`, `tests/test_decide.py::test_a_check_from_another_workflow_takes_it_out_of_the_queue`, `tests/test_decide.py::test_only_a_check_from_its_lens_workflow_moves_an_item`, `examples/invalid/facts.a-check-without-its-workflow.json` | | A Lens no workflow runs names none, and is read only from a check that names none | `tests/test_decide.py::test_a_check_from_another_workflow_is_unreadable_for_its_lens`, `tests/test_decide.py::test_only_a_check_from_its_lens_workflow_moves_an_item` | -| Each deterministic Lens names its workflow file, or null where no workflow runs it, and the file it names exists and runs a job under the Lens's check name | `examples/invalid/config.a-required-lens-with-no-workflow.json`, `tests/test_tools.py::test_a_lens_read_from_a_workflow_that_does_not_run_its_check_is_caught` | -| The port gives each check run GitHub Actions reports with the path of its check suite's workflow run, and null only for a check no workflow ran | waiting on `adapters/` | -| A Role's change to a Lens's own workflow file, a change under `.github/`, is refused unless a person approved it | waiting on `harness/` | +| Each deterministic Lens names its workflow file, and one whose App is GitHub Actions names one; one of another App names none | `examples/invalid/config.a-required-lens-with-no-workflow.json`, `tests/test_tools.py::test_a_lens_run_by_actions_names_its_workflow_and_no_other_does` | +| The file a Lens names exists, runs on both `pull_request` and `merge_group`, and has a job whose own `name:`, or whose id where it has none, is the Lens's check, with no `strategy.matrix`, job-level `if:` or `uses:`; a workflow's or a step's `name:` names no check | `tests/test_tools.py::test_a_lens_read_from_a_workflow_that_does_not_run_its_check_is_caught`, `tests/test_tools.py::test_only_a_jobs_own_name_or_its_id_names_a_lens_check`, `tests/test_tools.py::test_a_lens_job_that_would_never_report_under_its_name_is_caught` | +| A Decision shows a workflow path only in the form a Lens's workflow takes, `.github/workflows/` and a name of letters, digits, `.`, `_` and `-` ending .yml or .yaml, no longer than `max_path_length`; any other, dynamic paths among them, is shown `unreadable` | `tests/test_decide.py::test_a_workflow_path_is_shown_only_in_its_form` | +| No Factory App holds `workflows` or `security_events`, and no workflow grants a job `security-events: write` | `tests/test_tools.py::test_no_app_may_change_a_workflow_or_upload_to_code_scanning`, `examples/workflows/invalid/permissions.security-events-on-a-job.yml` | +| The port gives each check run GitHub Actions reports with the path of its check suite's workflow run as GitHub gives it, a dynamic one verbatim, and null only for a check no workflow ran | waiting on `adapters/` | +| A Role's change to a Lens's own workflow file, a change under `.github/`, is refused unless a person approved it; a change by anyone to a Lens's inputs needs a person, "Kernel: a change to the Factory's own rules or a Lens's inputs needs a person" (issue 113) | waiting on `harness/` | | Whether an upload to code scanning under CodeQL's tool and category, by an identity holding `security_events: write`, can replace the CodeQL verdict is a platform fact, unverified until trial P-36 grounds it | `sim/grounding.py`, `tools/check_trials.py` | | Each deterministic Lens names its check run's exact name and its App, and no Lens or check is named twice | `examples/invalid/config.a-required-lens-with-no-check.json`, `tests/test_tools.py::test_a_lens_or_a_check_named_twice_is_refused` | | The status on the item has one name, and it is no required Lens's or its check's, in any case | `tests/test_tools.py::test_the_status_on_the_item_never_carries_a_required_lens_name` | diff --git a/docs/standards/workflows.md b/docs/standards/workflows.md index 1c90b03f..16586449 100644 --- a/docs/standards/workflows.md +++ b/docs/standards/workflows.md @@ -61,10 +61,10 @@ jobs: contents: read # then only what this job needs ``` -No personal access token, no `secrets: inherit`; every checkout sets `persist-credentials: false`. +No personal access token, no `secrets: inherit`; every checkout sets `persist-credentials: false`. No job is granted `security-events: write`: with it, an upload to code scanning could stand for CodeQL's verdict. Org standard: ci-standards.md, "Permissions Policy". Refusing a personal token and inherited secrets departs from github-settings.md, "Required secrets (all repos)": decision 0014. -Held by: `examples/workflows/invalid/permissions.none-declared.yml`, `examples/workflows/invalid/token.a-persons-token.yml`, `examples/workflows/invalid/credentials.a-token-left-on-disk.yml` +Held by: `examples/workflows/invalid/permissions.none-declared.yml`, `examples/workflows/invalid/token.a-persons-token.yml`, `examples/workflows/invalid/credentials.a-token-left-on-disk.yml`, `examples/workflows/invalid/permissions.security-events-on-a-job.yml` ## Text from outside is never a command diff --git a/examples/invalid/config.a-change-by-a-person-skips-the-model-review.json b/examples/invalid/config.a-change-by-a-person-skips-the-model-review.json index 9bda4f38..c95e12a3 100644 --- a/examples/invalid/config.a-change-by-a-person-skips-the-model-review.json +++ b/examples/invalid/config.a-change-by-a-person-skips-the-model-review.json @@ -57,6 +57,7 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 } } diff --git a/examples/invalid/config.a-neutral-check-passes.json b/examples/invalid/config.a-neutral-check-passes.json index 099be17a..8c79138d 100644 --- a/examples/invalid/config.a-neutral-check-passes.json +++ b/examples/invalid/config.a-neutral-check-passes.json @@ -56,6 +56,7 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 } } diff --git a/examples/invalid/config.a-required-lens-with-no-app.json b/examples/invalid/config.a-required-lens-with-no-app.json index 4e4f12c2..b16a3c98 100644 --- a/examples/invalid/config.a-required-lens-with-no-app.json +++ b/examples/invalid/config.a-required-lens-with-no-app.json @@ -55,6 +55,7 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 } } diff --git a/examples/invalid/config.a-required-lens-with-no-check.json b/examples/invalid/config.a-required-lens-with-no-check.json index 02aaaa1e..dfb14ea9 100644 --- a/examples/invalid/config.a-required-lens-with-no-check.json +++ b/examples/invalid/config.a-required-lens-with-no-check.json @@ -55,6 +55,7 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 } } diff --git a/examples/invalid/config.a-required-lens-with-no-workflow.json b/examples/invalid/config.a-required-lens-with-no-workflow.json index 5b89cf7b..16a59435 100644 --- a/examples/invalid/config.a-required-lens-with-no-workflow.json +++ b/examples/invalid/config.a-required-lens-with-no-workflow.json @@ -55,6 +55,7 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 } } diff --git a/examples/invalid/config.a-staleness-limit-copied-from-the-lifecycle.json b/examples/invalid/config.a-staleness-limit-copied-from-the-lifecycle.json index ecdbf49c..2b7660b7 100644 --- a/examples/invalid/config.a-staleness-limit-copied-from-the-lifecycle.json +++ b/examples/invalid/config.a-staleness-limit-copied-from-the-lifecycle.json @@ -56,7 +56,8 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 }, "stale_after_minutes": { "draft": 1440, diff --git a/examples/invalid/config.conclusion-mapped-to-nothing.json b/examples/invalid/config.conclusion-mapped-to-nothing.json index 3e1e6dbb..ea75cc17 100644 --- a/examples/invalid/config.conclusion-mapped-to-nothing.json +++ b/examples/invalid/config.conclusion-mapped-to-nothing.json @@ -56,6 +56,7 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 } } diff --git a/examples/invalid/config.no-deterministic-lens.json b/examples/invalid/config.no-deterministic-lens.json index d362fe45..e65629b7 100644 --- a/examples/invalid/config.no-deterministic-lens.json +++ b/examples/invalid/config.no-deterministic-lens.json @@ -37,6 +37,7 @@ "max_unmerged_closes": 1, "org_max_active_runs": 4, "max_diff_lines": 4000, - "max_acts_per_item": 6 + "max_acts_per_item": 6, + "max_path_length": 255 } } diff --git a/examples/workflows/invalid/permissions.security-events-on-a-job.yml b/examples/workflows/invalid/permissions.security-events-on-a-job.yml new file mode 100644 index 00000000..a844581e --- /dev/null +++ b/examples/workflows/invalid/permissions.security-events-on-a-job.yml @@ -0,0 +1,22 @@ +# Must be rejected. With security-events: write a job can upload to code scanning under CodeQL's tool and +# category, and so might stand for CodeQL's verdict (design, trial P-36). +name: factory +on: + pull_request: + merge_group: +permissions: {} +jobs: + verify: + name: verify + runs-on: ubuntu-24.04 + timeout-minutes: 30 + permissions: + contents: read + security-events: write + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + fetch-depth: 0 + - run: make verify + - run: python3 -m tools.mutants diff --git a/kernel/derive.py b/kernel/derive.py index 2a4a7947..b0b0a8cd 100644 --- a/kernel/derive.py +++ b/kernel/derive.py @@ -89,6 +89,21 @@ def from_its_workflow(check: dict, lens: dict) -> bool: return "workflow" in check and check["workflow"] == lens["workflow"] +def shown_path(path, config: dict) -> str: + """`path` as a Decision's evidence shows it: as it is only when it is in the form a Lens's workflow takes and no + longer than the configuration's limit, and otherwise `unreadable`. A path is text from outside, so a look-alike of + a Lens's own file, or a character that turns the text round, never reaches a Decision.""" + if not (isinstance(path, str) and path.startswith(t.WORKFLOW_DIR) + and len(path) <= config["limits"]["max_path_length"]): + return "unreadable" + name = path[len(t.WORKFLOW_DIR):] + for ending in t.WORKFLOW_ENDINGS: + stem = name[:-len(ending)] + if name.endswith(ending) and stem != "" and all(c in t.FILE_NAME_CHARS for c in stem): + return path + return "unreadable" + + def reports_for(check: dict, lens: dict) -> bool: """Whether `check` reports for the required Lens `lens`: its check's name, from the App and the workflow file the configuration names.""" @@ -103,8 +118,8 @@ def foreign_checks(checks: list, config: dict) -> list: ruleset pins `verify` to its App, and the Kernel pins every required Lens to its App and its file. Such a check makes its Lens unreadable whatever it, or the Lens's own checks, conclude (I-13). One from another App is given as `:from_app:`, the id `unreadable` when it is not a whole number; one from the Lens's App and another - file as `:from_workflow:`, the path `unreadable` when no file was read. So the Maintainer can tell it - from a check that gave no verdict. + file as `:from_workflow:`, the path `unreadable` when no file was read or it is not in the form a + Lens's workflow takes (shown_path). So the Maintainer can tell it from a check that gave no verdict. """ out = [] for lens in config["required_lenses"]["deterministic"]: @@ -115,8 +130,7 @@ def foreign_checks(checks: list, config: dict) -> list: app = c.get("app_id") said = "%s:from_app:%s" % (lens["lens"], app if type(app) is int else "unreadable") else: - path = c.get("workflow") - said = "%s:from_workflow:%s" % (lens["lens"], path if isinstance(path, str) else "unreadable") + said = "%s:from_workflow:%s" % (lens["lens"], shown_path(c.get("workflow"), config)) if said not in out: out.append(said) return out diff --git a/kernel/terms.py b/kernel/terms.py index 9fa7272a..f7b0579e 100644 --- a/kernel/terms.py +++ b/kernel/terms.py @@ -71,6 +71,11 @@ CRITERION_PREFIX = "AC-" CONTENT_PREFIX = "sha256:" HEX_DIGITS = "0123456789abcdef" +# The one form in which a Decision's evidence shows a workflow file's path, the form a Lens's workflow takes in +# schemas/config.schema.json: this directory, a name of these characters alone, and one of these endings. +WORKFLOW_DIR = ".github/workflows/" +WORKFLOW_ENDINGS = (".yml", ".yaml") +FILE_NAME_CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789._-" # Classes of pull request whose author is an App: an agent's, or a dependency update's. BOT_CLASSES = ("agent", "dependency") diff --git a/schemas/config.schema.json b/schemas/config.schema.json index 6df6413c..8e6da428 100644 --- a/schemas/config.schema.json +++ b/schemas/config.schema.json @@ -62,12 +62,12 @@ "minimum": 1 }, "workflow": { - "description": "For a Lens GitHub Actions runs, the path of its workflow file, which only the Maintainer changes; null for a Lens no workflow runs. GitHub Actions is one App for every job in the repo, one a pull request adds among them, so a check under the Lens's name from any other file is not the Lens's", + "description": "For a Lens GitHub Actions runs, the path of its workflow file; null for a Lens no workflow runs. GitHub Actions is one App for every job in the repo, one a pull request adds among them, so a check under the Lens's name from any other path is not the Lens's. The pin is to the path, not to what the file says. tools/check_workflows.py refuses null for a Lens whose App is GitHub Actions, and holds the file to running the Lens's check", "type": [ "string", "null" ], - "pattern": "^\\.github/workflows/[^/]+\\.ya?ml$" + "pattern": "^\\.github/workflows/[A-Za-z0-9._-]+\\.ya?ml$" } } } @@ -123,7 +123,8 @@ "max_unmerged_closes", "org_max_active_runs", "max_diff_lines", - "max_acts_per_item" + "max_acts_per_item", + "max_path_length" ], "properties": { "max_attempts": { @@ -153,6 +154,11 @@ "max_acts_per_item": { "type": "integer", "minimum": 1 + }, + "max_path_length": { + "description": "The longest path of a workflow file a Decision's evidence shows. A longer one, or one not in the form a Lens's workflow takes, is shown as unreadable", + "type": "integer", + "minimum": 1 } } } diff --git a/schemas/facts.schema.json b/schemas/facts.schema.json index da0c0285..0bf011f6 100644 --- a/schemas/facts.schema.json +++ b/schemas/facts.schema.json @@ -311,7 +311,7 @@ "minimum": 1 }, "workflow": { - "description": "The path of the workflow file that ran the check, as GitHub gives it for the workflow run of the check run's check suite (its `path`, such as .github/workflows/verify.yml); null for a check no workflow ran, as one GitHub Advanced Security reports. GitHub Actions is one App for every job in the repo, so a required Lens that it runs is read only from the file the configuration names for it", + "description": "The path of the workflow file that ran the check, as GitHub gives it for the workflow run of the check run's check suite (its `path`, such as .github/workflows/verify.yml), passed on verbatim, a dynamic/ one too; null only for a check no workflow ran, as one GitHub Advanced Security reports. GitHub Actions is one App for every job in the repo, so a required Lens that it runs is read only from the path the configuration names for it. The path is text from outside: a Decision shows it only in the configuration's form", "type": [ "string", "null" diff --git a/sim/grounding.py b/sim/grounding.py index 17b4f982..c48e0690 100644 --- a/sim/grounding.py +++ b/sim/grounding.py @@ -28,8 +28,9 @@ ("any_app_posts_a_check_under_any_name", "Any App with checks: write can post a check run under any name, a " "required Lens's too, and it stays on that head: a check run cannot be deleted", (), None), ("a_check_names_the_workflow_that_ran_it", "Every check run GitHub Actions reports is read with the path of the " - "workflow file that ran it, the path of its check suite's workflow run, with the permissions the reader and the " - "Controller hold; a check run no workflow ran, as GitHub Advanced Security's, is read with none", ("P-35",), None), + "workflow file that ran it, the path of its check suite's workflow run as GitHub gives it, a dynamic/ one " + "verbatim, with the permissions the reader and the Controller hold; a check run no workflow ran, as GitHub " + "Advanced Security's, is read with none", ("P-35",), None), ("actions_runs_a_job_under_any_name", "GitHub Actions reports every job in the repo as one App, 15368, one a pull " "request adds in a workflow file of its own under a required Lens's check name among them, and its check run " "names that file", ("P-35",), None), diff --git a/tests/test_decide.py b/tests/test_decide.py index 90b26f99..ef5a3997 100644 --- a/tests/test_decide.py +++ b/tests/test_decide.py @@ -4,6 +4,7 @@ import datetime import json import random +import re import unittest from kernel import derive @@ -54,6 +55,17 @@ def dismissed(rv: dict, by, was: str = "changes_requested") -> dict: return dict(rv, state="dismissed", dismissed_by=dict(by) if isinstance(by, dict) else by, dismissed_from=was) +# The one form in which a Decision's evidence shows a workflow file's path: the configuration's own, and no longer than +# its limit. Any other text is shown as `unreadable`, so a path cannot carry a look-alike or a control character into it. +WORKFLOW_FORM = re.compile(r"^\.github/workflows/[A-Za-z0-9._-]+\.ya?ml$") + + +def shown(path) -> str: + """How a Decision's evidence shows the workflow file a check names.""" + ok = isinstance(path, str) and WORKFLOW_FORM.match(path) and not path.endswith("\n") + return path if ok and len(path) <= LIMITS["max_path_length"] else "unreadable" + + def with_check(i: int, conclusion: str) -> list: """Every required check passing but the i-th, which concludes as GitHub says, or is not there ("absent").""" if conclusion == "absent": @@ -1147,8 +1159,7 @@ def test_a_check_from_another_workflow_is_unreadable_for_its_lens(self): # I-13 # The same file in another case, or from another directory, is another file; and null is no file read. others += [check["workflow"].upper(), "x/" + check["workflow"], None] for path in others + ["absent"]: - said = "%s:from_workflow:%s" % (lens["lens"], path if isinstance(path, str) and path != "absent" - else "unreadable") + said = "%s:from_workflow:%s" % (lens["lens"], shown(None if path == "absent" else path)) for conclusion in ("success", "failure"): impostor = dict(check, workflow=path, conclusion=conclusion) if path == "absent": @@ -1192,6 +1203,33 @@ def test_a_check_from_another_workflow_is_unreadable_for_its_lens(self): # I-13 other = dict(actions, name="template-drift", workflow=OTHER_WORKFLOW, conclusion="failure") self.assertEqual(go(facts(checks=GREEN + [other]), REVIEWED)["action"], "issue_approval") + def test_a_workflow_path_is_shown_only_in_its_form(self): # I-13 + # The evidence shows the path of another workflow file only in the form the configuration names one in, and no + # longer than the configuration's limit: anything else could carry text from outside into a Decision, a + # look-alike of a Lens's own file or a control character that turns the text round, and is shown `unreadable`. + # Whatever it shows, the check is not the Lens's. + actions = next(c for c in GREEN if isinstance(c["workflow"], str)) + lens = LENS_OF[actions["name"]] + limit = LIMITS["max_path_length"] + longest = ".github/workflows/" + "a" * (limit - len(".github/workflows/.yml")) + ".yml" + self.assertEqual(len(longest), limit) + for path in (OTHER_WORKFLOW, ".github/workflows/x.yaml", ".github/workflows/a_b-c.1.yml", longest): + d = go(facts(checks=GREEN + [dict(actions, workflow=path)]), REVIEWED) + self.assertEqual((d["reason"], d["evidence"]), ("check_unreadable", ["%s:from_workflow:%s" % (lens, path)]), + path) + unshown = ["", " ", ".github/workflows/verify .yml", ".github/workflows/v\u0435rify.yml", + "\u202e.github/workflows/verify.yml", ".github/workflows/verify.yml\u200b", + ".github/workflows/" + "a" * 5000 + ".yml", longest[:-4] + "a.yml", + "dynamic/github-code-scanning/codeql", "dynamic/workflows/x.yml", ".github/workflows_a.yml", + ".github/workflows/sub/x.yml", "../.github/workflows/x.yml", + ".github/workflows/x.yml\n", ".github/workflows/.yml", ".github/workflows/x.YML", + ".github/workflows/x.json", "x/" + actions["workflow"], actions["workflow"].upper(), 1, [], {}] + for path in unshown: + self.assertEqual(shown(path), "unreadable", path) + d = go(facts(checks=GREEN + [dict(actions, workflow=path)]), REVIEWED) + self.assertEqual((d["state"], d["reason"], d["evidence"], d["outcome"]), + ("verifying", "check_unreadable", ["%s:from_workflow:unreadable" % lens], "defer"), path) + def test_every_check_a_lens_reports_from_its_own_app_is_read(self): # I-13, I-27 # GitHub can hold several checks with one name from one App: two workflows with the same job name, say. Each # is read, never the last alone. Any that fails fails the Lens; failing none, any that gave no verdict leaves diff --git a/tests/test_simulator.py b/tests/test_simulator.py index 69d6602e..6604de8f 100644 --- a/tests/test_simulator.py +++ b/tests/test_simulator.py @@ -286,18 +286,26 @@ def test_a_dequeue_at_the_act_ceiling_is_the_one_act_past_it(self): # I-8 def test_the_seeds_take_a_pull_request_out_of_the_queue_for_each_cause(self): # The faults while queued are reached: a dismissed approval, an open gate and a Hold each take one out. # So is another App's passing check under the name of a Lens that failed, which no Run or act may follow, and - # one from another workflow file of the Lens's own App. - causes, impostors, workflows = set(), 0, 0 - for seed in SEEDS: - w = World(seed) - for _ in range(250): - w.step() - causes |= {line.split("(")[-1].rstrip(")") for line in w.trace if "apply: dequeued" in line} - impostors += sum("another App posted" in line for line in w.trace) - workflows += sum("another workflow posted" in line for line in w.trace) + # one from another workflow file of the Lens's own App, which the Kernel shows as from that file. + causes, impostors, workflows, shown = set(), 0, 0, set() + + def recording(f, r, c, cfg, now): + d = decide(f, r, c, cfg, now) + shown.update(e.split(":")[1] for e in d["evidence"] if e.count(":") >= 2) + return d + + with mock.patch.object(world, "decide", recording): + for seed in SEEDS: + w = World(seed) + for _ in range(250): + w.step() + causes |= {line.split("(")[-1].rstrip(")") for line in w.trace if "apply: dequeued" in line} + impostors += sum("another App posted" in line for line in w.trace) + workflows += sum("another workflow posted" in line for line in w.trace) self.assertLessEqual({"approval_lost", "gate_open", "human_hold"}, causes) self.assertTrue(impostors) self.assertTrue(workflows) + self.assertLessEqual({"from_app", "from_workflow"}, shown) def test_the_seeds_serve_items_with_an_acceptance_contract_and_without(self): # I-31 # Some worlds carry a contract, and its evidence goes missing for a while, fails, is given twice and disagrees, diff --git a/tests/test_tools.py b/tests/test_tools.py index b60c9bba..65726356 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -492,6 +492,18 @@ def check(self) -> list: facts = (ROOT / "schemas" / "facts.schema.json").read_text() return check_apps.run(tree({"config/apps.json": json.dumps(self.table), "schemas/facts.schema.json": facts})) + def test_no_app_may_change_a_workflow_or_upload_to_code_scanning(self): + # GitHub refuses an App's push under .github/workflows/ without `workflows`, and with `security_events: write` + # an App could upload to code scanning under CodeQL's tool and category. No Factory App holds either, at any + # level, whatever need it names. + self.assertEqual(self.check(), []) + for name in ("workflows", "security_events"): + for level in ("read", "write"): + self.setUp() + self.apps["controller"]["permissions"][name] = {"level": level, "needs": ["read:facts.checks"]} + said = "controller: holds %s; no Factory App may" % name + self.assertTrue(caught(self.check(), said), (name, level)) + def test_a_permission_nothing_needs_is_caught(self): self.apps["controller"]["permissions"]["checks"]["needs"] = [] del self.apps["reader"]["permissions"]["issues"]["needs"] @@ -935,6 +947,58 @@ def test_a_lens_read_from_a_workflow_that_does_not_run_its_check_is_caught(self) root = clone({lens["workflow"]: renamed}) self.assertTrue(caught(check_workflows.run(root), "runs no job named %r" % lens["check"]), lens["lens"]) + def lens_problems(self, text: str, lens: str = "verify") -> list: + """The problems check_workflows finds when the Lens `lens` is read from a workflow file holding `text`.""" + defaults = json.loads((ROOT / "config" / "defaults.json").read_text()) + lenses = [dict(x, workflow=".github/workflows/lens.yml") if x["lens"] == lens else x + for x in defaults["required_lenses"]["deterministic"]] + cfg = dict(defaults, required_lenses=dict(defaults["required_lenses"], deterministic=lenses)) + root = clone({"config/defaults.json": json.dumps(cfg, indent=2) + "\n", ".github/workflows/lens.yml": text}) + return [p for p in check_workflows.run(root) if "the Lens %s" % lens in p] + + def test_only_a_jobs_own_name_or_its_id_names_a_lens_check(self): + # GitHub names a job's check run by the job's own name, or by its id where it has none. A workflow's name, or a + # step's, names no check run. + self.assertEqual(self.lens_problems(GOOD), []) + self.assertEqual(self.lens_problems(GOOD.replace(" name: verify\n", "")), []) + elsewhere = GOOD.replace("name: factory\n", "name: verify\n").replace(" verify:\n name: verify\n", " build:\n") + elsewhere = elsewhere.replace(" - run: make verify\n", " - name: verify\n run: make verify\n") + self.assertNotEqual(elsewhere, GOOD) + self.assertTrue(caught(self.lens_problems(elsewhere), "runs no job named 'verify'")) + renamed = GOOD.replace(" verify:\n name: verify\n", " verify:\n name: build\n") + self.assertTrue(caught(self.lens_problems(renamed), "runs no job named 'verify'")) + + def test_a_lens_job_that_would_never_report_under_its_name_is_caught(self): + # A matrix names each of its check runs after the job and its values; a job-level if: can skip the job; a job + # that calls a reusable workflow reports under the caller's and the callee's names; and a workflow that does not + # run on both pull_request and merge_group reports on no head, or not in the merge queue. Each leaves the Lens + # waiting for ever. + matrix = GOOD.replace(" timeout-minutes: 30\n", + " timeout-minutes: 30\n strategy:\n matrix:\n python:\n - 3.9\n") + skipped = GOOD.replace(" timeout-minutes: 30\n", + " timeout-minutes: 30\n if: github.event_name == 'pull_request'\n") + called = "\n".join(GOOD.split("\n")[:9]) + "\n uses: combsmith/factory/.github/workflows/v.yml@v1-stable\n" + cases = [(matrix, "has a strategy.matrix"), (skipped, "has a job-level if:"), + (called, "calls a reusable workflow with uses:"), + (GOOD.replace(" merge_group:\n", ""), "not on both pull_request and merge_group"), + (GOOD.replace(" pull_request:\n", ""), "not on both pull_request and merge_group")] + for text, said in cases: + self.assertNotEqual(text, GOOD, said) + self.assertTrue(caught(self.lens_problems(text), said), said) + + def test_a_lens_run_by_actions_names_its_workflow_and_no_other_does(self): + # GitHub Actions reports every job in the repo, so a Lens it runs must name its file; no other App runs one. + defaults = json.loads((ROOT / "config" / "defaults.json").read_text()) + for lens in defaults["required_lenses"]["deterministic"]: + if lens["app_id"] == check_workflows.ACTIONS_APP: + changed, said = dict(lens, workflow=None), "is run by GitHub Actions and names no workflow file" + else: + changed, said = dict(lens, workflow=".github/workflows/verify.yml"), "only GitHub Actions runs one" + lenses = [changed if x is lens else x for x in defaults["required_lenses"]["deterministic"]] + cfg = dict(defaults, required_lenses=dict(defaults["required_lenses"], deterministic=lenses)) + root = clone({"config/defaults.json": json.dumps(cfg, indent=2) + "\n"}) + self.assertTrue(caught(check_workflows.run(root), said), lens["lens"]) + def test_a_live_workflow_is_held_to_the_rules(self): bad = (ROOT / "examples" / "workflows" / "invalid" / "pin.a-tag-can-move.yml").read_text() problems = check_workflows.run(tree({".github/workflows/x.yml": bad, "examples/workflows/verify.yml": GOOD})) diff --git a/tools/check_apps.py b/tools/check_apps.py index 6c58ed97..8ec085ba 100644 --- a/tools/check_apps.py +++ b/tools/check_apps.py @@ -3,6 +3,7 @@ A need is a code. `effect:` is an Effect a Result requests and `apply` makes; `write:` is a write the Controller makes that is not an Effect; `read:` is something read. The table in `config/apps.json` maps each need to one GitHub permission and level, or says what it waits on. +No App holds `workflows` or `security_events` (NEVER). A write permission serves a read of the same permission too, since GitHub's write includes read. """ from __future__ import annotations @@ -16,6 +17,10 @@ PREFIX = "combsmith-sf-" WRITES = ("effect:", "write:") FACT = "read:facts." +# Permissions no Factory App holds, at any level. Without `workflows` GitHub refuses an App's push under +# .github/workflows/, so a Role cannot change what a required Lens runs; with `security_events` an App could upload to +# code scanning under CodeQL's tool and category, and might stand for CodeQL's verdict (design, trial P-36). +NEVER = ("workflows", "security_events") REPO = re.compile(r"^[a-z0-9][a-z0-9-]*/[a-z0-9][a-z0-9._-]*$") @@ -104,6 +109,8 @@ def run(root: pathlib.Path = ROOT) -> list: problems.append("%s: no permissions can be read; give the App an object of permissions" % app) continue for name, grant in sorted(permissions.items()): + if name in NEVER: + problems.append("%s: holds %s; no Factory App may, at any level: take it out" % (app, name)) problems += _grant(app, name, grant, needs) for need in grant.get("needs", []) if isinstance(grant, dict) else []: if isinstance(need, str) and need.startswith(WRITES): diff --git a/tools/check_workflows.py b/tools/check_workflows.py index 5cbfa434..7223d033 100644 --- a/tools/check_workflows.py +++ b/tools/check_workflows.py @@ -7,6 +7,12 @@ Each rule has at least one counter-example under examples/workflows/invalid/, named for the rule, that must be rejected for that rule and no other. A rule with no counter-example fails this check. + +It also holds the configuration's required Lenses to the workflows that run them (lens_workflows): a Lens whose App is +GitHub Actions names its workflow file and no other Lens names one, and the file it names exists, runs on both +pull_request and merge_group, and has a job whose own `name:`, or whose id where it has none, is the Lens's check, +with no `strategy.matrix`, no job-level `if:` and no `uses:`. Only `jobs..name` is read: a workflow's or a step's +`name:` names no check run. """ from __future__ import annotations @@ -23,7 +29,8 @@ "reusable workflows, to its channel tag; nothing local but a reusable workflow", "target": "a workflow runs only on pull_request, merge_group, push, schedule, workflow_dispatch or " "workflow_call, and never on pull_request_target", - "permissions": "permissions are declared at the top and grant nothing that can write; never read-all or write-all", + "permissions": "permissions are declared at the top and grant nothing that can write; never read-all or write-all; " + "and no job is granted security-events: write, with which an upload could stand for CodeQL's verdict", "expression": "an expression stands only as the whole of a value under env:, with: or secrets:, or in if:", "length": "a run: block is at most ten lines", "install": "a run: line is `make ` or `python3 -m ` and nothing else, so nothing is " @@ -37,6 +44,8 @@ "soft": "no step or job continues on error", } EVENTS = {"pull_request", "merge_group", "push", "schedule", "workflow_dispatch", "workflow_call"} +# The id GitHub gives the App GitHub Actions: it reports every job in every workflow of the repo. +ACTIONS_APP = 15368 # A workflow the org requires as it wrote it. It is exempt from the rules named and from no other, # and it is held to its text by a hash taken with the pins left out: to change it, change this. EXEMPT = { @@ -139,6 +148,9 @@ def under(*path): any(e["value"] not in ("read", "none") for e in granted) or \ any(e["key"] == "permissions" and e["value"] not in ("", "{}") for e in entries): found.append(("permissions", "no top-level permissions, one that can write, or read-all or write-all")) + if any(e["path"][-1:] == ["permissions"] and e["key"] == "security-events" and e["value"] == "write" + for e in entries): + found.append(("permissions", "a job is granted security-events: write")) for e in entries: key, value = e["key"], e["value"] @@ -193,29 +205,72 @@ def under(*path): return sorted(set(found)) +def lens_job_problems(text: str, check: str) -> list: + """Why a workflow holding `text` would never report the check run `check` on every head and in the merge queue. + + GitHub names a job's check run by the job's own `name:`, or by its id where it has none; a workflow's or a step's + `name:` names none. A job with a `strategy.matrix` reports under its name and its values, one with a job-level + `if:` can be skipped, and one that calls a reusable workflow with `uses:` reports under the caller's and the + callee's names. A workflow that does not run on both pull_request and merge_group reports on no head, or not in + the queue. Each would leave the Lens waiting for ever. + """ + entries, _ = read(text) + top = {e["key"]: e for e in entries if e["path"] == []} + events = {e["key"] for e in entries if e["path"] == ["on"]} + if top.get("on", {}).get("value"): + events.add(top["on"]["value"]) + problems = [] + if not {"pull_request", "merge_group"} <= events: + problems.append("runs on %s, not on both pull_request and merge_group, so its check would not report on every " + "head and in the queue" % (", ".join(sorted(events)) or "nothing")) + jobs = [e["key"] for e in entries if e["path"] == ["jobs"]] + named = [] + for job in jobs: + inside = {e["key"]: e for e in entries if e["path"] == ["jobs", job]} + if inside.get("name", {}).get("value", job) == check: + named.append((job, inside)) + if not named: + return problems + ["runs no job named %r: only a job's own name, or its id where it has none, names its check" + % check] + for job, inside in named: + matrix = [e for e in entries if e["path"] == ["jobs", job, "strategy"] and e["key"] == "matrix"] + for there, said in ((matrix, "has a strategy.matrix"), ("if" in inside, "has a job-level if:"), + ("uses" in inside, "calls a reusable workflow with uses:")): + if there: + problems.append("has the job %s, and it %s, so its check would never report under that name alone" + % (job, said)) + return problems + + def lens_workflows(root: pathlib.Path) -> list: - """Each required Lens GitHub Actions runs names a workflow file that exists and runs a job under its check's name. + """Each required Lens names the workflow file it is read from exactly where GitHub Actions runs it, and that file + reports its check on every head and in the merge queue (lens_job_problems). - The Kernel reads such a Lens only from that file (`config/defaults.json`). Gone, moved, or with the job renamed, - every pull request's check would be another file's or none, and the Lens unreadable on all of them. + The Kernel reads a Lens whose App is GitHub Actions only from the file `config/defaults.json` names for it, and + reads a Lens no workflow runs only from a check that names none. A Lens of GitHub Actions with no file could be + passed by any job in the repo; another App's with one would never be read. A file gone, moved, or whose job would + never report under the Lens's check name, would leave the Lens unreadable or waiting on every pull request. """ config = root / "config" / "defaults.json" if not config.is_file(): return [] problems = [] for lens in json.loads(config.read_text())["required_lenses"]["deterministic"]: - rel = lens.get("workflow") + rel, where = lens.get("workflow"), "config/defaults.json: the Lens %s" % lens.get("lens") + actions = lens.get("app_id") == ACTIONS_APP and type(lens.get("app_id")) is int + if actions and not isinstance(rel, str): + problems.append("%s is run by GitHub Actions and names no workflow file; name the file whose job runs it" + % where) + if not actions and rel is not None: + problems.append("%s names the workflow file %s, and only GitHub Actions runs one; make it null" % (where, rel)) if not isinstance(rel, str): continue path = root / rel if not path.is_file(): - problems.append("config/defaults.json: the Lens %s is read from %s, which does not exist" % (lens["lens"], rel)) + problems.append("%s is read from %s, which does not exist" % (where, rel)) continue - entries, _ = read(path.read_text(errors="replace")) - names = [e["value"] for e in entries if e["key"] == "name" and len(e["path"]) == 2 and e["path"][0] == "jobs"] - if lens["check"] not in names: - problems.append("config/defaults.json: the Lens %s is read from %s, which runs no job named %r" - % (lens["lens"], rel, lens["check"])) + problems += ["%s is read from %s, which %s" % (where, rel, why) + for why in lens_job_problems(path.read_text(errors="replace"), lens["check"])] return problems diff --git a/tools/mutants.py b/tools/mutants.py index d521e352..eaf969a6 100644 --- a/tools/mutants.py +++ b/tools/mutants.py @@ -225,10 +225,38 @@ def moved_after(rel: str, block: str, anchor: str) -> tuple: ("a Lens is read from another workflow file", "config/defaults.json", '"workflow": ".github/workflows/verify.yml"', '"workflow": ".github/workflows/ci.yml"'), ("a Lens's workflow file need not exist", "tools/check_workflows.py", - ' problems.append("config/defaults.json: the Lens %s is read from %s, which does not exist"' - ' % (lens["lens"], rel))\n', " pass\n"), + ' problems.append("%s is read from %s, which does not exist" % (where, rel))\n', " pass\n"), ("a Lens's workflow file need not run its check", "tools/check_workflows.py", - ' if lens["check"] not in names:\n', " if False:\n"), + " if not named:\n", " if False:\n"), + ("a job's id names its check though the job has a name", "tools/check_workflows.py", + ' if inside.get("name", {}).get("value", job) == check:\n', + ' if check in (job, inside.get("name", {}).get("value")):\n'), + ("a workflow's or a step's name names a Lens's check", "tools/check_workflows.py", + " named = []\n", ' named = [(check, {}) for e in entries if e["key"] == "name" and e["value"] == check]\n'), + ("a Lens's job may have a matrix", "tools/check_workflows.py", + '(matrix, "has a strategy.matrix"), ', ""), + ("a Lens's job may have a job-level if:", "tools/check_workflows.py", + '("if" in inside, "has a job-level if:"),', ""), + ("a Lens's job may call a reusable workflow", "tools/check_workflows.py", + '\n ("uses" in inside, "calls a reusable workflow with uses:")', ""), + ("a Lens's workflow need not run on merge_group", "tools/check_workflows.py", + ' if not {"pull_request", "merge_group"} <= events:\n', ' if not {"pull_request"} <= events:\n'), + ("a Lens's workflow need not run on pull_request", "tools/check_workflows.py", + ' if not {"pull_request", "merge_group"} <= events:\n', ' if not {"merge_group"} <= events:\n'), + ("a Lens of GitHub Actions may name no workflow file", "tools/check_workflows.py", + " if actions and not isinstance(rel, str):\n", " if False:\n"), + ("a Lens of another App may name a workflow file", "tools/check_workflows.py", + " if not actions and rel is not None:\n", " if False:\n"), + ("a job may be granted security-events: write", "tools/check_workflows.py", + 'e["key"] == "security-events" and e["value"] == "write"', "False"), + ("an App may hold a permission no Factory App may", "tools/check_apps.py", + " if name in NEVER:\n", " if False:\n"), + ("an App may hold workflows", "tools/check_apps.py", + 'NEVER = ("workflows", "security_events")', 'NEVER = ("security_events",)'), + ("an App may hold security_events", "tools/check_apps.py", + 'NEVER = ("workflows", "security_events")', 'NEVER = ("workflows",)'), + ("the impostor workflow posts from the Lens's own file", "sim/world.py", + '"workflow": IMPOSTOR_WORKFLOW,', '"workflow": WORKFLOWS[lens],'), ("a neutral check is read as passing", "config/defaults.json", '"neutral": "unreadable"', '"neutral": "passing"'), ("a neutral check is read as failing", "config/defaults.json", @@ -288,10 +316,20 @@ def moved_after(rel: str, block: str, anchor: str) -> tuple: ("an App id that is no whole number is shown as it is", "kernel/derive.py", 'app if type(app) is int else "unreadable")', "app)"), ("another workflow file's check is shown as another App's", "kernel/derive.py", - ' said = "%s:from_workflow:%s" % (lens["lens"], path if isinstance(path, str) else "unreadable")\n', + ' said = "%s:from_workflow:%s" % (lens["lens"], shown_path(c.get("workflow"), config))\n', ' said = "%s:from_app:%s" % (lens["lens"], c.get("app_id"))\n'), - ("a workflow that was not read is shown as it is", "kernel/derive.py", - 'path if isinstance(path, str) else "unreadable")', "path)"), + ("a workflow path is shown whatever its form", "kernel/derive.py", + 'shown_path(c.get("workflow"), config))', 'c.get("workflow"))'), + ("a workflow path of any length is shown", "kernel/derive.py", + '\n and len(path) <= config["limits"]["max_path_length"]):', "):"), + ("a workflow path of any characters is shown", "kernel/derive.py", + "all(c in t.FILE_NAME_CHARS for c in stem)", "True"), + ("a workflow path in any directory is shown", "kernel/derive.py", + "path.startswith(t.WORKFLOW_DIR)", "True"), + ("a workflow path with no file name is shown", "kernel/derive.py", + 'stem != "" and ', ""), + ("a workflow path with any ending is shown", "kernel/derive.py", + "name.endswith(ending) and ", ""), ("a check from another App and another file is shown as another file's", "kernel/derive.py", " if not from_its_app(c, lens):\n", " if from_its_workflow(c, lens):\n"), ("the last check with a Lens's name wins", "kernel/derive.py", From 8f589ca8b2f2728079ab4885154b341ae91e507e Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Fri, 16 Oct 2026 07:46:49 -0500 Subject: [PATCH 4/4] Lens workflow check: an unreadable Lens table is a problem, and the check goes on CodeRabbit on #114: a malformed config/defaults.json raised out of the workflow check. It now reports the field, as check_apps.py does, and the workflows are still read. Co-Authored-By: Claude Opus 5.5 --- tests/test_tools.py | 12 ++++++++++++ tools/check_workflows.py | 13 ++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/test_tools.py b/tests/test_tools.py index 65726356..9f9d52d0 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -999,6 +999,18 @@ def test_a_lens_run_by_actions_names_its_workflow_and_no_other_does(self): root = clone({"config/defaults.json": json.dumps(cfg, indent=2) + "\n"}) self.assertTrue(caught(check_workflows.run(root), said), lens["lens"]) + def test_an_unreadable_lens_table_is_a_problem_and_the_check_goes_on(self): + # A check collects every problem: a broken config/defaults.json is reported, and the workflows are still read. + bad = (ROOT / "examples" / "workflows" / "invalid" / "pin.a-tag-can-move.yml").read_text() + for text, said in (("{", "cannot be read"), ("{}", "cannot be read"), + ('{"required_lenses": {"deterministic": [1]}}', "is not a list of Lenses"), + ('{"required_lenses": {"deterministic": [{"lens": "verify", "app_id": 15368, ' + '"workflow": ".github/workflows/verify.yml"}]}}', "names no check")): + problems = check_workflows.run(tree({"config/defaults.json": text, ".github/workflows/verify.yml": GOOD, + ".github/workflows/x.yml": bad, "examples/workflows/verify.yml": GOOD})) + self.assertTrue(caught(problems, said), text) + self.assertTrue(caught(problems, ".github/workflows/x.yml: [pin]"), text) + def test_a_live_workflow_is_held_to_the_rules(self): bad = (ROOT / "examples" / "workflows" / "invalid" / "pin.a-tag-can-move.yml").read_text() problems = check_workflows.run(tree({".github/workflows/x.yml": bad, "examples/workflows/verify.yml": GOOD})) diff --git a/tools/check_workflows.py b/tools/check_workflows.py index 7223d033..bdb2e6a6 100644 --- a/tools/check_workflows.py +++ b/tools/check_workflows.py @@ -254,8 +254,16 @@ def lens_workflows(root: pathlib.Path) -> list: config = root / "config" / "defaults.json" if not config.is_file(): return [] + try: + lenses = json.loads(config.read_text())["required_lenses"]["deterministic"] + except (OSError, ValueError, KeyError, TypeError) as err: + return ["config/defaults.json: required_lenses.deterministic cannot be read, so no Lens's workflow file can be " + "checked: %r" % err] + if not isinstance(lenses, list) or not all(isinstance(lens, dict) for lens in lenses): + return ["config/defaults.json: required_lenses.deterministic is not a list of Lenses, so no Lens's workflow " + "file can be checked"] problems = [] - for lens in json.loads(config.read_text())["required_lenses"]["deterministic"]: + for lens in lenses: rel, where = lens.get("workflow"), "config/defaults.json: the Lens %s" % lens.get("lens") actions = lens.get("app_id") == ACTIONS_APP and type(lens.get("app_id")) is int if actions and not isinstance(rel, str): @@ -269,6 +277,9 @@ def lens_workflows(root: pathlib.Path) -> list: if not path.is_file(): problems.append("%s is read from %s, which does not exist" % (where, rel)) continue + if not isinstance(lens.get("check"), str): + problems.append("%s names no check, so its workflow file cannot be read for it" % where) + continue problems += ["%s is read from %s, which %s" % (where, rel, why) for why in lens_job_problems(path.read_text(errors="replace"), lens["check"])] return problems