fix(cli): block config prototype pollution (#54)

packages/cli/src/config.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from "vitest";
+import { getConfigValue, mergeConfig, setConfigValue, type JsonObject } from "./config.js";
+
+describe("config helpers", () => {
+  it("sets and reads nested own properties", () => {
+    const config: JsonObject = {};
+
+    setConfigValue("waiting.arcade.defaultGame", "snake", config);
+
+    expect(getConfigValue("waiting.arcade.defaultGame", config)).toBe("snake");
+  });
+
+  it.each(["__proto__", "prototype", "constructor"])(
+    "rejects unsafe path key: %s",
+    (key) => {
+      const marker = "logicsrcPrototypePollution";
+      const config: JsonObject = {};
+
+      expect(() => setConfigValue(`${key}.${marker}`, "true", config)).toThrow(
+        `Unsafe config key: ${key}`
+      );
+      expect(Object.hasOwn(Object.prototype, marker)).toBe(false);
+    }
+  );
+
+  it("rejects unsafe keys while merging parsed config", () => {
+    const override = JSON.parse(
+      '{"__proto__":{"logicsrcPrototypePollution":true}}'
+    ) as JsonObject;
+
+    expect(() => mergeConfig({}, override)).toThrow("Unsafe config key: __proto__");
+    expect(Object.hasOwn(Object.prototype, "logicsrcPrototypePollution")).toBe(false);
+  });
+
+  it("does not read inherited config values", () => {
+    const config = Object.create({ inherited: "secret" }) as JsonObject;
+
+    expect(getConfigValue("inherited", config)).toBeUndefined();
+  });
+});

packages/cli/src/config.ts

@@ -4,6 +4,8 @@ import { homedir } from "node:os";
 
 export type JsonObject = Record<string, unknown>;
 
+const UNSAFE_CONFIG_KEYS = new Set(["__proto__", "prototype", "constructor"]);
+
 export const defaultConfig: JsonObject = {
   waiting: {
     arcade: {
@@ -40,7 +42,12 @@ export function writeConfig(config: JsonObject) {
 }
 
 export function getConfigValue(path: string, config = readConfig()) {
-  return path.split(".").reduce<unknown>((current, key) => (isObject(current) ? current[key] : undefined), config);
+  return path.split(".").reduce<unknown>((current, key) => {
+    if (UNSAFE_CONFIG_KEYS.has(key) || !isObject(current) || !Object.hasOwn(current, key)) {
+      return undefined;
+    }
+    return current[key];
+  }, config);
 }
 
 export function setConfigValue(path: string, rawValue: string, config = readConfig()) {
@@ -48,9 +55,10 @@ export function setConfigValue(path: string, rawValue: string, config = readConf
   if (parts.length === 0) {
     throw new Error("Config path cannot be empty.");
   }
+  parts.forEach(assertSafeConfigKey);
   let current: JsonObject = config;
   for (const part of parts.slice(0, -1)) {
-    if (!isObject(current[part])) {
+    if (!Object.hasOwn(current, part) || !isObject(current[part])) {
       current[part] = {};
     }
     current = current[part] as JsonObject;
@@ -71,9 +79,10 @@ export function parseConfigValue(value: string): unknown {
   }
 }
 
-function mergeConfig(base: JsonObject, override: JsonObject): JsonObject {
+export function mergeConfig(base: JsonObject, override: JsonObject): JsonObject {
   for (const [key, value] of Object.entries(override)) {
-    if (isObject(value) && isObject(base[key])) {
+    assertSafeConfigKey(key);
+    if (isObject(value) && Object.hasOwn(base, key) && isObject(base[key])) {
       base[key] = mergeConfig(base[key] as JsonObject, value);
     } else {
       base[key] = value;
@@ -85,3 +94,9 @@ function mergeConfig(base: JsonObject, override: JsonObject): JsonObject {
 function isObject(value: unknown): value is JsonObject {
   return typeof value === "object" && value !== null && !Array.isArray(value);
 }
+
+function assertSafeConfigKey(key: string) {
+  if (UNSAFE_CONFIG_KEYS.has(key)) {
+    throw new Error(`Unsafe config key: ${key}`);
+  }
+}