fix: harden config prototype-key guards (#22968) (thanks @Clawborn)

This commit is contained in:
Peter Steinberger
2026-02-22 00:24:54 +01:00
parent e23c08b5f4
commit 95dab6e019
5 changed files with 30 additions and 5 deletions

View File

@@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- Chat/UI: strip inline reply/audio directive tags (`[[reply_to_current]]`, `[[reply_to:<id>]]`, `[[audio_as_voice]]`) from displayed chat history, live chat event output, and session preview snippets so control tags no longer leak into user-visible surfaces.
- Security/Config: block prototype-key traversal during config merge patch and legacy migration merge helpers (`__proto__`, `constructor`, `prototype`) to prevent prototype pollution during config mutation flows. (#22968) Thanks @Clawborn.
- Security/Shell env: validate login-shell executable paths for shell-env fallback (`/etc/shells` + trusted prefixes) and block `SHELL` in dangerous env override policy paths so untrusted shell-path injection falls back safely to `/bin/sh`. Thanks @athuljayaram for reporting.
- Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating.
- Security/Exec: in non-default setups that manually add `sort` to `tools.exec.safeBins`, block `sort --compress-program` so allowlist-mode safe-bin checks cannot bypass approval. Thanks @tdjackey for reporting.

View File

@@ -0,0 +1,23 @@
import { afterEach, describe, expect, it } from "vitest";
import { mergeMissing } from "./legacy.shared.js";
describe("mergeMissing prototype pollution guard", () => {
afterEach(() => {
delete (Object.prototype as Record<string, unknown>).polluted;
});
it("ignores __proto__ keys without polluting Object.prototype", () => {
const target = { safe: { keep: true } } as Record<string, unknown>;
const source = JSON.parse('{"safe":{"next":1},"__proto__":{"polluted":true}}') as Record<
string,
unknown
>;
mergeMissing(target, source);
expect((target.safe as Record<string, unknown>).keep).toBe(true);
expect((target.safe as Record<string, unknown>).next).toBe(1);
expect(target.polluted).toBeUndefined();
expect((Object.prototype as Record<string, unknown>).polluted).toBeUndefined();
});
});

View File

@@ -12,6 +12,7 @@ export type LegacyConfigMigration = {
import { isSafeExecutableValue } from "../infra/exec-safety.js";
import { isRecord } from "../utils.js";
import { isBlockedObjectKey } from "./prototype-keys.js";
export { isRecord };
export const getRecord = (value: unknown): Record<string, unknown> | null =>
@@ -32,7 +33,7 @@ export const ensureRecord = (
export const mergeMissing = (target: Record<string, unknown>, source: Record<string, unknown>) => {
for (const [key, value] of Object.entries(source)) {
if (value === undefined) {
if (value === undefined || isBlockedObjectKey(key)) {
continue;
}
const existing = target[key];

View File

@@ -9,6 +9,7 @@ describe("applyMergePatch prototype pollution guard", () => {
expect(result.b).toBe(2);
expect(result.a).toBe(1);
expect(Object.prototype.hasOwnProperty.call(result, "__proto__")).toBe(false);
expect(result.polluted).toBeUndefined();
expect(({} as Record<string, unknown>).polluted).toBeUndefined();
});
@@ -35,6 +36,7 @@ describe("applyMergePatch prototype pollution guard", () => {
expect(result.nested.y).toBe(2);
expect(result.nested.x).toBe(1);
expect(Object.prototype.hasOwnProperty.call(result.nested, "__proto__")).toBe(false);
expect(result.nested.polluted).toBeUndefined();
expect(({} as Record<string, unknown>).polluted).toBeUndefined();
});
});

View File

@@ -1,10 +1,8 @@
import { isPlainObject } from "../utils.js";
import { isBlockedObjectKey } from "./prototype-keys.js";
type PlainObject = Record<string, unknown>;
/** Keys that must never be merged to prevent prototype-pollution attacks. */
const BLOCKED_KEYS = new Set(["__proto__", "constructor", "prototype"]);
type MergePatchOptions = {
mergeObjectArraysById?: boolean;
};
@@ -73,7 +71,7 @@ export function applyMergePatch(
const result: PlainObject = isPlainObject(base) ? { ...base } : {};
for (const [key, value] of Object.entries(patch)) {
if (BLOCKED_KEYS.has(key)) {
if (isBlockedObjectKey(key)) {
continue;
}
if (value === null) {