From 14c137c5eaa07e1c860ec46c52713e0af9625fe4 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Wed, 26 Aug 2026 14:37:05 -0400 Subject: [PATCH 1/7] fix(dev): allow sandboxed npm to read the greptile tool store The Seatbelt profile denies all reads under $HOME and never re-allowed ~/.config/posthog/tools, while on-activate wraps the greptile CLI npm install in bin/dev-sandbox (default on macOS). npm lstats and reads its --prefix, so the install failed with EPERM on every mac dev with the sandbox on, and hogli review fell back to manual install guidance. Read-allow only the tools subpath plus metadata on the ~/.config and ~/.config/posthog dir nodes for path canonicalization, matching the existing ~/.local -> ~/.local/share/uv pattern. The rest of ~/.config (gcloud, gh, op) stays denied. Co-Authored-By: Claude Fable 5 --- bin/dev-sandbox.sb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/bin/dev-sandbox.sb b/bin/dev-sandbox.sb index ee3ce3b76a46..46516598ab85 100644 --- a/bin/dev-sandbox.sb +++ b/bin/dev-sandbox.sb @@ -68,6 +68,10 @@ (subpath (string-append (param "HOME") "/Library/Application Support/com.vercel.cli")) (subpath (string-append (param "HOME") "/Library/Application Support/turborepo")) (subpath (string-append (param "HOME") "/.duckdb")) ; duckdb extension cache (ducklake warmup installs/stats extensions here) + ;; machine-shared tool store. on-activate installs the greptile CLI here with + ;; sandboxed npm, and npm reads its --prefix as well as writing it. Only this + ;; subpath, so the rest of ~/.config (gcloud, gh, op) stays denied. + (subpath (string-append (param "HOME") "/.config/posthog/tools")) ;; git global config (read by version-detection / git-aware tooling). Config ;; only, by exact path — credentials (~/.git-credentials, keychain) stay blocked. (literal (string-append (param "HOME") "/.gitconfig")) @@ -79,10 +83,14 @@ ;; Canonicalization (realpath) lstats every path component, so uv resolving its ;; managed-interpreter symlinks needs metadata on the dir nodes between $HOME and -;; ~/.local/share/uv. Metadata only — contents of ~/.local{,/share} stay denied. +;; ~/.local/share/uv, and npm canonicalizing its --prefix needs the same between +;; $HOME and ~/.config/posthog/tools. Metadata only — contents of ~/.local{,/share} +;; and ~/.config{,/posthog} stay denied. (allow file-read-metadata (literal (string-append (param "HOME") "/.local")) - (literal (string-append (param "HOME") "/.local/share"))) + (literal (string-append (param "HOME") "/.local/share")) + (literal (string-append (param "HOME") "/.config")) + (literal (string-append (param "HOME") "/.config/posthog"))) ;; Keep the crates.io publish token blocked even though ~/.cargo is allowed above. (deny file-read* From e231f09fa536f9320f651cd18633ffc58874e436 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Wed, 26 Aug 2026 15:49:47 -0400 Subject: [PATCH 2/7] fix(dev): write-deny the greptile store and publish installs from staging The tool store is an execute-later target: activation links its binary into the venv and hogli review runs it in the developer's shell, outside Seatbelt. The profile allowed sandboxed writes there, so a compromised dependency in a sandboxed dev service could replace the binary and run with the developer's full account on the next review. Deny sandboxed writes to ~/.config/posthog/tools, and to the ~/.config and ~/.config/posthog nodes so a rename cannot swap in a symlinked ancestor. Drop the store read allowance: npm now installs into a sandbox-writable staging dir and the unsandboxed activation shell publishes node_modules into the store. The self-test asserts the store and its ancestor nodes reject sandboxed writes and that entries inside ~/.config stay writable. Co-Authored-By: Claude Fable 5 --- .flox/env/on-activate.sh | 19 ++++++++++++++++--- bin/dev-sandbox-selftest | 23 +++++++++++++++++++++++ bin/dev-sandbox.sb | 26 +++++++++++++++----------- 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/.flox/env/on-activate.sh b/.flox/env/on-activate.sh index baf9e18b18c6..7a968b6eea35 100755 --- a/.flox/env/on-activate.sh +++ b/.flox/env/on-activate.sh @@ -374,6 +374,14 @@ fi # the venv bin (Step 2b), so worktrees on different branches resolve their # own pin. A failed install must not break activation; the CLI is only needed # at PR-open time and `hogli review` prints install guidance when absent. +# +# The store is an execute-later target: Step 2b links the binary into the venv +# and `hogli review` runs it in the developer's shell, outside the sandbox. The +# sandbox therefore write-denies the store (see bin/dev-sandbox.sb), so npm +# installs into a sandbox-writable staging dir and this unsandboxed shell +# publishes the result. Do not point npm at the store directly: sandboxed, the +# write is denied, and unsandboxed it would leave the store writable to any +# compromised dependency the sandbox runs later. _GREPTILE_VERSION="3.4.1" _GREPTILE_STORE="$HOME/.config/posthog/tools/greptile/$_GREPTILE_VERSION" _GREPTILE_BIN="$_GREPTILE_STORE/node_modules/.bin/greptile" @@ -388,13 +396,18 @@ _install_greptile() { # activations (fresh worktrees) installing the same version. flock 9 || exit 1 if [[ ! -x "$_GREPTILE_BIN" || ! -f "$_GREPTILE_STAMP" ]]; then + _staging=$(mktemp -d) || exit 1 + trap 'rm -rf "$_staging"' EXIT if [[ "$_DEV_SANDBOX_INSTALLS" -eq 1 ]]; then # printf %q: dev-sandbox re-parses its command string, so the path - # must survive a $HOME with spaces or quotes. - "$FLOX_ENV_PROJECT/bin/dev-sandbox" "npm install --prefix $(printf '%q' "$_GREPTILE_STORE") --no-fund --no-audit greptile@$_GREPTILE_VERSION" || exit 1 + # must survive a TMPDIR with spaces or quotes. + "$FLOX_ENV_PROJECT/bin/dev-sandbox" "npm install --prefix $(printf '%q' "$_staging") --no-fund --no-audit greptile@$_GREPTILE_VERSION" || exit 1 else - npm install --prefix "$_GREPTILE_STORE" --no-fund --no-audit "greptile@$_GREPTILE_VERSION" || exit 1 + npm install --prefix "$_staging" --no-fund --no-audit "greptile@$_GREPTILE_VERSION" || exit 1 fi + [[ -x "$_staging/node_modules/.bin/greptile" ]] || exit 1 + rm -rf "$_GREPTILE_STORE/node_modules" || exit 1 + mv "$_staging/node_modules" "$_GREPTILE_STORE/node_modules" || exit 1 [[ -x "$_GREPTILE_BIN" ]] || exit 1 touch "$_GREPTILE_STAMP" || exit 1 fi diff --git a/bin/dev-sandbox-selftest b/bin/dev-sandbox-selftest index 0d4a60883201..51a476b9d071 100755 --- a/bin/dev-sandbox-selftest +++ b/bin/dev-sandbox-selftest @@ -187,6 +187,29 @@ check_write_blocked ".git/hooks/.dev_sandbox_selftest_probe" ".git/hooks" if [[ -d "$HOME/.cargo/bin" ]]; then check_write_blocked "$HOME/.cargo/bin/.dev_sandbox_selftest_probe" "cargo bin" fi +# The machine-shared tool store: flox activation publishes the greptile CLI here +# and hogli review executes it in the developer's shell, outside Seatbelt — the +# same execute-later route as cargo bin above. Creating the dir is safe: this is +# the store's real location, and activation creates it the same way. +mkdir -p "$HOME/.config/posthog/tools" 2>/dev/null +if [[ -d "$HOME/.config/posthog/tools" ]]; then + check_write_blocked "$HOME/.config/posthog/tools/.dev_sandbox_selftest_probe" "machine tool store (greptile runs unsandboxed)" + # A rename could swap in a symlinked ancestor and move the store out from + # under the subpath deny. chmod needs the same file-write* verdict as rename + # without touching the real directories, and the bits it sets are already set. + $SANDBOX "chmod u+rwx '$HOME/.config/posthog'" >/dev/null 2>&1 + report_write_blocked $? "tool store parent node (\$HOME/.config/posthog rename swap)" + $SANDBOX "chmod u+rwx '$HOME/.config'" >/dev/null 2>&1 + report_write_blocked $? "\$HOME/.config node (rename swap)" + # Positive control: the ancestor denies are literals, not subpaths — entries + # inside ~/.config must stay writable or sandboxed tools lose their config dirs. + if $SANDBOX "echo x > '$HOME/.config/.dev_sandbox_selftest_probe'" >/dev/null 2>&1; then + pass "write allowed: entry inside ~/.config (the deny is the node, not a subpath)" + rm -f "$HOME/.config/.dev_sandbox_selftest_probe" + else + die "write blocked, should be allowed: entry inside ~/.config" + fi +fi # Homebrew's bin/sbin/Cellar: developer-owned and on PATH, same execute-later route # as cargo bin above — no git-config indirection needed to reach it. for hb_bin in /opt/homebrew/bin /usr/local/bin; do diff --git a/bin/dev-sandbox.sb b/bin/dev-sandbox.sb index 2dd5c4e08955..a459a294838d 100644 --- a/bin/dev-sandbox.sb +++ b/bin/dev-sandbox.sb @@ -68,10 +68,6 @@ (subpath (string-append (param "HOME") "/Library/Application Support/com.vercel.cli")) (subpath (string-append (param "HOME") "/Library/Application Support/turborepo")) (subpath (string-append (param "HOME") "/.duckdb")) ; duckdb extension cache (ducklake warmup installs/stats extensions here) - ;; machine-shared tool store. on-activate installs the greptile CLI here with - ;; sandboxed npm, and npm reads its --prefix as well as writing it. Only this - ;; subpath, so the rest of ~/.config (gcloud, gh, op) stays denied. - (subpath (string-append (param "HOME") "/.config/posthog/tools")) ;; git global config (read by version-detection / git-aware tooling). Config ;; only, by exact path — credentials (~/.git-credentials, keychain) stay blocked. (literal (string-append (param "HOME") "/.gitconfig")) @@ -83,14 +79,10 @@ ;; Canonicalization (realpath) lstats every path component, so uv resolving its ;; managed-interpreter symlinks needs metadata on the dir nodes between $HOME and -;; ~/.local/share/uv, and npm canonicalizing its --prefix needs the same between -;; $HOME and ~/.config/posthog/tools. Metadata only — contents of ~/.local{,/share} -;; and ~/.config{,/posthog} stay denied. +;; ~/.local/share/uv. Metadata only — contents of ~/.local{,/share} stay denied. (allow file-read-metadata (literal (string-append (param "HOME") "/.local")) - (literal (string-append (param "HOME") "/.local/share")) - (literal (string-append (param "HOME") "/.config")) - (literal (string-append (param "HOME") "/.config/posthog"))) + (literal (string-append (param "HOME") "/.local/share"))) ;; Keep the crates.io publish token blocked even though ~/.cargo is allowed above. (deny file-read* @@ -191,7 +183,19 @@ (subpath "/usr/local/sbin") (subpath "/usr/local/Cellar") (subpath "/usr/local/lib") - (subpath (string-append (param "HOME") "/.cargo/bin"))) ; binaries on PATH + (subpath (string-append (param "HOME") "/.cargo/bin")) ; binaries on PATH + ;; The machine-shared tool store. on-activate publishes the greptile CLI here + ;; (npm installs into a sandbox-writable staging dir; the unsandboxed activation + ;; shell moves the result in), links it into the venv bin, and `hogli review` + ;; executes it in the developer's shell — the same execute-later route as + ;; cargo bin above. + (subpath (string-append (param "HOME") "/.config/posthog/tools")) + ;; …and the ancestor nodes, so a rename cannot swap in a symlinked ~/.config or + ;; ~/.config/posthog and move the store out from under the subpath deny — the + ;; same node-swap route as the /opt/homebrew/etc denies above. Entries inside + ;; ~/.config are their own paths and stay writable. + (literal (string-append (param "HOME") "/.config/posthog")) + (literal (string-append (param "HOME") "/.config"))) ;; husky's installer writes .husky/_/ (a gitignored husky.sh plus a .gitignore) on ;; every sandboxed `pnpm install`, and it throws when that write fails, which would From 66d7c43e9b380d256e36de3c4ed85457f74da8a2 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Wed, 26 Aug 2026 16:17:44 -0400 Subject: [PATCH 3/7] fix(dev): install the greptile store unsandboxed instead of via staging The staging dir lived under TMPDIR, which every sandboxed dev service can write. A compromised dependency running concurrently with the install could swap the staged binary before the activation shell published it, reopening the execute-later escape the staging flow was meant to close. Drop the sandboxed npm run entirely: the trusted activation shell installs into the write-denied store with --ignore-scripts, so no package code runs at install time and the CLI runs unsandboxed at review time anyway. Every input npm reads is pinned away from sandboxed writers: the cache lives inside the store, the user config is disabled and the profile now write-denies ~/.npmrc (the ~/.gitconfig escape with npm as the trusted tool), and cwd plus a fresh package.json keep the project-config lookup inside the store. The self-test asserts the ~/.npmrc deny. Co-Authored-By: Claude Fable 5 --- .flox/env/on-activate.sh | 39 ++++++++++++++++++++++----------------- bin/dev-sandbox-selftest | 8 ++++++++ bin/dev-sandbox.sb | 14 +++++++++----- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/.flox/env/on-activate.sh b/.flox/env/on-activate.sh index 7a968b6eea35..834b91eb8ec2 100755 --- a/.flox/env/on-activate.sh +++ b/.flox/env/on-activate.sh @@ -377,15 +377,23 @@ fi # # The store is an execute-later target: Step 2b links the binary into the venv # and `hogli review` runs it in the developer's shell, outside the sandbox. The -# sandbox therefore write-denies the store (see bin/dev-sandbox.sb), so npm -# installs into a sandbox-writable staging dir and this unsandboxed shell -# publishes the result. Do not point npm at the store directly: sandboxed, the -# write is denied, and unsandboxed it would leave the store writable to any -# compromised dependency the sandbox runs later. +# sandbox therefore write-denies the store (see bin/dev-sandbox.sb) and this +# trusted, unsandboxed shell performs the install itself. Skipping the sandbox +# is safe here: --ignore-scripts keeps package code from running at install +# time (npm only downloads, integrity-checks, and extracts), and the CLI runs +# unsandboxed at review time anyway. Do not sandbox the npm run instead: any +# sandbox-writable path it installs through (a TMPDIR staging dir, the shared +# ~/.npm cache) gives a concurrently sandboxed dependency a window to swap the +# binary before it is published. Every input this npm run reads is pinned away +# from sandboxed writers: the cache lives inside the write-denied store, the +# user config is disabled (and the sandbox write-denies ~/.npmrc anyway), and +# cwd plus a fresh package.json keep the project-config lookup inside the +# store instead of walking up to $HOME. _GREPTILE_VERSION="3.4.1" _GREPTILE_STORE="$HOME/.config/posthog/tools/greptile/$_GREPTILE_VERSION" _GREPTILE_BIN="$_GREPTILE_STORE/node_modules/.bin/greptile" _GREPTILE_STAMP="$_GREPTILE_STORE/.complete" +_GREPTILE_CACHE="$_GREPTILE_STORE/.npm-cache" _install_greptile() { # Explicit `|| return`/`|| exit`: callers suppress errexit, so a failed @@ -396,18 +404,15 @@ _install_greptile() { # activations (fresh worktrees) installing the same version. flock 9 || exit 1 if [[ ! -x "$_GREPTILE_BIN" || ! -f "$_GREPTILE_STAMP" ]]; then - _staging=$(mktemp -d) || exit 1 - trap 'rm -rf "$_staging"' EXIT - if [[ "$_DEV_SANDBOX_INSTALLS" -eq 1 ]]; then - # printf %q: dev-sandbox re-parses its command string, so the path - # must survive a TMPDIR with spaces or quotes. - "$FLOX_ENV_PROJECT/bin/dev-sandbox" "npm install --prefix $(printf '%q' "$_staging") --no-fund --no-audit greptile@$_GREPTILE_VERSION" || exit 1 - else - npm install --prefix "$_staging" --no-fund --no-audit "greptile@$_GREPTILE_VERSION" || exit 1 - fi - [[ -x "$_staging/node_modules/.bin/greptile" ]] || exit 1 - rm -rf "$_GREPTILE_STORE/node_modules" || exit 1 - mv "$_staging/node_modules" "$_GREPTILE_STORE/node_modules" || exit 1 + cd "$_GREPTILE_STORE" || exit 1 + # Rebuild from a clean slate: npm trusts an existing node_modules tree, + # so leftovers from an interrupted install (or from before the store was + # write-denied) must not survive into the published result. + rm -rf node_modules package-lock.json "$_GREPTILE_CACHE" || exit 1 + echo '{}' > package.json || exit 1 + NPM_CONFIG_USERCONFIG=/dev/null npm install --cache "$_GREPTILE_CACHE" \ + --ignore-scripts --no-fund --no-audit "greptile@$_GREPTILE_VERSION" || exit 1 + rm -rf "$_GREPTILE_CACHE" [[ -x "$_GREPTILE_BIN" ]] || exit 1 touch "$_GREPTILE_STAMP" || exit 1 fi diff --git a/bin/dev-sandbox-selftest b/bin/dev-sandbox-selftest index 51a476b9d071..3f24c4130473 100755 --- a/bin/dev-sandbox-selftest +++ b/bin/dev-sandbox-selftest @@ -236,6 +236,14 @@ if [[ -n "$gitcommon" && -f "$gitcommon/config" ]]; then else echo " -- repo .git/config not found, skipping" fi +# npm user config: registry/proxy/cafile keys redirect where an unsandboxed npm +# (the activation's tool-store install) downloads code from — the ~/.gitconfig +# escape with npm as the trusted tool. +if [[ -f "$HOME/.npmrc" ]]; then + check_write_blocked_existing "$HOME/.npmrc" "npm user config (registry/proxy redirect)" +else + check_write_blocked "$HOME/.npmrc" "npm user config (registry/proxy redirect)" +fi # The scripts that actually run outside the sandbox. core.hooksPath points git at # .husky, so the .git/hooks deny above guards a directory git never reads once that # is set; these are the real hooks. .flox/env holds the activation scripts, and diff --git a/bin/dev-sandbox.sb b/bin/dev-sandbox.sb index a459a294838d..bbee17fc0713 100644 --- a/bin/dev-sandbox.sb +++ b/bin/dev-sandbox.sb @@ -156,6 +156,11 @@ (literal (string-append (param "HOME") "/.gitconfig")) ; core.hooksPath / credential.helper = !cmd (literal (string-append (param "HOME") "/.gitconfig.local")) ; included by ~/.gitconfig — same escape if writable (subpath (string-append (param "HOME") "/.config/git")) ; XDG git config — same hooksPath/helper escape + ;; npm user config. Its registry, proxy, and cafile keys redirect where an + ;; unsandboxed npm (such as the activation's tool-store install above) + ;; downloads code from, which is the ~/.gitconfig escape with npm as the + ;; trusted tool. Reads were already denied with the rest of $HOME. + (literal (string-append (param "HOME") "/.npmrc")) ;; Homebrew's system-scope gitconfig, read by a brew-installed git on every ;; invocation and owned by the developer's account — unlike the Xcode CLT one, ;; which is root-owned and so already out of reach. Same hooksPath/fsmonitor @@ -184,11 +189,10 @@ (subpath "/usr/local/Cellar") (subpath "/usr/local/lib") (subpath (string-append (param "HOME") "/.cargo/bin")) ; binaries on PATH - ;; The machine-shared tool store. on-activate publishes the greptile CLI here - ;; (npm installs into a sandbox-writable staging dir; the unsandboxed activation - ;; shell moves the result in), links it into the venv bin, and `hogli review` - ;; executes it in the developer's shell — the same execute-later route as - ;; cargo bin above. + ;; The machine-shared tool store. The unsandboxed flox activation installs the + ;; greptile CLI here (npm --ignore-scripts, so no package code runs), links it + ;; into the venv bin, and `hogli review` executes it in the developer's shell — + ;; the same execute-later route as cargo bin above. (subpath (string-append (param "HOME") "/.config/posthog/tools")) ;; …and the ancestor nodes, so a rename cannot swap in a symlinked ~/.config or ;; ~/.config/posthog and move the store out from under the subpath deny — the From dce5c5c60a80411ede1a3daeab8d18ddf5d02923 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Wed, 26 Aug 2026 16:27:10 -0400 Subject: [PATCH 4/7] fix(dev): resolve npm and flock into the Nix store for the greptile install The unsandboxed install resolved npm from a PATH that activation prepends with the venv bin, and $FLOX_ENV/bin reaches binaries through the repo's .flox/run symlink. Both are sandbox-writable, so a planted shim would run outside the sandbox with the developer's account. Resolve npm and flock with /usr/bin/readlink -f, refuse any result outside /nix/store, and pin PATH to the system dirs for the rest of the install. The resolved npm's shebang points at the store node, so node is never looked up via PATH. Co-Authored-By: Claude Fable 5 --- .flox/env/on-activate.sh | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/.flox/env/on-activate.sh b/.flox/env/on-activate.sh index 834b91eb8ec2..4de47104e6a7 100755 --- a/.flox/env/on-activate.sh +++ b/.flox/env/on-activate.sh @@ -386,9 +386,11 @@ fi # ~/.npm cache) gives a concurrently sandboxed dependency a window to swap the # binary before it is published. Every input this npm run reads is pinned away # from sandboxed writers: the cache lives inside the write-denied store, the -# user config is disabled (and the sandbox write-denies ~/.npmrc anyway), and -# cwd plus a fresh package.json keep the project-config lookup inside the -# store instead of walking up to $HOME. +# user config is disabled (and the sandbox write-denies ~/.npmrc anyway), cwd +# plus a fresh package.json keep the project-config lookup inside the store +# instead of walking up to $HOME, and npm and flock themselves are resolved +# into the immutable Nix store rather than taken from a PATH that contains +# sandbox-writable dirs. _GREPTILE_VERSION="3.4.1" _GREPTILE_STORE="$HOME/.config/posthog/tools/greptile/$_GREPTILE_VERSION" _GREPTILE_BIN="$_GREPTILE_STORE/node_modules/.bin/greptile" @@ -398,11 +400,24 @@ _GREPTILE_CACHE="$_GREPTILE_STORE/.npm-cache" _install_greptile() { # Explicit `|| return`/`|| exit`: callers suppress errexit, so a failed # install would otherwise fall through and stamp the broken state. + # + # PATH here contains sandbox-writable dirs: the venv bin prepended above, and + # $FLOX_ENV/bin, which the repo's .flox/run symlink can be repointed at. A + # planted npm or flock shim there would run outside the sandbox. Resolve both + # through /usr/bin/readlink (a system binary, not PATH-resolved) and refuse + # anything that lands outside the immutable Nix store. The resolved npm's + # shebang is Nix-patched to the store node, so it does not look up node via + # PATH; the coreutils below come from the pinned system dirs. + local npm_bin flock_bin + npm_bin="$(/usr/bin/readlink -f "$(command -v npm)" 2>/dev/null)" || return 1 + flock_bin="$(/usr/bin/readlink -f "$(command -v flock)" 2>/dev/null)" || return 1 + [[ "$npm_bin" == /nix/store/* && "$flock_bin" == /nix/store/* ]] || return 1 + local PATH="/usr/bin:/bin" mkdir -p "$_GREPTILE_STORE" || return 1 ( # The store is shared across checkouts, so serialize concurrent # activations (fresh worktrees) installing the same version. - flock 9 || exit 1 + "$flock_bin" 9 || exit 1 if [[ ! -x "$_GREPTILE_BIN" || ! -f "$_GREPTILE_STAMP" ]]; then cd "$_GREPTILE_STORE" || exit 1 # Rebuild from a clean slate: npm trusts an existing node_modules tree, @@ -410,7 +425,7 @@ _install_greptile() { # write-denied) must not survive into the published result. rm -rf node_modules package-lock.json "$_GREPTILE_CACHE" || exit 1 echo '{}' > package.json || exit 1 - NPM_CONFIG_USERCONFIG=/dev/null npm install --cache "$_GREPTILE_CACHE" \ + NPM_CONFIG_USERCONFIG=/dev/null "$npm_bin" install --cache "$_GREPTILE_CACHE" \ --ignore-scripts --no-fund --no-audit "greptile@$_GREPTILE_VERSION" || exit 1 rm -rf "$_GREPTILE_CACHE" [[ -x "$_GREPTILE_BIN" ]] || exit 1 From 98f3ea802cd231e03a7c766885844e719c910713 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Thu, 27 Aug 2026 08:03:51 -0400 Subject: [PATCH 5/7] fix(dev): refuse a greptile binary outside write-denied install locations Co-Authored-By: Claude Fable 5 --- bin/dev-sandbox.sb | 7 ++- tools/hogli-commands/hogli_commands/review.py | 45 +++++++++++++++++++ .../hogli_commands/tests/test_review.py | 29 ++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/bin/dev-sandbox.sb b/bin/dev-sandbox.sb index bbee17fc0713..c2ec9bba720c 100644 --- a/bin/dev-sandbox.sb +++ b/bin/dev-sandbox.sb @@ -192,7 +192,12 @@ ;; The machine-shared tool store. The unsandboxed flox activation installs the ;; greptile CLI here (npm --ignore-scripts, so no package code runs), links it ;; into the venv bin, and `hogli review` executes it in the developer's shell — - ;; the same execute-later route as cargo bin above. + ;; the same execute-later route as cargo bin above. The venv bin symlink itself + ;; cannot be denied here: sandboxed `uv sync` creates and populates the venv, so + ;; a sandboxed dependency can replace that link. `hogli review` therefore refuses + ;; a greptile that resolves outside the write-denied locations (see + ;; _TRUSTED_INSTALL_ROOTS in tools/hogli-commands/hogli_commands/review.py); + ;; this deny is what makes the store binary it does run trustworthy. (subpath (string-append (param "HOME") "/.config/posthog/tools")) ;; …and the ancestor nodes, so a rename cannot swap in a symlinked ~/.config or ;; ~/.config/posthog and move the store out from under the subpath deny — the diff --git a/tools/hogli-commands/hogli_commands/review.py b/tools/hogli-commands/hogli_commands/review.py index 4dfcbe81fde3..192bf4c8aefe 100644 --- a/tools/hogli-commands/hogli_commands/review.py +++ b/tools/hogli-commands/hogli_commands/review.py @@ -30,8 +30,10 @@ from __future__ import annotations +import sys import shutil import subprocess +from pathlib import Path import click from hogli.manifest import REPO_ROOT @@ -44,6 +46,39 @@ "Re-enter the flox environment (activation installs it), " "or install it with `brew install greptileai/tap/greptile` or `npm install -g greptile`, then re-run." ) + +# PATH in an activated shell starts with the venv bin, which the dev sandbox +# can write to (the sandboxed `uv sync` populates it), so a compromised +# dependency could plant a `greptile` there that would run unsandboxed with +# the developer's account. Only execute a binary that resolves into a location +# bin/dev-sandbox.sb write-denies: the PostHog tool store, the Homebrew +# prefixes, or the immutable Nix store. Keep this list in sync with the +# `deny file-write*` block in bin/dev-sandbox.sb. +_TRUSTED_INSTALL_ROOTS: tuple[Path, ...] = ( + Path.home() / ".config" / "posthog" / "tools", + Path("/opt/homebrew/bin"), + Path("/opt/homebrew/sbin"), + Path("/opt/homebrew/Cellar"), + Path("/opt/homebrew/lib"), + Path("/usr/local/bin"), + Path("/usr/local/sbin"), + Path("/usr/local/Cellar"), + Path("/usr/local/lib"), + Path("/nix/store"), +) + + +def _untrusted_location(binary: str) -> Path | None: + # The sandbox only exists on macOS; elsewhere the check would just break + # legitimate installs (an npm prefix under $HOME) for no protection. + if sys.platform != "darwin": + return None + resolved = Path(binary).resolve() + if any(resolved.is_relative_to(root) for root in _TRUSTED_INSTALL_ROOTS): + return None + return resolved + + _SIGNIN_HINT = ( "Run `greptile login`, or set GREPTILE_API_KEY in .env.local (see .env.local.example). " "No access? The reviewing-before-pr skill has a harness-review fallback." @@ -112,6 +147,16 @@ def run(branch: str | None, instructions: str | None, force: bool, do_check: boo if binary is None: click.secho(f"Greptile CLI not found. {_INSTALL_HINT}", fg="red", err=True) return 1 + untrusted = _untrusted_location(binary) + if untrusted is not None: + click.secho( + f"Not running greptile from {untrusted}: only write-protected install locations are trusted " + "(the PostHog tool store, Homebrew, or the Nix store). " + "Re-enter the flox environment to reinstall it, or use `brew install greptileai/tap/greptile`, then re-run.", + fg="red", + err=True, + ) + return 1 if not _signed_in(binary): click.secho(f"Not signed in to Greptile. {_SIGNIN_HINT}", fg="yellow", err=True) return EX_CONFIG diff --git a/tools/hogli-commands/hogli_commands/tests/test_review.py b/tools/hogli-commands/hogli_commands/tests/test_review.py index ad41082a2479..760742ef7a67 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_review.py +++ b/tools/hogli-commands/hogli_commands/tests/test_review.py @@ -2,6 +2,7 @@ import subprocess from collections.abc import Callable, Iterator +from pathlib import Path import pytest from unittest.mock import MagicMock, patch @@ -62,6 +63,34 @@ def test_missing_cli_names_the_install_command(self, greptile_on_path: MagicMock assert result.exit_code == 1 assert "brew install greptileai/tap/greptile" in result.output + @pytest.mark.parametrize( + "platform,which_path,expected_exit", + [ + ("darwin", str(Path.home() / ".config/posthog/tools/greptile/3.4.1/node_modules/.bin/greptile"), 0), + ("darwin", "/usr/local/bin/greptile", 0), + ("darwin", "/Users/dev/posthog/.flox/cache/venv/bin/greptile", 1), + ("linux", "/home/dev/.npm-global/bin/greptile", 0), + ], + ) + @patch("hogli_commands.review.subprocess.run") + def test_darwin_runs_greptile_only_from_write_denied_locations( + self, + mock_run: MagicMock, + greptile_on_path: MagicMock, + platform: str, + which_path: str, + expected_exit: int, + ) -> None: + greptile_on_path.return_value = which_path + mock_run.side_effect = _fake_greptile(status=0) + with patch("hogli_commands.review.sys.platform", platform): + result = runner.invoke(cli, ["review"]) + assert result.exit_code == expected_exit + if expected_exit == 1: + # A planted binary must never execute, not even as an auth probe. + assert mock_run.call_args_list == [] + assert "flox" in result.output + @patch("hogli_commands.review.subprocess.run") def test_signed_out_exits_ex_config_without_starting_a_review(self, mock_run: MagicMock) -> None: mock_run.return_value = _proc(1, stderr="error: not signed in. Set GREPTILE_API_KEY or run `greptile login`.") From 5ec8f169fb7c3144287119c59aec040804cab837 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Thu, 27 Aug 2026 08:25:18 -0400 Subject: [PATCH 6/7] fix(dev): distrust pre-deny greptile stores and pin the review exec path Co-Authored-By: Claude Fable 5 --- .flox/env/on-activate.sh | 19 +++- tools/hogli-commands/hogli_commands/review.py | 97 +++++++++++++------ .../hogli_commands/tests/test_review.py | 48 +++++++++ 3 files changed, 128 insertions(+), 36 deletions(-) diff --git a/.flox/env/on-activate.sh b/.flox/env/on-activate.sh index 4de47104e6a7..47b62da7bd78 100755 --- a/.flox/env/on-activate.sh +++ b/.flox/env/on-activate.sh @@ -394,7 +394,11 @@ fi _GREPTILE_VERSION="3.4.1" _GREPTILE_STORE="$HOME/.config/posthog/tools/greptile/$_GREPTILE_VERSION" _GREPTILE_BIN="$_GREPTILE_STORE/node_modules/.bin/greptile" -_GREPTILE_STAMP="$_GREPTILE_STORE/.complete" +# The stamp is not named `.complete`: stores stamped with that name predate the +# sandbox write-deny, so their contents can include files a sandboxed +# dependency planted. The new name makes every such store fail the stamp check +# and rebuild once from a clean slate under the deny. +_GREPTILE_STAMP="$_GREPTILE_STORE/.complete-v2" _GREPTILE_CACHE="$_GREPTILE_STORE/.npm-cache" _install_greptile() { @@ -414,6 +418,12 @@ _install_greptile() { [[ "$npm_bin" == /nix/store/* && "$flock_bin" == /nix/store/* ]] || return 1 local PATH="/usr/bin:/bin" mkdir -p "$_GREPTILE_STORE" || return 1 + # A lock file planted as a symlink while the store was still sandbox-writable + # would make the `9>` redirection below truncate the symlink's target with + # the developer's own account. The write-deny keeps a new one from appearing. + if [[ -L "$_GREPTILE_STORE/.install.lock" ]]; then + rm -f "$_GREPTILE_STORE/.install.lock" || return 1 + fi ( # The store is shared across checkouts, so serialize concurrent # activations (fresh worktrees) installing the same version. @@ -421,9 +431,10 @@ _install_greptile() { if [[ ! -x "$_GREPTILE_BIN" || ! -f "$_GREPTILE_STAMP" ]]; then cd "$_GREPTILE_STORE" || exit 1 # Rebuild from a clean slate: npm trusts an existing node_modules tree, - # so leftovers from an interrupted install (or from before the store was - # write-denied) must not survive into the published result. - rm -rf node_modules package-lock.json "$_GREPTILE_CACHE" || exit 1 + # and a store from before the write-deny can hold planted files, such as + # a package.json or .npmrc symlink the writes below would follow. Remove + # every entry except the lock this subshell holds open. + find . -mindepth 1 -maxdepth 1 ! -name .install.lock -exec rm -rf {} + || exit 1 echo '{}' > package.json || exit 1 NPM_CONFIG_USERCONFIG=/dev/null "$npm_bin" install --cache "$_GREPTILE_CACHE" \ --ignore-scripts --no-fund --no-audit "greptile@$_GREPTILE_VERSION" || exit 1 diff --git a/tools/hogli-commands/hogli_commands/review.py b/tools/hogli-commands/hogli_commands/review.py index 192bf4c8aefe..8cca43b8891a 100644 --- a/tools/hogli-commands/hogli_commands/review.py +++ b/tools/hogli-commands/hogli_commands/review.py @@ -30,6 +30,7 @@ from __future__ import annotations +import os import sys import shutil import subprocess @@ -67,16 +68,26 @@ Path("/nix/store"), ) +# greptile's shebang is `#!/usr/bin/env node`, and it spawns git, so its +# subprocesses resolve executables from PATH at execution time. Give them a +# PATH of write-denied and system dirs only, so a binary planted in the +# sandbox-writable venv bin (first on the activated shell's PATH) cannot run. +_TRUSTED_PATH_DIRS = ( + "/opt/homebrew/bin", + "/usr/local/bin", + "/usr/bin", + "/bin", + "/usr/sbin", + "/sbin", +) -def _untrusted_location(binary: str) -> Path | None: - # The sandbox only exists on macOS; elsewhere the check would just break - # legitimate installs (an npm prefix under $HOME) for no protection. - if sys.platform != "darwin": - return None - resolved = Path(binary).resolve() + +def _trusted_location(path: str) -> Path | None: + """The canonical path when it sits in a write-denied install location, else None.""" + resolved = Path(path).resolve() if any(resolved.is_relative_to(root) for root in _TRUSTED_INSTALL_ROOTS): - return None - return resolved + return resolved + return None _SIGNIN_HINT = ( @@ -99,35 +110,37 @@ def _untrusted_location(binary: str) -> Path | None: _CHECK_COMMIT_LIMIT = 20 -def _probe(cmd: list[str]) -> subprocess.CompletedProcess[str] | None: +def _probe(cmd: list[str], env: dict[str, str] | None) -> subprocess.CompletedProcess[str] | None: try: - return subprocess.run(cmd, capture_output=True, text=True, timeout=_PROBE_TIMEOUT_SECONDS) + return subprocess.run(cmd, capture_output=True, text=True, timeout=_PROBE_TIMEOUT_SECONDS, env=env) except (OSError, subprocess.SubprocessError): return None -def _signed_in(binary: str) -> bool: +def _signed_in(binary: str, env: dict[str, str] | None) -> bool: """False only on Greptile's explicit signed-out error; any other ``config`` failure falls through to the review call, which reports the real problem. The string match is forced: greptile exits 1 for signed-out and for ordinary failures alike.""" - result = _probe([binary, "config"]) + result = _probe([binary, "config"], env) return result is None or result.returncode == 0 or "not signed in" not in (result.stdout + result.stderr).lower() -def _branch_commits(base: str | None) -> list[str]: +def _branch_commits(base: str | None, env: dict[str, str] | None) -> list[str]: # Match change_detection's base convention: origin/master, then master for # clones without the remote ref. for ref in [base] if base is not None else ["origin/master", "master"]: - result = _probe(["git", "-C", str(REPO_ROOT), "rev-list", f"--max-count={_CHECK_COMMIT_LIMIT}", f"{ref}..HEAD"]) + result = _probe( + ["git", "-C", str(REPO_ROOT), "rev-list", f"--max-count={_CHECK_COMMIT_LIMIT}", f"{ref}..HEAD"], env + ) if result is not None and result.returncode == 0: return result.stdout.split() or ["HEAD"] return ["HEAD"] -def check(binary: str, base: str | None) -> int: - for commit in _branch_commits(base): - status = _probe([binary, "review", "status", "--commit", commit]) +def check(binary: str, base: str | None, env: dict[str, str] | None) -> int: + for commit in _branch_commits(base, env): + status = _probe([binary, "review", "status", "--commit", commit], env) if status is None: # A hung or broken probe would hang or break for every commit too. break @@ -147,24 +160,44 @@ def run(branch: str | None, instructions: str | None, force: bool, do_check: boo if binary is None: click.secho(f"Greptile CLI not found. {_INSTALL_HINT}", fg="red", err=True) return 1 - untrusted = _untrusted_location(binary) - if untrusted is not None: - click.secho( - f"Not running greptile from {untrusted}: only write-protected install locations are trusted " - "(the PostHog tool store, Homebrew, or the Nix store). " - "Re-enter the flox environment to reinstall it, or use `brew install greptileai/tap/greptile`, then re-run.", - fg="red", - err=True, - ) - return 1 - if not _signed_in(binary): + env: dict[str, str] | None = None + # The dev sandbox only exists on macOS; elsewhere these checks would just + # break legitimate installs (an npm prefix under $HOME) for no protection. + if sys.platform == "darwin": + trusted = _trusted_location(binary) + if trusted is None: + click.secho( + f"Not running greptile from {Path(binary).resolve()}: only write-protected install locations " + "are trusted (the PostHog tool store, Homebrew, or the Nix store). " + "Re-enter the flox environment to reinstall it, or use `brew install greptileai/tap/greptile`, " + "then re-run.", + fg="red", + err=True, + ) + return 1 + # Execute the canonical path, not the PATH entry which() found: that + # entry is usually the sandbox-writable venv symlink, which a sandboxed + # process could repoint between this validation and the exec. + binary = str(trusted) + node = shutil.which("node") + node_dir = None if node is None else _trusted_location(node) + if node_dir is None: + click.secho( + "node was not found in a write-protected location, and greptile runs on node. " + "Re-enter the flox environment, then re-run.", + fg="red", + err=True, + ) + return 1 + env = {**os.environ, "PATH": ":".join([str(node_dir.parent), *_TRUSTED_PATH_DIRS])} + if not _signed_in(binary, env): click.secho(f"Not signed in to Greptile. {_SIGNIN_HINT}", fg="yellow", err=True) return EX_CONFIG if do_check: - return check(binary, branch) + return check(binary, branch, env) if not force: - status = _probe([binary, "review", "status", "--commit", "HEAD"]) + status = _probe([binary, "review", "status", "--commit", "HEAD"], env) if status is not None and status.returncode == _STATUS_COMPLETED: click.secho( "HEAD already has a completed review. Showing it. Pass --force to start a new one.", @@ -175,7 +208,7 @@ def run(branch: str | None, instructions: str | None, force: bool, do_check: boo return 0 if status is not None and status.returncode == _STATUS_RUNNING: click.secho("A review for this branch is still running. Resuming it.", fg="cyan", err=True) - return subprocess.run([binary, "review", "--resume"]).returncode + return subprocess.run([binary, "review", "--resume"], env=env).returncode cmd = [binary, "review"] if branch is not None: @@ -183,7 +216,7 @@ def run(branch: str | None, instructions: str | None, force: bool, do_check: boo if instructions is not None: cmd += ["--instructions", instructions] # Inherit stdio so Greptile's own progress and interactive review view work. - return subprocess.run(cmd).returncode + return subprocess.run(cmd, env=env).returncode @click.command( diff --git a/tools/hogli-commands/hogli_commands/tests/test_review.py b/tools/hogli-commands/hogli_commands/tests/test_review.py index 760742ef7a67..64e6721705ee 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_review.py +++ b/tools/hogli-commands/hogli_commands/tests/test_review.py @@ -9,6 +9,7 @@ from click.testing import CliRunner from hogli.cli import cli +from hogli_commands import review runner = CliRunner() @@ -91,6 +92,53 @@ def test_darwin_runs_greptile_only_from_write_denied_locations( assert mock_run.call_args_list == [] assert "flox" in result.output + @patch("hogli_commands.review.subprocess.run") + def test_darwin_executes_the_resolved_path_not_the_symlink( + self, mock_run: MagicMock, greptile_on_path: MagicMock, tmp_path: Path + ) -> None: + link = tmp_path / "greptile" + link.symlink_to(_BINARY) + greptile_on_path.return_value = str(link) + mock_run.side_effect = _fake_greptile(status=0) + with patch("hogli_commands.review.sys.platform", "darwin"): + result = runner.invoke(cli, ["review"]) + assert result.exit_code == 0 + greptile_calls = [call.args[0] for call in mock_run.call_args_list if call.args[0][0] != "git"] + assert greptile_calls + # The writable symlink can be repointed after validation; only the + # canonical write-denied path may reach exec. + assert all(cmd[0] == _BINARY for cmd in greptile_calls) + + @pytest.mark.parametrize( + "node_path,expected_exit", + [ + ("/opt/homebrew/bin/node", 0), + ("/Users/dev/posthog/.flox/cache/venv/bin/node", 1), + ], + ) + @patch("hogli_commands.review.subprocess.run") + def test_darwin_pins_path_to_a_trusted_node( + self, + mock_run: MagicMock, + greptile_on_path: MagicMock, + node_path: str, + expected_exit: int, + ) -> None: + greptile_on_path.side_effect = lambda name: {"greptile": _BINARY, "node": node_path}[name] + mock_run.side_effect = _fake_greptile(status=0) + with patch("hogli_commands.review.sys.platform", "darwin"): + result = runner.invoke(cli, ["review"]) + assert result.exit_code == expected_exit + if expected_exit == 0: + assert mock_run.call_args_list + # greptile's shebang resolves node from PATH, so every subprocess + # must get the pinned PATH: the trusted node's dir, then only + # write-denied and system dirs. + pinned = ":".join([str(Path(node_path).resolve().parent), *review._TRUSTED_PATH_DIRS]) + assert all(call.kwargs["env"]["PATH"] == pinned for call in mock_run.call_args_list) + else: + assert mock_run.call_args_list == [] + @patch("hogli_commands.review.subprocess.run") def test_signed_out_exits_ex_config_without_starting_a_review(self, mock_run: MagicMock) -> None: mock_run.return_value = _proc(1, stderr="error: not signed in. Set GREPTILE_API_KEY or run `greptile login`.") From d0ef2ff1f887ce0594911395226acec245b5e79f Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Thu, 27 Aug 2026 08:36:00 -0400 Subject: [PATCH 7/7] fix(dev): keep the original greptile store stamp name Co-Authored-By: Claude Fable 5 --- .flox/env/on-activate.sh | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.flox/env/on-activate.sh b/.flox/env/on-activate.sh index 47b62da7bd78..6253f2c6bc86 100755 --- a/.flox/env/on-activate.sh +++ b/.flox/env/on-activate.sh @@ -394,11 +394,7 @@ fi _GREPTILE_VERSION="3.4.1" _GREPTILE_STORE="$HOME/.config/posthog/tools/greptile/$_GREPTILE_VERSION" _GREPTILE_BIN="$_GREPTILE_STORE/node_modules/.bin/greptile" -# The stamp is not named `.complete`: stores stamped with that name predate the -# sandbox write-deny, so their contents can include files a sandboxed -# dependency planted. The new name makes every such store fail the stamp check -# and rebuild once from a clean slate under the deny. -_GREPTILE_STAMP="$_GREPTILE_STORE/.complete-v2" +_GREPTILE_STAMP="$_GREPTILE_STORE/.complete" _GREPTILE_CACHE="$_GREPTILE_STORE/.npm-cache" _install_greptile() {