diff --git a/.github/scripts/agents-guard.js b/.github/scripts/agents-guard.js index 54a33548..b312b99b 100644 --- a/.github/scripts/agents-guard.js +++ b/.github/scripts/agents-guard.js @@ -10,6 +10,8 @@ const path = require('path'); const DEFAULT_MARKER = ''; const DEFAULT_PROTECTED_PATHS = ['.github/workflows/agents-*.yml']; +const DEPENDENCY_UPDATE_BOT_LOGINS = new Set(['dependabot[bot]', 'renovate[bot]']); +const TRUSTED_DEPENDENCY_AUTHOR_ASSOCIATIONS = new Set(['OWNER', 'MEMBER', 'COLLABORATOR']); const LEGACY_ALLOW_REMOVED_PATHS = [ // Keepalive consolidation retired the standalone keepalive sweeps. '.github/workflows/agents-75-keepalive-on-gate.yml', @@ -396,6 +398,83 @@ function extractLabelNames(labels) { ); } +function parseActionReferenceLine(line) { + const match = String(line || '').match(/^\s*(?:-\s*)?uses:\s*["']?([^@\s#'"]+)@([^\s#'"]+)["']?(?:\s*(?:#.*)?)?$/i); + if (!match) { + return null; + } + + return { + action: match[1].toLowerCase(), + ref: match[2], + }; +} + +function sameActionSequence(left, right) { + if (left.length !== right.length) { + return false; + } + + for (let index = 0; index < left.length; index += 1) { + if (left[index].action !== right[index].action) { + return false; + } + } + + return true; +} + +function sameActionRefSequence(left, right) { + if (left.length !== right.length) { + return false; + } + + for (let index = 0; index < left.length; index += 1) { + if (left[index].action !== right[index].action || left[index].ref !== right[index].ref) { + return false; + } + } + + return true; +} + +function patchChangesOnlyActionReferences(patch) { + if (!patch || typeof patch !== 'string') { + return false; + } + + const removedRefs = []; + const addedRefs = []; + for (const rawLine of patch.split(/\r?\n/)) { + if (!rawLine || rawLine.startsWith('+++') || rawLine.startsWith('---')) { + continue; + } + + const marker = rawLine[0]; + if (marker !== '+' && marker !== '-') { + continue; + } + + const parsed = parseActionReferenceLine(rawLine.slice(1)); + if (!parsed) { + return false; + } + + if (marker === '-') { + removedRefs.push(parsed); + } else { + addedRefs.push(parsed); + } + } + + if (removedRefs.length === 0 || addedRefs.length === 0) { + return false; + } + + return sameActionSequence(removedRefs, addedRefs) && + !sameActionRefSequence(removedRefs, addedRefs); +} + function evaluateGuard({ files = [], labels = [], @@ -404,6 +483,7 @@ function evaluateGuard({ protectedPaths = DEFAULT_PROTECTED_PATHS, labelName = 'agents:allow-change', authorLogin = '', + authorAssociation = '', marker = DEFAULT_MARKER, repository = process.env.GITHUB_REPOSITORY || '', } = {}) { @@ -524,16 +604,37 @@ function evaluateGuard({ } const normalizedAuthor = authorLogin ? String(authorLogin).toLowerCase() : ''; - const authorIsCodeowner = normalizedAuthor && codeownerLogins.has(normalizedAuthor); + const normalizedAuthorAssociation = authorAssociation + ? String(authorAssociation).toUpperCase() + : ''; + const authorIsCodeowner = Boolean( + normalizedAuthor && codeownerLogins.has(normalizedAuthor), + ); const hasExternalApproval = [...codeownerLogins].some((login) => approvedLogins.has(login)); - const hasCodeownerApproval = hasExternalApproval || authorIsCodeowner; + const hasCodeownerApproval = Boolean(hasExternalApproval || authorIsCodeowner); const hasProtectedChanges = modifiedProtectedPaths.size > 0; - // Security note: Allow `agents:allow-change` label to bypass CODEOWNER approval - // ONLY for automated dependency PRs from known bots (dependabot, renovate). - // Human PRs or other bot PRs still require CODEOWNER approval even with label. - const isAutomatedPR = normalizedAuthor && (normalizedAuthor === 'dependabot[bot]' || normalizedAuthor === 'renovate[bot]'); - const needsApproval = hasProtectedChanges && !hasCodeownerApproval && !(hasAllowLabel && isAutomatedPR); + const protectedChangesAreDependencyOnly = hasProtectedChanges && relevantFiles + .filter((file) => file.status === 'modified' && matchProtectedPath(file.filename || '')) + .every((file) => patchChangesOnlyActionReferences(file.patch || '')); + const isDependencyUpdateBot = Boolean( + normalizedAuthor && DEPENDENCY_UPDATE_BOT_LOGINS.has(normalizedAuthor), + ); + const authorCanUseDependencyBypass = Boolean( + isDependencyUpdateBot || + (normalizedAuthorAssociation && + TRUSTED_DEPENDENCY_AUTHOR_ASSOCIATIONS.has(normalizedAuthorAssociation)), + ); + // Security note: the `agents:allow-change` label can bypass CODEOWNER approval + // only for protected workflow dependency reference updates. This keeps routine + // action version alignment moving while arbitrary workflow logic edits still + // require CODEOWNER review. + const hasDependencyUpgradeBypass = Boolean( + hasAllowLabel && + authorCanUseDependencyBypass && + protectedChangesAreDependencyOnly, + ); + const needsApproval = hasProtectedChanges && !hasCodeownerApproval && !hasDependencyUpgradeBypass; const needsLabel = hasProtectedChanges && !hasAllowLabel && !hasCodeownerApproval; const failureReasons = []; @@ -619,6 +720,10 @@ function evaluateGuard({ authorIsCodeowner, needsLabel, needsApproval, + hasDependencyUpgradeBypass, + protectedChangesAreDependencyOnly, + isDependencyUpdateBot, + authorCanUseDependencyBypass, modifiedProtectedPaths: [...modifiedProtectedPaths], touchedProtectedPaths: [...touchedProtectedPaths], fatalViolations, @@ -633,6 +738,7 @@ module.exports = { evaluateGuard, parseCodeowners, globToRegExp, + patchChangesOnlyActionReferences, validatePullRequestTargetSafety, detectPullRequestTargetViolations, }; diff --git a/.github/workflows/agents-guard.yml b/.github/workflows/agents-guard.yml index 42ca4222..933f0e41 100644 --- a/.github/workflows/agents-guard.yml +++ b/.github/workflows/agents-guard.yml @@ -208,6 +208,8 @@ jobs: const authorLogin = context.payload.pull_request.user && context.payload.pull_request.user.login; + const authorAssociation = + context.payload.pull_request.author_association || ''; const { owner, repo } = context.repo; const baseRef = context.payload.pull_request.base.sha; const labelName = 'agents:allow-change'; @@ -420,6 +422,7 @@ jobs: protectedPaths, labelName, authorLogin, + authorAssociation, marker, }); diff --git a/WORKFLOW_USER_GUIDE.md b/WORKFLOW_USER_GUIDE.md index a934f186..b4ac0497 100644 --- a/WORKFLOW_USER_GUIDE.md +++ b/WORKFLOW_USER_GUIDE.md @@ -165,7 +165,7 @@ Issue: "Add user authentication" | `agent:needs-attention` | Human intervention required | | `agents:auto-pilot-pause` | Auto-pilot paused | | `agents:auto-pilot-failed` | Auto-pilot stopped due to errors | -| `agents:allow-change` | Permission signal for `agents-guard`; bypasses CODEOWNER approval only for automated dependency PRs from Dependabot/Renovate. Auto-applied by `maint-auto-label-dep-prs.yml`; manual application does not bypass guard enforcement. | +| `agents:allow-change` | Permission signal for `agents-guard`; bypasses guard-level CODEOWNER approval only for dependency-only `uses:` reference updates from Dependabot/Renovate or repository owner/member/collaborator PRs. Auto-applied to dependency-bot PRs by `maint-auto-label-dep-prs.yml`; arbitrary workflow logic edits still require review. | | `needs-human` | Escalated to human | | `follow-up` | Created as follow-up to another issue/PR | | `duplicate` | Potential duplicate detected | @@ -850,7 +850,7 @@ The Workflows repository includes maintenance workflows that handle sync, update **Trigger:** When Dependabot or Renovate opens a PR **Labels Applied:** -- `agents:allow-change` (so protected-workflow changes can be reviewed without manual label work) +- `agents:allow-change` (so dependency-bot protected-workflow version updates can use the guarded dependency-update lane) **Use When:** Automatic, no action needed