Skip to content

Commit 97777a7

Browse files
giodl73-repoGio Della-Libera
giodl73-repo
and
Gio Della-Libera
authored
policy: repair channel ingress findings (#99720)
* policy: repair channel ingress findings * policy: skip scoped inherited ingress repairs * fix(policy): keep doctor lint findings generic --------- Co-authored-by: Gio Della-Libera <[email protected]>
1 parent 4b3941c commit 97777a7

5 files changed

Lines changed: 322 additions & 1 deletion

File tree

‎docs/cli/policy.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,10 @@ workspace config:
947947
`agents.list[].tools.deny` when policy requires those tools to be denied
948948
- set insecure `gateway.controlUi.*` toggles to `false`
949949
- set `gateway.mode=local` when policy denies remote gateway mode
950+
- set reported channel ingress `groupPolicy` paths to `allowlist` when policy
951+
denies open group ingress
952+
- set reported channel ingress `requireMention` paths to `true` when policy
953+
requires group mentions
950954
- set `logging.redactSensitive=tools` when policy requires sensitive logging
951955
redaction
952956
- set `diagnostics.otel.captureContent=false`, or
@@ -963,6 +967,10 @@ root `tools.deny`, because adding the required tool to root config would affect
963967
more than the scoped policy target. Agent-local required-deny repairs can update
964968
the reported `agents.list[].tools.deny` path.
965969

970+
Scoped channel ingress repairs are skipped when the finding reports inherited
971+
`channels.defaults.*`, because changing the shared channel default would affect
972+
more than the scoped policy target.
973+
966974
```jsonc
967975
{
968976
"plugins": {

‎extensions/policy/src/doctor/automatic-repairs.ts‎

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ const AUTOMATIC_REPAIR_CHECK_IDS = new Set<PolicyCheckId>([
2222
CHECK_IDS.policyToolsRequiredDenyMissing,
2323
CHECK_IDS.policyGatewayControlUiInsecure,
2424
CHECK_IDS.policyGatewayRemoteEnabled,
25+
CHECK_IDS.policyIngressOpenGroupsDenied,
26+
CHECK_IDS.policyIngressGroupMentionRequired,
2527
CHECK_IDS.policyDataHandlingRedactionDisabled,
2628
CHECK_IDS.policyDataHandlingTelemetryContentCapture,
2729
]);
@@ -95,6 +97,10 @@ function applyAutomaticPatch(
9597
return disableInsecureControlUi(cfg, findings);
9698
case CHECK_IDS.policyGatewayRemoteEnabled:
9799
return disableRemoteGatewayMode(cfg, findings);
100+
case CHECK_IDS.policyIngressOpenGroupsDenied:
101+
return setFindingConfigValues(cfg, findings, "groupPolicy", "allowlist");
102+
case CHECK_IDS.policyIngressGroupMentionRequired:
103+
return setFindingConfigValues(cfg, findings, "requireMention", true);
98104
case CHECK_IDS.policyDataHandlingRedactionDisabled:
99105
if (hasScopedPolicyRequirement(findings)) {
100106
return skippedUnsafeScopedRepair(
@@ -245,6 +251,36 @@ function disableTelemetryContentCapture(cfg: OpenClawConfig): RepairPatch {
245251
};
246252
}
247253

254+
function setFindingConfigValues(
255+
cfg: OpenClawConfig,
256+
findings: readonly HealthFinding[],
257+
fieldName: string,
258+
value: unknown,
259+
): RepairPatch {
260+
const next = cloneConfig(cfg);
261+
const changes: string[] = [];
262+
const warnings: string[] = [];
263+
for (const finding of findings) {
264+
if (isScopedInheritedChannelDefaultFinding(finding)) {
265+
warnings.push(
266+
`Skipped scoped channel ingress repair for ${configPathLabel(finding.ocPath ?? "")}. The finding reports inherited channels.defaults config, so changing it would affect more than the scoped channel target.`,
267+
);
268+
continue;
269+
}
270+
if (
271+
finding.ocPath === undefined ||
272+
configPathSegments(finding.ocPath).at(-1) !== fieldName ||
273+
!setValueAtOcPath(next, finding.ocPath, value)
274+
) {
275+
continue;
276+
}
277+
changes.push(`Set ${configPathLabel(finding.ocPath)}=${String(value)} for policy conformance.`);
278+
}
279+
return changes.length > 0
280+
? { config: next as OpenClawConfig, changes: uniqueStrings(changes), warnings }
281+
: { config: cfg, changes, warnings: uniqueStrings(warnings) };
282+
}
283+
248284
function cloneConfig(cfg: OpenClawConfig): ConfigRecord {
249285
return structuredClone(cfg) as ConfigRecord;
250286
}
@@ -298,7 +334,41 @@ function configPathSegments(ocPath: string): readonly string[] {
298334
if (!ocPath.startsWith(prefix)) {
299335
return [];
300336
}
301-
return ocPath.slice(prefix.length).split("/").filter(Boolean);
337+
return splitConfigPath(ocPath.slice(prefix.length));
338+
}
339+
340+
function splitConfigPath(path: string): readonly string[] {
341+
const segments: string[] = [];
342+
let current = "";
343+
let quoted = false;
344+
let escaped = false;
345+
for (const char of path) {
346+
if (escaped) {
347+
current += char;
348+
escaped = false;
349+
continue;
350+
}
351+
if (quoted && char === "\\") {
352+
escaped = true;
353+
continue;
354+
}
355+
if (char === '"') {
356+
quoted = !quoted;
357+
continue;
358+
}
359+
if (!quoted && char === "/") {
360+
if (current !== "") {
361+
segments.push(current);
362+
}
363+
current = "";
364+
continue;
365+
}
366+
current += char;
367+
}
368+
if (current !== "") {
369+
segments.push(current);
370+
}
371+
return quoted ? [] : segments;
302372
}
303373

304374
function configPathLabel(ocPath: string): string {
@@ -317,6 +387,40 @@ function missingRequiredTool(finding: HealthFinding): string | undefined {
317387
return finding.message.match(/required tool '([^']+)'/)?.[1]?.trim();
318388
}
319389

390+
function setValueAtOcPath(cfg: ConfigRecord, ocPath: string, value: unknown): boolean {
391+
const segments = configPathSegments(ocPath);
392+
if (segments.length === 0) {
393+
return false;
394+
}
395+
let current: unknown = cfg;
396+
for (let index = 0; index < segments.length - 1; index += 1) {
397+
const segment = segments[index];
398+
if (segment === undefined || segment.startsWith("#")) {
399+
return false;
400+
}
401+
if (!isRecord(current)) {
402+
return false;
403+
}
404+
const existing = current[segment];
405+
if (existing !== undefined && !isRecord(existing)) {
406+
return false;
407+
}
408+
if (existing === undefined) {
409+
current[segment] = {};
410+
}
411+
current = current[segment];
412+
}
413+
if (!isRecord(current)) {
414+
return false;
415+
}
416+
const last = segments.at(-1);
417+
if (last === undefined || last.startsWith("#") || current[last] === value) {
418+
return false;
419+
}
420+
current[last] = value;
421+
return true;
422+
}
423+
320424
function workspaceRepairsEnabled(ctx: HealthRepairContext): boolean {
321425
const plugins = isRecord(ctx.cfg.plugins) ? ctx.cfg.plugins : {};
322426
const entries = isRecord(plugins.entries) ? plugins.entries : {};
@@ -344,6 +448,13 @@ function skippedUnsafeScopedRepair(cfg: OpenClawConfig, warning: string): Repair
344448
return { config: cfg, changes: [], warnings: [warning] };
345449
}
346450

451+
function isScopedInheritedChannelDefaultFinding(finding: HealthFinding): boolean {
452+
return (
453+
hasScopedPolicyRequirement([finding]) &&
454+
finding.ocPath?.startsWith("oc://openclaw.config/channels/defaults/") === true
455+
);
456+
}
457+
347458
function ensureRecord(parent: ConfigRecord, key: string): ConfigRecord {
348459
const current = parent[key];
349460
if (isRecord(current)) {

‎extensions/policy/src/doctor/register.test.ts‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1739,6 +1739,194 @@ describe("registerPolicyDoctorChecks", () => {
17391739
});
17401740
});
17411741

1742+
it("repairs automatic channel ingress narrowing findings", async () => {
1743+
const configPath = join(workspaceDir, "openclaw.jsonc");
1744+
const cfg = {
1745+
...cfgWithPolicy({ workspaceRepairs: true }),
1746+
channels: {
1747+
telegram: {
1748+
groupPolicy: "open",
1749+
requireMention: false,
1750+
groups: {
1751+
ops: {
1752+
topics: {
1753+
incidents: { requireMention: false },
1754+
},
1755+
},
1756+
},
1757+
},
1758+
},
1759+
} as unknown as OpenClawConfig;
1760+
await fs.writeFile(configPath, "{}", "utf-8");
1761+
await fs.writeFile(
1762+
join(workspaceDir, "policy.jsonc"),
1763+
JSON.stringify({
1764+
ingress: {
1765+
channels: {
1766+
denyOpenGroups: true,
1767+
requireMentionInGroups: true,
1768+
},
1769+
},
1770+
}),
1771+
"utf-8",
1772+
);
1773+
1774+
const openGroups = await runPolicyRepairCheck(
1775+
"policy/ingress-open-groups-denied",
1776+
repairCtx(configPath, cfg),
1777+
);
1778+
const mentions = await runPolicyRepairCheck(
1779+
"policy/ingress-group-mention-required",
1780+
repairCtx(configPath, openGroups.config),
1781+
);
1782+
1783+
expect([...openGroups.changes, ...mentions.changes]).toEqual([
1784+
"Set channels.telegram.groupPolicy=allowlist for policy conformance.",
1785+
"Set channels.telegram.groups.ops.topics.incidents.requireMention=true for policy conformance.",
1786+
"Set channels.telegram.requireMention=true for policy conformance.",
1787+
]);
1788+
expect(mentions.remainingFindings).toEqual([]);
1789+
expect(mentions.config).toMatchObject({
1790+
channels: {
1791+
telegram: {
1792+
groupPolicy: "allowlist",
1793+
requireMention: true,
1794+
groups: {
1795+
ops: {
1796+
topics: {
1797+
incidents: { requireMention: true },
1798+
},
1799+
},
1800+
},
1801+
},
1802+
},
1803+
});
1804+
});
1805+
1806+
it("repairs quoted channel ingress paths without splitting slash segments", async () => {
1807+
const configPath = join(workspaceDir, "openclaw.jsonc");
1808+
const cfg = {
1809+
...cfgWithPolicy({ workspaceRepairs: true }),
1810+
channels: {
1811+
"team/sebby": {
1812+
requireMention: false,
1813+
},
1814+
},
1815+
} as unknown as OpenClawConfig;
1816+
await fs.writeFile(configPath, "{}", "utf-8");
1817+
await fs.writeFile(
1818+
join(workspaceDir, "policy.jsonc"),
1819+
JSON.stringify({
1820+
ingress: {
1821+
channels: {
1822+
requireMentionInGroups: true,
1823+
},
1824+
},
1825+
}),
1826+
"utf-8",
1827+
);
1828+
1829+
const result = await runPolicyRepairCheck(
1830+
"policy/ingress-group-mention-required",
1831+
repairCtx(configPath, cfg),
1832+
);
1833+
1834+
expect(result.status).toBe("repaired");
1835+
expect(result.findings).toEqual([
1836+
expect.objectContaining({
1837+
ocPath: 'oc://openclaw.config/channels/"team/sebby"/requireMention',
1838+
}),
1839+
]);
1840+
expect(result.changes).toEqual([
1841+
"Set channels.team/sebby.requireMention=true for policy conformance.",
1842+
]);
1843+
expect(result.remainingFindings).toEqual([]);
1844+
expect(result.config.channels?.["team/sebby"]).toEqual({ requireMention: true });
1845+
expect(result.config.channels).not.toHaveProperty('"team');
1846+
});
1847+
1848+
it("skips scoped channel ingress repairs that would mutate inherited defaults", async () => {
1849+
const configPath = join(workspaceDir, "openclaw.jsonc");
1850+
const cfg = {
1851+
...cfgWithPolicy({ workspaceRepairs: true }),
1852+
channels: {
1853+
defaults: { groupPolicy: "open" },
1854+
telegram: {},
1855+
},
1856+
} as unknown as OpenClawConfig;
1857+
await fs.writeFile(configPath, "{}", "utf-8");
1858+
await fs.writeFile(
1859+
join(workspaceDir, "policy.jsonc"),
1860+
JSON.stringify({
1861+
scopes: {
1862+
telegram: {
1863+
channelIds: ["telegram"],
1864+
ingress: {
1865+
channels: {
1866+
denyOpenGroups: true,
1867+
},
1868+
},
1869+
},
1870+
},
1871+
}),
1872+
"utf-8",
1873+
);
1874+
1875+
const result = await runPolicyRepairCheck(
1876+
"policy/ingress-open-groups-denied",
1877+
repairCtx(configPath, cfg),
1878+
);
1879+
1880+
expect(result.status).toBe("skipped");
1881+
expect(result.reason).toBe("policy automatic repair had no config changes to apply");
1882+
expect(result.changes).toEqual([]);
1883+
expect(result.warnings).toEqual([
1884+
"Skipped scoped channel ingress repair for channels.defaults.groupPolicy. The finding reports inherited channels.defaults config, so changing it would affect more than the scoped channel target.",
1885+
]);
1886+
expect(result.config.channels?.defaults).toEqual({ groupPolicy: "open" });
1887+
expect(result.config.channels?.telegram).toEqual({});
1888+
expect(result.remainingFindings).toEqual([
1889+
expect.objectContaining({
1890+
checkId: "policy/ingress-open-groups-denied",
1891+
ocPath: "oc://openclaw.config/channels/defaults/groupPolicy",
1892+
requirement: "oc://policy.jsonc/scopes/telegram/ingress/channels/denyOpenGroups",
1893+
}),
1894+
]);
1895+
});
1896+
1897+
it("does not repair channel ingress config without workspace repair opt-in", async () => {
1898+
const configPath = join(workspaceDir, "openclaw.jsonc");
1899+
const cfg = {
1900+
...cfgWithPolicy(),
1901+
channels: { telegram: { groupPolicy: "open" } },
1902+
} as unknown as OpenClawConfig;
1903+
await fs.writeFile(configPath, "{}", "utf-8");
1904+
await fs.writeFile(
1905+
join(workspaceDir, "policy.jsonc"),
1906+
JSON.stringify({
1907+
ingress: {
1908+
channels: {
1909+
denyOpenGroups: true,
1910+
},
1911+
},
1912+
}),
1913+
"utf-8",
1914+
);
1915+
1916+
const result = await runPolicyRepairCheck(
1917+
"policy/ingress-open-groups-denied",
1918+
repairCtx(configPath, cfg),
1919+
);
1920+
1921+
expect(result.status).toBe("skipped");
1922+
expect(result.reason).toBe("workspace repairs are disabled");
1923+
expect(result.changes).toEqual([]);
1924+
expect(result.warnings).toEqual([
1925+
"Skipped policy config repair. Enable plugins.entries.policy.config.workspaceRepairs to let doctor --fix edit workspace policy config.",
1926+
]);
1927+
expect(result.config.channels?.telegram).toEqual({ groupPolicy: "open" });
1928+
});
1929+
17421930
it("dry-runs required tool deny repairs without mutating config", async () => {
17431931
const configPath = join(workspaceDir, "openclaw.jsonc");
17441932
const cfg = {

0 commit comments

Comments
 (0)