From 471906db620a74feea0806c641cae969f6fc6c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 21:33:56 +0200 Subject: [PATCH 01/13] =?UTF-8?q?feat(release):=20move=20the=20bot-token?= =?UTF-8?q?=20action's=20pin=20in=20consumers=20(MIP-0086=20=C2=A75.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tested: release.py --self-test red then green; devkit quality gate Co-Authored-By: Claude --- scripts/release.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/release.py b/scripts/release.py index 48d7746..d7ebd2e 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -27,7 +27,7 @@ # A pin is a v-tag in one of these contexts; actions/checkout@v7 and history in CHANGELOG are not. # \s* because some repos spread settings.json's marketplace entry over lines (#65). PIN = re.compile( - rf"(marola-devkit/v|marola-devkit/\.github/workflows/[\w.-]+@v|devkit-ref: v" + rf"(marola-devkit/v|marola-devkit/\.github/(?:workflows|actions)/[\w.-]+@v|devkit-ref: v" rf'|marola-devkit",\s*"ref": "v|\*\*Status:\*\* this is v)({SEMVER})()' ) PIN_GLOBS = ["README.md", "docs/**/*.md", ".github/workflows/*.yml"] @@ -180,14 +180,18 @@ def self_test(): ), ".github/workflows/notify-umbrella.yml": "# uses: marola-dev/marola-devkit/.github/workflows/n.yml@v0.5.1\n", "CHANGELOG.md": "# Changelog\n\nIntro.\n\n## v0.5.1 — 2026-10-07\n\n- old\n", + ".github/workflows/ci.yml": " - uses: marola-dev/marola-devkit/.github/actions/bot-token@v0.5.1\n", } for rel, text in files.items(): (root / rel).parent.mkdir(parents=True, exist_ok=True) (root / rel).write_text(text) - assert len(found(root)) == 8, found(root) + assert len(found(root)) == 9, found(root) assert any("0.4.0" in m for m in mismatches(root, "0.5.1")), "a stale pin went unnoticed" bump(root, "0.6.0") assert not mismatches(root, "0.6.0"), mismatches(root, "0.6.0") + assert "bot-token@v0.6.0" in (root / ".github/workflows/ci.yml").read_text(), ( + "an action pin did not move" + ) assert "actions/checkout@v7.0.0" in (root / "README.md").read_text(), ( "a third-party pin moved" ) From 9af8e71f2156d82a8f61a08b1ca1e560899f8783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 21:37:03 +0200 Subject: [PATCH 02/13] =?UTF-8?q?feat(bot-token):=20mint=20scoped=20App=20?= =?UTF-8?q?tokens=20and=20commit=20as=20the=20bot=20(MIP-0086=20=C2=A75.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Composite action over actions/create-github-app-token: refuses an empty repositories list, always passes owner plus the named repos, and sets git to the bot's noreply identity. bot-token-test.yml proves the refusal and the one-repo, no-pull-request-write scope live. Tested: actionlint; the live bot-token test runs on the PR Co-Authored-By: Claude --- .github/actions/bot-token/action.yml | 68 ++++++++++++++++++++++++++++ .github/workflows/bot-token-test.yml | 60 ++++++++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 .github/actions/bot-token/action.yml create mode 100644 .github/workflows/bot-token-test.yml diff --git a/.github/actions/bot-token/action.yml b/.github/actions/bot-token/action.yml new file mode 100644 index 0000000..e7d00b7 --- /dev/null +++ b/.github/actions/bot-token/action.yml @@ -0,0 +1,68 @@ +# bot-token — a marola-pr-bot or marola-merge-bot token for named repos, with git committing as that +# bot (MIP-0086 §5.4). A composite action cannot read secrets: the caller passes the App's id and key. +name: bot-token +description: Mint a GitHub App installation token for named marola-dev repos and make git commit as the App's bot user. + +inputs: + app-id: + description: vars.PR_BOT_APP_ID or vars.MERGE_BOT_APP_ID + required: true + private-key: + description: secrets.PR_BOT_KEY or secrets.MERGE_BOT_KEY + required: true + repositories: + description: >- + Comma- or newline-separated repo names in marola-dev. Never empty: with owner set and no list, + the token would cover every repo the App is installed on. + required: true + permission-contents: { description: "read | write", required: false, default: "" } + permission-pull-requests: { description: "read | write", required: false, default: "" } + permission-workflows: { description: "write", required: false, default: "" } + permission-checks: { description: "read", required: false, default: "" } + permission-statuses: { description: "read", required: false, default: "" } + permission-issues: { description: "read | write", required: false, default: "" } + +outputs: + token: { description: "The installation token, valid for one hour.", value: "${{ steps.mint.outputs.token }}" } + app-slug: { description: "The App's slug.", value: "${{ steps.mint.outputs.app-slug }}" } + installation-id: { description: "The installation's id.", value: "${{ steps.mint.outputs.installation-id }}" } + user-name: { description: "[bot]", value: "${{ steps.who.outputs.name }}" } + user-email: { description: "+[bot]@users.noreply.github.com", value: "${{ steps.who.outputs.email }}" } + +runs: + using: composite + steps: + - name: refuse an empty repositories list + shell: bash + env: + REPOS: ${{ inputs.repositories }} + run: | + [ -n "$(tr -d ' ,\n\t' <<<"$REPOS")" ] || { echo "::error::bot-token: repositories is empty; name every repo the token needs"; exit 1; } + + - id: mint + uses: actions/create-github-app-token@v3 + with: + app-id: ${{ inputs.app-id }} + private-key: ${{ inputs.private-key }} + owner: marola-dev + repositories: ${{ inputs.repositories }} + permission-contents: ${{ inputs.permission-contents }} + permission-pull-requests: ${{ inputs.permission-pull-requests }} + permission-workflows: ${{ inputs.permission-workflows }} + permission-checks: ${{ inputs.permission-checks }} + permission-statuses: ${{ inputs.permission-statuses }} + permission-issues: ${{ inputs.permission-issues }} + + # The commit email needs the bot user's id, which is not the App id (create-github-app-token's README). + - id: who + shell: bash + env: + GH_TOKEN: ${{ steps.mint.outputs.token }} + SLUG: ${{ steps.mint.outputs.app-slug }} + run: | + set -euo pipefail + uid="$(gh api "users/${SLUG}%5Bbot%5D" --jq .id)" + name="${SLUG}[bot]" email="${uid}+${SLUG}[bot]@users.noreply.github.com" + git config --global user.name "$name" + git config --global user.email "$email" + { echo "name=$name"; echo "email=$email"; } >>"$GITHUB_OUTPUT" diff --git a/.github/workflows/bot-token-test.yml b/.github/workflows/bot-token-test.yml new file mode 100644 index 0000000..4180e89 --- /dev/null +++ b/.github/workflows/bot-token-test.yml @@ -0,0 +1,60 @@ +# bot-token-test — the live test of .github/actions/bot-token (MIP-0086 §7). It needs PR_BOT_KEY, +# so it skips fork PRs, which receive no secrets. +name: bot-token test + +on: + pull_request: + paths: [".github/actions/bot-token/**", ".github/workflows/bot-token-test.yml"] + workflow_dispatch: + +permissions: {} + +jobs: + refuses-empty-list: + if: github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v7 + - id: bot + continue-on-error: true + uses: ./.github/actions/bot-token + with: + app-id: ${{ vars.PR_BOT_APP_ID }} + private-key: ${{ secrets.PR_BOT_KEY }} + repositories: "" + - name: the empty list was refused + env: + OUTCOME: ${{ steps.bot.outcome }} + run: '[ "$OUTCOME" = failure ] || { echo "::error::bot-token minted with no repositories"; exit 1; }' + + mints-scoped: + if: github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v7 + - id: bot + uses: ./.github/actions/bot-token + with: + app-id: ${{ vars.PR_BOT_APP_ID }} + private-key: ${{ secrets.PR_BOT_KEY }} + repositories: marola-devkit + permission-contents: read + - name: one repo, no pull-request write, git commits as the bot + env: + GH_TOKEN: ${{ steps.bot.outputs.token }} + SLUG: ${{ steps.bot.outputs.app-slug }} + EMAIL: ${{ steps.bot.outputs.user-email }} + run: | + set -euo pipefail + repos="$(gh api installation/repositories --jq '[.repositories[].full_name] | join(",")')" + [ "$repos" = marola-dev/marola-devkit ] || { echo "::error::the token reaches $repos"; exit 1; } + # An invalid body: 403 means no permission, 422 would mean the permission was granted. + probe() { curl -s -o /dev/null -w '%{http_code}' -X POST -H "Authorization: Bearer $GH_TOKEN" \ + -H 'Accept: application/vnd.github+json' "https://api.github.com/repos/$1" -d '{}'; } + [ "$(probe marola-dev/marola-devkit/pulls)" = 403 ] || { echo "::error::pull_requests was granted"; exit 1; } + [ "$(probe marola-dev/marola-site/git/refs)" != 422 ] || { echo "::error::the token can write marola-site"; exit 1; } + [[ "$(git config --global user.email)" =~ ^[0-9]+\+${SLUG}\[bot\]@users\.noreply\.github\.com$ ]] || { echo "::error::git email is $(git config --global user.email)"; exit 1; } + [ "$(git config --global user.email)" = "$EMAIL" ] + [ "$(git config --global user.name)" = "${SLUG}[bot]" ] From 97f0859c142991f9bdbe193f2e17e3a79f794b81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 21:41:22 +0200 Subject: [PATCH 03/13] fix(bot-token): refuse unsafe input, scope git identity to the job, assert the probes Refuse a token with no permission-* input (v3 skips empty ones, so it would hold every App permission). Clean the repositories list in bash and pass that to the mint step, refusing blanks and non-bare names. Write the bot identity to a job-scoped GIT_CONFIG_GLOBAL file instead of the runner's global config. The test mints with contents write, adds a positive control, and asserts marola-site is 403 or 404. Tested: actionlint; guard step exercised locally; the live bot-token test runs on the PR Co-Authored-By: Claude --- .github/actions/bot-token/action.yml | 30 ++++++++++++++++---- .github/workflows/bot-token-test.yml | 42 ++++++++++++++++++++++------ 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/.github/actions/bot-token/action.yml b/.github/actions/bot-token/action.yml index e7d00b7..bd2d115 100644 --- a/.github/actions/bot-token/action.yml +++ b/.github/actions/bot-token/action.yml @@ -1,5 +1,5 @@ # bot-token — a marola-pr-bot or marola-merge-bot token for named repos, with git committing as that -# bot (MIP-0086 §5.4). A composite action cannot read secrets: the caller passes the App's id and key. +# bot for the rest of the job (MIP-0086 §5.4). A composite action cannot read secrets: the caller passes the App's id and key. name: bot-token description: Mint a GitHub App installation token for named marola-dev repos and make git commit as the App's bot user. @@ -32,12 +32,26 @@ outputs: runs: using: composite steps: - - name: refuse an empty repositories list + # The action's own parser would turn "\r\n" into [] and mint for every installed repo, so the list + # is cleaned here and the cleaned list is what the mint step gets. v3 skips empty permission-* + # inputs, and with none set the token holds every permission the App has. + - id: guard shell: bash env: REPOS: ${{ inputs.repositories }} + PERMS: ${{ inputs.permission-contents }}${{ inputs.permission-pull-requests }}${{ inputs.permission-workflows }}${{ inputs.permission-checks }}${{ inputs.permission-statuses }}${{ inputs.permission-issues }} run: | - [ -n "$(tr -d ' ,\n\t' <<<"$REPOS")" ] || { echo "::error::bot-token: repositories is empty; name every repo the token needs"; exit 1; } + set -euo pipefail + [ -n "${PERMS//[[:space:]]/}" ] || { echo "::error::bot-token: no permission-* input set; the token would hold every permission the App has"; exit 1; } + cleaned="" + while IFS= read -r repo; do + repo="${repo//[[:space:]]/}" + [ -n "$repo" ] || continue + [[ "$repo" =~ ^[A-Za-z0-9._-]+$ ]] || { echo "::error::bot-token: '$repo' is not a bare repo name in marola-dev"; exit 1; } + cleaned+="${cleaned:+,}$repo" + done < <(tr ',' '\n' <<<"$REPOS") + [ -n "$cleaned" ] || { echo "::error::bot-token: repositories is empty; name every repo the token needs"; exit 1; } + echo "repositories=$cleaned" >>"$GITHUB_OUTPUT" - id: mint uses: actions/create-github-app-token@v3 @@ -45,7 +59,7 @@ runs: app-id: ${{ inputs.app-id }} private-key: ${{ inputs.private-key }} owner: marola-dev - repositories: ${{ inputs.repositories }} + repositories: ${{ steps.guard.outputs.repositories }} permission-contents: ${{ inputs.permission-contents }} permission-pull-requests: ${{ inputs.permission-pull-requests }} permission-workflows: ${{ inputs.permission-workflows }} @@ -53,6 +67,7 @@ runs: permission-statuses: ${{ inputs.permission-statuses }} permission-issues: ${{ inputs.permission-issues }} + # GIT_CONFIG_GLOBAL, not `git config --global`, which would outlive the job on a self-hosted runner. # The commit email needs the bot user's id, which is not the App id (create-github-app-token's README). - id: who shell: bash @@ -63,6 +78,9 @@ runs: set -euo pipefail uid="$(gh api "users/${SLUG}%5Bbot%5D" --jq .id)" name="${SLUG}[bot]" email="${uid}+${SLUG}[bot]@users.noreply.github.com" - git config --global user.name "$name" - git config --global user.email "$email" + cfg="$RUNNER_TEMP/bot-token.gitconfig" + if [ -f ~/.gitconfig ]; then cp ~/.gitconfig "$cfg"; else : >"$cfg"; fi + git config --file "$cfg" user.name "$name" + git config --file "$cfg" user.email "$email" + echo "GIT_CONFIG_GLOBAL=$cfg" >>"$GITHUB_ENV" { echo "name=$name"; echo "email=$email"; } >>"$GITHUB_OUTPUT" diff --git a/.github/workflows/bot-token-test.yml b/.github/workflows/bot-token-test.yml index 4180e89..0ca4b8a 100644 --- a/.github/workflows/bot-token-test.yml +++ b/.github/workflows/bot-token-test.yml @@ -10,23 +10,47 @@ on: permissions: {} jobs: - refuses-empty-list: + refuses-unsafe-input: if: github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork runs-on: ubuntu-latest timeout-minutes: 5 steps: - uses: actions/checkout@v7 - - id: bot + - id: empty continue-on-error: true uses: ./.github/actions/bot-token with: app-id: ${{ vars.PR_BOT_APP_ID }} private-key: ${{ secrets.PR_BOT_KEY }} repositories: "" - - name: the empty list was refused + permission-contents: read + - id: blank + continue-on-error: true + uses: ./.github/actions/bot-token + with: + app-id: ${{ vars.PR_BOT_APP_ID }} + private-key: ${{ secrets.PR_BOT_KEY }} + repositories: "\r\n" + permission-contents: read + - id: qualified + continue-on-error: true + uses: ./.github/actions/bot-token + with: + app-id: ${{ vars.PR_BOT_APP_ID }} + private-key: ${{ secrets.PR_BOT_KEY }} + repositories: marola-dev/marola-devkit + permission-contents: read + - id: noperm + continue-on-error: true + uses: ./.github/actions/bot-token + with: + app-id: ${{ vars.PR_BOT_APP_ID }} + private-key: ${{ secrets.PR_BOT_KEY }} + repositories: marola-devkit + - name: every one was refused env: - OUTCOME: ${{ steps.bot.outcome }} - run: '[ "$OUTCOME" = failure ] || { echo "::error::bot-token minted with no repositories"; exit 1; }' + OUTCOMES: ${{ steps.empty.outcome }} ${{ steps.blank.outcome }} ${{ steps.qualified.outcome }} ${{ steps.noperm.outcome }} + run: '[ "$OUTCOMES" = "failure failure failure failure" ] || { echo "::error::bot-token accepted unsafe input: $OUTCOMES"; exit 1; }' mints-scoped: if: github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork @@ -40,8 +64,8 @@ jobs: app-id: ${{ vars.PR_BOT_APP_ID }} private-key: ${{ secrets.PR_BOT_KEY }} repositories: marola-devkit - permission-contents: read - - name: one repo, no pull-request write, git commits as the bot + permission-contents: write + - name: one repo, contents only, git commits as the bot env: GH_TOKEN: ${{ steps.bot.outputs.token }} SLUG: ${{ steps.bot.outputs.app-slug }} @@ -53,8 +77,10 @@ jobs: # An invalid body: 403 means no permission, 422 would mean the permission was granted. probe() { curl -s -o /dev/null -w '%{http_code}' -X POST -H "Authorization: Bearer $GH_TOKEN" \ -H 'Accept: application/vnd.github+json' "https://api.github.com/repos/$1" -d '{}'; } + [ "$(probe marola-dev/marola-devkit/git/refs)" = 422 ] || { echo "::error::contents write was not granted"; exit 1; } [ "$(probe marola-dev/marola-devkit/pulls)" = 403 ] || { echo "::error::pull_requests was granted"; exit 1; } - [ "$(probe marola-dev/marola-site/git/refs)" != 422 ] || { echo "::error::the token can write marola-site"; exit 1; } + # A repo outside the token reads as 404 to an installation token, 403 where it is visible. + case "$(probe marola-dev/marola-site/git/refs)" in 403|404) ;; *) echo "::error::the token can write marola-site"; exit 1;; esac [[ "$(git config --global user.email)" =~ ^[0-9]+\+${SLUG}\[bot\]@users\.noreply\.github\.com$ ]] || { echo "::error::git email is $(git config --global user.email)"; exit 1; } [ "$(git config --global user.email)" = "$EMAIL" ] [ "$(git config --global user.name)" = "${SLUG}[bot]" ] From 7ee391923230924413b42af524db82b425bf84c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 21:46:04 +0200 Subject: [PATCH 04/13] =?UTF-8?q?feat(merge-key-check):=20keep=20MERGE=5FB?= =?UTF-8?q?OT=5FKEY=20out=20of=20jobs=20that=20run=20third-party=20code=20?= =?UTF-8?q?(MIP-0086=20=C2=A75.6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tested: merge_key_check.py --self-test red then green; devkit quality gate Co-Authored-By: Claude --- flake.nix | 1 + justfile | 1 + scripts/merge_key_check.py | 122 +++++++++++++++++++++++++++++++++++++ tests/self-tests.sh | 2 +- 4 files changed, 125 insertions(+), 1 deletion(-) create mode 100755 scripts/merge_key_check.py diff --git a/flake.nix b/flake.nix index 671c8ef..a6c6ce3 100644 --- a/flake.nix +++ b/flake.nix @@ -47,6 +47,7 @@ deps-stack = "scripts/deps-stack.sh"; deps-merge = "scripts/deps-merge.sh"; ruleset-sync = "scripts/ruleset-sync.sh"; + merge-key-check = "scripts/merge_key_check.py"; api-docs-push = "scripts/api-docs-push.sh"; gha-runner = "scripts/gha-runner.sh"; setup-runners = "scripts/setup-runners.sh"; diff --git a/justfile b/justfile index d8f87d6..bca1ffd 100644 --- a/justfile +++ b/justfile @@ -21,6 +21,7 @@ quality: actionlint bash tests/self-tests.sh scripts/agents-check.sh + python3 scripts/merge_key_check.py . python3 scripts/docs_lint.py python3 scripts/skills_vendor.py check --lock plugins/marola-devkit/skills/skills.lock python3 scripts/release.py --check diff --git a/scripts/merge_key_check.py b/scripts/merge_key_check.py new file mode 100755 index 0000000..ff1ddd9 --- /dev/null +++ b/scripts/merge_key_check.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python3 +"""merge-key-check: MERGE_BOT_KEY only where MIP-0086 §5.6 allows it. + +merge-key-check [ROOT] # ROOT/.github/workflows against ROOT/.github/merge-key-allowlist +merge-key-check --self-test +""" + +from __future__ import annotations + +import argparse +import re +import sys +import tempfile +from pathlib import Path + +import yaml + +KEY = "MERGE_BOT_KEY" +ALLOWLIST = ".github/merge-key-allowlist" +USES_OK = re.compile( + r"^(actions/checkout|actions/create-github-app-token|actions/download-artifact" + r"|marola-dev/marola-devkit/\.github/actions/bot-token|\./\.github/actions/bot-token)(@|$)" +) +# Whatever fetches or builds third-party code must not share a job with the key. +RUN_BAD = re.compile(r"(? set[str]: + f = root / ALLOWLIST + if not f.is_file(): + return set() + lines = (ln.strip() for ln in f.read_text().splitlines()) + return {ln for ln in lines if ln and not ln.startswith("#")} + + +def problems(root: Path) -> list[str]: + allow = allowed(root) + out = [ + f"{ALLOWLIST}: {rel} does not exist" for rel in sorted(allow) if not (root / rel).is_file() + ] + for wf in sorted((root / ".github" / "workflows").glob("*.y*ml")): + rel = wf.relative_to(root).as_posix() + text = wf.read_text() + if KEY not in text: + continue + if rel not in allow: + out.append(f"{rel}: names {KEY} but is not in {ALLOWLIST}") + continue + doc = yaml.safe_load(text) or {} + if KEY in yaml.safe_dump(doc.get("env") or {}): + out.append(f"{rel}: workflow-level env hands {KEY} to every job") + for name, job in (doc.get("jobs") or {}).items(): + if KEY not in yaml.safe_dump(job): + continue + for i, step in enumerate(job.get("steps") or [], 1): + uses = step.get("uses") + if uses and not USES_OK.match(uses): + out.append(f"{rel}: job {name}, step {i}: `{uses}` runs beside {KEY}") + m = RUN_BAD.search(step.get("run") or "") + if m: + out.append(f"{rel}: job {name}, step {i}: `{m.group(1)}` runs beside {KEY}") + return out + + +def self_test() -> None: + def case(files: dict[str, str]) -> list[str]: + with tempfile.TemporaryDirectory() as d: + root = Path(d) + (root / ".github" / "workflows").mkdir(parents=True) + for rel, text in files.items(): + (root / rel).parent.mkdir(parents=True, exist_ok=True) + (root / rel).write_text(text) + return problems(root) + + wf = ".github/workflows/merge.yml" + clean = ( + "jobs:\n merge:\n runs-on: ubuntu-latest\n steps:\n" + " - uses: actions/checkout@v7\n" + " - id: bot\n uses: ./.github/actions/bot-token\n" + " with:\n private-key: ${{ secrets.MERGE_BOT_KEY }}\n" + " - run: scripts/merge.sh\n" + ) + keyless = "\n build:\n runs-on: ubuntu-latest\n steps:\n - run: nix develop --command just build\n" + assert case({".github/workflows/ci.yml": "jobs: {}\n"}) == [], "no_reference_is_clean" + got = case({wf: clean}) + assert len(got) == 1 and "not in" in got[0], f"unlisted_file_fails: {got}" + assert case({wf: clean, ALLOWLIST: f"# merge jobs\n{wf}\n"}) == [], "listed_clean_job_passes" + got = case({wf: clean + " - run: nix develop --command true\n", ALLOWLIST: wf}) + assert len(got) == 1 and "`nix`" in got[0], f"nix_beside_key_fails: {got}" + got = case({wf: clean + " - uses: some/action@v1\n", ALLOWLIST: wf}) + assert len(got) == 1 and "some/action" in got[0], f"third_party_action_fails: {got}" + got = case( + {wf: clean.replace("actions/checkout@v7", "actions/checkout-evil@v1"), ALLOWLIST: wf} + ) + assert len(got) == 1, f"lookalike_action_fails: {got}" + assert case({wf: clean + keyless, ALLOWLIST: wf}) == [], "nix_in_a_keyless_job_passes" + got = case({wf: "env:\n K: ${{ secrets.MERGE_BOT_KEY }}\n" + clean, ALLOWLIST: wf}) + assert any("workflow-level" in p for p in got), f"workflow_env_fails: {got}" + got = case({ALLOWLIST: ".github/workflows/gone.yml\n"}) + assert len(got) == 1 and "does not exist" in got[0], f"stale_allowlist_entry_fails: {got}" + assert not RUN_BAD.search("set -euo pipefail\nnixpkgs=1 scripts/x.sh /nix/store/y"), ( + "no_false_positive" + ) + print("merge-key-check self-test: ok") + + +def main() -> int: + ap = argparse.ArgumentParser(description=(__doc__ or "").splitlines()[0]) + ap.add_argument("root", nargs="?", default=".") + ap.add_argument("--self-test", action="store_true") + a = ap.parse_args() + if a.self_test: + self_test() + return 0 + found = problems(Path(a.root)) + for p in found: + print(f"merge-key-check: {p}", file=sys.stderr) + return 1 if found else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/self-tests.sh b/tests/self-tests.sh index 1cc3e20..7725cc1 100755 --- a/tests/self-tests.sh +++ b/tests/self-tests.sh @@ -15,7 +15,7 @@ sh_tests=( ) py_tests=( scripts/gemini_review.py scripts/pr_label_nlp.py scripts/workflow_runners.py scripts/docs_lint.py - scripts/skills_vendor.py scripts/release.py scripts/wiring.py + scripts/skills_vendor.py scripts/release.py scripts/wiring.py scripts/merge_key_check.py scripts/lib/req_merge.py scripts/lib/uses_merge.py scripts/lib/mip_index_merge.py scripts/lib/tasks_issues.py plugins/marola-devkit/skills/voice-note-ingest/scripts/transcribe.py ) From 791bff45123f8fc1b798d79052fee034d5373de0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 21:49:53 +0200 Subject: [PATCH 05/13] fix(merge-key-check): case-insensitive key match; flag reusable calls, containers and custom shells Tested: merge_key_check.py --self-test red then green; devkit quality gate Co-Authored-By: Claude --- scripts/merge_key_check.py | 48 +++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/scripts/merge_key_check.py b/scripts/merge_key_check.py index ff1ddd9..d405d99 100755 --- a/scripts/merge_key_check.py +++ b/scripts/merge_key_check.py @@ -21,6 +21,9 @@ r"^(actions/checkout|actions/create-github-app-token|actions/download-artifact" r"|marola-dev/marola-devkit/\.github/actions/bot-token|\./\.github/actions/bot-token)(@|$)" ) +# Secret names are case-insensitive when referenced, so every presence test is too. +HAS_KEY = re.compile(KEY, re.I) +SHELL_OK = re.compile(r"^(bash|sh)(\s|$)") # Whatever fetches or builds third-party code must not share a job with the key. RUN_BAD = re.compile(r"(? list[str]: for wf in sorted((root / ".github" / "workflows").glob("*.y*ml")): rel = wf.relative_to(root).as_posix() text = wf.read_text() - if KEY not in text: + if not HAS_KEY.search(text): continue if rel not in allow: out.append(f"{rel}: names {KEY} but is not in {ALLOWLIST}") continue doc = yaml.safe_load(text) or {} - if KEY in yaml.safe_dump(doc.get("env") or {}): + if HAS_KEY.search(yaml.safe_dump(doc.get("env") or {})): out.append(f"{rel}: workflow-level env hands {KEY} to every job") for name, job in (doc.get("jobs") or {}).items(): - if KEY not in yaml.safe_dump(job): + if job.get("secrets") == "inherit": + out.append(f"{rel}: job {name}: `secrets: inherit` hands {KEY} to the callee") + if not HAS_KEY.search(yaml.safe_dump(job)): continue + if job.get("uses"): + out.append(f"{rel}: job {name}: reusable workflow call runs beside {KEY}") + for field in ("container", "services"): + if job.get(field): + out.append(f"{rel}: job {name}: `{field}` runs images beside {KEY}") for i, step in enumerate(job.get("steps") or [], 1): uses = step.get("uses") if uses and not USES_OK.match(uses): out.append(f"{rel}: job {name}, step {i}: `{uses}` runs beside {KEY}") + sh = step.get("shell") + if sh and not SHELL_OK.match(sh): + out.append(f"{rel}: job {name}, step {i}: shell `{sh}` runs beside {KEY}") m = RUN_BAD.search(step.get("run") or "") if m: out.append(f"{rel}: job {name}, step {i}: `{m.group(1)}` runs beside {KEY}") @@ -98,6 +111,35 @@ def case(files: dict[str, str]) -> list[str]: assert any("workflow-level" in p for p in got), f"workflow_env_fails: {got}" got = case({ALLOWLIST: ".github/workflows/gone.yml\n"}) assert len(got) == 1 and "does not exist" in got[0], f"stale_allowlist_entry_fails: {got}" + lower = clean.replace(KEY, KEY.lower()) + got = case({wf: lower + " - run: npm ci\n"}) + assert len(got) == 1 and "not in" in got[0], f"lowercase_unlisted_fails: {got}" + got = case({wf: lower + " - run: nix develop\n", ALLOWLIST: wf}) + assert len(got) == 1 and "`nix`" in got[0], f"lowercase_job_checked: {got}" + call = "\n call:\n uses: o/r/.github/workflows/x.yml@v1\n secrets:\n k: ${{ secrets.MERGE_BOT_KEY }}\n" + got = case({wf: clean + call, ALLOWLIST: wf}) + assert len(got) == 1 and "reusable workflow" in got[0], f"reusable_call_fails: {got}" + inherit = "\n call:\n uses: o/r/.github/workflows/x.yml@v1\n secrets: inherit\n" + got = case({wf: clean + inherit, ALLOWLIST: wf}) + assert any("inherit" in p for p in got), f"secrets_inherit_fails: {got}" + got = case( + {wf: clean.replace(" steps:", " container: node:20\n steps:"), ALLOWLIST: wf} + ) + assert len(got) == 1 and "container" in got[0], f"container_fails: {got}" + got = case( + { + wf: clean.replace( + " steps:", " services:\n db:\n image: x\n steps:" + ), + ALLOWLIST: wf, + } + ) + assert len(got) == 1 and "services" in got[0], f"services_fails: {got}" + got = case({wf: clean + " shell: nix develop --command bash {0}\n", ALLOWLIST: wf}) + assert len(got) == 1 and "shell" in got[0], f"custom_shell_fails: {got}" + assert case({wf: clean + " shell: bash -e {0}\n", ALLOWLIST: wf}) == [], ( + "bash_shell_passes" + ) assert not RUN_BAD.search("set -euo pipefail\nnixpkgs=1 scripts/x.sh /nix/store/y"), ( "no_false_positive" ) From f6818570882c3ee09ca652da52968363acf206f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 21:53:06 +0200 Subject: [PATCH 06/13] fix(merge-key-check): check defaults.run.shell and flag secrets: inherit in any workflow once an allowlist exists Tested: merge_key_check.py --self-test red then green; devkit quality gate Co-Authored-By: Claude --- scripts/merge_key_check.py | 45 +++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/scripts/merge_key_check.py b/scripts/merge_key_check.py index d405d99..fcac8ef 100755 --- a/scripts/merge_key_check.py +++ b/scripts/merge_key_check.py @@ -36,6 +36,13 @@ def allowed(root: Path) -> set[str]: return {ln for ln in lines if ln and not ln.startswith("#")} +def default_shell(rel: str, where: str, node: dict) -> list[str]: + sh = ((node.get("defaults") or {}).get("run") or {}).get("shell") + if sh and not SHELL_OK.match(sh): + return [f"{rel}: {where}: defaults.run.shell `{sh}` runs beside {KEY}"] + return [] + + def problems(root: Path) -> list[str]: allow = allowed(root) out = [ @@ -44,19 +51,26 @@ def problems(root: Path) -> list[str]: for wf in sorted((root / ".github" / "workflows").glob("*.y*ml")): rel = wf.relative_to(root).as_posix() text = wf.read_text() - if not HAS_KEY.search(text): - continue - if rel not in allow: - out.append(f"{rel}: names {KEY} but is not in {ALLOWLIST}") - continue doc = yaml.safe_load(text) or {} + if not HAS_KEY.search(text) or rel not in allow: + if HAS_KEY.search(text): + out.append(f"{rel}: names {KEY} but is not in {ALLOWLIST}") + if (root / ALLOWLIST).is_file(): + out += [ + f"{rel}: job {n}: `secrets: inherit` hands {KEY} to the callee" + for n, j in (doc.get("jobs") or {}).items() + if j.get("secrets") == "inherit" + ] + continue if HAS_KEY.search(yaml.safe_dump(doc.get("env") or {})): out.append(f"{rel}: workflow-level env hands {KEY} to every job") + out += default_shell(rel, "workflow", doc) for name, job in (doc.get("jobs") or {}).items(): if job.get("secrets") == "inherit": out.append(f"{rel}: job {name}: `secrets: inherit` hands {KEY} to the callee") if not HAS_KEY.search(yaml.safe_dump(job)): continue + out += default_shell(rel, f"job {name}", job) if job.get("uses"): out.append(f"{rel}: job {name}: reusable workflow call runs beside {KEY}") for field in ("container", "services"): @@ -140,6 +154,27 @@ def case(files: dict[str, str]) -> list[str]: assert case({wf: clean + " shell: bash -e {0}\n", ALLOWLIST: wf}) == [], ( "bash_shell_passes" ) + got = case({wf: "env:\n K: ${{ secrets.merge_bot_key }}\n" + clean, ALLOWLIST: wf}) + assert any("workflow-level" in p for p in got), f"lowercase_workflow_env_fails: {got}" + nixsh = "nix develop --command bash {0}" + got = case({wf: f"defaults:\n run:\n shell: {nixsh}\n" + clean, ALLOWLIST: wf}) + assert any("defaults" in p for p in got), f"workflow_default_shell_fails: {got}" + got = case( + { + wf: clean.replace( + " steps:", f" defaults:\n run:\n shell: {nixsh}\n steps:" + ), + ALLOWLIST: wf, + } + ) + assert any("defaults" in p for p in got), f"job_default_shell_fails: {got}" + other = ".github/workflows/other.yml" + plain = "jobs:\n c:\n uses: o/r/.github/workflows/x.yml@v1\n secrets: inherit\n" + got = case({wf: clean, other: plain, ALLOWLIST: wf}) + assert len(got) == 1 and other in got[0] and "inherit" in got[0], ( + f"unlisted_inherit_fails: {got}" + ) + assert case({other: plain}) == [], "inherit_without_allowlist_passes" assert not RUN_BAD.search("set -euo pipefail\nnixpkgs=1 scripts/x.sh /nix/store/y"), ( "no_false_positive" ) From 74c9408ea04a542378158c507f674bd527cd20e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 21:55:40 +0200 Subject: [PATCH 07/13] feat(notify-umbrella, gemini-review): marola-pr-bot first, the old names as fallback (MIP-0086) notify-umbrella mints a marola-pr-bot token for the umbrella from pr_bot_key and still reads the old PAT in `token` when it is absent. gemini-review reads PR_BOT_* first, falls back to GEMINI_*, and no longer requires GEMINI_APP_PRIVATE_KEY. Tested: actionlint; devkit quality gate Co-Authored-By: Claude --- .github/workflows/gemini-review.yml | 13 ++++++---- .github/workflows/notify-umbrella.yml | 35 ++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/.github/workflows/gemini-review.yml b/.github/workflows/gemini-review.yml index 02e98bf..d018582 100644 --- a/.github/workflows/gemini-review.yml +++ b/.github/workflows/gemini-review.yml @@ -6,8 +6,9 @@ # model, which an agent loop spent on a single PR. The fix is applied from that same answer, only # where its `original` text still matches, and never under .github/. # -# Identity is the org App behind vars.GEMINI_APP_ID / secrets.GEMINI_APP_PRIVATE_KEY, not -# GITHUB_TOKEN: a push made with GITHUB_TOKEN starts no CI on the PR. +# Identity is `marola-pr-bot` (vars.PR_BOT_APP_ID / secrets.PR_BOT_KEY, MIP-0086), falling back to +# the GEMINI_* names until they are deleted, not GITHUB_TOKEN: a push made with GITHUB_TOKEN starts +# no CI on the PR. # # Callers trigger on pull_request_target, so a fork PR gets the secrets too (only a maintainer can # request the team). A fork's head is checked out as text into .pr-head and never run; it gets a @@ -51,7 +52,9 @@ on: GEMINI_API_KEY: required: true GEMINI_APP_PRIVATE_KEY: - required: true + required: false + PR_BOT_KEY: + required: false concurrency: group: gemini-review-${{ github.event.pull_request.number }} @@ -72,8 +75,8 @@ jobs: id: app uses: actions/create-github-app-token@v2 with: - app-id: ${{ vars.GEMINI_APP_ID }} - private-key: ${{ secrets.GEMINI_APP_PRIVATE_KEY }} + app-id: ${{ vars.PR_BOT_APP_ID || vars.GEMINI_APP_ID }} + private-key: ${{ secrets.PR_BOT_KEY || secrets.GEMINI_APP_PRIVATE_KEY }} permission-contents: write permission-pull-requests: write diff --git a/.github/workflows/notify-umbrella.yml b/.github/workflows/notify-umbrella.yml index f4407b0..e20b52f 100644 --- a/.github/workflows/notify-umbrella.yml +++ b/.github/workflows/notify-umbrella.yml @@ -15,11 +15,12 @@ # notify: # uses: marola-dev/marola-devkit/.github/workflows/notify-umbrella.yml@v0.9.0 # secrets: -# token: ${{ secrets.MAROLA_CROSS_REPO_PAT }} +# pr_bot_key: ${{ secrets.PR_BOT_KEY }} # -# MAROLA_CROSS_REPO_PAT: the org's fine-grained PAT with Contents: read & write on the umbrella — -# repository_dispatch needs it; GITHUB_TOKEN cannot reach another repository. Unset is a notice, -# never a failure: the umbrella's own daily cron still picks the change up. +# pr_bot_key: marola-pr-bot's key (MIP-0086), minted here into a token for the umbrella only, with +# Contents write, which repository_dispatch needs. `token` is the old PAT, read only when +# pr_bot_key is absent, until every caller moves (MIP-0086 row 16). Neither set is a notice, never a +# failure: the umbrella's own daily cron still picks the change up. name: notify-umbrella on: @@ -34,8 +35,11 @@ on: type: string default: ubuntu-latest secrets: + pr_bot_key: + description: "secrets.PR_BOT_KEY" + required: false token: - description: "Fine-grained PAT with Contents: read & write on the umbrella." + description: "Deprecated: the old PAT, read only when pr_bot_key is absent." required: false permissions: {} @@ -44,6 +48,8 @@ jobs: dispatch: runs-on: ${{ inputs.runner }} timeout-minutes: 2 + env: + HAS_PR_BOT_KEY: ${{ secrets.pr_bot_key != '' }} steps: # A reusable workflow can only keep or lower the caller's permissions, and every caller sets # permissions: {} — so this reads the compare API instead of checking the repo out. Even a @@ -75,9 +81,26 @@ jobs: fi echo "changed=$changed" >>"$GITHUB_OUTPUT" + - name: umbrella owner and name + id: um + env: + UMBRELLA: ${{ inputs.umbrella }} + run: echo "owner=${UMBRELLA%%/*}" >>"$GITHUB_OUTPUT"; echo "name=${UMBRELLA#*/}" >>"$GITHUB_OUTPUT" + + - name: marola-pr-bot token for the umbrella + id: app + if: env.HAS_PR_BOT_KEY == 'true' + uses: actions/create-github-app-token@v3 + with: + app-id: ${{ vars.PR_BOT_APP_ID }} + private-key: ${{ secrets.pr_bot_key }} + owner: ${{ steps.um.outputs.owner }} + repositories: ${{ steps.um.outputs.name }} + permission-contents: write + - name: repository_dispatch -> ${{ inputs.umbrella }} env: - TOKEN: ${{ secrets.token }} + TOKEN: ${{ steps.app.outputs.token || secrets.token }} UMBRELLA: ${{ inputs.umbrella }} REPO: ${{ github.repository }} SHA: ${{ github.sha }} From 40d9b3a69267c93b32a58024dd5144013c2367f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 21:57:16 +0200 Subject: [PATCH 08/13] docs: marola-pr-bot, bot-token, merge-key-check and pr_bot_key (MIP-0086) Tested: actionlint; devkit quality gate Co-Authored-By: Claude --- AGENTS.md | 2 +- CHANGELOG.md | 5 +++++ README.md | 6 +++--- docs/4-reference_config.md | 3 ++- docs/4-reference_tools.md | 1 + docs/4-reference_workflows.md | 32 +++++++++++++++++++++++++------- 6 files changed, 37 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d0e260c..af06fc3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,7 +83,7 @@ An agent starts work only on an issue carrying `agent-ready`, in this repo (MIP- Commits carry `Tested:` and `Co-Authored-By: Claude `, nothing else. `.claude/settings.json` sets the attribution; `pr-flow` (`just pr`) fills a missing `Tested:`. -The one exception is `gemini-review`'s fix commit, written by Gemini as `marola-gemini-bot`: its +The one exception is `gemini-review`'s fix commit, written by Gemini as `marola-pr-bot`: its second trailer is `Co-Authored-By: Gemini `. ## Phase discipline (hard rule) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76939f5..2c478c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ marketplace `ref` at once. - `gemini-review.yml` has no `cost-trailer` input any more; a caller that passes it must remove it. - `gemini-review.yml` falls back to `gemini-3.6-flash`; Google deprecated `gemini-3.5-flash` and redirects it there (#106). An org or repo `GEMINI_MODEL` variable still wins. +- `.github/actions/bot-token` mints a `marola-pr-bot` or `marola-merge-bot` token for named repos + (MIP-0086 §5.4); `release.py --consumer` moves its pin. `merge-key-check` runs in `just quality` + (§5.6). `notify-umbrella` takes `pr_bot_key` (the `token` PAT is still read as fallback); + `gemini-review` reads `PR_BOT_*` with `GEMINI_*` as fallback, and `GEMINI_APP_PRIVATE_KEY` is no + longer required (#79). ## v0.8.3 — 2026-10-10 diff --git a/README.md b/README.md index d5626b5..4c23c8c 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,7 @@ version, not its `main` (MIP-0074 §5.3). Their first real run is in a consuming **Gemini review on request** — on a PR, request the reviewer `marola-dev/gemini` (sidebar → Reviewers, or `gh pr edit --add-reviewer marola-dev/gemini`). A few minutes later -`marola-gemini-bot` posts one review: a short summary and at most 10 inline comments tagged +`marola-pr-bot` posts one review: a short summary and at most 10 inline comments tagged `[high]`/`[medium]`/`[low]`, with a suggestion block where the fix is exact. It then pushes one `fix: apply Gemini review findings` commit with the fixes whose original text still matches, after the repo's `check-command` passes, and comments what it fixed and what it left. Request the @@ -239,8 +239,8 @@ jobs: check-command: "" # a fast gate that needs no Nix, e.g. node scripts/site_check.js ``` -It needs the org's `marola-gemini-bot` App installed on the repo, the `gemini` team with Read on -it, and the org's `GEMINI_API_KEY`, `GEMINI_APP_PRIVATE_KEY` and `GEMINI_APP_ID`. One Gemini API +It needs the org's `marola-pr-bot` App installed on the repo, the `gemini` team with Read on +it, and the org's `GEMINI_API_KEY`, `PR_BOT_KEY` and `PR_BOT_APP_ID` (the `GEMINI_APP_*` names still work). One Gemini API call per review keeps it on the free tier, about 20 reviews a day per model; `vars.GEMINI_MODEL` picks another model. This repo calls it from `gemini.yml`, with `bash tests/self-tests.sh` as the check. Details: [docs/4-reference_workflows.md](docs/4-reference_workflows.md#gemini-review). diff --git a/docs/4-reference_config.md b/docs/4-reference_config.md index 49d045e..75549c2 100644 --- a/docs/4-reference_config.md +++ b/docs/4-reference_config.md @@ -21,7 +21,8 @@ has a default, so a repo that sets none of them gets the org's behaviour. | `MAROLA_RUNNER_SOURCE` | `setup-runners` | `~/code/actions-runner`, then `~/actions-runner` | An unpacked actions-runner release to copy from | | `MAROLA_OBSIDIAN_VAULT` | the `obsidian-vault` skill | `~/Documents/2nd-brain` | The maintainer's Obsidian vault | | `MAROLA_BUMP_PAT` | `bump-consumers.yml` | — | Not an environment variable: the org secret, readable by marola-devkit alone, whose PAT opens the pin-bump PRs ([releases](3-development.md#releases)) | -| `MAROLA_CROSS_REPO_PAT` | a caller of `notify-umbrella` | — | Not an environment variable: the repository secret a caller passes as the workflow's `token` ([workflows](4-reference_workflows.md#notify-umbrella)) | +| `PR_BOT_KEY`, `PR_BOT_APP_ID` | a caller of `notify-umbrella` and `gemini-review` | — | Not environment variables: the org secret and variable of the `marola-pr-bot` App, passed as `notify-umbrella`'s `pr_bot_key` ([workflows](4-reference_workflows.md#notify-umbrella)) | +| `MAROLA_CROSS_REPO_PAT` | a caller of `notify-umbrella` | — | Deprecated fallback, read only without `pr_bot_key`: the repository secret a caller passes as the workflow's `token` | `MAROLA_TRACES` also appears in `scripts/`, but only as sample text in `tasks_issues.py`'s self-test (a marola-app task row); the devkit does not read it. diff --git a/docs/4-reference_tools.md b/docs/4-reference_tools.md index 087e938..736728b 100644 --- a/docs/4-reference_tools.md +++ b/docs/4-reference_tools.md @@ -54,6 +54,7 @@ extension rather than a devkit tool. | `backfill-pr-labels` | `backfill-pr-labels.sh` | `just pr-labels-backfill` | The same for every merged or closed PR with no label; `--limit`, `--nlp` (compare with `pr-label-nlp`), `--nlp-apply-unscoped` (fill only an `area/unscoped` gap) | | `pr-label-nlp` | `pr_label_nlp.py` | (none) | A local TF-IDF classifier for `area/*`, kept to compare against the deterministic one, never applied on its own. `--pr N` or `--title`/`--body` | | `ruleset-sync` | `ruleset-sync.sh` | `just rulesets-check`, `just rulesets-apply` | `check [owner/repo…]` diffs a repo's live branch ruleset against `.github/rulesets/main-rule.json`, plus that repo's bypass actors in `.github/rulesets/bypass-extras.json` (each with a reason); `apply ` creates or updates it; `--all-org ORG` covers an org | +| `merge-key-check` | `merge_key_check.py` | `just quality` | Fails when `MERGE_BOT_KEY` appears in a workflow job outside the allowlist in `.github/merge-key-allowlist`, or in a job that fetches or builds third-party code (MIP-0086 §5.6). Once the allowlist exists, `secrets: inherit` is flagged in every workflow of the repo | | `agents-check` | `agents-check.sh` | `just agents-check` | Compare AGENTS.md's invariants block byte for byte with the pinned devkit's `agents/invariants.md` (`--block` or `MAROLA_INVARIANTS_BLOCK` to use another) | | `docs-lint` | `docs_lint.py` | (none) | MIP-0074 §7's stale-content check over `README.md` and `docs/**/*.md`: undefined or unmarked foreign recipes, another repo's paths, split-era wording, `docs/index.md`, links leaving the repo | | `wiring` | `wiring.py` | `just wiring` | MIP-0076 §5.1's four tables (artifact, dispatch, pin bump, deploy), parsed from the umbrella's and every submodule's workflows, pin files, scripts, justfiles, Dockerfiles, compose files and `build.sbt`, with each devkit workflow call's ref (each call is read at that ref: from a local devkit clone's tags, else fetched from GitHub, a tag cached under `~/.cache/marola-wiring`). `wiring [FILE]` prints the block or rewrites it between FILE's `wiring:start`/`wiring:end` markers; `--check [FILE]` fails on a stale block, a dispatch sent or listened for on one side only, or an artifact nobody reads that the umbrella's `wiring.allow` (`: ` per line) does not list; run it in an umbrella checkout with submodules | diff --git a/docs/4-reference_workflows.md b/docs/4-reference_workflows.md index 68d043c..9b217f9 100644 --- a/docs/4-reference_workflows.md +++ b/docs/4-reference_workflows.md @@ -132,7 +132,7 @@ jobs: notify: uses: marola-dev/marola-devkit/.github/workflows/notify-umbrella.yml@v0.9.0 secrets: - token: ${{ secrets.MAROLA_CROSS_REPO_PAT }} + pr_bot_key: ${{ secrets.PR_BOT_KEY }} ``` | Input | Default | Notes | @@ -140,10 +140,12 @@ jobs: | `umbrella` | `marola-dev/marola` | MAROLA_UMBRELLA, MIP-0070 §5.6 | | `runner` | `ubuntu-latest` | resolved in the *caller's* repo | -**Secret** `token` (optional): every repo passes the org secret `MAROLA_CROSS_REPO_PAT`, a -fine-grained PAT with Contents: read & write on the umbrella (`repository_dispatch` needs it; -`GITHUB_TOKEN` cannot reach another repo). Unset is a notice, not a failure — the umbrella's daily -cron still catches the change. +**Secret** `pr_bot_key` (optional): every repo passes the org secret `PR_BOT_KEY`, and the workflow +mints a `marola-pr-bot` token (`vars.PR_BOT_APP_ID`) for the umbrella alone, with Contents write +(`repository_dispatch` needs it; `GITHUB_TOKEN` cannot reach another repo). **Secret** `token` is +the deprecated fallback, the old `MAROLA_CROSS_REPO_PAT`, read only when `pr_bot_key` is absent +(MIP-0086). With neither set it is a notice, not a failure — the umbrella's daily cron still +catches the change. A zero `github.event.before` (a branch's first push), a compare API call that doesn't answer 200 (e.g. `before` unknown to it after a force-push), or a truncated compare (the API caps its file @@ -370,8 +372,10 @@ jobs: | `extra-trailers` | `""` | e.g. marola-site's `MIP: none — Gemini review fix` | | `gemini-model` | `""` | falls back to `vars.GEMINI_MODEL`, then `gemini-3.6-flash` | -Secrets: `GEMINI_API_KEY` and `GEMINI_APP_PRIVATE_KEY`; variable `GEMINI_APP_ID`. All three are -set at the org, scoped to the repos the `marola-gemini-bot` App is installed on. The App needs +Secrets: `GEMINI_API_KEY` and `PR_BOT_KEY`; variable `PR_BOT_APP_ID`. All three are set at the +org, scoped to the repos the `marola-pr-bot` App is installed on. `GEMINI_APP_PRIVATE_KEY` and +`GEMINI_APP_ID` are read when the `PR_BOT_*` names are unset, and no caller needs to pass +`GEMINI_APP_PRIVATE_KEY` any more (MIP-0086). The App needs Contents and Pull requests read/write and no Workflows permission, so GitHub itself refuses a push from it that edits a workflow; a push with its token, unlike `GITHUB_TOKEN`'s, starts the PR's CI. @@ -408,6 +412,20 @@ jobs: Secret `token`, optional: a PR opened with the default `GITHUB_TOKEN` starts no CI; one opened with a PAT or App token does. +## bot-token (composite action) + +`uses: marola-dev/marola-devkit/.github/actions/bot-token@v0.9.0` mints an installation token for +`marola-pr-bot` or `marola-merge-bot` on named `marola-dev` repos, and points git at that bot's +identity for the rest of the job (MIP-0086 §5.4). A composite action cannot read secrets, so the +caller passes `app-id` (`vars.PR_BOT_APP_ID` or `vars.MERGE_BOT_APP_ID`) and `private-key` +(`secrets.PR_BOT_KEY` or `secrets.MERGE_BOT_KEY`). `repositories` is required, a comma- or +newline-separated list of repo names; an empty list fails the step, because the token would +otherwise cover every repo the App is installed on, and so does a call with no `permission-*` input +set (`permission-contents`, `-pull-requests`, `-workflows`, `-checks`, `-statuses`, `-issues`). +Outputs: `token`, `app-slug`, `installation-id`, `user-name` and `user-email`. The git identity is +job-scoped, through `GIT_CONFIG_GLOBAL` rather than `git config --global`. `release.py --consumer` +moves its `@v…` pin with the other devkit pins. + ## devkit-ci (not reusable) This repo's own CI: installs Nix, creates or verifies `flake.lock`, runs `nix flake check`, then From 7f26bc0f31c3c9f167c371e1c6dcdffaee500d3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 22:06:26 +0200 Subject: [PATCH 09/13] fix(merge-key-check): allow a listed local reusable workflow; one inherit message A keyed job may call a local reusable workflow only when that file is itself on the allowlist, so the callee is scanned under the same rules. Tested: merge_key_check.py --self-test (red before, green after), merge_key_check.py ., ruff Co-Authored-By: Claude --- scripts/merge_key_check.py | 44 ++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/scripts/merge_key_check.py b/scripts/merge_key_check.py index fcac8ef..7012909 100755 --- a/scripts/merge_key_check.py +++ b/scripts/merge_key_check.py @@ -21,6 +21,7 @@ r"^(actions/checkout|actions/create-github-app-token|actions/download-artifact" r"|marola-dev/marola-devkit/\.github/actions/bot-token|\./\.github/actions/bot-token)(@|$)" ) +LOCAL_WF = re.compile(r"^\./\.github/workflows/[^/@]+$") # Secret names are case-insensitive when referenced, so every presence test is too. HAS_KEY = re.compile(KEY, re.I) SHELL_OK = re.compile(r"^(bash|sh)(\s|$)") @@ -52,26 +53,22 @@ def problems(root: Path) -> list[str]: rel = wf.relative_to(root).as_posix() text = wf.read_text() doc = yaml.safe_load(text) or {} - if not HAS_KEY.search(text) or rel not in allow: - if HAS_KEY.search(text): - out.append(f"{rel}: names {KEY} but is not in {ALLOWLIST}") - if (root / ALLOWLIST).is_file(): - out += [ - f"{rel}: job {n}: `secrets: inherit` hands {KEY} to the callee" - for n, j in (doc.get("jobs") or {}).items() - if j.get("secrets") == "inherit" - ] - continue - if HAS_KEY.search(yaml.safe_dump(doc.get("env") or {})): - out.append(f"{rel}: workflow-level env hands {KEY} to every job") - out += default_shell(rel, "workflow", doc) + keyed = bool(HAS_KEY.search(text)) and rel in allow + if HAS_KEY.search(text) and not keyed: + out.append(f"{rel}: names {KEY} but is not in {ALLOWLIST}") + if keyed: + if HAS_KEY.search(yaml.safe_dump(doc.get("env") or {})): + out.append(f"{rel}: workflow-level env hands {KEY} to every job") + out += default_shell(rel, "workflow", doc) for name, job in (doc.get("jobs") or {}).items(): - if job.get("secrets") == "inherit": + if (root / ALLOWLIST).is_file() and job.get("secrets") == "inherit": out.append(f"{rel}: job {name}: `secrets: inherit` hands {KEY} to the callee") - if not HAS_KEY.search(yaml.safe_dump(job)): + if not keyed or not HAS_KEY.search(yaml.safe_dump(job)): continue out += default_shell(rel, f"job {name}", job) - if job.get("uses"): + uses = job.get("uses") + # A listed local callee is scanned itself, so the key does not leave the audited set. + if uses and not (LOCAL_WF.match(uses) and uses[2:] in allow): out.append(f"{rel}: job {name}: reusable workflow call runs beside {KEY}") for field in ("container", "services"): if job.get(field): @@ -133,6 +130,21 @@ def case(files: dict[str, str]) -> list[str]: call = "\n call:\n uses: o/r/.github/workflows/x.yml@v1\n secrets:\n k: ${{ secrets.MERGE_BOT_KEY }}\n" got = case({wf: clean + call, ALLOWLIST: wf}) assert len(got) == 1 and "reusable workflow" in got[0], f"reusable_call_fails: {got}" + local = ".github/workflows/bump.yml" + callee = "jobs:\n b:\n runs-on: ubuntu-latest\n steps:\n - run: echo ${{ secrets.MERGE_BOT_KEY }}\n" + lcall = "\n call:\n uses: ./.github/workflows/bump.yml\n secrets:\n k: ${{ secrets.MERGE_BOT_KEY }}\n" + got = case({wf: clean + lcall, local: callee, ALLOWLIST: f"{wf}\n{local}\n"}) + assert got == [], f"listed_local_reusable_passes: {got}" + got = case({wf: clean + lcall, local: callee, ALLOWLIST: wf}) + assert any("reusable workflow" in p for p in got), f"unlisted_local_reusable_fails: {got}" + got = case( + { + wf: clean + lcall, + local: callee + " - run: npm ci\n", + ALLOWLIST: f"{wf}\n{local}\n", + } + ) + assert len(got) == 1 and local in got[0] and "`npm`" in got[0], f"listed_callee_scanned: {got}" inherit = "\n call:\n uses: o/r/.github/workflows/x.yml@v1\n secrets: inherit\n" got = case({wf: clean + inherit, ALLOWLIST: wf}) assert any("inherit" in p for p in got), f"secrets_inherit_fails: {got}" From a0f302e687b024fe7fbd0da8dcb9d2b44cffe42e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 22:08:01 +0200 Subject: [PATCH 10/13] fix(bot-token): checkout permission, client-id, docs for the merge key and the PR bot bot-token-test jobs get contents: read for checkout. The mint steps pass client-id (app-id is deprecated in create-github-app-token v3); actionlint's bundled v3 metadata predates it, so notify-umbrella.yml ignores those two messages. gemini-review callers pass secrets explicitly, since merge-key-check rejects inherit. The docs name the real secrets and the persist-credentials trap. Tested: merge_key_check.py --self-test and ., actionlint, devkit-quality.sh (quality rc=0) Co-Authored-By: Claude --- .github/actionlint.yaml | 7 +++++++ .github/actions/bot-token/action.yml | 2 +- .github/workflows/bot-token-test.yml | 4 ++++ .github/workflows/notify-umbrella.yml | 2 +- README.md | 4 +++- docs/3-development.md | 7 ++++--- docs/4-reference_tools.md | 2 +- docs/4-reference_workflows.md | 8 ++++++-- 8 files changed, 27 insertions(+), 9 deletions(-) diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml index b0ee329..36be39f 100644 --- a/.github/actionlint.yaml +++ b/.github/actionlint.yaml @@ -2,3 +2,10 @@ self-hosted-runner: labels: - marola-sea + +# actionlint 1.7.12 bundles create-github-app-token v3 metadata from before `client-id` existed. +paths: + .github/workflows/notify-umbrella.yml: + ignore: + - 'missing input "app-id" which is required by action "actions/create-github-app-token@v3"' + - 'input "client-id" is not defined in action "actions/create-github-app-token@v3"' diff --git a/.github/actions/bot-token/action.yml b/.github/actions/bot-token/action.yml index bd2d115..5f655fe 100644 --- a/.github/actions/bot-token/action.yml +++ b/.github/actions/bot-token/action.yml @@ -56,7 +56,7 @@ runs: - id: mint uses: actions/create-github-app-token@v3 with: - app-id: ${{ inputs.app-id }} + client-id: ${{ inputs.app-id }} private-key: ${{ inputs.private-key }} owner: marola-dev repositories: ${{ steps.guard.outputs.repositories }} diff --git a/.github/workflows/bot-token-test.yml b/.github/workflows/bot-token-test.yml index 0ca4b8a..026148d 100644 --- a/.github/workflows/bot-token-test.yml +++ b/.github/workflows/bot-token-test.yml @@ -14,6 +14,8 @@ jobs: if: github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork runs-on: ubuntu-latest timeout-minutes: 5 + permissions: + contents: read steps: - uses: actions/checkout@v7 - id: empty @@ -56,6 +58,8 @@ jobs: if: github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork runs-on: ubuntu-latest timeout-minutes: 5 + permissions: + contents: read steps: - uses: actions/checkout@v7 - id: bot diff --git a/.github/workflows/notify-umbrella.yml b/.github/workflows/notify-umbrella.yml index e20b52f..05da54b 100644 --- a/.github/workflows/notify-umbrella.yml +++ b/.github/workflows/notify-umbrella.yml @@ -92,7 +92,7 @@ jobs: if: env.HAS_PR_BOT_KEY == 'true' uses: actions/create-github-app-token@v3 with: - app-id: ${{ vars.PR_BOT_APP_ID }} + client-id: ${{ vars.PR_BOT_APP_ID }} private-key: ${{ secrets.pr_bot_key }} owner: ${{ steps.um.outputs.owner }} repositories: ${{ steps.um.outputs.name }} diff --git a/README.md b/README.md index 4c23c8c..355e462 100644 --- a/README.md +++ b/README.md @@ -233,7 +233,9 @@ jobs: gemini: if: github.event.requested_team.slug == 'gemini' uses: marola-dev/marola-devkit/.github/workflows/gemini-review.yml@v0.9.0 - secrets: inherit + secrets: # not `inherit`: merge-key-check rejects it in a repo that holds the merge key + GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} + PR_BOT_KEY: ${{ secrets.PR_BOT_KEY }} with: devkit-ref: v0.9.0 check-command: "" # a fast gate that needs no Nix, e.g. node scripts/site_check.js diff --git a/docs/3-development.md b/docs/3-development.md index a38ab43..ffde7a0 100644 --- a/docs/3-development.md +++ b/docs/3-development.md @@ -119,9 +119,10 @@ Never install graphify any other way, pass `--backend`, or run its installers or ## Secrets and cost -The devkit deploys nothing. Its one secret is `MAROLA_BUMP_PAT`, read by `bump-consumers.yml` -alone; its reusable workflows take what the caller passes (the `notify-umbrella` token, -`MAROLA_CROSS_REPO_PAT` in the callers) or the default `GITHUB_TOKEN`. +The devkit deploys nothing. `bot-token-test` reads the org's `PR_BOT_KEY` and `PR_BOT_APP_ID`; +`MAROLA_BUMP_PAT` (`bump-consumers.yml`) and `MAROLA_CROSS_REPO_PAT` (the callers) remain until +MIP-0086 row 11 retires them. Its reusable workflows take what the caller passes or the default +`GITHUB_TOKEN`. `setup-runners` and `gha-runner` touch a self-hosted runner only when a human runs them ([runners](4-reference_runners.md)). diff --git a/docs/4-reference_tools.md b/docs/4-reference_tools.md index 736728b..030142e 100644 --- a/docs/4-reference_tools.md +++ b/docs/4-reference_tools.md @@ -54,7 +54,7 @@ extension rather than a devkit tool. | `backfill-pr-labels` | `backfill-pr-labels.sh` | `just pr-labels-backfill` | The same for every merged or closed PR with no label; `--limit`, `--nlp` (compare with `pr-label-nlp`), `--nlp-apply-unscoped` (fill only an `area/unscoped` gap) | | `pr-label-nlp` | `pr_label_nlp.py` | (none) | A local TF-IDF classifier for `area/*`, kept to compare against the deterministic one, never applied on its own. `--pr N` or `--title`/`--body` | | `ruleset-sync` | `ruleset-sync.sh` | `just rulesets-check`, `just rulesets-apply` | `check [owner/repo…]` diffs a repo's live branch ruleset against `.github/rulesets/main-rule.json`, plus that repo's bypass actors in `.github/rulesets/bypass-extras.json` (each with a reason); `apply ` creates or updates it; `--all-org ORG` covers an org | -| `merge-key-check` | `merge_key_check.py` | `just quality` | Fails when `MERGE_BOT_KEY` appears in a workflow job outside the allowlist in `.github/merge-key-allowlist`, or in a job that fetches or builds third-party code (MIP-0086 §5.6). Once the allowlist exists, `secrets: inherit` is flagged in every workflow of the repo | +| `merge-key-check` | `merge_key_check.py` | `just quality` | Fails when `MERGE_BOT_KEY` appears in a workflow outside the allowlist `.github/merge-key-allowlist` (workflow files, one repo-relative path per line, `#` comments), or in a job that runs third-party actions or fetch/build commands, calls a reusable workflow (except a listed local one), sets `container`/`services` or a non-bash shell, or sits beside workflow-level env carrying the key (MIP-0086 §5.6). Once the allowlist exists, `secrets: inherit` is flagged repo-wide | | `agents-check` | `agents-check.sh` | `just agents-check` | Compare AGENTS.md's invariants block byte for byte with the pinned devkit's `agents/invariants.md` (`--block` or `MAROLA_INVARIANTS_BLOCK` to use another) | | `docs-lint` | `docs_lint.py` | (none) | MIP-0074 §7's stale-content check over `README.md` and `docs/**/*.md`: undefined or unmarked foreign recipes, another repo's paths, split-era wording, `docs/index.md`, links leaving the repo | | `wiring` | `wiring.py` | `just wiring` | MIP-0076 §5.1's four tables (artifact, dispatch, pin bump, deploy), parsed from the umbrella's and every submodule's workflows, pin files, scripts, justfiles, Dockerfiles, compose files and `build.sbt`, with each devkit workflow call's ref (each call is read at that ref: from a local devkit clone's tags, else fetched from GitHub, a tag cached under `~/.cache/marola-wiring`). `wiring [FILE]` prints the block or rewrites it between FILE's `wiring:start`/`wiring:end` markers; `--check [FILE]` fails on a stale block, a dispatch sent or listened for on one side only, or an artifact nobody reads that the umbrella's `wiring.allow` (`: ` per line) does not list; run it in an umbrella checkout with submodules | diff --git a/docs/4-reference_workflows.md b/docs/4-reference_workflows.md index 9b217f9..a8d5899 100644 --- a/docs/4-reference_workflows.md +++ b/docs/4-reference_workflows.md @@ -357,7 +357,9 @@ jobs: gemini: if: github.event.requested_team.slug == 'gemini' uses: marola-dev/marola-devkit/.github/workflows/gemini-review.yml@v0.9.0 - secrets: inherit + secrets: # not `inherit`: merge-key-check rejects it in a repo that holds the merge key + GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} + PR_BOT_KEY: ${{ secrets.PR_BOT_KEY }} with: devkit-ref: v0.9.0 check-command: "" # e.g. node scripts/site_check.js @@ -423,7 +425,9 @@ newline-separated list of repo names; an empty list fails the step, because the otherwise cover every repo the App is installed on, and so does a call with no `permission-*` input set (`permission-contents`, `-pull-requests`, `-workflows`, `-checks`, `-statuses`, `-issues`). Outputs: `token`, `app-slug`, `installation-id`, `user-name` and `user-email`. The git identity is -job-scoped, through `GIT_CONFIG_GLOBAL` rather than `git config --global`. `release.py --consumer` +job-scoped, through `GIT_CONFIG_GLOBAL` rather than `git config --global`. A job that pushes to +the repo it checked out needs `actions/checkout` with `persist-credentials: false`; otherwise +checkout's `GITHUB_TOKEN` extraheader is used for the push and no CI starts. `release.py --consumer` moves its `@v…` pin with the other devkit pins. ## devkit-ci (not reusable) From a0a70f4a0883c5fb8eb2ea4d9835a08efb60d20a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 22:12:15 +0200 Subject: [PATCH 11/13] fix(merge-key-check): a listed local callee gets the key only by its own name; back to app-id A caller could map `k: ${{ secrets.MERGE_BOT_KEY }}` to a listed local callee that only reads secrets.k, so the callee never named the key and was never scanned. The mapping entry must now be called MERGE_BOT_KEY. The mint steps pass app-id again and the actionlint ignore block is gone; the README no longer claims the GEMINI_APP_* names work next to a snippet that does not pass them. Closes #79 Tested: merge_key_check.py --self-test (red without the fix, green with) and ., mkc-review probes a-d, actionlint, devkit-quality.sh (quality rc=0) Co-Authored-By: Claude --- .github/actionlint.yaml | 7 ------- .github/actions/bot-token/action.yml | 2 +- .github/workflows/notify-umbrella.yml | 2 +- README.md | 2 +- docs/4-reference_tools.md | 2 +- scripts/merge_key_check.py | 14 +++++++++++++- 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml index 36be39f..b0ee329 100644 --- a/.github/actionlint.yaml +++ b/.github/actionlint.yaml @@ -2,10 +2,3 @@ self-hosted-runner: labels: - marola-sea - -# actionlint 1.7.12 bundles create-github-app-token v3 metadata from before `client-id` existed. -paths: - .github/workflows/notify-umbrella.yml: - ignore: - - 'missing input "app-id" which is required by action "actions/create-github-app-token@v3"' - - 'input "client-id" is not defined in action "actions/create-github-app-token@v3"' diff --git a/.github/actions/bot-token/action.yml b/.github/actions/bot-token/action.yml index 5f655fe..bd2d115 100644 --- a/.github/actions/bot-token/action.yml +++ b/.github/actions/bot-token/action.yml @@ -56,7 +56,7 @@ runs: - id: mint uses: actions/create-github-app-token@v3 with: - client-id: ${{ inputs.app-id }} + app-id: ${{ inputs.app-id }} private-key: ${{ inputs.private-key }} owner: marola-dev repositories: ${{ steps.guard.outputs.repositories }} diff --git a/.github/workflows/notify-umbrella.yml b/.github/workflows/notify-umbrella.yml index 05da54b..e20b52f 100644 --- a/.github/workflows/notify-umbrella.yml +++ b/.github/workflows/notify-umbrella.yml @@ -92,7 +92,7 @@ jobs: if: env.HAS_PR_BOT_KEY == 'true' uses: actions/create-github-app-token@v3 with: - client-id: ${{ vars.PR_BOT_APP_ID }} + app-id: ${{ vars.PR_BOT_APP_ID }} private-key: ${{ secrets.pr_bot_key }} owner: ${{ steps.um.outputs.owner }} repositories: ${{ steps.um.outputs.name }} diff --git a/README.md b/README.md index 355e462..1e67c9a 100644 --- a/README.md +++ b/README.md @@ -242,7 +242,7 @@ jobs: ``` It needs the org's `marola-pr-bot` App installed on the repo, the `gemini` team with Read on -it, and the org's `GEMINI_API_KEY`, `PR_BOT_KEY` and `PR_BOT_APP_ID` (the `GEMINI_APP_*` names still work). One Gemini API +it, and the org's `GEMINI_API_KEY`, `PR_BOT_KEY` and `PR_BOT_APP_ID`. One Gemini API call per review keeps it on the free tier, about 20 reviews a day per model; `vars.GEMINI_MODEL` picks another model. This repo calls it from `gemini.yml`, with `bash tests/self-tests.sh` as the check. Details: [docs/4-reference_workflows.md](docs/4-reference_workflows.md#gemini-review). diff --git a/docs/4-reference_tools.md b/docs/4-reference_tools.md index 030142e..e3f6ce1 100644 --- a/docs/4-reference_tools.md +++ b/docs/4-reference_tools.md @@ -54,7 +54,7 @@ extension rather than a devkit tool. | `backfill-pr-labels` | `backfill-pr-labels.sh` | `just pr-labels-backfill` | The same for every merged or closed PR with no label; `--limit`, `--nlp` (compare with `pr-label-nlp`), `--nlp-apply-unscoped` (fill only an `area/unscoped` gap) | | `pr-label-nlp` | `pr_label_nlp.py` | (none) | A local TF-IDF classifier for `area/*`, kept to compare against the deterministic one, never applied on its own. `--pr N` or `--title`/`--body` | | `ruleset-sync` | `ruleset-sync.sh` | `just rulesets-check`, `just rulesets-apply` | `check [owner/repo…]` diffs a repo's live branch ruleset against `.github/rulesets/main-rule.json`, plus that repo's bypass actors in `.github/rulesets/bypass-extras.json` (each with a reason); `apply ` creates or updates it; `--all-org ORG` covers an org | -| `merge-key-check` | `merge_key_check.py` | `just quality` | Fails when `MERGE_BOT_KEY` appears in a workflow outside the allowlist `.github/merge-key-allowlist` (workflow files, one repo-relative path per line, `#` comments), or in a job that runs third-party actions or fetch/build commands, calls a reusable workflow (except a listed local one), sets `container`/`services` or a non-bash shell, or sits beside workflow-level env carrying the key (MIP-0086 §5.6). Once the allowlist exists, `secrets: inherit` is flagged repo-wide | +| `merge-key-check` | `merge_key_check.py` | `just quality` | Fails when `MERGE_BOT_KEY` appears in a workflow outside the allowlist `.github/merge-key-allowlist` (workflow files, one repo-relative path per line, `#` comments), or in a job that runs third-party actions or fetch/build commands, calls a reusable workflow (except a listed local one), sets `container`/`services` or a shell other than bash or sh, or sits beside workflow-level env carrying the key (MIP-0086 §5.6). Once the allowlist exists, `secrets: inherit` is flagged repo-wide | | `agents-check` | `agents-check.sh` | `just agents-check` | Compare AGENTS.md's invariants block byte for byte with the pinned devkit's `agents/invariants.md` (`--block` or `MAROLA_INVARIANTS_BLOCK` to use another) | | `docs-lint` | `docs_lint.py` | (none) | MIP-0074 §7's stale-content check over `README.md` and `docs/**/*.md`: undefined or unmarked foreign recipes, another repo's paths, split-era wording, `docs/index.md`, links leaving the repo | | `wiring` | `wiring.py` | `just wiring` | MIP-0076 §5.1's four tables (artifact, dispatch, pin bump, deploy), parsed from the umbrella's and every submodule's workflows, pin files, scripts, justfiles, Dockerfiles, compose files and `build.sbt`, with each devkit workflow call's ref (each call is read at that ref: from a local devkit clone's tags, else fetched from GitHub, a tag cached under `~/.cache/marola-wiring`). `wiring [FILE]` prints the block or rewrites it between FILE's `wiring:start`/`wiring:end` markers; `--check [FILE]` fails on a stale block, a dispatch sent or listened for on one side only, or an artifact nobody reads that the umbrella's `wiring.allow` (`: ` per line) does not list; run it in an umbrella checkout with submodules | diff --git a/scripts/merge_key_check.py b/scripts/merge_key_check.py index 7012909..16037e8 100755 --- a/scripts/merge_key_check.py +++ b/scripts/merge_key_check.py @@ -70,6 +70,14 @@ def problems(root: Path) -> list[str]: # A listed local callee is scanned itself, so the key does not leave the audited set. if uses and not (LOCAL_WF.match(uses) and uses[2:] in allow): out.append(f"{rel}: job {name}: reusable workflow call runs beside {KEY}") + elif uses: + # Under another name the callee never names the key, so it would go unscanned. + passed = job.get("secrets") + for k, v in (passed if isinstance(passed, dict) else {}).items(): + if HAS_KEY.search(str(v)) and k.lower() != KEY.lower(): + out.append( + f"{rel}: job {name}: passes {KEY} to {uses} under another name ({k})" + ) for field in ("container", "services"): if job.get(field): out.append(f"{rel}: job {name}: `{field}` runs images beside {KEY}") @@ -132,7 +140,7 @@ def case(files: dict[str, str]) -> list[str]: assert len(got) == 1 and "reusable workflow" in got[0], f"reusable_call_fails: {got}" local = ".github/workflows/bump.yml" callee = "jobs:\n b:\n runs-on: ubuntu-latest\n steps:\n - run: echo ${{ secrets.MERGE_BOT_KEY }}\n" - lcall = "\n call:\n uses: ./.github/workflows/bump.yml\n secrets:\n k: ${{ secrets.MERGE_BOT_KEY }}\n" + lcall = "\n call:\n uses: ./.github/workflows/bump.yml\n secrets:\n MERGE_BOT_KEY: ${{ secrets.MERGE_BOT_KEY }}\n" got = case({wf: clean + lcall, local: callee, ALLOWLIST: f"{wf}\n{local}\n"}) assert got == [], f"listed_local_reusable_passes: {got}" got = case({wf: clean + lcall, local: callee, ALLOWLIST: wf}) @@ -145,6 +153,10 @@ def case(files: dict[str, str]) -> list[str]: } ) assert len(got) == 1 and local in got[0] and "`npm`" in got[0], f"listed_callee_scanned: {got}" + renamed = lcall.replace(" MERGE_BOT_KEY:", " k:") + hidden = "jobs:\n b:\n runs-on: ubuntu-latest\n steps:\n - run: npm ci && echo ${{ secrets.k }}\n" + got = case({wf: clean + renamed, local: hidden, ALLOWLIST: f"{wf}\n{local}\n"}) + assert any("under another name (k)" in p for p in got), f"renamed_key_fails: {got}" inherit = "\n call:\n uses: o/r/.github/workflows/x.yml@v1\n secrets: inherit\n" got = case({wf: clean + inherit, ALLOWLIST: wf}) assert any("inherit" in p for p in got), f"secrets_inherit_fails: {got}" From a134c067da9d6e461591f603c227e519202827c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Mon, 12 Oct 2026 22:45:21 +0200 Subject: [PATCH 12/13] =?UTF-8?q?test(bot-token):=20print=20what=20each=20?= =?UTF-8?q?probe=20saw;=20prove=20scoping=20with=20a=20dry-run=20push=20(M?= =?UTF-8?q?IP-0086=20=C2=A77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first live run (38305859769) failed at the marola-site API probe with a code that was never printed. probe now prints the code and GitHub's message, the cross-repo API probe only fails on a 2xx, and a git push --dry-run with the bot token to devkit (must succeed) and marola-site (must be refused) is the scoping test §7 asks for. Tested: actionlint; devkit quality gate; probe() exercised unauthenticated Co-Authored-By: Claude --- .github/workflows/bot-token-test.yml | 33 +++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/.github/workflows/bot-token-test.yml b/.github/workflows/bot-token-test.yml index 026148d..1a8fc87 100644 --- a/.github/workflows/bot-token-test.yml +++ b/.github/workflows/bot-token-test.yml @@ -62,6 +62,8 @@ jobs: contents: read steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - id: bot uses: ./.github/actions/bot-token with: @@ -78,13 +80,32 @@ jobs: set -euo pipefail repos="$(gh api installation/repositories --jq '[.repositories[].full_name] | join(",")')" [ "$repos" = marola-dev/marola-devkit ] || { echo "::error::the token reaches $repos"; exit 1; } + # Prints " "; callers compare the code, errors show the whole line. + probe() { + local body code + body="$(mktemp)" + code="$(curl -s -o "$body" -w '%{http_code}' -X POST -H "Authorization: Bearer $GH_TOKEN" \ + -H 'Accept: application/vnd.github+json' "https://api.github.com/repos/$1" -d '{}')" + echo "$code $(jq -r '.message // empty' "$body" 2>/dev/null)" + } # An invalid body: 403 means no permission, 422 would mean the permission was granted. - probe() { curl -s -o /dev/null -w '%{http_code}' -X POST -H "Authorization: Bearer $GH_TOKEN" \ - -H 'Accept: application/vnd.github+json' "https://api.github.com/repos/$1" -d '{}'; } - [ "$(probe marola-dev/marola-devkit/git/refs)" = 422 ] || { echo "::error::contents write was not granted"; exit 1; } - [ "$(probe marola-dev/marola-devkit/pulls)" = 403 ] || { echo "::error::pull_requests was granted"; exit 1; } - # A repo outside the token reads as 404 to an installation token, 403 where it is visible. - case "$(probe marola-dev/marola-site/git/refs)" in 403|404) ;; *) echo "::error::the token can write marola-site"; exit 1;; esac + refs="$(probe marola-dev/marola-devkit/git/refs)"; echo "devkit git/refs: $refs" + [ "${refs%% *}" = 422 ] || { echo "::error::contents write was not granted: $refs"; exit 1; } + pulls="$(probe marola-dev/marola-devkit/pulls)"; echo "devkit pulls: $pulls" + [ "${pulls%% *}" = 403 ] || { echo "::error::pull_requests was granted: $pulls"; exit 1; } + # Informational: run 38305859769 got neither 403 nor 404 here (code not recorded), so an API + # probe on another public repo proves nothing; the dry-run push below does. + site="$(probe marola-dev/marola-site/git/refs)"; echo "site git/refs: $site" + case "${site%% *}" in 2??) echo "::error::the token wrote to marola-site: $site"; exit 1;; esac + ref="refs/heads/bot-token-probe-${{ github.run_id }}" + git push --dry-run "https://x-access-token:${GH_TOKEN}@github.com/marola-dev/marola-devkit.git" "HEAD:$ref" >push-devkit.log 2>&1 \ + || { echo "::error::dry-run push to marola-devkit failed"; tail -n 5 push-devkit.log; exit 1; } + echo "dry-run push to marola-devkit: ok" + if git push --dry-run "https://x-access-token:${GH_TOKEN}@github.com/marola-dev/marola-site.git" "HEAD:$ref" >push-site.log 2>&1; then + echo "::error::dry-run push to marola-site succeeded"; tail -n 5 push-site.log; exit 1 + fi + grep -Eq '403|denied' push-site.log || { echo "::error::push to marola-site failed, but not with 403/denied"; tail -n 5 push-site.log; exit 1; } + echo "dry-run push to marola-site: refused ($(tail -n 1 push-site.log))" [[ "$(git config --global user.email)" =~ ^[0-9]+\+${SLUG}\[bot\]@users\.noreply\.github\.com$ ]] || { echo "::error::git email is $(git config --global user.email)"; exit 1; } [ "$(git config --global user.email)" = "$EMAIL" ] [ "$(git config --global user.name)" = "${SLUG}[bot]" ] From acb6c4cc926ee3334449b672a4031d4b4566be98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Guilhermo=20de=20Barros=20Val=C3=A9rio?= Date: Tue, 13 Oct 2026 22:45:39 +0200 Subject: [PATCH 13/13] test(bot-token): drop the git/refs probes the live run showed prove nothing Run 38306845732 printed `422 Invalid request.` for a `{}` POST to git/refs on both marola-devkit and marola-site, a repo the token cannot reach: that endpoint validates the body before checking permission, so 422 is not evidence of write access. The dry-run pushes (devkit ok, marola-site 403) already prove contents scoping; the pulls probe stays, since it returned 403 before validation. Tested: actionlint; devkit quality gate (outside Nix); run 38306845732's printed codes Co-Authored-By: Claude --- .github/workflows/bot-token-test.yml | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/.github/workflows/bot-token-test.yml b/.github/workflows/bot-token-test.yml index 1a8fc87..c6784e4 100644 --- a/.github/workflows/bot-token-test.yml +++ b/.github/workflows/bot-token-test.yml @@ -88,15 +88,10 @@ jobs: -H 'Accept: application/vnd.github+json' "https://api.github.com/repos/$1" -d '{}')" echo "$code $(jq -r '.message // empty' "$body" 2>/dev/null)" } - # An invalid body: 403 means no permission, 422 would mean the permission was granted. - refs="$(probe marola-dev/marola-devkit/git/refs)"; echo "devkit git/refs: $refs" - [ "${refs%% *}" = 422 ] || { echo "::error::contents write was not granted: $refs"; exit 1; } + # pulls checks permission before the body: 403 means pull_requests was not granted. git/refs + # does not (run 38306845732 got 422 for `{}` on marola-site too), so pushes prove contents. pulls="$(probe marola-dev/marola-devkit/pulls)"; echo "devkit pulls: $pulls" [ "${pulls%% *}" = 403 ] || { echo "::error::pull_requests was granted: $pulls"; exit 1; } - # Informational: run 38305859769 got neither 403 nor 404 here (code not recorded), so an API - # probe on another public repo proves nothing; the dry-run push below does. - site="$(probe marola-dev/marola-site/git/refs)"; echo "site git/refs: $site" - case "${site%% *}" in 2??) echo "::error::the token wrote to marola-site: $site"; exit 1;; esac ref="refs/heads/bot-token-probe-${{ github.run_id }}" git push --dry-run "https://x-access-token:${GH_TOKEN}@github.com/marola-dev/marola-devkit.git" "HEAD:$ref" >push-devkit.log 2>&1 \ || { echo "::error::dry-run push to marola-devkit failed"; tail -n 5 push-devkit.log; exit 1; }