From 2ce691d8cafc615bd60f65d5e5345138c7e52599 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 11 May 2026 16:52:32 -0400 Subject: [PATCH] fix(desktop): address CodeQL alerts on PR #20059 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - settings/helpers.ts: harden setNested against prototype pollution. POLLUTING_PATH_PARTS check is now applied at every assignment site (loop + leaf) and uses Object.defineProperty so CodeQL can see the guard inline rather than via a helper function call. - lib/markdown-preprocess.ts: rebuild the dangling-fence close regex from a fence-char + length instead of marker.replace(...). The marker is captured by `(`{3,}|~{3,})` so it can only be backticks or tildes, but CodeQL was tracing tainted input text into the RegExp source and flagging hostname dots from input as part of the pattern (false positive js/incomplete-hostname-regexp on the test fixture URLs). Reconstructing from a literal char breaks the dataflow. - scripts/notarize-artifact.cjs: drop args from the run() rejection message. Args carry --key-id / --issuer / key file path; the existing outer catch already squashes errors to a generic line, but CodeQL was flagging the args.join(' ') as clear-text logging of APPLE_API_KEY_ID. Composer DOM-text-as-HTML alerts (composer/index.tsx:379, :547) are already addressed in 4dd9732a9 — innerHTML assignment was replaced with renderComposerContents which builds DOM via replaceChildren / append text nodes (no HTML interpretation). --- apps/desktop/scripts/notarize-artifact.cjs | 5 ++- apps/desktop/src/app/settings/helpers.ts | 34 ++++++++++++++++++--- apps/desktop/src/lib/markdown-preprocess.ts | 7 ++++- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/apps/desktop/scripts/notarize-artifact.cjs b/apps/desktop/scripts/notarize-artifact.cjs index ab8fef7387e..89a4901c5cc 100644 --- a/apps/desktop/scripts/notarize-artifact.cjs +++ b/apps/desktop/scripts/notarize-artifact.cjs @@ -7,7 +7,10 @@ function run(command, args) { return new Promise((resolve, reject) => { execFile(command, args, (error, stdout, stderr) => { if (error) { - reject(new Error(`${command} ${args.join(' ')} failed: ${stderr?.trim() || stdout?.trim() || error.message}`)) + // Intentionally omit args from the rejection message: callers pass + // notarization credentials (key id, issuer, key file path) here, and + // surfacing them in error output would land in CI logs. + reject(new Error(`${command} failed: ${stderr?.trim() || stdout?.trim() || error.message}`)) return } resolve() diff --git a/apps/desktop/src/app/settings/helpers.ts b/apps/desktop/src/app/settings/helpers.ts index 7c17a3b2c31..b1723009ccc 100644 --- a/apps/desktop/src/app/settings/helpers.ts +++ b/apps/desktop/src/app/settings/helpers.ts @@ -25,16 +25,32 @@ export const providerPriority = (name: string) => PROVIDER_GROUPS.find(g => g.na const POLLUTING_PATH_PARTS = new Set(['__proto__', 'constructor', 'prototype']) +function isSafePart(part: string): boolean { + return part.length > 0 && !POLLUTING_PATH_PARTS.has(part) +} + function configPathParts(path: string): string[] { const parts = path.split('.') - if (parts.some(part => !part || POLLUTING_PATH_PARTS.has(part))) { + if (!parts.every(isSafePart)) { throw new Error(`Unsafe config path: ${path}`) } return parts } +function safeSet(target: Record, key: string, value: unknown): void { + if (!isSafePart(key)) { + throw new Error(`Unsafe config key: ${key}`) + } + Object.defineProperty(target, key, { + value, + writable: true, + enumerable: true, + configurable: true, + }) +} + export function getNested(obj: HermesConfigRecord, path: string): unknown { let cur: unknown = obj @@ -43,6 +59,10 @@ export function getNested(obj: HermesConfigRecord, path: string): unknown { return undefined } + if (!Object.prototype.hasOwnProperty.call(cur, part)) { + return undefined + } + cur = (cur as Record)[part] } @@ -57,14 +77,20 @@ export function setNested(obj: HermesConfigRecord, path: string, value: unknown) for (let i = 0; i < parts.length - 1; i += 1) { const part = parts[i] - if (cur[part] == null || typeof cur[part] !== 'object') { - cur[part] = {} + if (!isSafePart(part)) { + throw new Error(`Unsafe config path part: ${part}`) + } + + const existing = Object.prototype.hasOwnProperty.call(cur, part) ? cur[part] : undefined + + if (existing == null || typeof existing !== 'object') { + safeSet(cur, part, {}) } cur = cur[part] as Record } - cur[parts[parts.length - 1]] = value + safeSet(cur, parts[parts.length - 1], value) return clone } diff --git a/apps/desktop/src/lib/markdown-preprocess.ts b/apps/desktop/src/lib/markdown-preprocess.ts index 17347bbfc68..e08aef06291 100644 --- a/apps/desktop/src/lib/markdown-preprocess.ts +++ b/apps/desktop/src/lib/markdown-preprocess.ts @@ -32,7 +32,12 @@ function scrubBacktickNoise(text: string): string { const marker = match[2] || '```' const info = match[3] || '' const body = match[4] || '' - const closeRe = new RegExp(`\\n[ \\t]*${marker.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}[ \\t]*(?=\\n|$)`) + // `marker` is captured by `(`{3,}|~{3,})` above, so its only meta-character + // is the backtick or tilde. Reconstruct the close-fence pattern from a + // literal char + length to keep the regex source free of tainted input + // (and to keep CodeQL's hostname-regexp dataflow happy). + const fenceChar = marker[0] === '~' ? '\\~' : '`' + const closeRe = new RegExp(`\\n[ \\t]*${fenceChar}{${marker.length}}[ \\t]*(?=\\n|$)`) if (!closeRe.test(body) && sanitizeLanguageTag(info) && !isLikelyProseFence(info, body)) { protectedRanges.push({ end: text.length, start })