From 68fea71933895915918246906e47f1b254fbc4e9 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 15 Mar 2026 03:39:33 +0000
Subject: [PATCH] Address remaining PR review comments

- Pin cache-apt-pkgs-action to commit SHA for supply-chain safety
- Fix Homebrew post_install to use with_env block instead of env hash
  in system() call (idiomatic Homebrew pattern)
- Add clarifying comments to service file, preremove.sh, and nfpm.yaml
  explaining user/group creation, directory ownership, and upgrade handling

https://claude.ai/code/session_01Vx1EsNrNySgsc8Y67dGzCn
---
 .github/workflows/debian.yml    |  2 +-
 .github/workflows/homebrew.yml  | 10 +++++++---
 brew_dist/archivebox.rb         |  4 +++-
 pkg/debian/archivebox.service   |  3 +++
 pkg/debian/nfpm.yaml            |  2 +-
 pkg/debian/scripts/preremove.sh |  4 +++-
 6 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/.github/workflows/debian.yml b/.github/workflows/debian.yml
index def8cd8c..04979adf 100644
--- a/.github/workflows/debian.yml
+++ b/.github/workflows/debian.yml
@@ -90,7 +90,7 @@ jobs:
           enable-cache: true
 
       - name: Install build dependencies
-        uses: awalsh128/cache-apt-pkgs-action@v1.6.0
+        uses: awalsh128/cache-apt-pkgs-action@acb598e5ddbc6f68a970c5da0688d2f3a9f04d05 # v1.6.0
         with:
           packages: build-essential python3-dev python3-setuptools libssl-dev libldap2-dev libsasl2-dev zlib1g-dev libatomic1
           version: 1.0
diff --git a/.github/workflows/homebrew.yml b/.github/workflows/homebrew.yml
index ca475917..dccb6dda 100644
--- a/.github/workflows/homebrew.yml
+++ b/.github/workflows/homebrew.yml
@@ -48,7 +48,7 @@ jobs:
 
       - name: Install build dependencies (Linux)
         if: runner.os == 'Linux'
-        uses: awalsh128/cache-apt-pkgs-action@v1.6.0
+        uses: awalsh128/cache-apt-pkgs-action@acb598e5ddbc6f68a970c5da0688d2f3a9f04d05 # v1.6.0
         with:
           packages: build-essential python3-dev python3-setuptools libssl-dev libldap2-dev libsasl2-dev zlib1g-dev libatomic1
           version: 1.0
@@ -108,7 +108,9 @@ ${RESOURCES}
 
   def post_install
     (var/"archivebox").mkpath
-    system({ "DATA_DIR" => var/"archivebox" }, bin/"archivebox", "install", "--binproviders", "pip,npm")
+    with_env(DATA_DIR: var/"archivebox") do
+      system bin/"archivebox", "install", "--binproviders", "pip,npm"
+    end
   end
 
   service do
@@ -275,7 +277,9 @@ ${RESOURCES}
 
   def post_install
     (var/"archivebox").mkpath
-    system({ "DATA_DIR" => var/"archivebox" }, bin/"archivebox", "install", "--binproviders", "pip,npm")
+    with_env(DATA_DIR: var/"archivebox") do
+      system bin/"archivebox", "install", "--binproviders", "pip,npm"
+    end
   end
 
   service do
diff --git a/brew_dist/archivebox.rb b/brew_dist/archivebox.rb
index c0521d7a..6f9d8ada 100644
--- a/brew_dist/archivebox.rb
+++ b/brew_dist/archivebox.rb
@@ -41,7 +41,9 @@ class Archivebox < Formula
   def post_install
     # Install runtime dependencies (plugins, JS extractors, etc.)
     (var/"archivebox").mkpath
-    system({ "DATA_DIR" => var/"archivebox" }, bin/"archivebox", "install", "--binproviders", "pip,npm")
+    with_env(DATA_DIR: var/"archivebox") do
+      system bin/"archivebox", "install", "--binproviders", "pip,npm"
+    end
   end
 
   service do
diff --git a/pkg/debian/archivebox.service b/pkg/debian/archivebox.service
index 916f2cd9..b3fe89ad 100644
--- a/pkg/debian/archivebox.service
+++ b/pkg/debian/archivebox.service
@@ -1,3 +1,6 @@
+# The archivebox user/group and /var/lib/archivebox directory are created by
+# postinstall.sh (which runs after dpkg unpacks the package contents).
+
 [Unit]
 Description=ArchiveBox Web Archiving Server
 After=network.target
diff --git a/pkg/debian/nfpm.yaml b/pkg/debian/nfpm.yaml
index 8fc55afc..0e592af1 100644
--- a/pkg/debian/nfpm.yaml
+++ b/pkg/debian/nfpm.yaml
@@ -55,7 +55,7 @@ contents:
     file_info:
       mode: 0644
 
-  # Create data directory
+  # Create data directory (unpacked as root; postinstall.sh chowns to archivebox user)
   - dst: /var/lib/archivebox
     type: dir
     file_info:
diff --git a/pkg/debian/scripts/preremove.sh b/pkg/debian/scripts/preremove.sh
index f7674ce0..6d48ab24 100755
--- a/pkg/debian/scripts/preremove.sh
+++ b/pkg/debian/scripts/preremove.sh
@@ -2,7 +2,9 @@
 # preremove script for archivebox .deb package
 set -e
 
-# Only clean up on full removal, not during upgrade
+# Only clean up on full removal, not during upgrade.
+# dpkg passes "$1" as "remove", "purge", or "upgrade" — we skip cleanup on
+# upgrade so the venv and service persist across package version bumps.
 if [ "$1" = "remove" ] || [ "$1" = "purge" ]; then
     # Stop the service if running
     if command -v systemctl >/dev/null 2>&1 && [ -d /run/systemd/system ]; then