Write prewarm secret files under the data root, not Dir.tmpdir

lib/dev/build_container.rb

@@ -2,9 +2,9 @@
 # frozen_string_literal: true
 
 require "digest"
+require "fileutils"
 require "pathname"
 require "securerandom"
-require "tmpdir"
 require "yaml"
 
 require "dev/build_watcher"
@@ -38,6 +38,12 @@ class BuildContainer
     # remote engine brings its sync strategy instead of reaching this raise.
     class LocalMountsUnsupportedError < RuntimeError; end
 
+    # The per-uid secrets dir under the data root failed verification (symlink,
+    # non-directory, or owned by another user). Writing 0600 secret files into
+    # a dir someone else controls lets its owner swap contents between file
+    # write and container bind mount, so this is a hard stop, never a fallback.
+    class SecretDirCompromisedError < RuntimeError; end
+
     # Always-hashed inputs. deps.lock (app/test deps, e.g. SML) and build-deps.lock
     # (build deps, e.g. the engine) join the Dockerfile so a dependency bump
     # invalidates a prewarmed image. Missing files are skipped (see content_tag).
@@ -541,17 +547,74 @@ def run_watched(argv, container:)
     # Write each secret value to a private host temp file for bind-mounting into
     # the prewarm container. Returns {id => path}; caller deletes the files.
     #
+    # The files live under the data root, NOT Dir.tmpdir: on macOS + colima the
+    # VM shares $HOME and /Users/Shared but not /var/folders, and docker turns a
+    # bind mount from an unshared host path into an empty directory — the
+    # prewarm then reads an empty secret and fails far from the cause.
+    #
     # @param secrets [Hash{String => String}]
     # @return [Hash{String => String}] secret id => temp file path
+    # @raise [SecretDirCompromisedError] when the per-uid dir fails verification
     sig { params(secrets: T::Hash[String, String]).returns(T::Hash[String, String]) }
     def write_secret_files(secrets)
+      dir = secrets_dir
+      sweep_stale_secrets(dir)
       secrets.each_with_object({}) do |(id, value), files|
-        path = File.join(Dir.tmpdir, "dev-secret-#{SecureRandom.hex(8)}")
+        path = File.join(dir, "dev-secret-#{SecureRandom.hex(8)}")
         File.open(path, File::WRONLY | File::CREAT | File::EXCL, 0o600) { |f| f.write(value) }
         files[id] = path
       end
     end
 
+    # The per-identity secrets dir under the data root: secrets-<uid>, 0700,
+    # verified before use. Per-uid rather than a shared sticky-1777 dir because
+    # a shared dir's owner can unlink and replace anyone's files (sticky only
+    # stops non-owners) — a secret-substitution vector between file write and
+    # bind mount. Verification guards the unprovisioned-machine case: the data
+    # root's parent (/Users/Shared) ships world-writable, so any local user can
+    # pre-own the tree before provisioning; a pre-existing entry here is
+    # attacker-suspect until lstat proves it a real directory we own. The dir
+    # sits directly under the data root (not a shared tmp/) so no world-writable
+    # parent can rename a verified dir out from under us post-check.
+    #
+    # @return [String] verified dir path
+    # @raise [SecretDirCompromisedError]
+    sig { returns(String) }
+    def secrets_dir
+      dir = File.join(Dev::DataRoot.path, "secrets-#{Process.uid}")
+      begin
+        Dir.mkdir(dir, 0o700)
+      rescue Errno::EEXIST
+        # Pre-existing entry: verified below like everything else.
+      end
+      st = File.lstat(dir)
+      unless st.directory? && st.uid == Process.uid
+        raise SecretDirCompromisedError,
+          "#{dir} is not a directory owned by uid #{Process.uid} " \
+          "(found #{st.directory? ? "dir" : "non-dir"} owned by uid #{st.uid}). " \
+          "Refusing to write secrets there — remove it and re-run."
+      end
+      # Ours, but normalize the mode (a setgid data root propagates g+s on
+      # Linux; older dev versions never created this dir, so no legacy modes).
+      File.chmod(0o700, dir) if (st.mode & 0o7777) != 0o700
+      dir
+    end
+
+    # Delete day-old dev-secret-* leftovers. The caller's ensure covers normal
+    # failures, but SIGKILL strands 0600 files under the data root, which —
+    # unlike /var/folders — macOS never purges. A day's grace keeps concurrent
+    # runs' live files safe (they exist for minutes, not hours).
+    #
+    # @param dir [String] the verified per-uid secrets dir
+    sig { params(dir: String).void }
+    def sweep_stale_secrets(dir)
+      Dir.glob(File.join(dir, "dev-secret-*")).each do |path|
+        File.delete(path) if Time.now - File.mtime(path) > 86_400
+      rescue Errno::ENOENT
+        # A concurrent sweep won the race; the file is gone either way.
+      end
+    end
+
     # --- internal helpers ------------------------------------------------
 
     # Unique name for the throwaway prewarm container; pid + random suffix so

test/dev/build_container_test.rb

@@ -1058,6 +1058,11 @@ def build_container(engine: FakeContainerEngine.new)
   end
 
   test "write_secret_files writes each secret to a private temp file" do
+    Given "a scratch data root"
+    root = Dir.mktmpdir("bc-data-root-")
+    original = ENV["DEV_DATA_ROOT"]
+    ENV["DEV_DATA_ROOT"] = root
+
     When "writing secret files"
     files = build_container.write_secret_files({ "TOK" => "s3cr3t" })
 
@@ -1066,7 +1071,107 @@ def build_container(engine: FakeContainerEngine.new)
     (File.stat(files["TOK"]).mode & 0o777) == 0o600
 
     Cleanup
-    files.each_value { |p| File.delete(p) if File.exist?(p) }
+    ENV["DEV_DATA_ROOT"] = original
+    FileUtils.rm_rf(root)
+  end
+
+  test "write_secret_files places files in a private per-uid dir under the data root, never Dir.tmpdir" do
+    Given "a scratch data root"
+    # macOS + colima: the VM shares $HOME and /Users/Shared, NOT /var/folders
+    # (Dir.tmpdir). A bind mount from an unshared path silently mounts an empty
+    # directory, so the prewarm reads an empty secret and fails downstream.
+    root = Dir.mktmpdir("bc-data-root-")
+    original = ENV["DEV_DATA_ROOT"]
+    ENV["DEV_DATA_ROOT"] = root
+
+    When "writing secret files"
+    files = build_container.write_secret_files({ "TOK" => "s3cr3t" })
+
+    Then "each file lives in secrets-<uid>, a real dir owned by us, mode 0700"
+    dir = File.join(root, "secrets-#{Process.uid}")
+    files.values.all? { |p| File.dirname(p) == dir }
+    st = File.lstat(dir)
+    st.directory? == true
+    st.uid == Process.uid
+    (st.mode & 0o7777) == 0o700
+
+    Cleanup
+    ENV["DEV_DATA_ROOT"] = original
+    FileUtils.rm_rf(root)
+  end
+
+  test "write_secret_files refuses a symlinked secrets dir (planted redirect)" do
+    Given "an attacker-planted symlink where the per-uid dir belongs"
+    # The data root's parent (/Users/Shared) ships world-writable on macOS: on
+    # an unprovisioned machine any local user can pre-own the tree and plant a
+    # symlink so our 0600 files land in a directory they control.
+    root = Dir.mktmpdir("bc-data-root-")
+    original = ENV["DEV_DATA_ROOT"]
+    ENV["DEV_DATA_ROOT"] = root
+    elsewhere = File.join(root, "attacker-controlled")
+    FileUtils.mkdir_p(elsewhere)
+    File.symlink(elsewhere, File.join(root, "secrets-#{Process.uid}"))
+
+    When "writing secret files"
+    build_container.write_secret_files({ "TOK" => "s3cr3t" })
+
+    Then "the planted dir is refused, not used"
+    raises Dev::BuildContainer::SecretDirCompromisedError
+
+    Cleanup
+    ENV["DEV_DATA_ROOT"] = original
+    FileUtils.rm_rf(root)
+  end
+
+  test "write_secret_files refuses a secrets dir owned by another user" do
+    Given "a per-uid dir whose owner is not us"
+    root = Dir.mktmpdir("bc-data-root-")
+    original = ENV["DEV_DATA_ROOT"]
+    ENV["DEV_DATA_ROOT"] = root
+    dir = File.join(root, "secrets-#{Process.uid}")
+    FileUtils.mkdir_p(dir)
+    foreign = stub(directory?: true, uid: Process.uid + 1, mode: 0o40700)
+    File.stubs(:lstat).with(dir).returns(foreign)
+
+    When "writing secret files"
+    build_container.write_secret_files({ "TOK" => "s3cr3t" })
+
+    Then "the foreign dir is refused — its owner could swap files under us"
+    raises Dev::BuildContainer::SecretDirCompromisedError
+
+    Cleanup
+    File.unstub(:lstat)
+    ENV["DEV_DATA_ROOT"] = original
+    FileUtils.rm_rf(root)
+  end
+
+  test "write_secret_files sweeps stale secret files a killed run left behind" do
+    Given "a leftover secret file from a SIGKILLed run, and a fresh one"
+    # The ensure-block deletion covers normal failures, but SIGKILL leaves 0600
+    # files under the data root, which macOS never purges (unlike /var/folders).
+    root = Dir.mktmpdir("bc-data-root-")
+    original = ENV["DEV_DATA_ROOT"]
+    ENV["DEV_DATA_ROOT"] = root
+    dir = File.join(root, "secrets-#{Process.uid}")
+    FileUtils.mkdir_p(dir)
+    FileUtils.chmod(0o700, dir)
+    stale = File.join(dir, "dev-secret-stale")
+    fresh = File.join(dir, "dev-secret-fresh")
+    File.write(stale, "old")
+    File.write(fresh, "new")
+    File.utime(Time.now - 172_800, Time.now - 172_800, stale)
+
+    When "writing secret files"
+    files = build_container.write_secret_files({ "TOK" => "s3cr3t" })
+
+    Then "the day-old file is gone; the recent one (a concurrent run's) survives"
+    !File.exist?(stale)
+    File.exist?(fresh)
+    File.read(files["TOK"]) == "s3cr3t"
+
+    Cleanup
+    ENV["DEV_DATA_ROOT"] = original
+    FileUtils.rm_rf(root)
   end
 
   test "service_container_name keys the name by image, workspace, and tag" do