Merge pull request #131 from ausimian/fix/issue-125-build-cache-hardening

build: harden dev/CI source-build cache trust

Author: ausimian

MAINTAINING.md

@@ -35,6 +35,12 @@ The switch is driven by a `File.dir?("c_src")` check in mix.exs's
 `compilers/0` — the hex `package[:files]` list ships only `lib/` and
 the docs, so consumers land on the download path automatically.
 
+`EMILY_CACHE` must point at a **private, user-owned** directory: the
+source build statically links `$EMILY_CACHE/mlx-<v>-<variant>/lib/libmlx.a`
+into the NIF, so the build refuses to reuse a cache dir owned by another
+user (and keeps its own dirs `0700`) to stop a shared cache from planting
+native code. The per-user macOS/XDG defaults already satisfy this.
+
 Variant selection is unified via the `:variant` app-config key:
 in-repo builds read `EMILY_MLX_VARIANT` env var (`aot`|`jit`,
 default `aot`) through `config/config.exs` and stash the atom as

RELEASE.md

@@ -1,5 +1,14 @@
 ### Security
 
+- The dev/CI source-build path now refuses to trust an MLX install
+  directory it doesn't own and keeps the build cache `0700`, so a shared
+  or attacker-controlled `EMILY_CACHE` can't plant a `libmlx.a` that is
+  then statically linked into the NIF. Fixed system tools (`getconf`,
+  `id`, `sw_vers`, plus `xcrun`/`sysctl`/`ps` in `build-mlx.sh`) resolve
+  from absolute/system paths rather than `$PATH`, and the MLX-build lock
+  records the holder's process start time so a recycled PID can't be
+  mistaken for the original holder. Build-time only; no runtime change.
+
 - Precompiled NIF downloads are now verified against checksums pinned
   inside the hex package (`native_checksums.txt`) rather than a `.sha256`
   sidecar fetched from the same GitHub release as the tarball. Because

lib/mix/tasks/emily.doctor.ex

@@ -333,7 +333,7 @@ defmodule Mix.Tasks.Emily.Doctor do
   defp macos_version do
     case :os.type() do
       {:unix, :darwin} ->
-        case System.cmd("sw_vers", ["-productVersion"], stderr_to_stdout: true) do
+        case System.cmd("/usr/bin/sw_vers", ["-productVersion"], stderr_to_stdout: true) do
           {version, 0} -> String.trim(version)
           _ -> :unknown
         end

mix.exs

@@ -251,7 +251,7 @@ defmodule Emily.MixProject do
   defp default_cache_dir do
     case :os.type() do
       {:unix, :darwin} ->
-        {out, 0} = System.cmd("getconf", ["DARWIN_USER_CACHE_DIR"])
+        {out, 0} = System.cmd("/usr/bin/getconf", ["DARWIN_USER_CACHE_DIR"])
         Path.join(String.trim(out), "emily")
 
       _ ->
@@ -260,6 +260,45 @@ defmodule Emily.MixProject do
     end
   end
 
+  # ---------- Cache-dir trust (dev/CI source build) ----------
+
+  # The MLX install dir is statically linked into libemily, so a planted
+  # libmlx.a is arbitrary native code in the BEAM. Refuse to trust (or
+  # reuse) a cache/install dir owned by another user — the exposure is a
+  # shared EMILY_CACHE on a multi-user host — and keep our own dirs 0700.
+  defp current_uid do
+    {out, 0} = System.cmd("/usr/bin/id", ["-u"])
+    out |> String.trim() |> String.to_integer()
+  end
+
+  defp assert_owned!(dir) do
+    case File.stat(dir) do
+      {:ok, %File.Stat{uid: uid}} ->
+        unless uid == current_uid() do
+          Mix.raise("""
+          Refusing to trust #{dir}: it is owned by uid #{uid}, not you
+          (uid #{current_uid()}). A shared or attacker-controlled cache
+          could plant a malicious libmlx.a that is statically linked into
+          the NIF. Point EMILY_CACHE at a private, user-owned directory.
+          """)
+        end
+
+      {:error, :enoent} ->
+        :ok
+
+      {:error, reason} ->
+        Mix.raise("Cannot stat #{dir}: #{inspect(reason)}")
+    end
+  end
+
+  defp prepare_cache_dir! do
+    cache = cache_dir()
+    File.mkdir_p!(cache)
+    assert_owned!(cache)
+    File.chmod!(cache, 0o700)
+    cache
+  end
+
   defp mlx_variant do
     case Application.get_env(:emily, :variant, :aot) do
       :aot -> "aot"
@@ -292,7 +331,9 @@ defmodule Emily.MixProject do
 
   defp build_mlx(args) do
     _ = arch_tag()
+    assert_owned!(cache_dir())
     dir = mlx_install_dir()
+    assert_owned!(dir)
 
     if "--force" in args do
       File.rm_rf!(dir)
@@ -328,7 +369,7 @@ defmodule Emily.MixProject do
     script = Path.expand("scripts/build-mlx.sh", File.cwd!())
     jit_flag = if mlx_variant() == "jit", do: "1", else: "0"
 
-    File.mkdir_p!(cache_dir())
+    prepare_cache_dir!()
 
     Mix.shell().info("Building MLX #{@mlx_version} (#{mlx_variant()}) from source")
 

scripts/build-mlx.sh

@@ -11,6 +11,12 @@
 
 set -euo pipefail
 
+# Resolve the fixed macOS system tools (uname, xcrun, sysctl, ps, id, …)
+# from the real system bin dirs regardless of a poisoned inbound $PATH.
+# The original PATH is appended so user-installed build tools (cmake,
+# ninja) still resolve.
+export PATH="/usr/bin:/bin:/usr/sbin:/sbin:${PATH}"
+
 if [[ $# -ne 4 ]]; then
   echo "usage: $0 <mlx-src-dir> <mlx-version> <jit 0|1> <install-prefix>" >&2
   exit 2
@@ -70,8 +76,10 @@ fi
 #
 # `flock(1)` isn't shipped on macOS, so we use atomic `mkdir` as the
 # lock primitive. The lock dir is keyed on PREFIX, which both
-# contexts share. A PID file inside lets us reclaim a stale lock if
-# the previous holder died without cleanup.
+# contexts share. A token file (PID + process start time) inside lets
+# us reclaim a stale lock if the previous holder died without cleanup —
+# the start time guards against PID reuse, so a recycled PID belonging
+# to an unrelated live process is still treated as stale.
 BUILD_DIR="${PREFIX}.build"
 STAGING="${PREFIX}.staging"
 LOCK_DIR="${PREFIX}.lock"
@@ -101,24 +109,46 @@ cleanup() {
 }
 trap cleanup EXIT
 
+# Record the lock holder as "PID\n<process start time>". A recycled PID
+# (same number, different process) has a different start time, so the
+# stale-lock reclaim below can't mistake an unrelated live process for
+# the original holder — `kill -0` alone can't tell them apart.
+write_lock_token() {
+  { echo "$$"; ps -o lstart= -p "$$" 2>/dev/null; } > "$LOCK_PID_FILE"
+}
+
+# Is the recorded holder ($1=pid, $2=start time) still the live process
+# that took the lock?
+holder_is_live() {
+  local pid="$1" started="$2"
+  [[ -n "$pid" ]] || return 1
+  kill -0 "$pid" 2>/dev/null || return 1
+  local now
+  now=$(ps -o lstart= -p "$pid" 2>/dev/null || true)
+  [[ -n "$now" && "$now" == "$started" ]]
+}
+
 while :; do
   if mkdir "$LOCK_DIR" 2>/dev/null; then
-    echo "$$" > "$LOCK_PID_FILE"
+    write_lock_token
     acquired_lock=1
     break
   fi
 
-  holder=""
-  [[ -r "$LOCK_PID_FILE" ]] && holder=$(cat "$LOCK_PID_FILE" 2>/dev/null || true)
+  holder_pid=""
+  holder_started=""
+  if [[ -r "$LOCK_PID_FILE" ]]; then
+    { IFS= read -r holder_pid; IFS= read -r holder_started; } < "$LOCK_PID_FILE" 2>/dev/null || true
+  fi
 
-  if [[ -n "$holder" ]] && ! kill -0 "$holder" 2>/dev/null; then
-    echo "==> Reclaiming stale MLX-build lock (dead PID $holder)" >&2
+  if [[ -n "$holder_pid" ]] && ! holder_is_live "$holder_pid" "$holder_started"; then
+    echo "==> Reclaiming stale MLX-build lock (holder PID ${holder_pid} is gone)" >&2
     rm -rf "$LOCK_DIR"
     continue
   fi
 
   if (( printed_wait == 0 )); then
-    echo "==> Waiting for concurrent MLX build${holder:+ (PID $holder)} on ${PREFIX}" >&2
+    echo "==> Waiting for concurrent MLX build${holder_pid:+ (PID $holder_pid)} on ${PREFIX}" >&2
     printed_wait=1
   fi
   sleep 1