diff --git a/.claude/skills/ci/SKILL.md b/.claude/skills/ci/SKILL.md index 78b88da601..977f9aa4f3 100644 --- a/.claude/skills/ci/SKILL.md +++ b/.claude/skills/ci/SKILL.md @@ -33,13 +33,35 @@ partitions on the compilation-error overlay in production; a separate `AddTracki variables) and **fails RED naming exactly what to provision**. Adding an input = one line in its `missing` array. - **Gates depend on it (`needs: [preflight, …]`) and run unconditionally** — no input-shaped `if:`. -- **The ONE legitimate exemption is a FORK PR** (GitHub withholds org secrets by design). Express it - **once**, as a check on the *event* (`github.event.pull_request.head.repo.fork != true`) — never - as a "the secret is empty" check. At the job level those two look identical and only one is safe. +- **The ONE unconditionally legitimate exemption is a FORK PR** (GitHub withholds org secrets by + design, and no maintainer action changes that). Express it **once**, as a check on the *event* + (`github.event.pull_request.head.repo.fork != true`) — never as a "the secret is empty" check. At + the job level those two look identical and only one is safe. - **Propagate into the required check.** `collect-results` runs with `always()` and is the ONLY required status check, so it needs `preflight` in `needs` **plus an explicit fail step** — a skipped dependency does not fail an `always()` job, which would re-open the trapdoor one level up. +### 🚨 The SECOND secret store — and the one precondition for a `dependabot[bot]` exemption + +**A run opened by Dependabot resolves `secrets.X` against a SEPARATE store** (Settings → Secrets and +variables → **Dependabot**), so a secret that is provisioned on the Actions tab and used by every +other run resolves EMPTY. The tell is `Secret source: Dependabot` in the run log, and the preflight +does exactly what it should: it fails RED naming a secret the maintainer can see in the UI. **The +remediation line must therefore name the STORE** — `Settings → Secrets → Actions` is misleading +here — and the fix is to provision the same names into the Dependabot store, never an `if:`. + +Core's `dotnet-test.yml` *does* carry `github.actor != 'dependabot[bot]'` on `shared-rules` and +`cross-repo-pair`. That is legitimate for exactly one reason, and it is a **precondition, not a +precedent**: those gates also trigger on `merge_group`, where the Actions store IS available, and +the merge queue is the only path to `main` — *the exemption moves WHERE the gate runs, never +WHETHER it runs*. **Measured 2026-09-06: only `Systemorph/MeshWeaver` has a `merge_queue` rule.** +Plugins, Reinsurance, SocialMedia, Crm, Manufacturing, Education and Memex have none — several +carry a `merge_group:` trigger whose event never fires, which is the trap. Copying that `if:` into +a satellite is a skip-trapdoor, and a silent one: the satellites' required contexts are the gate +jobs themselves, and an absent required context counts as SATISFIED. + +Full reference: [DependabotSecretStore.md](../../../src/MeshWeaver.Documentation/Data/Architecture/DependabotSecretStore.md). + **Legitimate `continue-on-error` (do not "fix" these):** the `Publish Test Results` reporter (the TRX summarize step is the real gate; a GitHub-API 429 must not fail the run) and the green-marker push/prune (losing a marker costs a redundant run, never correctness). The test is *what does a @@ -230,6 +252,9 @@ Full reference: - [ ] No `continue-on-error` on a gate's input step; no `if:` asking whether a secret/variable is set. Fork-PR exemption expressed once, on the event. - [ ] Missing external inputs fail a `preflight` job RED, naming what to provision. +- [ ] Adding a required secret? It is provisioned in **both** stores (Actions *and* Dependabot), and + the preflight's remediation line names the store. A `dependabot[bot]` exemption only where the + same gate runs on `merge_group` — core only. - [ ] The required check has `preflight` in `needs` **and** an explicit fail step. - [ ] Deleting a public framework symbol? The node JSON and the live mesh were searched too. - [ ] Not adding `cancel-in-progress` on `main`. diff --git a/AGENTS.md b/AGENTS.md index 0de64b9137..5de04ba5c0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,6 +87,8 @@ Full reference: [/gui](.claude/skills/gui/SKILL.md) · [GUI/DataBinding.md](src/ **A CI gate must never carry `continue-on-error:` on the step that fetches its input, nor an `if:` that asks whether a secret/variable is set.** GitHub paints a skipped job the same colour as a passed one, so "the gate never ran" and "the gate passed" become indistinguishable. Instead: one `preflight` job asserts every external input and fails RED naming what to provision; gates `needs:` it and run unconditionally; the fork-PR exemption is expressed once, on the *event*, never as "the secret is empty"; the required check carries `preflight` in `needs` plus an explicit fail step. The same applies to guard TESTS — a guard whose subject moved and whose roots did not passes having checked nothing. +🚨 **A Dependabot run reads a SECOND secret store, and a preflight red there is a PROVISIONING task, never a workflow bug.** `Secret source: Dependabot` in the log means `secrets.X` resolved against Settings → Secrets → **Dependabot**, so a secret sitting on the Actions tab reads EMPTY — and the preflight's own "set it under → Actions" line sends the reader to the wrong store. **A secret any `pull_request` lane requires is provisioned in BOTH stores** (`gh secret set --app dependabot --repo …`, maintainer-only: values are unreadable through the API); adding a `missing+=` line is a two-store act, and nothing in CI can see the second store. 🚨 **Do NOT reach for `github.actor != 'dependabot[bot]'`.** Core carries it on `shared-rules`/`cross-repo-pair` for ONE reason — those gates also run on `merge_group`, where the Actions store IS available, so *the exemption moves WHERE the gate runs, never WHETHER it runs*. Measured 2026-09-06: **only core has a `merge_queue` rule**; every satellite (and Memex) has none, and several carry a `merge_group:` trigger whose event never fires. Copying that `if:` into a satellite skips the gate outright — invisibly, because its required contexts are the gate jobs themselves and an absent required context counts as SATISFIED. Full reference: [DependabotSecretStore.md](src/MeshWeaver.Documentation/Data/Architecture/DependabotSecretStore.md). + **And never hand-roll (or copy-paste) a node repo's CI.** The shared jobs live here as `workflow_call` workflows (`.github/workflows/node-repo-{validate,compile-check,gate,tag-modules,publish-bake}.yml`); MeshWeaver.Plugins / .Education / .Reinsurance / .SocialMedia call them and keep only repo-specific policy. Adopting one renames that repo's required-status-check contexts to ` / ` — do it in the same change. Full reference: [/ci](.claude/skills/ci/SKILL.md). 🚨 **A change that spans this repo and a plugin repo lands DELETING-HALF-LAST, and CI now holds you to it.** A pull request whose diff removes a `public` top-level type from `src/` — a departure, a move, or a whole assembly leaving — **or a public MEMBER of a type that stays** (#3137 deleted two fields and reddened every Plugins PR for three hours) must declare its counterpart in the PR BODY: `Pairs-with: Systemorph/MeshWeaver.Plugins#904`, or `Pairs-with: none — ` when nothing outside core referenced it (a reason citing a live-mesh sweep must quote `searched: true`; `searched: false` is refused). The `Cross-repo pair (public surface)` gate resolves it and stays red while the counterpart is open, draft, or merged into anything but its repo's default branch. Ordinary work never meets it (measured: `main~25 → main` removes ZERO public types and two members, both #3137). 🚨 **The gate sees exactly TWO of the seven break shapes** — a public type leaving `src/`, and a public member leaving a type that stays. It does NOT see: an added overload making a dependent's `` ambiguous (`CS0419` under `-warnaserror` — this bit MeshWeaver.SocialMedia on 2026-09-04 when it moved its core pin), a JSON envelope's field names changing, a comment another repo parses as data, an i18n VALUE change, or — #3276, shape 7 — a method's BEHAVIOUR changing behind an **unchanged signature**, which no surface detector can see by construction. **There is no `Dependent suites (MeshWeaver.Plugins)` check — do not wait for one.** Compile-and-running the dependent's suite against the candidate core commit is #2689's stated acceptance criterion and is NOT built; the partial cover that does exist is the dependent's own CI reacting to core's release event, and [CrossRepoPairGate.md](src/MeshWeaver.Documentation/Data/Architecture/CrossRepoPairGate.md) says per shape whether that actually runs. So a green core PR does not mean the dependents still build — #3276 is a live instance. **And for a dependent that PINS core** (MeshWeaver.Plugins, `MW_PLATFORM_REF`) **even that partial cover does not fire**: its `platform-ref` job resolves the same pin on the release event as on a PR, so the event moves what is BAKED, not what `src/` compiles against — the pin bump IS its integration test, and #3276 sat 48 commits that way. Full reference: [CrossRepoPairGate.md](src/MeshWeaver.Documentation/Data/Architecture/CrossRepoPairGate.md) · [/ci](.claude/skills/ci/SKILL.md). diff --git a/src/MeshWeaver.Documentation/Data/Architecture/DependabotSecretStore.md b/src/MeshWeaver.Documentation/Data/Architecture/DependabotSecretStore.md new file mode 100644 index 0000000000..84efdca25a --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/Architecture/DependabotSecretStore.md @@ -0,0 +1,197 @@ +--- +Name: The Dependabot Secret Store +Category: Architecture +Description: A Dependabot-triggered run reads a SEPARATE secret store, so a preflight that is behaving perfectly fails red naming secrets that are provisioned. Why the answer is provisioning and not an actor exemption — and the single precondition that makes an actor exemption legitimate, which core meets and no satellite does. +Icon: +--- + +# The Dependabot Secret Store + +**Every repository has TWO secret stores, and a Dependabot-triggered workflow run can only see the +second one.** Settings → Secrets and variables has an **Actions** tab and a **Dependabot** tab; +they share names but nothing else. A run opened by `dependabot[bot]` resolves `secrets.X` against +the *Dependabot* store, so a secret that exists — visibly, on the Actions tab, used by every other +run of the same workflow — resolves to the empty string. + +The tell is one line in the run log, above the first step: + +``` +Secret source: Dependabot +``` + +This is not a bug and not a misconfiguration. GitHub puts a Dependabot pull request in the same +trust class as a fork pull request: the diff was composed from a third-party registry's metadata, so +the run gets a read-only `GITHUB_TOKEN` and no Actions secrets. The Dependabot store exists so that +Dependabot's own update jobs can reach private registries; workflow runs it triggers inherit it. + +## Why this reads as a workflow bug and is not one + +A `preflight` job asserts every externally-provisioned input and fails RED naming what to provision +— the shape AGENTS.md requires, because *a gate that cannot run must never go grey and read as a +pass*. On a Dependabot run it does exactly that: + +``` +##[error]Required CI inputs are not provisioned, so the heavy gate cannot run. + A gate that cannot run fails RED — it must never go grey and read as a pass. +##[error]missing: secrets.REGISTRY_PUBLISH_TOKEN — … +``` + +Everything about that message is correct except the reader's next move. The maintainer opens the +Actions tab, finds `REGISTRY_PUBLISH_TOKEN` sitting there, and concludes the gate is broken. The +error names the right secret and, unless it says so, the *wrong store*. + +**So the first fix is textual and costs nothing: every preflight's remediation line must name the +store the run actually read.** `Set them under Settings → Secrets and variables → Actions` is +actively misleading on a Dependabot run. + +## The fleet, measured 2026-09-06 + +`gh api repos/Systemorph//actions/secrets` and `.../dependabot/secrets` return **names only** +— never values, from any credential. Both stores, every repo that has a `.github/dependabot.yml`: + +| repo | Actions | Dependabot | in Actions only (raw store diff) | +|---|---:|---:|---| +| MeshWeaver | 14 | **0** | all 14 | +| MeshWeaver.Plugins | 12 | 4 | `ACR_PUSH_PASSWORD`, `ACR_PUSH_USERNAME`, `AZURE_CLIENT_ID`, `AZURE_SUBSCRIPTION_ID`, `AZURE_TENANT_ID`, `MESHWEAVER_REPO_TOKEN`, `PLATFORM_WEBHOOK_SECRET`, `REGISTRY_PUBLISH_TOKEN` | +| MeshWeaver.Reinsurance | 10 | 4 | `AZURE_CLIENT_ID`, `AZURE_SUBSCRIPTION_ID`, `AZURE_TENANT_ID`, `MESHWEAVER_REPO_TOKEN`, `MW_REGISTRY_KEY`, `PLATFORM_WEBHOOK_SECRET` | +| MeshWeaver.SocialMedia | 11 | 4 | the six above **+** `REGISTRY_PUBLISH_TOKEN` | +| MeshWeaver.Crm | 9 | 4 | `AZURE_CLIENT_ID`, `AZURE_SUBSCRIPTION_ID`, `AZURE_TENANT_ID`, `MW_REGISTRY_KEY`, `PLATFORM_WEBHOOK_SECRET` | +| MeshWeaver.Manufacturing | 9 | 5 | `ACR_PASSWORD`, `ACR_USERNAME`, `MW_REGISTRY_INSTANCE_KEY`, `PLATFORM_WEBHOOK_SECRET` | +| MeshWeaver.Education | 9 | 2 | `AZURE_CLIENT_ID`, `AZURE_SUBSCRIPTION_ID`, `AZURE_TENANT_ID`, `MESHWEAVER_APP_ID`, `MESHWEAVER_APP_PRIVATE_KEY`, `MW_REGISTRY_KEY`, `PLATFORM_WEBHOOK_SECRET` | +| Memex | 8 | **0** | all 8 | + +**No repository has a secret in the Dependabot store that is absent from Actions.** The mirror is +always a subset — it drifts one way only, which is why nobody notices it drifting. + +### The raw diff is not the failure set + +Most of those secrets are consumed by push-only lanes (`publish-bake`, `tag-modules`) or by steps +gated on `if: inputs.publish`, and are simply unused on a pull request. What actually reds a +Dependabot PR is the intersection of the raw diff with **the set each repo's `preflight` asserts on +a `pull_request` event** — and preflights here deliberately assert every input the *later lanes* +consume, not only their own, so that a provisioning gap fails in one cheap job instead of twenty +minutes into a bake: + +| repo | asserted on a PR and missing from the Dependabot store | status | +|---|---|---| +| MeshWeaver.Manufacturing | `ACR_USERNAME`, `ACR_PASSWORD`, `MW_REGISTRY_INSTANCE_KEY` | **measured red** — run 33357319017, `Required CI inputs` names all three | +| MeshWeaver.SocialMedia | `REGISTRY_PUBLISH_TOKEN` | **measured red** — run 33354469022, `Secret source: Dependabot` in the same log | +| MeshWeaver.Reinsurance | `MW_REGISTRY_KEY`, `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, `AZURE_SUBSCRIPTION_ID` | **derived** from `main`'s preflight list × the store. Its one measured run (33354473532) predates that list and died one job later at `compile-check` with `compose-sealed-modules.sh: --registry-url needs --registry-key` — the same absent `MW_REGISTRY_KEY`, surfacing late and unrecognisably | +| MeshWeaver.Crm | *none* | **control** — its preflight asserts exactly the four secrets its Dependabot store holds | +| MeshWeaver.Education | *none* | **control** — PR-time preflight wants `MW_REGISTRY_USERNAME` + `MW_REGISTRY_PASSWORD`, both mirrored; its `AZURE_*` assertion lives in the push-only `publish-bake` | +| MeshWeaver.Plugins | *none* — latent | preflight wants `MW_TEST_IMAGE` + `ACR_*`, all mirrored. `REGISTRY_PUBLISH_TOKEN` reaches `modules-floor`/`modules-rest`, whose publish step is `if: inputs.publish` (false on a PR) | +| MeshWeaver | *none today* — **latent, and the store is empty** | `dotnet-test.yml`'s `shared-rules` and `cross-repo-pair` both assert `MESHWEAVER_APP_ID`/`MESHWEAVER_APP_PRIVATE_KEY`. They are exempted on the actor instead — see below | +| Memex | not measured | store empty; no Dependabot PR open to measure against | + +Crm and Education are the two controls that make the rest of the table credible: same shared +workflows, same event, green — because their two stores agree. + +## Why the answer is provisioning, not an actor exemption + +The tempting fix is `if: github.actor != 'dependabot[bot]'` on the heavy gate. AGENTS.md permits an +exemption expressed **on the event** and forbids one expressed as *"the secret is empty"*, and +`github.actor` is an event property, so the shape looks compliant. It is not, for the satellites, +and the reason is worth stating precisely because core does carry that exact `if:`. + +**1 · The fork exemption states an impossibility; a Dependabot exemption states a preference.** +A fork PR *cannot* be given org secrets — GitHub withholds them and no maintainer action changes +that, so skipping is the honest signal and the message ("ask a maintainer") is the only actionable +one. A Dependabot run *can* be given them: one `gh secret set --app dependabot` per name. Exempting +it is therefore a decision not to gate, wearing the costume of a structural limit. + +**2 · Core's exemption is legitimate for one reason, and the reason is a precondition, not a +precedent.** `dotnet-test.yml` says it plainly: + +> 🚨 And this is NOT a skip-trapdoor, for a reason specific to this workflow: it also triggers on +> `merge_group`, where the credential IS available. So an exempted PR still has its shared rule +> blocks checked — on the queue's temporary ref, with the token minted, BEFORE the merge lands. +> **The exemption moves WHERE the gate runs, never WHETHER it runs.** + +That is the test. An actor exemption is legitimate **only** where the same gate also runs on +`merge_group` *and* the merge queue is the only path to the default branch. + +**Measured 2026-09-06: core meets it and nothing else does.** `Systemorph/MeshWeaver` ruleset +2128472 carries a `merge_queue` rule. Every other repo in the fleet — Plugins, Reinsurance, +SocialMedia, Crm, Manufacturing, Education, Memex — has only `deletion`, `non_fast_forward`, +`copilot_code_review` and (some) `pull_request` / `required_status_checks`; no `merge_queue` rule in +any ruleset and no merge-queue block in classic branch protection. Several satellites *do* carry a +`merge_group:` trigger in `ci.yml`, which is the trap: **the trigger exists, the event never fires.** +Copying core's `if:` into them would move the gate to an event that has no path to `main`. + +**3 · And in a satellite the skipped gate would not even look skipped.** The satellites' required +contexts are the gate jobs themselves (`compile-check / …`, `test-repos / …`), not a single +`collect-results`. A required context that never appears counts as **satisfied** +— see [Reading CI Signals](/Doc/Architecture/ReadingCiSignals). The Dependabot PR would go from +"red, honestly" to "green, having proven nothing". + +**4 · A Dependabot PR is the case where the gate matters most.** Manufacturing and Crm watch exactly +one ecosystem, `github-actions`. The diff *is* the CI's own plumbing — `actions/download-artifact` +7 → 8 in Manufacturing#40. A gate exempted from the PR that changes the gate's own machinery is the +worst possible place to spend the exemption. + +Conclusion: **provision the missing names into the Dependabot store.** For core specifically the +exemption stands, because core has the queue — but core's empty Dependabot store is worth +remembering as the reason those two gates were exempted rather than fixed. + +## The rule + +> **A secret that any `pull_request`-triggered lane requires is provisioned in BOTH stores.** +> Adding a line to a preflight's `missing` array is a two-store act. There is no gate anywhere that +> can see the second store, so nothing but this rule keeps them in step. + +That invisibility is why this recurred. #2249 (2026-08) was the same defect, closed after +provisioning four names into two satellites. Since then Reinsurance added `MW_REGISTRY_KEY` and the +`AZURE_*` triple to its preflight, SocialMedia added `REGISTRY_PUBLISH_TOKEN`, and Manufacturing was +created — each into Actions only, each re-opening the hole, none visible to any review or gate. +#3399 is the second occurrence. + +Two smaller things the audit surfaced, worth converging separately: the same credential is called +`MW_REGISTRY_INSTANCE_KEY` in Manufacturing and `MW_REGISTRY_KEY` everywhere else, and core's and +Memex's Dependabot stores are empty rather than partial. + +## Auditing it + +Two REST reads per repo. Both return names only — a secret **value** is not readable through the +API from any credential, which is also why mirroring cannot be automated by an agent and needs +whoever holds the values. + +```bash +gh api repos/Systemorph/MeshWeaver.SocialMedia/actions/secrets --jq '.total_count, (.secrets[].name)' +gh api repos/Systemorph/MeshWeaver.SocialMedia/dependabot/secrets --jq '.total_count, (.secrets[].name)' +``` + +Read `total_count` before reading the list: a truncated listing that is summarised as a count is how +a partial mirror reads as a complete one. Then diff against what the repo's preflight asserts: + +```bash +gh api repos/Systemorph/MeshWeaver.SocialMedia/contents/.github/workflows/ci.yml --jq '.content' \ + | base64 -d | grep -n 'missing+=' +``` + +And confirm the store a red run actually read, rather than inferring it: + +```bash +gh api repos/Systemorph//actions/jobs//logs | grep -m1 'Secret source:' +``` + +## Remediating it + +One command per missing name, same value as the Actions secret. This is repository administration +with fleet consequences — it needs the person who holds the values, and it cannot be done from a +pull request: + +```bash +gh secret set --app dependabot --repo Systemorph/MeshWeaver. +``` + +Then re-run the Dependabot PR's workflow. The positive signal is the preflight's own success line, +not merely a green wall. + +## Related + +- [Reading CI Signals](/Doc/Architecture/ReadingCiSignals) — why an absent required context counts + as satisfied, which is what makes an ill-considered exemption dangerous rather than merely lax. +- [The Cross-Repo Pair Gate](/Doc/Architecture/CrossRepoPairGate) — the other core gate carrying the + dependabot actor exemption, for the same credential and under the same merge-queue precondition. +- [Module Build Architecture](/Doc/Architecture/ModuleBuildArchitecture) — the shared + `workflow_call` lanes every satellite's `preflight` feeds. diff --git a/src/MeshWeaver.Documentation/Data/Architecture/ReadingCiSignals.md b/src/MeshWeaver.Documentation/Data/Architecture/ReadingCiSignals.md index 2c22a0bdc2..e141a6eb7c 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/ReadingCiSignals.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/ReadingCiSignals.md @@ -754,3 +754,5 @@ whole protocol is [The Merge Queue](/Doc/Architecture/MergeQueue). [The Merge Queue](/Doc/Architecture/MergeQueue) · [Module Versioning](/Doc/Architecture/ModuleVersioning) · [Modules](/Doc/Architecture/Modules) · [Deploying Plugin Changes](/Doc/Architecture/DeployingPluginChanges) +· [The Dependabot Secret Store](/Doc/Architecture/DependabotSecretStore) — a red that names a secret +you can see provisioned, and why the exemption that would silence it is only safe in core