fix(spm): eliminate TOCTOU races in cache prep and spm.sh cleanup (DEVA11Y-482,483)

Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift

@@ -202,37 +202,71 @@ 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)
+        // Extract into a per-invocation staging directory so that concurrent plugin
+        // invocations cannot corrupt each other's binary state. Once extraction
+        // completes we atomically move the staging directory into the final
+        // version directory. If the move fails because another invocation already
+        // published the version directory, we treat that instance as the winner and
+        // read the existing binary instead (see DEVA11Y-482 / F-013).
+        let stagingDirectory = cacheRoot.appendingPathComponent(
+            "\(info.version).tmp.\(UUID().uuidString)",
+            isDirectory: true
+        )
+        defer { try? fileManager.removeItem(at: stagingDirectory) }
+
+        try fileManager.createDirectory(at: stagingDirectory, withIntermediateDirectories: true)
 
         Diagnostics.remark("BrowserStackAccessibilityLint: Downloading CLI \(info.version)...")
 
         #if os(Windows)
-        let archiveURL = versionDirectory.appendingPathComponent("browserstack-cli.zip")
+        let archiveURL = stagingDirectory.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: stagingDirectory)
         try? fileManager.removeItem(at: archiveURL)
         #else
-        try extractWithBsdtar(from: info.resolvedURL, into: versionDirectory)
+        try extractWithBsdtar(from: info.resolvedURL, into: stagingDirectory)
         #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)
+        // Normalize the extracted layout inside the staging directory so the
+        // executable lives at <staging>/<executableName>.
+        let stagedBinary = try locateExecutable(in: stagingDirectory, preferredName: executableName)
+        let stagedExecutableURL = stagingDirectory.appendingPathComponent(executableName, isDirectory: false)
+        if stagedBinary.path != stagedExecutableURL.path {
+            if fileManager.fileExists(atPath: stagedExecutableURL.path) {
+                try fileManager.removeItem(at: stagedExecutableURL)
+            }
+            try fileManager.moveItem(at: stagedBinary, to: stagedExecutableURL)
+        }
+        try ensureExecutablePermissions(at: stagedExecutableURL)
+
+        // Atomically publish the staged directory into place. moveItem throws if the
+        // destination already exists, which means another concurrent invocation won
+        // the race — fall back to reading the published binary.
+        do {
+            if forceDownload, fileManager.fileExists(atPath: versionDirectory.path) {
+                // Caller explicitly requested a fresh download. Replace any existing
+                // version directory atomically via a swap through a temp location.
+                let obsoleteDirectory = cacheRoot.appendingPathComponent(
+                    "\(info.version).old.\(UUID().uuidString)",
+                    isDirectory: true
+                )
+                try? fileManager.moveItem(at: versionDirectory, to: obsoleteDirectory)
+                defer { try? fileManager.removeItem(at: obsoleteDirectory) }
+                try fileManager.moveItem(at: stagingDirectory, to: versionDirectory)
+            } else {
+                try fileManager.moveItem(at: stagingDirectory, to: versionDirectory)
             }
-            try fileManager.moveItem(at: locatedBinary, to: finalBinaryURL)
+        } catch {
+            // Another invocation already populated the version directory. If its
+            // binary is present and executable, use it; otherwise surface the error.
+            if fileManager.isExecutableFile(atPath: expectedExecutableURL.path) {
+                return BrowserStackCLIArtifact(version: info.version, executableURL: expectedExecutableURL)
+            }
+            throw error
         }
 
-        try ensureExecutablePermissions(at: finalBinaryURL)
-        return BrowserStackCLIArtifact(version: info.version, executableURL: finalBinaryURL)
+        return BrowserStackCLIArtifact(version: info.version, executableURL: expectedExecutableURL)
     }
 
 #if !os(Windows)

scripts/bash/spm.sh

@@ -39,12 +39,21 @@ EOF
 }
 
 a11y_scan() {
-  # Ensure Package.swift is removed on exit (acts like a finally block)
+  # Scan target is always the directory the user invoked us from.
+  local scan_root="${PWD}"
+
+  # Per-invocation staging directory for the synthetic manifest. Using a unique
+  # temp dir (instead of writing Package.swift into the user's CWD) prevents
+  # concurrent invocations from deleting each other's manifest on exit
+  # (DEVA11Y-483 / F-014). Only created when the user has no Package.swift.
+  local synthetic_pkg_dir=""
+
+  # Ensure the per-invocation temp dir is removed on exit (acts like a finally
+  # block). We only ever delete our own temp dir, never files in the user's CWD.
   cleanup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
+      if [ -n "$synthetic_pkg_dir" ] && [ -d "$synthetic_pkg_dir" ]; then
+          rm -rf -- "$synthetic_pkg_dir"
       fi
-      rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
   }
   trap cleanup EXIT
 
@@ -53,7 +62,9 @@ a11y_scan() {
           return
       fi
 
-      cat > Package.swift <<EOF
+      synthetic_pkg_dir="$(mktemp -d "${TMPDIR:-/tmp}/bstack-a11y-spm.XXXXXX")"
+
+      cat > "${synthetic_pkg_dir}/Package.swift" <<EOF
 // swift-tools-version: 5.9
 import PackageDescription
 
@@ -69,14 +80,24 @@ EOF
 
   setup
   if [[ -z "$EXTRA_ARGS" ]]; then
-    EXTRA_ARGS="--include **/*.swift --include **/*.xib --include **/*.storyboard"
+    EXTRA_ARGS="--include ${scan_root}/**/*.swift --include ${scan_root}/**/*.xib --include ${scan_root}/**/*.storyboard"
+  fi
+
+  # When we synthesized a manifest, run the plugin against that temp package
+  # directory (--package-path). The scan still targets the user's sources because
+  # the include globs are rooted at the original working directory. When the user
+  # already has a Package.swift, scan it in place exactly as before.
+  local package_path_args=()
+  if [ -n "$synthetic_pkg_dir" ]; then
+    package_path_args=(--package-path "$synthetic_pkg_dir")
   fi
+
   env -i HOME="$HOME" \
       XCODE_VERSION_ACTUAL="$XCODE_VERSION_ACTUAL"\
       BROWSERSTACK_USERNAME="$BROWSERSTACK_USERNAME"\
       BROWSERSTACK_ACCESS_KEY="$BROWSERSTACK_ACCESS_KEY"\
       PATH="$PATH" \
-      swift package plugin \
+      swift package "${package_path_args[@]}" plugin \
           --allow-writing-to-directory ~/.cache\
           --allow-writing-to-package-directory\
           --allow-network-connections 'all(ports: [])'\

scripts/fish/spm.sh

@@ -52,12 +52,21 @@ EOF
 }
 
 a11y_scan() {
-  # Ensure Package.swift is removed on exit (acts like a finally block)
+  # Scan target is always the directory the user invoked us from.
+  local scan_root="${PWD}"
+
+  # Per-invocation staging directory for the synthetic manifest. Using a unique
+  # temp dir (instead of writing Package.swift into the user's CWD) prevents
+  # concurrent invocations from deleting each other's manifest on exit
+  # (DEVA11Y-483 / F-014). Only created when the user has no Package.swift.
+  local synthetic_pkg_dir=""
+
+  # Ensure the per-invocation temp dir is removed on exit (acts like a finally
+  # block). We only ever delete our own temp dir, never files in the user's CWD.
   cleanup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
+      if [ -n "$synthetic_pkg_dir" ] && [ -d "$synthetic_pkg_dir" ]; then
+          rm -rf -- "$synthetic_pkg_dir"
       fi
-      rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
   }
   trap cleanup EXIT
 
@@ -66,7 +75,9 @@ a11y_scan() {
           return
       fi
 
-      cat > Package.swift <<EOF
+      synthetic_pkg_dir="$(mktemp -d "${TMPDIR:-/tmp}/bstack-a11y-spm.XXXXXX")"
+
+      cat > "${synthetic_pkg_dir}/Package.swift" <<EOF
 // swift-tools-version: 5.9
 import PackageDescription
 
@@ -82,14 +93,24 @@ EOF
 
   setup
   if [[ -z "$EXTRA_ARGS" ]]; then
-    EXTRA_ARGS="--include **/*.swift --include **/*.xib --include **/*.storyboard"
+    EXTRA_ARGS="--include ${scan_root}/**/*.swift --include ${scan_root}/**/*.xib --include ${scan_root}/**/*.storyboard"
+  fi
+
+  # When we synthesized a manifest, run the plugin against that temp package
+  # directory (--package-path). The scan still targets the user's sources because
+  # the include globs are rooted at the original working directory. When the user
+  # already has a Package.swift, scan it in place exactly as before.
+  local package_path_args=()
+  if [ -n "$synthetic_pkg_dir" ]; then
+    package_path_args=(--package-path "$synthetic_pkg_dir")
   fi
+
   env -i HOME="$HOME" \
       XCODE_VERSION_ACTUAL="$XCODE_VERSION_ACTUAL"\
       BROWSERSTACK_USERNAME="$BROWSERSTACK_USERNAME"\
       BROWSERSTACK_ACCESS_KEY="$BROWSERSTACK_ACCESS_KEY"\
       PATH="$PATH" \
-      swift package plugin \
+      swift package "${package_path_args[@]}" plugin \
           --allow-writing-to-directory ~/.cache\
           --allow-writing-to-package-directory\
           --allow-network-connections 'all(ports: [])'\

scripts/zsh/spm.sh

@@ -51,12 +51,21 @@ EOF
 }
 
 a11y_scan() {
-  # Ensure Package.swift is removed on exit (acts like a finally block)
+  # Scan target is always the directory the user invoked us from.
+  local scan_root="${PWD}"
+
+  # Per-invocation staging directory for the synthetic manifest. Using a unique
+  # temp dir (instead of writing Package.swift into the user's CWD) prevents
+  # concurrent invocations from deleting each other's manifest on exit
+  # (DEVA11Y-483 / F-014). Only created when the user has no Package.swift.
+  local synthetic_pkg_dir=""
+
+  # Ensure the per-invocation temp dir is removed on exit (acts like a finally
+  # block). We only ever delete our own temp dir, never files in the user's CWD.
   cleanup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
+      if [ -n "$synthetic_pkg_dir" ] && [ -d "$synthetic_pkg_dir" ]; then
+          rm -rf -- "$synthetic_pkg_dir"
       fi
-      rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
   }
   trap cleanup EXIT
 
@@ -65,7 +74,9 @@ a11y_scan() {
           return
       fi
 
-      cat > Package.swift <<EOF
+      synthetic_pkg_dir="$(mktemp -d "${TMPDIR:-/tmp}/bstack-a11y-spm.XXXXXX")"
+
+      cat > "${synthetic_pkg_dir}/Package.swift" <<EOF
 // swift-tools-version: 5.9
 import PackageDescription
 
@@ -81,14 +92,24 @@ EOF
 
   setup
   if [[ -z "$EXTRA_ARGS" ]]; then
-    EXTRA_ARGS="--include **/*.swift --include **/*.xib --include **/*.storyboard"
+    EXTRA_ARGS="--include ${scan_root}/**/*.swift --include ${scan_root}/**/*.xib --include ${scan_root}/**/*.storyboard"
+  fi
+
+  # When we synthesized a manifest, run the plugin against that temp package
+  # directory (--package-path). The scan still targets the user's sources because
+  # the include globs are rooted at the original working directory. When the user
+  # already has a Package.swift, scan it in place exactly as before.
+  local package_path_args=()
+  if [ -n "$synthetic_pkg_dir" ]; then
+    package_path_args=(--package-path "$synthetic_pkg_dir")
   fi
+
   env -i HOME="$HOME" \
       XCODE_VERSION_ACTUAL="$XCODE_VERSION_ACTUAL"\
       BROWSERSTACK_USERNAME="$BROWSERSTACK_USERNAME"\
       BROWSERSTACK_ACCESS_KEY="$BROWSERSTACK_ACCESS_KEY"\
       PATH="$PATH" \
-      swift package plugin \
+      swift package "${package_path_args[@]}" plugin \
           --allow-writing-to-directory ~/.cache\
           --allow-writing-to-package-directory\
           --allow-network-connections 'all(ports: [])'\