diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index cfdbf26958d..d928b1f433b 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -12,9 +12,14 @@ /.github/workflows/codeql-android-critical-security.yml @openclaw/openclaw-secops /.github/workflows/codeql-critical-quality.yml @openclaw/openclaw-secops /.github/workflows/dependency-guard.yml @openclaw/openclaw-secops +/.github/workflows/security-sensitive-guard.yml @openclaw/openclaw-secops /test/scripts/dependency-guard-workflow.test.ts @openclaw/openclaw-secops /test/scripts/dependency-guard-script.test.ts @openclaw/openclaw-secops +/test/scripts/security-sensitive-guard-workflow.test.ts @openclaw/openclaw-secops +/test/scripts/security-sensitive-guard-script.test.ts @openclaw/openclaw-secops /scripts/github/dependency-guard.mjs @openclaw/openclaw-secops +/scripts/github/security-sensitive-guard.mjs @openclaw/openclaw-secops +/.gitignore @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/security-sensitive-guard.yml b/.github/workflows/security-sensitive-guard.yml new file mode 100644 index 00000000000..0a46bdf0cd9 --- /dev/null +++ b/.github/workflows/security-sensitive-guard.yml @@ -0,0 +1,55 @@ +name: Security Sensitive Guard + +on: + 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 + +concurrency: + group: security-sensitive-guard-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + security-sensitive-guard-detect: + if: ${{ !github.event.pull_request.draft }} + runs-on: ubuntu-24.04 + timeout-minutes: 5 + 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 security-sensitive changes + env: + GITHUB_TOKEN: ${{ github.token }} + OPENCLAW_SECURITY_APPROVERS: vincentkoc,steipete,joshavant + OPENCLAW_SECURITY_SENSITIVE_GUARD_MODE: detect + OPENCLAW_SECURITY_TEAM_SLUG: openclaw-secops + run: node scripts/github/security-sensitive-guard.mjs + + security-sensitive-guard: + if: ${{ !github.event.pull_request.draft && always() }} + needs: + - security-sensitive-guard-detect + runs-on: ubuntu-24.04 + timeout-minutes: 5 + 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: Enforce security-sensitive guard + env: + GITHUB_TOKEN: ${{ github.token }} + OPENCLAW_SECURITY_APPROVERS: vincentkoc,steipete,joshavant + OPENCLAW_SECURITY_SENSITIVE_GUARD_MODE: enforce + OPENCLAW_SECURITY_TEAM_SLUG: openclaw-secops + run: node scripts/github/security-sensitive-guard.mjs diff --git a/scripts/github/security-sensitive-guard.mjs b/scripts/github/security-sensitive-guard.mjs new file mode 100644 index 00000000000..0f8bc90ad82 --- /dev/null +++ b/scripts/github/security-sensitive-guard.mjs @@ -0,0 +1,717 @@ +#!/usr/bin/env node + +// GitHub security-sensitive file guard: detects sensitive boundary files, +// manages sticky comments/labels, and requires SHA-bound secops/admin approval. +import { appendFile, readFile } from "node:fs/promises"; +import { readBoundedResponseText } from "../lib/bounded-response.mjs"; + +/** Marker used to identify security-sensitive guard comments. */ +export const securitySensitiveGuardMarker = ""; +export const securitySensitiveChangedLabel = "security-sensitive-changed"; +export const allowSecuritySensitiveCommand = "/allow-security-sensitive-change"; +export const GITHUB_ERROR_BODY_MAX_BYTES = 64 * 1024; +export const GITHUB_API_REQUEST_TIMEOUT_MS = 30_000; + +const securityTeamSlug = process.env.OPENCLAW_SECURITY_TEAM_SLUG ?? "openclaw-secops"; +const maxListedFiles = 25; +const securitySensitiveFiles = [ + { + path: ".gitignore", + reason: + "Controls ignored secret and local files, including common `.env` files, before they can be accidentally committed.", + }, +]; + +export function securitySensitiveFileDefinitions() { + return securitySensitiveFiles.map((entry) => ({ ...entry })); +} + +export function securitySensitiveFileDefinition(filename) { + return securitySensitiveFiles.find((entry) => entry.path === filename) ?? null; +} + +export function isSecuritySensitiveFile(filename) { + return securitySensitiveFileDefinition(filename) !== null; +} + +export function sanitizeDisplayValue(value) { + return String(value) + .replace(/[\p{Cc}]/gu, "?") + .slice(0, 240); +} + +export function markdownCode(value) { + return `\`${sanitizeDisplayValue(value).replaceAll("`", "\\`")}\``; +} + +function* securitySensitiveOverrideCandidates({ comments, expectedSha, newerThan }) { + if (!expectedSha) { + return; + } + const commandPattern = /^\/allow-security-sensitive-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; + } + yield { + login, + reason: reason ? sanitizeDisplayValue(reason) : null, + sha: expectedSha, + url: comment.html_url, + }; + } + } +} + +export function findSecuritySensitiveOverrideCommand({ + comments, + expectedSha, + isSecurityMember, + newerThan, +}) { + for (const candidate of securitySensitiveOverrideCandidates({ + comments, + expectedSha, + newerThan, + })) { + if (isSecurityMember(candidate.login)) { + return candidate; + } + } + return null; +} + +export async function findSecuritySensitiveOverrideCommandAsync(input) { + for (const candidate of securitySensitiveOverrideCandidates(input)) { + if (await input.isSecurityMember(candidate.login)) { + return candidate; + } + } + 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 securitySensitiveGuardCommentHeadSha(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 securitySensitiveOverrideExpectedSha(existingGuardComment, currentHeadSha) { + if ( + !currentHeadSha || + existingGuardComment?.body?.includes("### Security-sensitive changes are blocked") !== true + ) { + return null; + } + return securitySensitiveGuardCommentHeadSha(existingGuardComment) === currentHeadSha + ? currentHeadSha + : null; +} + +export function isSecuritySensitiveGuardAuthorizedForHead(comment, currentHeadSha) { + return ( + Boolean(currentHeadSha) && + comment?.body?.includes("### Security-sensitive change authorized") === true && + securitySensitiveGuardCommentHeadSha(comment) === currentHeadSha + ); +} + +export function isSecuritySensitiveGuardTrustedForHead(comment, currentHeadSha) { + return ( + Boolean(currentHeadSha) && + comment?.body?.includes("### Security-sensitive changes noted") === true && + securitySensitiveGuardCommentHeadSha(comment) === currentHeadSha + ); +} + +export function securityApproverSet(value) { + return new Set( + String(value ?? "") + .split(/[\s,]+/u) + .map((login) => login.trim().toLowerCase()) + .filter(Boolean), + ); +} + +export function securitySensitiveGuardCommentAuthors(value) { + return new Set( + String(value ?? "github-actions[bot]") + .split(/[\s,]+/u) + .map((login) => login.trim().toLowerCase()) + .filter(Boolean), + ); +} + +export function isSecuritySensitiveGuardMarkerComment(comment, trustedAuthors) { + const login = comment.user?.login?.toLowerCase(); + return Boolean( + login && trustedAuthors.has(login) && comment.body?.includes(securitySensitiveGuardMarker), + ); +} + +function sortedSecuritySensitiveChanges(filenames) { + const byPath = new Map(); + for (const filename of filenames) { + if (typeof filename !== "string") { + continue; + } + const definition = securitySensitiveFileDefinition(filename); + if (definition) { + byPath.set(definition.path, definition); + } + } + return [...byPath.values()].toSorted((left, right) => left.path.localeCompare(right.path)); +} + +export function collectSecuritySensitiveChanges(files) { + const filenames = []; + for (const file of files) { + if (typeof file === "string") { + filenames.push(file); + continue; + } + if (file && typeof file === "object") { + filenames.push(file.filename, file.previous_filename); + } + } + return sortedSecuritySensitiveChanges(filenames); +} + +function renderChangedFileLines(changes) { + const listedFiles = changes.slice(0, maxListedFiles); + const omittedCount = changes.length - listedFiles.length; + const lines = listedFiles.map( + (change) => `- ${markdownCode(change.path)}: ${sanitizeDisplayValue(change.reason)}`, + ); + if (omittedCount > 0) { + lines.push(`- ${omittedCount} additional security-sensitive files not shown`); + } + return lines; +} + +export function renderSecuritySensitiveAwarenessComment(changes) { + return [ + securitySensitiveGuardMarker, + "", + "### Security-sensitive file changes detected", + "", + "This PR changes files that define security boundaries. Maintainers should confirm these changes are intentional.", + "", + "Changed files:", + ...renderChangedFileLines(changes), + "", + "Maintainer follow-up:", + "- Review whether each security-sensitive file change is intentional.", + "- Confirm the change does not weaken secret, credential, or local-state protection.", + "- If this PR intentionally needs the change, a repository admin or member of `@openclaw/openclaw-secops` must approve the exact head SHA.", + ].join("\n"); +} + +export function renderAuthorizedSecuritySensitiveComment(override) { + const lines = [ + securitySensitiveGuardMarker, + "", + "### Security-sensitive change authorized", + "", + "This PR includes security-sensitive file changes. A repository admin or member of `@openclaw/openclaw-secops` authorized this exact head SHA with `/allow-security-sensitive-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 renderTrustedSecuritySensitiveComment({ actor, headSha, changes }) { + return [ + securitySensitiveGuardMarker, + "", + "### Security-sensitive changes noted", + "", + "This PR includes security-sensitive file changes. The guard is informational because the PR author is a repository admin or a member of `@openclaw/openclaw-secops`.", + "", + `- Current SHA: ${markdownCode(headSha ?? "")}`, + `- Trusted actor: @${sanitizeDisplayValue(actor.login)}`, + `- Trusted role: ${markdownCode(actor.reason)}`, + "", + "Changed files:", + ...renderChangedFileLines(changes), + "", + "Security review is still recommended before merge when the change is intentional.", + ].join("\n"); +} + +export function renderClearedSecuritySensitiveGuardComment({ headSha }) { + return [ + securitySensitiveGuardMarker, + "", + "### Security-sensitive guard cleared", + "", + "This PR no longer has blocked security-sensitive file changes. A future security-sensitive change requires a fresh `/allow-security-sensitive-change` comment after the guard blocks that new head SHA.", + "", + `- Current SHA: ${markdownCode(headSha ?? "")}`, + ].join("\n"); +} + +export function renderBlockedSecuritySensitiveComment({ headSha, changes }) { + return [ + securitySensitiveGuardMarker, + "", + "### Security-sensitive changes are blocked", + "", + "OpenClaw does not accept security-sensitive file changes through PRs unless a repository admin or security explicitly authorizes the current head SHA.", + "", + "Detected security-sensitive changes:", + ...renderChangedFileLines(changes), + "", + "If this PR intentionally needs these changes, ask a repository admin or member of `@openclaw/openclaw-secops` to comment:", + "", + "```text", + allowSecuritySensitiveCommand, + "```", + "", + `The action will approve the current head SHA (${markdownCode(headSha ?? "")}) when it reruns. A later push requires a fresh approval.`, + ].join("\n"); +} + +export function securitySensitiveGuardTrustedActorCandidates({ + pullRequest, + event, + currentHeadSha, +}) { + const eventHeadSha = event?.pull_request?.head?.sha; + const eventAfterSha = event?.after; + const eventMatchesCurrentHead = + Boolean(currentHeadSha) && + (eventHeadSha === currentHeadSha || eventAfterSha === currentHeadSha); + if (!eventMatchesCurrentHead) { + return []; + } + const candidates = []; + const seen = new Set(); + for (const [source, login] of [["pull request author", pullRequest?.user?.login]]) { + if (typeof login !== "string" || login.length === 0) { + continue; + } + const normalizedLogin = login.toLowerCase(); + if (seen.has(normalizedLogin)) { + continue; + } + seen.add(normalizedLogin); + candidates.push({ login, source }); + } + return candidates; +} + +export async function findTrustedSecuritySensitiveGuardActor({ + candidates, + isSecuritySensitiveApprover, +}) { + for (const candidate of candidates) { + const role = await isSecuritySensitiveApprover(candidate.login); + if (role) { + return { + login: candidate.login, + reason: `${candidate.source}; ${role}`, + }; + } + } + return null; +} + +function githubErrorBodyTooLarge(maxBytes) { + return new Error(`GitHub error response body exceeded ${maxBytes} bytes`); +} + +export async function readBoundedGitHubErrorText(response, maxBytes = GITHUB_ERROR_BODY_MAX_BYTES) { + return await readBoundedResponseText(response, "GitHub error", maxBytes, { + createTooLargeError: () => githubErrorBodyTooLarge(maxBytes), + }); +} + +function timeoutError(path, method, timeoutMs) { + return new Error(`GitHub API ${method} ${path} exceeded timeout ${timeoutMs}ms`); +} + +function combineAbortSignals(signals) { + const activeSignals = signals.filter(Boolean); + if (activeSignals.length === 0) { + return undefined; + } + if (activeSignals.length === 1) { + return activeSignals[0]; + } + return AbortSignal.any(activeSignals); +} + +export function githubApi(token, options = {}) { + const fetchImpl = options.fetchImpl ?? fetch; + const timeoutMs = options.timeoutMs ?? GITHUB_API_REQUEST_TIMEOUT_MS; + const baseHeaders = { + accept: "application/vnd.github+json", + authorization: `Bearer ${token}`, + "user-agent": "openclaw-security-sensitive-guard", + "x-github-api-version": "2022-11-28", + }; + const request = async (path, requestOptions = {}) => { + const method = requestOptions.method ?? "GET"; + const timeoutController = new AbortController(); + let timeout; + const timeoutPromise = new Promise((_, reject) => { + timeout = setTimeout(() => { + timeoutController.abort(); + reject(timeoutError(path, method, timeoutMs)); + }, timeoutMs); + timeout.unref?.(); + }); + const operationPromise = (async () => { + const response = await fetchImpl(`https://api.github.com${path}`, { + ...requestOptions, + signal: combineAbortSignals([requestOptions.signal, timeoutController.signal]), + headers: { ...baseHeaders, ...requestOptions.headers }, + }); + if (response.status === 204) { + return null; + } + if (!response.ok) { + let errorText; + try { + errorText = await readBoundedGitHubErrorText(response); + } catch (bodyError) { + errorText = bodyError instanceof Error ? bodyError.message : String(bodyError); + } + const error = new Error(`${response.status} ${response.statusText}: ${errorText}`); + error.status = response.status; + throw error; + } + return response.json(); + })(); + operationPromise.catch(() => {}); + try { + return await Promise.race([operationPromise, timeoutPromise]); + } finally { + clearTimeout(timeout); + } + }; + 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; + } + } + }, + }; +} + +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 eventPullRequest = event.pull_request; + if (!eventPullRequest) { + console.log("No pull_request payload found; skipping."); + return; + } + + const api = githubApi(token); + const explicitSecurityApprovers = securityApproverSet(process.env.OPENCLAW_SECURITY_APPROVERS); + const trustedCommentAuthors = securitySensitiveGuardCommentAuthors( + process.env.OPENCLAW_SECURITY_SENSITIVE_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_SECURITY_SENSITIVE_GUARD_MODE ?? "enforce"; + const files = await api.paginate(`${pullPath}/files`); + const securitySensitiveChanges = collectSecuritySensitiveChanges(files); + + const [comments, labels] = await Promise.all([ + api.paginate(`${issuePath}/comments`), + api.paginate(`${issuePath}/labels`), + ]); + const existingGuardComment = comments.find((comment) => + isSecuritySensitiveGuardMarkerComment(comment, trustedCommentAuthors), + ); + 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`)); + labelNames.delete(label); + }; + 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`)); + labelNames.add(label); + }; + const upsertComment = async (comment, body) => { + if (comment) { + return 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 (securitySensitiveChanges.length === 0) { + await removeLabelIfPresent(securitySensitiveChangedLabel); + if (existingGuardComment) { + await upsertComment( + existingGuardComment, + renderClearedSecuritySensitiveGuardComment({ headSha: pullRequest.head?.sha }), + ); + } + await writeSummary( + "## Security Sensitive Guard\n\nNo security-sensitive file changes detected.", + ); + console.log("No security-sensitive file changes detected."); + return; + } + + await addLabelIfMissing(securitySensitiveChangedLabel); + await writeSummary( + [ + "## Security Sensitive Guard", + "", + `Detected ${securitySensitiveChanges.length} security-sensitive file change(s).`, + "", + ...securitySensitiveChanges.map((change) => `- ${markdownCode(change.path)}`), + ].join("\n"), + ); + console.log(`Detected ${securitySensitiveChanges.length} security-sensitive file change(s).`); + + const membershipCache = new Map(); + const permissionCache = new Map(); + const isSecurityMember = async (login) => { + const normalizedLogin = login.toLowerCase(); + if (explicitSecurityApprovers.has(normalizedLogin)) { + return true; + } + if (membershipCache.has(normalizedLogin)) { + return membershipCache.get(normalizedLogin); + } + try { + const membership = await api.request( + `/orgs/${owner}/teams/${securityTeamSlug}/memberships/${encodeURIComponent(login)}`, + ); + const allowed = membership?.state === "active"; + membershipCache.set(normalizedLogin, allowed); + return allowed; + } catch (error) { + if (error?.status !== 404) { + console.warn(`Could not verify ${login} against ${securityTeamSlug}: ${error.message}`); + } + membershipCache.set(normalizedLogin, false); + return false; + } + }; + const isRepositoryAdmin = async (login) => { + const normalizedLogin = login.toLowerCase(); + if (permissionCache.has(normalizedLogin)) { + return permissionCache.get(normalizedLogin); + } + try { + const result = await api.request( + `/repos/${owner}/${repo}/collaborators/${encodeURIComponent(login)}/permission`, + ); + const allowed = result?.permission === "admin"; + permissionCache.set(normalizedLogin, allowed); + return allowed; + } catch (error) { + if (error?.status !== 404) { + console.warn(`Could not verify repository permission for ${login}: ${error.message}`); + } + permissionCache.set(normalizedLogin, false); + return false; + } + }; + const isSecuritySensitiveApprover = async (login) => { + if (await isSecurityMember(login)) { + return securityTeamSlug; + } + if (await isRepositoryAdmin(login)) { + return "repository admin"; + } + return null; + }; + const currentHeadSha = pullRequest.head?.sha; + if (isSecuritySensitiveGuardTrustedForHead(existingGuardComment, currentHeadSha)) { + await writeSummary( + [ + "## Security Sensitive Guard", + "", + `Security-sensitive changes remain informational for a trusted actor at ${markdownCode(currentHeadSha)}.`, + ].join("\n"), + ); + console.log("Security-sensitive changes remain informational for this head SHA."); + return; + } + const trustedActor = await findTrustedSecuritySensitiveGuardActor({ + candidates: securitySensitiveGuardTrustedActorCandidates({ + pullRequest, + event, + currentHeadSha, + }), + isSecuritySensitiveApprover, + }); + if (trustedActor) { + await upsertComment( + existingGuardComment, + renderTrustedSecuritySensitiveComment({ + actor: trustedActor, + changes: securitySensitiveChanges, + headSha: currentHeadSha, + }), + ); + await writeSummary( + [ + "## Security Sensitive Guard", + "", + `Security-sensitive changes noted for trusted actor @${sanitizeDisplayValue(trustedActor.login)} and allowed to continue.`, + ].join("\n"), + ); + console.log("Security-sensitive changes noted for trusted actor; guard is informational."); + return; + } + if (isSecuritySensitiveGuardAuthorizedForHead(existingGuardComment, currentHeadSha)) { + await writeSummary( + [ + "## Security Sensitive Guard", + "", + `Security-sensitive changes remain authorized for ${markdownCode(currentHeadSha)}.`, + ].join("\n"), + ); + console.log("Security-sensitive changes remain authorized for this head SHA."); + return; + } + const override = await findSecuritySensitiveOverrideCommandAsync({ + comments, + expectedSha: securitySensitiveOverrideExpectedSha(existingGuardComment, currentHeadSha), + isSecurityMember: async (login) => Boolean(await isSecuritySensitiveApprover(login)), + newerThan: existingGuardComment?.updated_at ?? existingGuardComment?.created_at, + }); + if (override) { + await upsertComment(existingGuardComment, renderAuthorizedSecuritySensitiveComment(override)); + await writeSummary( + [ + "## Security Sensitive Guard", + "", + `Security-sensitive changes authorized by @${sanitizeDisplayValue(override.login)} for ${markdownCode(override.sha)}.`, + ].join("\n"), + ); + console.log("Security-sensitive changes authorized by trusted override."); + return; + } + if (mode === "detect") { + await upsertComment( + existingGuardComment, + renderSecuritySensitiveAwarenessComment(securitySensitiveChanges), + ); + await writeSummary( + "## Security Sensitive Guard\n\nSecurity-sensitive enforcement deferred to the final guard job.", + ); + console.log("Security-sensitive enforcement deferred to the final guard job."); + return; + } + + await upsertComment( + existingGuardComment, + renderBlockedSecuritySensitiveComment({ + changes: securitySensitiveChanges, + headSha: pullRequest.head?.sha, + }), + ); + await writeSummary( + "## Security Sensitive Guard\n\nSecurity-sensitive changes are blocked without a current admin or secops override.", + ); + throw new Error( + "Security-sensitive changes require removal or a current admin or secops override.", + ); +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main().catch( + /** @param {unknown} error */ (error) => { + console.error(error instanceof Error ? error.message : error); + process.exitCode = 1; + }, + ); +} diff --git a/scripts/sync-openclaw-label-colors.mjs b/scripts/sync-openclaw-label-colors.mjs index db5cd91e72b..1f3499f8c5e 100644 --- a/scripts/sync-openclaw-label-colors.mjs +++ b/scripts/sync-openclaw-label-colors.mjs @@ -138,6 +138,7 @@ const EXACT_COLORS = new Map( "dedupe:child": COLORS.paleBlue, dependencies: COLORS.neutralGray, "dependencies-changed": COLORS.paleYellow, + "security-sensitive-changed": COLORS.paleYellow, github_actions: COLORS.neutralGray, go: COLORS.neutralGray, java: COLORS.neutralGray, diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 84a55098073..73f9ca3e753 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -401,6 +401,10 @@ const BROAD_ONLY_TEST_HELPERS = new Set(["test/helpers/poll.ts"]); const TOOLING_SOURCE_TEST_TARGETS = new Map([ [".crabbox.yaml", ["test/scripts/package-acceptance-workflow.test.ts"]], [".github/workflows/ci.yml", ["test/scripts/ci-workflow-guards.test.ts"]], + [ + ".github/workflows/security-sensitive-guard.yml", + ["test/scripts/security-sensitive-guard-workflow.test.ts"], + ], [ ".github/workflows/ci-check-testbox.yml", ["test/scripts/ci-workflow-guards.test.ts", "test/scripts/package-acceptance-workflow.test.ts"], @@ -504,6 +508,10 @@ const TOOLING_SOURCE_TEST_TARGETS = new Map([ ], ], ["scripts/dependency-changes-report.mjs", ["test/scripts/dependency-changes-report.test.ts"]], + [ + "scripts/github/security-sensitive-guard.mjs", + ["test/scripts/security-sensitive-guard-script.test.ts"], + ], [ "scripts/dependency-ownership-surface-report.mjs", ["test/scripts/dependency-ownership-surface-report.test.ts"], diff --git a/test/scripts/security-sensitive-guard-script.test.ts b/test/scripts/security-sensitive-guard-script.test.ts new file mode 100644 index 00000000000..5bb90a42355 --- /dev/null +++ b/test/scripts/security-sensitive-guard-script.test.ts @@ -0,0 +1,248 @@ +// Security Sensitive Guard Script tests cover sensitive file guard behavior. +import { describe, expect, it } from "vitest"; +import { + allowSecuritySensitiveCommand, + collectSecuritySensitiveChanges, + findSecuritySensitiveOverrideCommand, + findSecuritySensitiveOverrideCommandAsync, + findTrustedSecuritySensitiveGuardActor, + isSecuritySensitiveFile, + isSecuritySensitiveGuardAuthorizedForHead, + isSecuritySensitiveGuardMarkerComment, + isSecuritySensitiveGuardTrustedForHead, + markdownCode, + renderAuthorizedSecuritySensitiveComment, + renderBlockedSecuritySensitiveComment, + renderClearedSecuritySensitiveGuardComment, + renderSecuritySensitiveAwarenessComment, + renderTrustedSecuritySensitiveComment, + sanitizeDisplayValue, + securityApproverSet, + securitySensitiveFileDefinition, + securitySensitiveFileDefinitions, + securitySensitiveGuardCommentAuthors, + securitySensitiveGuardCommentHeadSha, + securitySensitiveGuardMarker, + securitySensitiveGuardTrustedActorCandidates, + securitySensitiveOverrideExpectedSha, +} from "../../scripts/github/security-sensitive-guard.mjs"; + +const headSha = "a".repeat(40); +const staleSha = "b".repeat(40); + +describe("security-sensitive guard script", () => { + it("detects only registered security-sensitive file surfaces", () => { + expect(securitySensitiveFileDefinitions()).toEqual([ + { + path: ".gitignore", + reason: + "Controls ignored secret and local files, including common `.env` files, before they can be accidentally committed.", + }, + ]); + expect(isSecuritySensitiveFile(".gitignore")).toBe(true); + expect(isSecuritySensitiveFile("docs/.gitignore")).toBe(false); + expect(isSecuritySensitiveFile("package.json")).toBe(false); + expect(securitySensitiveFileDefinition(".gitignore")?.reason).toContain(".env"); + }); + + it("detects renames away from registered security-sensitive file surfaces", () => { + expect( + collectSecuritySensitiveChanges([ + { + filename: ".gitignore.disabled", + previous_filename: ".gitignore", + status: "renamed", + }, + ]), + ).toEqual([securitySensitiveFileDefinition(".gitignore")]); + }); + + it("accepts only security-member override commands for the current head sha", () => { + const comments = [ + { + body: "/allow-security-sensitive-change not enough", + created_at: "2026-05-28T20:00:00Z", + user: { login: "not-security" }, + }, + { + body: "/allow-security-sensitive-change stale approval", + created_at: "2026-05-28T20:01:00Z", + user: { login: "security-user" }, + }, + { + body: "/allow-security-sensitive-change reviewed .gitignore", + created_at: "2026-05-28T20:03:00Z", + html_url: "https://example.test/comment", + user: { login: "security-user" }, + }, + ]; + + const override = findSecuritySensitiveOverrideCommand({ + comments, + expectedSha: headSha, + isSecurityMember: (login) => login === "security-user", + newerThan: "2026-05-28T20:02:00Z", + }); + + expect(override).toEqual({ + login: "security-user", + reason: "reviewed .gitignore", + sha: headSha, + url: "https://example.test/comment", + }); + }); + + it("rejects stale or non-security override commands", async () => { + const comments = [ + { + body: "/allow-security-sensitive-change stale approval", + created_at: "2026-05-28T20:00:00Z", + user: { login: "security-user" }, + }, + { + body: "/allow-security-sensitive-change not enough", + created_at: "2026-05-28T20:02:00Z", + user: { login: "not-security" }, + }, + ]; + + await expect( + findSecuritySensitiveOverrideCommandAsync({ + comments, + expectedSha: headSha, + isSecurityMember: async (login) => login === "security-user", + newerThan: "2026-05-28T20:01:00Z", + }), + ).resolves.toBeNull(); + }); + + it("binds override commands to the head sha in the blocked guard comment", () => { + const blockedComment = { + body: renderBlockedSecuritySensitiveComment({ + changes: [securitySensitiveFileDefinition(".gitignore")], + headSha, + }), + }; + const staleBlockedComment = { + body: renderBlockedSecuritySensitiveComment({ + changes: [securitySensitiveFileDefinition(".gitignore")], + headSha: staleSha, + }), + }; + + expect(securitySensitiveGuardCommentHeadSha(blockedComment)).toBe(headSha); + expect(securitySensitiveOverrideExpectedSha(blockedComment, headSha)).toBe(headSha); + expect(securitySensitiveOverrideExpectedSha(staleBlockedComment, headSha)).toBeNull(); + }); + + it("preserves same-head authorization across reruns", () => { + const authorizedComment = { + body: renderAuthorizedSecuritySensitiveComment({ + login: "security-user", + reason: null, + sha: headSha, + }), + }; + + expect(securitySensitiveGuardCommentHeadSha(authorizedComment)).toBe(headSha); + expect(isSecuritySensitiveGuardAuthorizedForHead(authorizedComment, headSha)).toBe(true); + expect(isSecuritySensitiveGuardAuthorizedForHead(authorizedComment, staleSha)).toBe(false); + expect(securitySensitiveOverrideExpectedSha(authorizedComment, headSha)).toBeNull(); + }); + + it("recognizes trusted security-sensitive guard actors automatically", async () => { + const sameActorCandidates = securitySensitiveGuardTrustedActorCandidates({ + pullRequest: { user: { login: "repo-admin" } }, + event: { pull_request: { head: { sha: headSha } }, sender: { login: "repo-admin" } }, + currentHeadSha: headSha, + }); + const staleAuthorCandidate = securitySensitiveGuardTrustedActorCandidates({ + pullRequest: { user: { login: "repo-admin" } }, + event: { pull_request: { head: { sha: staleSha } }, sender: { login: "repo-admin" } }, + currentHeadSha: headSha, + }); + + expect(sameActorCandidates).toEqual([{ login: "repo-admin", source: "pull request author" }]); + expect(staleAuthorCandidate).toEqual([]); + + await expect( + findTrustedSecuritySensitiveGuardActor({ + candidates: sameActorCandidates, + isSecuritySensitiveApprover: async (login) => + login === "repo-admin" ? "repository admin" : null, + }), + ).resolves.toEqual({ + login: "repo-admin", + reason: "pull request author; repository admin", + }); + }); + + it("trusts only configured security-sensitive guard marker comment authors", () => { + const trustedAuthors = securitySensitiveGuardCommentAuthors( + "github-actions[bot], openclaw-security-guard[bot]", + ); + + expect( + isSecuritySensitiveGuardMarkerComment( + { + body: securitySensitiveGuardMarker, + user: { login: "openclaw-security-guard[bot]" }, + }, + trustedAuthors, + ), + ).toBe(true); + expect( + isSecuritySensitiveGuardMarkerComment( + { + body: securitySensitiveGuardMarker, + user: { login: "contributor" }, + }, + trustedAuthors, + ), + ).toBe(false); + }); + + it("renders deterministic awareness, blocked, trusted, authorized, and cleared comments", () => { + const changes = [securitySensitiveFileDefinition(".gitignore")]; + const awarenessBody = renderSecuritySensitiveAwarenessComment(changes); + const blockedBody = renderBlockedSecuritySensitiveComment({ changes, headSha }); + const trustedBody = renderTrustedSecuritySensitiveComment({ + actor: { login: "repo-admin", reason: "pull request author; repository admin" }, + changes, + headSha, + }); + const authorizedBody = renderAuthorizedSecuritySensitiveComment({ + login: "security-user", + reason: "reviewed .gitignore", + sha: headSha, + }); + const clearedBody = renderClearedSecuritySensitiveGuardComment({ headSha }); + + expect(awarenessBody).toContain(securitySensitiveGuardMarker); + expect(awarenessBody).toContain("Security-sensitive file changes detected"); + expect(awarenessBody).toContain("`.gitignore`"); + expect(awarenessBody).toContain(".env"); + expect(blockedBody).toContain("Security-sensitive changes are blocked"); + expect(blockedBody).toContain(allowSecuritySensitiveCommand); + expect(blockedBody).toContain(`current head SHA (\`${headSha}\`)`); + expect(trustedBody).toContain("Security-sensitive changes noted"); + expect(trustedBody).toContain("@repo-admin"); + expect(isSecuritySensitiveGuardTrustedForHead({ body: trustedBody }, headSha)).toBe(true); + expect(authorizedBody).toContain("Security-sensitive change authorized"); + expect(authorizedBody).toContain("`reviewed .gitignore`"); + expect(clearedBody).toContain("Security-sensitive guard cleared"); + expect(clearedBody).toContain("requires a fresh `/allow-security-sensitive-change` comment"); + }); + + it("sanitizes display values and markdown code", () => { + expect(sanitizeDisplayValue("abc\u0000def")).toBe("abc?def"); + expect(sanitizeDisplayValue("x".repeat(300))).toHaveLength(240); + expect(markdownCode("`quoted`")).toBe("`\\`quoted\\``"); + }); + + it("parses explicit security approver allowlists", () => { + expect(securityApproverSet("vincentkoc, steipete\njoshavant")).toEqual( + new Set(["vincentkoc", "steipete", "joshavant"]), + ); + }); +}); diff --git a/test/scripts/security-sensitive-guard-workflow.test.ts b/test/scripts/security-sensitive-guard-workflow.test.ts new file mode 100644 index 00000000000..9d6def087bd --- /dev/null +++ b/test/scripts/security-sensitive-guard-workflow.test.ts @@ -0,0 +1,130 @@ +// Security Sensitive Guard Workflow tests cover sensitive file guard workflow behavior. +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; +import { parse } from "yaml"; + +const WORKFLOW = ".github/workflows/security-sensitive-guard.yml"; +const CODEOWNERS = ".github/CODEOWNERS"; + +type WorkflowStep = { + env?: Record; + name?: string; + run?: string; + uses?: string; + with?: Record; +}; + +type WorkflowJob = { + if?: string; + needs?: string | string[]; + permissions?: Record; + steps?: WorkflowStep[]; +}; + +type Workflow = { + jobs?: Record; + name?: string; + permissions?: Record; +}; + +function readWorkflow(): Workflow { + return parse(readFileSync(WORKFLOW, "utf8")) as Workflow; +} + +describe("security-sensitive guard workflow", () => { + it("uses the security-sensitive guard check name", () => { + const parsed = readWorkflow(); + + expect(parsed.name).toBe("Security Sensitive Guard"); + expect(parsed.jobs).toHaveProperty("security-sensitive-guard-detect"); + expect(parsed.jobs).toHaveProperty("security-sensitive-guard"); + }); + + it("uses a metadata-only pull_request_target workflow with bounded write permissions", () => { + const workflow = readFileSync(WORKFLOW, "utf8"); + const parsed = readWorkflow(); + + expect(workflow).toContain("pull_request_target:"); + expect(workflow).toContain("checks trusted base script only; never checks out PR head"); + expect(parsed.permissions).toEqual({ + contents: "read", + "pull-requests": "write", + issues: "write", + }); + expect(parsed.jobs?.["security-sensitive-guard-detect"]?.permissions).toBeUndefined(); + expect(parsed.jobs?.["security-sensitive-guard"]?.permissions).toBeUndefined(); + }); + + it("checks out only trusted base scripts and does not execute PR-controlled code", () => { + const workflow = readFileSync(WORKFLOW, "utf8"); + const forbiddenSnippets = [ + "github.event.pull_request.head", + "pullRequest.head", + "pnpm install", + "npm install", + "pnpm dlx", + "actions: write", + "id-token: write", + ]; + + for (const snippet of forbiddenSnippets) { + expect(workflow).not.toContain(snippet); + } + + const jobs = readWorkflow().jobs ?? {}; + for (const jobName of ["security-sensitive-guard-detect", "security-sensitive-guard"]) { + const steps = jobs[jobName]?.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/security-sensitive-guard.mjs"); + } + }); + + it("keeps detection separate from the final required check", () => { + const jobs = readWorkflow().jobs ?? {}; + const detectJob = jobs["security-sensitive-guard-detect"]; + const finalJob = jobs["security-sensitive-guard"]; + const detectSteps = detectJob?.steps ?? []; + const finalSteps = finalJob?.steps ?? []; + + expect(finalJob?.needs).toEqual(["security-sensitive-guard-detect"]); + expect(finalJob?.if).toContain("always()"); + expect(detectSteps[1].env?.OPENCLAW_SECURITY_SENSITIVE_GUARD_MODE).toBe("detect"); + expect(finalSteps[1].env?.OPENCLAW_SECURITY_SENSITIVE_GUARD_MODE).toBe("enforce"); + expect(finalSteps[1].env?.OPENCLAW_SECURITY_TEAM_SLUG).toBe("openclaw-secops"); + expect(finalSteps[1].env?.OPENCLAW_SECURITY_APPROVERS).toBe("vincentkoc,steipete,joshavant"); + }); + + it("uses a dedicated checked-in script and detects the intended file surfaces", () => { + const workflow = readFileSync(WORKFLOW, "utf8"); + const script = readFileSync("scripts/github/security-sensitive-guard.mjs", "utf8"); + + expect(workflow).toContain("scripts/github/security-sensitive-guard.mjs"); + expect(script).toContain('"security-sensitive-changed"'); + expect(script).toContain('path: ".gitignore"'); + expect(script).toContain(".env"); + expect(script).toContain("/allow-security-sensitive-change"); + expect(script).toContain("openclaw-secops"); + expect(script).toContain("/memberships/"); + 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", () => { + const codeowners = readFileSync(CODEOWNERS, "utf8"); + expect(codeowners).toContain( + "/.github/workflows/security-sensitive-guard.yml @openclaw/openclaw-secops", + ); + expect(codeowners).toContain( + "/test/scripts/security-sensitive-guard-workflow.test.ts @openclaw/openclaw-secops", + ); + expect(codeowners).toContain( + "/test/scripts/security-sensitive-guard-script.test.ts @openclaw/openclaw-secops", + ); + expect(codeowners).toContain( + "/scripts/github/security-sensitive-guard.mjs @openclaw/openclaw-secops", + ); + expect(codeowners).toContain("/.gitignore @openclaw/openclaw-secops"); + }); +}); diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 9a3caf2dfe5..c9afb9be7c1 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -498,6 +498,15 @@ describe("scripts/test-projects changed-target routing", () => { }); }); + it("keeps security-sensitive guard workflow edits on guard workflow tests", () => { + expect( + resolveChangedTestTargetPlan([".github/workflows/security-sensitive-guard.yml"]), + ).toEqual({ + mode: "targets", + targets: ["test/scripts/security-sensitive-guard-workflow.test.ts"], + }); + }); + it("keeps Crabbox and Testbox workflow edits on workflow regression tests", () => { for (const workflowPath of [ ".github/workflows/ci-check-testbox.yml", @@ -570,6 +579,11 @@ describe("scripts/test-projects changed-target routing", () => { targets: ["test/scripts/dependency-changes-report.test.ts"], }); + expect(resolveChangedTestTargetPlan(["scripts/github/security-sensitive-guard.mjs"])).toEqual({ + mode: "targets", + targets: ["test/scripts/security-sensitive-guard-script.test.ts"], + }); + expect( resolveChangedTestTargetPlan(["scripts/dependency-ownership-surface-report.mjs"]), ).toEqual({