refactor(github): consolidate guard display sanitization (#123620)

This commit is contained in:
Peter Steinberger
2026-08-14 13:29:21 -07:00
committed by GitHub
parent 44fe1b15a3
commit b822e6e425
7 changed files with 31 additions and 74 deletions
+10 -15
View File
@@ -15,6 +15,7 @@ import {
normalizeGuardLoginSet,
readBoundedGitHubErrorText,
readBoundedGitHubJson,
sanitizeGuardDisplayValue,
} from "./guard-shared.mjs";
/** Marker used to identify dependency guard comments. */
@@ -159,18 +160,12 @@ function stableJson(value) {
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("`", "\\`")}\``;
return `\`${sanitizeGuardDisplayValue(value).replaceAll("`", "\\`")}\``;
}
function shellQuote(value) {
return `'${sanitizeDisplayValue(value).replaceAll("'", "'\\''")}'`;
return `'${sanitizeGuardDisplayValue(value).replaceAll("'", "'\\''")}'`;
}
function* dependencyOverrideCandidates({ comments, expectedSha, newerThan }) {
@@ -188,7 +183,7 @@ function* dependencyOverrideCandidates({ comments, expectedSha, newerThan }) {
}
yield {
login,
reason: reason ? sanitizeDisplayValue(reason) : null,
reason: reason ? sanitizeGuardDisplayValue(reason) : null,
sha: expectedSha,
url: comment.html_url,
};
@@ -313,7 +308,7 @@ export function renderAuthorizedDependencyComment(override) {
"This PR includes dependency graph changes. A repository admin or member of `@openclaw/openclaw-secops` authorized this exact head SHA with `/allow-dependencies-change`.",
"",
`- Approved SHA: ${markdownCode(override.sha)}`,
`- Approved by: @${sanitizeDisplayValue(override.login)}`,
`- Approved by: @${sanitizeGuardDisplayValue(override.login)}`,
];
if (override.reason) {
lines.push(`- Reason: ${markdownCode(override.reason)}`);
@@ -332,7 +327,7 @@ export function renderTrustedDependencyComment({ actor, headSha }) {
"This PR includes dependency graph changes. The dependency guard is informational because the PR author is a repository admin or a member of `@openclaw/openclaw-secops`.",
"",
`- Current SHA: ${markdownCode(headSha ?? "<head-sha>")}`,
`- Trusted actor: @${sanitizeDisplayValue(actor.login)}`,
`- Trusted actor: @${sanitizeGuardDisplayValue(actor.login)}`,
`- Trusted role: ${markdownCode(actor.reason)}`,
"",
"Security review is still recommended before merge when the dependency graph change is intentional.",
@@ -360,7 +355,7 @@ export function renderRemovalOnlyDependencyComment({ dependencyGraphChanges, hea
}
export function renderAutoscrubbedDependencyComment({ baseBranch, lockfileChanges, commitSha }) {
const safeBranch = sanitizeDisplayValue(baseBranch ?? "main");
const safeBranch = sanitizeGuardDisplayValue(baseBranch ?? "main");
const fileLines = lockfileChanges.map((path) => `- ${markdownCode(path)}`);
return `${dependencyGraphGuardMarker}
@@ -411,7 +406,7 @@ export function renderBlockedDependencyComment({
dependencyManifestChanges,
autoscrubStatus,
}) {
const safeBranch = sanitizeDisplayValue(baseBranch ?? "main");
const safeBranch = sanitizeGuardDisplayValue(baseBranch ?? "main");
const baseRef = shellQuote(`origin/${safeBranch}`);
const reasons = [];
for (const path of lockfileChanges) {
@@ -839,7 +834,7 @@ async function main() {
[
"## Dependency Guard",
"",
`Dependency graph change noted for trusted actor @${sanitizeDisplayValue(trustedActor.login)} and allowed to continue.`,
`Dependency graph change noted for trusted actor @${sanitizeGuardDisplayValue(trustedActor.login)} and allowed to continue.`,
].join("\n"),
);
console.log("Dependency graph change noted for trusted actor; guard is informational.");
@@ -987,7 +982,7 @@ async function main() {
[
"## Dependency Guard",
"",
`Dependency graph change authorized by @${sanitizeDisplayValue(override.login)} for ${markdownCode(override.sha)}.`,
`Dependency graph change authorized by @${sanitizeGuardDisplayValue(override.login)} for ${markdownCode(override.sha)}.`,
].join("\n"),
);
console.log("Dependency graph change authorized by trusted override.");
+6
View File
@@ -8,6 +8,12 @@ export const GITHUB_API_REQUEST_TIMEOUT_MS = 30_000;
const githubApiRetryStatuses = new Set([502, 503, 504]);
const githubApiRetryDelaysMs = [1_000, 2_000, 4_000];
export function sanitizeGuardDisplayValue(value) {
return String(value)
.replace(/[\p{Cc}]/gu, "?")
.slice(0, 240);
}
/**
* @param {string | null | undefined} value
* @param {string} [fallback]
+10 -26
View File
@@ -16,6 +16,7 @@ import {
normalizeGuardLoginSet,
readBoundedGitHubErrorText,
readBoundedGitHubJson,
sanitizeGuardDisplayValue,
} from "./guard-shared.mjs";
/** Marker used to identify security-sensitive guard comments. */
@@ -62,14 +63,8 @@ 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("`", "\\`")}\``;
return `\`${sanitizeGuardDisplayValue(value).replaceAll("`", "\\`")}\``;
}
function* securitySensitiveOverrideCandidates({ comments, expectedSha, newerThan }) {
@@ -87,7 +82,7 @@ function* securitySensitiveOverrideCandidates({ comments, expectedSha, newerThan
}
yield {
login,
reason: reason ? sanitizeDisplayValue(reason) : null,
reason: reason ? sanitizeGuardDisplayValue(reason) : null,
sha: expectedSha,
url: comment.html_url,
};
@@ -170,10 +165,6 @@ export function isSecuritySensitiveGuardTrustedForHead(comment, currentHeadSha)
);
}
export function securityApproverSet(value) {
return normalizeGuardLoginSet(value);
}
export function securitySensitiveGuardCommentAuthors(value) {
return normalizeGuardLoginSet(value, "github-actions[bot]");
}
@@ -217,7 +208,7 @@ 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)}`,
(change) => `- ${markdownCode(change.path)}: ${sanitizeGuardDisplayValue(change.reason)}`,
);
if (omittedCount > 0) {
lines.push(`- ${omittedCount} additional security-sensitive files not shown`);
@@ -252,7 +243,7 @@ export function renderAuthorizedSecuritySensitiveComment(override) {
"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)}`,
`- Approved by: @${sanitizeGuardDisplayValue(override.login)}`,
];
if (override.reason) {
lines.push(`- Reason: ${markdownCode(override.reason)}`);
@@ -270,7 +261,7 @@ export function renderTrustedSecuritySensitiveComment({ actor, headSha, changes
"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 ?? "<head-sha>")}`,
`- Trusted actor: @${sanitizeDisplayValue(actor.login)}`,
`- Trusted actor: @${sanitizeGuardDisplayValue(actor.login)}`,
`- Trusted role: ${markdownCode(actor.reason)}`,
"",
"Changed files:",
@@ -343,13 +334,6 @@ export async function findTrustedSecuritySensitiveGuardActor({
return null;
}
export function githubApi(token, options = {}) {
return createGitHubApi(token, {
...options,
userAgent: "openclaw-security-sensitive-guard",
});
}
async function writeSummary(markdown) {
const summaryPath = process.env.GITHUB_STEP_SUMMARY;
if (!summaryPath) {
@@ -374,8 +358,8 @@ async function main() {
return;
}
const api = githubApi(token);
const explicitSecurityApprovers = securityApproverSet(process.env.OPENCLAW_SECURITY_APPROVERS);
const api = createGitHubApi(token, { userAgent: "openclaw-security-sensitive-guard" });
const explicitSecurityApprovers = normalizeGuardLoginSet(process.env.OPENCLAW_SECURITY_APPROVERS);
const trustedCommentAuthors = securitySensitiveGuardCommentAuthors(
process.env.OPENCLAW_SECURITY_SENSITIVE_GUARD_COMMENT_BOTS,
);
@@ -479,7 +463,7 @@ async function main() {
[
"## Security Sensitive Guard",
"",
`Security-sensitive changes noted for trusted actor @${sanitizeDisplayValue(trustedActor.login)} and allowed to continue.`,
`Security-sensitive changes noted for trusted actor @${sanitizeGuardDisplayValue(trustedActor.login)} and allowed to continue.`,
].join("\n"),
);
console.log("Security-sensitive changes noted for trusted actor; guard is informational.");
@@ -508,7 +492,7 @@ async function main() {
[
"## Security Sensitive Guard",
"",
`Security-sensitive changes authorized by @${sanitizeDisplayValue(override.login)} for ${markdownCode(override.sha)}.`,
`Security-sensitive changes authorized by @${sanitizeGuardDisplayValue(override.login)} for ${markdownCode(override.sha)}.`,
].join("\n"),
);
console.log("Security-sensitive changes authorized by trusted override.");