Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 34 additions & 25 deletions .github/scripts/standards-sync-stuck-automerge-alert.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -799,24 +799,19 @@ async function runLookup({ openIssues = [], envOverrides = {} } = {}) {
}
}

async function runClose({ openIssues = [] } = {}) {
async function runClose({ issueNumber } = {}) {
const script = extractStepScript("Close recovered tracking issue");
const keys = ["MARKER", "ISSUE_AUTHOR_LOGIN"];
const keys = ["ISSUE_NUMBER"];
const originalValues = Object.fromEntries(
keys.map((key) => [key, process.env[key]]),
);
Object.assign(process.env, {
MARKER,
ISSUE_AUTHOR_LOGIN: "github-actions[bot]",
});
Object.assign(process.env, { ISSUE_NUMBER: String(issueNumber) });
const comments = [];
const updates = [];
try {
const github = {
paginate: async (_fn, _params) => openIssues,
rest: {
issues: {
listForRepo: () => {},
createComment: async (params) => comments.push(params),
update: async (params) => updates.push(params),
},
Expand Down Expand Up @@ -913,25 +908,39 @@ test("the lookup still fails closed on a genuine ambiguity between two bot-autho
assert.match(failedWith, /found 2 issues carrying marker/u);
});

test("close ignores a decoy issue and takes no action when none of the candidates are bot-authored", async () => {
const { comments, updates } = await runClose({
openIssues: [
issue({
number: 5,
login: "some-random-user",
type: "User",
body: `${MARKER}\ndecoy`,
}),
],
});
assert.equal(comments.length, 0);
assert.equal(updates.length, 0);
// The close step resolves no issue of its own — it acts on the number the
// lookup above already author-filtered. That makes its `if:` the whole of its
// decoy protection: without the `issue-number != ''` conjunct it would run
// with an empty number on every healthy run, and without the lookup running
// unconditionally the number would always be empty on exactly those runs. The
// two halves are only correct together, so both are asserted structurally —
// no mock can reach an `if:` expression.
test("the close step acts only on the author-filtered lookup result, and only when there is one", () => {
const closeStep = workflow.slice(
workflow.indexOf("- name: Close recovered tracking issue"),
);
const closeCondition = /\n\s+if: ([^\n]*)/u.exec(closeStep)[1];
assert.match(closeCondition, /steps\.scan\.outputs\.stuck-count == '0'/u);
assert.match(closeCondition, /steps\.scan\.outputs\.unarmed-count == '0'/u);
assert.match(
closeCondition,
/steps\.tracking\.outputs\.issue-number != ''/u,
"the close step must be guarded on the lookup having resolved an issue",
);

const lookupStep = workflow.slice(
workflow.indexOf("- name: Find existing tracking issue"),
);
const lookupHeader = lookupStep.slice(0, lookupStep.indexOf("script: |"));
assert.doesNotMatch(
lookupHeader,
/\n\s+if:/u,
"the lookup must run unconditionally, or the close step is starved on every healthy run",
);
});

test("close comments on and closes the genuine bot-authored tracking issue", async () => {
const { comments, updates } = await runClose({
openIssues: [issue({ number: 5, body: `${MARKER}\nreport` })],
});
test("close comments on and closes the tracking issue the lookup resolved", async () => {
const { comments, updates } = await runClose({ issueNumber: 5 });
assert.equal(comments.length, 1);
assert.equal(comments[0].issue_number, 5);
assert.equal(updates.length, 1);
Expand Down
118 changes: 53 additions & 65 deletions .github/workflows/standards-sync-stuck-automerge-alert.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,24 @@ name: standards-sync-stuck-automerge-alert
# The target repo list is derived by checking out standards and running its
# own sync-manifest.sh interpreter, the same way standards-sync.yml's plan job
# does — never a second, hand-maintained inventory. The tracking issue is
# opened in the CALLING repository, authored by a second App token this
# workflow mints for itself (scoped to issues:write on that repo only — see
# the "Mint App token for this repository's tracking issue" step), not in any
# scanned target: a scheduled caller in melodic-software/medley gets the
# alert filed in medley, not scattered across the 8 targets.
# opened in the CALLING repository, not in any scanned target: a scheduled
# caller in melodic-software/medley gets the alert filed in medley, not
# scattered across the 8 targets.
#
# CONSUMER CONTRACT: the caller's job must grant `issues: write` (a called
# workflow can only downgrade the caller's grant, never elevate it), matching
# link-check.yml's and pulumi-version-drift-check.yml's callers. The App
# secrets below remain required — they mint the cross-repository read token
# that scans the targets — but they no longer author the tracking issue.
#
# One asymmetry with those two, for a consumer governed by
# melodic-software/standards' runner-policy: a reviewed contract may pin the
# caller's grant through `allowedCallerPermissions`, but only when every
# mapped secret's input name equals the repository secret it reads
# (`NAME: ${{ secrets.NAME }}`) — which the kebab-case inputs below cannot
# satisfy. Those two workflows take no secrets, so they pin the grant freely.
# A contract for this workflow either omits that pin or renames these inputs;
# nothing forces the pin, and granting `issues: write` does not.
on:
workflow_call:
inputs:
Expand Down Expand Up @@ -70,31 +83,32 @@ env:
# `author.login` omits the REST-only `[bot]` suffix; `__typename` is the
# authoritative bot-vs-human discriminator, so both are checked together.
BOT_LOGIN: melodic-standards-sync
# The identity that opens/updates/closes this workflow's own tracking
# issue: a second App token (minted below, scoped to issues:write on only
# the calling repository) authors every REST write this workflow makes, so
# its bot login is the adoption filter — not the ambient GITHUB_TOKEN's
# shared github-actions[bot] identity every workflow in a repo can author
# as. App authorship is strictly tighter decoy resistance: a decoy from any
# other workflow in the caller repo no longer qualifies. It also drops the
# caller's need to grant issues:write to the ambient token at all, which
# resolves a runner-policy contract conflict in melodic-software/standards
# (components/runner-policy): a contract with a write allowedCallerPermissions
# entry must map its secrets as an exact identity passthrough
# `${{ secrets.<name> }}`, which this reusable's kebab-case App-secret
# inputs (app-client-id/app-private-key) can never satisfy. Login format
# (`<app-slug>[bot]`, type Bot) is empirically verified: `gh api
# users/melodic-standards-sync%5Bbot%5D` returns login
# "melodic-standards-sync[bot]", type "Bot".
ISSUE_AUTHOR_LOGIN: melodic-standards-sync[bot]
# The identity that opens/updates/closes this workflow's own tracking issue.
# Every REST write below runs on the job's ambient GITHUB_TOKEN, so the
# issue's author.login is always github-actions[bot] (type Bot) — the same
# authorship link-check.yml, queue-monitor-liveness.yml, and
# tool-version-drift-check.yml already use, and the adoption filter the
# lookup below applies before any marker or title matching.
#
# An App identity is narrower — github-actions[bot] is shared by every
# workflow in the caller repo, so a decoy another workflow there authored
# qualifies where an App-only filter would reject it; both still require
# write access to that repo, and this is the posture
# queue-monitor-liveness.yml already ships. It is also not available here:
# a caller-scoped App token can only be minted where the App is installed,
# and the caller need not be in that installation at all — the App this
# workflow's secrets name is scoped to the sync TARGETS, and the sync
# source is deliberately not one of them.
ISSUE_AUTHOR_LOGIN: 'github-actions[bot]'

jobs:
detect-stuck-armed-prs:
name: Detect standards-sync auto-merge PRs that cannot merge themselves
runs-on: ${{ inputs.runner }}
timeout-minutes: 15
permissions:
contents: read # The ambient token no longer authors any issue write; see ISSUE_AUTHOR_LOGIN above.
contents: read
issues: write # The ambient token authors the rolling tracking issue; see ISSUE_AUTHOR_LOGIN above.
steps:
- name: Check out standards
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
Expand Down Expand Up @@ -438,19 +452,6 @@ jobs:

require('node:fs').writeFileSync(process.env.GITHUB_WORKSPACE + '/.stuck-automerge-report.md', reportBody);

# owner/repositories intentionally omitted: create-github-app-token
# defaults both to the current (calling) repository when unset — the
# exact narrow scope this token needs, distinct from the read-only
# every-target token minted above. Runs unconditionally: both the
# open/update and close branches below need it.
- name: Mint App token for this repository's tracking issue
id: issue-token
uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
with:
client-id: ${{ secrets.app-client-id }}
private-key: ${{ secrets.app-private-key }}
permission-issues: write

# A marker embedded in the body (not a title search) is exact and
# survives a retitle, and matches the mechanism pulumi-version-drift-
# check.yml, link-check.yml, and queue-monitor-liveness.yml already use.
Expand All @@ -461,15 +462,19 @@ jobs:
# was ported off the shared find-tracking-issue.sh bash block — that
# block stays a generated consumer only for the two workflows that run
# on a fixed hosted runner).
#
# Unconditional, matching link-check.yml: the open/update branch and the
# recovery-close branch below partition the outcome space, and both need
# the resolved issue number, so resolving it once here is what lets the
# close branch carry the same `issue-number != ''` guard link-check.yml
# does instead of repeating the lookup with its own author filter.
- name: Find existing tracking issue
id: tracking
if: steps.scan.outputs.stuck-count != '0' || steps.scan.outputs.unarmed-count != '0'
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
env:
MARKER: '<!-- ci-workflows:standards-sync-stuck-automerge-alert:v1:active -->'
ISSUE_TITLE: '[Alert] standards-sync auto-merge PR(s) needing attention'
with:
github-token: ${{ steps.issue-token.outputs.token }}
script: |
const marker = process.env.MARKER;
const issueTitle = process.env.ISSUE_TITLE;
Expand All @@ -484,9 +489,11 @@ jobs:
// The marker and title strings are public (embedded verbatim in
// this workflow file), so matching on them alone would let anyone
// with issue-creation permission in the caller repo craft a decoy
// this automation adopts. Restricting to issues this workflow's
// own identity authored closes that: an attacker's issue is never
// a candidate at all, no matter what text it carries.
// this automation adopts — then overwrites or closes. Restricting
// to issues authored by the caller's Actions identity narrows
// candidates to what a workflow in that repo wrote, so an issue
// opened by a human or a third-party App is never a candidate at
// all, whatever text it carries.
const candidates = openIssues.filter(
(issue) => !issue.pull_request && issue.user?.login === issueAuthorLogin && issue.user?.type === 'Bot',
);
Expand Down Expand Up @@ -516,49 +523,30 @@ jobs:
if: steps.scan.outputs.stuck-count != '0' || steps.scan.outputs.unarmed-count != '0'
uses: peter-evans/create-issue-from-file@fca9117c27cdc29c6c4db3b86c48e4115a786710 # v6.0.0
with:
token: ${{ steps.issue-token.outputs.token }}
title: '[Alert] standards-sync auto-merge PR(s) needing attention'
issue-number: ${{ steps.tracking.outputs.issue-number }}
content-filepath: ${{ github.workspace }}/.stuck-automerge-report.md

# actions/github-script rather than a `gh issue close` run step, for
# the same runner-tool-availability reason as the lookup step above.
- name: Close recovered tracking issue
if: steps.scan.outputs.stuck-count == '0' && steps.scan.outputs.unarmed-count == '0'
if: steps.scan.outputs.stuck-count == '0' && steps.scan.outputs.unarmed-count == '0' && steps.tracking.outputs.issue-number != ''
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
env:
MARKER: '<!-- ci-workflows:standards-sync-stuck-automerge-alert:v1:active -->'
ISSUE_NUMBER: ${{ steps.tracking.outputs.issue-number }}
with:
github-token: ${{ steps.issue-token.outputs.token }}
script: |
const marker = process.env.MARKER;
const issueAuthorLogin = process.env.ISSUE_AUTHOR_LOGIN;
const openIssues = await github.paginate(github.rest.issues.listForRepo, {
owner: context.repo.owner,
repo: context.repo.repo,
state: 'open',
per_page: 100,
});
// Same author restriction as the lookup step: only ever act on an
// issue this workflow's own identity opened, never a decoy.
const match = openIssues.find(
(issue) =>
!issue.pull_request &&
issue.user?.login === issueAuthorLogin &&
issue.user?.type === 'Bot' &&
(issue.body ?? '').includes(marker),
);
if (!match) return;
const issueNumber = Number(process.env.ISSUE_NUMBER);
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: match.number,
issue_number: issueNumber,
body: 'No standards-sync pull request is currently armed and blocked, or unarmed past the threshold; closing this rolling report.',
});
await github.rest.issues.update({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: match.number,
issue_number: issueNumber,
state: 'closed',
state_reason: 'completed',
});
Expand Down