Repository navigation
docs: the Dependabot secret store, and the one precondition for an actor exemption - #3412
Merged
Merged
Conversation
…tor exemption Dependabot-triggered runs resolve `secrets.X` against a SEPARATE store, so a preflight that is behaving exactly as designed fails RED naming secrets the maintainer can see provisioned on the Actions tab. Three satellite PRs have been red on this since 2026-08-31, and it is the second occurrence (#2249 was the first) because nothing in CI can see the second store. The dangerous fix is one keystroke away: core's `dotnet-test.yml` already carries `github.actor != 'dependabot[bot]'` on `shared-rules` and `cross-repo-pair`, so copying it into a satellite looks sanctioned. It is not. That exemption is legitimate in core for exactly one reason, which the file itself states — 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 Systemorph/MeshWeaver has a `merge_queue` rule (ruleset 2128472). Plugins, Reinsurance, SocialMedia, Crm, Manufacturing, Education and Memex have none — and several carry a `merge_group:` trigger whose event never fires, which is the trap. In a satellite that `if:` would skip the gate outright, invisibly: their required contexts are the gate jobs themselves, and an absent required context counts as SATISFIED. Conserves the audit as a doc page (both stores for all eight repos, the preflight-required subset that actually reds, the two controls that make the table credible, the re-runnable commands) and states the rule where someone adding a `missing+=` line will read it. No gate behaviour changes; no secret was created, read or printed. Refs #3399, #2249 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
Documentation-only changes are consistent with existing CI guidance and add clear, actionable instructions without altering workflow behavior.
Pull request overview
Documents and codifies a fleet-wide CI operational pitfall: Dependabot-triggered runs read from a separate secret store, and copying core’s dependabot[bot] actor exemption into satellites can create a silent skip-trapdoor unless the merge-queue/merge_group precondition holds.
Changes:
- Adds a new architecture doc page explaining the Dependabot secret store split, how to audit it, and why provisioning (not actor exemptions) is the correct remediation in satellites.
- Updates AGENTS.md and the
/ciskill to explicitly warn about the Dependabot store and the single precondition that makes an actor exemption safe in core. - Cross-links the new guidance from Reading CI Signals.
File summaries
| File | Description |
|---|---|
| src/MeshWeaver.Documentation/Data/Architecture/ReadingCiSignals.md | Adds a related-link entry pointing to the new Dependabot secret-store doc. |
| src/MeshWeaver.Documentation/Data/Architecture/DependabotSecretStore.md | New architecture documentation page capturing the audit, rationale, and remediation commands. |
| AGENTS.md | Adds an explicit Dependabot secret-store warning next to the existing “no skip-trapdoors” guidance. |
| .claude/skills/ci/SKILL.md | Extends CI skill guidance/checklist to cover Dependabot secret-store mirroring and actor-exemption preconditions. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| **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 <NAME> --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). |
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #3399, #2249.
The problem, and the fix that is one keystroke away from being wrong
A Dependabot-triggered run resolves
secrets.Xagainst a separate store(Settings → Secrets and variables → Dependabot), so a secret provisioned on the Actions tab and
used by every other run of the same workflow reads EMPTY. The
preflightthen does exactly whatAGENTS.md requires — fails RED naming what to provision — and the maintainer opens the Actions tab,
finds the secret sitting there, and concludes the gate is broken. Three satellite PRs have been red
on this since 2026-08-31, and this is the second occurrence: #2249 was the first, closed after
provisioning, and it recurred because nothing in CI can see the second store.
The dangerous fix looks sanctioned. Core's
dotnet-test.ymlalready carriesgithub.actor != 'dependabot[bot]'onshared-rules(line 1997) andcross-repo-pair(line 2128),with
collect-resultsmirroring the same predicate. So the obvious move is to copy it into thesatellites. That would be a skip-trapdoor, and a silent one.
Core's exemption is legitimate for one reason, which that file states itself:
That is a precondition, not a precedent — and it is testable.
Measured 2026-09-06
merge_queuerule?deletion+non_fast_forward+copilot_code_review; classic protection has no merge-queue blockmerge_queueOnly core has a merge queue. Several satellites do carry a
merge_group:trigger inci.yml— which is the trap: the trigger exists and the event never fires. And in a satellite the skip would
not even look like one: their required contexts are the gate jobs themselves (
compile-check / …,test-repos / …), not a singlecollect-results, and an absent required context counts assatisfied. The PR would go from red, honestly to green, having proven nothing.
Also measured, both stores for all eight repos that carry a
.github/dependabot.yml(names only —values are unreadable through the API from any credential, which is why mirroring needs the
maintainer). The full table is in the doc page; the operative half:
ACR_USERNAME,ACR_PASSWORD,MW_REGISTRY_INSTANCE_KEY— measured, run 33357319017REGISTRY_PUBLISH_TOKEN— measured, run 33354469022MW_REGISTRY_KEY,AZURE_CLIENT_ID,AZURE_TENANT_ID,AZURE_SUBSCRIPTION_ID— derived; its one measured run died a job later atcompose-sealed-modules.sh: --registry-url needs --registry-keyWhat this PR does
Doc/Architecture/DependabotSecretStore— the audit, the reasoning, the precondition, and there-runnable commands, per the conserve-work-products rule (the finding otherwise lives only in an
issue thread).
AGENTS.md— one paragraph beside the existing fork-exemption sentence, because that is theexact point at which the wrong fix gets written.
.claude/skills/ci/SKILL.md— the same rule in the preflight section and one checklist line.ReadingCiSignals.md— a Related link.It changes no gate's behaviour, adds no job, touches no workflow, and creates, reads or prints
no secret. The remaining work is provisioning — eight
gh secret set --app dependabotcommandsacross three repos, listed on #3399 — which is correctly not something a pull request can do.
Verification
dotnet build test/MeshWeaver.Documentation.Test -c Release -warnaserror→0 Warning(s) 0 Error(s), 1:52 elapsed.DocumentationLinkIntegrityTest→Passed! Failed: 0, Passed: 1(568 ms) — every link in thenew page and the edited one resolves.
AGENTS.mdedit is outside bothshared-rule:marked blocks, so the mirrored blocks arebyte-identical and the shared-rules gate is unaffected.
No What's New entry — pure-internal (CI provisioning + agent instructions), no user-visible
effect.
🤖 Generated with Claude Code