From c8f2bbf76ddcc0356e0929dbd5a13e14269105c4 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Thu, 28 May 2026 18:13:54 -0700 Subject: [PATCH] ci: guard dependency graph PR changes (#87791) --- .github/CODEOWNERS | 2 + .../workflows/dependency-change-awareness.yml | 167 +---- .../github/dependency-change-awareness.mjs | 630 ++++++++++++++++++ ...dependency-change-awareness-script.test.ts | 279 ++++++++ ...pendency-change-awareness-workflow.test.ts | 75 ++- 5 files changed, 966 insertions(+), 187 deletions(-) create mode 100644 scripts/github/dependency-change-awareness.mjs create mode 100644 test/scripts/dependency-change-awareness-script.test.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 16688d596b7..bfb8116e111 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -13,6 +13,8 @@ /.github/workflows/codeql-critical-quality.yml @openclaw/openclaw-secops /.github/workflows/dependency-change-awareness.yml @openclaw/openclaw-secops /test/scripts/dependency-change-awareness-workflow.test.ts @openclaw/openclaw-secops +/test/scripts/dependency-change-awareness-script.test.ts @openclaw/openclaw-secops +/scripts/github/dependency-change-awareness.mjs @openclaw/openclaw-secops /package-lock.json @openclaw/openclaw-secops /npm-shrinkwrap.json @openclaw/openclaw-secops /extensions/*/package-lock.json @openclaw/openclaw-secops diff --git a/.github/workflows/dependency-change-awareness.yml b/.github/workflows/dependency-change-awareness.yml index 37d649ceda4..9c75a5b56c9 100644 --- a/.github/workflows/dependency-change-awareness.yml +++ b/.github/workflows/dependency-change-awareness.yml @@ -1,10 +1,11 @@ name: Dependency Change Awareness on: - pull_request_target: # zizmor: ignore[dangerous-triggers] metadata-only workflow; no checkout or untrusted code execution + pull_request_target: # zizmor: ignore[dangerous-triggers] checks trusted base script only; never checks out PR head types: [opened, reopened, synchronize, ready_for_review] permissions: + contents: read pull-requests: write issues: write @@ -18,159 +19,15 @@ jobs: runs-on: ubuntu-24.04 timeout-minutes: 5 steps: - - name: Label and comment on dependency changes - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 + - name: Check out trusted base workflow scripts + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: - script: | - const marker = ""; - const labelName = "dependencies-changed"; - const maxListedFiles = 25; - const pullRequest = context.payload.pull_request; + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false - if (!pullRequest) { - core.info("No pull_request payload found; skipping."); - return; - } - - const isDependencyFile = (filename) => - filename === "package.json" || - filename === "package-lock.json" || - filename === "npm-shrinkwrap.json" || - filename === "pnpm-lock.yaml" || - filename === "pnpm-workspace.yaml" || - filename === "ui/package.json" || - filename.startsWith("patches/") || - /^packages\/[^/]+\/package\.json$/u.test(filename) || - /^extensions\/[^/]+\/package-lock\.json$/u.test(filename) || - /^extensions\/[^/]+\/npm-shrinkwrap\.json$/u.test(filename) || - /^extensions\/[^/]+\/package\.json$/u.test(filename); - - const sanitizeDisplayValue = (value) => - String(value) - .replace(/[\u0000-\u001f\u007f]/gu, "?") - .slice(0, 240); - const markdownCode = (value) => - `\`${sanitizeDisplayValue(value).replaceAll("`", "\\`")}\``; - const ignoreUnavailableWritePermission = (action) => (error) => { - if (error?.status === 403) { - core.warning( - `Skipping dependency change ${action}; token does not have issue write permission.`, - ); - return; - } - if (error?.status === 404 || error?.status === 422) { - core.warning(`Dependency change ${action} is unavailable.`); - return; - } - throw error; - }; - - const files = await github.paginate(github.rest.pulls.listFiles, { - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: pullRequest.number, - per_page: 100, - }); - const dependencyFiles = files - .map((file) => file.filename) - .filter((filename) => typeof filename === "string" && isDependencyFile(filename)) - .sort((left, right) => left.localeCompare(right)); - - const comments = await github.paginate(github.rest.issues.listComments, { - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pullRequest.number, - per_page: 100, - }); - const existingComment = comments.find( - (comment) => - comment.user?.login === "github-actions[bot]" && comment.body?.includes(marker), - ); - - const labels = await github.paginate(github.rest.issues.listLabelsOnIssue, { - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pullRequest.number, - per_page: 100, - }); - const hasLabel = labels.some((label) => label.name === labelName); - - if (dependencyFiles.length === 0) { - if (hasLabel) { - await github.rest.issues.removeLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pullRequest.number, - name: labelName, - }).catch(ignoreUnavailableWritePermission("label removal")); - } - if (existingComment) { - await github.rest.issues.deleteComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existingComment.id, - }).catch(ignoreUnavailableWritePermission("comment deletion")); - } - await core.summary - .addHeading("Dependency Change Awareness") - .addRaw("No dependency-related file changes detected.") - .write(); - core.info("No dependency-related file changes detected."); - return; - } - - if (!hasLabel) { - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pullRequest.number, - labels: [labelName], - }).catch(ignoreUnavailableWritePermission(`label "${labelName}" update`)); - } - - const listedFiles = dependencyFiles.slice(0, maxListedFiles); - const omittedCount = dependencyFiles.length - listedFiles.length; - const fileLines = listedFiles.map((filename) => `- ${markdownCode(filename)}`); - if (omittedCount > 0) { - fileLines.push(`- ${omittedCount} additional dependency-related files not shown`); - } - - const body = [ - marker, - "", - "### Dependency Changes Detected", - "", - "This PR changes dependency-related files. Maintainers should confirm these changes are intentional.", - "", - "Changed files:", - ...fileLines, - "", - "Maintainer follow-up:", - "- Review whether the dependency changes are intentional.", - "- Inspect resolved package deltas when lockfile, shrinkwrap, or workspace dependency policy changes are present.", - "- Treat `package-lock.json` and `npm-shrinkwrap.json` diffs as security-review surfaces.", - "- Run `pnpm deps:changes:report -- --base-ref origin/main --markdown /tmp/dependency-changes.md --json /tmp/dependency-changes.json` locally for detailed release-style evidence.", - ].join("\n"); - - if (existingComment) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existingComment.id, - body, - }).catch(ignoreUnavailableWritePermission("comment update")); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pullRequest.number, - body, - }).catch(ignoreUnavailableWritePermission("comment creation")); - } - - await core.summary - .addHeading("Dependency Change Awareness") - .addRaw(`Detected ${dependencyFiles.length} dependency-related file change(s).`) - .addList(dependencyFiles.map((filename) => markdownCode(filename))) - .write(); - core.notice(`Detected ${dependencyFiles.length} dependency-related file change(s).`); + - name: Label, comment, and guard dependency changes + env: + GITHUB_TOKEN: ${{ github.token }} + OPENCLAW_SECURITY_APPROVERS: vincentkoc,steipete,joshavant + OPENCLAW_SECURITY_TEAM_SLUG: openclaw-secops + run: node scripts/github/dependency-change-awareness.mjs diff --git a/scripts/github/dependency-change-awareness.mjs b/scripts/github/dependency-change-awareness.mjs new file mode 100644 index 00000000000..ae7dab19af2 --- /dev/null +++ b/scripts/github/dependency-change-awareness.mjs @@ -0,0 +1,630 @@ +#!/usr/bin/env node + +import { appendFile, readFile } from "node:fs/promises"; + +export const dependencyChangeMarker = ""; +export const dependencyGraphGuardMarker = ""; +export const dependencyChangedLabel = "dependencies-changed"; +export const allowDependenciesCommand = "/allow-dependencies-change"; + +const maxListedFiles = 25; +const securityTeamSlug = process.env.OPENCLAW_SECURITY_TEAM_SLUG ?? "openclaw-secops"; +const dependencyManifestFields = [ + "dependencies", + "devDependencies", + "optionalDependencies", + "peerDependencies", + "peerDependenciesMeta", + "bundleDependencies", + "bundledDependencies", + "dependenciesMeta", + "overrides", + "resolutions", + "packageManager", + "workspaces", + "pnpm", + "name", + "version", + "engines", + "os", + "cpu", + "libc", +]; + +export function isDependencyFile(filename) { + return ( + filename.endsWith("package-lock.json") || + filename.endsWith("npm-shrinkwrap.json") || + filename.endsWith("pnpm-lock.yaml") || + filename === "pnpm-workspace.yaml" || + filename.startsWith("patches/") + ); +} + +export function isDependencyManifest(filename) { + return filename.endsWith("package.json"); +} + +export function isPackageLockfile(filename) { + return ( + filename.endsWith("pnpm-lock.yaml") || + filename.endsWith("package-lock.json") || + filename.endsWith("npm-shrinkwrap.json") + ); +} + +export function dependencyFieldChanges(baseManifest, headManifest) { + const changes = []; + for (const field of dependencyManifestFields) { + if (stableJson(baseManifest?.[field] ?? null) !== stableJson(headManifest?.[field] ?? null)) { + changes.push(field); + } + } + return changes; +} + +function stableJson(value) { + if (value === null || typeof value !== "object" || Array.isArray(value)) { + return JSON.stringify(value); + } + const sorted = {}; + for (const key of Object.keys(value).toSorted((left, right) => left.localeCompare(right))) { + sorted[key] = value[key]; + } + return JSON.stringify(sorted); +} + +export function sanitizeDisplayValue(value) { + return String(value).replace(/[\p{Cc}]/gu, "?").slice(0, 240); +} + +export function markdownCode(value) { + return `\`${sanitizeDisplayValue(value).replaceAll("`", "\\`")}\``; +} + +function shellQuote(value) { + return `'${sanitizeDisplayValue(value).replaceAll("'", "'\\''")}'`; +} + +export function findDependencyOverrideCommand({ + comments, + expectedSha, + isSecurityMember, + newerThan, +}) { + if (!expectedSha) { + return null; + } + const commandPattern = /^\/allow-dependencies-change(?:\s+(.+))?$/gimu; + for (const comment of comments.toReversed()) { + const body = comment.body ?? ""; + for (const match of body.matchAll(commandPattern)) { + const reason = match[1]?.trim(); + const login = comment.user?.login; + if (!login || !isCommentNewerThan(comment, newerThan)) { + continue; + } + if (isSecurityMember(login)) { + return { + login, + reason: reason ? sanitizeDisplayValue(reason) : null, + sha: expectedSha, + url: comment.html_url, + }; + } + } + } + return null; +} + +export async function findDependencyOverrideCommandAsync({ + comments, + expectedSha, + isSecurityMember, + newerThan, +}) { + if (!expectedSha) { + return null; + } + const commandPattern = /^\/allow-dependencies-change(?:\s+(.+))?$/gimu; + for (const comment of comments.toReversed()) { + const body = comment.body ?? ""; + for (const match of body.matchAll(commandPattern)) { + const reason = match[1]?.trim(); + const login = comment.user?.login; + if (!login || !isCommentNewerThan(comment, newerThan)) { + continue; + } + if (await isSecurityMember(login)) { + return { + login, + reason: reason ? sanitizeDisplayValue(reason) : null, + sha: expectedSha, + url: comment.html_url, + }; + } + } + } + return null; +} + +function isCommentNewerThan(comment, newerThan) { + if (!newerThan) { + return false; + } + const commentTime = Date.parse(comment.created_at ?? ""); + const barrierTime = Date.parse(newerThan); + return Number.isFinite(commentTime) && Number.isFinite(barrierTime) && commentTime > barrierTime; +} + +export function dependencyGuardCommentHeadSha(comment) { + const body = comment?.body ?? ""; + const patterns = [ + /Approved SHA:\s+`([a-f0-9]{40})`/iu, + /current head SHA\s+\(`([a-f0-9]{40})`\)/iu, + /Current SHA:\s+`([a-f0-9]{40})`/iu, + ]; + for (const pattern of patterns) { + const match = body.match(pattern); + if (match?.[1]) { + return match[1]; + } + } + return null; +} + +export function dependencyOverrideExpectedSha(existingGuardComment, currentHeadSha) { + if ( + !currentHeadSha || + existingGuardComment?.body?.includes("### Dependency graph changes are blocked") !== true + ) { + return null; + } + return dependencyGuardCommentHeadSha(existingGuardComment) === currentHeadSha + ? currentHeadSha + : null; +} + +export function isDependencyGuardAuthorizedForHead(comment, currentHeadSha) { + return ( + Boolean(currentHeadSha) && + comment?.body?.includes("### Dependency graph change authorized") === true && + dependencyGuardCommentHeadSha(comment) === currentHeadSha + ); +} + +export function securityApproverSet(value) { + return new Set( + String(value ?? "") + .split(/[\s,]+/u) + .map((login) => login.trim().toLowerCase()) + .filter(Boolean), + ); +} + +export function renderDependencyAwarenessComment(dependencyFiles) { + const listedFiles = dependencyFiles.slice(0, maxListedFiles); + const omittedCount = dependencyFiles.length - listedFiles.length; + const fileLines = listedFiles.map((filename) => `- ${markdownCode(filename)}`); + if (omittedCount > 0) { + fileLines.push(`- ${omittedCount} additional dependency-related files not shown`); + } + + return [ + dependencyChangeMarker, + "", + "### Dependency Changes Detected", + "", + "This PR changes dependency-related files. Maintainers should confirm these changes are intentional.", + "", + "Changed files:", + ...fileLines, + "", + "Maintainer follow-up:", + "- Review whether the dependency changes are intentional.", + "- Inspect resolved package deltas when lockfile, shrinkwrap, or workspace dependency policy changes are present.", + "- Treat `package-lock.json` and `npm-shrinkwrap.json` diffs as security-review surfaces.", + "- Run `pnpm deps:changes:report -- --base-ref origin/main --markdown /tmp/dependency-changes.md --json /tmp/dependency-changes.json` locally for detailed release-style evidence.", + ].join("\n"); +} + +export function renderAuthorizedDependencyComment(override) { + const lines = [ + dependencyGraphGuardMarker, + "", + "### Dependency graph change authorized", + "", + "This PR includes dependency graph changes. A member of `@openclaw/openclaw-secops` authorized this exact head SHA with `/allow-dependencies-change`.", + "", + `- Approved SHA: ${markdownCode(override.sha)}`, + `- Approved by: @${sanitizeDisplayValue(override.login)}`, + ]; + if (override.reason) { + lines.push(`- Reason: ${markdownCode(override.reason)}`); + } + lines.push( + "", + "A later push changes the PR head SHA and requires a fresh security approval.", + ); + return lines.join("\n"); +} + +export function renderClearedDependencyGuardComment({ headSha }) { + return [ + dependencyGraphGuardMarker, + "", + "### Dependency graph guard cleared", + "", + "This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh `/allow-dependencies-change` comment after the guard blocks that new head SHA.", + "", + `- Current SHA: ${markdownCode(headSha ?? "")}`, + ].join("\n"); +} + +export function renderBlockedDependencyComment({ + baseBranch, + headSha, + lockfileChanges, + dependencyManifestChanges, +}) { + const safeBranch = sanitizeDisplayValue(baseBranch ?? "main"); + const baseRef = shellQuote(`origin/${safeBranch}`); + const reasons = []; + for (const path of lockfileChanges) { + reasons.push(`- ${markdownCode(path)} changed.`); + } + for (const change of dependencyManifestChanges) { + reasons.push( + `- ${markdownCode(change.path)} changed ${change.fields.map(markdownCode).join(", ")}.`, + ); + } + const removalSteps = lockfileChanges.length > 0 + ? [ + "", + "To remove accidental lockfile residue, restore the lockfile changes from the target branch:", + "", + "```bash", + "git fetch origin", + `git checkout ${baseRef} -- ${lockfileChanges.map(shellQuote).join(" ")}`, + 'git commit -m "Remove dependency lockfile change"', + "git push", + "```", + ] + : []; + return [ + dependencyGraphGuardMarker, + "", + "### Dependency graph changes are blocked", + "", + "OpenClaw does not accept dependency graph changes through PRs unless security explicitly authorizes the current head SHA. Dependency updates are generated internally by maintainers so external PRs cannot accidentally or intentionally alter the resolved graph.", + "", + "Detected dependency graph changes:", + ...reasons, + ...removalSteps, + "", + "If this PR intentionally needs a dependency graph change, ask a member of `@openclaw/openclaw-secops` to comment:", + "", + "```text", + allowDependenciesCommand, + "```", + "", + `The action will approve the current head SHA (${markdownCode(headSha ?? "")}) when it reruns. A later push requires a fresh approval.`, + ].join("\n"); +} + +function githubApi(token) { + const baseHeaders = { + accept: "application/vnd.github+json", + authorization: `Bearer ${token}`, + "user-agent": "openclaw-dependency-change-awareness", + "x-github-api-version": "2022-11-28", + }; + const request = async (path, options = {}) => { + const response = await fetch(`https://api.github.com${path}`, { + ...options, + headers: { ...baseHeaders, ...options.headers }, + }); + if (response.status === 204) { + return null; + } + if (!response.ok) { + const error = new Error(`${response.status} ${response.statusText}: ${await response.text()}`); + error.status = response.status; + throw error; + } + return response.json(); + }; + return { + request, + paginate: async (path) => { + const items = []; + for (let page = 1; ; page += 1) { + const separator = path.includes("?") ? "&" : "?"; + const pageItems = await request(`${path}${separator}per_page=100&page=${page}`); + items.push(...pageItems); + if (pageItems.length < 100) { + return items; + } + } + }, + }; +} + +function decodeContentFile(payload) { + if (!payload || payload.type !== "file" || typeof payload.content !== "string") { + return null; + } + return Buffer.from(payload.content, payload.encoding ?? "base64").toString("utf8"); +} + +async function readJsonFileAtRef(api, { owner, repo, path, ref }) { + if (!ref) { + return null; + } + const encodedPath = path.split("/").map(encodeURIComponent).join("/"); + const payload = await api + .request(`/repos/${owner}/${repo}/contents/${encodedPath}?ref=${encodeURIComponent(ref)}`) + .catch((error) => { + if (error?.status === 404) { + return null; + } + throw error; + }); + const text = decodeContentFile(payload); + return text ? JSON.parse(text) : null; +} + +async function collectDependencyManifestChanges(api, { owner, repo, pullRequest, files }) { + const manifestPaths = files + .map((file) => file.filename) + .filter((filename) => typeof filename === "string" && isDependencyManifest(filename)) + .toSorted((left, right) => left.localeCompare(right)); + const changes = []; + for (const path of manifestPaths) { + const [baseManifest, headManifest] = await Promise.all([ + readJsonFileAtRef(api, { + owner, + repo, + path, + ref: pullRequest.base?.sha, + }), + readJsonFileAtRef(api, { + owner, + repo, + path, + ref: pullRequest.head?.sha, + }), + ]); + const fields = dependencyFieldChanges(baseManifest, headManifest); + if (fields.length > 0) { + changes.push({ path, fields }); + } + } + return changes; +} + +async function writeSummary(markdown) { + const summaryPath = process.env.GITHUB_STEP_SUMMARY; + if (!summaryPath) { + console.log(markdown); + return; + } + await appendFile(summaryPath, `${markdown}\n`); +} + +async function main() { + const token = process.env.GITHUB_TOKEN; + const eventPath = process.env.GITHUB_EVENT_PATH; + const repository = process.env.GITHUB_REPOSITORY; + if (!token || !eventPath || !repository) { + throw new Error("GITHUB_TOKEN, GITHUB_EVENT_PATH, and GITHUB_REPOSITORY are required."); + } + const [owner, repo] = repository.split("/"); + const event = JSON.parse(await readFile(eventPath, "utf8")); + const pullRequest = event.pull_request; + if (!pullRequest) { + console.log("No pull_request payload found; skipping."); + return; + } + + const api = githubApi(token); + const explicitSecurityApprovers = securityApproverSet(process.env.OPENCLAW_SECURITY_APPROVERS); + const issuePath = `/repos/${owner}/${repo}/issues/${pullRequest.number}`; + const pullPath = `/repos/${owner}/${repo}/pulls/${pullRequest.number}`; + const files = await api.paginate(`${pullPath}/files`); + const dependencyFiles = files + .map((file) => file.filename) + .filter((filename) => typeof filename === "string" && isDependencyFile(filename)) + .toSorted((left, right) => left.localeCompare(right)); + const lockfileChanges = dependencyFiles.filter(isPackageLockfile); + const dependencyManifestChanges = await collectDependencyManifestChanges(api, { + owner, + repo, + pullRequest, + files, + }); + const hasDependencyGraphChange = + lockfileChanges.length > 0 || dependencyManifestChanges.length > 0; + const dependencyGraphFiles = [ + ...dependencyFiles, + ...dependencyManifestChanges.map((change) => change.path), + ].toSorted((left, right) => left.localeCompare(right)); + + const [comments, labels] = await Promise.all([ + api.paginate(`${issuePath}/comments`), + api.paginate(`${issuePath}/labels`), + ]); + const findBotComment = (marker) => + comments.find( + (comment) => comment.user?.login === "github-actions[bot]" && comment.body?.includes(marker), + ); + const existingDependencyComment = findBotComment(dependencyChangeMarker); + const existingGuardComment = findBotComment(dependencyGraphGuardMarker); + const labelNames = new Set(labels.map((label) => label.name)); + + const ignoreUnavailableWritePermission = (action) => (error) => { + if (error?.status === 403) { + console.warn(`Skipping ${action}; token does not have write permission.`); + return; + } + if (error?.status === 404 || error?.status === 422) { + console.warn(`${action} is unavailable.`); + return; + } + throw error; + }; + const removeLabelIfPresent = async (label) => { + if (!labelNames.has(label)) { + return; + } + await api.request(`${issuePath}/labels/${encodeURIComponent(label)}`, { + method: "DELETE", + }).catch(ignoreUnavailableWritePermission(`label "${label}" removal`)); + }; + const addLabelIfMissing = async (label) => { + if (labelNames.has(label)) { + return; + } + await api.request(`${issuePath}/labels`, { + method: "POST", + body: JSON.stringify({ labels: [label] }), + }).catch(ignoreUnavailableWritePermission(`label "${label}" update`)); + }; + const deleteCommentIfPresent = async (comment) => { + if (!comment) { + return; + } + await api.request(`/repos/${owner}/${repo}/issues/comments/${comment.id}`, { + method: "DELETE", + }).catch(ignoreUnavailableWritePermission("comment deletion")); + }; + const upsertComment = async (comment, body) => { + if (comment) { + await api.request(`/repos/${owner}/${repo}/issues/comments/${comment.id}`, { + method: "PATCH", + body: JSON.stringify({ body }), + }).catch(ignoreUnavailableWritePermission("comment update")); + return; + } + await api.request(`${issuePath}/comments`, { + method: "POST", + body: JSON.stringify({ body }), + }).catch(ignoreUnavailableWritePermission("comment creation")); + }; + + if (dependencyGraphFiles.length === 0) { + await removeLabelIfPresent(dependencyChangedLabel); + await deleteCommentIfPresent(existingDependencyComment); + if (existingGuardComment) { + await upsertComment( + existingGuardComment, + renderClearedDependencyGuardComment({ headSha: pullRequest.head?.sha }), + ); + } + await writeSummary("## Dependency Change Awareness\n\nNo dependency-related file changes detected."); + console.log("No dependency-related file changes detected."); + return; + } + + await addLabelIfMissing(dependencyChangedLabel); + await upsertComment( + existingDependencyComment, + renderDependencyAwarenessComment(dependencyGraphFiles), + ); + await writeSummary( + [ + "## Dependency Change Awareness", + "", + `Detected ${dependencyGraphFiles.length} dependency-related file change(s).`, + "", + ...dependencyGraphFiles.map((filename) => `- ${markdownCode(filename)}`), + ].join("\n"), + ); + console.log(`Detected ${dependencyGraphFiles.length} dependency-related file change(s).`); + + if (!hasDependencyGraphChange) { + if (existingGuardComment) { + await upsertComment( + existingGuardComment, + renderClearedDependencyGuardComment({ headSha: pullRequest.head?.sha }), + ); + } + return; + } + + const membershipCache = new Map(); + const isSecurityMember = async (login) => { + const normalizedLogin = login.toLowerCase(); + if (explicitSecurityApprovers.size > 0) { + return explicitSecurityApprovers.has(normalizedLogin); + } + if (membershipCache.has(login)) { + return membershipCache.get(login); + } + try { + const membership = await api.request( + `/orgs/${owner}/teams/${securityTeamSlug}/memberships/${encodeURIComponent(login)}`, + ); + const allowed = membership?.state === "active"; + membershipCache.set(login, allowed); + return allowed; + } catch (error) { + if (error?.status !== 404) { + console.warn(`Could not verify ${login} against ${securityTeamSlug}: ${error.message}`); + } + membershipCache.set(login, false); + return false; + } + }; + const currentHeadSha = pullRequest.head?.sha; + if (isDependencyGuardAuthorizedForHead(existingGuardComment, currentHeadSha)) { + await writeSummary( + [ + "## Dependency Graph Guard", + "", + `Dependency graph change remains authorized for ${markdownCode(currentHeadSha)}.`, + ].join("\n"), + ); + console.log("Dependency graph change remains authorized for this head SHA."); + return; + } + const override = await findDependencyOverrideCommandAsync({ + comments, + expectedSha: dependencyOverrideExpectedSha(existingGuardComment, currentHeadSha), + isSecurityMember, + newerThan: existingGuardComment?.updated_at ?? existingGuardComment?.created_at, + }); + if (override) { + await upsertComment(existingGuardComment, renderAuthorizedDependencyComment(override)); + await writeSummary( + [ + "## Dependency Graph Guard", + "", + `Dependency graph change authorized by @${sanitizeDisplayValue(override.login)} for ${markdownCode(override.sha)}.`, + ].join("\n"), + ); + console.log("Dependency graph change authorized by security override."); + return; + } + + await upsertComment( + existingGuardComment, + renderBlockedDependencyComment({ + baseBranch: pullRequest.base?.ref ?? "main", + headSha: pullRequest.head?.sha, + lockfileChanges, + dependencyManifestChanges, + }), + ); + await writeSummary( + "## Dependency Graph Guard\n\nDependency graph changes are blocked without a current secops override.", + ); + throw new Error("Dependency graph changes require removal or a current secops override."); +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main().catch((error) => { + console.error(error instanceof Error ? error.message : error); + process.exitCode = 1; + }); +} diff --git a/test/scripts/dependency-change-awareness-script.test.ts b/test/scripts/dependency-change-awareness-script.test.ts new file mode 100644 index 00000000000..bdd3899d0b2 --- /dev/null +++ b/test/scripts/dependency-change-awareness-script.test.ts @@ -0,0 +1,279 @@ +import { describe, expect, it } from "vitest"; + +import { + dependencyGuardCommentHeadSha, + dependencyFieldChanges, + dependencyOverrideExpectedSha, + findDependencyOverrideCommand, + findDependencyOverrideCommandAsync, + isDependencyGuardAuthorizedForHead, + isDependencyFile, + isDependencyManifest, + isPackageLockfile, + renderAuthorizedDependencyComment, + renderBlockedDependencyComment, + renderClearedDependencyGuardComment, + sanitizeDisplayValue, + securityApproverSet, +} from "../../scripts/github/dependency-change-awareness.mjs"; + +const headSha = "a".repeat(40); +const staleSha = "b".repeat(40); + +describe("dependency change awareness script", () => { + it("detects dependency awareness file surfaces", () => { + expect(isDependencyFile("pnpm-lock.yaml")).toBe(true); + expect(isDependencyFile("package.json")).toBe(false); + expect(isDependencyFile("ui/package.json")).toBe(false); + expect(isDependencyFile("packages/core/package.json")).toBe(false); + expect(isDependencyFile("qa/convex-credential-broker/package.json")).toBe(false); + expect(isDependencyFile("extensions/slack/npm-shrinkwrap.json")).toBe(true); + expect(isDependencyFile("tools/nested/pnpm-lock.yaml")).toBe(true); + expect(isDependencyFile("src/index.ts")).toBe(false); + expect(isPackageLockfile("pnpm-lock.yaml")).toBe(true); + expect(isPackageLockfile("extensions/slack/npm-shrinkwrap.json")).toBe(true); + expect(isPackageLockfile("package.json")).toBe(false); + }); + + it("compares package manifest fields that can affect dependency resolution", () => { + expect(isDependencyManifest("package.json")).toBe(true); + expect(isDependencyManifest("extensions/slack/package.json")).toBe(true); + expect(isDependencyManifest("qa/convex-credential-broker/package.json")).toBe(true); + expect(isDependencyManifest("src/index.ts")).toBe(false); + expect( + dependencyFieldChanges( + { scripts: { test: "old" }, dependencies: { a: "1" } }, + { scripts: { test: "new" }, dependencies: { a: "1" } }, + ), + ).toEqual([]); + expect( + dependencyFieldChanges( + { dependencies: { a: "1" }, devDependencies: { b: "1" } }, + { dependencies: { a: "2" }, devDependencies: { b: "1", c: "1" } }, + ), + ).toEqual(["dependencies", "devDependencies"]); + expect( + dependencyFieldChanges( + { + optionalDependencies: { a: "1" }, + peerDependencies: { b: "1" }, + overrides: { c: "1" }, + packageManager: "pnpm@10.0.0", + pnpm: { patchedDependencies: { d: "patches/d.patch" } }, + scripts: { test: "old" }, + }, + { + optionalDependencies: { a: "2" }, + peerDependencies: { b: "2" }, + overrides: { c: "2" }, + packageManager: "pnpm@10.1.0", + pnpm: { patchedDependencies: { d: "patches/d2.patch" } }, + scripts: { test: "new" }, + }, + ), + ).toEqual([ + "optionalDependencies", + "peerDependencies", + "overrides", + "packageManager", + "pnpm", + ]); + }); + + it("accepts only security-member override commands for the current head sha", () => { + const comments = [ + { + body: "/allow-dependencies-change not enough", + created_at: "2026-05-28T20:00:00Z", + user: { login: "not-security" }, + }, + { + body: "/allow-dependencies-change stale approval", + created_at: "2026-05-28T20:01:00Z", + user: { login: "security-user" }, + }, + { + body: "/allow-dependencies-change reviewed dependency graph", + created_at: "2026-05-28T20:03:00Z", + html_url: "https://example.test/comment", + user: { login: "security-user" }, + }, + ]; + + const override = findDependencyOverrideCommand({ + comments, + expectedSha: headSha, + isSecurityMember: (login) => login === "security-user", + newerThan: "2026-05-28T20:02:00Z", + }); + + expect(override).toEqual({ + login: "security-user", + reason: "reviewed dependency graph", + sha: headSha, + url: "https://example.test/comment", + }); + }); + + it("rejects stale or non-security override commands", async () => { + const comments = [ + { + body: "/allow-dependencies-change stale approval", + created_at: "2026-05-28T20:00:00Z", + user: { login: "security-user" }, + }, + { + body: "/allow-dependencies-change not enough", + created_at: "2026-05-28T20:02:00Z", + user: { login: "not-security" }, + }, + ]; + + await expect( + findDependencyOverrideCommandAsync({ + comments, + expectedSha: headSha, + isSecurityMember: async (login) => login === "security-user", + newerThan: "2026-05-28T20:01:00Z", + }), + ).resolves.toBeNull(); + }); + + it("rejects override commands without a freshness barrier", () => { + const override = findDependencyOverrideCommand({ + comments: [ + { + body: "/allow-dependencies-change", + created_at: "2026-05-28T20:03:00Z", + user: { login: "security-user" }, + }, + ], + expectedSha: headSha, + isSecurityMember: (login) => login === "security-user", + }); + + expect(override).toBeNull(); + }); + + it("accepts override commands without a reason", () => { + const override = findDependencyOverrideCommand({ + comments: [ + { + body: "/allow-dependencies-change", + created_at: "2026-05-28T20:03:00Z", + user: { login: "security-user" }, + }, + ], + expectedSha: headSha, + isSecurityMember: (login) => login === "security-user", + newerThan: "2026-05-28T20:02:00Z", + }); + + expect(override).toEqual({ + login: "security-user", + reason: null, + sha: headSha, + url: undefined, + }); + }); + + it("binds override commands to the head sha in the blocked guard comment", () => { + const blockedComment = { + body: renderBlockedDependencyComment({ + baseBranch: "main", + headSha, + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [], + }), + }; + const staleBlockedComment = { + body: renderBlockedDependencyComment({ + baseBranch: "main", + headSha: staleSha, + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [], + }), + }; + + expect(dependencyGuardCommentHeadSha(blockedComment)).toBe(headSha); + expect(dependencyOverrideExpectedSha(blockedComment, headSha)).toBe(headSha); + expect(dependencyOverrideExpectedSha(staleBlockedComment, headSha)).toBeNull(); + }); + + it("preserves same-head authorization across reruns", () => { + const authorizedComment = { + body: renderAuthorizedDependencyComment({ + login: "security-user", + reason: null, + sha: headSha, + }), + }; + + expect(dependencyGuardCommentHeadSha(authorizedComment)).toBe(headSha); + expect(isDependencyGuardAuthorizedForHead(authorizedComment, headSha)).toBe(true); + expect(isDependencyGuardAuthorizedForHead(authorizedComment, staleSha)).toBe(false); + expect(dependencyOverrideExpectedSha(authorizedComment, headSha)).toBeNull(); + }); + + it("renders deterministic removal guidance for blocked lockfile changes", () => { + const body = renderBlockedDependencyComment({ + baseBranch: "main", + headSha, + lockfileChanges: ["pnpm-lock.yaml", "extensions/slack/npm-shrinkwrap.json"], + dependencyManifestChanges: [ + { + path: "package.json", + fields: ["dependencies"], + }, + ], + }); + + expect(body).toContain(""); + expect(body).toContain("Dependency graph changes are blocked"); + expect(body).toContain("`pnpm-lock.yaml` changed."); + expect(body).toContain("`extensions/slack/npm-shrinkwrap.json` changed."); + expect(body).toContain("`package.json` changed `dependencies`."); + expect(body).toContain( + "git checkout 'origin/main' -- 'pnpm-lock.yaml' 'extensions/slack/npm-shrinkwrap.json'", + ); + expect(body).toContain("/allow-dependencies-change"); + expect(body).toContain(`current head SHA (\`${headSha}\`)`); + expect(body).toContain("A later push requires a fresh approval."); + }); + + it("shell-quotes PR-controlled paths in removal guidance", () => { + const body = renderBlockedDependencyComment({ + baseBranch: "release/canary branch", + headSha, + lockfileChanges: [ + "dir with spaces/pnpm-lock.yaml", + "safe/quote'$(touch bad);/package-lock.json", + ], + dependencyManifestChanges: [], + }); + + expect(body).toContain( + "git checkout 'origin/release/canary branch' -- 'dir with spaces/pnpm-lock.yaml' 'safe/quote'\\''$(touch bad);/package-lock.json'", + ); + }); + + it("renders a cleared guard comment that preserves approval freshness", () => { + const body = renderClearedDependencyGuardComment({ headSha }); + + expect(body).toContain(""); + expect(body).toContain("Dependency graph guard cleared"); + expect(body).toContain(headSha); + expect(body).toContain("requires a fresh `/allow-dependencies-change` comment"); + }); + + it("parses explicit security approver allowlists", () => { + expect(securityApproverSet("vincentkoc, steipete\njoshavant")).toEqual( + new Set(["vincentkoc", "steipete", "joshavant"]), + ); + }); + + it("sanitizes display values", () => { + expect(sanitizeDisplayValue("abc\u0000def")).toBe("abc?def"); + expect(sanitizeDisplayValue("x".repeat(300))).toHaveLength(240); + }); +}); diff --git a/test/scripts/dependency-change-awareness-workflow.test.ts b/test/scripts/dependency-change-awareness-workflow.test.ts index cd917c48e55..9672df182ca 100644 --- a/test/scripts/dependency-change-awareness-workflow.test.ts +++ b/test/scripts/dependency-change-awareness-workflow.test.ts @@ -31,23 +31,22 @@ describe("dependency change awareness workflow", () => { const parsed = readWorkflow(); expect(workflow).toContain("pull_request_target:"); - expect(workflow).toContain("metadata-only workflow; no checkout or untrusted code execution"); + expect(workflow).toContain("checks trusted base script only; never checks out PR head"); expect(parsed.permissions).toEqual({ + contents: "read", "pull-requests": "write", issues: "write", }); }); - it("does not checkout or execute PR-controlled code", () => { + it("checks out only trusted base scripts and does not execute PR-controlled code", () => { const workflow = readFileSync(WORKFLOW, "utf8"); const forbiddenSnippets = [ - "actions/checkout", "github.event.pull_request.head", "pullRequest.head", "pnpm install", "npm install", "pnpm dlx", - "contents: write", "actions: write", "id-token: write", "secrets.", @@ -59,44 +58,53 @@ describe("dependency change awareness workflow", () => { } const steps = readWorkflow().jobs?.["dependency-change-awareness"]?.steps ?? []; - expect(steps).toHaveLength(1); - expect(steps[0].run).toBeUndefined(); + expect(steps).toHaveLength(2); + expect(steps[0].uses).toBe("actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd"); + expect(steps[0].with?.ref).toBe("${{ github.event.pull_request.base.sha }}"); + expect(steps[0].with?.["persist-credentials"]).toBe(false); + expect(steps[1].run).toBe("node scripts/github/dependency-change-awareness.mjs"); }); - it("uses a pinned GitHub Script action and bounded sticky comments", () => { + it("uses a dedicated checked-in script and bounded sticky comments", () => { const workflow = readFileSync(WORKFLOW, "utf8"); const steps = readWorkflow().jobs?.["dependency-change-awareness"]?.steps ?? []; - const step = steps[0]; + const runStep = steps[1]; + const script = readFileSync("scripts/github/dependency-change-awareness.mjs", "utf8"); - expect(step.uses).toBe("actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3"); - expect(step.with?.script).toContain(""); - expect(step.with?.script).toContain("const maxListedFiles = 25;"); - expect(step.with?.script).toContain("const sanitizeDisplayValue = (value)"); - expect(step.with?.script).toContain('.replace(/[\\u0000-\\u001f\\u007f]/gu, "?")'); - expect(step.with?.script).toContain(".slice(0, 240)"); - expect(step.with?.script).toContain('comment.user?.login === "github-actions[bot]"'); - expect(step.with?.script).toContain("github.rest.pulls.listFiles"); - expect(step.with?.script).toContain("github.rest.issues.createComment"); - expect(step.with?.script).toContain("github.rest.issues.updateComment"); - expect(step.with?.script).toContain("github.rest.issues.deleteComment"); - expect(step.with?.script).toContain("ignoreUnavailableWritePermission"); - expect(step.with?.script).toContain("error?.status === 403"); - expect(workflow).toContain('"dependencies-changed"'); + expect(runStep.env?.OPENCLAW_SECURITY_TEAM_SLUG).toBe("openclaw-secops"); + expect(runStep.env?.OPENCLAW_SECURITY_APPROVERS).toBe("vincentkoc,steipete,joshavant"); + expect(workflow).toContain("scripts/github/dependency-change-awareness.mjs"); + expect(script).toContain('"dependencies-changed"'); + expect(script).not.toContain('"blocked: dependencies"'); }); it("detects the intended dependency-related file surfaces", () => { - const script = readWorkflow().jobs?.["dependency-change-awareness"]?.steps?.[0].with?.script; - expect(script).toContain('filename === "package.json"'); - expect(script).toContain('filename === "package-lock.json"'); - expect(script).toContain('filename === "npm-shrinkwrap.json"'); - expect(script).toContain('filename === "pnpm-lock.yaml"'); + const script = readFileSync("scripts/github/dependency-change-awareness.mjs", "utf8"); + expect(script).toContain('filename.endsWith("package.json")'); + expect(script).toContain('filename.endsWith("package-lock.json")'); + expect(script).toContain('filename.endsWith("npm-shrinkwrap.json")'); + expect(script).toContain('filename.endsWith("pnpm-lock.yaml")'); expect(script).toContain('filename === "pnpm-workspace.yaml"'); - expect(script).toContain('filename === "ui/package.json"'); expect(script).toContain('filename.startsWith("patches/")'); - expect(script).toContain("^packages\\/[^/]+\\/package\\.json$"); - expect(script).toContain("^extensions\\/[^/]+\\/package-lock\\.json$"); - expect(script).toContain("^extensions\\/[^/]+\\/npm-shrinkwrap\\.json$"); - expect(script).toContain("^extensions\\/[^/]+\\/package\\.json$"); + expect(script).toContain("dependencyGraphFiles"); + }); + + it("blocks package lockfile and manifest graph changes unless secops approves the current head sha", () => { + const script = readFileSync("scripts/github/dependency-change-awareness.mjs", "utf8"); + expect(script).toContain('filename.endsWith("pnpm-lock.yaml")'); + expect(script).toContain('filename.endsWith("package-lock.json")'); + expect(script).toContain('filename.endsWith("npm-shrinkwrap.json")'); + expect(script).toContain('"optionalDependencies"'); + expect(script).toContain('"peerDependencies"'); + expect(script).toContain('"overrides"'); + expect(script).toContain('"packageManager"'); + expect(script).toContain("/allow-dependencies-change"); + expect(script).toContain("openclaw-secops"); + expect(script).toContain("securityApproverSet"); + expect(script).toContain("/memberships/"); + expect(script).toContain("isCommentNewerThan"); + expect(script).toContain("A later push requires a fresh approval."); + expect(script).toContain("process.exitCode = 1"); }); it("requires secops review for future workflow or guard changes", () => { @@ -107,6 +115,9 @@ describe("dependency change awareness workflow", () => { expect(codeowners).toContain( "/test/scripts/dependency-change-awareness-workflow.test.ts @openclaw/openclaw-secops", ); + expect(codeowners).toContain( + "/scripts/github/dependency-change-awareness.mjs @openclaw/openclaw-secops", + ); expect(codeowners).toContain("/package-lock.json @openclaw/openclaw-secops"); expect(codeowners).toContain("/npm-shrinkwrap.json @openclaw/openclaw-secops"); expect(codeowners).toContain("/extensions/*/package-lock.json @openclaw/openclaw-secops");