From 29b32050c1baa8af59899537f1adac807a2fdc81 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Fri, 29 May 2026 19:37:16 -0700 Subject: [PATCH] feat(ci): autoscrub dependency lockfile-only PR changes (#87796) * ci: autoscrub dependency lockfile residue * ci: harden dependency autoscrub commits * ci: scope dependency autoscrub tokens * ci: split autoscrub base reads * ci: expand autoscrub proof comment --- .github/workflows/dependency-guard.yml | 80 +++- scripts/github/dependency-guard.mjs | 383 ++++++++++++++++-- test/scripts/dependency-guard-script.test.ts | 256 +++++++++++- .../scripts/dependency-guard-workflow.test.ts | 112 ++++- 4 files changed, 797 insertions(+), 34 deletions(-) diff --git a/.github/workflows/dependency-guard.yml b/.github/workflows/dependency-guard.yml index 289ee1c1fa0..ea076978dd2 100644 --- a/.github/workflows/dependency-guard.yml +++ b/.github/workflows/dependency-guard.yml @@ -14,10 +14,85 @@ concurrency: cancel-in-progress: true jobs: - dependency-guard: + dependency-guard-detect: if: ${{ !github.event.pull_request.draft }} runs-on: ubuntu-24.04 timeout-minutes: 5 + outputs: + autoscrub: ${{ steps.guard.outputs.autoscrub }} + autoscrub-owner: ${{ steps.guard.outputs.autoscrub-owner }} + autoscrub-repository: ${{ steps.guard.outputs.autoscrub-repository }} + steps: + - name: Check out trusted base workflow scripts + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + + - name: Detect dependency changes + id: guard + env: + GITHUB_TOKEN: ${{ github.token }} + OPENCLAW_DEPENDENCY_GUARD_MODE: detect + OPENCLAW_SECURITY_APPROVERS: vincentkoc,steipete,joshavant + OPENCLAW_SECURITY_TEAM_SLUG: openclaw-secops + run: node scripts/github/dependency-guard.mjs + + dependency-guard-autoscrub: + if: ${{ !github.event.pull_request.draft && needs.dependency-guard-detect.outputs.autoscrub == 'true' }} + needs: dependency-guard-detect + runs-on: ubuntu-24.04 + timeout-minutes: 5 + permissions: + contents: read + issues: write + pull-requests: read + steps: + - name: Check out trusted base workflow scripts + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + + - name: Create autoscrub app token + id: app-token + continue-on-error: true + uses: actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3 # v3.1.1 + with: + app-id: "2729701" + private-key: ${{ secrets.GH_APP_PRIVATE_KEY }} + owner: ${{ needs.dependency-guard-detect.outputs.autoscrub-owner }} + repositories: ${{ needs.dependency-guard-detect.outputs.autoscrub-repository }} + permission-contents: write + + - name: Create fallback autoscrub app token + id: app-token-fallback + continue-on-error: true + if: steps.app-token.outcome == 'failure' + uses: actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3 # v3.1.1 + with: + app-id: "2971289" + private-key: ${{ secrets.GH_APP_PRIVATE_KEY_FALLBACK }} + owner: ${{ needs.dependency-guard-detect.outputs.autoscrub-owner }} + repositories: ${{ needs.dependency-guard-detect.outputs.autoscrub-repository }} + permission-contents: write + + - name: Remove package lockfile changes + env: + GITHUB_TOKEN: ${{ github.token }} + OPENCLAW_DEPENDENCY_GUARD_AUTOSCRUB_TOKEN: ${{ steps.app-token.outputs.token || steps.app-token-fallback.outputs.token }} + OPENCLAW_DEPENDENCY_GUARD_MODE: autoscrub + OPENCLAW_SECURITY_APPROVERS: vincentkoc,steipete,joshavant + OPENCLAW_SECURITY_TEAM_SLUG: openclaw-secops + run: node scripts/github/dependency-guard.mjs + + dependency-guard: + if: ${{ !github.event.pull_request.draft && always() }} + needs: + - dependency-guard-detect + - dependency-guard-autoscrub + runs-on: ubuntu-24.04 + timeout-minutes: 5 steps: - name: Check out trusted base workflow scripts uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -25,9 +100,10 @@ jobs: ref: ${{ github.event.pull_request.base.sha }} persist-credentials: false - - name: Label, comment, and guard dependency changes + - name: Enforce dependency guard env: GITHUB_TOKEN: ${{ github.token }} + OPENCLAW_DEPENDENCY_GUARD_MODE: enforce OPENCLAW_SECURITY_APPROVERS: vincentkoc,steipete,joshavant OPENCLAW_SECURITY_TEAM_SLUG: openclaw-secops run: node scripts/github/dependency-guard.mjs diff --git a/scripts/github/dependency-guard.mjs b/scripts/github/dependency-guard.mjs index d9884425780..b0e370804c6 100644 --- a/scripts/github/dependency-guard.mjs +++ b/scripts/github/dependency-guard.mjs @@ -10,6 +10,7 @@ export const allowDependenciesCommand = "/allow-dependencies-change"; export const GITHUB_ERROR_BODY_MAX_BYTES = 64 * 1024; const maxListedFiles = 25; +const autoscrubCommitMessage = "chore: remove dependency lockfile change"; const securityTeamSlug = process.env.OPENCLAW_SECURITY_TEAM_SLUG ?? "openclaw-secops"; const dependencyManifestFields = [ "dependencies", @@ -65,6 +66,46 @@ export function dependencyFieldChanges(baseManifest, headManifest) { return changes; } +export function shouldAutoscrubDependencyLockfiles({ + dependencyFiles = [], + lockfileChanges, + dependencyManifestChanges = [], +}) { + return ( + lockfileChanges.length > 0 && + dependencyManifestChanges.length === 0 && + dependencyFiles.every(isPackageLockfile) + ); +} + +export function canAutoscrubPullRequest({ owner, repo, pullRequest }) { + return autoscrubTargetRepository({ owner, repo, pullRequest }) !== null; +} + +function autoscrubTargetRepository({ owner, repo, pullRequest }) { + const baseRepository = `${owner}/${repo}`; + const headRepository = pullRequest.head?.repo; + const headRepositoryName = headRepository?.full_name; + if ( + typeof pullRequest.head?.ref === "string" && + pullRequest.head.ref.length > 0 && + typeof pullRequest.head?.sha === "string" && + pullRequest.head.sha.length > 0 + ) { + if (headRepositoryName === baseRepository) { + return { owner, repo }; + } + + if (pullRequest.maintainer_can_modify === true && typeof headRepositoryName === "string") { + const [headOwner, headRepo] = headRepositoryName.split("/"); + if (headOwner && headRepo) { + return { owner: headOwner, repo: headRepo }; + } + } + } + return null; +} + function stableJson(value) { if (value === null || typeof value !== "object" || Array.isArray(value)) { return JSON.stringify(value); @@ -190,6 +231,20 @@ export function securityApproverSet(value) { ); } +export function dependencyGuardCommentAuthors(value) { + return new Set( + String(value ?? "github-actions[bot]") + .split(/[\s,]+/u) + .map((login) => login.trim().toLowerCase()) + .filter(Boolean), + ); +} + +export function isDependencyGuardMarkerComment(comment, marker, trustedAuthors) { + const login = comment.user?.login?.toLowerCase(); + return Boolean(login && trustedAuthors.has(login) && comment.body?.includes(marker)); +} + export function renderDependencyAwarenessComment(dependencyFiles) { const listedFiles = dependencyFiles.slice(0, maxListedFiles); const omittedCount = dependencyFiles.length - listedFiles.length; @@ -201,7 +256,7 @@ export function renderDependencyAwarenessComment(dependencyFiles) { return [ dependencyChangeMarker, "", - "### Dependency Changes Detected", + "### Dependency Guard", "", "This PR changes dependency-related files. Maintainers should confirm these changes are intentional.", "", @@ -234,6 +289,30 @@ export function renderAuthorizedDependencyComment(override) { return lines.join("\n"); } +export function renderAutoscrubbedDependencyComment({ baseBranch, lockfileChanges, commitSha }) { + const safeBranch = sanitizeDisplayValue(baseBranch ?? "main"); + const fileLines = lockfileChanges.map((path) => `- ${markdownCode(path)}`); + return `${dependencyGraphGuardMarker} + +### Dependency lockfile changes were removed + +OpenClaw does not accept package lockfile changes through PRs. This PR did not change dependency graph fields in package manifests, so the workflow restored the lockfile residue from the target branch automatically. + +Restored lockfiles: +${fileLines.join("\n")} + +- Target branch: ${markdownCode(safeBranch)} +- Cleanup commit: ${markdownCode(commitSha)} +- Workflow action: restored each listed lockfile from the target branch and pushed the cleanup commit to this PR head. +- Verification result: this PR no longer carries those package lockfile diffs after the cleanup commit. + +No action is needed unless this PR intentionally requires a dependency update. If it does, mention that in the PR and a maintainer will handle the dependency update internally.`; +} + +export function isAutoscrubbedDependencyComment(comment) { + return comment?.body?.includes("### Dependency lockfile changes were removed") === true; +} + export function renderClearedDependencyGuardComment({ headSha }) { return [ dependencyGraphGuardMarker, @@ -251,6 +330,7 @@ export function renderBlockedDependencyComment({ headSha, lockfileChanges, dependencyManifestChanges, + autoscrubStatus, }) { const safeBranch = sanitizeDisplayValue(baseBranch ?? "main"); const baseRef = shellQuote(`origin/${safeBranch}`); @@ -259,20 +339,19 @@ export function renderBlockedDependencyComment({ reasons.push(`- ${markdownCode(path)} changed.`); } for (const change of dependencyManifestChanges) { - reasons.push( - `- ${markdownCode(change.path)} changed ${change.fields.map(markdownCode).join(", ")}.`, - ); + reasons.push(renderManifestChangeLine(change)); } + const autoscrubLines = renderAutoscrubStatusLines(autoscrubStatus); const removalSteps = lockfileChanges.length > 0 ? [ "", - "To remove accidental lockfile residue, restore the lockfile changes from the target branch:", + "To remove lockfile changes, restore them from the target branch:", "", "```bash", "git fetch origin", `git checkout ${baseRef} -- ${lockfileChanges.map(shellQuote).join(" ")}`, - 'git commit -m "Remove dependency lockfile change"', + `git commit -m ${shellQuote(autoscrubCommitMessage)}`, "git push", "```", ] @@ -282,10 +361,11 @@ export function renderBlockedDependencyComment({ "", "### 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.", + "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 change the resolved graph.", "", "Detected dependency graph changes:", ...reasons, + ...autoscrubLines, ...removalSteps, "", "If this PR intentionally needs a dependency graph change, ask a member of `@openclaw/openclaw-secops` to comment:", @@ -298,6 +378,47 @@ export function renderBlockedDependencyComment({ ].join("\n"); } +function renderAutoscrubStatusLines(status) { + if (!status) { + return []; + } + if (status.kind === "not-attempted") { + return [ + "", + "Auto-scrub was not attempted because this workflow can only push deterministic cleanup commits to PR branches that maintainers can modify. Please remove the lockfile changes manually.", + ]; + } + if (status.kind === "blocked-by-dependency-manifest-fields") { + return [ + "", + "Auto-scrub was not attempted because this PR changes package manifest dependency graph fields:", + ...status.changes.map(renderManifestChangeLine), + "", + "Dependency graph changes must be reviewed by security or handled by maintainers internally. Please remove lockfile changes manually if they are not needed.", + ]; + } + if (status.kind === "blocked-by-other-dependency-files") { + return [ + "", + "Auto-scrub was not attempted because this PR also changes dependency-related files that are not package lockfiles:", + ...status.files.map((path) => `- ${markdownCode(path)}`), + "", + "Please remove lockfile changes manually if they are not needed.", + ]; + } + if (status.kind === "failed") { + return [ + "", + `Auto-scrub was attempted, but GitHub rejected the cleanup commit: ${markdownCode(status.reason)}. Please remove the lockfile changes manually.`, + ]; + } + return []; +} + +function renderManifestChangeLine(change) { + return `- ${markdownCode(change.path)} changed ${change.fields.map(markdownCode).join(", ")}.`; +} + function githubErrorBodyTooLarge(maxBytes) { return new Error(`GitHub error response body exceeded ${maxBytes} bytes`); } @@ -338,6 +459,21 @@ export function githubApi(token) { }; return { request, + graphql: async (query, variables) => { + const result = await request("/graphql", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ query, variables }), + }); + if (Array.isArray(result.errors) && result.errors.length > 0) { + const error = new Error( + result.errors.map((entry) => entry.message ?? "GraphQL error").join("; "), + ); + error.errors = result.errors; + throw error; + } + return result.data; + }, paginate: async (path) => { const items = []; for (let page = 1; ; page += 1) { @@ -376,6 +512,38 @@ async function readJsonFileAtRef(api, { owner, repo, path, ref }) { return text ? JSON.parse(text) : null; } +async function readContentFileMetadataAtRef(api, { owner, repo, path, ref }) { + if (!ref) { + return null; + } + const encodedPath = path.split("/").map(encodeURIComponent).join("/"); + return api + .request(`/repos/${owner}/${repo}/contents/${encodedPath}?ref=${encodeURIComponent(ref)}`) + .catch((error) => { + if (error?.status === 404) { + return null; + } + throw error; + }); +} + +async function readBase64FileAtRef(api, { owner, repo, path, ref }) { + const file = await readContentFileMetadataAtRef(api, { owner, repo, path, ref }); + if (!file) { + return null; + } + if (file.encoding === "base64" && typeof file.content === "string" && file.content.length > 0) { + return file.content.replace(/\s+/gu, ""); + } + if (typeof file.sha === "string" && file.sha.length > 0) { + const blob = await api.request(`/repos/${owner}/${repo}/git/blobs/${file.sha}`); + if (blob.encoding === "base64" && typeof blob.content === "string" && blob.content.length > 0) { + return blob.content.replace(/\s+/gu, ""); + } + } + throw new Error(`Unable to read base64 file contents for ${path}`); +} + async function collectDependencyManifestChanges(api, { owner, repo, pullRequest, files }) { const manifestPaths = files .map((file) => file.filename) @@ -405,6 +573,52 @@ async function collectDependencyManifestChanges(api, { owner, repo, pullRequest, return changes; } +export async function createAutoscrubCommit( + { baseApi, writeApi }, + { owner, repo, pullRequest, lockfileChanges, targetRepository }, +) { + const headSha = pullRequest.head.sha; + const headRef = pullRequest.head.ref; + const writeOwner = targetRepository.owner; + const writeRepo = targetRepository.repo; + const additions = []; + const deletions = []; + for (const path of lockfileChanges) { + const contents = await readBase64FileAtRef(baseApi, { + owner, + repo, + path, + ref: pullRequest.base?.sha, + }); + if (contents) { + additions.push({ path, contents }); + } else { + deletions.push({ path }); + } + } + const data = await writeApi.graphql( + `mutation CreateAutoscrubCommit($input: CreateCommitOnBranchInput!) { + createCommitOnBranch(input: $input) { + commit { + oid + } + } + }`, + { + input: { + branch: { + repositoryNameWithOwner: `${writeOwner}/${writeRepo}`, + branchName: headRef, + }, + expectedHeadOid: headSha, + fileChanges: { additions, deletions }, + message: { headline: autoscrubCommitMessage }, + }, + }, + ); + return { sha: data.createCommitOnBranch.commit.oid }; +} + async function writeSummary(markdown) { const summaryPath = process.env.GITHUB_STEP_SUMMARY; if (!summaryPath) { @@ -414,6 +628,14 @@ async function writeSummary(markdown) { await appendFile(summaryPath, `${markdown}\n`); } +async function setOutput(name, value) { + const outputPath = process.env.GITHUB_OUTPUT; + if (!outputPath) { + return; + } + await appendFile(outputPath, `${name}=${value}\n`); +} + async function main() { const token = process.env.GITHUB_TOKEN; const eventPath = process.env.GITHUB_EVENT_PATH; @@ -423,16 +645,23 @@ async function main() { } const [owner, repo] = repository.split("/"); const event = JSON.parse(await readFile(eventPath, "utf8")); - const pullRequest = event.pull_request; - if (!pullRequest) { + const eventPullRequest = event.pull_request; + if (!eventPullRequest) { console.log("No pull_request payload found; skipping."); return; } const api = githubApi(token); + const autoscrubToken = process.env.OPENCLAW_DEPENDENCY_GUARD_AUTOSCRUB_TOKEN; + const autoscrubApi = autoscrubToken ? githubApi(autoscrubToken) : null; 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 trustedCommentAuthors = dependencyGuardCommentAuthors( + process.env.OPENCLAW_DEPENDENCY_GUARD_COMMENT_BOTS, + ); + const issuePath = `/repos/${owner}/${repo}/issues/${eventPullRequest.number}`; + const pullPath = `/repos/${owner}/${repo}/pulls/${eventPullRequest.number}`; + const pullRequest = await api.request(pullPath); + const mode = process.env.OPENCLAW_DEPENDENCY_GUARD_MODE ?? "enforce"; const files = await api.paginate(`${pullPath}/files`); const dependencyFiles = files .map((file) => file.filename) @@ -456,12 +685,12 @@ async function main() { 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 findDependencyGuardComment = (marker) => + comments.find((comment) => + isDependencyGuardMarkerComment(comment, marker, trustedCommentAuthors), ); - const existingDependencyComment = findBotComment(dependencyChangeMarker); - const existingGuardComment = findBotComment(dependencyGraphGuardMarker); + let dependencyComment = findDependencyGuardComment(dependencyChangeMarker); + const existingGuardComment = findDependencyGuardComment(dependencyGraphGuardMarker); const labelNames = new Set(labels.map((label) => label.name)); const ignoreUnavailableWritePermission = (action) => (error) => { @@ -484,6 +713,7 @@ async function main() { method: "DELETE", }) .catch(ignoreUnavailableWritePermission(`label "${label}" removal`)); + labelNames.delete(label); }; const addLabelIfMissing = async (label) => { if (labelNames.has(label)) { @@ -495,6 +725,7 @@ async function main() { body: JSON.stringify({ labels: [label] }), }) .catch(ignoreUnavailableWritePermission(`label "${label}" update`)); + labelNames.add(label); }; const deleteCommentIfPresent = async (comment) => { if (!comment) { @@ -508,15 +739,14 @@ async function main() { }; const upsertComment = async (comment, body) => { if (comment) { - await api + return await api .request(`/repos/${owner}/${repo}/issues/comments/${comment.id}`, { method: "PATCH", body: JSON.stringify({ body }), }) .catch(ignoreUnavailableWritePermission("comment update")); - return; } - await api + return await api .request(`${issuePath}/comments`, { method: "POST", body: JSON.stringify({ body }), @@ -526,8 +756,8 @@ async function main() { if (dependencyGraphFiles.length === 0) { await removeLabelIfPresent(dependencyChangedLabel); - await deleteCommentIfPresent(existingDependencyComment); - if (existingGuardComment) { + await deleteCommentIfPresent(dependencyComment); + if (existingGuardComment && !isAutoscrubbedDependencyComment(existingGuardComment)) { await upsertComment( existingGuardComment, renderClearedDependencyGuardComment({ headSha: pullRequest.head?.sha }), @@ -539,8 +769,8 @@ async function main() { } await addLabelIfMissing(dependencyChangedLabel); - await upsertComment( - existingDependencyComment, + dependencyComment = await upsertComment( + dependencyComment, renderDependencyAwarenessComment(dependencyGraphFiles), ); await writeSummary( @@ -555,7 +785,7 @@ async function main() { console.log(`Detected ${dependencyGraphFiles.length} dependency-related file change(s).`); if (!hasDependencyGraphChange) { - if (existingGuardComment) { + if (existingGuardComment && !isAutoscrubbedDependencyComment(existingGuardComment)) { await upsertComment( existingGuardComment, renderClearedDependencyGuardComment({ headSha: pullRequest.head?.sha }), @@ -564,6 +794,104 @@ async function main() { return; } + const autoscrubCandidate = shouldAutoscrubDependencyLockfiles({ + dependencyFiles, + lockfileChanges, + dependencyManifestChanges, + }); + const autoscrubTarget = autoscrubCandidate + ? autoscrubTargetRepository({ owner, repo, pullRequest }) + : null; + if (mode === "detect" && autoscrubTarget) { + await setOutput("autoscrub", "true"); + await setOutput("autoscrub-owner", autoscrubTarget.owner); + await setOutput("autoscrub-repository", autoscrubTarget.repo); + await writeSummary( + [ + "## Dependency Guard", + "", + `Detected ${lockfileChanges.length} autoscrubbable package lockfile change(s).`, + "", + ...lockfileChanges.map((filename) => `- ${markdownCode(filename)}`), + ].join("\n"), + ); + console.log("Detected autoscrubbable package lockfile changes."); + return; + } + if (mode === "detect") { + await setOutput("autoscrub", "false"); + await writeSummary( + "## Dependency Guard\n\nDependency graph enforcement deferred to the final guard job.", + ); + console.log("Dependency graph enforcement deferred to the final guard job."); + return; + } + + let autoscrubStatus = null; + if (mode === "autoscrub") { + if (autoscrubTarget) { + try { + if (!autoscrubApi) { + throw new Error("autoscrub app token was unavailable"); + } + const commit = await createAutoscrubCommit( + { baseApi: api, writeApi: autoscrubApi }, + { + owner, + repo, + pullRequest, + lockfileChanges, + targetRepository: autoscrubTarget, + }, + ); + await removeLabelIfPresent(dependencyChangedLabel); + await deleteCommentIfPresent(dependencyComment); + await upsertComment( + existingGuardComment, + renderAutoscrubbedDependencyComment({ + baseBranch: pullRequest.base?.ref ?? "main", + lockfileChanges, + commitSha: commit.sha, + }), + ); + await writeSummary( + [ + "## Dependency Guard", + "", + `Removed ${lockfileChanges.length} package lockfile change(s) in ${markdownCode(commit.sha)}.`, + "", + ...lockfileChanges.map((filename) => `- ${markdownCode(filename)}`), + ].join("\n"), + ); + console.log("Removed package lockfile changes with an autoscrub commit."); + return; + } catch (error) { + autoscrubStatus = { + kind: "failed", + reason: error instanceof Error ? error.message : String(error), + }; + console.warn(`Autoscrub failed: ${autoscrubStatus.reason}`); + } + } else { + autoscrubStatus = { kind: "not-attempted" }; + } + } else if (autoscrubCandidate && !autoscrubTarget) { + autoscrubStatus = { kind: "not-attempted" }; + } else if (lockfileChanges.length > 0 && dependencyManifestChanges.length > 0) { + autoscrubStatus = { + kind: "blocked-by-dependency-manifest-fields", + changes: dependencyManifestChanges, + }; + } else if (lockfileChanges.length > 0) { + const nonLockfileDependencyFiles = dependencyFiles.filter((path) => !isPackageLockfile(path)); + if (nonLockfileDependencyFiles.length > 0) { + autoscrubStatus = { + kind: "blocked-by-other-dependency-files", + files: nonLockfileDependencyFiles, + }; + } + } + const membershipCache = new Map(); const isSecurityMember = async (login) => { const normalizedLogin = login.toLowerCase(); @@ -592,7 +920,7 @@ async function main() { if (isDependencyGuardAuthorizedForHead(existingGuardComment, currentHeadSha)) { await writeSummary( [ - "## Dependency Graph Guard", + "## Dependency Guard", "", `Dependency graph change remains authorized for ${markdownCode(currentHeadSha)}.`, ].join("\n"), @@ -610,7 +938,7 @@ async function main() { await upsertComment(existingGuardComment, renderAuthorizedDependencyComment(override)); await writeSummary( [ - "## Dependency Graph Guard", + "## Dependency Guard", "", `Dependency graph change authorized by @${sanitizeDisplayValue(override.login)} for ${markdownCode(override.sha)}.`, ].join("\n"), @@ -626,10 +954,11 @@ async function main() { headSha: pullRequest.head?.sha, lockfileChanges, dependencyManifestChanges, + autoscrubStatus, }), ); await writeSummary( - "## Dependency Graph Guard\n\nDependency graph changes are blocked without a current secops override.", + "## Dependency 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."); } diff --git a/test/scripts/dependency-guard-script.test.ts b/test/scripts/dependency-guard-script.test.ts index 8750c8043ee..8d5267f04de 100644 --- a/test/scripts/dependency-guard-script.test.ts +++ b/test/scripts/dependency-guard-script.test.ts @@ -1,29 +1,36 @@ import { describe, expect, it } from "vitest"; import { GITHUB_ERROR_BODY_MAX_BYTES, + canAutoscrubPullRequest, + createAutoscrubCommit, + dependencyGuardCommentAuthors, dependencyGuardCommentHeadSha, dependencyFieldChanges, dependencyOverrideExpectedSha, findDependencyOverrideCommand, findDependencyOverrideCommandAsync, githubApi, + isAutoscrubbedDependencyComment, isDependencyGuardAuthorizedForHead, isDependencyFile, + isDependencyGuardMarkerComment, isDependencyManifest, isPackageLockfile, readBoundedGitHubErrorText, renderAuthorizedDependencyComment, + renderAutoscrubbedDependencyComment, renderBlockedDependencyComment, renderClearedDependencyGuardComment, sanitizeDisplayValue, securityApproverSet, + shouldAutoscrubDependencyLockfiles, } from "../../scripts/github/dependency-guard.mjs"; const headSha = "a".repeat(40); const staleSha = "b".repeat(40); describe("dependency guard script", () => { - it("detects dependency awareness file surfaces", () => { + it("detects dependency guard file surfaces", () => { expect(isDependencyFile("pnpm-lock.yaml")).toBe(true); expect(isDependencyFile("package.json")).toBe(false); expect(isDependencyFile("ui/package.json")).toBe(false); @@ -211,6 +218,43 @@ describe("dependency guard script", () => { expect(dependencyOverrideExpectedSha(authorizedComment, headSha)).toBeNull(); }); + it("trusts only configured dependency guard marker comment authors", () => { + const trustedAuthors = dependencyGuardCommentAuthors( + "github-actions[bot], openclaw-autoscrub[bot]", + ); + + expect( + isDependencyGuardMarkerComment( + { + body: "", + user: { login: "openclaw-autoscrub[bot]" }, + }, + "", + trustedAuthors, + ), + ).toBe(true); + expect( + isDependencyGuardMarkerComment( + { + body: "", + user: { login: "contributor" }, + }, + "", + trustedAuthors, + ), + ).toBe(false); + expect( + isDependencyGuardMarkerComment( + { + body: "no marker", + user: { login: "github-actions[bot]" }, + }, + "", + trustedAuthors, + ), + ).toBe(false); + }); + it("renders deterministic removal guidance for blocked lockfile changes", () => { const body = renderBlockedDependencyComment({ baseBranch: "main", @@ -253,6 +297,216 @@ describe("dependency guard script", () => { ); }); + it("autoscrubs only lockfile changes with no dependency manifest changes", () => { + expect( + shouldAutoscrubDependencyLockfiles({ + dependencyFiles: ["pnpm-lock.yaml"], + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [], + }), + ).toBe(true); + expect( + shouldAutoscrubDependencyLockfiles({ + dependencyFiles: ["pnpm-lock.yaml"], + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [{ path: "package.json", fields: ["dependencies"] }], + }), + ).toBe(false); + expect( + shouldAutoscrubDependencyLockfiles({ + dependencyFiles: [], + lockfileChanges: [], + dependencyManifestChanges: [], + }), + ).toBe(false); + expect( + shouldAutoscrubDependencyLockfiles({ + dependencyFiles: ["pnpm-lock.yaml", "patches/example.patch"], + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [], + }), + ).toBe(false); + expect( + shouldAutoscrubDependencyLockfiles({ + dependencyFiles: ["pnpm-lock.yaml", "pnpm-workspace.yaml"], + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [], + }), + ).toBe(false); + }); + + it("attempts autoscrub on PR branches maintainers can modify", () => { + const sameRepoPullRequest = { + head: { + ref: "contributor/change", + repo: { full_name: "openclaw/openclaw" }, + sha: headSha, + }, + }; + const forkPullRequest = { + head: { + ref: "contributor/change", + repo: { full_name: "external/openclaw" }, + sha: headSha, + }, + }; + const editableForkPullRequest = { + maintainer_can_modify: true, + head: { + ref: "contributor/change", + repo: { full_name: "external/openclaw" }, + sha: headSha, + }, + }; + + expect( + canAutoscrubPullRequest({ + owner: "openclaw", + repo: "openclaw", + pullRequest: sameRepoPullRequest, + }), + ).toBe(true); + expect( + canAutoscrubPullRequest({ + owner: "openclaw", + repo: "openclaw", + pullRequest: forkPullRequest, + }), + ).toBe(false); + expect( + canAutoscrubPullRequest({ + owner: "openclaw", + repo: "openclaw", + pullRequest: editableForkPullRequest, + }), + ).toBe(true); + }); + + it("renders deterministic autoscrub success comments", () => { + const body = renderAutoscrubbedDependencyComment({ + baseBranch: "main", + commitSha: staleSha, + lockfileChanges: ["pnpm-lock.yaml", "extensions/slack/npm-shrinkwrap.json"], + }); + + expect(body).toContain(""); + expect(body).toContain("Dependency lockfile changes were removed"); + expect(body).toContain("did not change dependency graph fields in package manifests"); + expect(body).toContain("`pnpm-lock.yaml`"); + expect(body).toContain("`extensions/slack/npm-shrinkwrap.json`"); + expect(body).toContain(`Cleanup commit: \`${staleSha}\``); + expect(body).toContain( + "restored each listed lockfile from the target branch and pushed the cleanup commit to this PR head", + ); + expect(body).toContain( + "this PR no longer carries those package lockfile diffs after the cleanup commit", + ); + expect(isAutoscrubbedDependencyComment({ body })).toBe(true); + }); + + it("renders fork and dependency-manifest autoscrub guidance", () => { + const forkBody = renderBlockedDependencyComment({ + baseBranch: "main", + headSha, + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [], + autoscrubStatus: { kind: "not-attempted" }, + }); + const unsafeBody = renderBlockedDependencyComment({ + baseBranch: "main", + headSha, + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [], + autoscrubStatus: { + kind: "blocked-by-dependency-manifest-fields", + changes: [{ path: "package.json", fields: ["dependencies"] }], + }, + }); + const mixedBody = renderBlockedDependencyComment({ + baseBranch: "main", + headSha, + lockfileChanges: ["pnpm-lock.yaml"], + dependencyManifestChanges: [], + autoscrubStatus: { + kind: "blocked-by-other-dependency-files", + files: ["patches/example.patch", "pnpm-workspace.yaml"], + }, + }); + + expect(forkBody).toContain("Auto-scrub was not attempted"); + expect(forkBody).toContain( + "only push deterministic cleanup commits to PR branches that maintainers can modify", + ); + expect(unsafeBody).toContain("changes package manifest dependency graph fields"); + expect(unsafeBody).toContain("`package.json` changed `dependencies`"); + expect(unsafeBody).toContain("Dependency graph changes must be reviewed by security"); + expect(mixedBody).toContain("also changes dependency-related files"); + expect(mixedBody).toContain("`patches/example.patch`"); + expect(mixedBody).toContain("`pnpm-workspace.yaml`"); + }); + + it("reads base lockfiles with the base API before writing autoscrub commits", async () => { + const calls: Array<{ api: string; path: string; variables?: unknown }> = []; + const baseApi = { + request: async (path: string) => { + calls.push({ api: "base", path }); + if (path.includes("/contents/pnpm-lock.yaml?")) { + return { + content: Buffer.from("base lockfile").toString("base64"), + encoding: "base64", + sha: "base-file", + type: "file", + }; + } + throw new Error(`unexpected base request: ${path}`); + }, + }; + const writeApi = { + graphql: async (_query: string, variables: unknown) => { + calls.push({ api: "write", path: "graphql", variables }); + return { createCommitOnBranch: { commit: { oid: staleSha } } }; + }, + }; + + const commit = await createAutoscrubCommit( + { baseApi, writeApi }, + { + owner: "openclaw", + repo: "openclaw", + pullRequest: { + base: { sha: "base-sha" }, + head: { ref: "contributor/change", sha: headSha }, + }, + lockfileChanges: ["pnpm-lock.yaml"], + targetRepository: { owner: "contributor", repo: "openclaw" }, + }, + ); + + expect(commit).toEqual({ sha: staleSha }); + expect(calls.map((call) => `${call.api}:${call.path}`)).toEqual([ + "base:/repos/openclaw/openclaw/contents/pnpm-lock.yaml?ref=base-sha", + "write:graphql", + ]); + expect(calls[1].variables).toMatchObject({ + input: { + branch: { + repositoryNameWithOwner: "contributor/openclaw", + branchName: "contributor/change", + }, + expectedHeadOid: headSha, + fileChanges: { + additions: [ + { + contents: Buffer.from("base lockfile").toString("base64"), + path: "pnpm-lock.yaml", + }, + ], + deletions: [], + }, + }, + }); + }); + it("renders a cleared guard comment that preserves approval freshness", () => { const body = renderClearedDependencyGuardComment({ headSha }); diff --git a/test/scripts/dependency-guard-workflow.test.ts b/test/scripts/dependency-guard-workflow.test.ts index 4a9565ba310..eae98922cd4 100644 --- a/test/scripts/dependency-guard-workflow.test.ts +++ b/test/scripts/dependency-guard-workflow.test.ts @@ -6,6 +6,8 @@ const WORKFLOW = ".github/workflows/dependency-guard.yml"; const CODEOWNERS = ".github/CODEOWNERS"; type WorkflowStep = { + "continue-on-error"?: boolean; + env?: Record; name?: string; run?: string; uses?: string; @@ -13,7 +15,11 @@ type WorkflowStep = { }; type WorkflowJob = { + if?: string; name?: string; + needs?: string | string[]; + outputs?: Record; + permissions?: Record; steps?: WorkflowStep[]; }; @@ -32,11 +38,13 @@ describe("dependency guard workflow", () => { const parsed = readWorkflow(); expect(parsed.name).toBe("Dependency Guard"); + expect(parsed.jobs).toHaveProperty("dependency-guard-detect"); + expect(parsed.jobs).toHaveProperty("dependency-guard-autoscrub"); expect(parsed.jobs).toHaveProperty("dependency-guard"); expect(parsed.jobs?.["dependency-guard"]?.name).toBeUndefined(); }); - it("uses a metadata-only pull_request_target workflow with minimal write permissions", () => { + it("uses a metadata-only pull_request_target workflow with bounded write permissions", () => { const workflow = readFileSync(WORKFLOW, "utf8"); const parsed = readWorkflow(); @@ -47,6 +55,13 @@ describe("dependency guard workflow", () => { "pull-requests": "write", issues: "write", }); + expect(parsed.jobs?.["dependency-guard-autoscrub"]?.permissions).toEqual({ + contents: "read", + issues: "write", + "pull-requests": "read", + }); + expect(parsed.jobs?.["dependency-guard-detect"]?.permissions).toBeUndefined(); + expect(parsed.jobs?.["dependency-guard"]?.permissions).toBeUndefined(); }); it("checks out only trusted base scripts and does not execute PR-controlled code", () => { @@ -59,7 +74,6 @@ describe("dependency guard workflow", () => { "pnpm dlx", "actions: write", "id-token: write", - "secrets.", "github.rest.issues.createLabel", ]; @@ -67,6 +81,72 @@ describe("dependency guard workflow", () => { expect(workflow).not.toContain(snippet); } + const parsed = readWorkflow(); + const jobs = [ + parsed.jobs?.["dependency-guard-detect"], + parsed.jobs?.["dependency-guard-autoscrub"], + parsed.jobs?.["dependency-guard"], + ]; + for (const job of jobs) { + const steps = job?.steps ?? []; + 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.at(-1)?.run).toBe("node scripts/github/dependency-guard.mjs"); + } + }); + + it("keeps contents write scoped to the conditional autoscrub job", () => { + const jobs = readWorkflow().jobs ?? {}; + const detectJob = jobs["dependency-guard-detect"]; + const autoscrubJob = jobs["dependency-guard-autoscrub"]; + const finalJob = jobs["dependency-guard"]; + + expect(detectJob?.outputs?.autoscrub).toBe("${{ steps.guard.outputs.autoscrub }}"); + expect(detectJob?.outputs?.["autoscrub-owner"]).toBe( + "${{ steps.guard.outputs.autoscrub-owner }}", + ); + expect(detectJob?.outputs?.["autoscrub-repository"]).toBe( + "${{ steps.guard.outputs.autoscrub-repository }}", + ); + expect(autoscrubJob?.needs).toBe("dependency-guard-detect"); + expect(autoscrubJob?.if).toContain("needs.dependency-guard-detect.outputs.autoscrub == 'true'"); + expect(finalJob?.needs).toEqual(["dependency-guard-detect", "dependency-guard-autoscrub"]); + expect(finalJob?.if).toContain("always()"); + + const detectSteps = detectJob?.steps ?? []; + const autoscrubSteps = autoscrubJob?.steps ?? []; + const finalSteps = finalJob?.steps ?? []; + expect(detectSteps[1].env?.OPENCLAW_DEPENDENCY_GUARD_MODE).toBe("detect"); + expect(autoscrubSteps[1].uses).toBe( + "actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3", + ); + expect(autoscrubSteps[1].with).toMatchObject({ + "app-id": "2729701", + owner: "${{ needs.dependency-guard-detect.outputs.autoscrub-owner }}", + repositories: "${{ needs.dependency-guard-detect.outputs.autoscrub-repository }}", + "permission-contents": "write", + }); + expect(autoscrubSteps[1]["continue-on-error"]).toBe(true); + expect(autoscrubSteps[2].uses).toBe( + "actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3", + ); + expect(autoscrubSteps[2].with).toMatchObject({ + "app-id": "2971289", + owner: "${{ needs.dependency-guard-detect.outputs.autoscrub-owner }}", + repositories: "${{ needs.dependency-guard-detect.outputs.autoscrub-repository }}", + "permission-contents": "write", + }); + expect(autoscrubSteps[2]["continue-on-error"]).toBe(true); + expect(autoscrubSteps[3].env?.GITHUB_TOKEN).toBe("${{ github.token }}"); + expect(autoscrubSteps[3].env?.OPENCLAW_DEPENDENCY_GUARD_AUTOSCRUB_TOKEN).toBe( + "${{ steps.app-token.outputs.token || steps.app-token-fallback.outputs.token }}", + ); + expect(autoscrubSteps[3].env?.OPENCLAW_DEPENDENCY_GUARD_MODE).toBe("autoscrub"); + expect(finalSteps[1].env?.OPENCLAW_DEPENDENCY_GUARD_MODE).toBe("enforce"); + }); + + it("preserves dependency-guard as the final required check", () => { const steps = readWorkflow().jobs?.["dependency-guard"]?.steps ?? []; expect(steps).toHaveLength(2); expect(steps[0].uses).toBe("actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd"); @@ -77,8 +157,8 @@ describe("dependency guard workflow", () => { it("uses a dedicated checked-in script and bounded sticky comments", () => { const workflow = readFileSync(WORKFLOW, "utf8"); - const steps = readWorkflow().jobs?.["dependency-guard"]?.steps ?? []; - const runStep = steps[1]; + const detectSteps = readWorkflow().jobs?.["dependency-guard-detect"]?.steps ?? []; + const runStep = detectSteps[1]; const script = readFileSync("scripts/github/dependency-guard.mjs", "utf8"); expect(runStep.env?.OPENCLAW_SECURITY_TEAM_SLUG).toBe("openclaw-secops"); @@ -114,9 +194,33 @@ describe("dependency guard workflow", () => { expect(script).toContain("/memberships/"); expect(script).toContain("isCommentNewerThan"); expect(script).toContain("A later push requires a fresh approval."); + expect(script).toContain("createAutoscrubCommit"); + expect(script).toContain("chore: remove dependency lockfile change"); expect(script).toContain("process.exitCode = 1"); }); + it("cleans dependency label and guard comment after successful autoscrub", () => { + const script = readFileSync("scripts/github/dependency-guard.mjs", "utf8"); + const autoscrubCommitIndex = script.indexOf("const commit = await createAutoscrubCommit"); + const removeLabelIndex = script.indexOf( + "await removeLabelIfPresent(dependencyChangedLabel)", + autoscrubCommitIndex, + ); + const deleteCommentIndex = script.indexOf( + "await deleteCommentIfPresent(dependencyComment)", + autoscrubCommitIndex, + ); + const autoscrubCommentIndex = script.indexOf( + "renderAutoscrubbedDependencyComment", + autoscrubCommitIndex, + ); + + expect(autoscrubCommitIndex).toBeGreaterThan(0); + expect(removeLabelIndex).toBeGreaterThan(autoscrubCommitIndex); + expect(deleteCommentIndex).toBeGreaterThan(autoscrubCommitIndex); + expect(autoscrubCommentIndex).toBeGreaterThan(deleteCommentIndex); + }); + it("requires secops review for future workflow or guard changes", () => { const codeowners = readFileSync(CODEOWNERS, "utf8"); expect(codeowners).toContain(