fix(security): harden account-key handling against prototype pollution

This commit is contained in:
Peter Steinberger
2026-02-24 01:09:23 +00:00
parent 12cc754332
commit f97c0922e1
24 changed files with 141 additions and 111 deletions

View File

@@ -2,6 +2,7 @@ import { getChannelDock } from "../../channels/dock.js";
import { normalizeChannelId } from "../../channels/plugins/index.js";
import type { OpenClawConfig } from "../../config/config.js";
import type { BlockStreamingCoalesceConfig } from "../../config/types.js";
import { resolveAccountEntry } from "../../routing/account-lookup.js";
import { normalizeAccountId } from "../../routing/session-key.js";
import {
INTERNAL_MESSAGE_CHANNEL,
@@ -45,7 +46,7 @@ function resolveProviderBlockStreamingCoalesce(params: {
}
const normalizedAccountId = normalizeAccountId(accountId);
const typed = providerCfg as ProviderBlockStreamingConfig;
const accountCfg = typed.accounts?.[normalizedAccountId];
const accountCfg = resolveAccountEntry(typed.accounts, normalizedAccountId);
return accountCfg?.blockStreamingCoalesce ?? typed.blockStreamingCoalesce;
}

View File

@@ -12,12 +12,17 @@ import {
import { resolveDiscordAccount } from "../../discord/accounts.js";
import { resolveDiscordUserAllowlist } from "../../discord/resolve-users.js";
import { resolveIMessageAccount } from "../../imessage/accounts.js";
import { isBlockedObjectKey } from "../../infra/prototype-keys.js";
import {
addChannelAllowFromStoreEntry,
readChannelAllowFromStore,
removeChannelAllowFromStoreEntry,
} from "../../pairing/pairing-store.js";
import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../../routing/session-key.js";
import {
DEFAULT_ACCOUNT_ID,
normalizeAccountId,
normalizeOptionalAccountId,
} from "../../routing/session-key.js";
import { resolveSignalAccount } from "../../signal/accounts.js";
import { resolveSlackAccount } from "../../slack/accounts.js";
import { resolveSlackUserAllowlist } from "../../slack/resolve-users.js";
@@ -199,13 +204,22 @@ function resolveAccountTarget(
const channels = (parsed.channels ??= {}) as Record<string, unknown>;
const channel = (channels[channelId] ??= {}) as Record<string, unknown>;
const normalizedAccountId = normalizeAccountId(accountId);
if (isBlockedObjectKey(normalizedAccountId)) {
return { target: channel, pathPrefix: `channels.${channelId}`, accountId: DEFAULT_ACCOUNT_ID };
}
const hasAccounts = Boolean(channel.accounts && typeof channel.accounts === "object");
const useAccount = normalizedAccountId !== DEFAULT_ACCOUNT_ID || hasAccounts;
if (!useAccount) {
return { target: channel, pathPrefix: `channels.${channelId}`, accountId: normalizedAccountId };
}
const accounts = (channel.accounts ??= {}) as Record<string, unknown>;
const account = (accounts[normalizedAccountId] ??= {}) as Record<string, unknown>;
const existingAccount = Object.hasOwn(accounts, normalizedAccountId)
? accounts[normalizedAccountId]
: undefined;
if (!existingAccount || typeof existingAccount !== "object") {
accounts[normalizedAccountId] = {};
}
const account = accounts[normalizedAccountId] as Record<string, unknown>;
return {
target: account,
pathPrefix: `channels.${channelId}.accounts.${normalizedAccountId}`,
@@ -361,6 +375,14 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo
reply: { text: "⚠️ Unknown channel. Add channel=<id> to the command." },
};
}
if (parsed.account?.trim() && !normalizeOptionalAccountId(parsed.account)) {
return {
shouldContinue: false,
reply: {
text: "⚠️ Invalid account id. Reserved keys (__proto__, constructor, prototype) are blocked.",
},
};
}
const accountId = normalizeAccountId(parsed.account ?? params.ctx.AccountId);
const scope = parsed.scope;

View File

@@ -645,6 +645,22 @@ describe("handleCommands /allowlist", () => {
expect(result.reply?.text).toContain("DM allowlist added");
});
it("rejects blocked account ids and keeps Object.prototype clean", async () => {
delete (Object.prototype as Record<string, unknown>).allowFrom;
const cfg = {
commands: { text: true, config: true },
channels: { telegram: { allowFrom: ["123"] } },
} as OpenClawConfig;
const params = buildPolicyParams("/allowlist add dm --account __proto__ 789", cfg);
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("Invalid account id");
expect((Object.prototype as Record<string, unknown>).allowFrom).toBeUndefined();
expect(writeConfigFileMock).not.toHaveBeenCalled();
});
it("removes DM allowlist entries from canonical allowFrom and deletes legacy dm.allowFrom", async () => {
const cases = [
{