From 382873fa7b8ade068428116310a33de115e46c47 Mon Sep 17 00:00:00 2001
From: Val Alexander <bunsthedev@gmail.com>
Date: Thu, 5 Mar 2026 18:41:11 -0600
Subject: [PATCH] fix(ui): address bot review comments

- fix(security): add prototype pollution protection to setPathValue/removePathValue
  - Block __proto__, prototype, constructor keys in config map paths
  - Prevents user-controlled map keys from polluting Object.prototype
  - Addresses CWE-1321 (Medium severity) flagged by Aisle Security Bot

- fix(test): document translate.test.ts fallback behavior
  - Add explicit comment that fallback path may mask auto-detection regressions
  - Track and warn when fallback setLocale is used
  - Addresses Greptile concern about test coverage gaps

- docs(test): coverage gap explanations
  - loadToolsCatalog tests removed: function was already deleted in earlier PR
  - lastErrorCode test removed: field no longer populated in production
  - Both test deletions are intentional cleanup, not coverage regressions
---
 ui/src/i18n/test/translate.test.ts         | 11 +++++++++++
 ui/src/ui/controllers/config/form-utils.ts | 17 +++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/ui/src/i18n/test/translate.test.ts b/ui/src/i18n/test/translate.test.ts
index f1b3f7ea8b2..160c30b5ebd 100644
--- a/ui/src/i18n/test/translate.test.ts
+++ b/ui/src/i18n/test/translate.test.ts
@@ -49,13 +49,24 @@ describe("i18n", () => {
     // vi.resetModules() may not cause full module re-evaluation in browser
     // mode; if the singleton wasn't re-created, manually trigger the load path
     // so we still verify locale loading + translation correctness.
+    // NOTE: The fallback path below means auto-detection regressions may be
+    // masked. If vi.resetModules() properly re-initializes the module, the
+    // locale should be set within the polling window. The fallback ensures
+    // translation correctness is still tested even when module re-init fails.
+    let usedFallback = false;
     for (let i = 0; i < 20 && fresh.i18n.getLocale() !== "zh-CN"; i++) {
       await new Promise((r) => setTimeout(r, 50));
     }
     if (fresh.i18n.getLocale() !== "zh-CN") {
+      usedFallback = true;
       await fresh.i18n.setLocale("zh-CN");
     }
 
+    // Warn if we had to use the fallback (auto-detection may have failed)
+    if (usedFallback) {
+      console.warn("[translate.test] Used fallback setLocale — auto-detection may not have run");
+    }
+
     expect(fresh.i18n.getLocale()).toBe("zh-CN");
     expect(fresh.t("common.health")).toBe("健康状况");
   });
diff --git a/ui/src/ui/controllers/config/form-utils.ts b/ui/src/ui/controllers/config/form-utils.ts
index 296b666e800..78b6978ab82 100644
--- a/ui/src/ui/controllers/config/form-utils.ts
+++ b/ui/src/ui/controllers/config/form-utils.ts
@@ -9,6 +9,13 @@ export function serializeConfigForm(form: Record<string, unknown>): string {
   return `${JSON.stringify(form, null, 2).trimEnd()}\n`;
 }
 
+// Block prototype pollution via user-controlled map keys
+const FORBIDDEN_KEYS = new Set(["__proto__", "prototype", "constructor"]);
+
+function isForbiddenKey(key: string | number): boolean {
+  return typeof key === "string" && FORBIDDEN_KEYS.has(key);
+}
+
 export function setPathValue(
   obj: Record<string, unknown> | unknown[],
   path: Array<string | number>,
@@ -17,6 +24,11 @@ export function setPathValue(
   if (path.length === 0) {
     return;
   }
+  // Reject paths containing forbidden keys to prevent prototype pollution
+  if (path.some(isForbiddenKey)) {
+    console.warn("[setPathValue] Rejected forbidden key in path:", path);
+    return;
+  }
   let current: Record<string, unknown> | unknown[] = obj;
   for (let i = 0; i < path.length - 1; i += 1) {
     const key = path[i];
@@ -59,6 +71,11 @@ export function removePathValue(
   if (path.length === 0) {
     return;
   }
+  // Reject paths containing forbidden keys to prevent prototype pollution
+  if (path.some(isForbiddenKey)) {
+    console.warn("[removePathValue] Rejected forbidden key in path:", path);
+    return;
+  }
   let current: Record<string, unknown> | unknown[] = obj;
   for (let i = 0; i < path.length - 1; i += 1) {
     const key = path[i];