From 9251d7e85603c879921845460578da18106ad1eb Mon Sep 17 00:00:00 2001 From: SoundMindsAI Date: Tue, 2 Jun 2026 21:57:07 -0400 Subject: [PATCH 1/2] fix(security): use execFileSync in gen-types + document path-injection FP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the 3 CodeQL findings from the first scan of main (PR #431 enabled CodeQL): - gen-types.mjs (medium, js/shell-command-injection-from-environment): the OPENAPI_URL env var was interpolated into an execSync shell string. Switch to execFileSync with an args array — no shell, nothing to inject into. It's a local dev script (CI never runs it) so the real risk was low, but this is the correct pattern regardless. node --check + eslint + prettier clean. - config_repos.py (high ×2, py/path-injection): false positives. `auth_ref` is constrained to `^[a-zA-Z0-9_-]+$` by CreateConfigRepoRequest (no slashes/dots → no traversal at the API boundary) AND _auth_ref_exists has a resolve()+relative_to() containment guard. CodeQL doesn't model the Pydantic pattern or recognize relative_to() as a sanitizer. Added a comment explaining the two guards; the alerts are dismissed as reviewed false positives. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: SoundMindsAI --- backend/app/api/v1/config_repos.py | 8 ++++++++ ui/scripts/gen-types.mjs | 8 ++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/backend/app/api/v1/config_repos.py b/backend/app/api/v1/config_repos.py index 0ca58f9a..032ac645 100644 --- a/backend/app/api/v1/config_repos.py +++ b/backend/app/api/v1/config_repos.py @@ -86,6 +86,14 @@ def _auth_ref_exists(auth_ref: str) -> bool: return False override = os.environ.get("RELYLOOP_SECRETS_DIR") secrets_root = Path(override).resolve() if override else Path("./secrets").resolve() + # Path containment guard. `auth_ref` is already constrained to + # `^[a-zA-Z0-9_-]+$` by CreateConfigRepoRequest (no slashes/dots → no + # traversal at the API boundary); this resolve()+relative_to() is a second + # layer that also catches symlink escape and any non-HTTP caller. CodeQL's + # py/path-injection flags this candidate/is_file() pair because it doesn't + # model the Pydantic pattern or recognize relative_to() as a sanitizer — + # dismissed as a reviewed false positive (the input cannot escape + # secrets_root). candidate = (secrets_root / auth_ref).resolve() try: candidate.relative_to(secrets_root) diff --git a/ui/scripts/gen-types.mjs b/ui/scripts/gen-types.mjs index 68b673ac..9c1fd80f 100644 --- a/ui/scripts/gen-types.mjs +++ b/ui/scripts/gen-types.mjs @@ -13,7 +13,7 @@ * Or via the package script: pnpm types:gen */ -import { execSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process'; import { readFileSync, writeFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { dirname, resolve } from 'node:path'; @@ -40,7 +40,11 @@ const BANNER = `// SPDX-FileCopyrightText: 2026 soundminds.ai `; console.log(`Generating ${OUTPUT} from ${SOURCE_URL}…`); -execSync(`npx openapi-typescript ${SOURCE_URL} -o ${OUTPUT}`, { +// execFileSync (no shell) instead of execSync with an interpolated string: +// SOURCE_URL comes from the OPENAPI_URL env var, and an interpolated shell +// command would let a crafted value inject. Passing args as an array runs the +// binary directly with no shell, so there is nothing to inject into. +execFileSync('npx', ['openapi-typescript', SOURCE_URL, '-o', OUTPUT], { stdio: 'inherit', cwd: UI_ROOT, }); From 4174ff8bf00937dcc55a70385aec2701edcfbf9f Mon Sep 17 00:00:00 2001 From: SoundMindsAI Date: Tue, 2 Jun 2026 22:11:56 -0400 Subject: [PATCH 2/2] fix(security): resolve npx.cmd on Windows in gen-types (Gemini #432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept Gemini's finding: execFileSync('npx', …) ENOENTs on Windows because the launcher is npx.cmd and there's no shell to resolve the extension. Pick the platform-correct executable name. Preserves the no-shell args-array form (the shell-injection fix); just restores the cross-platform parity the original execSync had. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: SoundMindsAI --- ui/scripts/gen-types.mjs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ui/scripts/gen-types.mjs b/ui/scripts/gen-types.mjs index 9c1fd80f..2f16d1f0 100644 --- a/ui/scripts/gen-types.mjs +++ b/ui/scripts/gen-types.mjs @@ -43,8 +43,11 @@ console.log(`Generating ${OUTPUT} from ${SOURCE_URL}…`); // execFileSync (no shell) instead of execSync with an interpolated string: // SOURCE_URL comes from the OPENAPI_URL env var, and an interpolated shell // command would let a crafted value inject. Passing args as an array runs the -// binary directly with no shell, so there is nothing to inject into. -execFileSync('npx', ['openapi-typescript', SOURCE_URL, '-o', OUTPUT], { +// binary directly with no shell, so there is nothing to inject into. On Windows +// the launcher is `npx.cmd` (no shell to resolve the `.cmd` extension), so pick +// the platform-correct executable name. +const NPX = process.platform === 'win32' ? 'npx.cmd' : 'npx'; +execFileSync(NPX, ['openapi-typescript', SOURCE_URL, '-o', OUTPUT], { stdio: 'inherit', cwd: UI_ROOT, });