fix(router): use deterministic security gates in finalizers

This commit is contained in:
Peter Steinberger 2026-04-29 08:33:09 +01:00
parent 7b7853b340
commit ca15ce3162
No known key found for this signature in database
4 changed files with 33 additions and 9 deletions

View File

@ -104,7 +104,7 @@ no-PR outcome and the audit file records the skip.
## Security Boundary
Security-sensitive work is centrally managed outside ProjectClownfish. The importer skips those clusters by default, the job schema rejects `security_sensitive: true`, the planner marks hydrated security-sensitive items only from explicit security labels or structured ClawSweeper security markers, `review-results` fails mutating recommendations against those items, and `apply-result` blocks live targets with security-sensitive labels/title/body.
Security-sensitive work is centrally managed outside ProjectClownfish. The importer skips those clusters by default, the job schema rejects `security_sensitive: true`, the planner marks hydrated security-sensitive items only from explicit security labels or structured ClawSweeper security markers, `review-results` fails mutating recommendations against those items, and live merge/close finalizers re-check those deterministic signals before mutating.
Use the central OpenClaw security path for:
@ -113,7 +113,7 @@ Use the central OpenClaw security path for:
- SSRF, XSS, CSRF, RCE, auth-token leakage, or similar security-class bugs.
This boundary is intentionally conservative. If a cluster is borderline, do not stage it here.
For adopted automerge jobs, do not classify security from review prose. ClawSweeper must emit a deterministic marker such as `<!-- clawsweeper-security:security-sensitive item=<pr> sha=<head-sha> -->` when the automerge loop should route the PR to central security handling.
For adopted automerge jobs, do not classify security from review prose at planning, repair, merge, or closeout time. ClawSweeper must emit a deterministic marker such as `<!-- clawsweeper-security:security-sensitive item=<pr> sha=<head-sha> -->` when the automerge loop should route the PR to central security handling.
## Auto-Closure

View File

@ -3,7 +3,7 @@ import fs from "node:fs";
import path from "node:path";
import { execFileSync } from "node:child_process";
import { createHash } from "node:crypto";
import { assertAllowedOwner, hasSecuritySignalText, parseArgs, parseJob, repoRoot, validateJob } from "./lib.mjs";
import { assertAllowedOwner, hasDeterministicSecuritySignal, parseArgs, parseJob, repoRoot, validateJob } from "./lib.mjs";
import { defaultCloseComment, externalMessageProvenance } from "./external-messages.mjs";
const MAINTAINER_AUTHOR_ASSOCIATIONS = new Set(["OWNER", "MEMBER", "COLLABORATOR"]);
@ -774,7 +774,9 @@ function validateLowSignalLiveState(repo, target, live, kind) {
}
function hasSecuritySignal(issue) {
return hasSecuritySignalText(issue.title, issue.body, issue.labels ?? []);
if (hasDeterministicSecuritySignal({ labels: issue.labels ?? [] })) return true;
const comments = ghPaged(`repos/${result.repo}/issues/${issue.number}/comments?per_page=100`).map((comment) => comment.body ?? "");
return hasDeterministicSecuritySignal({ comments });
}
function fetchIssue(repo, number) {

View File

@ -5,7 +5,7 @@ import { execFileSync } from "node:child_process";
import {
assertLiveWorkerCapacity,
currentProjectRepo,
hasSecuritySignalText,
hasDeterministicSecuritySignal,
parseArgs,
parseJob,
readMaxLiveWorkers,
@ -170,7 +170,7 @@ function classifyPullRequest(pull, publishedRecords) {
if (pull.isDraft) blockers.push("draft");
if (String(pull.baseRefName ?? "") !== "main") blockers.push(`base is ${pull.baseRefName || "unknown"}`);
if (hasSecuritySignalText(pull.title, pull.body, pull.labels ?? [])) blockers.push("security_hold");
if (hasDeterministicPullSecuritySignal(pull)) blockers.push("security_hold");
if (isSecurityRoutedAction(latestApplyAction)) blockers.push("security_route");
if (pull.mergeable === "UNKNOWN") {
blockers.push("mergeability_unknown");
@ -298,6 +298,16 @@ function summarizeChecks(checks) {
};
}
function hasDeterministicPullSecuritySignal(pull) {
return hasDeterministicSecuritySignal({
labels: pull.labels ?? [],
comments: [
(pull.comments ?? []).map((comment) => comment.body),
(pull.reviews ?? []).map((review) => review.body),
],
});
}
function displayCheckName(check) {
const workflow = String(check.workflowName ?? "");
const name = String(check.name ?? check.context ?? "unknown check");

View File

@ -2,7 +2,7 @@
import fs from "node:fs";
import path from "node:path";
import { execFileSync } from "node:child_process";
import { assertAllowedOwner, hasSecuritySignalText, parseArgs, parseJob, repoRoot, validateJob } from "./lib.mjs";
import { assertAllowedOwner, hasDeterministicSecuritySignal, parseArgs, parseJob, repoRoot, validateJob } from "./lib.mjs";
import { externalMessageProvenance, postMergeCloseoutComment } from "./external-messages.mjs";
const PASSING_CHECK_CONCLUSIONS = new Set(["SUCCESS", "SKIPPED", "NEUTRAL"]);
@ -242,7 +242,7 @@ function finalizePostMergeCloseout({ action, actionName, target, fixRef, fixUrl,
merge_commit_sha: finalized.merge_commit_sha ?? null,
};
}
if (hasSecuritySignalText(live.title, live.body, live.labels ?? [])) {
if (hasLiveSecuritySignal(target, live.labels ?? [])) {
return { ...base, status: "blocked", reason: "security-sensitive target requires central security triage" };
}
if (dryRun) {
@ -300,11 +300,23 @@ function ensureLabel(repo, name, color, description) {
}
}
function hasLiveSecuritySignal(number, labels) {
if (hasDeterministicSecuritySignal({ labels })) return true;
const bodies = ghWithRetry([
"api",
`repos/${result.repo}/issues/${number}/comments?per_page=100`,
"--paginate",
"--jq",
".[].body",
]);
return hasDeterministicSecuritySignal({ comments: [bodies] });
}
function validateMergeableFixPr({ pull, view, preflight }) {
if (pull.state !== "open") return `pull request is ${pull.state}`;
if (pull.draft || view.isDraft) return "pull request is draft";
if (String(view.baseRefName ?? pull.base?.ref ?? "") !== "main") return "pull request base is not main";
if (hasSecuritySignalText(pull.title, pull.body, pull.labels ?? [])) {
if (hasLiveSecuritySignal(pull.number, pull.labels ?? [])) {
return "security-sensitive PR requires central security triage";
}
if (view.mergeable !== "MERGEABLE") return `mergeable state is ${view.mergeable || "unknown"}`;