From f00a0a3bf910fc166608e8d6ff41e84ba02ae7fa Mon Sep 17 00:00:00 2001 From: Sunny Sethi Date: Tue, 26 May 2026 14:31:03 +0530 Subject: [PATCH] fix(security): use atomic temp-dir swap in prepareArtifact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F-013 / DEVA11Y-482 — prepareArtifact had a TOCTOU race (CWE-362) where the check-delete-create-download sequence left a large window for parallel builds to corrupt state. Download into a temp directory and atomically move to the version directory after completion. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../BrowserStackAccessibilityLint.swift | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift index 117e362..c901879 100644 --- a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift +++ b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift @@ -199,33 +199,40 @@ private struct BrowserStackCLIDownloader { return BrowserStackCLIArtifact(version: info.version, executableURL: expectedExecutableURL) } - if fileManager.fileExists(atPath: versionDirectory.path) { - try fileManager.removeItem(at: versionDirectory) - } - try fileManager.createDirectory(at: versionDirectory, withIntermediateDirectories: true) - Diagnostics.remark("BrowserStackAccessibilityLint: Downloading CLI \(info.version)...") + // Download into a temporary directory to avoid TOCTOU races + let tempDirectory = cacheRoot.appendingPathComponent(".download-\(UUID().uuidString)", isDirectory: true) + try fileManager.createDirectory(at: tempDirectory, withIntermediateDirectories: true) + defer { try? fileManager.removeItem(at: tempDirectory) } + #if os(Windows) - let archiveURL = versionDirectory.appendingPathComponent("browserstack-cli.zip") + let archiveURL = tempDirectory.appendingPathComponent("browserstack-cli.zip") try await download(from: info.resolvedURL, to: archiveURL) Diagnostics.remark("BrowserStackAccessibilityLint: Extracting CLI \(info.version)...") - try unzip(archive: archiveURL, into: versionDirectory) + try unzip(archive: archiveURL, into: tempDirectory) try? fileManager.removeItem(at: archiveURL) #else - try extractWithBsdtar(from: info.resolvedURL, into: versionDirectory) + try extractWithBsdtar(from: info.resolvedURL, into: tempDirectory) #endif - let locatedBinary = try locateExecutable(in: versionDirectory, preferredName: executableName) - let finalBinaryURL: URL - if locatedBinary.lastPathComponent == executableName { - finalBinaryURL = locatedBinary - } else { - finalBinaryURL = expectedExecutableURL - if fileManager.fileExists(atPath: finalBinaryURL.path) { - try fileManager.removeItem(at: finalBinaryURL) + let locatedBinary = try locateExecutable(in: tempDirectory, preferredName: executableName) + + // Atomically swap: remove old version dir, move temp into place + if fileManager.fileExists(atPath: versionDirectory.path) { + try fileManager.removeItem(at: versionDirectory) + } + try fileManager.moveItem(at: tempDirectory, to: versionDirectory) + + let finalBinaryURL = versionDirectory.appendingPathComponent(locatedBinary.lastPathComponent, isDirectory: false) + if locatedBinary.lastPathComponent != executableName { + let expectedURL = versionDirectory.appendingPathComponent(executableName, isDirectory: false) + if fileManager.fileExists(atPath: expectedURL.path) { + try fileManager.removeItem(at: expectedURL) } - try fileManager.moveItem(at: locatedBinary, to: finalBinaryURL) + try fileManager.moveItem(at: finalBinaryURL, to: expectedURL) + try ensureExecutablePermissions(at: expectedURL) + return BrowserStackCLIArtifact(version: info.version, executableURL: expectedURL) } try ensureExecutablePermissions(at: finalBinaryURL)