From 567e054e51cdeabc1bb137f20f10d2469d9bfc84 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 12:13:48 -0700 Subject: [PATCH 01/10] fix: address open CodeQL alerts in TypeScript code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close 21 open CodeQL alerts on main: Security - LargeFileWorker: remove dead `download` (untrusted-URL fetch) and `upload` actions; only `downloadAsFile` (SDK path-based) is used by callers. Closes #4 (client-side-request-forgery) and #17 (missing-origin-check). - orval/generate.ts: use `fs.mkdtempSync` for the OpenAPI spec temp file instead of a predictable `os.tmpdir()` path. Closes #5 (insecure-temporary-file). Code-quality - Drop redundant `this.page = page` / `this.request = request` in 11 e2e-tests classes ā€” TS parameter properties (`public readonly page: Page`, `private request: APIRequestContext`) already assign the field. Closes #22-#32 (useless-assignment-to-property). - Drop redundant null/undefined checks after narrowing in ReportTraceModal/utils, BenchmarkDetailsPanel, api/intake/utils, ActionMenu, useSubmitICLsFile. Closes #33-#37. - SafeSynthesizerJobReportRoute/util: drop unreachable `else if (score >= 8)` branches and the dead `UNAVAILABLE` fallback; add explicit `Number.isNaN` guard at the top of each grading helper. Closes #20, #21. - WorkspaceDashboardRoute: drop inner `MODEL_COMPARE_ENABLED ? a : b` ternary that always picked `a` (lives inside an outer `MODEL_COMPARE_ENABLED &&` guard); drop now-unused `getWorkspaceBaseModelsRoute` import. Closes #19. Signed-off-by: mschwab --- web/packages/sdk/orval/generate.ts | 6 +- .../studio/e2e-tests/api/customizations.ts | 4 +- web/packages/studio/e2e-tests/api/datasets.ts | 4 +- .../studio/e2e-tests/api/evaluations.ts | 4 +- web/packages/studio/e2e-tests/api/models.ts | 4 +- web/packages/studio/e2e-tests/api/projects.ts | 4 +- .../e2e-tests/pages/project-customizations.ts | 4 +- .../e2e-tests/pages/project-datasets.ts | 4 +- .../e2e-tests/pages/project-evaluations.ts | 4 +- .../studio/e2e-tests/pages/project-models.ts | 4 +- .../pages/project-safe-synthesizer.ts | 4 +- .../studio/e2e-tests/pages/projects.ts | 4 +- web/packages/studio/src/api/intake/utils.ts | 2 +- .../hooks/useSubmitICLsFile.ts | 9 +- .../src/components/ReportTraceModal/utils.ts | 2 +- .../evaluation/Configurations/ActionMenu.tsx | 2 +- .../BenchmarkDetailsPanel/index.tsx | 2 +- .../SafeSynthesizerJobReportRoute/util.ts | 36 +++----- .../routes/WorkspaceDashboardRoute/index.tsx | 12 +-- .../studio/src/workers/LargeFileWorker.ts | 90 +++++-------------- 20 files changed, 57 insertions(+), 148 deletions(-) diff --git a/web/packages/sdk/orval/generate.ts b/web/packages/sdk/orval/generate.ts index 6645502343..9030da4f55 100644 --- a/web/packages/sdk/orval/generate.ts +++ b/web/packages/sdk/orval/generate.ts @@ -109,7 +109,9 @@ const postProcessZodFiles = (zodPath: string) => { const main = async () => { console.log(`Generating types for: ${service}.`); const spec = await getFile(); - const tempFile = path.join(os.tmpdir(), `openapi-spec-${config.path}.yaml`); + // Per-run temp dir with random suffix avoids predictable paths in a shared tmp. + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openapi-spec-')); + const tempFile = path.join(tempDir, `${config.path}.yaml`); const clientVar = client ? `ORVAL_CLIENT=${client}` : ''; const target = client === 'zod' @@ -135,7 +137,7 @@ const main = async () => { postProcessZodFiles(`./generated/${config.path}/zod/default.ts`); } } finally { - fs.unlinkSync(tempFile); + fs.rmSync(tempDir, { recursive: true, force: true }); } }; diff --git a/web/packages/studio/e2e-tests/api/customizations.ts b/web/packages/studio/e2e-tests/api/customizations.ts index d40807d82a..6f77a1ba39 100644 --- a/web/packages/studio/e2e-tests/api/customizations.ts +++ b/web/packages/studio/e2e-tests/api/customizations.ts @@ -10,9 +10,7 @@ import { import { APIRequestContext } from '@playwright/test'; export class CustomizationsAPI { - constructor(private request: APIRequestContext) { - this.request = request; - } + constructor(private request: APIRequestContext) {} async createCustomizationJob(data: CustomizationJobInput) { const response = await this.request.post(`${NMP_BASE_URL}/v1/customization/jobs`, { diff --git a/web/packages/studio/e2e-tests/api/datasets.ts b/web/packages/studio/e2e-tests/api/datasets.ts index cf6169ce80..e5db8195cb 100644 --- a/web/packages/studio/e2e-tests/api/datasets.ts +++ b/web/packages/studio/e2e-tests/api/datasets.ts @@ -17,9 +17,7 @@ interface Dataset { } export class DatasetsAPI { - constructor(private request: APIRequestContext) { - this.request = request; - } + constructor(private request: APIRequestContext) {} private getFileNameFromPath(filePath: string) { const fileNameParts = filePath.split('/'); diff --git a/web/packages/studio/e2e-tests/api/evaluations.ts b/web/packages/studio/e2e-tests/api/evaluations.ts index 4b80a2bf16..3737b5a22b 100644 --- a/web/packages/studio/e2e-tests/api/evaluations.ts +++ b/web/packages/studio/e2e-tests/api/evaluations.ts @@ -10,9 +10,7 @@ type EvaluationConfig = Record; type EvaluationConfigInput = Record; export class EvaluationsAPI { - constructor(private request: APIRequestContext) { - this.request = request; - } + constructor(private request: APIRequestContext) {} async createEvaluationConfig(data: EvaluationConfigInput) { const response = await this.request.post(`${NMP_BASE_URL}/v1/evaluation/configs`, { diff --git a/web/packages/studio/e2e-tests/api/models.ts b/web/packages/studio/e2e-tests/api/models.ts index 635dac2ff5..df87d6e4a2 100644 --- a/web/packages/studio/e2e-tests/api/models.ts +++ b/web/packages/studio/e2e-tests/api/models.ts @@ -6,9 +6,7 @@ import { CreateModelEntityRequest, ModelEntity } from '@nemo/sdk/generated/platf import { APIRequestContext } from '@playwright/test'; export class ModelsAPI { - constructor(private request: APIRequestContext) { - this.request = request; - } + constructor(private request: APIRequestContext) {} async createModel(workspace: string, data: CreateModelEntityRequest) { const response = await this.request.post(`${NMP_BASE_URL}/v2/workspaces/${workspace}/models`, { diff --git a/web/packages/studio/e2e-tests/api/projects.ts b/web/packages/studio/e2e-tests/api/projects.ts index 9c883e18e3..17579b29b7 100644 --- a/web/packages/studio/e2e-tests/api/projects.ts +++ b/web/packages/studio/e2e-tests/api/projects.ts @@ -6,9 +6,7 @@ import { ProjectInput, Project, ProjectsPage } from '@nemo/sdk/generated/platfor import { APIRequestContext } from '@playwright/test'; export class ProjectsAPI { - constructor(private request: APIRequestContext) { - this.request = request; - } + constructor(private request: APIRequestContext) {} async createProject(workspace: string, data: ProjectInput) { const response = await this.request.post( diff --git a/web/packages/studio/e2e-tests/pages/project-customizations.ts b/web/packages/studio/e2e-tests/pages/project-customizations.ts index 4019077281..e0f36d41e0 100644 --- a/web/packages/studio/e2e-tests/pages/project-customizations.ts +++ b/web/packages/studio/e2e-tests/pages/project-customizations.ts @@ -5,9 +5,7 @@ import { CustomizationJob as CustomizationJobOutput } from '@nemo/sdk/vendored/c import { type Page } from '@playwright/test'; export class ProjectCustomizationsPage { - constructor(public readonly page: Page) { - this.page = page; - } + constructor(public readonly page: Page) {} async goto( projectNamespace: string, diff --git a/web/packages/studio/e2e-tests/pages/project-datasets.ts b/web/packages/studio/e2e-tests/pages/project-datasets.ts index a75a1c5743..5a7d779b77 100644 --- a/web/packages/studio/e2e-tests/pages/project-datasets.ts +++ b/web/packages/studio/e2e-tests/pages/project-datasets.ts @@ -10,9 +10,7 @@ import path from 'path'; /** Dataset shape for e2e page (files_url, name, etc.). */ type Dataset = { files_url?: string; name?: string; [key: string]: unknown }; export class ProjectDatasetsPage { - constructor(public readonly page: Page) { - this.page = page; - } + constructor(public readonly page: Page) {} private async openQuickActionsMenu(name: string, actionName: string) { const fileRow = await getRowByName(this.page, name); diff --git a/web/packages/studio/e2e-tests/pages/project-evaluations.ts b/web/packages/studio/e2e-tests/pages/project-evaluations.ts index d98aee4923..8d27b8adc0 100644 --- a/web/packages/studio/e2e-tests/pages/project-evaluations.ts +++ b/web/packages/studio/e2e-tests/pages/project-evaluations.ts @@ -5,9 +5,7 @@ import { waitForLongOperation } from '@e2e-tests/utils/pageUtils'; import { expect, type Page } from '@playwright/test'; export class ProjectEvaluationsPage { - constructor(public readonly page: Page) { - this.page = page; - } + constructor(public readonly page: Page) {} async gotoEvaluations(projectNamespace: string, projectName: string) { await this.page.goto(`projects/${projectNamespace}/${projectName}/evaluation/jobs`); diff --git a/web/packages/studio/e2e-tests/pages/project-models.ts b/web/packages/studio/e2e-tests/pages/project-models.ts index a384982c53..f8b3cf2d70 100644 --- a/web/packages/studio/e2e-tests/pages/project-models.ts +++ b/web/packages/studio/e2e-tests/pages/project-models.ts @@ -7,9 +7,7 @@ import { getRowByName } from '@e2e-tests/utils/tables'; import { expect, type Page } from '@playwright/test'; export class ProjectModelsPage { - constructor(public readonly page: Page) { - this.page = page; - } + constructor(public readonly page: Page) {} private async openQuickActionsMenu(modelName: string, actionName: string) { const modelRow = await getRowByName(this.page, modelName); diff --git a/web/packages/studio/e2e-tests/pages/project-safe-synthesizer.ts b/web/packages/studio/e2e-tests/pages/project-safe-synthesizer.ts index ed11b5a1dd..9b0a0b6f93 100644 --- a/web/packages/studio/e2e-tests/pages/project-safe-synthesizer.ts +++ b/web/packages/studio/e2e-tests/pages/project-safe-synthesizer.ts @@ -4,9 +4,7 @@ import { type Page } from '@playwright/test'; export class ProjectSafeSynthesizerPage { - constructor(public readonly page: Page) { - this.page = page; - } + constructor(public readonly page: Page) {} async goto(projectNamespace: string, projectName: string) { await this.page.goto(`projects/${projectNamespace}/${projectName}/safe-synthesizer`); diff --git a/web/packages/studio/e2e-tests/pages/projects.ts b/web/packages/studio/e2e-tests/pages/projects.ts index d728c07eb9..e9ad57f31a 100644 --- a/web/packages/studio/e2e-tests/pages/projects.ts +++ b/web/packages/studio/e2e-tests/pages/projects.ts @@ -8,9 +8,7 @@ import { expect, type Page } from '@playwright/test'; const PROJECTS_PAGE_URL = `projects?sort_by=created_at&order=desc`; export class ProjectsPage { - constructor(public readonly page: Page) { - this.page = page; - } + constructor(public readonly page: Page) {} private async openQuickActionsMenu(projectName: string, actionName: string) { const projectRow = await getRowByName(this.page, projectName); diff --git a/web/packages/studio/src/api/intake/utils.ts b/web/packages/studio/src/api/intake/utils.ts index 31eecfdd29..0793408a42 100644 --- a/web/packages/studio/src/api/intake/utils.ts +++ b/web/packages/studio/src/api/intake/utils.ts @@ -32,7 +32,7 @@ const processFilterObject = (obj: EntryFilter, params: URLSearchParams, prefix = }); } // Handle nested objects recursively - else if (typeof value === 'object' && value !== null) { + else if (typeof value === 'object') { processFilterObject(value, params, paramKey); } // Handle primitive values (string, number, boolean) diff --git a/web/packages/studio/src/components/PromptTuningForm/InContextLearningSection/hooks/useSubmitICLsFile.ts b/web/packages/studio/src/components/PromptTuningForm/InContextLearningSection/hooks/useSubmitICLsFile.ts index e670c35590..0d1d927272 100644 --- a/web/packages/studio/src/components/PromptTuningForm/InContextLearningSection/hooks/useSubmitICLsFile.ts +++ b/web/packages/studio/src/components/PromptTuningForm/InContextLearningSection/hooks/useSubmitICLsFile.ts @@ -36,12 +36,9 @@ export const useSubmitICLsFile = ( fileName, }; const hasFileAlready = currentICLs.some((icl) => icl.fileName === fileName); - let combinedICLs = currentICLs; - if (hasFileAlready) { - combinedICLs = currentICLs.map((icl) => (icl.fileName === fileName ? newICL : icl)); - } else { - combinedICLs = [...currentICLs, newICL]; - } + const combinedICLs = hasFileAlready + ? currentICLs.map((icl) => (icl.fileName === fileName ? newICL : icl)) + : [...currentICLs, newICL]; const iclFewShotExamples = combinedICLs.map((icl) => icl.content).join('\n'); const { prompt: compiledSystemPrompt, promptTemplate: newSystemPromptTemplate } = compileSystemPrompt({ diff --git a/web/packages/studio/src/components/ReportTraceModal/utils.ts b/web/packages/studio/src/components/ReportTraceModal/utils.ts index dc2c35c2b8..173c76a24f 100644 --- a/web/packages/studio/src/components/ReportTraceModal/utils.ts +++ b/web/packages/studio/src/components/ReportTraceModal/utils.ts @@ -62,7 +62,7 @@ Thanks!`; }; export const formatTags = (tags: TraceData['spans'][number]['tags']) => { - if (tags == null || tags === undefined) { + if (tags == null) { return 'empty'; } diff --git a/web/packages/studio/src/components/evaluation/Configurations/ActionMenu.tsx b/web/packages/studio/src/components/evaluation/Configurations/ActionMenu.tsx index af26541fe4..62c0ad9b23 100644 --- a/web/packages/studio/src/components/evaluation/Configurations/ActionMenu.tsx +++ b/web/packages/studio/src/components/evaluation/Configurations/ActionMenu.tsx @@ -42,7 +42,7 @@ export const ActionMenu: FC = ({ actions, slotTrigger }) => { {actions.map((action, key) => ( - {action.slotIcon && action.slotIcon} + {action.slotIcon} {action.slotLabel} diff --git a/web/packages/studio/src/components/sidePanels/BenchmarkDetailsPanel/index.tsx b/web/packages/studio/src/components/sidePanels/BenchmarkDetailsPanel/index.tsx index 315cca667f..f0512f5f20 100644 --- a/web/packages/studio/src/components/sidePanels/BenchmarkDetailsPanel/index.tsx +++ b/web/packages/studio/src/components/sidePanels/BenchmarkDetailsPanel/index.tsx @@ -33,7 +33,7 @@ export const BenchmarkDetailsPanel: FC = ({ const { metrics } = benchmark; if (typeof metrics[0] === 'string') return metrics.join(', '); return metrics - .map((m) => (m && typeof m === 'object' && m !== null && 'name' in m ? String(m.name) : '')) + .map((m) => (m && typeof m === 'object' && 'name' in m ? String(m.name) : '')) .filter(Boolean) .join(', '); })(); diff --git a/web/packages/studio/src/routes/SafeSynthesizerJobReportRoute/util.ts b/web/packages/studio/src/routes/SafeSynthesizerJobReportRoute/util.ts index f4cc7e8fe6..a88de1037f 100644 --- a/web/packages/studio/src/routes/SafeSynthesizerJobReportRoute/util.ts +++ b/web/packages/studio/src/routes/SafeSynthesizerJobReportRoute/util.ts @@ -12,33 +12,21 @@ export const GRADE_VALUES = { }; export function getDataPrivacyGradeLabel(score: number): string { - if (score < 2) { - return GRADE_VALUES.POOR; - } else if (score < 4) { - return GRADE_VALUES.MODERATE; - } else if (score < 6) { - return GRADE_VALUES.GOOD; - } else if (score < 8) { - return GRADE_VALUES.VERY_GOOD; - } else if (score >= 8) { - return GRADE_VALUES.EXCELLENT; - } - return GRADE_VALUES.UNAVAILABLE; + if (Number.isNaN(score)) return GRADE_VALUES.UNAVAILABLE; + if (score < 2) return GRADE_VALUES.POOR; + if (score < 4) return GRADE_VALUES.MODERATE; + if (score < 6) return GRADE_VALUES.GOOD; + if (score < 8) return GRADE_VALUES.VERY_GOOD; + return GRADE_VALUES.EXCELLENT; } export function getSyntheticQualityGradeLabel(score: number): string { - if (score < 2) { - return GRADE_VALUES.VERY_POOR; - } else if (score < 4) { - return GRADE_VALUES.POOR; - } else if (score < 6) { - return GRADE_VALUES.MODERATE; - } else if (score < 8) { - return GRADE_VALUES.GOOD; - } else if (score >= 8) { - return GRADE_VALUES.EXCELLENT; - } - return GRADE_VALUES.UNAVAILABLE; + if (Number.isNaN(score)) return GRADE_VALUES.UNAVAILABLE; + if (score < 2) return GRADE_VALUES.VERY_POOR; + if (score < 4) return GRADE_VALUES.POOR; + if (score < 6) return GRADE_VALUES.MODERATE; + if (score < 8) return GRADE_VALUES.GOOD; + return GRADE_VALUES.EXCELLENT; } export const GRADE_ORDER = [ diff --git a/web/packages/studio/src/routes/WorkspaceDashboardRoute/index.tsx b/web/packages/studio/src/routes/WorkspaceDashboardRoute/index.tsx index 80147c672f..57dd3585f8 100644 --- a/web/packages/studio/src/routes/WorkspaceDashboardRoute/index.tsx +++ b/web/packages/studio/src/routes/WorkspaceDashboardRoute/index.tsx @@ -22,11 +22,7 @@ import { import { ROUTES } from '@studio/constants/routes'; import { useWorkspaceFromPath } from '@studio/hooks/useWorkspaceFromPath'; import { useBreadcrumbs } from '@studio/providers/breadcrumbs/useBreadcrumbs'; -import { - getEvaluationMetricsRunRoute, - getModelCompareRoute, - getWorkspaceBaseModelsRoute, -} from '@studio/routes/utils'; +import { getEvaluationMetricsRunRoute, getModelCompareRoute } from '@studio/routes/utils'; import { DashboardCard } from '@studio/routes/WorkspaceDashboardRoute/DashboardCard'; import { ResourcesSection } from '@studio/routes/WorkspaceDashboardRoute/ResourcesSection'; import { Sliders, Boxes } from 'lucide-react'; @@ -67,11 +63,7 @@ export const WorkspaceDashboardRoute: FC = () => { title="Chat with a Model" description="Chat with base models and explore capabilities." actionLabel="Chat" - actionHref={ - MODEL_COMPARE_ENABLED - ? getModelCompareRoute(workspace) - : getWorkspaceBaseModelsRoute(workspace) - } + actionHref={getModelCompareRoute(workspace)} /> )} {/* Fine-tune a Model */} diff --git a/web/packages/studio/src/workers/LargeFileWorker.ts b/web/packages/studio/src/workers/LargeFileWorker.ts index e4a4458665..70f3a678ef 100644 --- a/web/packages/studio/src/workers/LargeFileWorker.ts +++ b/web/packages/studio/src/workers/LargeFileWorker.ts @@ -2,93 +2,45 @@ // SPDX-License-Identifier: Apache-2.0 import { DEFAULT_WORKSPACE } from '@nemo/common/src/models/constants'; -import { filesDownloadFile, filesUploadFile } from '@nemo/sdk/generated/platform/api'; +import { filesDownloadFile } from '@nemo/sdk/generated/platform/api'; import axios from 'axios'; export interface LargeFileWorkerMessage { dataset: string; workspace?: string; - file?: File; - action: 'download' | 'downloadAsFile' | 'upload'; - path?: string; - url?: string; + action: 'downloadAsFile'; + path: string; /** Access token passed from the main thread (localStorage is unavailable in workers). */ accessToken?: string; } /** - * This worker is used to download/upload large files from the server. - * It sends progress updates to the main thread. + * Downloads a file from the NeMo Files service as an ArrayBuffer. + * Goes through the SDK so the request URL is bound to the configured API base, + * not whatever URL the caller passes in. */ self.onmessage = async function (e: MessageEvent) { - const { dataset, workspace, file, action, url, path, accessToken } = e.data; + const { dataset, workspace, action, path, accessToken } = e.data; if (accessToken) { axios.defaults.headers.common['Authorization'] = `Bearer ${accessToken}`; } - switch (action) { - case 'download': { - if (!url) { - self.postMessage({ done: true, error: 'URL is required' }); - break; - } - try { - const response = await fetch(url); - const reader = response.body?.getReader(); - const contentLength = +response.headers.get('Content-Length')!; - let receivedLength = 0; - const chunks = []; - - while (true) { - const { done, value } = await reader!.read(); - if (done) break; - - chunks.push(value); - receivedLength += value.length; + if (action !== 'downloadAsFile') { + self.postMessage({ done: true, error: `Invalid action: ${action}` }); + return; + } - const progress = Math.floor((receivedLength / contentLength) * 100); - self.postMessage({ progress }); - } + if (!path) { + self.postMessage({ done: true, error: 'Path is required' }); + return; + } - const text = chunks.map((chunk: Uint8Array) => new TextDecoder().decode(chunk)).join(''); - self.postMessage({ done: true, text }); - } catch (error) { - self.postMessage({ done: true, error: String(error) }); - } - break; - } - case 'downloadAsFile': { - if (!path) { - self.postMessage({ done: true, error: 'Path is required' }); - break; - } - try { - const response = await filesDownloadFile(workspace || DEFAULT_WORKSPACE, dataset, path); - const arrayBuffer = await response.arrayBuffer(); - self.postMessage({ done: true, arrayBuffer }, { transfer: [arrayBuffer] }); - } catch (error) { - self.postMessage({ done: true, error: String(error) }); - } - break; - } - case 'upload': { - if (!file) { - self.postMessage({ done: true, error: 'File is required' }); - break; - } - try { - const blob = new Blob([await file.arrayBuffer()], { - type: file.type || 'application/octet-stream', - }); - await filesUploadFile(workspace || DEFAULT_WORKSPACE, dataset, file.name, blob); - self.postMessage({ done: true }); - } catch (error) { - self.postMessage({ done: true, error: String(error) }); - } - break; - } - default: - self.postMessage({ done: true, error: `Invalid action: ${action}` }); + try { + const response = await filesDownloadFile(workspace || DEFAULT_WORKSPACE, dataset, path); + const arrayBuffer = await response.arrayBuffer(); + self.postMessage({ done: true, arrayBuffer }, { transfer: [arrayBuffer] }); + } catch (error) { + self.postMessage({ done: true, error: String(error) }); } }; From e46160dbb89fc49661450e2e9cc1186bf16ca721 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 12:20:01 -0700 Subject: [PATCH 02/10] fix: refactor remaining CodeQL-flagged build scripts to argv form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop shell interpolation in dev/build scripts so user-supplied branch names, commit hashes, paths, and env values cannot be parsed as shell syntax. Also plug a TOCTOU and add origin allowlists for the two http-to-file fetches. - scripts/cherry-pick.ts: route every git call through execFileSync('git', [...]). Closes #6-#10 (indirect-cmd-line-injection). - scripts/git-utils.ts: openBrowser uses execFile + argv array; status/branch helpers use execFileSync with argv. Removes the brittle " ā†’ \" escape and the shell-interpolated browser command. Closes #1 (incomplete-sanitization) and #11 (indirect-cmd-line-injection). - sdk/orval/format-generated.ts: prettier runs via execFileSync. Closes #2 (shell-cmd-injection-from-env) and #13 (indirect-cmd-line-injection). - sdk/orval/generate.ts: orval runs via execFileSync, with its parameters passed in env instead of interpolated into a shell string; remote spec fetches are restricted to an allowlist of github/gitlab hosts; the existsSync+readFileSync TOCTOU in postProcessZodFiles is collapsed into a single try/catch on ENOENT. Closes #3 (file-system-race), #12 (indirect-cmd-line-injection), and #14 (http-to-file-access). - studio/scripts/fetch-styles.ts: validate that the fetch URL hostname matches the configured Kaizen CDN before fetching. Closes #15 (http-to-file-access). Signed-off-by: mschwab --- web/packages/scripts/src/cherry-pick.ts | 24 ++++++----- web/packages/scripts/src/git-utils.ts | 22 +++++----- web/packages/sdk/orval/format-generated.ts | 4 +- web/packages/sdk/orval/generate.ts | 46 ++++++++++++++------- web/packages/studio/scripts/fetch-styles.ts | 6 ++- 5 files changed, 63 insertions(+), 39 deletions(-) diff --git a/web/packages/scripts/src/cherry-pick.ts b/web/packages/scripts/src/cherry-pick.ts index 1a110b8824..2d424a3cfc 100644 --- a/web/packages/scripts/src/cherry-pick.ts +++ b/web/packages/scripts/src/cherry-pick.ts @@ -1,11 +1,13 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execSync } from 'child_process'; +import { execFileSync } from 'child_process'; import * as readline from 'readline'; import * as process from 'process'; import { openBrowser, getBaseUrl } from './git-utils.js'; +const git = (...args: string[]) => execFileSync('git', args, { stdio: 'inherit' }); + // Helper to prompt the user for input. function prompt(question: string): Promise { const rl = readline.createInterface({ @@ -29,9 +31,9 @@ async function handleMergeConflicts() { if (response === 'yes') { try { console.log('Staging resolved changes...'); - execSync('git add .', { stdio: 'inherit' }); + git('add', '.'); console.log('Attempting to continue cherry-pick...'); - execSync('git cherry-pick --continue', { stdio: 'inherit' }); + git('cherry-pick', '--continue'); console.log('Cherry-pick completed successfully after resolving conflicts.'); resolved = true; } catch { @@ -43,7 +45,7 @@ async function handleMergeConflicts() { await prompt('Waiting for you to resolve conflicts. Press enter to check again...'); } else if (response === 'abort') { console.log('Aborting cherry-pick...'); - execSync('git cherry-pick --abort', { stdio: 'inherit' }); + git('cherry-pick', '--abort'); process.exit(1); } else { console.log("Please answer 'yes', 'no', or 'abort'."); @@ -63,20 +65,20 @@ async function main() { try { console.log('Fetching latest changes from origin...'); - execSync('git fetch origin', { stdio: 'inherit' }); + git('fetch', 'origin'); console.log(`Checking out the release branch: ${releaseBranch}`); - execSync(`git checkout ${releaseBranch}`, { stdio: 'inherit' }); - execSync(`git pull origin ${releaseBranch}`, { stdio: 'inherit' }); + git('checkout', releaseBranch); + git('pull', 'origin', releaseBranch); // Create a new branch based on the release branch. const newBranchName = `cherry-pick-${commitHash.substring(0, 7)}`; console.log(`Creating and switching to new branch: ${newBranchName}`); - execSync(`git checkout -b ${newBranchName}`, { stdio: 'inherit' }); + git('checkout', '-b', newBranchName); console.log(`Attempting to cherry-pick commit: ${commitHash}`); try { - execSync(`git cherry-pick ${commitHash}`, { stdio: 'inherit' }); + git('cherry-pick', commitHash); console.log('Cherry-pick completed successfully without conflicts.'); } catch { console.error('Merge conflicts detected during cherry-pick!'); @@ -85,10 +87,10 @@ async function main() { // Push the new branch to origin. console.log(`Pushing branch ${newBranchName} to origin...`); - execSync(`git push origin ${newBranchName}`, { stdio: 'inherit' }); + git('push', 'origin', newBranchName); // Retrieve the remote URL to construct the merge request URL. - const remoteUrlRaw = execSync('git remote get-url origin').toString().trim(); + const remoteUrlRaw = execFileSync('git', ['remote', 'get-url', 'origin']).toString().trim(); const baseUrl = getBaseUrl(remoteUrlRaw); const mergeRequestUrl = `${baseUrl}/-/merge_requests/new?merge_request[source_branch]=${newBranchName}&merge_request[target_branch]=${releaseBranch}`; diff --git a/web/packages/scripts/src/git-utils.ts b/web/packages/scripts/src/git-utils.ts index 4b062a0889..a8fe7c0e2d 100644 --- a/web/packages/scripts/src/git-utils.ts +++ b/web/packages/scripts/src/git-utils.ts @@ -1,24 +1,26 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { exec, execSync } from 'child_process'; +import { execFile, execFileSync } from 'child_process'; import * as process from 'process'; /** * Open a URL in the default browser (works on macOS, Windows, and most Linux distros). */ export function openBrowser(url: string): void { - let command = ''; - // Escape the URL to prevent shell interpretation of special characters - const escapedUrl = url.replace(/"/g, '\\"'); + let cmd: string; + let args: string[]; if (process.platform === 'darwin') { - command = `open "${escapedUrl}"`; + cmd = 'open'; + args = [url]; } else if (process.platform === 'win32') { - command = `start "" "${escapedUrl}"`; + cmd = 'cmd'; + args = ['/c', 'start', '', url]; } else { - command = `xdg-open "${escapedUrl}"`; + cmd = 'xdg-open'; + args = [url]; } - exec(command, (error) => { + execFile(cmd, args, (error) => { if (error) { console.error('Failed to open browser:', error); } @@ -56,7 +58,7 @@ export function getBaseUrl(remoteUrl: string): string { // Check if git status is clean (no uncommitted changes) export function isGitStatusClean(): boolean { try { - const status = execSync('git status --porcelain').toString().trim(); + const status = execFileSync('git', ['status', '--porcelain']).toString().trim(); return status === ''; } catch (error) { console.error('Failed to check git status:', error); @@ -67,7 +69,7 @@ export function isGitStatusClean(): boolean { // Get the current branch name export function getCurrentBranch(): string { try { - return execSync('git rev-parse --abbrev-ref HEAD').toString().trim(); + return execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD']).toString().trim(); } catch (error) { console.error('Failed to get current branch:', error); throw error; diff --git a/web/packages/sdk/orval/format-generated.ts b/web/packages/sdk/orval/format-generated.ts index 85b860b811..ad58bb6792 100755 --- a/web/packages/sdk/orval/format-generated.ts +++ b/web/packages/sdk/orval/format-generated.ts @@ -7,7 +7,7 @@ * Runs prettier and eslint fix on generated API files, and prefixes unused parameters with underscores. */ -import { execSync } from 'child_process'; +import { execFileSync } from 'child_process'; import { mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; @@ -475,7 +475,7 @@ try { // Step 3: Run prettier console.log('Running prettier...'); - execSync(`prettier --write ${generatedPath}`, { + execFileSync('prettier', ['--write', generatedPath], { stdio: 'inherit', cwd: path.join(__dirname, '..'), }); diff --git a/web/packages/sdk/orval/generate.ts b/web/packages/sdk/orval/generate.ts index 9030da4f55..de88a318bc 100644 --- a/web/packages/sdk/orval/generate.ts +++ b/web/packages/sdk/orval/generate.ts @@ -7,7 +7,7 @@ * For private GitHub raw URLs, set the GITHUB_TOKEN environment variable. * For local files, no token is required. */ -import { execSync } from 'child_process'; +import { execFileSync } from 'child_process'; import fs from 'fs'; import os from 'os'; import { serviceConfigs } from './constants'; @@ -15,6 +15,12 @@ import path from 'path'; import { generateCustomFetcher } from './generateCustomFetcher'; import { getGithubTokenHeaders } from './githubTokenHeaders'; +const ALLOWED_SPEC_HOSTS = new Set([ + 'github.com', + 'raw.githubusercontent.com', + 'gitlab-master.nvidia.com', +]); + const githubToken = process.env.GITHUB_TOKEN; const client = process.env.ORVAL_CLIENT; @@ -28,8 +34,11 @@ if (!config) { const getFile = async () => { if (config.url.startsWith('http')) { const remoteUrl = new URL(config.url); + if (remoteUrl.protocol !== 'https:' || !ALLOWED_SPEC_HOSTS.has(remoteUrl.hostname)) { + throw new Error(`Refusing to fetch spec from disallowed host: ${remoteUrl.hostname}`); + } const headers = getGithubTokenHeaders(remoteUrl, githubToken); - const res = await fetch(config.url, headers ? { headers } : undefined); + const res = await fetch(remoteUrl, headers ? { headers } : undefined); if (Math.floor(res.status / 100) !== 2) { throw new Error(`${res.status} - Failed to fetch spec. ${res.statusText}`); } @@ -49,14 +58,18 @@ const getFile = async () => { const postProcessZodFiles = (zodPath: string) => { const zodDefaultFile = path.join(__dirname, '..', zodPath); - if (!fs.existsSync(zodDefaultFile)) { - console.log(`Zod file not found at ${zodDefaultFile}, skipping post-processing`); - return; + let content: string; + try { + content = fs.readFileSync(zodDefaultFile, 'utf8'); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + console.log(`Zod file not found at ${zodDefaultFile}, skipping post-processing`); + return; + } + throw err; } console.log(`Post-processing Zod file: ${zodDefaultFile}`); - - const content = fs.readFileSync(zodDefaultFile, 'utf8'); const lines = content.split('\n'); let fixCount = 0; @@ -109,10 +122,8 @@ const postProcessZodFiles = (zodPath: string) => { const main = async () => { console.log(`Generating types for: ${service}.`); const spec = await getFile(); - // Per-run temp dir with random suffix avoids predictable paths in a shared tmp. const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openapi-spec-')); const tempFile = path.join(tempDir, `${config.path}.yaml`); - const clientVar = client ? `ORVAL_CLIENT=${client}` : ''; const target = client === 'zod' ? `./generated/${config.path}/zod/index.ts` @@ -125,12 +136,17 @@ const main = async () => { } try { - execSync( - `ORVAL_SERVICE=${service} ORVAL_INPUT=${tempFile} ${clientVar} ORVAL_TARGET=${target} ORVAL_SCHEMAS=./generated/${config.path}/schema pnpm exec orval`, - { - stdio: 'inherit', - } - ); + const orvalEnv: NodeJS.ProcessEnv = { + ...process.env, + ORVAL_SERVICE: service, + ORVAL_INPUT: tempFile, + ORVAL_TARGET: target, + ORVAL_SCHEMAS: `./generated/${config.path}/schema`, + }; + if (client) { + orvalEnv.ORVAL_CLIENT = client; + } + execFileSync('pnpm', ['exec', 'orval'], { stdio: 'inherit', env: orvalEnv }); // Post-process Zod files if generating with zod client if (client === 'zod') { diff --git a/web/packages/studio/scripts/fetch-styles.ts b/web/packages/studio/scripts/fetch-styles.ts index eb5a032c07..ee2c067929 100644 --- a/web/packages/studio/scripts/fetch-styles.ts +++ b/web/packages/studio/scripts/fetch-styles.ts @@ -14,6 +14,7 @@ import { parse } from 'yaml'; const PKG = '@nvidia/foundations-react-core'; const CDN = 'https://webassets.nvidia.com/kaizen-ui-foundations'; +const CDN_URL = new URL(CDN); const FILES = ['base-external.css', 'components.css']; const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -69,7 +70,10 @@ async function readCachedVersion() { } async function fetchCss(version: string, file: string) { - const url = `${CDN}/${version}/${file}`; + const url = new URL(`${encodeURIComponent(version)}/${encodeURIComponent(file)}`, `${CDN}/`); + if (url.protocol !== 'https:' || url.host !== CDN_URL.host) { + throw new Error(`Refusing to fetch from disallowed origin: ${url.origin}`); + } const res = await fetch(url); if (!res.ok) { throw new Error(`Failed to fetch ${url}: ${res.status} ${res.statusText}`); From a42e8b5eacda32f208a20ccac90c30c69cd99726 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 12:34:38 -0700 Subject: [PATCH 03/10] fix: close remaining CodeQL alerts re-emitted on PR scan - scripts/git-utils.ts openBrowser: parse URL with `new URL()` and require http/https before spawning. Replace the Windows `cmd /c start` shell invocation with `rundll32 url.dll,FileProtocolHandler` so no branch goes through a shell. Pass `--` separator on darwin/linux so a URL starting with `-` cannot be parsed as an option. Closes #3951. - sdk/orval/generate.ts: delete the unused HTTP-fetch branch from `getFile()`. All current `serviceConfigs` reference local YAML paths, so the network->file write CodeQL flagged on line 131 (#14) no longer exists. Throws a clear error if a remote URL is configured. Signed-off-by: mschwab --- web/packages/scripts/src/git-utils.ts | 23 +++++++++++---- web/packages/sdk/orval/generate.ts | 41 +++++++-------------------- 2 files changed, 29 insertions(+), 35 deletions(-) diff --git a/web/packages/scripts/src/git-utils.ts b/web/packages/scripts/src/git-utils.ts index a8fe7c0e2d..9dc6d94800 100644 --- a/web/packages/scripts/src/git-utils.ts +++ b/web/packages/scripts/src/git-utils.ts @@ -5,20 +5,33 @@ import { execFile, execFileSync } from 'child_process'; import * as process from 'process'; /** - * Open a URL in the default browser (works on macOS, Windows, and most Linux distros). + * Open an HTTP/HTTPS URL in the default browser (works on macOS, Windows, and most Linux distros). */ export function openBrowser(url: string): void { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + console.error('Refusing to open invalid URL:', url); + return; + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + console.error('Refusing to open non-http(s) URL:', parsed.protocol); + return; + } + const safeUrl = parsed.toString(); + let cmd: string; let args: string[]; if (process.platform === 'darwin') { cmd = 'open'; - args = [url]; + args = ['--', safeUrl]; } else if (process.platform === 'win32') { - cmd = 'cmd'; - args = ['/c', 'start', '', url]; + cmd = 'rundll32'; + args = ['url.dll,FileProtocolHandler', safeUrl]; } else { cmd = 'xdg-open'; - args = [url]; + args = ['--', safeUrl]; } execFile(cmd, args, (error) => { if (error) { diff --git a/web/packages/sdk/orval/generate.ts b/web/packages/sdk/orval/generate.ts index de88a318bc..182ce75940 100644 --- a/web/packages/sdk/orval/generate.ts +++ b/web/packages/sdk/orval/generate.ts @@ -2,10 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 /** - * This script generates the types for the openapi specs. - * - * For private GitHub raw URLs, set the GITHUB_TOKEN environment variable. - * For local files, no token is required. + * This script generates the types for the openapi specs from local YAML files. */ import { execFileSync } from 'child_process'; import fs from 'fs'; @@ -13,15 +10,7 @@ import os from 'os'; import { serviceConfigs } from './constants'; import path from 'path'; import { generateCustomFetcher } from './generateCustomFetcher'; -import { getGithubTokenHeaders } from './githubTokenHeaders'; -const ALLOWED_SPEC_HOSTS = new Set([ - 'github.com', - 'raw.githubusercontent.com', - 'gitlab-master.nvidia.com', -]); - -const githubToken = process.env.GITHUB_TOKEN; const client = process.env.ORVAL_CLIENT; const service = process.argv[2] as keyof typeof serviceConfigs; @@ -31,24 +20,16 @@ if (!config) { throw new Error('Unsupported OpenAPI Spec.'); } -const getFile = async () => { - if (config.url.startsWith('http')) { - const remoteUrl = new URL(config.url); - if (remoteUrl.protocol !== 'https:' || !ALLOWED_SPEC_HOSTS.has(remoteUrl.hostname)) { - throw new Error(`Refusing to fetch spec from disallowed host: ${remoteUrl.hostname}`); - } - const headers = getGithubTokenHeaders(remoteUrl, githubToken); - const res = await fetch(remoteUrl, headers ? { headers } : undefined); - if (Math.floor(res.status / 100) !== 2) { - throw new Error(`${res.status} - Failed to fetch spec. ${res.statusText}`); - } - return await res.text(); - } else { - // Load local file otherwise - const filePath = path.resolve(__dirname, config.url); - const spec = fs.readFileSync(filePath, 'utf8'); - return spec; - } +if (config.url.startsWith('http')) { + throw new Error( + `Remote spec URLs are not supported by this script. Got: ${config.url}. ` + + `Vendor the spec locally and reference it by relative path.` + ); +} + +const getFile = () => { + const filePath = path.resolve(__dirname, config.url); + return fs.readFileSync(filePath, 'utf8'); }; /** From 0448c8fd7c976d19626d0a321a6dfcd946f728e3 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 12:56:25 -0700 Subject: [PATCH 04/10] fix: drop -- separator for xdg-open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xdg-open does not honor -- as an option terminator; passing it as an arg caused openBrowser to fail on Linux. URL is already validated to http(s), so the separator wasn't load-bearing ā€” just drop it on the Linux branch. Codex review on PR #75. Signed-off-by: mschwab --- web/packages/scripts/src/git-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/scripts/src/git-utils.ts b/web/packages/scripts/src/git-utils.ts index 9dc6d94800..9c949c91fa 100644 --- a/web/packages/scripts/src/git-utils.ts +++ b/web/packages/scripts/src/git-utils.ts @@ -31,7 +31,7 @@ export function openBrowser(url: string): void { args = ['url.dll,FileProtocolHandler', safeUrl]; } else { cmd = 'xdg-open'; - args = ['--', safeUrl]; + args = [safeUrl]; } execFile(cmd, args, (error) => { if (error) { From ec7aa93c150fb6a2a3014b09401335efa8b0bccc Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 13:00:28 -0700 Subject: [PATCH 05/10] fix: address CodeRabbit findings on PR #75 - scripts/git-utils.ts: drop `--` from macOS `open` argv too. `open`'s man page does not document `--` as an end-of-options separator. URL is already validated to http(s), so the separator wasn't load-bearing. - sdk/orval/format-generated.ts: on Windows, run prettier through `cmd.exe /c` so the `prettier.cmd` shim resolves. `execFileSync` on Windows cannot launch .cmd shims directly. - sdk/orval/generate.ts: same Windows wrap for `pnpm exec orval`. Signed-off-by: mschwab --- web/packages/scripts/src/git-utils.ts | 2 +- web/packages/sdk/orval/format-generated.ts | 13 +++++++++---- web/packages/sdk/orval/generate.ts | 7 ++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/web/packages/scripts/src/git-utils.ts b/web/packages/scripts/src/git-utils.ts index 9c949c91fa..92892d28de 100644 --- a/web/packages/scripts/src/git-utils.ts +++ b/web/packages/scripts/src/git-utils.ts @@ -25,7 +25,7 @@ export function openBrowser(url: string): void { let args: string[]; if (process.platform === 'darwin') { cmd = 'open'; - args = ['--', safeUrl]; + args = [safeUrl]; } else if (process.platform === 'win32') { cmd = 'rundll32'; args = ['url.dll,FileProtocolHandler', safeUrl]; diff --git a/web/packages/sdk/orval/format-generated.ts b/web/packages/sdk/orval/format-generated.ts index ad58bb6792..f4335570c0 100755 --- a/web/packages/sdk/orval/format-generated.ts +++ b/web/packages/sdk/orval/format-generated.ts @@ -475,10 +475,15 @@ try { // Step 3: Run prettier console.log('Running prettier...'); - execFileSync('prettier', ['--write', generatedPath], { - stdio: 'inherit', - cwd: path.join(__dirname, '..'), - }); + const isWindows = process.platform === 'win32'; + execFileSync( + isWindows ? 'cmd.exe' : 'prettier', + isWindows ? ['/c', 'prettier', '--write', generatedPath] : ['--write', generatedPath], + { + stdio: 'inherit', + cwd: path.join(__dirname, '..'), + } + ); console.log('āœ… Successfully processed generated files\n'); } catch (error) { diff --git a/web/packages/sdk/orval/generate.ts b/web/packages/sdk/orval/generate.ts index 182ce75940..2b6c57163c 100644 --- a/web/packages/sdk/orval/generate.ts +++ b/web/packages/sdk/orval/generate.ts @@ -127,7 +127,12 @@ const main = async () => { if (client) { orvalEnv.ORVAL_CLIENT = client; } - execFileSync('pnpm', ['exec', 'orval'], { stdio: 'inherit', env: orvalEnv }); + const isWindows = process.platform === 'win32'; + execFileSync( + isWindows ? 'cmd.exe' : 'pnpm', + isWindows ? ['/c', 'pnpm', 'exec', 'orval'] : ['exec', 'orval'], + { stdio: 'inherit', env: orvalEnv } + ); // Post-process Zod files if generating with zod client if (client === 'zod') { From c0691e95e86c89f9b6d52ac88e6628cea0e94378 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 13:05:38 -0700 Subject: [PATCH 06/10] fix: validate format-generated.ts servicePath argv The Windows cmd.exe /c wrap added in ec7aa93c15 re-opened a CodeQL data-flow finding (#3961, #3962) because generatedPath traces back to process.argv[2]. Validate the argv against a safe-char regex at entry so CodeQL sees it as sanitized before it flows into argv or paths. Signed-off-by: mschwab --- web/packages/sdk/orval/format-generated.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/web/packages/sdk/orval/format-generated.ts b/web/packages/sdk/orval/format-generated.ts index f4335570c0..d3c1eb58d7 100755 --- a/web/packages/sdk/orval/format-generated.ts +++ b/web/packages/sdk/orval/format-generated.ts @@ -16,15 +16,21 @@ import { fileURLToPath } from 'url'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); -// Get the service path from command line args -const servicePath = process.argv[2]; +const SERVICE_PATH_PATTERN = /^[a-zA-Z0-9_-]+$/; +const rawServicePath = process.argv[2]; -if (!servicePath) { +if (!rawServicePath) { console.error('Error: Service path is required'); console.error('Usage: node format-generated.js '); process.exit(1); } +if (!SERVICE_PATH_PATTERN.test(rawServicePath)) { + console.error(`Error: Invalid service path: ${rawServicePath}`); + process.exit(1); +} + +const servicePath: string = rawServicePath; const generatedPath = path.join(__dirname, '..', 'generated', servicePath); console.log(`\nšŸ“ Processing generated files in ${generatedPath}...`); From f2db03b92329e9283ff9659bf1ea851121476cd9 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 13:15:56 -0700 Subject: [PATCH 07/10] fix: replace regex with hardcoded Set allowlist for servicePath CodeQL did not recognize the regex check as a sanitizer; switching to a hardcoded Set lookup against known serviceConfigs paths so the data flow is reducible to a finite set of literal values. Signed-off-by: mschwab --- web/packages/sdk/orval/format-generated.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/web/packages/sdk/orval/format-generated.ts b/web/packages/sdk/orval/format-generated.ts index d3c1eb58d7..1218585741 100755 --- a/web/packages/sdk/orval/format-generated.ts +++ b/web/packages/sdk/orval/format-generated.ts @@ -11,12 +11,15 @@ import { execFileSync } from 'child_process'; import { mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; +import { serviceConfigs } from './constants'; // Get __dirname equivalent for ES modules const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); -const SERVICE_PATH_PATTERN = /^[a-zA-Z0-9_-]+$/; +const ALLOWED_SERVICE_PATHS: ReadonlySet = new Set( + Object.values(serviceConfigs).map((c) => c.path) +); const rawServicePath = process.argv[2]; if (!rawServicePath) { @@ -25,12 +28,13 @@ if (!rawServicePath) { process.exit(1); } -if (!SERVICE_PATH_PATTERN.test(rawServicePath)) { - console.error(`Error: Invalid service path: ${rawServicePath}`); +if (!ALLOWED_SERVICE_PATHS.has(rawServicePath)) { + console.error(`Error: Unknown service path: ${rawServicePath}`); + console.error(`Allowed: ${[...ALLOWED_SERVICE_PATHS].join(', ')}`); process.exit(1); } -const servicePath: string = rawServicePath; +const servicePath = rawServicePath; const generatedPath = path.join(__dirname, '..', 'generated', servicePath); console.log(`\nšŸ“ Processing generated files in ${generatedPath}...`); From 145c900ade6379ab9a4053182d422a90eb3a9f29 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 13:31:51 -0700 Subject: [PATCH 08/10] fix: use prettier Node API instead of subprocess Replace the prettier CLI invocation with prettier's programmatic format/resolveConfig/getFileInfo API. No subprocess means no cmd.exe wrap, no command-line argument flow, and the CodeQL indirect-command-line-injection / shell-cmd-injection-from-env alerts on format-generated.ts can resolve. Also fixes the Windows .cmd shim resolution problem CR raised, since prettier now runs in-process. The servicePath argv is still validated against a hardcoded Set of known serviceConfigs paths to prevent directory traversal via path.join. Signed-off-by: mschwab --- web/packages/sdk/orval/format-generated.ts | 47 +++++++++++++--------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/web/packages/sdk/orval/format-generated.ts b/web/packages/sdk/orval/format-generated.ts index 1218585741..064a8c132a 100755 --- a/web/packages/sdk/orval/format-generated.ts +++ b/web/packages/sdk/orval/format-generated.ts @@ -7,10 +7,10 @@ * Runs prettier and eslint fix on generated API files, and prefixes unused parameters with underscores. */ -import { execFileSync } from 'child_process'; import { mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; +import prettier from 'prettier'; import { serviceConfigs } from './constants'; // Get __dirname equivalent for ES modules @@ -462,41 +462,48 @@ function splitZodTagFilesIn(zodDir: string): void { } } -try { - // Step 1: Prefix unused parameters with underscore +async function formatWithPrettier(dir: string): Promise { + const entries = readdirSync(dir); + for (const entry of entries) { + const fullPath = path.join(dir, entry); + const stat = statSync(fullPath); + if (stat.isDirectory()) { + await formatWithPrettier(fullPath); + continue; + } + const fileInfo = await prettier.getFileInfo(fullPath); + if (fileInfo.ignored || !fileInfo.inferredParser) continue; + const opts = (await prettier.resolveConfig(fullPath)) ?? {}; + const source = readFileSync(fullPath, 'utf-8'); + const formatted = await prettier.format(source, { ...opts, filepath: fullPath }); + if (formatted !== source) { + writeFileSync(fullPath, formatted, 'utf-8'); + } + } +} + +async function run(): Promise { console.log('Prefixing unused parameters...'); const tsFiles = getTsFiles(generatedPath); let modifiedCount = 0; - for (const file of tsFiles) { if (prefixUnusedParameters(file)) { modifiedCount++; } } - console.log(` Modified ${modifiedCount} file(s)`); - // Step 2: Split large orval-generated zod tag files into per-operation files. - // Each tag file (e.g. zod/evaluator.ts) gets replaced with a barrel and a - // sibling directory of per-operation files (zod/evaluator/.ts). const zodDir = path.join(generatedPath, 'zod'); console.log('Splitting zod tag files by operation...'); splitZodTagFilesIn(zodDir); - // Step 3: Run prettier console.log('Running prettier...'); - const isWindows = process.platform === 'win32'; - execFileSync( - isWindows ? 'cmd.exe' : 'prettier', - isWindows ? ['/c', 'prettier', '--write', generatedPath] : ['--write', generatedPath], - { - stdio: 'inherit', - cwd: path.join(__dirname, '..'), - } - ); + await formatWithPrettier(generatedPath); console.log('āœ… Successfully processed generated files\n'); -} catch (error) { +} + +run().catch((error) => { console.error('āŒ Error during processing:', (error as Error).message); process.exit(1); -} +}); From daef8ad24e8c52582d9ff6877d17ccb33923af14 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 13:38:33 -0700 Subject: [PATCH 09/10] fix: use readdirSync withFileTypes to avoid statSync TOCTOU CodeQL flagged the statSync -> readFileSync / writeFileSync pair in formatWithPrettier as a file-system-race. Getting Dirent entries from readdirSync(dir, { withFileTypes: true }) lets us check isDirectory / isFile inline without a separate stat round-trip, closing the alert. Signed-off-by: mschwab --- web/packages/sdk/orval/format-generated.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/packages/sdk/orval/format-generated.ts b/web/packages/sdk/orval/format-generated.ts index 064a8c132a..4c7ec0ba3b 100755 --- a/web/packages/sdk/orval/format-generated.ts +++ b/web/packages/sdk/orval/format-generated.ts @@ -463,14 +463,14 @@ function splitZodTagFilesIn(zodDir: string): void { } async function formatWithPrettier(dir: string): Promise { - const entries = readdirSync(dir); + const entries = readdirSync(dir, { withFileTypes: true }); for (const entry of entries) { - const fullPath = path.join(dir, entry); - const stat = statSync(fullPath); - if (stat.isDirectory()) { + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { await formatWithPrettier(fullPath); continue; } + if (!entry.isFile()) continue; const fileInfo = await prettier.getFileInfo(fullPath); if (fileInfo.ignored || !fileInfo.inferredParser) continue; const opts = (await prettier.resolveConfig(fullPath)) ?? {}; From f8929fc1908a42de84911dc8f6cda6b95edbbef1 Mon Sep 17 00:00:00 2001 From: mschwab Date: Wed, 27 May 2026 13:52:41 -0700 Subject: [PATCH 10/10] fix: drop remaining statSync usages in format-generated.ts Codex flagged that getTsFiles and splitZodTagFilesIn still used the readdir-string + statSync pattern, leaving two more file-system-race sinks even after formatWithPrettier was converted. Switch both to readdirSync(dir, { withFileTypes: true }) and use Dirent.isFile() / isDirectory() inline. Removes the last statSync from this script. Signed-off-by: mschwab --- web/packages/sdk/orval/format-generated.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/web/packages/sdk/orval/format-generated.ts b/web/packages/sdk/orval/format-generated.ts index 4c7ec0ba3b..c06b20424e 100755 --- a/web/packages/sdk/orval/format-generated.ts +++ b/web/packages/sdk/orval/format-generated.ts @@ -7,7 +7,7 @@ * Runs prettier and eslint fix on generated API files, and prefixes unused parameters with underscores. */ -import { mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from 'fs'; +import { mkdirSync, readdirSync, readFileSync, writeFileSync, type Dirent } from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; import prettier from 'prettier'; @@ -46,15 +46,14 @@ function getTsFiles(dir: string): string[] { const files: string[] = []; try { - const entries = readdirSync(dir); + const entries = readdirSync(dir, { withFileTypes: true }); for (const entry of entries) { - const fullPath = path.join(dir, entry); - const stat = statSync(fullPath); + const fullPath = path.join(dir, entry.name); - if (stat.isDirectory()) { + if (entry.isDirectory()) { files.push(...getTsFiles(fullPath)); - } else if (entry.endsWith('.ts')) { + } else if (entry.isFile() && entry.name.endsWith('.ts')) { files.push(fullPath); } } @@ -445,19 +444,18 @@ function splitZodTagFile(filePath: string): number { } function splitZodTagFilesIn(zodDir: string): void { - let entries: string[]; + let entries: Dirent[]; try { - entries = readdirSync(zodDir); + entries = readdirSync(zodDir, { withFileTypes: true }); } catch { return; } for (const entry of entries) { - const fullPath = path.join(zodDir, entry); - if (!entry.endsWith('.ts')) continue; - if (!statSync(fullPath).isFile()) continue; + if (!entry.isFile() || !entry.name.endsWith('.ts')) continue; + const fullPath = path.join(zodDir, entry.name); const count = splitZodTagFile(fullPath); if (count > 0) { - console.log(` Split ${entry} into ${count} operation files`); + console.log(` Split ${entry.name} into ${count} operation files`); } } }