fix(secrets): reject prototype-polluting mutation paths (#102840)

* fix(secrets): reject prototype-polluting mutation paths

Co-authored-by: yetval <yetvald@gmail.com>

* docs(secrets): document blocked mutation segments

* chore(secrets): keep release notes in PR metadata

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Yuval Dinodia
2026-07-09 19:17:31 -04:00
committed by GitHub
parent 50cb0d66b8
commit beda4218af
5 changed files with 87 additions and 14 deletions
+1 -1
View File
@@ -511,7 +511,7 @@ Supported evidence entries:
## uiHints reference
`uiHints` is a map from config field names to small rendering hints.
`uiHints` is a map from config field names to small rendering hints. Keys can use dots for nested config fields, but no path segment may be `__proto__`, `constructor`, or `prototype`; setup rejects those names.
```json
{
+34
View File
@@ -20,6 +20,9 @@ function createAgentListConfig(): OpenClawConfig {
});
}
const BLOCKED_PATH_SEGMENTS = ["__proto__", "constructor", "prototype"];
const POLLUTION_PROBE = "openclawPathPollutionProbe";
describe("secrets path utils", () => {
it("deletePathStrict compacts arrays via splice", () => {
const config = asConfig({});
@@ -56,6 +59,37 @@ describe("secrets path utils", () => {
);
});
it.each(BLOCKED_PATH_SEGMENTS)(
"setPathCreateStrict rejects %s before creating partial containers",
(blockedSegment) => {
const config = asConfig({});
expect(() =>
setPathCreateStrict(config, ["safe", blockedSegment, POLLUTION_PROBE], "yes"),
).toThrow(/prototype-polluting/);
expect(config).toEqual({});
expect(Object.hasOwn(Object.prototype, POLLUTION_PROBE)).toBe(false);
},
);
it.each([
["leading", (blockedSegment: string) => [blockedSegment, POLLUTION_PROBE]],
["middle", (blockedSegment: string) => ["safe", blockedSegment, POLLUTION_PROBE]],
["leaf", (blockedSegment: string) => ["safe", "value", blockedSegment]],
] as const)("all mutation helpers reject blocked segments at the %s", (_position, pathFor) => {
for (const blockedSegment of BLOCKED_PATH_SEGMENTS) {
const segments = pathFor(blockedSegment);
const config = asConfig({ safe: { value: "kept" } });
expect(() => setPathCreateStrict(config, segments, "changed")).toThrow(/prototype-polluting/);
expect(() => setPathExistingStrict(config, segments, "changed")).toThrow(
/prototype-polluting/,
);
expect(() => deletePathStrict(config, segments)).toThrow(/prototype-polluting/);
expect(config).toEqual({ safe: { value: "kept" } });
}
});
it("setPathExistingStrict throws when path does not already exist", () => {
const config = createAgentListConfig();
expect(() =>
+13 -6
View File
@@ -1,5 +1,6 @@
/** Strict dotted-path get/set/delete helpers for secrets migration targets. */
import { isDeepStrictEqual } from "node:util";
import { isBlockedObjectKey } from "../infra/prototype-keys.js";
import { parseConfigPathArrayIndex } from "../shared/path-array-index.js";
import { isRecord } from "./shared.js";
@@ -23,6 +24,16 @@ function expectedContainer(nextSegment: string): "array" | "object" {
return looksLikeArrayIndexSegment(nextSegment) ? "array" : "object";
}
function assertSafeMutationPath(segments: string[]): void {
if (segments.length === 0) {
throw new Error("Target path is empty.");
}
const blockedSegment = segments.find(isBlockedObjectKey);
if (blockedSegment) {
throw new Error(`Refusing to mutate prototype-polluting path segment "${blockedSegment}".`);
}
}
function parseArrayLeafTarget(
cursor: unknown,
leaf: string,
@@ -39,9 +50,7 @@ function traverseToLeafParent(params: {
segments: string[];
requireExistingSegment: boolean;
}): unknown {
if (params.segments.length === 0) {
throw new Error("Target path is empty.");
}
assertSafeMutationPath(params.segments);
let cursor: unknown = params.root;
for (let index = 0; index < params.segments.length - 1; index += 1) {
@@ -108,9 +117,7 @@ export function setPathCreateStrict(
segments: string[],
value: unknown,
): boolean {
if (segments.length === 0) {
throw new Error("Target path is empty.");
}
assertSafeMutationPath(segments);
let cursor: unknown = root;
let changed = false;
+2 -7
View File
@@ -3,6 +3,7 @@ import { isRecord as isObjectRecord } from "@openclaw/normalization-core/record-
import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization";
import type { SecretProviderConfig, SecretRef } from "../config/types.secrets.js";
import { SecretProviderSchema } from "../config/zod-schema.core.js";
import { isBlockedObjectKey } from "../infra/prototype-keys.js";
import { isValidSecretProviderAlias, isValidSecretRef } from "./ref-contract.js";
import { parseDotPath, toDotPath } from "./shared.js";
import { resolvePlanTargetAgainstRegistry, type ResolvedPlanTarget } from "./target-registry.js";
@@ -60,16 +61,10 @@ export type SecretsApplyPlan = {
};
};
const FORBIDDEN_PATH_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]);
function isSecretProviderConfigShape(value: unknown): value is SecretProviderConfig {
return SecretProviderSchema.safeParse(value).success;
}
function hasForbiddenPathSegment(segments: string[]): boolean {
return segments.some((segment) => FORBIDDEN_PATH_SEGMENTS.has(segment));
}
/** Resolves a user-supplied plan target through the registry after path safety checks. */
export function resolveValidatedPlanTarget(candidate: {
type?: SecretsPlanTargetType;
@@ -91,7 +86,7 @@ export function resolveValidatedPlanTarget(candidate: {
Array.isArray(candidate.pathSegments) && candidate.pathSegments.length > 0
? normalizeStringEntries(candidate.pathSegments)
: parseDotPath(path);
if (segments.length === 0 || hasForbiddenPathSegment(segments) || path !== toDotPath(segments)) {
if (segments.length === 0 || segments.some(isBlockedObjectKey) || path !== toDotPath(segments)) {
return null;
}
// Registry resolution is the ownership gate; caller-provided paths must map to a known
+37
View File
@@ -357,6 +357,43 @@ describe("setupPluginConfig", () => {
expect(result.plugins?.entries?.brave?.config?.["webSearch.mode"]).toBeUndefined();
});
it("rejects prototype-polluting dotted uiHint paths without mutating config", async () => {
const pollutionProbe = "openclawPluginPollutionProbe";
loadPluginManifestRegistry.mockReturnValue({
plugins: [
{
...makeManifestPlugin("unsafe-plugin", {
[`safe.__proto__.${pollutionProbe}`]: { label: "Unsafe field" },
}),
enabledByDefault: true,
},
],
});
const config: OpenClawConfig = {
plugins: { entries: { "unsafe-plugin": { enabled: true } } },
};
await expect(
setupPluginConfig({
config,
prompter: {
intro: vi.fn(async () => {}),
outro: vi.fn(async () => {}),
note: vi.fn(async () => {}),
select: vi.fn(async () => "") as unknown as WizardPrompter["select"],
multiselect: vi.fn(async () => [
"unsafe-plugin",
]) as unknown as WizardPrompter["multiselect"],
text: vi.fn(async () => "owned") as unknown as WizardPrompter["text"],
confirm: vi.fn(async () => true),
progress: vi.fn(() => ({ update: vi.fn(), stop: vi.fn() })),
},
}),
).rejects.toThrow(/prototype-polluting/);
expect(config.plugins?.entries?.["unsafe-plugin"]?.config).toBeUndefined();
expect(({} as Record<string, unknown>)[pollutionProbe]).toBeUndefined();
});
it("coerces integer schema fields from text input", async () => {
loadPluginManifestRegistry.mockReturnValue({
plugins: [