mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-19 15:11:40 +00:00
policy: preview review-required gateway repairs (#99776)
* policy: preview review-required gateway repairs * docs(policy): document gateway review previews * docs(policy): document gateway review previews * docs(policy): document gateway review previews * docs(policy): document gateway review previews --------- Co-authored-by: Gio Della-Libera <giodl@microsoft.com>
This commit is contained in:
@@ -975,6 +975,13 @@ more than the scoped policy target. Gateway HTTP URL-fetch allowlist findings
|
||||
remain manual because automatic repair cannot choose the correct endpoint URL
|
||||
allowlist values.
|
||||
|
||||
Gateway bind and node-command findings stay review-required. When
|
||||
`policy/gateway-non-loopback-bind` or `policy/gateway-node-command-denied`
|
||||
can be mapped to a config path, `doctor --fix` reports the proposed
|
||||
`gateway.bind` or `gateway.nodes.denyCommands` change as skipped preview
|
||||
guidance. It does not apply the change, and the finding does not count as
|
||||
repaired until an operator reviews and updates config or policy.
|
||||
|
||||
```jsonc
|
||||
{
|
||||
"plugins": {
|
||||
|
||||
@@ -1797,6 +1797,127 @@ describe("registerPolicyDoctorChecks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("previews review-required gateway bind repair without mutating config", async () => {
|
||||
const configPath = join(workspaceDir, "openclaw.jsonc");
|
||||
const cfg = {
|
||||
...cfgWithPolicy({ workspaceRepairs: true }),
|
||||
gateway: { bind: "lan" },
|
||||
} as unknown as OpenClawConfig;
|
||||
await fs.writeFile(configPath, "{}", "utf-8");
|
||||
await fs.writeFile(
|
||||
join(workspaceDir, "policy.jsonc"),
|
||||
JSON.stringify({ gateway: { exposure: { allowNonLoopbackBind: false } } }),
|
||||
"utf-8",
|
||||
);
|
||||
|
||||
const result = await runPolicyRepairCheck(
|
||||
"policy/gateway-non-loopback-bind",
|
||||
repairCtx(configPath, cfg),
|
||||
);
|
||||
|
||||
expect(result.status).toBe("skipped");
|
||||
expect(result.reason).toBe("policy repair requires review before changing config");
|
||||
expect(result.changes).toEqual([
|
||||
"Review required: set gateway.bind=loopback for policy conformance.",
|
||||
]);
|
||||
expect(result.warnings).toEqual([
|
||||
"Review required: set gateway.bind=loopback for policy conformance.",
|
||||
]);
|
||||
expect(result.effects).toEqual([
|
||||
{
|
||||
kind: "config",
|
||||
action: "would-set-after-review",
|
||||
target: "gateway.bind=loopback",
|
||||
dryRunSafe: true,
|
||||
},
|
||||
]);
|
||||
expect(result.config.gateway?.bind).toBe("lan");
|
||||
expect(result.remainingFindings).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("previews review-required custom gateway bind repair without mutating config", async () => {
|
||||
const configPath = join(workspaceDir, "openclaw.jsonc");
|
||||
const cfg = {
|
||||
...cfgWithPolicy({ workspaceRepairs: true }),
|
||||
gateway: { bind: "custom", customBindHost: "10.0.0.4" },
|
||||
} as unknown as OpenClawConfig;
|
||||
await fs.writeFile(configPath, "{}", "utf-8");
|
||||
await fs.writeFile(
|
||||
join(workspaceDir, "policy.jsonc"),
|
||||
JSON.stringify({ gateway: { exposure: { allowNonLoopbackBind: false } } }),
|
||||
"utf-8",
|
||||
);
|
||||
|
||||
const result = await runPolicyRepairCheck(
|
||||
"policy/gateway-non-loopback-bind",
|
||||
repairCtx(configPath, cfg),
|
||||
);
|
||||
|
||||
expect(result.findings).toEqual([
|
||||
expect.objectContaining({
|
||||
ocPath: "oc://openclaw.config/gateway/customBindHost",
|
||||
}),
|
||||
]);
|
||||
expect(result.status).toBe("skipped");
|
||||
expect(result.changes).toEqual([
|
||||
"Review required: set gateway.bind=loopback for policy conformance.",
|
||||
]);
|
||||
expect(result.warnings).toEqual([
|
||||
"Review required: set gateway.bind=loopback for policy conformance.",
|
||||
]);
|
||||
expect(result.effects).toEqual([
|
||||
{
|
||||
kind: "config",
|
||||
action: "would-set-after-review",
|
||||
target: "gateway.bind=loopback",
|
||||
dryRunSafe: true,
|
||||
},
|
||||
]);
|
||||
expect(result.config.gateway).toMatchObject({
|
||||
bind: "custom",
|
||||
customBindHost: "10.0.0.4",
|
||||
});
|
||||
expect(result.remainingFindings).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("previews review-required gateway node command repairs without mutating config", async () => {
|
||||
const configPath = join(workspaceDir, "openclaw.jsonc");
|
||||
const cfg = {
|
||||
...cfgWithPolicy({ workspaceRepairs: true }),
|
||||
gateway: { nodes: { denyCommands: ["mcp.help"] } },
|
||||
} as unknown as OpenClawConfig;
|
||||
await fs.writeFile(configPath, "{}", "utf-8");
|
||||
await fs.writeFile(
|
||||
join(workspaceDir, "policy.jsonc"),
|
||||
JSON.stringify({ gateway: { nodes: { denyCommands: ["mcp.help", "system.run"] } } }),
|
||||
"utf-8",
|
||||
);
|
||||
|
||||
const result = await runPolicyRepairCheck(
|
||||
"policy/gateway-node-command-denied",
|
||||
repairCtx(configPath, cfg),
|
||||
);
|
||||
|
||||
expect(result.status).toBe("skipped");
|
||||
expect(result.reason).toBe("policy repair requires review before changing config");
|
||||
expect(result.changes).toEqual([
|
||||
"Review required: add system.run to gateway.nodes.denyCommands for policy conformance.",
|
||||
]);
|
||||
expect(result.warnings).toEqual([
|
||||
"Review required: add system.run to gateway.nodes.denyCommands for policy conformance.",
|
||||
]);
|
||||
expect(result.effects).toEqual([
|
||||
{
|
||||
kind: "config",
|
||||
action: "would-append-after-review",
|
||||
target: "gateway.nodes.denyCommands += system.run",
|
||||
dryRunSafe: true,
|
||||
},
|
||||
]);
|
||||
expect(result.config.gateway?.nodes?.denyCommands).toEqual(["mcp.help"]);
|
||||
expect(result.remainingFindings).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("repairs automatic channel ingress narrowing findings", async () => {
|
||||
const configPath = join(workspaceDir, "openclaw.jsonc");
|
||||
const cfg = {
|
||||
@@ -2161,7 +2282,7 @@ describe("registerPolicyDoctorChecks", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not register repair for review-required policy findings", () => {
|
||||
it("does not register repair for non-previewable policy findings", () => {
|
||||
const check = registerChecks().find(
|
||||
(entry) => entry.id === "policy/gateway-http-url-fetch-unrestricted",
|
||||
);
|
||||
|
||||
138
extensions/policy/src/doctor/review-required-repairs.ts
Normal file
138
extensions/policy/src/doctor/review-required-repairs.ts
Normal file
@@ -0,0 +1,138 @@
|
||||
// Policy review-required repairs surface proposed config changes without applying them.
|
||||
import type {
|
||||
HealthFinding,
|
||||
HealthRepairContext,
|
||||
HealthRepairEffect,
|
||||
HealthRepairResult,
|
||||
} from "openclaw/plugin-sdk/health";
|
||||
import { POLICY_FIX_METADATA_BY_CHECK_ID } from "./fix-metadata.js";
|
||||
import { CHECK_IDS, type POLICY_CHECK_IDS } from "./metadata.js";
|
||||
|
||||
type PolicyCheckId = (typeof POLICY_CHECK_IDS)[number];
|
||||
|
||||
const REVIEW_REQUIRED_REPAIR_CHECK_IDS = new Set<PolicyCheckId>([
|
||||
CHECK_IDS.policyGatewayNonLoopbackBind,
|
||||
CHECK_IDS.policyGatewayNodeCommandDenied,
|
||||
]);
|
||||
|
||||
export function previewPolicyReviewRequiredRepair(
|
||||
_ctx: HealthRepairContext,
|
||||
findings: readonly HealthFinding[],
|
||||
checkId: PolicyCheckId,
|
||||
): Promise<HealthRepairResult> {
|
||||
const metadata = POLICY_FIX_METADATA_BY_CHECK_ID.get(checkId);
|
||||
if (!REVIEW_REQUIRED_REPAIR_CHECK_IDS.has(checkId) || metadata?.fixClass !== "reviewRequired") {
|
||||
return Promise.resolve({
|
||||
status: "skipped",
|
||||
reason: "policy finding does not have a review-required repair preview",
|
||||
changes: [],
|
||||
});
|
||||
}
|
||||
if (
|
||||
findings.length === 0 ||
|
||||
findings.some(
|
||||
(finding) =>
|
||||
finding.checkId !== checkId ||
|
||||
POLICY_FIX_METADATA_BY_CHECK_ID.get(finding.checkId)?.fixClass !== "reviewRequired",
|
||||
)
|
||||
) {
|
||||
return Promise.resolve({
|
||||
status: "skipped",
|
||||
reason: "policy finding is not classified as review-required",
|
||||
changes: [],
|
||||
});
|
||||
}
|
||||
|
||||
const previews = findings.flatMap((finding) => previewForFinding(finding, checkId));
|
||||
if (previews.length === 0) {
|
||||
return Promise.resolve({
|
||||
status: "skipped",
|
||||
reason: "policy review-required repair had no previewable config changes",
|
||||
changes: [],
|
||||
});
|
||||
}
|
||||
|
||||
return Promise.resolve({
|
||||
status: "skipped",
|
||||
reason: "policy repair requires review before changing config",
|
||||
changes: uniqueStrings(previews.map((preview) => preview.change)),
|
||||
warnings: uniqueStrings(previews.map((preview) => preview.change)),
|
||||
effects: uniqueEffects(previews.map((preview) => preview.effect)),
|
||||
});
|
||||
}
|
||||
|
||||
function previewForFinding(
|
||||
finding: HealthFinding,
|
||||
checkId: PolicyCheckId,
|
||||
): readonly { readonly change: string; readonly effect: HealthRepairEffect }[] {
|
||||
switch (checkId) {
|
||||
case CHECK_IDS.policyGatewayNonLoopbackBind:
|
||||
return previewGatewayLoopbackBind(finding);
|
||||
case CHECK_IDS.policyGatewayNodeCommandDenied:
|
||||
return previewGatewayNodeDenyCommand(finding);
|
||||
default:
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
function previewGatewayLoopbackBind(
|
||||
finding: HealthFinding,
|
||||
): readonly { readonly change: string; readonly effect: HealthRepairEffect }[] {
|
||||
if (
|
||||
finding.ocPath !== "oc://openclaw.config/gateway/bind" &&
|
||||
finding.ocPath !== "oc://openclaw.config/gateway/customBindHost"
|
||||
) {
|
||||
return [];
|
||||
}
|
||||
return [
|
||||
{
|
||||
change: "Review required: set gateway.bind=loopback for policy conformance.",
|
||||
effect: {
|
||||
kind: "config",
|
||||
action: "would-set-after-review",
|
||||
target: "gateway.bind=loopback",
|
||||
dryRunSafe: true,
|
||||
},
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
function previewGatewayNodeDenyCommand(
|
||||
finding: HealthFinding,
|
||||
): readonly { readonly change: string; readonly effect: HealthRepairEffect }[] {
|
||||
const command = finding.message.match(/Gateway node command '([^']+)'/)?.[1]?.trim();
|
||||
if (
|
||||
command === undefined ||
|
||||
command === "" ||
|
||||
finding.ocPath !== "oc://openclaw.config/gateway/nodes/denyCommands"
|
||||
) {
|
||||
return [];
|
||||
}
|
||||
return [
|
||||
{
|
||||
change: `Review required: add ${command} to gateway.nodes.denyCommands for policy conformance.`,
|
||||
effect: {
|
||||
kind: "config",
|
||||
action: "would-append-after-review",
|
||||
target: `gateway.nodes.denyCommands += ${command}`,
|
||||
dryRunSafe: true,
|
||||
},
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
function uniqueStrings(values: readonly string[]): readonly string[] {
|
||||
return [...new Set(values)];
|
||||
}
|
||||
|
||||
function uniqueEffects(values: readonly HealthRepairEffect[]): readonly HealthRepairEffect[] {
|
||||
const seen = new Set<string>();
|
||||
return values.filter((value) => {
|
||||
const key = JSON.stringify(value);
|
||||
if (seen.has(key)) {
|
||||
return false;
|
||||
}
|
||||
seen.add(key);
|
||||
return true;
|
||||
});
|
||||
}
|
||||
@@ -3,6 +3,7 @@ import type { HealthCheck, HealthFinding } from "openclaw/plugin-sdk/health";
|
||||
import type { PolicyEvidence } from "../../policy-state.js";
|
||||
import { repairPolicyAutomaticNarrower } from "../automatic-repairs.js";
|
||||
import { CHECK_IDS } from "../metadata.js";
|
||||
import { previewPolicyReviewRequiredRepair } from "../review-required-repairs.js";
|
||||
import type { PolicyDoctorCheckDeps } from "../types.js";
|
||||
import { readPolicyBoolean, readStringList } from "../utils.js";
|
||||
|
||||
@@ -17,6 +18,13 @@ export function createPolicyGatewayChecks(deps: PolicyDoctorCheckDeps): readonly
|
||||
async detect(ctx) {
|
||||
return findingsForCheck(await evaluatePolicy(ctx), CHECK_IDS.policyGatewayNonLoopbackBind);
|
||||
},
|
||||
repair(ctx, findings) {
|
||||
return previewPolicyReviewRequiredRepair(
|
||||
ctx,
|
||||
findings,
|
||||
CHECK_IDS.policyGatewayNonLoopbackBind,
|
||||
);
|
||||
},
|
||||
};
|
||||
const policyGatewayAuthDisabledCheck: HealthCheck = {
|
||||
id: CHECK_IDS.policyGatewayAuthDisabled,
|
||||
@@ -108,6 +116,13 @@ export function createPolicyGatewayChecks(deps: PolicyDoctorCheckDeps): readonly
|
||||
async detect(ctx) {
|
||||
return findingsForCheck(await evaluatePolicy(ctx), CHECK_IDS.policyGatewayNodeCommandDenied);
|
||||
},
|
||||
repair(ctx, findings) {
|
||||
return previewPolicyReviewRequiredRepair(
|
||||
ctx,
|
||||
findings,
|
||||
CHECK_IDS.policyGatewayNodeCommandDenied,
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
return [
|
||||
|
||||
@@ -292,7 +292,7 @@ describe("runDoctorHealthRepairs", () => {
|
||||
return {
|
||||
status: "skipped",
|
||||
reason: "manual confirmation required",
|
||||
changes: [],
|
||||
changes: ["Review required before changing gateway.mode."],
|
||||
};
|
||||
},
|
||||
}),
|
||||
@@ -304,6 +304,7 @@ describe("runDoctorHealthRepairs", () => {
|
||||
expect(result.checksRepaired).toBe(0);
|
||||
expect(result.checksValidated).toBe(0);
|
||||
expect(result.remainingFindings).toEqual([]);
|
||||
expect(result.changes).toEqual(["Review required before changing gateway.mode."]);
|
||||
expect(result.warnings).toEqual(["test/skipped repair skipped: manual confirmation required"]);
|
||||
});
|
||||
|
||||
|
||||
@@ -130,6 +130,7 @@ async function runSplitHealthCheck(
|
||||
warnings.push(...(result.warnings ?? []));
|
||||
diffs.push(...(result.diffs ?? []));
|
||||
effects.push(...(result.effects ?? []));
|
||||
changes.push(...result.changes);
|
||||
const status = result.status ?? "repaired";
|
||||
if (status !== "repaired") {
|
||||
warnings.push(`${check.id} repair ${status}${result.reason ? `: ${result.reason}` : ""}`);
|
||||
@@ -138,7 +139,6 @@ async function runSplitHealthCheck(
|
||||
if (result.config !== undefined && opts.dryRun !== true) {
|
||||
cfg = result.config;
|
||||
}
|
||||
changes.push(...result.changes);
|
||||
checksRepaired++;
|
||||
if (opts.dryRun === true) {
|
||||
return repairRunResult(cfg, findings, remainingFindings, changes, warnings, diffs, effects, {
|
||||
|
||||
Reference in New Issue
Block a user