From 86bfc108aea5b3799ea1897d9c141d6cbab3a776 Mon Sep 17 00:00:00 2001 From: Anso Date: Fri, 5 Jun 2026 23:02:08 -0400 Subject: [PATCH] fix(security): resolve open CodeQL path-injection and temp-file alerts (#1322) Re-establish the path-containment barrier inline at the backup readdir sink in restoreStackFiles. The sink previously built backupDir through the getBackupDir helper, whose stack-name validation the static analyzer does not trace, leaving a flagged path-injection sink. The barrier now resolves backupDir against its root and asserts containment inline, the same pattern already used in backupStackFiles. Behavior is unchanged for valid stack names (already validated one line above by resolveStackDir). Scope the js/insecure-temporary-file rule out of e2e/** in the CodeQL config. End-to-end fixtures must seed files into the backend's COMPOSE_DIR so the API under test can read them back; that path is a fixed location under /tmp in both CI and local dev, which the rule flags. A randomized temp directory does not apply because the backend resolves against its own COMPOSE_DIR. Production code is still analyzed. --- .github/codeql/codeql-config.yml | 13 +++++++++++++ backend/src/services/FileSystemService.ts | 12 +++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml index eb7f5682..b65e0275 100644 --- a/.github/codeql/codeql-config.yml +++ b/.github/codeql/codeql-config.yml @@ -13,3 +13,16 @@ query-filters: - backend/src/utils/apiTokenFormat.ts - backend/src/routes/apiTokens.ts - backend/src/__tests__/** + # End-to-end fixtures seed files directly into the backend's COMPOSE_DIR so + # the API under test can read them back. Both the test runner and the backend + # use a fixed path under /tmp (the spec fallback and the CI start-app default + # are both /tmp/compose), so CodeQL flags the hardcoded literal as + # js/insecure-temporary-file. A randomized mkdtemp does not apply here: the + # backend resolves paths against its own COMPOSE_DIR, so a fixture written + # elsewhere would be invisible to it. The predictable-temp-path threat is also + # moot on the ephemeral, single-tenant machines these specs run on. + # Scoped to e2e/** so production code is still analyzed. + - exclude: + id: js/insecure-temporary-file + paths: + - e2e/** diff --git a/backend/src/services/FileSystemService.ts b/backend/src/services/FileSystemService.ts index 7a1cd093..45a174cf 100644 --- a/backend/src/services/FileSystemService.ts +++ b/backend/src/services/FileSystemService.ts @@ -816,7 +816,17 @@ export class FileSystemService { const debug = isDebugEnabled(); const t0 = Date.now(); const stackDir = this.resolveStackDir(stackName); - const backupDir = this.getBackupDir(stackName); + // Canonical js/path-injection barrier at the backup read sink: resolve the + // backup dir against its root and confirm containment inline, mirroring + // backupStackFiles. stackName is already validated by resolveStackDir above; + // re-establishing containment at the readdir sink itself lets static + // analysis see the barrier, which it does not credit through the + // getBackupDir helper. + const backupRoot = path.resolve(getBackupBaseDir()); + const backupDir = path.resolve(backupRoot, String(this.nodeId), stackName); + if (!backupDir.startsWith(backupRoot + path.sep)) { + throw Object.assign(new Error('Path escapes backup directory'), { code: 'INVALID_PATH' }); + } const items = await fsPromises.readdir(backupDir); const backedUp = new Set(items);