Greptile Summary

This test-only PR updates UI test fixtures and expectations across 16 files to align with the dashboard-v2 refactor — covering chat, config, cron, navigation, sessions, agents, and i18n. Most changes are straightforward fixture updates (new theme/themeMode split, added navWidth, removed deprecated fields) and test renames to match the new component structure.

Key observations:

• Net-new coverage: hasOperatorReadAccess, hasMissingSkillDependencies (app-settings), tail-URL blur class (markdown), runUpdate error-payload surfacing (config controller), and section-tab active-state (config browser) all gain new test cases.
• Coverage reductions: Two entire test files are deleted (controllers/agents.test.ts, views/agents-panels-tools-skills.browser.test.ts) with no replacement. controllers/cron.test.ts loses ~350 lines covering edit/clone/cancel, validation, pagination, filtering, and paged run history. app-gateway.node.test.ts loses the structured auth-error code path test. It would help to confirm whether the corresponding production functions were removed in an earlier stacked PR, or whether follow-up coverage is planned.
• translate.test.ts fallback: The new setTimeout-based polling + setLocale fallback can silently pass even when locale auto-detection on module load fails, slightly weakening the original assertion's intent.
• navigation.browser.test.ts: Consistently adding await nextFrame() after every updateComplete is a correct and welcome fix for async rendering races.
• open-external-url.test.ts: Switching from vi.stubGlobal to vi.spyOn is a cleaner, more isolated approach.

Confidence Score: 4/5

• Safe to merge — no production code is changed; only test fixtures and expectations are updated to match the already-landed dashboard-v2 refactor.
• Score reflects that this is a test-only PR with no runtime risk, but notable coverage is removed (agents controller, agents panel browser tests, most cron controller tests) without clear confirmation that the underlying production functions were also removed. The translate test's fallback path and the missing auth-error detail-code test are minor additional concerns.
• ui/src/ui/controllers/agents.test.ts and ui/src/ui/views/agents-panels-tools-skills.browser.test.ts (deleted with no replacement), ui/src/ui/controllers/cron.test.ts (significant coverage reduction), ui/src/ui/app-gateway.node.test.ts (auth-error path removed)

_{Last reviewed commit: 22dcd44}

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🟡 Prototype pollution via editable map keys in config form renderer

Description

The config form renderer allows editing arbitrary additionalProperties map keys. When a user renames a map entry key, the code assigns the value using next[nextKey] = ... on a plain object.

• nextKey is user-controlled (text input)
• If nextKey is "__proto__" (or similar), assignment can trigger the __proto__ setter on Object.prototype and mutate the target object's prototype (prototype pollution / prototype mutation)
• This path bypasses the new forbidden-key protections added to setPathValue/removePathValue, because renderMapField patches the whole object at path rather than calling onPatch([...path, key], ...)

Vulnerable code:

const nextKey = (e.target as HTMLInputElement).value.trim();
...
const next = { ...value };
...
next[nextKey] = next[key];
delete next[key];
onPatch(path, next);

Security impact depends on how configForm is later used, but prototype mutation can:

• cause logic errors (e.g., key in obj checks and other prototype-aware operations)
• create hard-to-debug UI state corruption/DoS
• become a stepping stone if any downstream logic performs unsafe merges or trusts object shape

Note: analyzeConfigSchema normalizes additionalProperties: true to {} (untyped map), making more schemas render as editable map fields and increasing exposure to this issue.

Recommendation

Block prototype-pollution keys for map entry names in the renderer (and ideally also when accepting whole-object patches), and/or store map objects as null-prototype dictionaries.

Option A: reject forbidden keys (recommended minimum):

const FORBIDDEN = new Set(["__proto__", "prototype", "constructor"]);
if (FORBIDDEN.has(nextKey)) {
  ​// show validation error to user instead of silently failing
  return;
}

Option B: safe assignment that does not trigger the __proto__ setter:

Object.defineProperty(next, nextKey, {
  value: next[key],
  enumerable: true,
  configurable: true,
  writable: true,
});
delete next[key];

Option C: use null-prototype containers for maps:

const next = Object.assign(Object.create(null), value);
​// then set keys normally

Also consider validating/sanitizing map keys server-side when applying config, since raw mode can introduce arbitrary keys.

Analyzed PR: #36857 at commit 382873f

_{Last updated on: 2026-03-06T01:25:12Z}

Closing this stacked PR. The base branches diverged ~3,135 commits from main/ui/touch-ups, making cherry-picks impractical.

What was kept (applied manually to ui/touch-ups):

1. Prototype pollution fix from form-utils.ts — FORBIDDEN_KEYS guard in setPathValue/removePathValue (CWE-1321, flagged by Aisle Security Bot). Added with regression tests.
2. open-external-url.test.ts cleanup — switched from vi.stubGlobal to vi.spyOn(window, "open") for cleaner test isolation.
3. runUpdate error surfacing test — new test covering the res.ok === false payload path in the config controller.
4. FOUC-prevention script — rewrote the index.html theme pre-render concept to match the current theme system (theme/themeMode → data-theme/data-theme-mode with legacy mapping and prefers-color-scheme support).

What was discarded and why:

• All production TS deletions — removed files (connect-error.ts, chat-model-ref.ts, local-storage.ts, chat/export.ts, session-cache.ts) and gutted modules (storage.ts, gateway.ts, types.ts, app-render.ts) are still actively imported/used on ui/touch-ups.
• All CSS rewrites — tightly coupled to the removed TS code and restructured views; cannot be applied independently.
• New CSS files (glass.css, agent-chat.css) — for a UI structure that doesn't exist on ui/touch-ups.
• Test coverage reductions — cron controller/view tests, agents.test.ts, agents-panels-tools-skills.browser.test.ts, and gateway auth-error tests were removed but the production functions still exist.
• translate.test.ts fallback — polling + setLocale fallback masks auto-detection regressions (flagged by both bot reviewers).
• Cron view onLoadRuns assertion change (1→2) — blesses a duplicate-fetch bug (flagged by bot reviewer).
• Favicon/icon updates — visually identical to current assets; just different compression.