Skip to content

fix(setup): respect PUID/PGID + look up www-data from /etc/passwd#2

Merged
sylvesterdamgaard merged 5 commits intomainfrom
fix/respect-puid-pgid
May 7, 2026
Merged

fix(setup): respect PUID/PGID + look up www-data from /etc/passwd#2
sylvesterdamgaard merged 5 commits intomainfrom
fix/respect-puid-pgid

Conversation

@sylvesterdamgaard
Copy link
Copy Markdown
Contributor

Problem

internal/setup/permissions.go hardcoded uid/gid 82 in setupLaravel, setupSymfony, and setupWordPress:

```go
// Set ownership (www-data UID/GID typically 82 on Alpine, 33 on Debian)
pm.chownRecursive(filepath.Join(pm.workdir, "storage"), 82, 82)
pm.chownRecursive(filepath.Join(pm.workdir, "bootstrap", "cache"), 82, 82)
```

On Debian-based images (`php-fpm-nginx:8.5-bookworm` and friends) `www-data` is uid 33, so the chown left storage/ at uid 82 (UNKNOWN user) while the fpm worker actually runs as uid 33 — a mismatch that breaks every framework directory the binary tries to "fix".

Concrete symptom: Laravel view cache writes fail, `tempnam` falls back to `/tmp`, the debug error renderer escalates the resulting E_NOTICE → ErrorException → 500 on every route. Setting `PUID=33 PGID=33` in the deployment env had no effect because the Go binary ignored those vars.

Fix

New `resolveAppUser` helper picks the uid/gid in this order:

  1. `PUID` / `PGID` env vars — explicit operator override. Both must be valid non-negative integers or the override is ignored.
  2. `/etc/passwd` lookup of `www-data` — handles Debian (33) and Alpine (82) without hardcoding either, so the same binary produces correct ownership in both baseimages.
  3. Fallback to the previous `82, 82` Alpine-convention default.

The chosen path is logged so an operator can see which one resolved:

```
INFO Setting up permissions framework=laravel
INFO App user from /etc/passwd lookup user=www-data uid=33 gid=33
```

Tests

  • `TestReadPuidPgidEnv` covers the env parser including edge cases (one set, the other unset; non-numeric; negative; uid 0 valid).
  • `TestResolveAppUser_PUIDOverridesPasswdLookup` confirms the env override wins over the passwd lookup.
  • `TestResolveAppUser_FallsBackWhenLookupFails` confirms the Alpine-convention fallback only kicks in when both prior paths miss.

`go test ./internal/setup/...` passes.

Test plan

  • Unit tests pass (`go test ./internal/setup/...`)
  • Re-tag the affected baseimages (`php-fpm-nginx:*-bookworm`) and verify a Laravel pod starts with `storage/` + `bootstrap/cache/` owned by uid 33 without any postStart hook

setupLaravel/Symfony/WordPress hardcoded uid/gid 82 (Alpine
convention) for the chown step. On Debian-based images www-data
is uid 33, so the chown produced uid 82 (UNKNOWN user) and the
fpm worker — running as the real www-data uid 33 — could not
write storage/, bootstrap/cache/ or var/. Symptom: every Laravel
view-cache write fell back to /tmp via tempnam, which Laravel's
debug error renderer escalated to a 500 response on every route.

Resolution order is now:

  1. `PUID`/`PGID` env vars — explicit operator override.
  2. /etc/passwd lookup of `www-data` — handles Debian (33) and
     Alpine (82) correctly without baking the uid into the binary.
  3. Fallback to the previous Alpine-convention 82/82.

The chosen path is logged so it's visible in container startup
output. New tests cover the env-var parser (incl. validation
edge cases) and the override-wins-over-passwd contract.
…ents

- Document PUID/PGID env vars in environment-variables.md
- Update docker-integration.md to note auto-ownership at startup
- Create CHANGELOG.md covering v1.2.2 through unreleased
- Refactor resolveAppUser to be called once in Setup() instead of per-framework
- Add warn log when only one of PUID/PGID is set
- Add negative PGID test case for symmetry
- Enhance logs command with additional functionality and tests
- Improve TUI model and apiclient error handling
- Fix logtail rotator minor issues
- Add warn log when PUID/PGID values are set but invalid (non-numeric)
- Update list command formatting
… lint error

model_test.go in the tui package accessed the unexported baseURL field
on apiclient.Client, causing a typecheck failure in golangci-lint.
The --remote flag default was changed from "http://localhost:9180" to ""
(auto-discover Unix socket) but the test was not updated.
@sylvesterdamgaard sylvesterdamgaard merged commit f637c21 into main May 7, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant