Skip to content

Commit f00a0a3

Browse files
sunny-seclaude
andcommitted
fix(security): use atomic temp-dir swap in prepareArtifact
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) <noreply@anthropic.com>
1 parent 0428b32 commit f00a0a3

1 file changed

Lines changed: 24 additions & 17 deletions

File tree

Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -199,33 +199,40 @@ private struct BrowserStackCLIDownloader {
199199
return BrowserStackCLIArtifact(version: info.version, executableURL: expectedExecutableURL)
200200
}
201201

202-
if fileManager.fileExists(atPath: versionDirectory.path) {
203-
try fileManager.removeItem(at: versionDirectory)
204-
}
205-
try fileManager.createDirectory(at: versionDirectory, withIntermediateDirectories: true)
206-
207202
Diagnostics.remark("BrowserStackAccessibilityLint: Downloading CLI \(info.version)...")
208203

204+
// Download into a temporary directory to avoid TOCTOU races
205+
let tempDirectory = cacheRoot.appendingPathComponent(".download-\(UUID().uuidString)", isDirectory: true)
206+
try fileManager.createDirectory(at: tempDirectory, withIntermediateDirectories: true)
207+
defer { try? fileManager.removeItem(at: tempDirectory) }
208+
209209
#if os(Windows)
210-
let archiveURL = versionDirectory.appendingPathComponent("browserstack-cli.zip")
210+
let archiveURL = tempDirectory.appendingPathComponent("browserstack-cli.zip")
211211
try await download(from: info.resolvedURL, to: archiveURL)
212212
Diagnostics.remark("BrowserStackAccessibilityLint: Extracting CLI \(info.version)...")
213-
try unzip(archive: archiveURL, into: versionDirectory)
213+
try unzip(archive: archiveURL, into: tempDirectory)
214214
try? fileManager.removeItem(at: archiveURL)
215215
#else
216-
try extractWithBsdtar(from: info.resolvedURL, into: versionDirectory)
216+
try extractWithBsdtar(from: info.resolvedURL, into: tempDirectory)
217217
#endif
218218

219-
let locatedBinary = try locateExecutable(in: versionDirectory, preferredName: executableName)
220-
let finalBinaryURL: URL
221-
if locatedBinary.lastPathComponent == executableName {
222-
finalBinaryURL = locatedBinary
223-
} else {
224-
finalBinaryURL = expectedExecutableURL
225-
if fileManager.fileExists(atPath: finalBinaryURL.path) {
226-
try fileManager.removeItem(at: finalBinaryURL)
219+
let locatedBinary = try locateExecutable(in: tempDirectory, preferredName: executableName)
220+
221+
// Atomically swap: remove old version dir, move temp into place
222+
if fileManager.fileExists(atPath: versionDirectory.path) {
223+
try fileManager.removeItem(at: versionDirectory)
224+
}
225+
try fileManager.moveItem(at: tempDirectory, to: versionDirectory)
226+
227+
let finalBinaryURL = versionDirectory.appendingPathComponent(locatedBinary.lastPathComponent, isDirectory: false)
228+
if locatedBinary.lastPathComponent != executableName {
229+
let expectedURL = versionDirectory.appendingPathComponent(executableName, isDirectory: false)
230+
if fileManager.fileExists(atPath: expectedURL.path) {
231+
try fileManager.removeItem(at: expectedURL)
227232
}
228-
try fileManager.moveItem(at: locatedBinary, to: finalBinaryURL)
233+
try fileManager.moveItem(at: finalBinaryURL, to: expectedURL)
234+
try ensureExecutablePermissions(at: expectedURL)
235+
return BrowserStackCLIArtifact(version: info.version, executableURL: expectedURL)
229236
}
230237

231238
try ensureExecutablePermissions(at: finalBinaryURL)

0 commit comments

Comments
 (0)