From 6998a22527ae9fc63b1f7b639bc613fc7ed1255c Mon Sep 17 00:00:00 2001 From: nexxo Date: Tue, 28 Jul 2026 23:53:25 +0200 Subject: [PATCH] Repair ownership by looking inside, not at the door MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ownership repair added in 1.1.1 sampled the owner of each top-level directory and skipped the recursion when it already matched. node_modules was owned by www-data; node_modules/.vite-temp, left behind by an earlier root build, was not. So the repair walked past it, `npm run build` failed with EACCES, and the deployment stopped in maintenance mode — on the release whose whole point was that this could not happen. A directory's owner says nothing about its contents. It now walks each tree once with find and changes only the entries that are wrong, which is also cheaper than the detect-then-chown-everything it replaces. Verified against the shape that actually failed: a root-owned file inside a www-data-owned node_modules, repaired. Co-Authored-By: Claude Opus 5 --- VERSION | 2 +- deploy/update.sh | 15 ++++++++++++--- tests/Feature/DeploymentRunsAsTheAppUserTest.php | 8 +++++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/VERSION b/VERSION index 26aaba0..6085e94 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.2.0 +1.2.1 diff --git a/deploy/update.sh b/deploy/update.sh index d498e1c..06abad8 100755 --- a/deploy/update.sh +++ b/deploy/update.sh @@ -79,11 +79,20 @@ in_app() { docker compose exec -T -u www-data app "$@"; } # node_modules/ left owned by root would make the very next composer or npm # step fail outright. normalise_ownership() { + # find, not `stat` on the directory itself. The first version of this + # sampled the top-level owner and skipped the recursion when it matched — + # and node_modules was owned by www-data while node_modules/.vite-temp, + # left by an earlier root build, was not. The deployment then failed at + # `npm run build` with EACCES, in maintenance mode, on the very release + # that was supposed to prevent it. A directory's owner says nothing about + # its contents. + # + # One walk that changes only what is wrong, rather than a detection pass + # and then a blanket chown -R over thirty thousand files. docker compose exec -T -u root app sh -c ' - chown -R www-data:www-data storage bootstrap/cache 2>/dev/null || true - for d in vendor node_modules public/build; do + for d in storage bootstrap/cache vendor node_modules public/build; do [ -d "$d" ] || continue - [ "$(stat -c %U "$d" 2>/dev/null)" = "www-data" ] || chown -R www-data:www-data "$d" 2>/dev/null || true + find "$d" ! -user www-data -exec chown www-data:www-data {} + 2>/dev/null || true done ' >/dev/null 2>&1 || true } diff --git a/tests/Feature/DeploymentRunsAsTheAppUserTest.php b/tests/Feature/DeploymentRunsAsTheAppUserTest.php index 9b4050c..e3d8e67 100644 --- a/tests/Feature/DeploymentRunsAsTheAppUserTest.php +++ b/tests/Feature/DeploymentRunsAsTheAppUserTest.php @@ -57,7 +57,13 @@ it('repairs ownership before it needs it, not after', function () { $update = File::get(base_path('deploy/update.sh')); expect($update)->toContain('normalise_ownership') - ->and($update)->toContain('chown -R www-data:www-data storage bootstrap/cache'); + // Every directory the deployment writes into, and node_modules by name: + // the first version sampled the top-level owner and skipped the + // recursion when it matched, which is how a root-owned + // node_modules/.vite-temp under a www-data-owned node_modules failed + // the build in maintenance mode. + ->and($update)->toContain('! -user www-data -exec chown www-data:www-data') + ->and($update)->toContain('node_modules'); // Before the first unprivileged step, or it cannot help: composer and npm // would already have failed on a root-owned vendor directory.