mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-09 22:14:34 +00:00
Security audit: suggest valid gateway.nodes.denyCommands entries (#29713)
Merged via squash.
Prepared head SHA: db23298f98
Co-authored-by: liquidhorizon88-bot <257047709+liquidhorizon88-bot@users.noreply.github.com>
Co-authored-by: grp06 <1573959+grp06@users.noreply.github.com>
Reviewed-by: @grp06
This commit is contained in:
committed by
GitHub
parent
e8cb0484ce
commit
d95cf256e7
@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
### Fixes
|
### Fixes
|
||||||
|
|
||||||
- Security/auth labels: remove token and API-key snippets from user-facing auth status labels so `/status` and `/models` do not expose credential fragments. (#33262) thanks @cu1ch3n.
|
- Security/auth labels: remove token and API-key snippets from user-facing auth status labels so `/status` and `/models` do not expose credential fragments. (#33262) thanks @cu1ch3n.
|
||||||
|
- Security/audit denyCommands guidance: suggest likely exact node command IDs for unknown `gateway.nodes.denyCommands` entries so ineffective denylist entries are easier to correct. (#29713) thanks @liquidhorizon88-bot.
|
||||||
- Docs/security hardening guidance: document Docker `DOCKER-USER` + UFW policy and add cross-linking from Docker install docs for VPS/public-host setups. (#27613) thanks @dorukardahan.
|
- Docs/security hardening guidance: document Docker `DOCKER-USER` + UFW policy and add cross-linking from Docker install docs for VPS/public-host setups. (#27613) thanks @dorukardahan.
|
||||||
- Docs/security threat-model links: replace relative `.md` links with Mintlify-compatible root-relative routes in security docs to prevent broken internal navigation. (#27698) thanks @clawdoo.
|
- Docs/security threat-model links: replace relative `.md` links with Mintlify-compatible root-relative routes in security docs to prevent broken internal navigation. (#27698) thanks @clawdoo.
|
||||||
- iOS/Voice timing safety: guard system speech start/finish callbacks to the active utterance to avoid misattributed start events during rapid stop/restart cycles. (#33304) thanks @mbelinky; original implementation direction by @ngutman.
|
- iOS/Voice timing safety: guard system speech start/finish callbacks to the active utterance to avoid misattributed start events during rapid stop/restart cycles. (#33304) thanks @mbelinky; original implementation direction by @ngutman.
|
||||||
|
|||||||
@@ -77,9 +77,12 @@ describe("createFeishuWSClient proxy handling", () => {
|
|||||||
expect(options?.agent).toBeUndefined();
|
expect(options?.agent).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("prefers HTTPS proxy vars over HTTP proxy vars across runtimes", () => {
|
it("uses proxy env precedence: https_proxy first, then HTTPS_PROXY, then http_proxy/HTTP_PROXY", () => {
|
||||||
|
// NOTE: On Windows, environment variables are case-insensitive, so it's not
|
||||||
|
// possible to set both https_proxy and HTTPS_PROXY to different values.
|
||||||
|
// Keep this test cross-platform by asserting precedence via mutually-exclusive
|
||||||
|
// setups.
|
||||||
process.env.https_proxy = "http://lower-https:8001";
|
process.env.https_proxy = "http://lower-https:8001";
|
||||||
process.env.HTTPS_PROXY = "http://upper-https:8002";
|
|
||||||
process.env.http_proxy = "http://lower-http:8003";
|
process.env.http_proxy = "http://lower-http:8003";
|
||||||
process.env.HTTP_PROXY = "http://upper-http:8004";
|
process.env.HTTP_PROXY = "http://upper-http:8004";
|
||||||
|
|
||||||
@@ -108,6 +111,18 @@ describe("createFeishuWSClient proxy handling", () => {
|
|||||||
expect(options.agent).toEqual({ proxyUrl: expectedHttpsProxy });
|
expect(options.agent).toEqual({ proxyUrl: expectedHttpsProxy });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("uses HTTPS_PROXY when https_proxy is unset", () => {
|
||||||
|
process.env.HTTPS_PROXY = "http://upper-https:8002";
|
||||||
|
process.env.http_proxy = "http://lower-http:8003";
|
||||||
|
|
||||||
|
createFeishuWSClient(baseAccount);
|
||||||
|
|
||||||
|
expect(httpsProxyAgentCtorMock).toHaveBeenCalledTimes(1);
|
||||||
|
expect(httpsProxyAgentCtorMock).toHaveBeenCalledWith("http://upper-https:8002");
|
||||||
|
const options = firstWsClientOptions();
|
||||||
|
expect(options.agent).toEqual({ proxyUrl: "http://upper-https:8002" });
|
||||||
|
});
|
||||||
|
|
||||||
it("passes HTTP_PROXY to ws client when https vars are unset", () => {
|
it("passes HTTP_PROXY to ws client when https vars are unset", () => {
|
||||||
process.env.HTTP_PROXY = "http://upper-http:8999";
|
process.env.HTTP_PROXY = "http://upper-http:8999";
|
||||||
|
|
||||||
|
|||||||
@@ -240,6 +240,61 @@ function looksLikeNodeCommandPattern(value: string): boolean {
|
|||||||
return /\s/.test(value) || value.includes("group:");
|
return /\s/.test(value) || value.includes("group:");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function editDistance(a: string, b: string): number {
|
||||||
|
if (a === b) {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
if (!a) {
|
||||||
|
return b.length;
|
||||||
|
}
|
||||||
|
if (!b) {
|
||||||
|
return a.length;
|
||||||
|
}
|
||||||
|
|
||||||
|
const dp: number[] = Array.from({ length: b.length + 1 }, (_, j) => j);
|
||||||
|
|
||||||
|
for (let i = 1; i <= a.length; i++) {
|
||||||
|
let prev = dp[0];
|
||||||
|
dp[0] = i;
|
||||||
|
for (let j = 1; j <= b.length; j++) {
|
||||||
|
const temp = dp[j];
|
||||||
|
const cost = a[i - 1] === b[j - 1] ? 0 : 1;
|
||||||
|
dp[j] = Math.min(dp[j] + 1, dp[j - 1] + 1, prev + cost);
|
||||||
|
prev = temp;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return dp[b.length];
|
||||||
|
}
|
||||||
|
|
||||||
|
function suggestKnownNodeCommands(unknown: string, known: Set<string>): string[] {
|
||||||
|
const needle = unknown.trim();
|
||||||
|
if (!needle) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fast path: prefix-ish suggestions.
|
||||||
|
const prefix = needle.includes(".") ? needle.split(".").slice(0, 2).join(".") : needle;
|
||||||
|
const prefixHits = Array.from(known)
|
||||||
|
.filter((cmd) => cmd.startsWith(prefix))
|
||||||
|
.slice(0, 3);
|
||||||
|
if (prefixHits.length > 0) {
|
||||||
|
return prefixHits;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fuzzy: Levenshtein over a small-ish known set.
|
||||||
|
const ranked = Array.from(known)
|
||||||
|
.map((cmd) => ({ cmd, d: editDistance(needle, cmd) }))
|
||||||
|
.toSorted((a, b) => a.d - b.d || a.cmd.localeCompare(b.cmd));
|
||||||
|
|
||||||
|
const best = ranked[0]?.d ?? Infinity;
|
||||||
|
const threshold = Math.max(2, Math.min(4, best));
|
||||||
|
return ranked
|
||||||
|
.filter((r) => r.d <= threshold)
|
||||||
|
.slice(0, 3)
|
||||||
|
.map((r) => r.cmd);
|
||||||
|
}
|
||||||
|
|
||||||
function resolveToolPolicies(params: {
|
function resolveToolPolicies(params: {
|
||||||
cfg: OpenClawConfig;
|
cfg: OpenClawConfig;
|
||||||
agentTools?: AgentToolsConfig;
|
agentTools?: AgentToolsConfig;
|
||||||
@@ -944,9 +999,17 @@ export function collectNodeDenyCommandPatternFindings(cfg: OpenClawConfig): Secu
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (unknownExact.length > 0) {
|
if (unknownExact.length > 0) {
|
||||||
detailParts.push(
|
const unknownDetails = unknownExact
|
||||||
`Unknown command names (not in defaults/allowCommands): ${unknownExact.join(", ")}`,
|
.map((entry) => {
|
||||||
);
|
const suggestions = suggestKnownNodeCommands(entry, knownCommands);
|
||||||
|
if (suggestions.length === 0) {
|
||||||
|
return entry;
|
||||||
|
}
|
||||||
|
return `${entry} (did you mean: ${suggestions.join(", ")})`;
|
||||||
|
})
|
||||||
|
.join(", ");
|
||||||
|
|
||||||
|
detailParts.push(`Unknown command names (not in defaults/allowCommands): ${unknownDetails}`);
|
||||||
}
|
}
|
||||||
const examples = Array.from(knownCommands).slice(0, 8);
|
const examples = Array.from(knownCommands).slice(0, 8);
|
||||||
|
|
||||||
|
|||||||
@@ -1156,6 +1156,45 @@ description: test skill
|
|||||||
expect(finding?.severity).toBe("warn");
|
expect(finding?.severity).toBe("warn");
|
||||||
expect(finding?.detail).toContain("system.*");
|
expect(finding?.detail).toContain("system.*");
|
||||||
expect(finding?.detail).toContain("system.runx");
|
expect(finding?.detail).toContain("system.runx");
|
||||||
|
expect(finding?.detail).toContain("did you mean");
|
||||||
|
expect(finding?.detail).toContain("system.run");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("suggests prefix-matching commands for unknown denyCommands entries", async () => {
|
||||||
|
const cfg: OpenClawConfig = {
|
||||||
|
gateway: {
|
||||||
|
nodes: {
|
||||||
|
denyCommands: ["system.run.prep"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const res = await audit(cfg);
|
||||||
|
const finding = res.findings.find(
|
||||||
|
(f) => f.checkId === "gateway.nodes.deny_commands_ineffective",
|
||||||
|
);
|
||||||
|
expect(finding?.severity).toBe("warn");
|
||||||
|
expect(finding?.detail).toContain("system.run.prep");
|
||||||
|
expect(finding?.detail).toContain("did you mean");
|
||||||
|
expect(finding?.detail).toContain("system.run.prepare");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps unknown denyCommands entries without suggestions when no close command exists", async () => {
|
||||||
|
const cfg: OpenClawConfig = {
|
||||||
|
gateway: {
|
||||||
|
nodes: {
|
||||||
|
denyCommands: ["zzzzzzzzzzzzzz"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const res = await audit(cfg);
|
||||||
|
const finding = res.findings.find(
|
||||||
|
(f) => f.checkId === "gateway.nodes.deny_commands_ineffective",
|
||||||
|
);
|
||||||
|
expect(finding?.severity).toBe("warn");
|
||||||
|
expect(finding?.detail).toContain("zzzzzzzzzzzzzz");
|
||||||
|
expect(finding?.detail).not.toContain("did you mean");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("scores dangerous gateway.nodes.allowCommands by exposure", async () => {
|
it("scores dangerous gateway.nodes.allowCommands by exposure", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user