From e15424a50a8e80873481c3d34624eb954ff08d33 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 26 Jul 2026 14:37:43 -0700 Subject: [PATCH 1/8] chore(ci): define the build-and-deploy graph once (ENG-1053) The dev/staging/prod trio was the same pipeline three times, and the drift it hid was real: prod built its image without waiting for lint or tests, staging and prod disagreed on deploy's needs list, and inference's prod migrate keyed its concurrency group on a matrix it does not have. One repo-local build-deploy.yml now holds the graph; the three callers hold the trigger, the concurrency group, the checks the build waits on, the permission ceiling, and the env knobs. migrate/deploy/integration-tests collapse to one matrixed path gated on a has-envs boolean, because an empty matrix concludes the whole run as failed with no failed job in it. Also fixes what the pipelines were saying. Two pushes to staging had concluded startup_failure with zero jobs -- a caller granted no pull-requests scope while a callee declared one, and that cap is enforced when the file loads -- so staging served the previous image for ten hours and nothing alerted, because the notify job is a job inside the run. Prevention is a shared lint gate, detection is a scheduled sweep; the same fix closed an unnoticed prod window. Promotion PRs stop rebuilding images nothing deploys, and a release is one run tree instead of two. --- .github/actionlint.yaml | 9 + .github/workflows/ci.yml | 86 ++++ .github/workflows/notify-startup-failure.yml | 262 ++++++++++++ .github/workflows/pr-env-comment.yml | 175 ++++++++ .github/workflows/release-pr.yml | 6 +- .github/workflows/workflow-lint.yml | 165 ++++++++ README.md | 163 +++++++ k8s-secret/action.yml | 94 +++++ .../workflow_permissions.cpython-313.pyc | Bin 0 -> 16179 bytes scripts/workflow_permissions.py | 368 ++++++++++++++++ ...w_permissions.cpython-313-pytest-8.4.1.pyc | Bin 0 -> 61526 bytes tests/test_workflow_permissions.py | 396 ++++++++++++++++++ 12 files changed, 1722 insertions(+), 2 deletions(-) create mode 100644 .github/actionlint.yaml create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/notify-startup-failure.yml create mode 100644 .github/workflows/pr-env-comment.yml create mode 100644 .github/workflows/workflow-lint.yml create mode 100644 k8s-secret/action.yml create mode 100644 scripts/__pycache__/workflow_permissions.cpython-313.pyc create mode 100644 scripts/workflow_permissions.py create mode 100644 tests/__pycache__/test_workflow_permissions.cpython-313-pytest-8.4.1.pyc create mode 100644 tests/test_workflow_permissions.py diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml new file mode 100644 index 0000000..3a842c6 --- /dev/null +++ b/.github/actionlint.yaml @@ -0,0 +1,9 @@ +# actionlint needs to be told about self-hosted runner labels, or every +# `runs-on:` referring to one is an error and the real findings drown in noise. +# +# mdb-dev newdev EKS pod, IRSA into the build/ECR account +# mdb-prod newprod EKS pod +self-hosted-runner: + labels: + - mdb-dev + - mdb-prod diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..93ed726 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,86 @@ +# CI for this repo. +# +# It defines the pipelines of eight other repos and had no checks of its own, so a +# malformed reusable reached every consumer's next run before anyone noticed. Every +# call site floats on `@main` (decision 11.BH), which makes that the widest blast +# radius we have. +# +# It runs the same three layers `workflow-lint.yml` gives consumers, but against +# the PR's own checkout rather than through the reusable, because the reusable +# pulls the shared script from `main` and this is the repo where the script itself +# is under review. +# +# This is the repo's first `pull_request` trigger, so a fork PR can run it. That +# is acceptable and deliberate: no secrets, no self-hosted runner, read-only token. + +name: CI + +on: + pull_request: + push: + branches: + - main + +defaults: + run: + shell: bash + +permissions: + contents: read + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Run actionlint + env: + VERSION: "1.7.12" + run: | + set -euo pipefail + curl -fsSL "https://github.com/rhysd/actionlint/releases/download/v${VERSION}/actionlint_${VERSION}_linux_amd64.tar.gz" \ + | tar -xz actionlint + ./actionlint -color + + - name: Set up uv + uses: astral-sh/setup-uv@v5 + + - name: Unit-test the shared CI scripts + run: uv run --with pyyaml --with pytest --python 3.12 -m pytest tests/ -q + + - name: Check that this repo's own reusables compose + run: | + uv run --with pyyaml --python 3.12 scripts/workflow_permissions.py \ + --workflow-dir .github/workflows + + - name: Audit workflows with zizmor (advisory) + env: + GH_TOKEN: ${{ github.token }} + run: | + set -uo pipefail + uvx zizmor@1.28.0 .github/workflows | tee zizmor.out || true + { + echo "### zizmor (advisory)" + echo + echo '```' + tail -40 zizmor.out + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + notify: + # One job covers both outcomes: a `uses:` job cannot branch on status, so + # the aggregate result picks the failed or recovered message, and a + # cancelled run stays silent. Push-only: a PR author already sees their own + # red check, and this repo's PRs can come from forks. + needs: [lint] + if: ${{ github.event_name == 'push' && !cancelled() && !contains(needs.*.result, 'cancelled') }} + permissions: + contents: read + actions: read # the prior-run lookup behind the recovery message + uses: ./.github/workflows/notify-main-failure.yml + with: + env-name: "github-actions CI" + status: ${{ contains(needs.*.result, 'failure') && 'failed' || 'recovered' }} + runs-on: ubuntu-latest + secrets: inherit diff --git a/.github/workflows/notify-startup-failure.yml b/.github/workflows/notify-startup-failure.yml new file mode 100644 index 0000000..f818a78 --- /dev/null +++ b/.github/workflows/notify-startup-failure.yml @@ -0,0 +1,262 @@ +# Reusable workflow: alert the engineering channel when a pipeline on a +# protected branch concluded WITHOUT EVER STARTING. +# +# Why this exists as a separate mechanism from notify-main-failure.yml, rather +# than another mode of it. +# +# `notify-main-failure` is a job INSIDE the pipeline it reports on. That covers +# every failure where the run exists: a failed step, a mid-graph failure that +# only skips its dependents, a timeout. It cannot cover the case where GitHub +# rejects the run at load time — `conclusion: startup_failure`, zero jobs — for +# the plain reason that there is no job to put the alert in. The run appears in +# the Actions tab and nowhere else. +# +# That case is not exotic and it is not cosmetic. Any of these produce it: +# - a caller job granting narrower `permissions:` than a called workflow's +# jobs declare (a called workflow may never exceed its caller's grant, and +# the cap is enforced when the FILE LOADS, not when a job runs) +# - a `uses: ./.github/workflows/x.yml` path that does not exist on the ref +# - malformed YAML, an unknown `on:` filter, a bad matrix expression +# +# Its consequence is the worst-shaped one available: a merge to a deploy branch +# reports nothing, so the branch is believed deployed and is not. Two pushes to +# auth `staging` sat undeployed for ten hours this way, and the first thing the +# channel heard was the RECOVERY message from the run that fixed it. +# +# So this workflow watches from outside, on a schedule. +# +# What it alerts on, per branch, per workflow file: the newest conclusive run is +# a `startup_failure` and it started inside the lookback window. +# +# That deliberately repeats. With the caller sweeping every 30 minutes and a +# 90-minute window, one failing push produces about three messages and then +# silence, and a second failing push starts the count again. The alternative was +# to alert only on the TRANSITION into a broken state, which gives exactly one +# message per break — and that was rejected after replaying it against the auth +# incident, where it would have alerted once at 01:20 and then said nothing about +# the 08:00 push into the already-broken branch, because the run before that one +# had also failed to start. Exactly-once is the wrong target for "the branch is +# not deployed": no state store is available to a stateless sweep, so the choice +# is between a message that can be missed and a few that cannot, and a missed +# alert is the entire failure being fixed here. A bounded reminder is the answer, +# and the window is what bounds it. +# +# Cancelled and skipped runs are not evidence either way and are ignored, the +# same reading notify-main-failure's recovery check uses. +# +# It posts through the Slack Web API directly rather than the composite action +# the other alerts use, because the number of findings is not known until the +# sweep runs, and because an alert path that only fires when something is +# already broken is the last place to want a third-party action's templating +# between us and the message. +# +# Requires (reach this workflow via `secrets: inherit` in the caller): +# - secrets.SLACK_ENG_CHANNEL_ID (the engineering channel; org secret) +# - secrets.GH_ACTIONS_SLACK_BOT_TOKEN (the Slack bot token; org secret) +# The Slack bot must be a member of the channel SLACK_ENG_CHANNEL_ID resolves to. +# +# The caller's job needs `actions: read` for the run + job history. A called +# workflow can never hold more than its caller grants, so without it the sweep +# has nothing to read; it says so in the summary and exits green rather than +# reddening on a permissions gap. +# +# Scheduling note for callers: GitHub only runs `schedule` triggers from a +# repository's DEFAULT branch. A watchdog caller merged to `staging` and no +# further is inert. `workflow_dispatch` is there so it can be proven before it +# reaches `main`. + +name: Notify pipeline startup failure + +on: + workflow_call: + inputs: + branches: + description: "Space-separated branches to sweep (deploy branches worth alerting on)" + type: string + default: "main staging" + lookback-minutes: + description: "Only alert on a startup failure that began this recently" + type: number + default: 90 + runs-on: + description: "Runner label for the sweep job" + type: string + default: mdb-dev + workflow_dispatch: + inputs: + branches: + description: "Space-separated branches to sweep" + type: string + default: "main staging" + lookback-minutes: + description: "Lookback window in minutes (widen it to replay a past incident)" + type: number + default: 90 + dry-run: + description: "Report findings to the job summary without posting to Slack" + type: boolean + default: false + runs-on: + description: "Runner label" + type: string + default: ubuntu-latest + +defaults: + run: + shell: bash + +permissions: + contents: read + actions: read # run + job history for the sweep + +jobs: + sweep: + runs-on: ${{ inputs.runs-on }} + steps: + - name: Find pipelines that never started + id: find + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + BRANCHES: ${{ inputs.branches }} + LOOKBACK: ${{ inputs.lookback-minutes }} + run: | + set -uo pipefail + + CUTOFF=$(date -u -d "-${LOOKBACK} minutes" +%s 2>/dev/null) || \ + CUTOFF=$(date -u -v-"${LOOKBACK}"M +%s) # BSD date, for a macOS runner + + echo '[]' > findings.json + + for BRANCH in ${BRANCHES}; do + # Fail loud into the summary, quiet into the exit code: a watchdog + # that reddens the repo when the API is briefly unhappy gets muted, + # and a muted watchdog is worse than none. + if ! gh api "repos/${REPO}/actions/runs?branch=${BRANCH}&status=completed&per_page=100" \ + > "runs-${BRANCH//\//-}.json" 2>error.txt; then + { + echo "### Could not read run history for \`${BRANCH}\`" + echo + echo "Usually a missing \`actions: read\` grant on the calling job." + echo '```' + cat error.txt + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + continue + fi + + # Newest conclusive run per workflow FILE (a `run-name:` override + # changes the display name, the path is stable). The previous run's + # conclusion rides along for the message rather than as a filter, so + # a failing push into an already-broken branch is still reported. + jq --argjson cutoff "${CUTOFF}" --arg branch "${BRANCH}" ' + [ .workflow_runs[] + | select(.conclusion == "success" or .conclusion == "failure" + or .conclusion == "timed_out" or .conclusion == "startup_failure") + | { id, path, name, conclusion, created_at, head_sha, html_url, + actor: (.triggering_actor.login // "unknown"), + title: (.display_title // "") } ] + | group_by(.path) + | map(sort_by(.created_at) | reverse) + | map(select(.[0].conclusion == "startup_failure")) + | map(.[0] + { branch: $branch, previous: (.[1].conclusion // "none") }) + | map(select((.created_at | fromdateiso8601) >= $cutoff)) + ' "runs-${BRANCH//\//-}.json" > "candidates-${BRANCH//\//-}.json" + + jq -s 'add' findings.json "candidates-${BRANCH//\//-}.json" > merged.json + mv merged.json findings.json + done + + # Keep only the runs where NO job ran, which is the whole point: those + # are the ones the pipeline's own notify job could not have reported. + # An unreadable job list still alerts — by this point a startup failure + # definitely happened, and silence is the failure mode being fixed. + jq -c '.[]' findings.json | while read -r ROW; do + ID=$(jq -r '.id' <<< "$ROW") + if COUNT=$(gh api "repos/${REPO}/actions/runs/${ID}/jobs" --jq '.total_count' 2>/dev/null); then + [ "${COUNT}" = "0" ] || continue + else + COUNT="unknown" + fi + jq -c --arg jobs "${COUNT}" '. + {jobs: $jobs}' <<< "$ROW" + done | jq -s '.' > reportable.json + + COUNT=$(jq 'length' reportable.json) + echo "count=${COUNT}" >> "$GITHUB_OUTPUT" + + { + echo "### Startup-failure sweep" + echo + echo "Branches: \`${BRANCHES}\` · window: last ${LOOKBACK} min" + echo + if [ "${COUNT}" = "0" ]; then + echo "No pipeline concluded without starting. Nothing to report." + else + echo "| Workflow | Branch | Commit | Jobs | Previous run | Run |" + echo "| --- | --- | --- | --- | --- | --- |" + jq -r '.[] | "| `\(.path)` | `\(.branch)` | `\(.head_sha[0:8])` | \(.jobs) | \(.previous) | [\(.id)](\(.html_url)) |"' reportable.json + fi + } >> "$GITHUB_STEP_SUMMARY" + + - name: Post the alert + if: steps.find.outputs.count != '0' && !inputs.dry-run + env: + SLACK_BOT_TOKEN: ${{ secrets.GH_ACTIONS_SLACK_BOT_TOKEN }} + CHANNEL: ${{ secrets.SLACK_ENG_CHANNEL_ID }} + REPO: ${{ github.repository }} + REPO_NAME: ${{ github.event.repository.name }} + SERVER: ${{ github.server_url }} + SWEEP_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -uo pipefail + + # One message per sweep with a section per pipeline. Findings are + # almost always singular; a broken shared reusable is when they are not, + # and that is exactly when one grouped message beats several. + jq -n \ + --slurpfile findings reportable.json \ + --arg repo "$REPO" --arg repo_name "$REPO_NAME" \ + --arg server "$SERVER" --arg channel "$CHANNEL" --arg sweep "$SWEEP_URL" ' + { + channel: $channel, + text: "\($repo_name) — a pipeline concluded without ever starting", + attachments: [{ + color: "#FF4444", + blocks: ( + [{ + type: "section", + text: { + type: "mrkdwn", + text: ":rotating_light: *<\($server)/\($repo)|\($repo_name)>* — \($findings[0] | length) pipeline(s) *never started*, so nothing in the run could report it. The branch is not deployed." + } + }] + + ( $findings[0] | map({ + type: "section", + fields: [ + { type: "mrkdwn", text: "*Workflow*\n<\($server)/\($repo)/actions/runs/\(.id)|\(.path)>" }, + { type: "mrkdwn", text: "*Branch*\n<\($server)/\($repo)/tree/\(.branch)|\(.branch)>" }, + { type: "mrkdwn", text: "*Commit*\n<\($server)/\($repo)/commit/\(.head_sha)|\(.head_sha[0:8])>" }, + { type: "mrkdwn", text: "*Pushed by*\n\(.actor)" } + ] + }) ) + + [{ + type: "context", + elements: [{ + type: "mrkdwn", + text: "`startup_failure` with no jobs: GitHub rejected the run before scheduling anything — check the caller/callee `permissions:` relationship and the workflow YAML on that ref. <\($sweep)|Sweep run>" + }] + }] + ) + }] + }' > payload.json + + RESPONSE=$(curl -sS -X POST https://slack.com/api/chat.postMessage \ + -H "Authorization: Bearer ${SLACK_BOT_TOKEN}" \ + -H 'Content-type: application/json; charset=utf-8' \ + --data @payload.json) + + if [ "$(jq -r '.ok' <<< "$RESPONSE")" != "true" ]; then + echo "Slack rejected the alert: $(jq -r '.error // "unknown"' <<< "$RESPONSE")" + exit 1 + fi + echo "Alert posted." >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/pr-env-comment.yml b/.github/workflows/pr-env-comment.yml new file mode 100644 index 0000000..e0817b9 --- /dev/null +++ b/.github/workflows/pr-env-comment.yml @@ -0,0 +1,175 @@ +# Reusable workflow: post (and keep updating) one comment telling reviewers where +# a pull request's environment is and how to sign in. +# +# Every repo that deploys a PR environment wants this and only one had it, so the +# reviewer's first move on the other four was to reconstruct a hostname from a +# namespace convention or ask someone. The mechanism belongs here. +# +# THE FACTS DO NOT. This repo is public and has to stay public (a public caller +# may only consume public reusable workflows, and cowork, cowork-server, and anton +# are public callers), so anything hard-coded here is world-readable. The account +# you sign in as, the Secret and Secrets Manager bundle holding its password, the +# namespace convention, and the internal host pattern are not credentials, but +# together they are a free recon package: which username to spray, and exactly +# what to reach for once inside a cluster. +# +# So every one of those arrives as an INPUT from the private caller. This file +# renders a table and maintains one marker-keyed comment; it names nothing of ours. +# A reusable that cannot be described without naming our infrastructure has not +# earned promotion and stays in the repo that owns it. +# +# Layout is deliberate: the two things a reviewer actually wants — who to sign in +# as, and the one command that prints the password — come FIRST, before any links +# or caveats. The previous version led with four paragraphs of rationale and put +# the account halfway down. +# +# It never posts the password itself. A PR comment is permanent, org-wide, and +# un-redactable, and the credential is one `kubectl` away for anyone who can reach +# the cluster; printing it here would put a live admin password in the timeline of +# every PR forever. +# +# k8s-secret-ok: the `kubectl get secret` this renders is a command for a human to +# run, assembled from caller inputs. This workflow never reads the value. +# +# The calling job must grant `pull-requests: write`, and no job here declares it: a +# called workflow can never exceed its caller's grant and that cap is enforced when +# the file LOADS, so a scope named here would reject the run of any caller that +# does not grant it — including, in a shared pipeline, the push callers that never +# comment on anything. + +name: PR environment comment + +on: + workflow_call: + inputs: + env-name: + description: "The environment's name, e.g. pr-auth-123. Shown as the heading." + type: string + required: true + links: + description: >- + JSON array of {"label","url"} objects, rendered as a table in order. + e.g. '[{"label":"Console","url":"https://..."}]' + type: string + default: "[]" + login-email: + description: "Account to sign in as. Omitted from the comment when empty." + type: string + default: "" + login-note: + description: "One short line about that account (realm, roles it carries)." + type: string + default: "" + password-namespace: + description: "Namespace for the rendered password command. Defaults to env-name." + type: string + default: "" + password-secret: + description: "Secret holding the password. No command is rendered when empty." + type: string + default: "" + password-key: + description: "Key within that Secret." + type: string + default: "" + password-source-note: + description: "Where the same value comes from upstream, e.g. a Secrets Manager bundle name." + type: string + default: "" + notes: + description: "Markdown appended inside a collapsed
, for anything non-urgent." + type: string + default: "" + marker: + description: "HTML-comment key identifying this comment, so it rolls instead of piling up." + type: string + default: "pr-env-access" + runs-on: + description: "Runner label" + type: string + default: ubuntu-latest + +defaults: + run: + shell: bash + +jobs: + comment: + runs-on: ${{ inputs.runs-on }} + steps: + - name: Post the environment access details + env: + GH_TOKEN: ${{ github.token }} + PR: ${{ github.event.pull_request.number }} + ENV_NAME: ${{ inputs.env-name }} + LINKS: ${{ inputs.links }} + LOGIN_EMAIL: ${{ inputs.login-email }} + LOGIN_NOTE: ${{ inputs.login-note }} + PW_NS: ${{ inputs.password-namespace || inputs.env-name }} + PW_SECRET: ${{ inputs.password-secret }} + PW_KEY: ${{ inputs.password-key }} + PW_SOURCE: ${{ inputs.password-source-note }} + NOTES: ${{ inputs.notes }} + MARKER_KEY: ${{ inputs.marker }} + run: | + set -euo pipefail + + if [ -z "${PR:-}" ]; then + echo "::error::No pull request in context; this workflow only makes sense on a pull_request run." + exit 1 + fi + + MARKER="" + { + echo "${MARKER}" + echo "## PR environment is up · \`${ENV_NAME}\`" + echo + + # The two things a reviewer came for, first. + if [ -n "$LOGIN_EMAIL" ]; then + echo -n "**Sign in as** \`${LOGIN_EMAIL}\`" + [ -n "$LOGIN_NOTE" ] && echo -n " — ${LOGIN_NOTE}" + echo + echo + fi + if [ -n "$PW_SECRET" ] && [ -n "$PW_KEY" ]; then + echo "**Password** (not in this comment on purpose, it is a live credential and a PR comment is permanent):" + echo + echo '```bash' + echo "kubectl -n ${PW_NS} get secret ${PW_SECRET} -o jsonpath='{.data.${PW_KEY}}' | base64 -d; echo" + echo '```' + echo + fi + + if [ "$(jq 'length' <<< "${LINKS:-[]}")" != "0" ]; then + echo "| | |" + echo "| :--- | :--- |" + jq -r '.[] | "| \(.label) | \(.url) |"' <<< "$LINKS" + echo + fi + + if [ -n "$NOTES" ] || [ -n "$PW_SOURCE" ]; then + echo "
More" + echo + [ -n "$PW_SOURCE" ] && { echo "${PW_SOURCE}"; echo; } + [ -n "$NOTES" ] && { echo "${NOTES}"; echo; } + echo "
" + echo + fi + + echo "Rebuilt on every push to this PR. Torn down when the PR closes or loses its \`deploy\` label." + } > body.md + + # One rolling comment per PR rather than a new one per push. + # --paginate, because the default page holds 30 comments and a busy PR + # would push the marker off it, making this post a duplicate on every + # later push. The jq emits nothing for a page with no match rather than + # an empty string: --paginate runs the filter per page and concatenates, + # so a `// ""` fallback would prepend blank lines to the id. + ID=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR}/comments" --paginate \ + --jq ".[] | select(.body | startswith(\"${MARKER}\")) | .id" | head -1) + if [ -n "$ID" ]; then + gh api -X PATCH "repos/${GITHUB_REPOSITORY}/issues/comments/${ID}" -F body=@body.md >/dev/null + else + gh api -X POST "repos/${GITHUB_REPOSITORY}/issues/${PR}/comments" -F body=@body.md >/dev/null + fi diff --git a/.github/workflows/release-pr.yml b/.github/workflows/release-pr.yml index c77a6fe..ddecfa4 100644 --- a/.github/workflows/release-pr.yml +++ b/.github/workflows/release-pr.yml @@ -114,8 +114,10 @@ jobs: EOF ) - # Dedent (the heredoc inside the run block gets indented) - BODY=$(echo "$BODY" | sed 's/^ //') + # Dedent (the heredoc inside the run block gets indented). Parameter + # expansion rather than sed: one process, and no pattern to escape. + BODY="${BODY//$'\n' /$'\n'}" + BODY="${BODY# }" PR_URL=$(gh pr create \ --repo "${REPO}" \ diff --git a/.github/workflows/workflow-lint.yml b/.github/workflows/workflow-lint.yml new file mode 100644 index 0000000..74d7556 --- /dev/null +++ b/.github/workflows/workflow-lint.yml @@ -0,0 +1,165 @@ +# Reusable workflow: lint the CALLING repo's GitHub Actions workflows. +# +# One place for the checks every service repo wants on its own pipelines, so they +# are not eight copies of a lint step drifting apart. Three layers, deliberately +# in this order, because they catch different things and only two of them are ours +# to maintain: +# +# actionlint The established syntax + expression + shellcheck linter. Blocking. +# Everything it finds is a mistake. +# permissions The one check neither tool does: a called workflow may never +# declare a permission its caller lacks. Blocking. See +# scripts/workflow_permissions.py for why this is not redundant — +# short version, it was verified against a tree that had already +# broken production and neither tool said a word. +# zizmor The established Actions SECURITY auditor (template injection, +# credential persistence, unpinned actions, excessive permissions). +# Advisory by default, because pointing it at existing pipelines +# surfaces a backlog, and a lint job that is red on arrival gets +# ignored rather than fixed. Flip `zizmor-blocking` per repo once +# its backlog is triaged — same rollout the pip-audit gate used. +# +# Nothing here is hand-rolled that a tool already does. The permission check is +# ~200 lines precisely because it is the only gap. +# +# Runner: defaults to ubuntu-latest. This job needs no cluster, no cloud, and no +# secrets, so it should not occupy a self-hosted runner, and it is safe on a +# public repo's fork PRs. + +name: Workflow lint + +on: + workflow_call: + inputs: + actionlint-version: + description: "actionlint release to run" + type: string + default: "1.7.12" + zizmor-version: + description: "zizmor release to run" + type: string + default: "1.28.0" + zizmor-blocking: + description: "Fail the job on zizmor findings (off until a repo's backlog is triaged)" + type: boolean + default: false + zizmor-persona: + description: "zizmor strictness (regular | pedantic | auditor)" + type: string + default: regular + default-permissions: + description: >- + What an undeclared workflow token carries in the calling repo, from + Settings -> Actions -> Workflow permissions (read | write). The + permission check compares against it, so a repo set to write needs to + say so or the check is stricter than reality. + type: string + default: read + runs-on: + description: "Runner label" + type: string + default: ubuntu-latest + +defaults: + run: + shell: bash + +permissions: + contents: read + +jobs: + lint: + runs-on: ${{ inputs.runs-on }} + steps: + - name: Check out the calling repo + uses: actions/checkout@v4 + + - name: Check out the shared CI scripts + uses: actions/checkout@v4 + with: + repository: mindsdb/github-actions + path: .ci-shared + + # Downloaded rather than run through a third-party action: one fewer + # mutable-tag dependency in the job that exists to police them. + - name: Run actionlint + env: + VERSION: ${{ inputs.actionlint-version }} + run: | + set -euo pipefail + curl -fsSL "https://github.com/rhysd/actionlint/releases/download/v${VERSION}/actionlint_${VERSION}_linux_amd64.tar.gz" \ + | tar -xz actionlint + ./actionlint -color + + - name: Set up uv + uses: astral-sh/setup-uv@v5 + + - name: Check that every reusable call composes + env: + DEFAULTS: ${{ inputs.default-permissions }} + run: | + set -euo pipefail + uv run --with pyyaml --python 3.12 \ + .ci-shared/scripts/workflow_permissions.py \ + --workflow-dir .github/workflows \ + --default-permissions "$DEFAULTS" + + # Not a tool, just a rule, because the failure it prevents is permanent. + # A cluster Secret read by hand is unmasked until someone remembers + # `::add-mask::`, and `::add-mask::` only scrubs output that comes AFTER it, + # so the correct-looking fetch-then-mask still has a window. The k8s-secret + # composite action closes it by construction (fetch, fail-if-empty, mask, + # export, in that order). A credential printed into an Actions log is + # world-readable to the org and unrevocable from there — you can rotate the + # secret, you cannot unpublish the log. + # + # A workflow that genuinely needs the raw form, or that only quotes the + # command for a human to run, exempts itself with a `k8s-secret-ok: ` + # comment anywhere in the file. Per FILE rather than per line, because the + # one real exemption we have is a command inside a heredoc that gets posted + # to a PR, and a per-line marker there would be published along with it. + - name: Check for hand-rolled cluster secret reads + run: | + set -uo pipefail + FOUND=0 + while IFS= read -r file; do + grep -q 'k8s-secret-ok:' "$file" && continue + if grep -nE 'kubectl.*get[[:space:]]+secret' "$file"; then + echo " ^ in $file" + FOUND=1 + fi + done < <(find .github/workflows -type f \( -name '*.yml' -o -name '*.yaml' \)) + if [ "$FOUND" = "1" ]; then + echo "::error::Read cluster secrets through mindsdb/github-actions/k8s-secret@main, which fails on an absent key and masks BEFORE exporting. ::add-mask:: only scrubs output that comes after it, so a hand-rolled fetch-then-mask still has a window, and a credential in an Actions log cannot be unpublished." + echo "If a file only quotes the command for a human, add a 'k8s-secret-ok: ' comment to it." + exit 1 + fi + echo "No hand-rolled cluster secret reads." + + - name: Audit workflows with zizmor + env: + VERSION: ${{ inputs.zizmor-version }} + PERSONA: ${{ inputs.zizmor-persona }} + BLOCKING: ${{ inputs.zizmor-blocking }} + # zizmor's online audits resolve action references against the API; the + # default token is enough and keeps it off the anonymous rate limit. + GH_TOKEN: ${{ github.token }} + run: | + set -uo pipefail + if [ "$BLOCKING" = "true" ]; then + uvx "zizmor@${VERSION}" --persona="$PERSONA" .github/workflows + else + # Advisory: report and move on. The findings land in the log and the + # step summary either way, so turning this into a gate later is a + # one-line input change rather than a discovery exercise. + uvx "zizmor@${VERSION}" --persona="$PERSONA" .github/workflows | tee zizmor.out || true + { + echo "### zizmor (advisory)" + echo + echo '```' + tail -40 zizmor.out + echo '```' + echo + echo "Set \`zizmor-blocking: true\` on the caller once these are triaged." + } >> "$GITHUB_STEP_SUMMARY" + fi diff --git a/README.md b/README.md index 2604682..f375a7a 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,169 @@ must be a member of that channel. message in either style, skipping the prior-run lookup so `recovered` always posts. +### The one failure it cannot see: a run that never started + +`notify-main-failure` is a job *inside* the pipeline, so it covers every failure +where the run exists. It cannot cover `conclusion: startup_failure` — GitHub +rejecting the run at load time, with **zero jobs** — because there is no job to +put the alert in. The run appears in the Actions tab and nowhere else, and the +branch is not deployed. + +That is not a corner case. It is produced by a caller job granting narrower +`permissions:` than a called workflow's jobs declare (the cap is checked when the +file *loads*, not when a job runs), by a `uses: ./.github/workflows/x.yml` path +that does not exist on the ref, and by malformed YAML. It has already cost auth +`staging` ten hours of running the previous image across two merges, with the +recovery message being the first thing the channel heard about it. + +`notify-startup-failure.yml` watches from outside, on a schedule. It sweeps the +deploy branches and alerts when the newest conclusive run of a workflow is a +`startup_failure` inside the lookback window, restricted to runs where no job ran +— precisely the set the in-run notify job could not have reported: + +```yaml +name: Pipeline watchdog +on: + schedule: + - cron: "*/30 * * * *" + workflow_dispatch: + +jobs: + startup-failures: + permissions: + contents: read + actions: read # run + job history for the sweep + uses: mindsdb/github-actions/.github/workflows/notify-startup-failure.yml@main + with: + branches: "main staging" + lookback-minutes: 90 + secrets: inherit +``` + +Alerts repeat, by design: one failing push produces about three messages at that +cadence and window, and then silence. Alerting only on the *transition* into a +broken state gives exactly one message per break, which was rejected after +replaying it against the auth incident — it would have said nothing about the +second failing push, since the run before that one had also failed to start. A +stateless sweep cannot be exactly-once, so the choice is a message that can be +missed or a few that cannot, and the window is what bounds the few. + +Two things to know when adding it. GitHub only runs `schedule` triggers from a +repository's **default branch**, so a watchdog merged to `staging` and no further +is inert — `workflow_dispatch` is there to prove it before it reaches `main`. +And widening `lookback-minutes` on a manual dispatch replays a past incident, +which is how to check it would have caught one. + +## Workflow lint + +`workflow-lint.yml` lints the CALLING repo's workflows. Three layers, only one of +which is ours: + +| Layer | Blocking | What it is | +| --- | --- | --- | +| `actionlint` | yes | The established syntax + expression + shellcheck linter | +| permission check | yes | `scripts/workflow_permissions.py` — the one gap neither tool covers | +| `zizmor` | advisory by default | The established Actions *security* auditor (template injection, credential persistence, unpinned actions) | + +```yaml + workflow-lint: + permissions: + contents: read + uses: mindsdb/github-actions/.github/workflows/workflow-lint.yml@main + with: + default-permissions: read # Settings -> Actions -> Workflow permissions + secrets: inherit +``` + +`zizmor` is advisory because pointing it at existing pipelines surfaces a backlog, +and a lint job that is red on arrival gets ignored rather than fixed. Flip +`zizmor-blocking: true` per repo once its backlog is triaged. + +**The permission check is not redundant with either tool**, which was verified +against a tree that had already broken a deploy branch: `actionlint` reported +nothing, and `zizmor --persona=auditor` reported only its generic +"no `permissions:` block" note, equally true of jobs that work fine. What it checks +is that no called workflow declares a permission its caller lacks. GitHub caps a +called workflow at the calling job's grant and enforces that cap when the workflow +**file is loaded**, so a job in a shared reusable naming a scope one caller does not +grant does not run with less — it rejects that caller's whole run as a +`startup_failure` with **zero jobs**, which means the pipeline's own notify job +cannot report it either. In `mindsdb/auth` that cost two silently undeployed merges +to `staging` and ten hours of serving the previous image. + +The rule it enforces: **a job in a shared reusable declares only what all its +callers grant, and inherits anything only one of them needs.** Inheriting is the +only thing that composes — the PR caller grants `pull-requests: write` and the +comment posts, the push callers grant `contents: read` and the same job runs +without a scope it never needed. And the reason it has to be a gate rather than a +convention: a pull request only exercises the PR caller, which is usually the one +that *does* grant the scope, so the mistake merges green and breaks on the merge +commit. + +It also refuses to read cluster secrets by hand — see `k8s-secret` below. + +Blind spot to know about: it can only read local (`./.github/workflows/...`) +callees, and lists remote ones as unchecked. The cap applies to those too, so a +scope added to a reusable *here* must be granted by every consumer's calling job. + +## Reading a Kubernetes secret + +`k8s-secret` fetches one key from a Secret, fails when it is absent, masks it, and +only then exports it — in that order, which is the part a hand-rolled fetch gets +wrong, because `::add-mask::` only scrubs output that comes *after* it. + +```yaml + - uses: mindsdb/github-actions/k8s-secret@main + with: + namespace: pr-auth-123 + secret: keycloak-secrets + key: PRIVATE_CLIENT_SECRET + env-var: PR_KEYCLOAK_CLIENT_SECRET +``` + +When to use it at all, versus a GitHub Environment secret: read the cluster for +values whose source of truth already lives in a namespace (AWS Secrets Manager → +k8s), because a second copy per GitHub Environment has to be rotated in lockstep +and a reference to a secret that no longer exists resolves to the **empty string** +rather than failing. Keep real per-environment credentials (dev/staging/prod) in +their GitHub Environment, behind that environment's protection rules, rather than +making them fetchable by any job that can reach the namespace. Throwaway creds from +the cluster, real creds from a gated environment. + +It is not a containment boundary: the ability to read Secrets belongs to the +self-hosted runner, which holds cluster credentials because it deploys. What bounds +*that* is who can trigger a run on such a runner (hence: a `pull_request`-triggered +job on a public repo needs a fork guard) and the runner ServiceAccount's RBAC. + +## PR environment comments + +`pr-env-comment.yml` posts and keeps updating one comment saying where a PR's +environment is and how to sign in. The account, Secret name, namespace, and hosts +arrive as **inputs** — this repo is public, and while none of those is a credential, +together they are a free recon package. The mechanism is shared; the facts stay in +the private caller. A reusable that cannot be described without naming our +infrastructure has not earned promotion here. + +```yaml + pr-env-comment: + needs: [deploy-pr-env] + # No `permissions:` here — see the permission rule above. The dev caller grants + # `pull-requests: write`; the push callers must not have to. + uses: mindsdb/github-actions/.github/workflows/pr-env-comment.yml@main + with: + env-name: pr-auth-${{ github.event.pull_request.number }} + login-email: someone@example.com + password-secret: some-secret + password-key: SOME_KEY + links: '[{"label":"Console","url":"https://..."}]' + secrets: inherit +``` + +Layout is fixed and deliberate: who to sign in as and the one command that prints +the password come first, links next, everything else in a collapsed `
`. +It never posts the password itself — a PR comment is permanent, org-wide, and +un-redactable. + ## CalVer releases `calver-release.yml` cuts the `v....` tag and its GitHub diff --git a/k8s-secret/action.yml b/k8s-secret/action.yml new file mode 100644 index 0000000..289f0a7 --- /dev/null +++ b/k8s-secret/action.yml @@ -0,0 +1,94 @@ +# Read one key out of a Kubernetes Secret into the job environment, masked. +# +# WHY THIS EXISTS, given the obvious alternative of a GitHub Environment secret. +# +# Both patterns are in use across these repos. The honest comparison: +# +# A GitHub Environment secret is auto-masked, needs no cluster, and is invisible +# to fork PRs. But it is a SECOND COPY of a value whose source of truth is AWS +# Secrets Manager (see auth's scripts/secrets/create-aws-secrets.py), duplicated +# per environment, rotated by hand in lockstep — and a reference to a secret that +# no longer exists resolves to the EMPTY STRING rather than failing. That is a +# real failure mode, not a theoretical one: a renamed token left a config job +# authenticating with "" and reporting a vendor 401 instead of "the secret is +# gone". +# +# Reading the cluster keeps one source of truth and fails loudly when the key is +# absent, which is why it is the right pattern for values that already live in a +# namespace. What it does NOT get for free is masking: GitHub only scrubs values +# it was told about, so a hand-rolled fetch is one careless `echo` away from +# printing a live credential into a permanent log. And `::add-mask::` only +# affects output AFTER it runs, so even a correct-looking fetch-then-mask has an +# ordering hazard baked in. +# +# This action removes the hazard by construction: fetch, fail if empty, mask, and +# only then export. There is no ordering for a caller to get wrong, and no +# unmasked variant to copy-paste. +# +# WHAT IT IS NOT. It is not a containment boundary. Every job that reaches a +# cluster Secret runs on a self-hosted runner that holds cluster credentials +# because it has to deploy, so the ability to read Secrets is a property of the +# RUNNER, not of this action — anyone who can land a workflow change on a branch +# that runs there could read the same values with or without it. The controls that +# actually bound that are (1) who can trigger a run on a cluster-credentialed +# runner, which is why a `pull_request`-triggered job on a public repo must carry +# a fork guard, and (2) the runner ServiceAccount's RBAC, which should be scoped +# to the namespaces it deploys. This action is about not leaking what a trusted +# job legitimately read. + +name: Read a Kubernetes secret +description: Export one key from a k8s Secret as a masked environment variable. + +inputs: + namespace: + description: "Namespace holding the Secret." + required: true + secret: + description: "Secret name." + required: true + key: + description: "Key within the Secret's data." + required: true + env-var: + description: "Environment variable to export it as, for later steps in the job." + required: true + mask: + description: >- + Register the value with the log scrubber. Leave this alone. Set false only + for a value that is genuinely not a credential (an id, a hostname), and say + why at the call site, because an unmasked value is unmaskable afterwards. + required: false + default: "true" + +runs: + using: "composite" + steps: + - name: Read ${{ inputs.secret }}/${{ inputs.key }} + shell: bash + env: + NAMESPACE: ${{ inputs.namespace }} + SECRET: ${{ inputs.secret }} + KEY: ${{ inputs.key }} + ENV_VAR: ${{ inputs.env-var }} + MASK: ${{ inputs.mask }} + run: | + set -euo pipefail + + # Assigned before export: `export X="$(cmd)"` takes its status from + # `export`, which always succeeds, so a failed lookup would sail past + # `set -e` and surface much later as an unexplained auth error. + VALUE="$(kubectl -n "$NAMESPACE" get secret "$SECRET" -o "jsonpath={.data.${KEY}}" | base64 -d)" + + if [ -z "$VALUE" ]; then + echo "::error::${SECRET}/${KEY} is empty or absent in namespace ${NAMESPACE}" + exit 1 + fi + + # Mask BEFORE the value can reach any log, including the environment-file + # diagnostics below. Ordering is the whole point of this action. + if [ "$MASK" = "true" ]; then + echo "::add-mask::${VALUE}" + fi + + echo "${ENV_VAR}=${VALUE}" >> "$GITHUB_ENV" + echo "Loaded ${SECRET}/${KEY} from ${NAMESPACE} into ${ENV_VAR}." diff --git a/scripts/__pycache__/workflow_permissions.cpython-313.pyc b/scripts/__pycache__/workflow_permissions.cpython-313.pyc new file mode 100644 index 0000000000000000000000000000000000000000..bd2864d54751eb13078900a0e56250bbc21d505c GIT binary patch literal 16179 zcmbt*Yj7Lam1Z~IXaEG?A}NV#J|qzmU$R~nCE3=4vM5uOED(x3lnj7ClY|HYbT=T0 zu$?fI+8Jub8PTy_rq$|Dsr4k4-O1K&)q;j2!U!|vo#c=IQI<9? z``3Qw_5&aVC)wGS*tmUPeeS*IeCKg*KlFG64%hURvCH4x$8o=<2mP|C3I5^V@ErFR zx0UmAGUw<0rsw!mCfOuX+WeeZHnU%gY{9SPIqNB#Y}3l@vK@I=zs+y&Hp>pbUN93a#8O&M!>zd(k&tvdiy6shdju9{j85HeGbpaOXM2)Qi6w?xdCDlAPZgG@Z6{ zoQo5Yo8bJF-6q-XuR_~u*0x6WG;pn)zqX0<*X=d2&N;b)<<%o^gOTU;xA_~IxXTu~ z(%2{F_;-&hPU#vsAr*hkpx8ZD{8H@IU`f zzsPDf_&fbw{uWAK=H0ty!zrmx42zL)JT67Wt4VosES|h7PK9U0gmguc#i$gChh+(MrX_hQrYNywLX4#p zwTUdo!;wj)TM(X&rB0?tkvpwC-rsgbOeH6!gcweVsR@a-jU~p#%gIp`jmzOgs#6Rn zqUczROi1WaNKK$FdK0fspdB?bh&wSb*e?$F#lbU2j`tt$6cfpmS}RN)igG%kbc_BZ z$e_m<-YUpaS_zNFOFCiwC&E(_k*h?K(~_7>;BTyCaEcgBqMZ6?G#Zb>OX--DiqFuy z#H+E?gcz3;rH{y$rOQ$T)1%RkX_YZVDUpg{5~+F^FBC!{B^8!a>FMBDI2KRKQYh3Z zj!I)mn(4$$YJz4brhu>sDVoO8V1|cjMzreUR5B`2g>)kJQd$Bfqv2?`IE-nccTC_V z8A(pk1Sp-Nl4KJ&9h;Wov4jMAl2;Q#iq4{BBH;v$b_~R-3xX#`3y8u( zfI1~SLxYpV>9jH-Mq^QGE<~m2cygv&9Kf0-6#FA-6j7^2dHL@rlM5=Gn>PZRoCP^~2 zi%ei3Qe2#n!g5pyk0#S8TAZtx7izjgmLf@T_sk9uq$ucC9Fvn%tc>Q5E#6q{8VHW1 zF!e)8QMwjWQjDZzG7iQ}VP!xP`1a~VEHXiBK|LoE#1&Flft0XG7>j}HSYt5dc~CbN zlR(jUIF?}47DJ(MBn25mRthsib0~wI^mXXKC=8~QB_$cZBGHQlCc$zK z`@!NAWmT5qVd_wsh)t9HLDGHd+J-`zSY|3Ii(Or)4~oNw!|7-&m6SsvNR0YoHXxGs zaVY`nBOp+^7Qv*&u1M5Og9HV%f<|yJn`>3{m}Df9;P#S=eW*DaPevw7W=?u34YFp$ zl$>TFNNlSv0tr1sD`NRkSPFTXx z8FdwhX?<9&iEw<3ME&Xn6^Y{{bO6!9BC#o8U?^Raf_ley--QxhO^k|QcrAp6^ z?wP4LmV_9mPY^_0YNO(k={g`v(4?XOK$WOhxea)N^#qV8=~3Y1^n^H-#_0a!?5AN6P2*}S4fZjfDd;>Y3OOA}fR|!S+#%QC3qW=} zsSq@QI0-BOf$AjIDHVIQPqxZfoarQ+bDE|iS1>iAkU)UJx-12+Nnt4|i{UMsa6lyM2i-)H3E*Z5J%B@^B^@ZxY;4*fFbY#q1cB6#s_Q_ys^4kE zOud|wrbvF(B~`=@ooSNtcn^U=8Q!@@yz`7KjfFx!G-Rq{6wC-x1cs+fV-N$A1)(Ae zRu3SYPB5vZr7V%#u%e`=kjLa*_>2q~A75~>j#UgB!n5TpntVo9-)Cf;`OF1N0&7^X z(0dD3HkFx*Xe^Q%QBsgi0O_TIZLt4*|DgXVFIp6=(?EQsU}H4k7gh}fy})QFgP51_S})7VSlE!95h7GlW}QwF#qr@ zvfkn@axxEV-sI)v%68%PoC}TWLVfSSRidLksz&J2c)mx8$g$~^VrYn9 zQ4c8H(=!D}Fc?e3Qo&%M+E}q}J?{k~q7%1SZnc5iu=zDtzN!5+Z@zw$o~mnlO~^Nh zuX*wtnqPD0Ya71ne2Ds-v1;_tfIqfszrYH;#Ra%?l)LVtQPUOP#8C}n{nT8#SHQn4 z-xejN-Ig;IU)#>S+mwJWdd^t!Vy|dptbH+8_p%ZuloRKzyEIyuue`PhC04X?ug#?= zl-N*GQMSY$;8E`QgDnXg)?H}h447~izLrkczo1Vy+LRxgXLmz8=c^d{%Er`DsKAFZ zRbq#3fWUFOBE@|fi`ey)m~nOl3Z0P$t){@awU)3ESEp@%!g+_JTt%N?Sro$obOt=vKW0o`Cc-+b>H3aQstA` z;S0;d!KL9~)*s4-Udlc%&sp=8^>4dgcP$)$ci`qgu5#O)^;2)njhC;#JU6wlZ?XOT zp7(mPEl=F_-8=r_sSi%wJM<6oFWdiD=YQ|~W%DY>U*K!|)a0tm>Y3tK&330CcVloqM+u%5F?KDeGN2vuRkN8cm@z#n z67NR`YIBw%OwI@>6<-mDV2-Hx%!*ilEX7ubr3kLS@*)5bmsBgIySp2@U6xX5IZ?1s z4|h!R0eT13hfV?TQn&)kB?N=8RABKV?G6UXchX9{!65UB$;(e9QH&*yCL_V1OrR`N zNT^Ups}9p75`>i^#~@&m5)9tq*o-j7kT+9hx{)SUVz~VU_lf1`s-?!-xN767YFF)) za&UDGt4>M@l;@(9o2%Qh>Y-ExS6Q>_rBo$XQ@>h8scNpeZncI|wVbzkwT@EtoKW#- z15#h?5UlFDuy;rBTI15toqtHL3XtYMK6`^?r-{Q>>10P8L!5 zj=3n%1q+#(J`0oMVzxYhdWr=%2(l{4Y;_CuYsP8l8f z5k8E>*C}2o4^pe;6)veiqHW!UHs;+nC<4on+>5e|L-jZT=q+N0x)f?4kBoR0sWTi1 zpL`N6d_0>6g={u$)G9l#A2q|T@1T}KGKN7{*T22<^_}yZ7seKc-w(bQ%(m~nd*NR6 z!`KJ0dn4JCv8?xU)^S;7e|pmpV1a=F6Vx{3FcD<)uBMn+_EEt>x*ek1F}e|`s3O!) zDY_B6DWr%=g#PT2#SBIlcI6T61y!#A zKVmG6UQQfJR~X9~;M=)1wGBJxvD)Y-fY^QS`W6ngH;#L(1>hvWBN}fqqVaIW5~dNm z-XdVo?l&Vs4~}(lB1NlB?OZR{j+FX@*tI3R5rZFwGaE`K`qqV$UN~}kP~6qMOPmhN zkRS3kNj;AuY=>xqI4My)WEg>_PzaGdbO6;$F$UI7tHna5q0mftD&DPx$E0998IE?e z;$_SYZ}o*j;!I46M-}w-GIiD`Y$peO`>W_xJO>`8@InYJTH(|qUIoJdfxraCjN#p) zofgp3gJ8^4R!x?v@T4TDF_@y2luS;F@z^BA;f>%2Io8S~qIxtw_(cc-F^{%eD43Fo z;US;3U>=uJ1p(m-1RKMN2%Kc{1Q{0>EOA6M;XOO_w-n5B_-etbI-|rjI=&PfP!!|i z@Fir@2!Dw`h4>m)eg)^MxzTyOb3V0j;oXZjFWzp?ZSKhly|c$w>{at^Z>E;*o0jaG z^4{t>LCsN??HiZu8}pSl^TTs5XB|yzDt^+vE9ZUU2a(0Vo96lcH-v@bA61HXB1_&U zvW_S6F3;@1m!F%t%H0a_=FfK4AGKM^i_eoNe~TM2gv8G`l`^-nEmHH?MhQ^Ll%6IA zqi7`Ceo&HAk`^(-Tx(IrFfAb-^ywo1*TAdwMQgw8g^SsjlYZQ zj7j)_6w(ZuJir*NUJ|t9H{F=CNItlZ9 zES!#~)J+>zWrnAj%2Y$6nleQgUyDVP?xgJ<=DAOxu9WB&)+{9P-){2U$%<_Q5|BE= z9-b3>cvXI`Z=B<f?zBrqigA9wd5rg&3Lbrm*t88XWJb2ByH^gNFNGABZUQ?G zQCR!hwER?|C8h+~A_dtZbsu6eP35U>qsk(+uEXs!aDKMpNMdt%RQ>(+}@>%lRvp|Z{p*<&*myl z=3FPUmXl0fteGLgTFesv@ZX|$njI<~=Zh>K;6`Yy?0O83VMPSs1+!hl4IEe048UmK zz-TL*ZNg*Vw1&sPe6+OuNiD7GTNd%2eA`3tM0XpwFRi8h>vaia8)gR9ef>oi0bMi( zXd1J&?~9pDVDNFJUN@i-w-^+aIS}(@p%CuBG%8YYmO% z2yesU48yjEY^@XgE8#eB2Dp$EUuN;3KGi9HmHCO}38)|x9#z1ffRvK}e(X0g*r)9^ zc#1OFqR=WGA+n#k2rfE-9~4z)4?ZKZ3}%nVBC!;9QQ}<`H>G`oDKspJ^Yn=?zZ$|O zk+HKyVvhJeDV%5wVS5>b5XeEtk(q|=KiVXwM@_A18%=#;2tA2pFv|cp;0E@NRo58c zqe54dKnr8nT6POV8HYXy7Uax0jd7@P%`NG~WP)0ikX)Pu1O%GX3F?8Fk)?Imwo{+z zv&j_1RBbtux`J??)}UXOld}9gWmqwBQwo%$m9=5cT*1m>6>EKHX9BYqj>oXu8b!mu z#h>zX+?Z-~c)tB=-djToZtJa@W&7qO`{ul>a{g4#)%=^Dy}6owKe)0eE$sW=={r}J zYW8K_`>6D}oNLQ(diUjO_WwuQVtQf!_lACKTdLWgb?;xPXvl8tySMiPU$*ghuA)Ed z=vVu$UA8wb*_-nRpZW0M2M7QD@WhCmd0VD19p+`0>PBCn$lsn#KxLkz{tz5vY_n5Hw**|?gL~g`Dv2R#Lj2NI^ zu3P6(%=+n*h&3M>F`|JL9Ox<=;oEK6j=`M{sR%|^eGAvg8GT`tV$7zlij$z5<7Hn%FMOeEcdXO0~i44ysL ze`4SQwtNr^fr0DR2@OVLa>1!TDJ(cvun{W8rGkz50#OC(jEq`c`&W=zA1t;>qxe7L zPq~WQEca$ZdGye&y~tOWsp;=UZ8*wXf6fa>z0Nf%pBRm?HJPVxaXtScwkC!i zg(L;|@@yS2?J*RgNQ$w}A%bG53Am$wVw{#M>mal>=n0|(GQKl!nv zHQ&_wZs*O;#Vfg{?&YREcUzYm4=yzxyx(~Ex@XR@;%!`LyOmn@wl8_xKXKQt)NWpg zygPn#d~qt*yeC(?cg~r2)#hAc-d%m;^!3xXb}w9e=kRS`wrS6O_udCCuIVYn7~R#g zrymD>LpuJ`5FjeaW#b8$Q&rg-~|NLaa2yBeHuAQm1c1B zSyi^TlEP7BEDI;UN;mj5+h-Jh zSN{52bCwn0eDv0_WtRx1RRfGaxwyRbXm0D#yu0eg;Pt^xgl zC?I+d30t6iO@m>|h*BBRE7E-O6OH0)CG6+;>WBSqCZM z^Qa&|VUeN+T&MKpZ&RrO1}xBCuu0c&778bTf>*=wNuNu7dxaL83aV~KK_~{H3Qpq~ zlKdCcVJlIgpV@#OT?Z@jU!t7ZXQ~EecNXra?NyvwmgiC7e^aGius$4>*7n&``K{Y$ z2R?1y{%-PSa=E!@$FcNGPOj8IZGL~xa!tolO~)sd4f&e-xA(uk ze_`JnPtP5v!sz#pl@xAx`{~!8{{Ermn%s{J*=%(E4BbV8`bikoML)H!e8w_8sobgb$AG2;S{E@b_AIPo?2lWjZ>IV>PG$2{UcrtUe0Nm4S7@H6C+q4pdxtaSz{x zDi@-=_%84h(AVe@GrUuMU%&wbvkqnI5r7iceC5D`adHcKknv_t{#Us9+^V5M7P1Od z6hLAoiMnkLJY0!2l)H^?5h~(?1zQ5=3vpDHjJXmpqNKY2NN;1Wbb~>a{S1z<>F%h`*&FlTDp=W` z`Wu(7Us~wPdA7}-$lE>N{wB-+bn}+k{uOc0-ShXwLracLK%!0U?+)D@y1%LO`rsS^ z!L1WHZ)?7??rrz$ZrJaT`FvZKc5iN5X=z^!{B2WSd@{ee|7|wY+ez@;PeRukPGI1}R+c)^jaBk<>UwO~|vA$Qtj@xxPai9Kz z-_r{ows4!f9+YJLmYk!*j!Z-M!+j$~U#;H*Tf>^&9gWTk>^{ z3j<4aTj_UjsjdS6<*5KbIR*4m@52E$w_&4U{)`Kpq@vQB43$QVf^u8pttF9lKYxHv z@G!xthjo|VR5bdF_6uZEdox)MA~?HjTWb+#O6(oICBB{VI}TV_YvPx6SHRc`!)~l0 zudp7f*Hy%$P7N$lvtbW`F^iTDoh~calH&X$^?HxzV1ur|{K5f~$c?*3HtHpCtc-{l zdPNXm{bfalj$aHN`U6&AI!2MOmC)o4*h=>qAp5`==FV93QG}mcGYZeh7X4jCjeClwuPmX;;0*LxwcB{Y3|9MU=$JH) zC#z>Uaa281(bc7UU0n#2WvU+g5CD-_OKN6XD%drHDO1o+3`4Rhq8^r4!@M5jYfrz`yF8)x+p$`+3k^uBo6#5@&cy4 zV40BO)0swmb)yrlz_rAy*z07G4uR8mH8QP##s=WVhcg9U$Q&x!5)koa$`pO3(}6=Y z+EylgY6Fa?LxS4JR@f<3-OD~9V-j%!Q8-4H93q~XUB=rx6knef7gaxWFkD&kKIuyg zcG$n0sR>_Y!4a0nS;VnWaYP=+k@iH2#a3nc2o292jz)v1!U|*yvgFHjBgQWXtRWRE za+-2qqFn1Vc2`n3eKvz*>Pjk#{S;11gYi&VX_04bMY#`1G<~<{; zPFBl`S`^(`c5Pa6ZCV)4xmstB{|=^dYI#fV(w5#_<*wO*e{$8XI6T=(-|hD09S4_o z9L)L-Wh)N<;@Cf)`q`4-GHje^(&4>HLzVarV@2D;wdn zJ3RBY`PBT_T=Kr78E9YC^m`k0Bjuak+&!OpF zw#^Mdk<1^RpP8Rrtj<=q|8O_yuaDjP&`VSMr+Br+Id|$!W#PaZ-^|u;%WnHdR=Du7 z<%PV<&Fot`#zzwx4B}uH1@72>lX}qac|1OrTd>fX6l9O0v|vk)(qUFsKU25%RKIrc z5COq4RQ;;t3kV#Ex}@3<%sPQ-scwFuSK=;}1u6F{Z?Q{CU3!e1%yv38GGZKD)Lz<& zeL?+u3wqn7GA--Yw!|45st9t)OO)@@?KQgnh;BqpW~cfZ*3LMhdZ`l!VL-x~b6@DE zvZNp8&9SjEBa5Y&rN#PY<`&CiHrJE%J~F&A4*eqw_4VF$FP24A!i@YK)vaWOGz9&? zCy`t*Lri^}R5`~v&9OAjbK;z)Y(}B{Z|L^#sG6OQ_s3(S>eNqBKG}=~9N;YZ+64zV zoyON6GBY#@BUHzi&Sx^mmmM@<#|Tt^d7KIubTA|JJM>77DKiI|Aj^mGSP=X(upy`V zufje{0fk16!3V(&Plo-ut&{wYcz^qg`4_f<2`^Piix@s|H{mgo4k-*VnxbHZ=9 zhTn2791U%2zy18ZDujDyZF3bl?%)G~Yij%4RB5$6G*>s7DEA>(&6`$xxhC=U`6c(B zPj>X?JiC6dZLx0t8-JC!vu(+<>w(qWWty#kgg@lWs4H-l+h*Ng*jjnv0f*Zcr+6nn z$v@!e{-~jXuXxnz;0O3e^;I|?{HTNHw?1+>__{~+Zoc|avy-oS)X>4Vte)o&@>QSW z+^H46+BsHAIq3ANlTz4>X#*1n925k0ZSqt!vFvP literal 0 HcmV?d00001 diff --git a/scripts/workflow_permissions.py b/scripts/workflow_permissions.py new file mode 100644 index 0000000..8b0faec --- /dev/null +++ b/scripts/workflow_permissions.py @@ -0,0 +1,368 @@ +"""Gate: a called workflow may never declare a permission its caller lacks. + +GitHub caps a called workflow's token at the calling job's grant, and it checks +that cap when the workflow FILE IS LOADED, not when a job runs. So a job in a +reusable workflow that names a scope one of its callers does not grant does not +quietly run with less: it rejects that caller's entire run as a +``startup_failure``, before anything is scheduled. + +That failure mode is uniquely bad. The run has zero jobs, so the pipeline's own +terminal notify job cannot fire, and nothing anywhere says the push did not +deploy. It cost this repo two undeployed merges to ``staging`` and ten hours of +serving the previous image, and the first thing the engineering channel heard +about it was the *recovery* message from the run that fixed it. + +No existing tool catches it, which is the only reason this file exists. Verified +against the failing tree: ``actionlint`` reports nothing (it does not resolve the +caller/callee permission relationship at all), and ``zizmor --persona=auditor`` +reports only its generic ``excessive-permissions`` note about a job with no +``permissions:`` block, which is equally true of jobs that work fine. Both run +alongside this check rather than being replaced by it. + +The other half of why it goes unnoticed: a pull request only ever exercises the +PR caller, and the PR caller is usually the one that DOES grant the scope. So the +change merges green and breaks on the merge commit. + +The rule this enforces, for every ``uses: ./.github/workflows/*.yml`` call: + + every permission declared anywhere in the callee's local subgraph must be + within the calling job's effective grant + +which in practice means a job in a shared reusable declares only what ALL its +callers grant and INHERITS anything only one of them needs. Inheriting is not a +loss of precision, it is the only thing that composes: the PR caller grants +``pull-requests: write`` and the comment posts, the push callers grant +``contents: read`` and the same job runs without the scope it never needed. + +Remote callees (``uses: org/repo/.github/workflows/x.yml@ref``) cannot be read +from here and are reported as unchecked rather than assumed fine. +""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +import yaml + +# Ordered, so "is this grant enough" is a comparison rather than a table of cases. +LEVELS: dict[str, int] = {"none": 0, "read": 1, "write": 2} + +# Every scope GitHub accepts in a `permissions:` block. Named explicitly so a +# typo in a workflow is a finding here rather than a scope that silently never +# applies. +SCOPES: frozenset[str] = frozenset( + { + "actions", + "attestations", + "checks", + "contents", + "deployments", + "discussions", + "id-token", + "issues", + "models", + "packages", + "pages", + "pull-requests", + "repository-projects", + "security-events", + "statuses", + } +) + +# What the workflow token carries when nothing declares anything. This is the +# repo's "Read repository contents and packages permissions" setting; confirm with +# gh api repos/mindsdb/auth/actions/permissions/workflow +DEFAULT_GRANTS: dict[str, dict[str, str]] = { + "read": {"contents": "read", "packages": "read"}, + "write": {scope: "write" for scope in SCOPES}, +} + +LOCAL_PREFIX = "./.github/workflows/" + + +class Violation: + """One caller/callee pair whose permissions cannot compose.""" + + def __init__( + self, + *, + caller: str, + caller_job: str, + callee: str, + callee_job: str | None, + scope: str, + declared: str, + granted: str, + path: tuple[str, ...], + ) -> None: + self.caller = caller + self.caller_job = caller_job + self.callee = callee + self.callee_job = callee_job + self.scope = scope + self.declared = declared + self.granted = granted + self.path = path + + def __str__(self) -> str: + where = f"{self.callee} job `{self.callee_job}`" if self.callee_job else f"{self.callee} (workflow level)" + chain = " -> ".join(self.path) + return ( + f"{self.caller} job `{self.caller_job}` grants {self.scope}: {self.granted}, " + f"but {where} declares {self.scope}: {self.declared}\n" + f" call chain: {chain}\n" + f" every run of {self.caller} would be rejected as a startup_failure, with no job to report it.\n" + f" fix: drop the `permissions:` block in the callee so it inherits the caller's ceiling, " + f"or grant {self.scope}: {self.declared} on `{self.caller_job}` if every caller should hold it." + ) + + +class UncheckedCall: + """A remote callee, recorded so the report never implies more coverage than it has.""" + + def __init__(self, *, caller: str, caller_job: str, uses: str) -> None: + self.caller = caller + self.caller_job = caller_job + self.uses = uses + + def __str__(self) -> str: + return f"{self.caller} job `{self.caller_job}` calls {self.uses} (remote, not readable from here)" + + +def triggers(workflow: dict) -> dict: + """The `on:` block, which YAML 1.1 parses as the boolean key ``True``. + + `on` is a YAML boolean, so ``yaml.safe_load("on: push")`` yields ``{True: + 'push'}``. Reading `"on"` alone silently finds nothing, which here would make + every workflow look like a reusable and skip the whole check. + """ + raw = workflow.get("on", workflow.get(True)) + if isinstance(raw, dict): + return raw + if isinstance(raw, str): + return {raw: None} + if isinstance(raw, list): + return {str(event): None for event in raw} + return {} + + +def is_entry_point(workflow: dict) -> bool: + """True when something other than another workflow can start this one. + + Only an entry point's own `permissions:` decide a grant. A `workflow_call`-only + workflow has no grant of its own — it runs with whatever its caller allowed — + so treating one as a root would compare its callees against a repo default + that never applies, and report violations that cannot happen. + """ + return any(event != "workflow_call" for event in triggers(workflow)) + + +def normalize(block: object) -> dict[str, str] | None: + """Turn a `permissions:` value into {scope: level}, or None when absent. + + Handles the two shorthands (`read-all`, `write-all`) and the explicit + all-none form (`permissions: {}`), which is a real and different thing from + declaring nothing at all: `{}` overrides an inherited grant down to nothing. + """ + if block is None: + return None + if isinstance(block, str): + if block == "read-all": + return {scope: "read" for scope in SCOPES} + if block == "write-all": + return {scope: "write" for scope in SCOPES} + raise ValueError(f"unknown permissions shorthand: {block!r}") + if isinstance(block, dict): + return {str(scope): str(level) for scope, level in block.items()} + raise ValueError(f"unreadable permissions block: {block!r}") + + +def level_of(grants: dict[str, str], scope: str) -> str: + return grants.get(scope, "none") + + +def load_workflows(workflow_dir: Path) -> dict[str, dict]: + """Parse every workflow, keyed by the `./.github/workflows/x.yml` form callers use.""" + workflows: dict[str, dict] = {} + for path in sorted(workflow_dir.glob("*.y*ml")): + with path.open(encoding="utf-8") as handle: + parsed = yaml.safe_load(handle) + if isinstance(parsed, dict): + workflows[f"{LOCAL_PREFIX}{path.name}"] = parsed + return workflows + + +def declared_permissions(workflow: dict) -> list[tuple[str | None, dict[str, str]]]: + """Every permission block in one workflow: (job name or None for workflow level, grants).""" + blocks: list[tuple[str | None, dict[str, str]]] = [] + top = normalize(workflow.get("permissions")) + if top is not None: + blocks.append((None, top)) + for job_name, job in (workflow.get("jobs") or {}).items(): + if not isinstance(job, dict): + continue + job_block = normalize(job.get("permissions")) + if job_block is not None: + blocks.append((str(job_name), job_block)) + return blocks + + +def local_calls(workflow: dict) -> list[tuple[str, str]]: + """(job name, callee key) for each job calling a local reusable workflow.""" + calls: list[tuple[str, str]] = [] + for job_name, job in (workflow.get("jobs") or {}).items(): + if not isinstance(job, dict): + continue + uses = job.get("uses") + if isinstance(uses, str) and uses.startswith(LOCAL_PREFIX): + calls.append((str(job_name), uses)) + return calls + + +def check(workflows: dict[str, dict], default_grants: dict[str, str]) -> tuple[list[Violation], list[UncheckedCall]]: + violations: list[Violation] = [] + unchecked: list[UncheckedCall] = [] + + for caller_key, caller in workflows.items(): + # Roots only. A reusable workflow's callees are checked through the walk + # below, against the grant of whichever entry point reached them. + if not is_entry_point(caller): + continue + caller_top = normalize(caller.get("permissions")) + for job_name, job in (caller.get("jobs") or {}).items(): + if not isinstance(job, dict): + continue + uses = job.get("uses") + if not isinstance(uses, str): + continue + if not uses.startswith(LOCAL_PREFIX): + unchecked.append(UncheckedCall(caller=caller_key, caller_job=str(job_name), uses=uses)) + continue + + # The calling job's effective grant: its own block, else the + # workflow's, else whatever the repo hands an undeclared token. + grant = normalize(job.get("permissions")) + if grant is None: + grant = caller_top if caller_top is not None else dict(default_grants) + + # The cap flows all the way down, so walk the whole local subgraph + # rather than the immediate callee alone. + violations.extend( + _walk( + workflows=workflows, + caller_key=caller_key, + caller_job=str(job_name), + grant=grant, + callee_key=uses, + path=(caller_key, uses), + seen={caller_key}, + ) + ) + + return violations, unchecked + + +def _walk( + *, + workflows: dict[str, dict], + caller_key: str, + caller_job: str, + grant: dict[str, str], + callee_key: str, + path: tuple[str, ...], + seen: set[str], +) -> list[Violation]: + if callee_key in seen: # a cycle would not load on GitHub either; do not hang on it + return [] + callee = workflows.get(callee_key) + if callee is None: + return [] + + found: list[Violation] = [] + for callee_job, declared in declared_permissions(callee): + for scope, level in declared.items(): + if scope not in SCOPES: + found.append( + Violation( + caller=caller_key, + caller_job=caller_job, + callee=callee_key, + callee_job=callee_job, + scope=scope, + declared=str(level), + granted="not a permission scope", + path=path, + ) + ) + continue + if LEVELS.get(str(level), 0) > LEVELS.get(level_of(grant, scope), 0): + found.append( + Violation( + caller=caller_key, + caller_job=caller_job, + callee=callee_key, + callee_job=callee_job, + scope=scope, + declared=str(level), + granted=level_of(grant, scope), + path=path, + ) + ) + + for _, nested in local_calls(callee): + found.extend( + _walk( + workflows=workflows, + caller_key=caller_key, + caller_job=caller_job, + grant=grant, + callee_key=nested, + path=path + (nested,), + seen=seen | {callee_key}, + ) + ) + return found + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--workflow-dir", + type=Path, + # Relative to the working directory, because this runs against whichever + # repo checked itself out beside this script, not against this one. + default=Path(".github/workflows"), + ) + parser.add_argument( + "--default-permissions", + choices=sorted(DEFAULT_GRANTS), + default="read", + help="what an undeclared workflow token carries in this repo (Settings -> Actions)", + ) + args = parser.parse_args(argv) + + workflows = load_workflows(args.workflow_dir) + if not workflows: + print(f"No workflows found under {args.workflow_dir}", file=sys.stderr) + return 1 + + violations, unchecked = check(workflows, DEFAULT_GRANTS[args.default_permissions]) + + for call in unchecked: + print(f"note: {call}") + + if violations: + print(f"\n{len(violations)} permission mismatch(es) would fail a run before it starts:\n", file=sys.stderr) + for violation in violations: + print(f" {violation}\n", file=sys.stderr) + return 1 + + print(f"\nChecked {len(workflows)} workflow(s): every local reusable call composes.") + return 0 + + +if __name__ == "__main__": # pragma: no cover - CLI entry point + raise SystemExit(main()) diff --git a/tests/__pycache__/test_workflow_permissions.cpython-313-pytest-8.4.1.pyc b/tests/__pycache__/test_workflow_permissions.cpython-313-pytest-8.4.1.pyc new file mode 100644 index 0000000000000000000000000000000000000000..6cd3c1c41039761eec1291bab654663b0b6cf9ec GIT binary patch literal 61526 zcmeHwd2}4dd1uds=@}e@gZCi~PY@(=@D@pn;(dw`gC^?7ZygY53O=QQ}%y0mKnN{+X#JexbdN({IE9M`& z`}?Z8tGfq17=R)rd86j=LsfNEb#--BeP?}jx3V%Q;rjD)-Os(dN0R=YBE}`_fxFMT zCFz@zBB_$%Qrt&es+;}w9QJDNuyoj`mWBBpzbfO7=SbjiPz~y7JF5(t~Y}m}y2x@%j){e2wLRWdzmgGjiKfT$n-Yqd?j!us07a} zP|D9NR4NqrMp>y`AU)?*7b!~=w^G&QQbM@nZuH}={UTIxX5q#%rJ8a+=YGzmE}lm2 z8j<@EWvNn&=hmHBu+gi`V(BjR(RtERNtwMsQs#)B3bVAiNSi0pmaw$>NLwJ%ma?>k zNLwV*ma(+ONP9%2EoW(uA}uV^R`j?VmJPm%kJFw^q{HdBme#`EDK(t#jfc;t)U(}v zsq^81xZ0o4v_vWy?un-3;Z+?ST9=v_NNdeTo`{)8YZ@5p=xAsP29Ki>Rv>;MK^1q# zyP|_yJe=wdN5g77+80hFyArW@GL1mEqoY5OjA^mX=ICI$x1(cQu%n|ZmF!ORG)4yo z`i7c@`ujRM>NRAH#nbVwbVo-xnv5Yj&=*Z2@?5G@3&-MJeNis(95ep|%>2Qx4O{Uj| z48T_h5R0K3!}|A0QLvYNMKwOCMLYXYEj2wk6vWtJ z&594C&}sA~>3gBWjgi>cmq@1hSZNBrL$klN!Ik$Oi>7!{#K!W082Yg*ol=ML-ej~tp7(a9Vng{dHJ%<+lU*KqpnhT5kSKn)72uoFDHkoA zZTb?d=V5x(1qdada^yi{sui~d%(;!(U?-C?OmYVfu0$6E!ol$ws5jSp$}e1qxk z#>cL@@^U=cm5O0xGYY1Gzm zIGjJx+^%5)H1`d5MYX20sZ=sO+1euO0W4 zvL<)Z)9zyT>-hb4&vL23-rRv(8>vh*Evo$*|?!D<>nDy7+d}RHlXR`9b zo8_U4N7Ol}#JQ99;tua5#na+yb}3$ThS^y@)>&n&v;3^HWLsyk`>eA9t@r%>vChU= z{9N9H)g|vutAp{Y?t6Y(9vjgOJ}n&|(9}hEV!l$04|?Ccm!j&UxT6w1p)Lc^oH{11 z61HDMw+{fIV-~Hrw&MDnYma7Hc4ez~Pwt*oqsue%ny;^XecAQJ*_s2z`zJ4_`v)Qe zSR(TNSiCzr*q81Sa}v)YdV!6rVFbP@od)Dr8>JRp?UF%&B;A&rR7b(pZoN;VyWnbf zQ6U3aFUK5cZSdy3gi`ZWglxoA)AGKq-gwv9eAV84`**cJc|3BUZCC4Y1(TczAaxx; zK45gbW=wT9?$lM3g8-3$S`)xU>8=BgVn9(t=h^tElTY`dIzZPvt+ zjoBp|ubs>;c|2FXBjeq{hZu^9%b1gQ*CXkh(ius0frP~>rg{|j0!j5M9zdVs1uRp1 zfPSS6P*(hEphs>9vyA^xRNf# zm0CjUsQ5A-r0i+;J}ITz>CNei{Zx`p)4*d_o5S}2iRNziZ3LNpYU4)tCOxm>Jx#6Q zr5H(X#Q5K(tY0a-9sgq(>6WKd@H|$y)H1$|_8M)BF0hrZvXdZt~c*T|{(ntYAu`(&9?ruf^-T3jsVKV4(ItL3S+Mw$4rl_ zAvk|eIASni0xRSyF0FeyV({_CHsZ|!1{pVR>AUUO|78S z+zGA0HT=|yRRcqt8?@kO^+5RXFoo6|NeJjkEe%>z(pVLP&#$6v8ymES;Pc^x#tLn1 z(A16S=c~TFXJDv>zmuNt`lIPc`~tCwQKD9_ z%6YFA@9WN&({p%dtg}WoSRq&i#zdqO zZ*?}3&>~5U3jnmRe;_@SUuVO7P17hsXhmBGoO@CFNvQ5?FTCX`WsD47$e>dd%OlXOS^1!%UPp#6kWX%|EP^;)hPDbq_DC$7+ zNSi2X4dBss;uzg%(iQ)FpLB%ZBi2Eow#d0Yu zC4EWnsZ9*SbQ>6-Q6+*FX`?|^aDgiEw3mS@L0Jn`;yW#9kv2yv!w~ohs>FPXj=X6GG)(G$Mz}ela%`?fmQ-f5jaNRmk6{GPyiYNYCDBb5I9NT6aWMWB)U~UO$kpE zpmDFB0nkW*#gMFtXd4;Uq&J8VX#x5y!X;r80}-J@a0pQ|(FFLL-|m2Nr(1X@HKCUw zUxDV@uRNEL+kxhu%Y_;c`thv4T==7fxyJS%EqvW~?Zj{O|8OBvGmY(;P(wDHl(hy#TTfm&n~_hVt!Hzg)d>A~ z<6mz7(e_-^$scWheZ{q&-+JMP+mV`SI++Qr&W27-$olUF5<{y+ln2J;lhjr{OV*6x z2DO!L70;L&zRz z&CAe0r9rN1(^v-8!x)+Baas^2)Ba1@l0-~MqAOvc{l~e8Rv1K8;T~NO2`LKMEeHgj4d_s zBlH@C?=(DG(%Hm~Y^dl@G|?6DiqT{Q@`IGh_{C^MgVqtOpRDZ4YmDVfU$13XI_efQ(u8GQ{Z&n_?$FT3W=WO-- zUT~M+Z(EDShyB;6@!@>fX0ayS|EFCvm?T_JN=Dn)W&F9CK--6_j74A@X{}@GStP0{ z0P9ysA()pVOjr_$k%;;Xy=o_cZUO@YG=TicgozpRqx4eUo$UBdAsmmTr@Z6x=Zxp%+v``Dg+c6 zFsK9o%atHtg;EY!sZ^*{J(Uer`8xV82T0A|7S~dJgCrw*hn{6q0@0ez7OJ<=9*k$B zRZ=^|%ctoMyI?s2ZJNR4ZPL@RNLllHPYZRB!G>FM62;9flN}iZJu|SZSxW7mB!d1f zgsRjwcC`tn-$)_m8_G^nTa=wzZV?dpPYba?nG~rP{3B2wBxD#)2Bf zNU*fcSzIp|cc>}+C3BmK0%#+3ML|FbTIhh;MvE+U(TFU#iYN*|2MiGIFK@jE;{~z= zAvLcWM`=a^=&4Nk%eW#4OlwID3(;&?mw9f0NHusak?Ld8M(wIw-HGBPd);ua$|8H+ zgnUTcbhUyh#hK!lDLJ`HZkim^Rhe^4E!4!9=s~WCRZ6*@@lrg6iz+u-BbyzF70y4g z*N3ryA@3%nLnbHSsm@1-vkt;9tUa5Gkd-S>M zD>IF|aw~U_$$LcVtG#1#^IP*4UOD)ym9JD@3f^4&XhweYN?SJc=!C5QZpe>b8Wu4g z7`LTm%@}T6fq>8~&BLgJ(<+#F;v(vTEdl>fatR=1dWl8AXD$NJbvZ8@4}K}|v!y_` zEd_y(;ZhK=Okd}P!(0l2VkszBDjxV!P}!=!fDg|(09suzp=mw1S81~^Q(vS^mkBUw z$wdm$d{jSA;AH||B=E}wz69Vf^HuZnS0WjKy;Fo4p_wyoYpEGmv?z&G zv~L1fW*X+1@^rTGbZ+IDG5HxW)kag-&u1DB?TLOZy!2 zKllk(#wMKKHsR!t;e?ZCFyR7X!UdJ`2R`8{^a;n1E=@Q-wdm7)fT^{bO)a9fiIW0( zJJ{1p%E3q!3PmJxJjYOWen-*yHLdbQHEYDrS|%CLS$(E)cW&jLF?p{!$81RMBSLFp zj-By6NXmBRbL_NW-z}y(UZpH`yxnJ4iyd&qDnVNcYS&rCMFu3Gd*xI6EOc++w4ipK zl>nRY;pk#?FPZC@PjN&<=DaZwzMy{>*hfOwpylH* zo~^>LmQvqp!`CRhl3v9=DeSttZwo= zq@D~wQEBVVHZtWbxQw25+Cj3>oeU^7sjKZlHjnojV=x9Wilr}#^|TkWxc1by%~@7GWu`eFu2M&I%Z>3BEx5lXk<9fi|#Zy z$`;}%!A5eEAOuYNA7=9dqaCvO(fLi-4&^F=VP4j zd`uxK?NL`f>bI#{QFnveLK^GbFp3(jgMAVRW>#c(VN1|**1U#l zM-KQ3J!hJ1uo^|?yq^)ukfqBUoyTJh6$R|EdWmX%g#c46T8w~fjPVvLm%P==QMWL~ zr1kbDjYh7}LL$H|ez-xrK{GHTA1mF#m6BG%V^2xKqjV)*Xo~g?kjqx;BX#{;rg496 z<$*Ezph}=g&CvWSOI~?CBX61rEzO3OA~2F-ARF2=fmHUpA#cir zmh$kJyy?>OJead2BRrB41w>$6-gL8i!IgDiubik}k*!`aGJI{xuRlLg|9H0k@l0q( zMpiPr6%Ml@WkS|}H;^9M!J}iAaE|2{*+c+&rVuK)J*BBHY+eB0qLgiQTjYiK)ck@Z z(r*zeu)N0VX31Za$0E)wVWN-6zU}!K`l5%B$_ydM2CCv{3WeJJ_+E`YPRcjlg#SoQ!AIFg_VA*ELw@ zVS^hXyJaSbGWL-0oB1b-;;ZRZK&mwD9{^yxDp%gDn02#a-dh#5mk*DWW$Nm$znD38 zGPC1Uw&K(8`n`eRyFn>XrAvoPp{_WgE&ztQ;uA9ca&nw-jUMBHaXC(?OHa$%Vm$dx zIZnvR%wfb()TjTu` zrfI2@#$syRvIcj+G=qKUBSrjK+fa zlI~W4BPu=x4Z%>6>F2hPUf8Mo0L06G2oj6h#;$*fs$I+5JF@wT5U40YMv_>C(I(q& z_Mx)?sR-N76v4)1WsJH>M>x#W;byy?2yMv$?!<1Na2m9LzOj^!1lS?R|rGH3jw*oKj-%sT^wdq&*#uU zjYuoYs=$@G&I;cGS$LbS;)SY~emCScQt|3B9vHW!Wz86FkcwCTW#qQZ5;H2&Cu3%@ zEG+PTRLepUj?CxCkxfS;UdA{wzl|f4XTIqIQE-hdn)$gH@v^*PHo{^hI%NWRpnkHoZcSCN< zgqHK@nA}9-PP&t`BqKapCJKnan{v|>*zRx8haVOZO!CaF?hk0)_z4G!p>MyY$$>JW z_WLNxD9#2Ka}@FcY%I9GBWl@gWt+Wh-oy3+!lWV2c^21qOurIRVCc)-RCW=xpvv|m2ex*tYJrts{dP<^FBBB!hcFdjaFHeJ{tYPd* zt(9FVNpcIBK*^lFk#`1;5(5;43x10P?9CrFKTGTxdgnj&`LFfRw0xNGQ;- z*d#l-2PK~>Gi(50M>uCnQey6X%BT3)nr9tbCHv>$5)<8u-4^ro=u6`Q%jF>G!Yu6xf%`_6`YqvVu040t}?*Eh+If^m+da)G1N zBvv?;j6Loxa`KiMi^m7pCOSxtrF$^U)1#*Pb0#EAgJ%k#tbDT`=9Z-X2mqAM+9@a< za`-t}lJ+2I94}Q}oVr=g)G!c#VW-ak?LGh{gU#I$>PXNtoMPc_NhV_szO$}GVN$ec zR~ab-y9(zGb==zG?YS#Qy_q?U*Y=M- zb^T;!&A#07{bTZhUjUNlmIRxTi4aUcmarhe%Ags#EqSYh?^B+;1pY~WM|wAa0DOnB zRNM~)-OZ8p26y@iQTU;n+&EM@8%A*h)E%67e0-s%pzKoO7G90kbvEIfb%K zsnHd8L8VryYY*xh2+C*RySCiucjK?_C5{w#SCm<)A1JelG)e|e6YtNic0r<~-fFL; znU*rQS4`OY#d&1SX{YTKb5H$+IkHOjPKD7vW!{Xw?sUc=z9qkmXNa#18lzOpPERV0 zuRX6+DYFkt<`jphDD(BNU8O8g7PeREUpq8|uN^Yp(Dsw{9n4oOJCtd}o3^m|z+h4rwJ?iVQ*TMf z9`vVh=mgBB;GG=)Y?C0l*ya`50D>bMi2B-M>|;Sd-^YR-Eo>hPOkS{$CGP?K*x zyhh-=1W4g&-3+qXx)0|roP65Cn-GXU(bwF^ z50uIMNG1Qa`8H=!0w)OhmzZgtB!kgL4Uv~Y5iY)xzMRwv!dPZWJTtG`Mi4GpK63EY z%B;Nj;rU;HhXzv1Zv6_J*)AQaVE`LHCS?7Wlb4bwDl-A$ad|13^6F_>TZ|{aDKEXe z)tbYIp{P+EY96T(c|-uoH$!tX^4igwZ+#AbzO&)V=W+|zB9vLUHXB+yA?v>z^4d!m zcx=oJirYy0&IS?7nz33{q_%zn~tK}ABL+YJr z@3;FBGCO#%n9EkUXrybX3l66A1BsB9Z<=cLnw+1h2Cm{67OiffFy2_NY#i+2%#tIS zc~9CFtoe&R5`Qr=nopeA-S6Mu)O_&M&C8~n&o}o<1DkF^ z<_Id-P(FNU7mlvN;b0KyFq2-KWJaz4^%K6r`Uzj#!~7^DI(~{B?xi0XhV!y|aY)w? z&bpc;M4OTdYgPXmL#LP}?`D0Pv4h#*lS$Wd(K%$?sXTn^Th2*>Tel?bisGjJ#e_=H z$PBCdGqg}6=QwQpj+2N39^3)_|UfBD=k)lOG!t5*zYTbeu{pbUB6(YtW3KIWh z5%x3{Byf2OZY^LHV$aZ71o2=(TF~aGdN6r9<6Q;fBp%ExEjWJT{h6V0+@rNM*2 zQOw6sK~i4oT})|F$(*K3iwa@DTd7n%Fa-%Y5-<#SLxdWNy0oaCJi1~p+rFFQI#2F& zF}7-~W2@v_3+Y42oPt0qnbYjaTA?6$&^>7?NTh~3N0KVf{Cj~j&u;3jP95pQhS8GQn(U=Y*{PZXkusXel}-?M8lq@{8 z41QL;%~G?p(aEa~`C2h~9l?Er#ka=LhNcF8H=bi4#AnRaf<|JID0alP-DZDn2?Rp#(r#z1TnHk3uI2FagtVQs^}Rr=8au z*mTD(fJiLSl_mozE*sL$CI$u|1mc_Au?-s@g}c(i2wd)dqV7W%S zuf;~6eZA+67c=`J*_9n*a#ZZx6|yvW-vvv3`yt7W4PJ1o(|}@U^_>wYfNyaBO}_(oJ5WT-+UWm;M2IIYg*^%!#EfXGoK{%!&Qyp=g5ylG~9FIgX4o?K~p6Xzw%T!^X+0hgtWBipVBg`!eT@?Zt# z^TLbWY$UMDQVSN^!VAdIN&3tVP3`f(JJ0wb2*3?LAeDnl^ClhHL?MMgAC$zO;wm%5_NYqbx#}V;ZDap@| zxQ8wF{qd+ah}j*C_CUSK)=sud5;ceU3HNjgb(lON;}Cls)rlIybcS-cONDEU@cC3< zcT;dU*q?~S`nX?r>bq9;hk#s`{v!$jHB0KB6Sz!(y3=W=)Nnnuk?icc;ERX}3BzX9 zI0Te+6aA4Iuls)T?a*IhY;ZE*ztVrFP#c{YDf+ zwq(y$o|MT}?WDimJ%hdzXy;?vcOkv+%9Uy$R^2^p>IaT}C+wrb*4}bPw;rX|IFYJF zV)lrYN}V#R9a}M2tm^c98-yNxF@p4KewIL0TJ##oNs6kCcH;W^cBIjWJ^WwU3OXPnl2FZ_Thk z#zBhjwTH;IHCf$WZG1mna$sPWm8S0tc zuw?M_K<>vu&!^Ssz_#$GcO88)ysl{-_ye5dbq>UR6smhQ9_~mbJJ@cPaAyizUZP3x z9Q()%0k*j$V~ym88K*pg6;RL8?ikY7YgA}kM|d##nL&tWVuGo`3H4e$7Csvvf;B*} zqvNrNuR!~kDY3gR)-=3DcGRc zWPMCJr+bDkl(a*7nMoUQ5Fqgjn(f(!A9#{8{**ctG=#*urlE$5 z4ry`A2iTw`d1XGp9H!F%v&a&e@X*c;=gS~Rh~og~D#If#&F<8Isgz;wov*U@OkOtH z*-&RO1^ziTi^(CGrsh?OevevaaS*?i%Om;cS+|Z`3E!kl6*~up^j56fV2f?C#Wvew zj}?eJ)qg@=S{(q5^n#1)gt-epXn>4`DFWFi79*SjiggLByaokPJfg32VDbm!`F)d_ zIc_vOqiX&qwe1*$f)}M*wNkJ)GizlwK>8erC~vvlm0NF>Nplw83P^zswL{}(0y5U>! z(yMzp8mA0AMoy0Opty~+%dq*?GiPlvk&FQHT}h9^@RoA2i1jW{Gj1b>iY&2$tOZfj zW|8>Tlf-vJj`rEtkJFBN+b=8ElWbQ{v&6<_Dk$#oz?fV=f+m}}jTp;qs~EX%$n|sz z@bly+f8V9&uPC_?Tp;fo=^h=vzW@4TnWnv&75k6?m<{clkoDgfQbO=uY^ESQF7Kmu z>SV3Erjm_p6x_THEI7E1BUc{7_3F$9c!=3nNK z$|7Qb5MhHZUNE=Sd>1T@AY_Z4Jw)4Ti&QOi zF|{G7+06FL;&-gj%Ffw{MYEK1i)_TA5FhxM_y8LZ8G~A8Zbq`SVxIN+Jd9S983dj2 zmuu|@Hl7v?t<%{usq@x1NL@1Cfia{o4VqDJ5kqRew_|UatufkZJdrV^W@*P>n_L>C zc9rFsj-^bw`^WR^mu$bV$WTH$L;4^_#sw%zC)>PJ4|2t5m_kI$UA3G&qtkLA>r}$) zSM>vE7(w?k6(Fik{XYaY68OIWa9zTL`YR}bXcE;8?n3ntc#@(=2{_Op1@wkmh7vY& z5Jp-MEy0aIzD%L75Mbm6Q5sI>ARw`E7GxCXGZX(|pn1?ToIHZln+-+rdhtT^rrJWZ zP#Ad>9<^Fk%aQizqUFQPI(Bxq_doG3M5tt89I3*hNDM5BrkyN)aCp3pi@u}jIqI7AQo=wz*ZCV_{ zwAh^4us^fa`L+52QnMmB^48!gz3Y*y|a z*9C_piX_q9gTrX5m~B_f9MKZ9+apn!MzjnUU2){ZMNMKRlSnoa?Srp#u~E&AqjnXW zj?=E9oaQ7^*_e}cb0_LnX6s;=d|=c&`r_-}Ow+zh{eDCSBD?X``!+I=Zrt@qxZUoW!D|J_pQopx9J__iSC4#dd~9N>o%u6J`<;dv;{+ z^}U&sr?aciWR^XXk)LHq?3rBXS%k+!&rZm;UsisWkeHrkiH)<8{7w$xWzU%5NwpyJ z6m#WA)0!(V;V;5^O+sE^)P4a7J@-dU=y`NP??EFkZ>ze5W=R1(x0HrsAsW#YA7YEL z!Q`+5B8#As;);)%R8vFEd7%)UtR7J_@@gQF5j9u08o{v;!ST@Q3EB3`%Bu;7=xLVN zI4jQY<`OazQZnp2N{gG-a~i|uZ1e;!hdlQdrH^c{Q!lw zh^qIDPw|{7BT4&4uTr)^dd_P|vsJ(1Um&TnA_E4L0ANrF!Y0uNo5T=X?Rw*flB6n6 zrW3RAM#H<>;sbryU!Up@lVz{g8yz4mH5B9U3w@R>aC^uamwUL>)#*Jj5YQ5^#idoZ ziS2w)%hA-NMw7LA={hCEJh#}MbQBtbaw#l< zV4T%@uNpe^?nF-`nPD@}P#j^^<2CuYHD99;iK|o^4GkW34TZ^MOkGQWK`Lb9% z9q&p*ZQU2;j-`fY#p350$($h87{hl-vFh^`bYQBnlXkem`uO4Hj(Yoo1_GzDr<0PP z!96@r|EiOoG(3AiO~oc9t0e1Tw%j=5f*OzdS}2JWKpKhgj1#-7u+Nr{F0zk~=N~B; zb4~X9#D41ALEe>N=*&ElJYx9NghF(A<9 zQ6ZTMRluj^B5C$v*QLF0Rn*+9o_Di)*4qo0jBLEpbaU~V(eBLVGnvL`GK-!?$(q`q z`TdnucRZAzqh6~_Q%F9qU@E}IS8B6P5%e5g<{qxFWN!N~Q~3ce?e041gjaK~LziO^ zLxi8&+veppSYg6_6p}p}u1Oz!CSt0sbEo3%Wz28lc_BspL#T0t+_Z>$Ay2y>N;vPS z6Q&Xlk{uyEbtV&y#b`Sk#k^LUPjOb<2tLhd8?(bQOF7FaMJdQxeU>+I&Mg#YC z-K@Lb9PPst)1p=;iWJ^aWL;$!F#S_no=hmA;DS8W$x;x$2nNn6!3D4qtrK28lYY~} z%T#8ie#_{U*l(Z>bhP$N`k&<9W=}fy|7oFWH27rWiPPEWij<^=%7&;8Q*EWc+8HGQftg>KSzG>M9T2`i>H7cF@Q0*Op*+aC{KZ}^ob zzW&5W>dm@Me?0tijn=@=FI)_7)fU4qs1vw00s&nc0V^}+Hj%8$$ZaAdDZtG;QJNcd z?M4Wvhi<_0IQCvaw@lrosdG3+H<-HNw<$#Zt11LOg|gU>)K1|R0xuD`L|`jbyP3c% z6g@%U%LJGzlPM^E6H$#M(Omw+A(sMi26u*?ncw>$KFL9}S^HN2*t02DOvnqf^1>?z z$K(|jAJ%`n>4{wJ&fk4{G%!)OY^-kCzkm9YKU2H&?TT$TE4JM7c^1NPv?s9F1-LMH zTk??SXo}xu@clAr!(LZ*{hoLIlz8bpzj?>Fxnr;E_fEXe?!9NEtwn(~dv4hZ{LHwy z9R(Wf_e5oud#pA5S)yLv5MCApNX1?ke=yG`p0Ey&Z`osQ(`{L*Q(XUfI9J<-Gg7~* zVWj3l6dVs>!<6lZ!&S-CqMl}n;iy#<<9BihPb!JTpWi8$>UO!9!2Z?Q2mD^!o+3eS zd;_!MgV7tN-zFR>;3A&Hzz?7*+^P0q3of)mn0^>Ntef?mPy(OyGj-QP@8{qr{rq9{GZ^UdR%X4O53$X?g98!W10XX( zQb|~p_hVPIhI6{qT0)mEqhZ{Pye^pv!^oYsB5;Slu(cmvoJhiso~-nVpyDJ}z1U(6 z{|Y8PAE%m3d_JWulFb6?B`V0U_%3SE_bAG+xIo}e7D!tK0i?SRgdgG|Epi^h708#d zYas$*3oNiMKRZEMbuW ziP!2yENhF2WMrDm+mrNz3Ap|d-8)lU%S2!vh^HX_qSGgvnKOujm<*BECoY81&C}v3 z1y+7YE`$vy4%NfQj$^68RONzFcySm=u{|_m1%eZ z-s%fgnE6`Jn$#v*S4hgxs@Bu$LgWaO9;&M;)J(U*%RBl^Fjq)pwF5t=S7EMpPTY&h zxO@pE9Q?y@F9`YyOSVyPqen~#TV|OCnyqHWhA~>Yxc=D`Gjsj*OsXKstoD60=%Vzt zo#Bux?GnPbS8sUT_xic(FJ^W;m3`#c%~k8J$6r5jJ(Bs<(d?pDNaskTS4JYeAmo5^ zqz})LF;fzN6C9*3agM-w0v7=C{zxR2>WW126+nUML|1CEhQXTJx*VHH-UWw zoRki0=>`E3;%l2x^p~Z-@*a8HS98nV=&Kr;bxXpZ(dBpP&s~pHR?E`wJnIVhaGu|s zGT*lMszbgo*FaE3U8qA`h_mbNdi*KWKtQYDRXl(`#S2)b_yGN+iGb$8uLgSLhCtp= z7;H~pBGFHE)4LfLF$Qeeyunx$iyR(?pQiYe`^qs0;v!yhye5-kkp7mOrr?X7S#Xi- zgVUt7V3#RuZFZ;jSQ!Is(k*ikpZ*kVLJ4L>@iXzMY!jc(Y-a+A@64w-G7lzmVe}QP z9dt4Aomm=;P2mNjSJ6jCX)Lt?%QGEIwF?u|k~zhqTr#I%^GfD4g}Fg}tE|j+RG8vf zXn6Q#sY~pTFzzvP*COIs+jjqRIC$tc9QRfHfoW!(`mv)cfaluF7@Pdp##f2`#P&@c zMY3>vTM5B?=+yTtY;uV&X*!(1@jQaro#yTrvu?*RR%ENRS;~2o2$N-uscOrYaLz46 zwIy>3_Yfs>nj@v=p^X%{I!{*Oe>sHue+mYt*3ljAy{g7W!>FV&mQWcJ#5^EjoZF`jX|T}#HZ}1J1inb% zO#*uXaLNp$VOCH=GXeHmF5nV%oT5(>I7(njx+Whqn~?Y5=~t^P!r4};?gUlB#IL6) z^l1Xj8t*iP&JbYy-m4UvAV8d-M)Q^nXbs2(wF?;2hr=6SI!-FkNc;kL)5vgKP34#B zIFXY?j(L)3lGY1QbbpC)9wB!iiXDqXuMCk}-UX|FXU{eHw~uD#Z$tFb5dP*u+Y!9} z83raf?cFXM@$$g9yps%&bVt2eGlm5eE5asFd=O ztN+J^N~sXIhup9~JSrvhAyX;Uj7q7wPb#H$Mq_NDjcR&WR7!2hG4*h%l)8sDQVOY* zS&meS#W~}XgdDK~>LQ1u6m_C4mT+e$B*hI!!*CWE?u>WCUo1J0WFlV8*k1Z@Nef0~ zs0^h)g|9@vH3uDGxiA_&1^o9+l-w&NI9C_e3Yde&4!LnAcg&I zAP?<`Rf zx9}M`rj#T&8DjkuE1TfLADST+;b`+Ij$9`Zy!Eh)Ds3B6E$`u+)08BXs?+2T%f!k{N?a31PhW}aBuSuSQml>xVe|zY?ZE%0?2-gw zRAu%+%2|?NzM+GR3kvsghNCNNINB`Z+e~uy{06gR8DiGR6VqjV#m>ehR|@U)dL#Wd1_&8u&WCSQdow;=^C(XcWMxB!mw$L zxio`vl>0b?$ruC<8nfn3;&w9Lq)rp5w+`2~ri4E?n5NMaxPtZs@RJ40R^BIL&d{oQ zk}8`LtqOll@&1AIkhv`f5)`(f$Iu@0<6dDyrfGbaHbl>y&898f(=JPg&S-lX3jWD2 z09sv^k#|hgKbEb341rAjW4U_p{z%!8t;e2nr0mFsuq9mo-9Soc8IO)x!Xg6_N5ER^ z*|TPBoYl_n%eDdel~%s7N$5 zYH#Y(nfa$Pa+I2SIu}B#qnVBm^!Zl1C-$6llHZfnaluZnD(n!2g2m5SxaK?+-KS3UJtP zO=TeLf#W?D4V6)HVR0NbTKakuPDinK1}=x;%DJfrN7)Q^HXF)Vjhs}+a4HVg9mevo z?XC$Gi&IJ+f~V^?h&0&|J!}^aEn_e*r2}8M^%v^2-k`Isjgww*7?0S_L;GBU(XiGJ z2i9!gtg++PXd=0KrY6KvJ>994?x=bjOQL7;haod|328y5&jm+^mJB2Y;y5)M-num$ z0@Oz*ZG$%?dsu1;oz&A5H2J<^zyAK}W9as2V3k(vh+$%1p?l2-BL{XJ-xoQy>-fRE zAAWPOmsp#wLRLMF$2u6AE!FV^5Cgrf7D3|$x2JKwZgl~gm?G~N2}}F{kHQH$RZ{gV zE+Fk;3+sOjPPFXKw(Nd=UAAT48-Z-gFJ+ELvMrI^+K$Y8EUZ1W(nfP3po5-FcMpfz zP|t*{|85|iaD0aF6#&LakE^{rJmm}Umm}J z;yjPK1rN?wwC&rqH?nu%{$1@)9%ohvc`uzbT1acHqE>Dr@L2+XO5kS%$jXlOJo}jZ zv(iGGmp{dyvqL?HEci(peVkh!m&HA-=y-tt*p7c z>MPGqR4&U_F3VM}n5b;bRyK}Cvz0B^wq`3om3iV+w(``);7z&aYgMmQai`3g=6$aZ zV-L|q-%Wq@mtJ`3g$e)4tbgUmV9wuo(R)+!eW~K5iV106R+@KZ)|m9j+lHsQkWdY(%3RntfBE<=H0QKyq>ld`-OK-8JN_<9za{%!)o^@rN5bDLdq*~Y>&Pt%zA-Cv z?D!8CvT&v&_A^SmHEa5LZ_jeOp18dHmV`eeb??!i_k(q==Gzj$``8fXYPl^De81Iw s%;h@bx-GFk?{~Xbxz^m40Ny|0YInK9w None: + (directory / name).write_text(yaml.safe_dump(body, sort_keys=False), encoding="utf-8") + + +def caller(job_permissions: dict | None = None, *, workflow_permissions: dict | None = None) -> dict: + job: dict = {"uses": "./.github/workflows/callee.yml"} + if job_permissions is not None: + job["permissions"] = job_permissions + body: dict = {"on": {"push": None}, "jobs": {"call": job}} + if workflow_permissions is not None: + body["permissions"] = workflow_permissions + return body + + +def callee(job_permissions: dict | None = None, *, workflow_permissions: dict | None = None) -> dict: + job: dict = {"runs-on": "mdb-dev", "steps": [{"run": "true"}]} + if job_permissions is not None: + job["permissions"] = job_permissions + body: dict = {"on": {"workflow_call": None}, "jobs": {"work": job}} + if workflow_permissions is not None: + body["permissions"] = workflow_permissions + return body + + +def run(tmp_path: Path, default: str = "read"): + workflows = gate.load_workflows(tmp_path) + return gate.check(workflows, gate.DEFAULT_GRANTS[default]) + + +class TestNormalize: + def test_absent_block_is_none_not_empty(self): + """Declaring nothing and declaring `{}` are different: only one overrides.""" + assert gate.normalize(None) is None + assert gate.normalize({}) == {} + + def test_read_all_and_write_all_shorthands_expand(self): + assert gate.normalize("read-all") == {scope: "read" for scope in gate.SCOPES} + assert gate.normalize("write-all") == {scope: "write" for scope in gate.SCOPES} + + def test_explicit_block_is_stringified(self): + assert gate.normalize({"contents": "read"}) == {"contents": "read"} + + def test_unknown_shorthand_is_an_error_not_a_silent_pass(self): + with pytest.raises(ValueError, match="unknown permissions shorthand"): + gate.normalize("all-of-it") + + def test_unreadable_block_is_an_error(self): + with pytest.raises(ValueError, match="unreadable permissions block"): + gate.normalize(["contents: read"]) + + +class TestGrantResolution: + def test_callee_declaring_nothing_always_composes(self, tmp_path): + write(tmp_path, "caller.yml", caller({"contents": "read"})) + write(tmp_path, "callee.yml", callee()) + violations, _ = run(tmp_path) + assert violations == [] + + def test_callee_within_the_grant_composes(self, tmp_path): + write(tmp_path, "caller.yml", caller({"contents": "read", "pull-requests": "write"})) + write(tmp_path, "callee.yml", callee({"pull-requests": "write"})) + violations, _ = run(tmp_path) + assert violations == [] + + def test_read_is_enough_for_a_read_declaration(self, tmp_path): + write(tmp_path, "caller.yml", caller({"contents": "write"})) + write(tmp_path, "callee.yml", callee({"contents": "read"})) + violations, _ = run(tmp_path) + assert violations == [] + + def test_read_is_not_enough_for_a_write_declaration(self, tmp_path): + write(tmp_path, "caller.yml", caller({"pull-requests": "read"})) + write(tmp_path, "callee.yml", callee({"pull-requests": "write"})) + violations, _ = run(tmp_path) + assert len(violations) == 1 + assert violations[0].scope == "pull-requests" + assert (violations[0].granted, violations[0].declared) == ("read", "write") + + def test_job_block_wins_over_the_workflow_block(self, tmp_path): + """A job-level block replaces the workflow-level one rather than adding to it.""" + write( + tmp_path, + "caller.yml", + caller({"contents": "read"}, workflow_permissions={"pull-requests": "write"}), + ) + write(tmp_path, "callee.yml", callee({"pull-requests": "write"})) + violations, _ = run(tmp_path) + assert len(violations) == 1, "the workflow-level pull-requests grant must not rescue the job" + + def test_workflow_block_applies_when_the_job_declares_nothing(self, tmp_path): + write(tmp_path, "caller.yml", caller(None, workflow_permissions={"pull-requests": "write"})) + write(tmp_path, "callee.yml", callee({"pull-requests": "write"})) + violations, _ = run(tmp_path) + assert violations == [] + + def test_repo_default_applies_when_nothing_is_declared(self, tmp_path): + write(tmp_path, "caller.yml", caller()) + write(tmp_path, "callee.yml", callee({"contents": "read"})) + violations, _ = run(tmp_path) + assert violations == [], "contents: read is inside the default read token" + + def test_repo_default_does_not_cover_other_scopes(self, tmp_path): + write(tmp_path, "caller.yml", caller()) + write(tmp_path, "callee.yml", callee({"issues": "read"})) + violations, _ = run(tmp_path) + assert len(violations) == 1 + assert violations[0].granted == "none" + + def test_a_write_all_repo_default_covers_everything(self, tmp_path): + write(tmp_path, "caller.yml", caller()) + write(tmp_path, "callee.yml", callee({"issues": "write"})) + violations, _ = run(tmp_path, default="write") + assert violations == [] + + def test_callee_workflow_level_block_is_checked_too(self, tmp_path): + write(tmp_path, "caller.yml", caller({"contents": "read"})) + write(tmp_path, "callee.yml", callee(None, workflow_permissions={"packages": "write"})) + violations, _ = run(tmp_path) + assert len(violations) == 1 + assert violations[0].callee_job is None + + +class TestTransitiveAndEdges: + def test_the_cap_reaches_a_nested_callee(self, tmp_path): + """The ceiling flows the whole way down, so the walk has to as well.""" + write(tmp_path, "caller.yml", caller({"contents": "read"})) + write( + tmp_path, + "callee.yml", + { + "on": {"workflow_call": None}, + "jobs": {"nest": {"uses": "./.github/workflows/deep.yml"}}, + }, + ) + write(tmp_path, "deep.yml", callee({"pull-requests": "write"})) + violations, _ = run(tmp_path) + assert len(violations) == 1 + assert violations[0].path == ( + "./.github/workflows/caller.yml", + "./.github/workflows/callee.yml", + "./.github/workflows/deep.yml", + ) + + def test_remote_callees_are_reported_unchecked_not_assumed_safe(self, tmp_path): + write( + tmp_path, + "caller.yml", + { + "on": {"push": None}, + "jobs": {"call": {"uses": "mindsdb/github-actions/.github/workflows/notify.yml@main"}}, + }, + ) + violations, unchecked = run(tmp_path) + assert violations == [] + assert len(unchecked) == 1 + assert "remote" in str(unchecked[0]) + + def test_a_cycle_terminates(self, tmp_path): + write(tmp_path, "caller.yml", caller({"contents": "read"})) + write( + tmp_path, + "callee.yml", + { + "on": {"workflow_call": None}, + "jobs": {"back": {"uses": "./.github/workflows/caller.yml"}}, + }, + ) + violations, _ = run(tmp_path) + assert violations == [] + + def test_a_missing_callee_file_is_not_a_permission_finding(self, tmp_path): + """A bad path is its own startup failure; actionlint and GitHub both say so.""" + write(tmp_path, "caller.yml", caller({"contents": "read"})) + violations, _ = run(tmp_path) + assert violations == [] + + def test_a_typo_in_a_scope_name_is_a_finding(self, tmp_path): + write(tmp_path, "caller.yml", caller({"contents": "read"})) + write(tmp_path, "callee.yml", callee({"pull_requests": "write"})) + violations, _ = run(tmp_path) + assert len(violations) == 1 + assert violations[0].granted == "not a permission scope" + + def test_non_dict_jobs_are_skipped_rather_than_crashing(self, tmp_path): + write(tmp_path, "caller.yml", {"on": {"push": None}, "jobs": {"weird": "not-a-mapping"}}) + write(tmp_path, "callee.yml", callee()) + violations, unchecked = run(tmp_path) + assert (violations, unchecked) == ([], []) + + def test_non_mapping_workflow_files_are_ignored(self, tmp_path): + (tmp_path / "notes.yml").write_text("- just a list\n", encoding="utf-8") + assert gate.load_workflows(tmp_path) == {} + + def test_a_reusable_workflow_is_not_treated_as_a_root(self, tmp_path): + """Its ceiling comes from its caller, so the repo default must not be applied to it. + + Without this, every reusable that calls another reusable is measured + against a grant it never runs with, and the gate cries wolf. + """ + write(tmp_path, "middle.yml", {"on": {"workflow_call": None}, "jobs": {"nest": {"uses": "./.github/workflows/deep.yml"}}}) + write(tmp_path, "deep.yml", callee({"pull-requests": "write"})) + violations, _ = run(tmp_path) + assert violations == [], "nothing can start middle.yml on its own" + + +class TestTriggerParsing: + def test_an_unquoted_on_key_parses_as_the_boolean_true(self, tmp_path): + """The trap: YAML 1.1 resolves a bare `on` to a boolean. + + Every hand-written workflow says `on:` unquoted, so the parsed key is + ``True`` rather than ``"on"``. A gate that only looked up ``"on"`` would + decide every workflow was a reusable and check nothing at all. (Note + ``yaml.safe_dump`` emits ``'on':`` quoted, so a fixture built through it + would hide this — hence the raw text here.) + """ + (tmp_path / "hand-written.yml").write_text( + "name: Hand written\non:\n push:\n branches: [main]\njobs:\n" + " call:\n permissions:\n contents: read\n" + " uses: ./.github/workflows/callee.yml\n", + encoding="utf-8", + ) + raw = yaml.safe_load((tmp_path / "hand-written.yml").read_text(encoding="utf-8")) + assert True in raw and "on" not in raw + assert gate.is_entry_point(raw) + assert gate.triggers(raw) == {"push": {"branches": ["main"]}} + + def test_a_quoted_on_key_is_read_too(self, tmp_path): + write(tmp_path, "caller.yml", caller({"contents": "read"})) + raw = yaml.safe_load((tmp_path / "caller.yml").read_text(encoding="utf-8")) + assert "on" in raw + assert gate.is_entry_point(raw) + + def test_a_string_trigger_is_an_entry_point(self): + assert gate.is_entry_point({"on": "push"}) + + def test_a_list_of_triggers_is_an_entry_point(self): + assert gate.is_entry_point({"on": ["push", "workflow_dispatch"]}) + + def test_workflow_call_alone_is_not_an_entry_point(self): + assert not gate.is_entry_point({"on": {"workflow_call": None}}) + + def test_a_workflow_call_plus_dispatch_is_an_entry_point(self): + assert gate.is_entry_point({"on": {"workflow_call": None, "workflow_dispatch": None}}) + + def test_a_missing_trigger_block_is_not_an_entry_point(self): + assert not gate.is_entry_point({}) + + +class TestTheIncident: + """Replay of the shape that took staging down, as a regression test.""" + + @staticmethod + def _incident_tree(tmp_path: Path) -> None: + # config-apply.yml as it was: `detect` and `plan` naming a PR scope so the + # dev caller's rolling comment would work. + write( + tmp_path, + "config-apply.yml", + { + "on": {"workflow_call": None}, + "jobs": { + "detect": { + "runs-on": "mdb-dev", + "permissions": {"contents": "read", "pull-requests": "read"}, + "steps": [{"run": "true"}], + }, + "plan": { + "runs-on": "mdb-dev", + "permissions": {"contents": "read", "pull-requests": "write"}, + "steps": [{"run": "true"}], + }, + }, + }, + ) + # The dev caller granted the scope, so every PR was green. + write( + tmp_path, + "dev-build-deploy.yml", + { + "on": {"pull_request": None}, + "jobs": { + "config-apply": { + "permissions": {"contents": "read", "pull-requests": "write"}, + "uses": "./.github/workflows/config-apply.yml", + } + }, + }, + ) + # The push callers did not, and neither run ever started. + for name in ("staging-build-deploy.yml", "prod-build-deploy.yml"): + write( + tmp_path, + name, + { + "on": {"push": None}, + "jobs": {"config-apply": {"uses": "./.github/workflows/config-apply.yml"}}, + }, + ) + + def test_the_original_shape_is_caught_on_both_push_paths(self, tmp_path): + self._incident_tree(tmp_path) + violations, _ = run(tmp_path) + + callers = {violation.caller for violation in violations} + assert callers == { + "./.github/workflows/staging-build-deploy.yml", + "./.github/workflows/prod-build-deploy.yml", + }, "the PR path was fine, which is exactly why this merged" + assert all(violation.scope == "pull-requests" for violation in violations) + + def test_the_finding_says_what_would_happen_and_how_to_fix_it(self, tmp_path): + self._incident_tree(tmp_path) + violations, _ = run(tmp_path) + message = str(violations[0]) + assert "startup_failure" in message + assert "no job to report it" in message + assert "inherits the caller's ceiling" in message + + def test_inheriting_in_the_callee_is_what_resolves_it(self, tmp_path): + """The fix that composes: the callee stops naming the scope only one caller grants.""" + self._incident_tree(tmp_path) + write( + tmp_path, + "config-apply.yml", + { + "on": {"workflow_call": None}, + "jobs": { + "detect": {"runs-on": "mdb-dev", "steps": [{"run": "true"}]}, + "plan": {"runs-on": "mdb-dev", "steps": [{"run": "true"}]}, + }, + }, + ) + violations, _ = run(tmp_path) + assert violations == [] + + +class TestCli: + def test_clean_tree_exits_zero(self, tmp_path, capsys): + write(tmp_path, "caller.yml", caller({"contents": "read"})) + write(tmp_path, "callee.yml", callee()) + assert gate.main(["--workflow-dir", str(tmp_path)]) == 0 + assert "every local reusable call composes" in capsys.readouterr().out + + def test_violations_exit_nonzero(self, tmp_path, capsys): + write(tmp_path, "caller.yml", caller({"contents": "read"})) + write(tmp_path, "callee.yml", callee({"pull-requests": "write"})) + assert gate.main(["--workflow-dir", str(tmp_path)]) == 1 + assert "would fail a run before it starts" in capsys.readouterr().err + + def test_an_empty_workflow_dir_is_a_failure_not_a_pass(self, tmp_path, capsys): + """A gate that reports success when it found nothing to check is worse than none.""" + assert gate.main(["--workflow-dir", str(tmp_path)]) == 1 + assert "No workflows found" in capsys.readouterr().err + + def test_the_repo_default_is_selectable(self, tmp_path): + write(tmp_path, "caller.yml", caller()) + write(tmp_path, "callee.yml", callee({"issues": "write"})) + assert gate.main(["--workflow-dir", str(tmp_path), "--default-permissions", "write"]) == 0 + assert gate.main(["--workflow-dir", str(tmp_path), "--default-permissions", "read"]) == 1 + + def test_this_repos_own_workflows_pass(self): + """The default target is `.github/workflows` under the working directory. + + Run from this repo's root that is this repo's own reusables, which is both + a smoke test of the default and the dogfood check: the repo that defines + everyone's pipelines should pass its own gate. + """ + assert gate.main(["--workflow-dir", str(_GATE_PATH.parents[1] / ".github" / "workflows")]) == 0 From 5dacfce08043f01aabb013a238e34866238b45d0 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 26 Jul 2026 14:49:10 -0700 Subject: [PATCH 2/8] chore(ci): enhance workflow checks and refactor for external usage (ENG-1053) Replaces `workflow_permissions.py` with `workflow_graph.py` to incorporate additional static checks on workflow graphs, addressing issues with multiple run trees and uncalled reusables. Updates related workflow and test references. Introduces `.gitignore` to exclude common Python and IDE-specific files. --- .github/workflows/ci.yml | 7 +- .github/workflows/workflow-lint.yml | 4 +- .gitignore | 228 ++++++++++++++++++ README.md | 2 +- ...kflow_permissions.py => workflow_graph.py} | 111 ++++++++- ...w_permissions.cpython-313-pytest-8.4.1.pyc | Bin 61526 -> 0 bytes ..._permissions.py => test_workflow_graph.py} | 77 +++++- 7 files changed, 419 insertions(+), 10 deletions(-) create mode 100644 .gitignore rename scripts/{workflow_permissions.py => workflow_graph.py} (74%) delete mode 100644 tests/__pycache__/test_workflow_permissions.cpython-313-pytest-8.4.1.pyc rename tests/{test_workflow_permissions.py => test_workflow_graph.py} (81%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93ed726..0b44443 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,8 +51,11 @@ jobs: - name: Check that this repo's own reusables compose run: | - uv run --with pyyaml --python 3.12 scripts/workflow_permissions.py \ - --workflow-dir .github/workflows + # --allow-external-reusables: everything here is called from OTHER repos, + # so "nothing in this repo calls it" is the normal state rather than a + # finding. Consumers run without the flag and do get told about orphans. + uv run --with pyyaml --python 3.12 scripts/workflow_graph.py \ + --workflow-dir .github/workflows --allow-external-reusables - name: Audit workflows with zizmor (advisory) env: diff --git a/.github/workflows/workflow-lint.yml b/.github/workflows/workflow-lint.yml index 74d7556..0df52e7 100644 --- a/.github/workflows/workflow-lint.yml +++ b/.github/workflows/workflow-lint.yml @@ -9,7 +9,7 @@ # Everything it finds is a mistake. # permissions The one check neither tool does: a called workflow may never # declare a permission its caller lacks. Blocking. See -# scripts/workflow_permissions.py for why this is not redundant — +# scripts/workflow_graph.py for why this is not redundant — # short version, it was verified against a tree that had already # broken production and neither tool said a word. # zizmor The established Actions SECURITY auditor (template injection, @@ -100,7 +100,7 @@ jobs: run: | set -euo pipefail uv run --with pyyaml --python 3.12 \ - .ci-shared/scripts/workflow_permissions.py \ + .ci-shared/scripts/workflow_graph.py \ --workflow-dir .github/workflows \ --default-permissions "$DEFAULTS" diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..78ebe0a --- /dev/null +++ b/.gitignore @@ -0,0 +1,228 @@ +# Byte-compiled / optimized / DLL files +__pycache__/ +*.py[codz] +*$py.class + +# C extensions +*.so + +# Distribution / packaging +.Python +build/ +develop-eggs/ +dist/ +downloads/ +eggs/ +.eggs/ +lib/ +lib64/ +parts/ +sdist/ +var/ +wheels/ +share/python-wheels/ +*.egg-info/ +.installed.cfg +*.egg +MANIFEST + +# PyInstaller +# Usually these files are written by a python script from a template +# before PyInstaller builds the exe, so as to inject date/other infos into it. +*.manifest +*.spec + +# Installer logs +pip-log.txt +pip-delete-this-directory.txt + +# Unit test / coverage reports +htmlcov/ +.tox/ +.nox/ +.coverage +.coverage.* +.cache +nosetests.xml +coverage.xml +*.cover +*.py.cover +.hypothesis/ +.pytest_cache/ +cover/ + +# Translations +*.mo +*.pot + +# Django stuff: +*.log +local_settings.py +db.sqlite3 +db.sqlite3-journal +staticfiles/ + +# Flask stuff: +instance/ +.webassets-cache + +# Scrapy stuff: +.scrapy + +# Sphinx documentation +docs/_build/ + +# PyBuilder +.pybuilder/ +target/ + +# Jupyter Notebook +.ipynb_checkpoints + +# IPython +profile_default/ +ipython_config.py + +# pyenv +# For a library or package, you might want to ignore these files since the code is +# intended to run in multiple environments; otherwise, check them in: +# .python-version + +# pipenv +# According to pypa/pipenv#598, it is recommended to include Pipfile.lock in version control. +# However, in case of collaboration, if having platform-specific dependencies or dependencies +# having no cross-platform support, pipenv may install dependencies that don't work, or not +# install all needed dependencies. +#Pipfile.lock + +# UV +# Similar to Pipfile.lock, it is generally recommended to include uv.lock in version control. +# This is especially recommended for binary packages to ensure reproducibility, and is more +# commonly ignored for libraries. +#uv.lock + +# poetry +# Similar to Pipfile.lock, it is generally recommended to include poetry.lock in version control. +# This is especially recommended for binary packages to ensure reproducibility, and is more +# commonly ignored for libraries. +# https://python-poetry.org/docs/basic-usage/#commit-your-poetrylock-file-to-version-control +#poetry.lock +#poetry.toml + +# pdm +# Similar to Pipfile.lock, it is generally recommended to include pdm.lock in version control. +# pdm recommends including project-wide configuration in pdm.toml, but excluding .pdm-python. +# https://pdm-project.org/en/latest/usage/project/#working-with-version-control +#pdm.lock +#pdm.toml +.pdm-python +.pdm-build/ + +# pixi +# Similar to Pipfile.lock, it is generally recommended to include pixi.lock in version control. +#pixi.lock +# Pixi creates a virtual environment in the .pixi directory, just like venv module creates one +# in the .venv directory. It is recommended not to include this directory in version control. +.pixi + +# PEP 582; used by e.g. github.com/David-OConnor/pyflow and github.com/pdm-project/pdm +__pypackages__/ + +# Celery stuff +celerybeat-schedule +celerybeat.pid + +# SageMath parsed files +*.sage.py + +# Environments +.env +.env.integration* +.envrc +.venv +env/ +venv/ +ENV/ +env.bak/ +venv.bak/ + +# Spyder project settings +.spyderproject +.spyproject + +# Rope project settings +.ropeproject + +# mkdocs documentation +/site + +# mypy +.mypy_cache/ +.dmypy.json +dmypy.json + +# Pyre type checker +.pyre/ + +# pytype static type analyzer +.pytype/ + +# Cython debug symbols +cython_debug/ + +# PyCharm +# JetBrains specific template is maintained in a separate JetBrains.gitignore that can +# be found at https://github.com/github/gitignore/blob/main/Global/JetBrains.gitignore +# and can be added to the global gitignore or merged into this file. For a more nuclear +# option (not recommended) you can uncomment the following to ignore the entire idea folder. +#.idea/ + +# Abstra +# Abstra is an AI-powered process automation framework. +# Ignore directories containing user credentials, local state, and settings. +# Learn more at https://abstra.io/docs +.abstra/ + +# Visual Studio Code +# Visual Studio Code specific template is maintained in a separate VisualStudioCode.gitignore +# that can be found at https://github.com/github/gitignore/blob/main/Global/VisualStudioCode.gitignore +# and can be added to the global gitignore or merged into this file. However, if you prefer, +# you could uncomment the following to ignore the entire vscode folder +# .vscode/ + +# Ruff stuff: +.ruff_cache/ + +# PyPI configuration file +.pypirc + +# Cursor +# Cursor is an AI-powered code editor. `.cursorignore` specifies files/directories to +# exclude from AI features like autocomplete and code analysis. Recommended for sensitive data +# refer to https://docs.cursor.com/context/ignore-files +.cursorignore +.cursorindexingignore + +# Marimo +marimo/_static/ +marimo/_lsp/ +__marimo__/ + +# Mac +.DS_Store + +# Deployment +deployment/**/charts +deployment/**/requirements.lock + +# JetBrains +.idea/ + +# sed -i.bak backups (e.g. from `make prefect/...set-image-tag` / set-env) +*.bak + +# UV +uv.lock + +# VSCode +.vscode/ \ No newline at end of file diff --git a/README.md b/README.md index f375a7a..6b16b12 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,7 @@ which is ours: | Layer | Blocking | What it is | | --- | --- | --- | | `actionlint` | yes | The established syntax + expression + shellcheck linter | -| permission check | yes | `scripts/workflow_permissions.py` — the one gap neither tool covers | +| permission check | yes | `scripts/workflow_graph.py` — the one gap neither tool covers | | `zizmor` | advisory by default | The established Actions *security* auditor (template injection, credential persistence, unpinned actions) | ```yaml diff --git a/scripts/workflow_permissions.py b/scripts/workflow_graph.py similarity index 74% rename from scripts/workflow_permissions.py rename to scripts/workflow_graph.py index 8b0faec..6388e2a 100644 --- a/scripts/workflow_permissions.py +++ b/scripts/workflow_graph.py @@ -1,4 +1,27 @@ -"""Gate: a called workflow may never declare a permission its caller lacks. +"""Two static checks on a repo's workflow graph that no existing tool performs. + +CHECK 1 — one run tree per event. + +A push or a pull request should produce ONE run whose tree shows everything that +happened, not several disconnected checks a reviewer has to find and correlate. +Two entry-point workflows on the same event means two run trees, and the second +one is the one nobody looks at: `auto-release.yml` and `prod-build-deploy.yml` +both fired on push to `main` for months, so every release was two runs, and the +tag could be cut for a commit whose deploy had been cancelled. + +Nothing warns about this. GitHub is perfectly happy to run ten workflows on one +push, and each looks healthy in isolation. So: workflows sharing an event key are +an error, and the fix is to fold the side one in as a `workflow_call` job. + +Schedules and `workflow_dispatch` are exempt — they have no pipeline to belong to, +which is the whole reason a nightly drift check or a manual smoke test is allowed +its own run. + +It also reports reusables nothing calls, as notes rather than errors, since a +library repo's reusables are called from other repos by design +(`--allow-external-reusables` silences that). + +CHECK 2 — a called workflow may never declare a permission its caller lacks. GitHub caps a called workflow's token at the calling job's grant, and it checks that cap when the workflow FILE IS LOADED, not when a job runs. So a job in a @@ -327,6 +350,70 @@ def _walk( return found +def event_keys(workflow: dict) -> list[str]: + """The events that can start this workflow, keyed finely enough to spot a clash. + + `push` is keyed per branch, because a workflow on `push: staging` and one on + `push: main` do not collide. Everything else keys on the event name alone. + + `schedule`, `workflow_dispatch`, and `workflow_call` are deliberately absent: a + schedule or a manual run has no pipeline to belong to, and a `workflow_call` is + not an entry point at all. + """ + keys: list[str] = [] + for event, config in triggers(workflow).items(): + if event in ("schedule", "workflow_dispatch", "workflow_call"): + continue + branches = None + if isinstance(config, dict): + branches = config.get("branches") + if isinstance(branches, list) and branches: + keys.extend(f"{event}:{branch}" for branch in branches) + else: + keys.append(str(event)) + return keys + + +def check_run_trees( + workflows: dict[str, dict], *, allow_external_reusables: bool = False +) -> tuple[list[str], list[str]]: + """Find events that produce more than one run, and reusables nothing calls. + + Returns (errors, notes). + """ + errors: list[str] = [] + notes: list[str] = [] + + by_event: dict[str, list[str]] = {} + for key, workflow in workflows.items(): + if not is_entry_point(workflow): + continue + for event in event_keys(workflow): + by_event.setdefault(event, []).append(key) + + for event, owners in sorted(by_event.items()): + if len(owners) > 1: + names = ", ".join(sorted(owners)) + errors.append( + f"`{event}` starts {len(owners)} workflows, so it produces {len(owners)} " + f"disconnected run trees: {names}\n" + f" Fold the side ones into the event's pipeline as `workflow_call` jobs, so one run " + f"shows everything that happened. A job that should not pay for itself on every run " + f"gates on a path check; a job behind an `environment:` approval needs an ungated plan " + f"job before it, since an approval blocks a job from starting." + ) + + if not allow_external_reusables: + called = {callee for workflow in workflows.values() for _, callee in local_calls(workflow)} + for key, workflow in workflows.items(): + if is_entry_point(workflow) or key in called: + continue + if "workflow_call" in triggers(workflow): + notes.append(f"{key} is a reusable that nothing in this repo calls") + + return errors, notes + + def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument( @@ -342,6 +429,11 @@ def main(argv: list[str] | None = None) -> int: default="read", help="what an undeclared workflow token carries in this repo (Settings -> Actions)", ) + parser.add_argument( + "--allow-external-reusables", + action="store_true", + help="this repo's reusables are called from other repos, so do not report them as uncalled", + ) args = parser.parse_args(argv) workflows = load_workflows(args.workflow_dir) @@ -350,17 +442,32 @@ def main(argv: list[str] | None = None) -> int: return 1 violations, unchecked = check(workflows, DEFAULT_GRANTS[args.default_permissions]) + tree_errors, tree_notes = check_run_trees( + workflows, allow_external_reusables=args.allow_external_reusables + ) for call in unchecked: print(f"note: {call}") + for note in tree_notes: + print(f"note: {note}") + + if tree_errors: + print(f"\n{len(tree_errors)} event(s) produce more than one run tree:\n", file=sys.stderr) + for error in tree_errors: + print(f" {error}\n", file=sys.stderr) if violations: print(f"\n{len(violations)} permission mismatch(es) would fail a run before it starts:\n", file=sys.stderr) for violation in violations: print(f" {violation}\n", file=sys.stderr) + + if tree_errors or violations: return 1 - print(f"\nChecked {len(workflows)} workflow(s): every local reusable call composes.") + print( + f"\nChecked {len(workflows)} workflow(s): every local reusable call composes, " + f"and every event produces exactly one run tree." + ) return 0 diff --git a/tests/__pycache__/test_workflow_permissions.cpython-313-pytest-8.4.1.pyc b/tests/__pycache__/test_workflow_permissions.cpython-313-pytest-8.4.1.pyc deleted file mode 100644 index 6cd3c1c41039761eec1291bab654663b0b6cf9ec..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 61526 zcmeHwd2}4dd1uds=@}e@gZCi~PY@(=@D@pn;(dw`gC^?7ZygY53O=QQ}%y0mKnN{+X#JexbdN({IE9M`& z`}?Z8tGfq17=R)rd86j=LsfNEb#--BeP?}jx3V%Q;rjD)-Os(dN0R=YBE}`_fxFMT zCFz@zBB_$%Qrt&es+;}w9QJDNuyoj`mWBBpzbfO7=SbjiPz~y7JF5(t~Y}m}y2x@%j){e2wLRWdzmgGjiKfT$n-Yqd?j!us07a} zP|D9NR4NqrMp>y`AU)?*7b!~=w^G&QQbM@nZuH}={UTIxX5q#%rJ8a+=YGzmE}lm2 z8j<@EWvNn&=hmHBu+gi`V(BjR(RtERNtwMsQs#)B3bVAiNSi0pmaw$>NLwJ%ma?>k zNLwV*ma(+ONP9%2EoW(uA}uV^R`j?VmJPm%kJFw^q{HdBme#`EDK(t#jfc;t)U(}v zsq^81xZ0o4v_vWy?un-3;Z+?ST9=v_NNdeTo`{)8YZ@5p=xAsP29Ki>Rv>;MK^1q# zyP|_yJe=wdN5g77+80hFyArW@GL1mEqoY5OjA^mX=ICI$x1(cQu%n|ZmF!ORG)4yo z`i7c@`ujRM>NRAH#nbVwbVo-xnv5Yj&=*Z2@?5G@3&-MJeNis(95ep|%>2Qx4O{Uj| z48T_h5R0K3!}|A0QLvYNMKwOCMLYXYEj2wk6vWtJ z&594C&}sA~>3gBWjgi>cmq@1hSZNBrL$klN!Ik$Oi>7!{#K!W082Yg*ol=ML-ej~tp7(a9Vng{dHJ%<+lU*KqpnhT5kSKn)72uoFDHkoA zZTb?d=V5x(1qdada^yi{sui~d%(;!(U?-C?OmYVfu0$6E!ol$ws5jSp$}e1qxk z#>cL@@^U=cm5O0xGYY1Gzm zIGjJx+^%5)H1`d5MYX20sZ=sO+1euO0W4 zvL<)Z)9zyT>-hb4&vL23-rRv(8>vh*Evo$*|?!D<>nDy7+d}RHlXR`9b zo8_U4N7Ol}#JQ99;tua5#na+yb}3$ThS^y@)>&n&v;3^HWLsyk`>eA9t@r%>vChU= z{9N9H)g|vutAp{Y?t6Y(9vjgOJ}n&|(9}hEV!l$04|?Ccm!j&UxT6w1p)Lc^oH{11 z61HDMw+{fIV-~Hrw&MDnYma7Hc4ez~Pwt*oqsue%ny;^XecAQJ*_s2z`zJ4_`v)Qe zSR(TNSiCzr*q81Sa}v)YdV!6rVFbP@od)Dr8>JRp?UF%&B;A&rR7b(pZoN;VyWnbf zQ6U3aFUK5cZSdy3gi`ZWglxoA)AGKq-gwv9eAV84`**cJc|3BUZCC4Y1(TczAaxx; zK45gbW=wT9?$lM3g8-3$S`)xU>8=BgVn9(t=h^tElTY`dIzZPvt+ zjoBp|ubs>;c|2FXBjeq{hZu^9%b1gQ*CXkh(ius0frP~>rg{|j0!j5M9zdVs1uRp1 zfPSS6P*(hEphs>9vyA^xRNf# zm0CjUsQ5A-r0i+;J}ITz>CNei{Zx`p)4*d_o5S}2iRNziZ3LNpYU4)tCOxm>Jx#6Q zr5H(X#Q5K(tY0a-9sgq(>6WKd@H|$y)H1$|_8M)BF0hrZvXdZt~c*T|{(ntYAu`(&9?ruf^-T3jsVKV4(ItL3S+Mw$4rl_ zAvk|eIASni0xRSyF0FeyV({_CHsZ|!1{pVR>AUUO|78S z+zGA0HT=|yRRcqt8?@kO^+5RXFoo6|NeJjkEe%>z(pVLP&#$6v8ymES;Pc^x#tLn1 z(A16S=c~TFXJDv>zmuNt`lIPc`~tCwQKD9_ z%6YFA@9WN&({p%dtg}WoSRq&i#zdqO zZ*?}3&>~5U3jnmRe;_@SUuVO7P17hsXhmBGoO@CFNvQ5?FTCX`WsD47$e>dd%OlXOS^1!%UPp#6kWX%|EP^;)hPDbq_DC$7+ zNSi2X4dBss;uzg%(iQ)FpLB%ZBi2Eow#d0Yu zC4EWnsZ9*SbQ>6-Q6+*FX`?|^aDgiEw3mS@L0Jn`;yW#9kv2yv!w~ohs>FPXj=X6GG)(G$Mz}ela%`?fmQ-f5jaNRmk6{GPyiYNYCDBb5I9NT6aWMWB)U~UO$kpE zpmDFB0nkW*#gMFtXd4;Uq&J8VX#x5y!X;r80}-J@a0pQ|(FFLL-|m2Nr(1X@HKCUw zUxDV@uRNEL+kxhu%Y_;c`thv4T==7fxyJS%EqvW~?Zj{O|8OBvGmY(;P(wDHl(hy#TTfm&n~_hVt!Hzg)d>A~ z<6mz7(e_-^$scWheZ{q&-+JMP+mV`SI++Qr&W27-$olUF5<{y+ln2J;lhjr{OV*6x z2DO!L70;L&zRz z&CAe0r9rN1(^v-8!x)+Baas^2)Ba1@l0-~MqAOvc{l~e8Rv1K8;T~NO2`LKMEeHgj4d_s zBlH@C?=(DG(%Hm~Y^dl@G|?6DiqT{Q@`IGh_{C^MgVqtOpRDZ4YmDVfU$13XI_efQ(u8GQ{Z&n_?$FT3W=WO-- zUT~M+Z(EDShyB;6@!@>fX0ayS|EFCvm?T_JN=Dn)W&F9CK--6_j74A@X{}@GStP0{ z0P9ysA()pVOjr_$k%;;Xy=o_cZUO@YG=TicgozpRqx4eUo$UBdAsmmTr@Z6x=Zxp%+v``Dg+c6 zFsK9o%atHtg;EY!sZ^*{J(Uer`8xV82T0A|7S~dJgCrw*hn{6q0@0ez7OJ<=9*k$B zRZ=^|%ctoMyI?s2ZJNR4ZPL@RNLllHPYZRB!G>FM62;9flN}iZJu|SZSxW7mB!d1f zgsRjwcC`tn-$)_m8_G^nTa=wzZV?dpPYba?nG~rP{3B2wBxD#)2Bf zNU*fcSzIp|cc>}+C3BmK0%#+3ML|FbTIhh;MvE+U(TFU#iYN*|2MiGIFK@jE;{~z= zAvLcWM`=a^=&4Nk%eW#4OlwID3(;&?mw9f0NHusak?Ld8M(wIw-HGBPd);ua$|8H+ zgnUTcbhUyh#hK!lDLJ`HZkim^Rhe^4E!4!9=s~WCRZ6*@@lrg6iz+u-BbyzF70y4g z*N3ryA@3%nLnbHSsm@1-vkt;9tUa5Gkd-S>M zD>IF|aw~U_$$LcVtG#1#^IP*4UOD)ym9JD@3f^4&XhweYN?SJc=!C5QZpe>b8Wu4g z7`LTm%@}T6fq>8~&BLgJ(<+#F;v(vTEdl>fatR=1dWl8AXD$NJbvZ8@4}K}|v!y_` zEd_y(;ZhK=Okd}P!(0l2VkszBDjxV!P}!=!fDg|(09suzp=mw1S81~^Q(vS^mkBUw z$wdm$d{jSA;AH||B=E}wz69Vf^HuZnS0WjKy;Fo4p_wyoYpEGmv?z&G zv~L1fW*X+1@^rTGbZ+IDG5HxW)kag-&u1DB?TLOZy!2 zKllk(#wMKKHsR!t;e?ZCFyR7X!UdJ`2R`8{^a;n1E=@Q-wdm7)fT^{bO)a9fiIW0( zJJ{1p%E3q!3PmJxJjYOWen-*yHLdbQHEYDrS|%CLS$(E)cW&jLF?p{!$81RMBSLFp zj-By6NXmBRbL_NW-z}y(UZpH`yxnJ4iyd&qDnVNcYS&rCMFu3Gd*xI6EOc++w4ipK zl>nRY;pk#?FPZC@PjN&<=DaZwzMy{>*hfOwpylH* zo~^>LmQvqp!`CRhl3v9=DeSttZwo= zq@D~wQEBVVHZtWbxQw25+Cj3>oeU^7sjKZlHjnojV=x9Wilr}#^|TkWxc1by%~@7GWu`eFu2M&I%Z>3BEx5lXk<9fi|#Zy z$`;}%!A5eEAOuYNA7=9dqaCvO(fLi-4&^F=VP4j zd`uxK?NL`f>bI#{QFnveLK^GbFp3(jgMAVRW>#c(VN1|**1U#l zM-KQ3J!hJ1uo^|?yq^)ukfqBUoyTJh6$R|EdWmX%g#c46T8w~fjPVvLm%P==QMWL~ zr1kbDjYh7}LL$H|ez-xrK{GHTA1mF#m6BG%V^2xKqjV)*Xo~g?kjqx;BX#{;rg496 z<$*Ezph}=g&CvWSOI~?CBX61rEzO3OA~2F-ARF2=fmHUpA#cir zmh$kJyy?>OJead2BRrB41w>$6-gL8i!IgDiubik}k*!`aGJI{xuRlLg|9H0k@l0q( zMpiPr6%Ml@WkS|}H;^9M!J}iAaE|2{*+c+&rVuK)J*BBHY+eB0qLgiQTjYiK)ck@Z z(r*zeu)N0VX31Za$0E)wVWN-6zU}!K`l5%B$_ydM2CCv{3WeJJ_+E`YPRcjlg#SoQ!AIFg_VA*ELw@ zVS^hXyJaSbGWL-0oB1b-;;ZRZK&mwD9{^yxDp%gDn02#a-dh#5mk*DWW$Nm$znD38 zGPC1Uw&K(8`n`eRyFn>XrAvoPp{_WgE&ztQ;uA9ca&nw-jUMBHaXC(?OHa$%Vm$dx zIZnvR%wfb()TjTu` zrfI2@#$syRvIcj+G=qKUBSrjK+fa zlI~W4BPu=x4Z%>6>F2hPUf8Mo0L06G2oj6h#;$*fs$I+5JF@wT5U40YMv_>C(I(q& z_Mx)?sR-N76v4)1WsJH>M>x#W;byy?2yMv$?!<1Na2m9LzOj^!1lS?R|rGH3jw*oKj-%sT^wdq&*#uU zjYuoYs=$@G&I;cGS$LbS;)SY~emCScQt|3B9vHW!Wz86FkcwCTW#qQZ5;H2&Cu3%@ zEG+PTRLepUj?CxCkxfS;UdA{wzl|f4XTIqIQE-hdn)$gH@v^*PHo{^hI%NWRpnkHoZcSCN< zgqHK@nA}9-PP&t`BqKapCJKnan{v|>*zRx8haVOZO!CaF?hk0)_z4G!p>MyY$$>JW z_WLNxD9#2Ka}@FcY%I9GBWl@gWt+Wh-oy3+!lWV2c^21qOurIRVCc)-RCW=xpvv|m2ex*tYJrts{dP<^FBBB!hcFdjaFHeJ{tYPd* zt(9FVNpcIBK*^lFk#`1;5(5;43x10P?9CrFKTGTxdgnj&`LFfRw0xNGQ;- z*d#l-2PK~>Gi(50M>uCnQey6X%BT3)nr9tbCHv>$5)<8u-4^ro=u6`Q%jF>G!Yu6xf%`_6`YqvVu040t}?*Eh+If^m+da)G1N zBvv?;j6Loxa`KiMi^m7pCOSxtrF$^U)1#*Pb0#EAgJ%k#tbDT`=9Z-X2mqAM+9@a< za`-t}lJ+2I94}Q}oVr=g)G!c#VW-ak?LGh{gU#I$>PXNtoMPc_NhV_szO$}GVN$ec zR~ab-y9(zGb==zG?YS#Qy_q?U*Y=M- zb^T;!&A#07{bTZhUjUNlmIRxTi4aUcmarhe%Ags#EqSYh?^B+;1pY~WM|wAa0DOnB zRNM~)-OZ8p26y@iQTU;n+&EM@8%A*h)E%67e0-s%pzKoO7G90kbvEIfb%K zsnHd8L8VryYY*xh2+C*RySCiucjK?_C5{w#SCm<)A1JelG)e|e6YtNic0r<~-fFL; znU*rQS4`OY#d&1SX{YTKb5H$+IkHOjPKD7vW!{Xw?sUc=z9qkmXNa#18lzOpPERV0 zuRX6+DYFkt<`jphDD(BNU8O8g7PeREUpq8|uN^Yp(Dsw{9n4oOJCtd}o3^m|z+h4rwJ?iVQ*TMf z9`vVh=mgBB;GG=)Y?C0l*ya`50D>bMi2B-M>|;Sd-^YR-Eo>hPOkS{$CGP?K*x zyhh-=1W4g&-3+qXx)0|roP65Cn-GXU(bwF^ z50uIMNG1Qa`8H=!0w)OhmzZgtB!kgL4Uv~Y5iY)xzMRwv!dPZWJTtG`Mi4GpK63EY z%B;Nj;rU;HhXzv1Zv6_J*)AQaVE`LHCS?7Wlb4bwDl-A$ad|13^6F_>TZ|{aDKEXe z)tbYIp{P+EY96T(c|-uoH$!tX^4igwZ+#AbzO&)V=W+|zB9vLUHXB+yA?v>z^4d!m zcx=oJirYy0&IS?7nz33{q_%zn~tK}ABL+YJr z@3;FBGCO#%n9EkUXrybX3l66A1BsB9Z<=cLnw+1h2Cm{67OiffFy2_NY#i+2%#tIS zc~9CFtoe&R5`Qr=nopeA-S6Mu)O_&M&C8~n&o}o<1DkF^ z<_Id-P(FNU7mlvN;b0KyFq2-KWJaz4^%K6r`Uzj#!~7^DI(~{B?xi0XhV!y|aY)w? z&bpc;M4OTdYgPXmL#LP}?`D0Pv4h#*lS$Wd(K%$?sXTn^Th2*>Tel?bisGjJ#e_=H z$PBCdGqg}6=QwQpj+2N39^3)_|UfBD=k)lOG!t5*zYTbeu{pbUB6(YtW3KIWh z5%x3{Byf2OZY^LHV$aZ71o2=(TF~aGdN6r9<6Q;fBp%ExEjWJT{h6V0+@rNM*2 zQOw6sK~i4oT})|F$(*K3iwa@DTd7n%Fa-%Y5-<#SLxdWNy0oaCJi1~p+rFFQI#2F& zF}7-~W2@v_3+Y42oPt0qnbYjaTA?6$&^>7?NTh~3N0KVf{Cj~j&u;3jP95pQhS8GQn(U=Y*{PZXkusXel}-?M8lq@{8 z41QL;%~G?p(aEa~`C2h~9l?Er#ka=LhNcF8H=bi4#AnRaf<|JID0alP-DZDn2?Rp#(r#z1TnHk3uI2FagtVQs^}Rr=8au z*mTD(fJiLSl_mozE*sL$CI$u|1mc_Au?-s@g}c(i2wd)dqV7W%S zuf;~6eZA+67c=`J*_9n*a#ZZx6|yvW-vvv3`yt7W4PJ1o(|}@U^_>wYfNyaBO}_(oJ5WT-+UWm;M2IIYg*^%!#EfXGoK{%!&Qyp=g5ylG~9FIgX4o?K~p6Xzw%T!^X+0hgtWBipVBg`!eT@?Zt# z^TLbWY$UMDQVSN^!VAdIN&3tVP3`f(JJ0wb2*3?LAeDnl^ClhHL?MMgAC$zO;wm%5_NYqbx#}V;ZDap@| zxQ8wF{qd+ah}j*C_CUSK)=sud5;ceU3HNjgb(lON;}Cls)rlIybcS-cONDEU@cC3< zcT;dU*q?~S`nX?r>bq9;hk#s`{v!$jHB0KB6Sz!(y3=W=)Nnnuk?icc;ERX}3BzX9 zI0Te+6aA4Iuls)T?a*IhY;ZE*ztVrFP#c{YDf+ zwq(y$o|MT}?WDimJ%hdzXy;?vcOkv+%9Uy$R^2^p>IaT}C+wrb*4}bPw;rX|IFYJF zV)lrYN}V#R9a}M2tm^c98-yNxF@p4KewIL0TJ##oNs6kCcH;W^cBIjWJ^WwU3OXPnl2FZ_Thk z#zBhjwTH;IHCf$WZG1mna$sPWm8S0tc zuw?M_K<>vu&!^Ssz_#$GcO88)ysl{-_ye5dbq>UR6smhQ9_~mbJJ@cPaAyizUZP3x z9Q()%0k*j$V~ym88K*pg6;RL8?ikY7YgA}kM|d##nL&tWVuGo`3H4e$7Csvvf;B*} zqvNrNuR!~kDY3gR)-=3DcGRc zWPMCJr+bDkl(a*7nMoUQ5Fqgjn(f(!A9#{8{**ctG=#*urlE$5 z4ry`A2iTw`d1XGp9H!F%v&a&e@X*c;=gS~Rh~og~D#If#&F<8Isgz;wov*U@OkOtH z*-&RO1^ziTi^(CGrsh?OevevaaS*?i%Om;cS+|Z`3E!kl6*~up^j56fV2f?C#Wvew zj}?eJ)qg@=S{(q5^n#1)gt-epXn>4`DFWFi79*SjiggLByaokPJfg32VDbm!`F)d_ zIc_vOqiX&qwe1*$f)}M*wNkJ)GizlwK>8erC~vvlm0NF>Nplw83P^zswL{}(0y5U>! z(yMzp8mA0AMoy0Opty~+%dq*?GiPlvk&FQHT}h9^@RoA2i1jW{Gj1b>iY&2$tOZfj zW|8>Tlf-vJj`rEtkJFBN+b=8ElWbQ{v&6<_Dk$#oz?fV=f+m}}jTp;qs~EX%$n|sz z@bly+f8V9&uPC_?Tp;fo=^h=vzW@4TnWnv&75k6?m<{clkoDgfQbO=uY^ESQF7Kmu z>SV3Erjm_p6x_THEI7E1BUc{7_3F$9c!=3nNK z$|7Qb5MhHZUNE=Sd>1T@AY_Z4Jw)4Ti&QOi zF|{G7+06FL;&-gj%Ffw{MYEK1i)_TA5FhxM_y8LZ8G~A8Zbq`SVxIN+Jd9S983dj2 zmuu|@Hl7v?t<%{usq@x1NL@1Cfia{o4VqDJ5kqRew_|UatufkZJdrV^W@*P>n_L>C zc9rFsj-^bw`^WR^mu$bV$WTH$L;4^_#sw%zC)>PJ4|2t5m_kI$UA3G&qtkLA>r}$) zSM>vE7(w?k6(Fik{XYaY68OIWa9zTL`YR}bXcE;8?n3ntc#@(=2{_Op1@wkmh7vY& z5Jp-MEy0aIzD%L75Mbm6Q5sI>ARw`E7GxCXGZX(|pn1?ToIHZln+-+rdhtT^rrJWZ zP#Ad>9<^Fk%aQizqUFQPI(Bxq_doG3M5tt89I3*hNDM5BrkyN)aCp3pi@u}jIqI7AQo=wz*ZCV_{ zwAh^4us^fa`L+52QnMmB^48!gz3Y*y|a z*9C_piX_q9gTrX5m~B_f9MKZ9+apn!MzjnUU2){ZMNMKRlSnoa?Srp#u~E&AqjnXW zj?=E9oaQ7^*_e}cb0_LnX6s;=d|=c&`r_-}Ow+zh{eDCSBD?X``!+I=Zrt@qxZUoW!D|J_pQopx9J__iSC4#dd~9N>o%u6J`<;dv;{+ z^}U&sr?aciWR^XXk)LHq?3rBXS%k+!&rZm;UsisWkeHrkiH)<8{7w$xWzU%5NwpyJ z6m#WA)0!(V;V;5^O+sE^)P4a7J@-dU=y`NP??EFkZ>ze5W=R1(x0HrsAsW#YA7YEL z!Q`+5B8#As;);)%R8vFEd7%)UtR7J_@@gQF5j9u08o{v;!ST@Q3EB3`%Bu;7=xLVN zI4jQY<`OazQZnp2N{gG-a~i|uZ1e;!hdlQdrH^c{Q!lw zh^qIDPw|{7BT4&4uTr)^dd_P|vsJ(1Um&TnA_E4L0ANrF!Y0uNo5T=X?Rw*flB6n6 zrW3RAM#H<>;sbryU!Up@lVz{g8yz4mH5B9U3w@R>aC^uamwUL>)#*Jj5YQ5^#idoZ ziS2w)%hA-NMw7LA={hCEJh#}MbQBtbaw#l< zV4T%@uNpe^?nF-`nPD@}P#j^^<2CuYHD99;iK|o^4GkW34TZ^MOkGQWK`Lb9% z9q&p*ZQU2;j-`fY#p350$($h87{hl-vFh^`bYQBnlXkem`uO4Hj(Yoo1_GzDr<0PP z!96@r|EiOoG(3AiO~oc9t0e1Tw%j=5f*OzdS}2JWKpKhgj1#-7u+Nr{F0zk~=N~B; zb4~X9#D41ALEe>N=*&ElJYx9NghF(A<9 zQ6ZTMRluj^B5C$v*QLF0Rn*+9o_Di)*4qo0jBLEpbaU~V(eBLVGnvL`GK-!?$(q`q z`TdnucRZAzqh6~_Q%F9qU@E}IS8B6P5%e5g<{qxFWN!N~Q~3ce?e041gjaK~LziO^ zLxi8&+veppSYg6_6p}p}u1Oz!CSt0sbEo3%Wz28lc_BspL#T0t+_Z>$Ay2y>N;vPS z6Q&Xlk{uyEbtV&y#b`Sk#k^LUPjOb<2tLhd8?(bQOF7FaMJdQxeU>+I&Mg#YC z-K@Lb9PPst)1p=;iWJ^aWL;$!F#S_no=hmA;DS8W$x;x$2nNn6!3D4qtrK28lYY~} z%T#8ie#_{U*l(Z>bhP$N`k&<9W=}fy|7oFWH27rWiPPEWij<^=%7&;8Q*EWc+8HGQftg>KSzG>M9T2`i>H7cF@Q0*Op*+aC{KZ}^ob zzW&5W>dm@Me?0tijn=@=FI)_7)fU4qs1vw00s&nc0V^}+Hj%8$$ZaAdDZtG;QJNcd z?M4Wvhi<_0IQCvaw@lrosdG3+H<-HNw<$#Zt11LOg|gU>)K1|R0xuD`L|`jbyP3c% z6g@%U%LJGzlPM^E6H$#M(Omw+A(sMi26u*?ncw>$KFL9}S^HN2*t02DOvnqf^1>?z z$K(|jAJ%`n>4{wJ&fk4{G%!)OY^-kCzkm9YKU2H&?TT$TE4JM7c^1NPv?s9F1-LMH zTk??SXo}xu@clAr!(LZ*{hoLIlz8bpzj?>Fxnr;E_fEXe?!9NEtwn(~dv4hZ{LHwy z9R(Wf_e5oud#pA5S)yLv5MCApNX1?ke=yG`p0Ey&Z`osQ(`{L*Q(XUfI9J<-Gg7~* zVWj3l6dVs>!<6lZ!&S-CqMl}n;iy#<<9BihPb!JTpWi8$>UO!9!2Z?Q2mD^!o+3eS zd;_!MgV7tN-zFR>;3A&Hzz?7*+^P0q3of)mn0^>Ntef?mPy(OyGj-QP@8{qr{rq9{GZ^UdR%X4O53$X?g98!W10XX( zQb|~p_hVPIhI6{qT0)mEqhZ{Pye^pv!^oYsB5;Slu(cmvoJhiso~-nVpyDJ}z1U(6 z{|Y8PAE%m3d_JWulFb6?B`V0U_%3SE_bAG+xIo}e7D!tK0i?SRgdgG|Epi^h708#d zYas$*3oNiMKRZEMbuW ziP!2yENhF2WMrDm+mrNz3Ap|d-8)lU%S2!vh^HX_qSGgvnKOujm<*BECoY81&C}v3 z1y+7YE`$vy4%NfQj$^68RONzFcySm=u{|_m1%eZ z-s%fgnE6`Jn$#v*S4hgxs@Bu$LgWaO9;&M;)J(U*%RBl^Fjq)pwF5t=S7EMpPTY&h zxO@pE9Q?y@F9`YyOSVyPqen~#TV|OCnyqHWhA~>Yxc=D`Gjsj*OsXKstoD60=%Vzt zo#Bux?GnPbS8sUT_xic(FJ^W;m3`#c%~k8J$6r5jJ(Bs<(d?pDNaskTS4JYeAmo5^ zqz})LF;fzN6C9*3agM-w0v7=C{zxR2>WW126+nUML|1CEhQXTJx*VHH-UWw zoRki0=>`E3;%l2x^p~Z-@*a8HS98nV=&Kr;bxXpZ(dBpP&s~pHR?E`wJnIVhaGu|s zGT*lMszbgo*FaE3U8qA`h_mbNdi*KWKtQYDRXl(`#S2)b_yGN+iGb$8uLgSLhCtp= z7;H~pBGFHE)4LfLF$Qeeyunx$iyR(?pQiYe`^qs0;v!yhye5-kkp7mOrr?X7S#Xi- zgVUt7V3#RuZFZ;jSQ!Is(k*ikpZ*kVLJ4L>@iXzMY!jc(Y-a+A@64w-G7lzmVe}QP z9dt4Aomm=;P2mNjSJ6jCX)Lt?%QGEIwF?u|k~zhqTr#I%^GfD4g}Fg}tE|j+RG8vf zXn6Q#sY~pTFzzvP*COIs+jjqRIC$tc9QRfHfoW!(`mv)cfaluF7@Pdp##f2`#P&@c zMY3>vTM5B?=+yTtY;uV&X*!(1@jQaro#yTrvu?*RR%ENRS;~2o2$N-uscOrYaLz46 zwIy>3_Yfs>nj@v=p^X%{I!{*Oe>sHue+mYt*3ljAy{g7W!>FV&mQWcJ#5^EjoZF`jX|T}#HZ}1J1inb% zO#*uXaLNp$VOCH=GXeHmF5nV%oT5(>I7(njx+Whqn~?Y5=~t^P!r4};?gUlB#IL6) z^l1Xj8t*iP&JbYy-m4UvAV8d-M)Q^nXbs2(wF?;2hr=6SI!-FkNc;kL)5vgKP34#B zIFXY?j(L)3lGY1QbbpC)9wB!iiXDqXuMCk}-UX|FXU{eHw~uD#Z$tFb5dP*u+Y!9} z83raf?cFXM@$$g9yps%&bVt2eGlm5eE5asFd=O ztN+J^N~sXIhup9~JSrvhAyX;Uj7q7wPb#H$Mq_NDjcR&WR7!2hG4*h%l)8sDQVOY* zS&meS#W~}XgdDK~>LQ1u6m_C4mT+e$B*hI!!*CWE?u>WCUo1J0WFlV8*k1Z@Nef0~ zs0^h)g|9@vH3uDGxiA_&1^o9+l-w&NI9C_e3Yde&4!LnAcg&I zAP?<`Rf zx9}M`rj#T&8DjkuE1TfLADST+;b`+Ij$9`Zy!Eh)Ds3B6E$`u+)08BXs?+2T%f!k{N?a31PhW}aBuSuSQml>xVe|zY?ZE%0?2-gw zRAu%+%2|?NzM+GR3kvsghNCNNINB`Z+e~uy{06gR8DiGR6VqjV#m>ehR|@U)dL#Wd1_&8u&WCSQdow;=^C(XcWMxB!mw$L zxio`vl>0b?$ruC<8nfn3;&w9Lq)rp5w+`2~ri4E?n5NMaxPtZs@RJ40R^BIL&d{oQ zk}8`LtqOll@&1AIkhv`f5)`(f$Iu@0<6dDyrfGbaHbl>y&898f(=JPg&S-lX3jWD2 z09sv^k#|hgKbEb341rAjW4U_p{z%!8t;e2nr0mFsuq9mo-9Soc8IO)x!Xg6_N5ER^ z*|TPBoYl_n%eDdel~%s7N$5 zYH#Y(nfa$Pa+I2SIu}B#qnVBm^!Zl1C-$6llHZfnaluZnD(n!2g2m5SxaK?+-KS3UJtP zO=TeLf#W?D4V6)HVR0NbTKakuPDinK1}=x;%DJfrN7)Q^HXF)Vjhs}+a4HVg9mevo z?XC$Gi&IJ+f~V^?h&0&|J!}^aEn_e*r2}8M^%v^2-k`Isjgww*7?0S_L;GBU(XiGJ z2i9!gtg++PXd=0KrY6KvJ>994?x=bjOQL7;haod|328y5&jm+^mJB2Y;y5)M-num$ z0@Oz*ZG$%?dsu1;oz&A5H2J<^zyAK}W9as2V3k(vh+$%1p?l2-BL{XJ-xoQy>-fRE zAAWPOmsp#wLRLMF$2u6AE!FV^5Cgrf7D3|$x2JKwZgl~gm?G~N2}}F{kHQH$RZ{gV zE+Fk;3+sOjPPFXKw(Nd=UAAT48-Z-gFJ+ELvMrI^+K$Y8EUZ1W(nfP3po5-FcMpfz zP|t*{|85|iaD0aF6#&LakE^{rJmm}Umm}J z;yjPK1rN?wwC&rqH?nu%{$1@)9%ohvc`uzbT1acHqE>Dr@L2+XO5kS%$jXlOJo}jZ zv(iGGmp{dyvqL?HEci(peVkh!m&HA-=y-tt*p7c z>MPGqR4&U_F3VM}n5b;bRyK}Cvz0B^wq`3om3iV+w(``);7z&aYgMmQai`3g=6$aZ zV-L|q-%Wq@mtJ`3g$e)4tbgUmV9wuo(R)+!eW~K5iV106R+@KZ)|m9j+lHsQkWdY(%3RntfBE<=H0QKyq>ld`-OK-8JN_<9za{%!)o^@rN5bDLdq*~Y>&Pt%zA-Cv z?D!8CvT&v&_A^SmHEa5LZ_jeOp18dHmV`eeb??!i_k(q==Gzj$``8fXYPl^De81Iw s%;h@bx-GFk?{~Xbxz^m40Ny|0YInK9w Date: Sun, 26 Jul 2026 15:26:49 -0700 Subject: [PATCH 3/8] docs: clarify secret storage guidelines in README (ENG-1053) Update README to provide clear guidelines on choosing secret storage locations based on the type of secret, emphasizing security considerations and correcting misconceptions about Kubernetes as the default. Removes redundant compiled Python file. --- .gitignore | 228 +----------------- README.md | 114 +++------ .../workflow_permissions.cpython-313.pyc | Bin 16179 -> 0 bytes 3 files changed, 32 insertions(+), 310 deletions(-) delete mode 100644 scripts/__pycache__/workflow_permissions.cpython-313.pyc diff --git a/.gitignore b/.gitignore index 78ebe0a..7a60b85 100644 --- a/.gitignore +++ b/.gitignore @@ -1,228 +1,2 @@ -# Byte-compiled / optimized / DLL files __pycache__/ -*.py[codz] -*$py.class - -# C extensions -*.so - -# Distribution / packaging -.Python -build/ -develop-eggs/ -dist/ -downloads/ -eggs/ -.eggs/ -lib/ -lib64/ -parts/ -sdist/ -var/ -wheels/ -share/python-wheels/ -*.egg-info/ -.installed.cfg -*.egg -MANIFEST - -# PyInstaller -# Usually these files are written by a python script from a template -# before PyInstaller builds the exe, so as to inject date/other infos into it. -*.manifest -*.spec - -# Installer logs -pip-log.txt -pip-delete-this-directory.txt - -# Unit test / coverage reports -htmlcov/ -.tox/ -.nox/ -.coverage -.coverage.* -.cache -nosetests.xml -coverage.xml -*.cover -*.py.cover -.hypothesis/ -.pytest_cache/ -cover/ - -# Translations -*.mo -*.pot - -# Django stuff: -*.log -local_settings.py -db.sqlite3 -db.sqlite3-journal -staticfiles/ - -# Flask stuff: -instance/ -.webassets-cache - -# Scrapy stuff: -.scrapy - -# Sphinx documentation -docs/_build/ - -# PyBuilder -.pybuilder/ -target/ - -# Jupyter Notebook -.ipynb_checkpoints - -# IPython -profile_default/ -ipython_config.py - -# pyenv -# For a library or package, you might want to ignore these files since the code is -# intended to run in multiple environments; otherwise, check them in: -# .python-version - -# pipenv -# According to pypa/pipenv#598, it is recommended to include Pipfile.lock in version control. -# However, in case of collaboration, if having platform-specific dependencies or dependencies -# having no cross-platform support, pipenv may install dependencies that don't work, or not -# install all needed dependencies. -#Pipfile.lock - -# UV -# Similar to Pipfile.lock, it is generally recommended to include uv.lock in version control. -# This is especially recommended for binary packages to ensure reproducibility, and is more -# commonly ignored for libraries. -#uv.lock - -# poetry -# Similar to Pipfile.lock, it is generally recommended to include poetry.lock in version control. -# This is especially recommended for binary packages to ensure reproducibility, and is more -# commonly ignored for libraries. -# https://python-poetry.org/docs/basic-usage/#commit-your-poetrylock-file-to-version-control -#poetry.lock -#poetry.toml - -# pdm -# Similar to Pipfile.lock, it is generally recommended to include pdm.lock in version control. -# pdm recommends including project-wide configuration in pdm.toml, but excluding .pdm-python. -# https://pdm-project.org/en/latest/usage/project/#working-with-version-control -#pdm.lock -#pdm.toml -.pdm-python -.pdm-build/ - -# pixi -# Similar to Pipfile.lock, it is generally recommended to include pixi.lock in version control. -#pixi.lock -# Pixi creates a virtual environment in the .pixi directory, just like venv module creates one -# in the .venv directory. It is recommended not to include this directory in version control. -.pixi - -# PEP 582; used by e.g. github.com/David-OConnor/pyflow and github.com/pdm-project/pdm -__pypackages__/ - -# Celery stuff -celerybeat-schedule -celerybeat.pid - -# SageMath parsed files -*.sage.py - -# Environments -.env -.env.integration* -.envrc -.venv -env/ -venv/ -ENV/ -env.bak/ -venv.bak/ - -# Spyder project settings -.spyderproject -.spyproject - -# Rope project settings -.ropeproject - -# mkdocs documentation -/site - -# mypy -.mypy_cache/ -.dmypy.json -dmypy.json - -# Pyre type checker -.pyre/ - -# pytype static type analyzer -.pytype/ - -# Cython debug symbols -cython_debug/ - -# PyCharm -# JetBrains specific template is maintained in a separate JetBrains.gitignore that can -# be found at https://github.com/github/gitignore/blob/main/Global/JetBrains.gitignore -# and can be added to the global gitignore or merged into this file. For a more nuclear -# option (not recommended) you can uncomment the following to ignore the entire idea folder. -#.idea/ - -# Abstra -# Abstra is an AI-powered process automation framework. -# Ignore directories containing user credentials, local state, and settings. -# Learn more at https://abstra.io/docs -.abstra/ - -# Visual Studio Code -# Visual Studio Code specific template is maintained in a separate VisualStudioCode.gitignore -# that can be found at https://github.com/github/gitignore/blob/main/Global/VisualStudioCode.gitignore -# and can be added to the global gitignore or merged into this file. However, if you prefer, -# you could uncomment the following to ignore the entire vscode folder -# .vscode/ - -# Ruff stuff: -.ruff_cache/ - -# PyPI configuration file -.pypirc - -# Cursor -# Cursor is an AI-powered code editor. `.cursorignore` specifies files/directories to -# exclude from AI features like autocomplete and code analysis. Recommended for sensitive data -# refer to https://docs.cursor.com/context/ignore-files -.cursorignore -.cursorindexingignore - -# Marimo -marimo/_static/ -marimo/_lsp/ -__marimo__/ - -# Mac -.DS_Store - -# Deployment -deployment/**/charts -deployment/**/requirements.lock - -# JetBrains -.idea/ - -# sed -i.bak backups (e.g. from `make prefect/...set-image-tag` / set-env) -*.bak - -# UV -uv.lock - -# VSCode -.vscode/ \ No newline at end of file +*.pyc diff --git a/README.md b/README.md index 6b16b12..e01dcfa 100644 --- a/README.md +++ b/README.md @@ -90,23 +90,11 @@ posts. ### The one failure it cannot see: a run that never started -`notify-main-failure` is a job *inside* the pipeline, so it covers every failure -where the run exists. It cannot cover `conclusion: startup_failure` — GitHub -rejecting the run at load time, with **zero jobs** — because there is no job to -put the alert in. The run appears in the Actions tab and nowhere else, and the -branch is not deployed. - -That is not a corner case. It is produced by a caller job granting narrower -`permissions:` than a called workflow's jobs declare (the cap is checked when the -file *loads*, not when a job runs), by a `uses: ./.github/workflows/x.yml` path -that does not exist on the ref, and by malformed YAML. It has already cost auth -`staging` ten hours of running the previous image across two merges, with the -recovery message being the first thing the channel heard about it. - -`notify-startup-failure.yml` watches from outside, on a schedule. It sweeps the -deploy branches and alerts when the newest conclusive run of a workflow is a -`startup_failure` inside the lookback window, restricted to runs where no job ran -— precisely the set the in-run notify job could not have reported: +`notify-main-failure` is a job *inside* the pipeline, so it covers every failure where the run exists. It cannot cover `conclusion: startup_failure` — GitHub rejecting the run at load time, with **zero jobs** — because there is no job to put the alert in. The run appears in the Actions tab and nowhere else, and the branch is not deployed. + +That is not a corner case. It is produced by a caller job granting narrower `permissions:` than a called workflow's jobs declare (the cap is checked when the file *loads*, not when a job runs), by a `uses: ./.github/workflows/x.yml` path that does not exist on the ref, and by malformed YAML. It has already cost auth `staging` ten hours of running the previous image across two merges, with the recovery message being the first thing the channel heard about it. + +`notify-startup-failure.yml` watches from outside, on a schedule. It sweeps the deploy branches and alerts when the newest conclusive run of a workflow is a `startup_failure` inside the lookback window, restricted to runs where no job ran — precisely the set the in-run notify job could not have reported: ```yaml name: Pipeline watchdog @@ -127,24 +115,13 @@ jobs: secrets: inherit ``` -Alerts repeat, by design: one failing push produces about three messages at that -cadence and window, and then silence. Alerting only on the *transition* into a -broken state gives exactly one message per break, which was rejected after -replaying it against the auth incident — it would have said nothing about the -second failing push, since the run before that one had also failed to start. A -stateless sweep cannot be exactly-once, so the choice is a message that can be -missed or a few that cannot, and the window is what bounds the few. +Alerts repeat, by design: one failing push produces about three messages at that cadence and window, and then silence. Alerting only on the *transition* into a broken state gives exactly one message per break, which was rejected after replaying it against the auth incident — it would have said nothing about the second failing push, since the run before that one had also failed to start. A stateless sweep cannot be exactly-once, so the choice is a message that can be missed or a few that cannot, and the window is what bounds the few. -Two things to know when adding it. GitHub only runs `schedule` triggers from a -repository's **default branch**, so a watchdog merged to `staging` and no further -is inert — `workflow_dispatch` is there to prove it before it reaches `main`. -And widening `lookback-minutes` on a manual dispatch replays a past incident, -which is how to check it would have caught one. +Two things to know when adding it. GitHub only runs `schedule` triggers from a repository's **default branch**, so a watchdog merged to `staging` and no further is inert — `workflow_dispatch` is there to prove it before it reaches `main`. And widening `lookback-minutes` on a manual dispatch replays a past incident, which is how to check it would have caught one. ## Workflow lint -`workflow-lint.yml` lints the CALLING repo's workflows. Three layers, only one of -which is ours: +`workflow-lint.yml` lints the CALLING repo's workflows. Three layers, only one of which is ours: | Layer | Blocking | What it is | | --- | --- | --- | @@ -162,42 +139,19 @@ which is ours: secrets: inherit ``` -`zizmor` is advisory because pointing it at existing pipelines surfaces a backlog, -and a lint job that is red on arrival gets ignored rather than fixed. Flip -`zizmor-blocking: true` per repo once its backlog is triaged. - -**The permission check is not redundant with either tool**, which was verified -against a tree that had already broken a deploy branch: `actionlint` reported -nothing, and `zizmor --persona=auditor` reported only its generic -"no `permissions:` block" note, equally true of jobs that work fine. What it checks -is that no called workflow declares a permission its caller lacks. GitHub caps a -called workflow at the calling job's grant and enforces that cap when the workflow -**file is loaded**, so a job in a shared reusable naming a scope one caller does not -grant does not run with less — it rejects that caller's whole run as a -`startup_failure` with **zero jobs**, which means the pipeline's own notify job -cannot report it either. In `mindsdb/auth` that cost two silently undeployed merges -to `staging` and ten hours of serving the previous image. - -The rule it enforces: **a job in a shared reusable declares only what all its -callers grant, and inherits anything only one of them needs.** Inheriting is the -only thing that composes — the PR caller grants `pull-requests: write` and the -comment posts, the push callers grant `contents: read` and the same job runs -without a scope it never needed. And the reason it has to be a gate rather than a -convention: a pull request only exercises the PR caller, which is usually the one -that *does* grant the scope, so the mistake merges green and breaks on the merge -commit. +`zizmor` is advisory because pointing it at existing pipelines surfaces a backlog, and a lint job that is red on arrival gets ignored rather than fixed. Flip `zizmor-blocking: true` per repo once its backlog is triaged. + +**The permission check is not redundant with either tool**, which was verified against a tree that had already broken a deploy branch: `actionlint` reported nothing, and `zizmor --persona=auditor` reported only its generic "no `permissions:` block" note, equally true of jobs that work fine. What it checks is that no called workflow declares a permission its caller lacks. GitHub caps a called workflow at the calling job's grant and enforces that cap when the workflow **file is loaded**, so a job in a shared reusable naming a scope one caller does not grant does not run with less — it rejects that caller's whole run as a `startup_failure` with **zero jobs**, which means the pipeline's own notify job cannot report it either. In `mindsdb/auth` that cost two silently undeployed merges to `staging` and ten hours of serving the previous image. + +The rule it enforces: **a job in a shared reusable declares only what all its callers grant, and inherits anything only one of them needs.** Inheriting is the only thing that composes — the PR caller grants `pull-requests: write` and the comment posts, the push callers grant `contents: read` and the same job runs without a scope it never needed. And the reason it has to be a gate rather than a convention: a pull request only exercises the PR caller, which is usually the one that *does* grant the scope, so the mistake merges green and breaks on the merge commit. It also refuses to read cluster secrets by hand — see `k8s-secret` below. -Blind spot to know about: it can only read local (`./.github/workflows/...`) -callees, and lists remote ones as unchecked. The cap applies to those too, so a -scope added to a reusable *here* must be granted by every consumer's calling job. +Blind spot to know about: it can only read local (`./.github/workflows/...`) callees, and lists remote ones as unchecked. The cap applies to those too, so a scope added to a reusable *here* must be granted by every consumer's calling job. ## Reading a Kubernetes secret -`k8s-secret` fetches one key from a Secret, fails when it is absent, masks it, and -only then exports it — in that order, which is the part a hand-rolled fetch gets -wrong, because `::add-mask::` only scrubs output that comes *after* it. +`k8s-secret` fetches one key from a Secret, fails when it is absent, masks it, and only then exports it — in that order, which is the part a hand-rolled fetch gets wrong, because `::add-mask::` only scrubs output that comes *after* it. ```yaml - uses: mindsdb/github-actions/k8s-secret@main @@ -208,28 +162,25 @@ wrong, because `::add-mask::` only scrubs output that comes *after* it. env-var: PR_KEYCLOAK_CLIENT_SECRET ``` -When to use it at all, versus a GitHub Environment secret: read the cluster for -values whose source of truth already lives in a namespace (AWS Secrets Manager → -k8s), because a second copy per GitHub Environment has to be rotated in lockstep -and a reference to a secret that no longer exists resolves to the **empty string** -rather than failing. Keep real per-environment credentials (dev/staging/prod) in -their GitHub Environment, behind that environment's protection rules, rather than -making them fetchable by any job that can reach the namespace. Throwaway creds from -the cluster, real creds from a gated environment. +### Which source a secret should come from + +There is no blanket answer, and "prefer Kubernetes because it is the source of truth" is wrong for the credentials that matter most. Pick by what the value is: + +| The value is | Source | Why | +| --- | --- | --- | +| Ephemeral and namespace-local (a per-PR environment's own credentials) | **Kubernetes**, via `k8s-secret` | No GitHub Environment can exist for `pr-auth-204`, so a copy is impossible; the namespace is the only source there is | +| A permanent environment's real credential (prod/staging DB, Stripe live, prod vendor tokens) | **GitHub Environment** | A job can only read it by declaring `environment: prod`, and that environment requires a reviewer. A k8s Secret has no such gate — *any* job on a runner with cluster read can fetch it, unreviewed | +| Needed to reach the cluster, or used on a GitHub-hosted runner | **GitHub secret** | Package-install tokens, the ArgoCD token that creates the namespace, Snyk, Slack. Reading these from the cluster is circular | + +The middle row is the one worth being explicit about, because the intuition points the wrong way. Moving prod credentials out of a GitHub Environment and into a cluster read would *remove* the required-reviewer gate standing in front of them today, and it would force the jobs that use them onto `mdb-prod` (nothing else can reach newprod), which means granting *more* jobs prod cluster access. Both are the wrong direction. + +What actually goes wrong with a GitHub secret is different and has a cheaper fix: a reference to a secret that does not exist resolves to the **empty string** rather than failing, so a rename reaches the vendor as no credential and comes back as an unexplained 401. The fix for that is to assert the value is non-empty and name it in the error, not to migrate its storage. -It is not a containment boundary: the ability to read Secrets belongs to the -self-hosted runner, which holds cluster credentials because it deploys. What bounds -*that* is who can trigger a run on such a runner (hence: a `pull_request`-triggered -job on a public repo needs a fork guard) and the runner ServiceAccount's RBAC. +`k8s-secret` is also not a containment boundary. The ability to read Secrets belongs to the self-hosted runner, which holds cluster credentials because it deploys. What bounds *that* is who can trigger a run on such a runner — hence a `pull_request`-triggered job on a public repo needs `if: github.event.pull_request.head.repo.full_name == github.repository` — and the runner ServiceAccount's RBAC. ## PR environment comments -`pr-env-comment.yml` posts and keeps updating one comment saying where a PR's -environment is and how to sign in. The account, Secret name, namespace, and hosts -arrive as **inputs** — this repo is public, and while none of those is a credential, -together they are a free recon package. The mechanism is shared; the facts stay in -the private caller. A reusable that cannot be described without naming our -infrastructure has not earned promotion here. +`pr-env-comment.yml` posts and keeps updating one comment saying where a PR's environment is and how to sign in. The account, Secret name, namespace, and hosts arrive as **inputs** — this repo is public, and while none of those is a credential, together they are a free recon package. The mechanism is shared; the facts stay in the private caller. A reusable that cannot be described without naming our infrastructure has not earned promotion here. ```yaml pr-env-comment: @@ -246,10 +197,7 @@ infrastructure has not earned promotion here. secrets: inherit ``` -Layout is fixed and deliberate: who to sign in as and the one command that prints -the password come first, links next, everything else in a collapsed `
`. -It never posts the password itself — a PR comment is permanent, org-wide, and -un-redactable. +Layout is fixed and deliberate: who to sign in as and the one command that prints the password come first, links next, everything else in a collapsed `
`. It never posts the password itself — a PR comment is permanent, org-wide, and un-redactable. ## CalVer releases diff --git a/scripts/__pycache__/workflow_permissions.cpython-313.pyc b/scripts/__pycache__/workflow_permissions.cpython-313.pyc deleted file mode 100644 index bd2864d54751eb13078900a0e56250bbc21d505c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 16179 zcmbt*Yj7Lam1Z~IXaEG?A}NV#J|qzmU$R~nCE3=4vM5uOED(x3lnj7ClY|HYbT=T0 zu$?fI+8Jub8PTy_rq$|Dsr4k4-O1K&)q;j2!U!|vo#c=IQI<9? z``3Qw_5&aVC)wGS*tmUPeeS*IeCKg*KlFG64%hURvCH4x$8o=<2mP|C3I5^V@ErFR zx0UmAGUw<0rsw!mCfOuX+WeeZHnU%gY{9SPIqNB#Y}3l@vK@I=zs+y&Hp>pbUN93a#8O&M!>zd(k&tvdiy6shdju9{j85HeGbpaOXM2)Qi6w?xdCDlAPZgG@Z6{ zoQo5Yo8bJF-6q-XuR_~u*0x6WG;pn)zqX0<*X=d2&N;b)<<%o^gOTU;xA_~IxXTu~ z(%2{F_;-&hPU#vsAr*hkpx8ZD{8H@IU`f zzsPDf_&fbw{uWAK=H0ty!zrmx42zL)JT67Wt4VosES|h7PK9U0gmguc#i$gChh+(MrX_hQrYNywLX4#p zwTUdo!;wj)TM(X&rB0?tkvpwC-rsgbOeH6!gcweVsR@a-jU~p#%gIp`jmzOgs#6Rn zqUczROi1WaNKK$FdK0fspdB?bh&wSb*e?$F#lbU2j`tt$6cfpmS}RN)igG%kbc_BZ z$e_m<-YUpaS_zNFOFCiwC&E(_k*h?K(~_7>;BTyCaEcgBqMZ6?G#Zb>OX--DiqFuy z#H+E?gcz3;rH{y$rOQ$T)1%RkX_YZVDUpg{5~+F^FBC!{B^8!a>FMBDI2KRKQYh3Z zj!I)mn(4$$YJz4brhu>sDVoO8V1|cjMzreUR5B`2g>)kJQd$Bfqv2?`IE-nccTC_V z8A(pk1Sp-Nl4KJ&9h;Wov4jMAl2;Q#iq4{BBH;v$b_~R-3xX#`3y8u( zfI1~SLxYpV>9jH-Mq^QGE<~m2cygv&9Kf0-6#FA-6j7^2dHL@rlM5=Gn>PZRoCP^~2 zi%ei3Qe2#n!g5pyk0#S8TAZtx7izjgmLf@T_sk9uq$ucC9Fvn%tc>Q5E#6q{8VHW1 zF!e)8QMwjWQjDZzG7iQ}VP!xP`1a~VEHXiBK|LoE#1&Flft0XG7>j}HSYt5dc~CbN zlR(jUIF?}47DJ(MBn25mRthsib0~wI^mXXKC=8~QB_$cZBGHQlCc$zK z`@!NAWmT5qVd_wsh)t9HLDGHd+J-`zSY|3Ii(Or)4~oNw!|7-&m6SsvNR0YoHXxGs zaVY`nBOp+^7Qv*&u1M5Og9HV%f<|yJn`>3{m}Df9;P#S=eW*DaPevw7W=?u34YFp$ zl$>TFNNlSv0tr1sD`NRkSPFTXx z8FdwhX?<9&iEw<3ME&Xn6^Y{{bO6!9BC#o8U?^Raf_ley--QxhO^k|QcrAp6^ z?wP4LmV_9mPY^_0YNO(k={g`v(4?XOK$WOhxea)N^#qV8=~3Y1^n^H-#_0a!?5AN6P2*}S4fZjfDd;>Y3OOA}fR|!S+#%QC3qW=} zsSq@QI0-BOf$AjIDHVIQPqxZfoarQ+bDE|iS1>iAkU)UJx-12+Nnt4|i{UMsa6lyM2i-)H3E*Z5J%B@^B^@ZxY;4*fFbY#q1cB6#s_Q_ys^4kE zOud|wrbvF(B~`=@ooSNtcn^U=8Q!@@yz`7KjfFx!G-Rq{6wC-x1cs+fV-N$A1)(Ae zRu3SYPB5vZr7V%#u%e`=kjLa*_>2q~A75~>j#UgB!n5TpntVo9-)Cf;`OF1N0&7^X z(0dD3HkFx*Xe^Q%QBsgi0O_TIZLt4*|DgXVFIp6=(?EQsU}H4k7gh}fy})QFgP51_S})7VSlE!95h7GlW}QwF#qr@ zvfkn@axxEV-sI)v%68%PoC}TWLVfSSRidLksz&J2c)mx8$g$~^VrYn9 zQ4c8H(=!D}Fc?e3Qo&%M+E}q}J?{k~q7%1SZnc5iu=zDtzN!5+Z@zw$o~mnlO~^Nh zuX*wtnqPD0Ya71ne2Ds-v1;_tfIqfszrYH;#Ra%?l)LVtQPUOP#8C}n{nT8#SHQn4 z-xejN-Ig;IU)#>S+mwJWdd^t!Vy|dptbH+8_p%ZuloRKzyEIyuue`PhC04X?ug#?= zl-N*GQMSY$;8E`QgDnXg)?H}h447~izLrkczo1Vy+LRxgXLmz8=c^d{%Er`DsKAFZ zRbq#3fWUFOBE@|fi`ey)m~nOl3Z0P$t){@awU)3ESEp@%!g+_JTt%N?Sro$obOt=vKW0o`Cc-+b>H3aQstA` z;S0;d!KL9~)*s4-Udlc%&sp=8^>4dgcP$)$ci`qgu5#O)^;2)njhC;#JU6wlZ?XOT zp7(mPEl=F_-8=r_sSi%wJM<6oFWdiD=YQ|~W%DY>U*K!|)a0tm>Y3tK&330CcVloqM+u%5F?KDeGN2vuRkN8cm@z#n z67NR`YIBw%OwI@>6<-mDV2-Hx%!*ilEX7ubr3kLS@*)5bmsBgIySp2@U6xX5IZ?1s z4|h!R0eT13hfV?TQn&)kB?N=8RABKV?G6UXchX9{!65UB$;(e9QH&*yCL_V1OrR`N zNT^Ups}9p75`>i^#~@&m5)9tq*o-j7kT+9hx{)SUVz~VU_lf1`s-?!-xN767YFF)) za&UDGt4>M@l;@(9o2%Qh>Y-ExS6Q>_rBo$XQ@>h8scNpeZncI|wVbzkwT@EtoKW#- z15#h?5UlFDuy;rBTI15toqtHL3XtYMK6`^?r-{Q>>10P8L!5 zj=3n%1q+#(J`0oMVzxYhdWr=%2(l{4Y;_CuYsP8l8f z5k8E>*C}2o4^pe;6)veiqHW!UHs;+nC<4on+>5e|L-jZT=q+N0x)f?4kBoR0sWTi1 zpL`N6d_0>6g={u$)G9l#A2q|T@1T}KGKN7{*T22<^_}yZ7seKc-w(bQ%(m~nd*NR6 z!`KJ0dn4JCv8?xU)^S;7e|pmpV1a=F6Vx{3FcD<)uBMn+_EEt>x*ek1F}e|`s3O!) zDY_B6DWr%=g#PT2#SBIlcI6T61y!#A zKVmG6UQQfJR~X9~;M=)1wGBJxvD)Y-fY^QS`W6ngH;#L(1>hvWBN}fqqVaIW5~dNm z-XdVo?l&Vs4~}(lB1NlB?OZR{j+FX@*tI3R5rZFwGaE`K`qqV$UN~}kP~6qMOPmhN zkRS3kNj;AuY=>xqI4My)WEg>_PzaGdbO6;$F$UI7tHna5q0mftD&DPx$E0998IE?e z;$_SYZ}o*j;!I46M-}w-GIiD`Y$peO`>W_xJO>`8@InYJTH(|qUIoJdfxraCjN#p) zofgp3gJ8^4R!x?v@T4TDF_@y2luS;F@z^BA;f>%2Io8S~qIxtw_(cc-F^{%eD43Fo z;US;3U>=uJ1p(m-1RKMN2%Kc{1Q{0>EOA6M;XOO_w-n5B_-etbI-|rjI=&PfP!!|i z@Fir@2!Dw`h4>m)eg)^MxzTyOb3V0j;oXZjFWzp?ZSKhly|c$w>{at^Z>E;*o0jaG z^4{t>LCsN??HiZu8}pSl^TTs5XB|yzDt^+vE9ZUU2a(0Vo96lcH-v@bA61HXB1_&U zvW_S6F3;@1m!F%t%H0a_=FfK4AGKM^i_eoNe~TM2gv8G`l`^-nEmHH?MhQ^Ll%6IA zqi7`Ceo&HAk`^(-Tx(IrFfAb-^ywo1*TAdwMQgw8g^SsjlYZQ zj7j)_6w(ZuJir*NUJ|t9H{F=CNItlZ9 zES!#~)J+>zWrnAj%2Y$6nleQgUyDVP?xgJ<=DAOxu9WB&)+{9P-){2U$%<_Q5|BE= z9-b3>cvXI`Z=B<f?zBrqigA9wd5rg&3Lbrm*t88XWJb2ByH^gNFNGABZUQ?G zQCR!hwER?|C8h+~A_dtZbsu6eP35U>qsk(+uEXs!aDKMpNMdt%RQ>(+}@>%lRvp|Z{p*<&*myl z=3FPUmXl0fteGLgTFesv@ZX|$njI<~=Zh>K;6`Yy?0O83VMPSs1+!hl4IEe048UmK zz-TL*ZNg*Vw1&sPe6+OuNiD7GTNd%2eA`3tM0XpwFRi8h>vaia8)gR9ef>oi0bMi( zXd1J&?~9pDVDNFJUN@i-w-^+aIS}(@p%CuBG%8YYmO% z2yesU48yjEY^@XgE8#eB2Dp$EUuN;3KGi9HmHCO}38)|x9#z1ffRvK}e(X0g*r)9^ zc#1OFqR=WGA+n#k2rfE-9~4z)4?ZKZ3}%nVBC!;9QQ}<`H>G`oDKspJ^Yn=?zZ$|O zk+HKyVvhJeDV%5wVS5>b5XeEtk(q|=KiVXwM@_A18%=#;2tA2pFv|cp;0E@NRo58c zqe54dKnr8nT6POV8HYXy7Uax0jd7@P%`NG~WP)0ikX)Pu1O%GX3F?8Fk)?Imwo{+z zv&j_1RBbtux`J??)}UXOld}9gWmqwBQwo%$m9=5cT*1m>6>EKHX9BYqj>oXu8b!mu z#h>zX+?Z-~c)tB=-djToZtJa@W&7qO`{ul>a{g4#)%=^Dy}6owKe)0eE$sW=={r}J zYW8K_`>6D}oNLQ(diUjO_WwuQVtQf!_lACKTdLWgb?;xPXvl8tySMiPU$*ghuA)Ed z=vVu$UA8wb*_-nRpZW0M2M7QD@WhCmd0VD19p+`0>PBCn$lsn#KxLkz{tz5vY_n5Hw**|?gL~g`Dv2R#Lj2NI^ zu3P6(%=+n*h&3M>F`|JL9Ox<=;oEK6j=`M{sR%|^eGAvg8GT`tV$7zlij$z5<7Hn%FMOeEcdXO0~i44ysL ze`4SQwtNr^fr0DR2@OVLa>1!TDJ(cvun{W8rGkz50#OC(jEq`c`&W=zA1t;>qxe7L zPq~WQEca$ZdGye&y~tOWsp;=UZ8*wXf6fa>z0Nf%pBRm?HJPVxaXtScwkC!i zg(L;|@@yS2?J*RgNQ$w}A%bG53Am$wVw{#M>mal>=n0|(GQKl!nv zHQ&_wZs*O;#Vfg{?&YREcUzYm4=yzxyx(~Ex@XR@;%!`LyOmn@wl8_xKXKQt)NWpg zygPn#d~qt*yeC(?cg~r2)#hAc-d%m;^!3xXb}w9e=kRS`wrS6O_udCCuIVYn7~R#g zrymD>LpuJ`5FjeaW#b8$Q&rg-~|NLaa2yBeHuAQm1c1B zSyi^TlEP7BEDI;UN;mj5+h-Jh zSN{52bCwn0eDv0_WtRx1RRfGaxwyRbXm0D#yu0eg;Pt^xgl zC?I+d30t6iO@m>|h*BBRE7E-O6OH0)CG6+;>WBSqCZM z^Qa&|VUeN+T&MKpZ&RrO1}xBCuu0c&778bTf>*=wNuNu7dxaL83aV~KK_~{H3Qpq~ zlKdCcVJlIgpV@#OT?Z@jU!t7ZXQ~EecNXra?NyvwmgiC7e^aGius$4>*7n&``K{Y$ z2R?1y{%-PSa=E!@$FcNGPOj8IZGL~xa!tolO~)sd4f&e-xA(uk ze_`JnPtP5v!sz#pl@xAx`{~!8{{Ermn%s{J*=%(E4BbV8`bikoML)H!e8w_8sobgb$AG2;S{E@b_AIPo?2lWjZ>IV>PG$2{UcrtUe0Nm4S7@H6C+q4pdxtaSz{x zDi@-=_%84h(AVe@GrUuMU%&wbvkqnI5r7iceC5D`adHcKknv_t{#Us9+^V5M7P1Od z6hLAoiMnkLJY0!2l)H^?5h~(?1zQ5=3vpDHjJXmpqNKY2NN;1Wbb~>a{S1z<>F%h`*&FlTDp=W` z`Wu(7Us~wPdA7}-$lE>N{wB-+bn}+k{uOc0-ShXwLracLK%!0U?+)D@y1%LO`rsS^ z!L1WHZ)?7??rrz$ZrJaT`FvZKc5iN5X=z^!{B2WSd@{ee|7|wY+ez@;PeRukPGI1}R+c)^jaBk<>UwO~|vA$Qtj@xxPai9Kz z-_r{ows4!f9+YJLmYk!*j!Z-M!+j$~U#;H*Tf>^&9gWTk>^{ z3j<4aTj_UjsjdS6<*5KbIR*4m@52E$w_&4U{)`Kpq@vQB43$QVf^u8pttF9lKYxHv z@G!xthjo|VR5bdF_6uZEdox)MA~?HjTWb+#O6(oICBB{VI}TV_YvPx6SHRc`!)~l0 zudp7f*Hy%$P7N$lvtbW`F^iTDoh~calH&X$^?HxzV1ur|{K5f~$c?*3HtHpCtc-{l zdPNXm{bfalj$aHN`U6&AI!2MOmC)o4*h=>qAp5`==FV93QG}mcGYZeh7X4jCjeClwuPmX;;0*LxwcB{Y3|9MU=$JH) zC#z>Uaa281(bc7UU0n#2WvU+g5CD-_OKN6XD%drHDO1o+3`4Rhq8^r4!@M5jYfrz`yF8)x+p$`+3k^uBo6#5@&cy4 zV40BO)0swmb)yrlz_rAy*z07G4uR8mH8QP##s=WVhcg9U$Q&x!5)koa$`pO3(}6=Y z+EylgY6Fa?LxS4JR@f<3-OD~9V-j%!Q8-4H93q~XUB=rx6knef7gaxWFkD&kKIuyg zcG$n0sR>_Y!4a0nS;VnWaYP=+k@iH2#a3nc2o292jz)v1!U|*yvgFHjBgQWXtRWRE za+-2qqFn1Vc2`n3eKvz*>Pjk#{S;11gYi&VX_04bMY#`1G<~<{; zPFBl`S`^(`c5Pa6ZCV)4xmstB{|=^dYI#fV(w5#_<*wO*e{$8XI6T=(-|hD09S4_o z9L)L-Wh)N<;@Cf)`q`4-GHje^(&4>HLzVarV@2D;wdn zJ3RBY`PBT_T=Kr78E9YC^m`k0Bjuak+&!OpF zw#^Mdk<1^RpP8Rrtj<=q|8O_yuaDjP&`VSMr+Br+Id|$!W#PaZ-^|u;%WnHdR=Du7 z<%PV<&Fot`#zzwx4B}uH1@72>lX}qac|1OrTd>fX6l9O0v|vk)(qUFsKU25%RKIrc z5COq4RQ;;t3kV#Ex}@3<%sPQ-scwFuSK=;}1u6F{Z?Q{CU3!e1%yv38GGZKD)Lz<& zeL?+u3wqn7GA--Yw!|45st9t)OO)@@?KQgnh;BqpW~cfZ*3LMhdZ`l!VL-x~b6@DE zvZNp8&9SjEBa5Y&rN#PY<`&CiHrJE%J~F&A4*eqw_4VF$FP24A!i@YK)vaWOGz9&? zCy`t*Lri^}R5`~v&9OAjbK;z)Y(}B{Z|L^#sG6OQ_s3(S>eNqBKG}=~9N;YZ+64zV zoyON6GBY#@BUHzi&Sx^mmmM@<#|Tt^d7KIubTA|JJM>77DKiI|Aj^mGSP=X(upy`V zufje{0fk16!3V(&Plo-ut&{wYcz^qg`4_f<2`^Piix@s|H{mgo4k-*VnxbHZ=9 zhTn2791U%2zy18ZDujDyZF3bl?%)G~Yij%4RB5$6G*>s7DEA>(&6`$xxhC=U`6c(B zPj>X?JiC6dZLx0t8-JC!vu(+<>w(qWWty#kgg@lWs4H-l+h*Ng*jjnv0f*Zcr+6nn z$v@!e{-~jXuXxnz;0O3e^;I|?{HTNHw?1+>__{~+Zoc|avy-oS)X>4Vte)o&@>QSW z+^H46+BsHAIq3ANlTz4>X#*1n925k0ZSqt!vFvP From ba609bcc4ef5fc56d340e5f2a321d68338f04811 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 26 Jul 2026 16:52:27 -0700 Subject: [PATCH 4/8] chore(workflow): refine PR environment comment security and improve workflow robustness (ENG-1053) Enhances PR environment comment workflow by removing sensitive details, promoting security by linking to protected documentation, and refines permission declarations to prevent runtime failures. Updates README and test cases to reflect these changes, ensuring clarity and consistency. These improvements address previous detection gaps and enhance auditability for pull request-based operations. --- .github/actionlint.yaml | 3 +- .github/workflows/notify-startup-failure.yml | 7 +- .github/workflows/pr-env-comment.yml | 143 +++++++++---------- .github/workflows/workflow-lint.yml | 4 +- README.md | 10 +- k8s-secret/action.yml | 8 +- scripts/workflow_graph.py | 55 +++++-- tests/test_workflow_graph.py | 38 ++++- 8 files changed, 154 insertions(+), 114 deletions(-) diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml index 3a842c6..a2d0b71 100644 --- a/.github/actionlint.yaml +++ b/.github/actionlint.yaml @@ -1,8 +1,7 @@ # actionlint needs to be told about self-hosted runner labels, or every # `runs-on:` referring to one is an error and the real findings drown in noise. # -# mdb-dev newdev EKS pod, IRSA into the build/ECR account -# mdb-prod newprod EKS pod +# mdb-dev, mdb-prod the org's self-hosted runner pools self-hosted-runner: labels: - mdb-dev diff --git a/.github/workflows/notify-startup-failure.yml b/.github/workflows/notify-startup-failure.yml index f818a78..d9cc65b 100644 --- a/.github/workflows/notify-startup-failure.yml +++ b/.github/workflows/notify-startup-failure.yml @@ -19,9 +19,10 @@ # - malformed YAML, an unknown `on:` filter, a bad matrix expression # # Its consequence is the worst-shaped one available: a merge to a deploy branch -# reports nothing, so the branch is believed deployed and is not. Two pushes to -# auth `staging` sat undeployed for ten hours this way, and the first thing the -# channel heard was the RECOVERY message from the run that fixed it. +# reports nothing, so the branch is believed deployed and is not. It has already +# happened to a consuming repo, whose deploy branch sat undeployed for hours across +# two merges, and where the first thing the engineering channel heard about it was +# the RECOVERY message from the run that fixed it. # # So this workflow watches from outside, on a schedule. # diff --git a/.github/workflows/pr-env-comment.yml b/.github/workflows/pr-env-comment.yml index e0817b9..b4affa3 100644 --- a/.github/workflows/pr-env-comment.yml +++ b/.github/workflows/pr-env-comment.yml @@ -1,41 +1,48 @@ # Reusable workflow: post (and keep updating) one comment telling reviewers where -# a pull request's environment is and how to sign in. +# a pull request's environment is. # # Every repo that deploys a PR environment wants this and only one had it, so the # reviewer's first move on the other four was to reconstruct a hostname from a # namespace convention or ask someone. The mechanism belongs here. # -# THE FACTS DO NOT. This repo is public and has to stay public (a public caller -# may only consume public reusable workflows, and cowork, cowork-server, and anton -# are public callers), so anything hard-coded here is world-readable. The account -# you sign in as, the Secret and Secrets Manager bundle holding its password, the -# namespace convention, and the internal host pattern are not credentials, but -# together they are a free recon package: which username to spray, and exactly -# what to reach for once inside a cluster. +# IT CARRIES NO CREDENTIAL DETAIL. Not the password (obviously), but also not the +# account name, not the Secret and key holding the password, and not the command +# that reads it. That is a deliberate narrowing, and the reasoning is worth keeping +# because the convenience argument is seductive: # -# So every one of those arrives as an INPUT from the private caller. This file -# renders a table and maintains one marker-keyed comment; it names nothing of ours. -# A reusable that cannot be described without naming our infrastructure has not -# earned promotion and stays in the repo that owns it. +# A `kubectl get secret` line grants nobody any capability. Anyone who can run it +# can already read every Secret in that namespace. So it helps only the people who +# least need help, and it pays for that by publishing the account name, the Secret +# name, and the key into a permanent, org-wide, searchable surface — a free map of +# what to grab, for whoever eventually gets cluster read. # -# Layout is deliberate: the two things a reviewer actually wants — who to sign in -# as, and the one command that prints the password — come FIRST, before any links -# or caveats. The previous version led with four paragraphs of rationale and put -# the account halfway down. +# Credentials live in the access-controlled runbook, which is one place to update and +# one place to audit. This comment links it. `access-doc-url` is that link. # -# It never posts the password itself. A PR comment is permanent, org-wide, and -# un-redactable, and the credential is one `kubectl` away for anyone who can reach -# the cluster; printing it here would put a live admin password in the timeline of -# every PR forever. +# THE FACTS STAY WITH THE CALLER. This repo is public and has to stay public (a +# public caller may only consume public reusable workflows, and cowork, +# cowork-server, and anton are public callers), so anything hard-coded here is +# world-readable. Hosts, namespace conventions, and the runbook link all arrive as +# INPUTS. A reusable that cannot be described without naming our infrastructure has +# not earned promotion and stays in the repo that owns it. # -# k8s-secret-ok: the `kubectl get secret` this renders is a command for a human to -# run, assembled from caller inputs. This workflow never reads the value. +# On a PUBLIC repo, `access-doc-url` and `notes` are dropped: both describe internals +# by nature. `links` are NOT dropped, because only the caller knows what a link points +# at — an internal dev hostname is worth suppressing, a build artifact on a run that +# is already public in the Actions tab is not. Composing the links is the caller's +# job, so judging them is too. # -# The calling job must grant `pull-requests: write`, and no job here declares it: a -# called workflow can never exceed its caller's grant and that cap is enforced when -# the file LOADS, so a scope named here would reject the run of any caller that -# does not grant it — including, in a shared pipeline, the push callers that never -# comment on anything. +# ON PERMISSIONS. The `comment` job DECLARES `pull-requests: write` rather than +# inheriting it, and that is deliberate after getting it wrong once: a job that +# declares nothing is invisible to the permission gate, so a caller that forgets the +# grant fails at RUN time with an opaque `HTTP 403` on the first `gh api` call +# instead of failing the lint. Declaring it means every caller must grant it, which +# is exactly the relationship the gate verifies. +# +# The consequence is that this workflow must be called from the pull-request pipeline +# DIRECTLY, not from a shared build-and-deploy reusable that push pipelines also +# call — otherwise those callers would have to grant a PR scope they never use, on +# the release path. name: PR environment comment @@ -43,7 +50,7 @@ on: workflow_call: inputs: env-name: - description: "The environment's name, e.g. pr-auth-123. Shown as the heading." + description: "The environment's name. Shown as the heading." type: string required: true links: @@ -52,28 +59,15 @@ on: e.g. '[{"label":"Console","url":"https://..."}]' type: string default: "[]" - login-email: - description: "Account to sign in as. Omitted from the comment when empty." - type: string - default: "" - login-note: - description: "One short line about that account (realm, roles it carries)." - type: string - default: "" - password-namespace: - description: "Namespace for the rendered password command. Defaults to env-name." - type: string - default: "" - password-secret: - description: "Secret holding the password. No command is rendered when empty." - type: string - default: "" - password-key: - description: "Key within that Secret." - type: string - default: "" - password-source-note: - description: "Where the same value comes from upstream, e.g. a Secrets Manager bundle name." + access-doc-url: + description: >- + Link to the runbook covering how to sign in: which account, and how to + obtain its password. + + A LINK, and never the method. Anything that lets a reader obtain a + credential is the credential, and a PR comment is a permanent, searchable, + un-redactable place to put one. Keep the method behind whatever already + guards it. type: string default: "" notes: @@ -96,21 +90,20 @@ defaults: jobs: comment: runs-on: ${{ inputs.runs-on }} + permissions: + contents: read + pull-requests: write # the rolling comment steps: - - name: Post the environment access details + - name: Post the environment details env: GH_TOKEN: ${{ github.token }} PR: ${{ github.event.pull_request.number }} ENV_NAME: ${{ inputs.env-name }} LINKS: ${{ inputs.links }} - LOGIN_EMAIL: ${{ inputs.login-email }} - LOGIN_NOTE: ${{ inputs.login-note }} - PW_NS: ${{ inputs.password-namespace || inputs.env-name }} - PW_SECRET: ${{ inputs.password-secret }} - PW_KEY: ${{ inputs.password-key }} - PW_SOURCE: ${{ inputs.password-source-note }} + DOC_URL: ${{ inputs.access-doc-url }} NOTES: ${{ inputs.notes }} MARKER_KEY: ${{ inputs.marker }} + IS_PRIVATE: ${{ github.event.repository.private }} run: | set -euo pipefail @@ -119,28 +112,21 @@ jobs: exit 1 fi + # Fail closed: `private` is present on a pull_request payload, and if it + # ever is not, the safe reading is "assume public". + if [ "${IS_PRIVATE:-false}" != "true" ]; then + LINKS="[]" + DOC_URL="" + NOTES="" + echo "Public repository: posting the environment name only." + fi + MARKER="" { echo "${MARKER}" echo "## PR environment is up · \`${ENV_NAME}\`" echo - # The two things a reviewer came for, first. - if [ -n "$LOGIN_EMAIL" ]; then - echo -n "**Sign in as** \`${LOGIN_EMAIL}\`" - [ -n "$LOGIN_NOTE" ] && echo -n " — ${LOGIN_NOTE}" - echo - echo - fi - if [ -n "$PW_SECRET" ] && [ -n "$PW_KEY" ]; then - echo "**Password** (not in this comment on purpose, it is a live credential and a PR comment is permanent):" - echo - echo '```bash' - echo "kubectl -n ${PW_NS} get secret ${PW_SECRET} -o jsonpath='{.data.${PW_KEY}}' | base64 -d; echo" - echo '```' - echo - fi - if [ "$(jq 'length' <<< "${LINKS:-[]}")" != "0" ]; then echo "| | |" echo "| :--- | :--- |" @@ -148,11 +134,16 @@ jobs: echo fi - if [ -n "$NOTES" ] || [ -n "$PW_SOURCE" ]; then + if [ -n "$DOC_URL" ]; then + echo "**Signing in:** [how to get an account and its password](${DOC_URL}). Deliberately not in this comment — a PR comment is permanent and world-visible to the org." + echo + fi + + if [ -n "$NOTES" ]; then echo "
More" echo - [ -n "$PW_SOURCE" ] && { echo "${PW_SOURCE}"; echo; } - [ -n "$NOTES" ] && { echo "${NOTES}"; echo; } + echo "${NOTES}" + echo echo "
" echo fi diff --git a/.github/workflows/workflow-lint.yml b/.github/workflows/workflow-lint.yml index 0df52e7..4d30bc3 100644 --- a/.github/workflows/workflow-lint.yml +++ b/.github/workflows/workflow-lint.yml @@ -10,8 +10,8 @@ # permissions The one check neither tool does: a called workflow may never # declare a permission its caller lacks. Blocking. See # scripts/workflow_graph.py for why this is not redundant — -# short version, it was verified against a tree that had already -# broken production and neither tool said a word. +# short version, it was verified against a real tree that had +# already broken a deploy branch, and neither tool said a word. # zizmor The established Actions SECURITY auditor (template injection, # credential persistence, unpinned actions, excessive permissions). # Advisory by default, because pointing it at existing pipelines diff --git a/README.md b/README.md index e01dcfa..c272a9c 100644 --- a/README.md +++ b/README.md @@ -156,9 +156,9 @@ Blind spot to know about: it can only read local (`./.github/workflows/...`) cal ```yaml - uses: mindsdb/github-actions/k8s-secret@main with: - namespace: pr-auth-123 - secret: keycloak-secrets - key: PRIVATE_CLIENT_SECRET + namespace: pr--123 + secret: + key: env-var: PR_KEYCLOAK_CLIENT_SECRET ``` @@ -168,7 +168,7 @@ There is no blanket answer, and "prefer Kubernetes because it is the source of t | The value is | Source | Why | | --- | --- | --- | -| Ephemeral and namespace-local (a per-PR environment's own credentials) | **Kubernetes**, via `k8s-secret` | No GitHub Environment can exist for `pr-auth-204`, so a copy is impossible; the namespace is the only source there is | +| Ephemeral and namespace-local (a per-PR environment's own credentials) | **Kubernetes**, via `k8s-secret` | No GitHub Environment can exist for `pr--204`, so a copy is impossible; the namespace is the only source there is | | A permanent environment's real credential (prod/staging DB, Stripe live, prod vendor tokens) | **GitHub Environment** | A job can only read it by declaring `environment: prod`, and that environment requires a reviewer. A k8s Secret has no such gate — *any* job on a runner with cluster read can fetch it, unreviewed | | Needed to reach the cluster, or used on a GitHub-hosted runner | **GitHub secret** | Package-install tokens, the ArgoCD token that creates the namespace, Snyk, Slack. Reading these from the cluster is circular | @@ -189,7 +189,7 @@ What actually goes wrong with a GitHub secret is different and has a cheaper fix # `pull-requests: write`; the push callers must not have to. uses: mindsdb/github-actions/.github/workflows/pr-env-comment.yml@main with: - env-name: pr-auth-${{ github.event.pull_request.number }} + env-name: pr--${{ github.event.pull_request.number }} login-email: someone@example.com password-secret: some-secret password-key: SOME_KEY diff --git a/k8s-secret/action.yml b/k8s-secret/action.yml index 289f0a7..603e6cb 100644 --- a/k8s-secret/action.yml +++ b/k8s-secret/action.yml @@ -5,10 +5,10 @@ # Both patterns are in use across these repos. The honest comparison: # # A GitHub Environment secret is auto-masked, needs no cluster, and is invisible -# to fork PRs. But it is a SECOND COPY of a value whose source of truth is AWS -# Secrets Manager (see auth's scripts/secrets/create-aws-secrets.py), duplicated -# per environment, rotated by hand in lockstep — and a reference to a secret that -# no longer exists resolves to the EMPTY STRING rather than failing. That is a +# to fork PRs. But it is a SECOND COPY of a value whose source of truth is a +# secrets manager that already populates the cluster, duplicated per environment +# and rotated by hand in lockstep — and a reference to a secret that no longer +# exists resolves to the EMPTY STRING rather than failing. That is a # real failure mode, not a theoretical one: a renamed token left a config job # authenticating with "" and reporting a vendor 401 instead of "the secret is # gone". diff --git a/scripts/workflow_graph.py b/scripts/workflow_graph.py index 6388e2a..9371707 100644 --- a/scripts/workflow_graph.py +++ b/scripts/workflow_graph.py @@ -31,9 +31,9 @@ That failure mode is uniquely bad. The run has zero jobs, so the pipeline's own terminal notify job cannot fire, and nothing anywhere says the push did not -deploy. It cost this repo two undeployed merges to ``staging`` and ten hours of -serving the previous image, and the first thing the engineering channel heard -about it was the *recovery* message from the run that fixed it. +deploy. It has already cost a consuming repo two undeployed merges to its deploy +branch, where the first thing the engineering channel heard about it was the +*recovery* message from the run that fixed it. No existing tool catches it, which is the only reason this file exists. Verified against the failing tree: ``actionlint`` reports nothing (it does not resolve the @@ -52,10 +52,18 @@ within the calling job's effective grant which in practice means a job in a shared reusable declares only what ALL its -callers grant and INHERITS anything only one of them needs. Inheriting is not a -loss of precision, it is the only thing that composes: the PR caller grants -``pull-requests: write`` and the comment posts, the push callers grant -``contents: read`` and the same job runs without the scope it never needed. +callers grant. Anything only ONE caller needs does not belong in a shared reusable +at all: move that job into the caller that has the grant. + +Do not reach for "just inherit it" — that was tried and it is a trap. A job with no +``permissions:`` block declares nothing for this check to compare, so it is +invisible here, and a caller that forgot the grant fails at RUN time with an opaque +``HTTP 403`` on its first API call rather than failing the lint. Declaring the scope +is what makes the relationship checkable at all. + +The blind spot that remains: this check can see what a job DECLARES, never what a +job NEEDS. A job that quietly requires a scope nobody granted is not detectable +statically, which is the second reason to declare rather than inherit. Remote callees (``uses: org/repo/.github/workflows/x.yml@ref``) cannot be read from here and are reported as unchecked rather than assumed fine. @@ -97,7 +105,7 @@ # What the workflow token carries when nothing declares anything. This is the # repo's "Read repository contents and packages permissions" setting; confirm with -# gh api repos/mindsdb/auth/actions/permissions/workflow +# gh api repos///actions/permissions/workflow DEFAULT_GRANTS: dict[str, dict[str, str]] = { "read": {"contents": "read", "packages": "read"}, "write": {scope: "write" for scope in SCOPES}, @@ -138,8 +146,10 @@ def __str__(self) -> str: f"but {where} declares {self.scope}: {self.declared}\n" f" call chain: {chain}\n" f" every run of {self.caller} would be rejected as a startup_failure, with no job to report it.\n" - f" fix: drop the `permissions:` block in the callee so it inherits the caller's ceiling, " - f"or grant {self.scope}: {self.declared} on `{self.caller_job}` if every caller should hold it." + f" fix: grant {self.scope}: {self.declared} on `{self.caller_job}` if EVERY caller should hold it, " + f"or move that callee job into the one caller that needs it. Do not just delete the callee's " + f"`permissions:` block: it silences this check without giving the job the scope, and the failure " + f"comes back at run time as an opaque 403." ) @@ -207,14 +217,26 @@ def level_of(grants: dict[str, str], scope: str) -> str: return grants.get(scope, "none") +# A workflow that must keep its own run tree says so, with a reason, the same way +# the secret-hygiene rule is opted out of. Written into the file rather than a +# central list, because the reason belongs next to the thing it excuses. +RUN_TREE_EXEMPT = "run-tree-ok:" + +EXEMPT: set[str] = set() + + def load_workflows(workflow_dir: Path) -> dict[str, dict]: """Parse every workflow, keyed by the `./.github/workflows/x.yml` form callers use.""" workflows: dict[str, dict] = {} + EXEMPT.clear() for path in sorted(workflow_dir.glob("*.y*ml")): - with path.open(encoding="utf-8") as handle: - parsed = yaml.safe_load(handle) + raw = path.read_text(encoding="utf-8") + parsed = yaml.safe_load(raw) if isinstance(parsed, dict): - workflows[f"{LOCAL_PREFIX}{path.name}"] = parsed + key = f"{LOCAL_PREFIX}{path.name}" + workflows[key] = parsed + if RUN_TREE_EXEMPT in raw: + EXEMPT.add(key) return workflows @@ -386,7 +408,7 @@ def check_run_trees( by_event: dict[str, list[str]] = {} for key, workflow in workflows.items(): - if not is_entry_point(workflow): + if not is_entry_point(workflow) or key in EXEMPT: continue for event in event_keys(workflow): by_event.setdefault(event, []).append(key) @@ -400,7 +422,10 @@ def check_run_trees( f" Fold the side ones into the event's pipeline as `workflow_call` jobs, so one run " f"shows everything that happened. A job that should not pay for itself on every run " f"gates on a path check; a job behind an `environment:` approval needs an ungated plan " - f"job before it, since an approval blocks a job from starting." + f"job before it, since an approval blocks a job from starting.\n" + f" If a workflow genuinely has to keep its own run tree (a vendor OIDC claim bound to its " + f"filename, a release-train hook on someone else's event), add a `run-tree-ok: ` " + f"comment to it." ) if not allow_external_reusables: diff --git a/tests/test_workflow_graph.py b/tests/test_workflow_graph.py index ca381ae..15213cd 100644 --- a/tests/test_workflow_graph.py +++ b/tests/test_workflow_graph.py @@ -1,11 +1,10 @@ """Unit tests for the workflow permission gate (``scripts/workflow_graph.py``). -The gate exists because of a real incident in ``mindsdb/auth``: -``config-apply.yml``'s ``detect`` and ``plan`` jobs declared a ``pull-requests`` -scope that the staging and prod push callers did not grant, GitHub rejected those -runs at file load with ``startup_failure`` and zero jobs, and two merges to -``staging`` sat undeployed for ten hours with nothing said — the terminal notify -job cannot fire in a run that never started. +The gate exists because of a real failure in a consuming repo: a shared reusable's +jobs declared a ``pull-requests`` scope that the push callers did not grant, GitHub +rejected those runs at file load with ``startup_failure`` and zero jobs, and merges +to a deploy branch went undeployed with nothing said — the terminal notify job +cannot fire in a run that never started. These tests pin the rule and, at the bottom, replay that exact shape, so the class cannot come back the next time someone adds a scope to a shared reusable, in any @@ -342,7 +341,8 @@ def test_the_finding_says_what_would_happen_and_how_to_fix_it(self, tmp_path): message = str(violations[0]) assert "startup_failure" in message assert "no job to report it" in message - assert "inherits the caller's ceiling" in message + assert "if EVERY caller should hold it" in message + assert "Do not just delete" in message, "the wrong fix has to be named as wrong" def test_inheriting_in_the_callee_is_what_resolves_it(self, tmp_path): """The fix that composes: the callee stops naming the scope only one caller grants.""" @@ -426,6 +426,30 @@ def test_an_uncalled_reusable_is_a_note_not_an_error(self, tmp_path): assert errors == [] assert len(notes) == 1 and "nothing in this repo calls" in notes[0] + def test_a_workflow_can_be_exempted_with_a_written_reason(self, tmp_path): + """Some duplication is deliberate: a vendor OIDC claim bound to a filename, or a + release-train hook that fires on another pipeline's event. The exemption is a + comment in the file, so the reason lives next to the thing it excuses.""" + write(tmp_path, "prod.yml", {"on": {"push": {"branches": ["main"]}}, "jobs": {"a": {"runs-on": "x"}}}) + (tmp_path / "hook.yml").write_text( + "# run-tree-ok: fires on the release PR merging, not on this pipeline's behalf\n" + "name: Hook\non:\n push:\n branches: [main]\njobs:\n b:\n runs-on: x\n", + encoding="utf-8", + ) + errors, _ = gate.check_run_trees(gate.load_workflows(tmp_path)) + assert errors == [] + + def test_an_unexcused_duplicate_is_still_an_error_alongside_an_exempt_one(self, tmp_path): + write(tmp_path, "prod.yml", {"on": {"push": {"branches": ["main"]}}, "jobs": {"a": {"runs-on": "x"}}}) + write(tmp_path, "other.yml", {"on": {"push": {"branches": ["main"]}}, "jobs": {"c": {"runs-on": "x"}}}) + (tmp_path / "hook.yml").write_text( + "# run-tree-ok: deliberate\nname: Hook\non:\n push:\n branches: [main]\njobs:\n b:\n runs-on: x\n", + encoding="utf-8", + ) + errors, _ = gate.check_run_trees(gate.load_workflows(tmp_path)) + assert len(errors) == 1 + assert "hook.yml" not in errors[0] + def test_a_library_repo_can_silence_the_orphan_note(self, tmp_path): """This repo's reusables are called from other repos, by design.""" write(tmp_path, "orphan.yml", {"on": {"workflow_call": None}, "jobs": {"a": {"runs-on": "x"}}}) From b021bbc1f0532055d8d18f6f22f2b6ed74190af6 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 26 Jul 2026 16:54:14 -0700 Subject: [PATCH 5/8] chore(workflow): enhance heading customization for PR environment comments (ENG-1053) Introduces a new input option to customize the heading text in PR environment comments, accommodating varied use cases such as installer artifacts. Simplifies message consistency by updating the subtext. These changes provide greater flexibility and clearer communication in pull request comments. --- .github/workflows/pr-env-comment.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-env-comment.yml b/.github/workflows/pr-env-comment.yml index b4affa3..89eedc9 100644 --- a/.github/workflows/pr-env-comment.yml +++ b/.github/workflows/pr-env-comment.yml @@ -50,9 +50,16 @@ on: workflow_call: inputs: env-name: - description: "The environment's name. Shown as the heading." + description: "What this comment is about — an environment name, a build kind. Shown in the heading." type: string required: true + heading: + description: >- + Heading text, with the env-name appended. Defaults to the per-PR + environment wording; a caller posting something else (installer + artifacts, say) supplies its own. + type: string + default: "PR environment is up" links: description: >- JSON array of {"label","url"} objects, rendered as a table in order. @@ -103,6 +110,7 @@ jobs: DOC_URL: ${{ inputs.access-doc-url }} NOTES: ${{ inputs.notes }} MARKER_KEY: ${{ inputs.marker }} + HEADING: ${{ inputs.heading }} IS_PRIVATE: ${{ github.event.repository.private }} run: | set -euo pipefail @@ -124,7 +132,7 @@ jobs: MARKER="" { echo "${MARKER}" - echo "## PR environment is up · \`${ENV_NAME}\`" + echo "## ${HEADING} · \`${ENV_NAME}\`" echo if [ "$(jq 'length' <<< "${LINKS:-[]}")" != "0" ]; then @@ -148,7 +156,7 @@ jobs: echo fi - echo "Rebuilt on every push to this PR. Torn down when the PR closes or loses its \`deploy\` label." + echo "Updated on every push to this PR." } > body.md # One rolling comment per PR rather than a new one per push. From 90b1a4909a948020eea040034ee65fd90e7187e5 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 26 Jul 2026 17:08:42 -0700 Subject: [PATCH 6/8] chore(workflow): refine secret retrieval check to skip comments (ENG-1053) Adjusts the grep command to ignore commented lines when checking for secret retrieval commands in scripts, preventing false positives. This ensures that only executable lines accessing secrets are flagged, enhancing the reliability of workflow audits. --- .github/workflows/workflow-lint.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/workflow-lint.yml b/.github/workflows/workflow-lint.yml index 4d30bc3..0dad854 100644 --- a/.github/workflows/workflow-lint.yml +++ b/.github/workflows/workflow-lint.yml @@ -124,7 +124,12 @@ jobs: FOUND=0 while IFS= read -r file; do grep -q 'k8s-secret-ok:' "$file" && continue - if grep -nE 'kubectl.*get[[:space:]]+secret' "$file"; then + # Comment lines are skipped: this rule is about a read that EXECUTES, + # and a workflow explaining the rule should not trip it. (It did, on + # the first repo that documented why its comment carries no secrets.) + # A commented-out read cannot leak anything until someone uncomments + # it, at which point this fires. + if grep -nE '^[[:space:]]*[^#[:space:]].*kubectl.*get[[:space:]]+secret' "$file"; then echo " ^ in $file" FOUND=1 fi From 67300f03bc150f722a431db60eb5e6b678abbfd8 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 26 Jul 2026 17:45:46 -0700 Subject: [PATCH 7/8] chore(workflow): implement absent state messaging for PR environments (ENG-1053) Adds the capability to indicate when a PR environment is absent, enhancing clarity by explicitly stating its non-existence and providing instructions to create one. This change improves developer experience by preventing confusion when a previously accessible environment becomes unavailable. --- .github/workflows/pr-env-comment.yml | 47 +++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pr-env-comment.yml b/.github/workflows/pr-env-comment.yml index 89eedc9..0b52774 100644 --- a/.github/workflows/pr-env-comment.yml +++ b/.github/workflows/pr-env-comment.yml @@ -57,9 +57,25 @@ on: description: >- Heading text, with the env-name appended. Defaults to the per-PR environment wording; a caller posting something else (installer - artifacts, say) supplies its own. + artifacts, say) supplies its own. Ignored when `status: absent`. type: string default: "PR environment is up" + status: + description: >- + `up` when the environment exists, `absent` when it does not. + + Absent is a real state worth saying out loud rather than staying quiet. + The namespace convention is documented, so someone WILL construct the URL + for a PR that never got the label and land on a dead host; and because + this comment rolls rather than reposting, one left behind after a teardown + points at something that used to work. Saying "there is no environment, + here is how to get one" answers both. + type: string + default: up + deploy-label: + description: "Label that creates the environment, named in the absent message." + type: string + default: deploy links: description: >- JSON array of {"label","url"} objects, rendered as a table in order. @@ -111,6 +127,8 @@ jobs: NOTES: ${{ inputs.notes }} MARKER_KEY: ${{ inputs.marker }} HEADING: ${{ inputs.heading }} + STATUS: ${{ inputs.status }} + DEPLOY_LABEL: ${{ inputs.deploy-label }} IS_PRIVATE: ${{ github.event.repository.private }} run: | set -euo pipefail @@ -120,18 +138,36 @@ jobs: exit 1 fi - # Fail closed: `private` is present on a pull_request payload, and if it - # ever is not, the safe reading is "assume public". + # On a public repo, drop what describes internals by nature: the runbook + # link and the free-text notes. NOT the links — an environment's own URL + # is already published on this PR by the deployment entry in the sidebar, + # so withholding it here hides it from the reviewer and from nobody else, + # and it is the one thing the comment exists to say. Whether a link is + # safe to publish is the caller's call, since only the caller knows what + # it points at. + # + # Fail closed on an absent value: `private` is present on a pull_request + # payload, and if it ever is not, the safe reading is "assume public". if [ "${IS_PRIVATE:-false}" != "true" ]; then - LINKS="[]" DOC_URL="" NOTES="" - echo "Public repository: posting the environment name only." + echo "Public repository: omitting the runbook link and the notes." fi MARKER="" { echo "${MARKER}" + + if [ "$STATUS" = "absent" ]; then + # No `exit` here: this block is a redirect group, not a subshell, so + # exiting would end the whole step and never post what it just wrote. + echo "## No PR environment for this pull request" + echo + echo "Add the \`${DEPLOY_LABEL}\` label and push to create one. It is torn down when the label is removed or the PR closes, so any URL you saw here earlier is gone." + echo + echo "Updated on every push to this PR." + else + echo "## ${HEADING} · \`${ENV_NAME}\`" echo @@ -157,6 +193,7 @@ jobs: fi echo "Updated on every push to this PR." + fi } > body.md # One rolling comment per PR rather than a new one per push. From 729b49c50baf57a900fd580aa6acc26f09351c0c Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 26 Jul 2026 17:50:39 -0700 Subject: [PATCH 8/8] chore(workflow): unify environment link tables in PR comments (ENG-1053) Introduces configurable standard links for per-PR environments, ensuring a comprehensive table of services is listed across all comments. Enhances clarity and consistency in PR comments by centralizing service links, reducing redundancy, and preventing misinterpretations about environment scope. --- .github/workflows/pr-env-comment.yml | 55 ++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-env-comment.yml b/.github/workflows/pr-env-comment.yml index 0b52774..e1385cb 100644 --- a/.github/workflows/pr-env-comment.yml +++ b/.github/workflows/pr-env-comment.yml @@ -76,10 +76,34 @@ on: description: "Label that creates the environment, named in the absent message." type: string default: deploy + standard-links: + description: >- + Render the per-PR environment's full service table, derived from + `env-name`. + + A PR environment is ONE namespace holding the whole stack — hub, auth, + Keycloak, inference, cowork-server — whichever repo anchored it. So the + table is a property of the environment, not of the repo, and every + caller composing its own produced five near-identical tables that each + listed only that repo's own service. Set false for a comment that is not + about an environment (build artifacts, say). + type: boolean + default: true + env-host-suffix: + description: "Domain the per-PR hosts sit under." + type: string + default: dev.mindshub.ai + default-tag: + description: >- + Image tag the services this PR does not build are running. Named in the + comment so nobody assumes the whole namespace is their branch. + type: string + default: staging links: description: >- - JSON array of {"label","url"} objects, rendered as a table in order. - e.g. '[{"label":"Console","url":"https://..."}]' + EXTRA links, appended after the standard table. JSON array of + {"label","url"} objects. For genuinely repo-specific things — a Swagger + path, a health endpoint — not for the service hosts, which are derived. type: string default: "[]" access-doc-url: @@ -123,6 +147,9 @@ jobs: PR: ${{ github.event.pull_request.number }} ENV_NAME: ${{ inputs.env-name }} LINKS: ${{ inputs.links }} + STANDARD: ${{ inputs.standard-links }} + HOST_SUFFIX: ${{ inputs.env-host-suffix }} + DEFAULT_TAG: ${{ inputs.default-tag }} DOC_URL: ${{ inputs.access-doc-url }} NOTES: ${{ inputs.notes }} MARKER_KEY: ${{ inputs.marker }} @@ -171,10 +198,30 @@ jobs: echo "## ${HEADING} · \`${ENV_NAME}\`" echo - if [ "$(jq 'length' <<< "${LINKS:-[]}")" != "0" ]; then + # The whole stack lives in this one namespace, so list the whole + # stack. Derived from the env name rather than passed in, because it + # is identical for every anchor repo. + ALL="${LINKS:-[]}" + if [ "$STANDARD" = "true" ]; then + ALL="$(jq -cn --arg e "$ENV_NAME" --arg d "$HOST_SUFFIX" --argjson extra "${LINKS:-[]}" ' + [ {label: "Hub (console)", url: "https://\($e).\($d)"}, + {label: "Auth API", url: "https://auth-\($e).\($d)"}, + {label: "Keycloak admin", url: "https://auth-\($e).\($d)/auth/admin/"}, + {label: "Inference API", url: "https://api-\($e).\($d)/v1"}, + {label: "cowork-server", url: "https://cowork-\($e).\($d)/api"} ] + + $extra + + [ {label: "Namespace", url: "`\($e)`"} ]')" + fi + + if [ "$(jq 'length' <<< "$ALL")" != "0" ]; then echo "| | |" echo "| :--- | :--- |" - jq -r '.[] | "| \(.label) | \(.url) |"' <<< "$LINKS" + jq -r '.[] | "| \(.label) | \(.url) |"' <<< "$ALL" + echo + fi + + if [ "$STANDARD" = "true" ]; then + echo "Every service is in this namespace. The ones this PR does not build run the \`${DEFAULT_TAG}\` image unless you link them with \`Deploys:\` in the PR body." echo fi