mirror of https://github.com/openclaw/openclaw.git
feat(zalouser): audit mutable group allowlists
This commit is contained in:
parent
88244c0942
commit
f36d8c09f1
|
|
@ -107,6 +107,40 @@ describe("doctor config flow", () => {
|
|||
).toBe(false);
|
||||
});
|
||||
|
||||
it("warns on mutable Zalouser group entries when dangerous name matching is disabled", async () => {
|
||||
const doctorWarnings = await collectDoctorWarnings({
|
||||
channels: {
|
||||
zalouser: {
|
||||
groups: {
|
||||
"Ops Room": { allow: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(
|
||||
doctorWarnings.some(
|
||||
(line) =>
|
||||
line.includes("mutable allowlist") && line.includes("channels.zalouser.groups: Ops Room"),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("does not warn on mutable Zalouser group entries when dangerous name matching is enabled", async () => {
|
||||
const doctorWarnings = await collectDoctorWarnings({
|
||||
channels: {
|
||||
zalouser: {
|
||||
dangerouslyAllowNameMatching: true,
|
||||
groups: {
|
||||
"Ops Room": { allow: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(doctorWarnings.some((line) => line.includes("channels.zalouser.groups"))).toBe(false);
|
||||
});
|
||||
|
||||
it("warns when imessage group allowlist is empty even if allowFrom is set", async () => {
|
||||
const doctorWarnings = await collectDoctorWarnings({
|
||||
channels: {
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ import {
|
|||
isMSTeamsMutableAllowEntry,
|
||||
isMattermostMutableAllowEntry,
|
||||
isSlackMutableAllowEntry,
|
||||
isZalouserMutableGroupEntry,
|
||||
} from "../security/mutable-allowlist-detectors.js";
|
||||
import { inspectTelegramAccount } from "../telegram/account-inspect.js";
|
||||
import { listTelegramAccountIds, resolveTelegramAccount } from "../telegram/accounts.js";
|
||||
|
|
@ -885,6 +886,27 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
|
|||
}
|
||||
}
|
||||
|
||||
for (const scope of collectProviderDangerousNameMatchingScopes(cfg, "zalouser")) {
|
||||
if (scope.dangerousNameMatchingEnabled) {
|
||||
continue;
|
||||
}
|
||||
const groups = asObjectRecord(scope.account.groups);
|
||||
if (!groups) {
|
||||
continue;
|
||||
}
|
||||
for (const entry of Object.keys(groups)) {
|
||||
if (!isZalouserMutableGroupEntry(entry)) {
|
||||
continue;
|
||||
}
|
||||
hits.push({
|
||||
channel: "zalouser",
|
||||
path: `${scope.prefix}.groups`,
|
||||
entry,
|
||||
dangerousFlagPath: scope.dangerousFlagPath,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return hits;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -18,7 +18,10 @@ import { readChannelAllowFromStore } from "../pairing/pairing-store.js";
|
|||
import { normalizeStringEntries } from "../shared/string-normalization.js";
|
||||
import type { SecurityAuditFinding, SecurityAuditSeverity } from "./audit.js";
|
||||
import { resolveDmAllowState } from "./dm-policy-shared.js";
|
||||
import { isDiscordMutableAllowEntry } from "./mutable-allowlist-detectors.js";
|
||||
import {
|
||||
isDiscordMutableAllowEntry,
|
||||
isZalouserMutableGroupEntry,
|
||||
} from "./mutable-allowlist-detectors.js";
|
||||
|
||||
function normalizeAllowFromList(list: Array<string | number> | undefined | null): string[] {
|
||||
return normalizeStringEntries(Array.isArray(list) ? list : undefined);
|
||||
|
|
@ -44,6 +47,22 @@ function addDiscordNameBasedEntries(params: {
|
|||
}
|
||||
}
|
||||
|
||||
function addZalouserMutableGroupEntries(params: {
|
||||
target: Set<string>;
|
||||
groups: unknown;
|
||||
source: string;
|
||||
}): void {
|
||||
if (!params.groups || typeof params.groups !== "object" || Array.isArray(params.groups)) {
|
||||
return;
|
||||
}
|
||||
for (const key of Object.keys(params.groups as Record<string, unknown>)) {
|
||||
if (!isZalouserMutableGroupEntry(key)) {
|
||||
continue;
|
||||
}
|
||||
params.target.add(`${params.source}:${key}`);
|
||||
}
|
||||
}
|
||||
|
||||
function collectInvalidTelegramAllowFromEntries(params: {
|
||||
entries: unknown;
|
||||
target: Set<string>;
|
||||
|
|
@ -467,6 +486,45 @@ export async function collectChannelSecurityFindings(params: {
|
|||
}
|
||||
}
|
||||
|
||||
if (plugin.id === "zalouser") {
|
||||
const zalouserCfg =
|
||||
(account as { config?: Record<string, unknown> } | null)?.config ??
|
||||
({} as Record<string, unknown>);
|
||||
const dangerousNameMatchingEnabled = isDangerousNameMatchingEnabled(zalouserCfg);
|
||||
const zalouserPathPrefix =
|
||||
orderedAccountIds.length > 1 || hasExplicitAccountPath
|
||||
? `channels.zalouser.accounts.${accountId}`
|
||||
: "channels.zalouser";
|
||||
const mutableGroupEntries = new Set<string>();
|
||||
addZalouserMutableGroupEntries({
|
||||
target: mutableGroupEntries,
|
||||
groups: zalouserCfg.groups,
|
||||
source: `${zalouserPathPrefix}.groups`,
|
||||
});
|
||||
if (mutableGroupEntries.size > 0) {
|
||||
const examples = Array.from(mutableGroupEntries).slice(0, 5);
|
||||
const more =
|
||||
mutableGroupEntries.size > examples.length
|
||||
? ` (+${mutableGroupEntries.size - examples.length} more)`
|
||||
: "";
|
||||
findings.push({
|
||||
checkId: "channels.zalouser.groups.mutable_entries",
|
||||
severity: dangerousNameMatchingEnabled ? "info" : "warn",
|
||||
title: dangerousNameMatchingEnabled
|
||||
? "Zalouser group routing uses break-glass name matching"
|
||||
: "Zalouser group routing contains mutable group entries",
|
||||
detail: dangerousNameMatchingEnabled
|
||||
? "Zalouser group-name routing is explicitly enabled via dangerouslyAllowNameMatching. This mutable-identity mode is operator-selected break-glass behavior and out-of-scope for vulnerability reports by itself. " +
|
||||
`Found: ${examples.join(", ")}${more}.`
|
||||
: "Zalouser group auth is ID-only by default, so unresolved group-name or slug entries are ignored for auth and can drift from the intended trusted group. " +
|
||||
`Found: ${examples.join(", ")}${more}.`,
|
||||
remediation: dangerousNameMatchingEnabled
|
||||
? "Prefer stable Zalo group IDs (for example group:<id> or provider-native g- ids), then disable dangerouslyAllowNameMatching."
|
||||
: "Prefer stable Zalo group IDs in channels.zalouser.groups, or explicitly opt in with dangerouslyAllowNameMatching=true if you accept mutable group-name matching.",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (plugin.id === "slack") {
|
||||
const slackCfg =
|
||||
(account as { config?: Record<string, unknown>; dm?: Record<string, unknown> } | null)
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ const execDockerRawUnavailable: NonNullable<SecurityAuditOptions["execDockerRawF
|
|||
};
|
||||
|
||||
function stubChannelPlugin(params: {
|
||||
id: "discord" | "slack" | "telegram";
|
||||
id: "discord" | "slack" | "telegram" | "zalouser";
|
||||
label: string;
|
||||
resolveAccount: (cfg: OpenClawConfig, accountId: string | null | undefined) => unknown;
|
||||
inspectAccount?: (cfg: OpenClawConfig, accountId: string | null | undefined) => unknown;
|
||||
|
|
@ -110,6 +110,27 @@ const telegramPlugin = stubChannelPlugin({
|
|||
},
|
||||
});
|
||||
|
||||
const zalouserPlugin = stubChannelPlugin({
|
||||
id: "zalouser",
|
||||
label: "Zalo Personal",
|
||||
listAccountIds: (cfg) => {
|
||||
const channel = (cfg.channels as Record<string, unknown> | undefined)?.zalouser as
|
||||
| { accounts?: Record<string, unknown> }
|
||||
| undefined;
|
||||
const ids = Object.keys(channel?.accounts ?? {});
|
||||
return ids.length > 0 ? ids : ["default"];
|
||||
},
|
||||
resolveAccount: (cfg, accountId) => {
|
||||
const resolvedAccountId = typeof accountId === "string" && accountId ? accountId : "default";
|
||||
const channel = (cfg.channels as Record<string, unknown> | undefined)?.zalouser as
|
||||
| { accounts?: Record<string, unknown> }
|
||||
| undefined;
|
||||
const base = (channel ?? {}) as Record<string, unknown>;
|
||||
const account = channel?.accounts?.[resolvedAccountId] ?? {};
|
||||
return { config: { ...base, ...account } };
|
||||
},
|
||||
});
|
||||
|
||||
function successfulProbeResult(url: string) {
|
||||
return {
|
||||
ok: true,
|
||||
|
|
@ -2324,6 +2345,75 @@ description: test skill
|
|||
});
|
||||
});
|
||||
|
||||
it("warns when Zalouser group routing contains mutable group entries", async () => {
|
||||
await withChannelSecurityStateDir(async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
channels: {
|
||||
zalouser: {
|
||||
enabled: true,
|
||||
groups: {
|
||||
"Ops Room": { allow: true },
|
||||
"group:g-123": { allow: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: false,
|
||||
includeChannelSecurity: true,
|
||||
plugins: [zalouserPlugin],
|
||||
});
|
||||
|
||||
const finding = res.findings.find(
|
||||
(entry) => entry.checkId === "channels.zalouser.groups.mutable_entries",
|
||||
);
|
||||
expect(finding).toBeDefined();
|
||||
expect(finding?.severity).toBe("warn");
|
||||
expect(finding?.detail).toContain("channels.zalouser.groups:Ops Room");
|
||||
expect(finding?.detail).not.toContain("group:g-123");
|
||||
});
|
||||
});
|
||||
|
||||
it("marks Zalouser mutable group routing as break-glass when dangerous matching is enabled", async () => {
|
||||
await withChannelSecurityStateDir(async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
channels: {
|
||||
zalouser: {
|
||||
enabled: true,
|
||||
dangerouslyAllowNameMatching: true,
|
||||
groups: {
|
||||
"Ops Room": { allow: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: false,
|
||||
includeChannelSecurity: true,
|
||||
plugins: [zalouserPlugin],
|
||||
});
|
||||
|
||||
const finding = res.findings.find(
|
||||
(entry) => entry.checkId === "channels.zalouser.groups.mutable_entries",
|
||||
);
|
||||
expect(finding).toBeDefined();
|
||||
expect(finding?.severity).toBe("info");
|
||||
expect(finding?.detail).toContain("out-of-scope");
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "channels.zalouser.allowFrom.dangerous_name_matching_enabled",
|
||||
severity: "info",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("does not warn when Discord allowlists use ID-style entries only", async () => {
|
||||
await withChannelSecurityStateDir(async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
|
|
|
|||
|
|
@ -99,3 +99,24 @@ export function isIrcMutableAllowEntry(raw: string): boolean {
|
|||
|
||||
return !normalized.includes("!") && !normalized.includes("@");
|
||||
}
|
||||
|
||||
export function isZalouserMutableGroupEntry(raw: string): boolean {
|
||||
const text = raw.trim();
|
||||
if (!text || text === "*") {
|
||||
return false;
|
||||
}
|
||||
|
||||
const normalized = text
|
||||
.replace(/^(zalouser|zlu):/i, "")
|
||||
.replace(/^group:/i, "")
|
||||
.trim();
|
||||
|
||||
if (!normalized) {
|
||||
return false;
|
||||
}
|
||||
if (/^\d+$/.test(normalized)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return !/^g-\S+$/i.test(normalized);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue