Skip to content

Commit f2940ab

Browse files
authored
fix: address multi-dimensional review findings (nginx routing, hardening, CI) (#7)
* fix(nginx): stop blocking GLPI's legacy .php URLs (apirest.php, front/*, ajax/*) The blanket `location ~ \.php$ { deny all; }` matched /apirest.php, /front/central.php, /ajax/*.php etc. before try_files could route them through public/index.php — so the REST API and the entire post-login UI returned 403 (the login page at / worked, which is why the original smoke test missed it). Removed the block; GLPI 11 routes everything through index.php and execution stays confined to it (the ^/index\.php$ location is the only fastcgi pass). Verified: /apirest.php 403->400, /front/central.php 403->302. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> * fix(compose): decouple health from scheduler, cap_drop all services, socket-proxy - app healthcheck is now php-fpm /ping only (was gated on a scheduler-touched cron heartbeat, so a dead scheduler made app unhealthy and blocked web from starting); dropped the now-unused glpi-heartbeat job. - cap_drop: ALL on db, valkey, scheduler, backup, app-assets, socket-proxy (only app+web had it) with minimal re-adds; verified every service still starts healthy. - ofelia no longer mounts the raw host docker socket (== host root): a least-privilege docker-socket-proxy (CONTAINERS+EXEC+POST only, healthchecked) sits in front; ofelia reaches it over the internal network. Pinned ofelia by digest. - forward GLPI_DEFAULT_LANGUAGE + SESSION_COOKIE_SECURE; add scheduler TZ; fix the mem_limit/pm.max_children sizing comment. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> * fix(image): cache-marker hash, session-cookie toggle, pm sizing, version tracking - entrypoint stores a SHA-256 of the cache DSN (not the cleartext DSN, which may carry a Redis password) and no longer logs the raw DSN; writes a session.cookie_secure drop-in from SESSION_COOKIE_SECURE so HTTPS deploys get the Secure flag. - pm.max_children 25->10 to actually fit the 1.5g app cgroup; comment explains the math. - docker-bake.hcl derives MINOR/MAJOR from GLPI_VERSION (no drift); dev target uses a neutral cache so it builds locally. Renovate now tracks the base images (PHP/Alpine) and the bake GLPI version too. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> * fix(ci+docs): manifest-aware GHCR cleanup, release make_latest, dedupe bots, dev override - cleanup-packages: switch to dataaxiom/ghcr-cleanup-action — actions/delete-package-versions is not manifest-list aware and could delete a live image's per-arch children or cosign/SLSA referrers (the failure mode that bricked ghcr's phpbu-docker). - release.yml: only the newest semver release moves the 'Latest' badge (make_latest gate). - remove dependabot.yml — Renovate already manages actions + docker + versions (no dup PRs). - compose.override.yml.example: GLPI_ENVIRONMENT_TYPE=development (GLPI has no APP_DEBUG), loopback-bind db/valkey; caddy overlay logging; .env redis-auth note; README/Makefile console + lint + backup-profile fixes. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> * ci(smoke-test): assert GLPI legacy .php entry points route (regression guard) The serve check only hits / (the login page), which stayed 200 even while a blanket nginx .php deny 403'd /apirest.php and /front/*.php. Add an explicit check that those entry points are not forbidden so the nginx-routing regression can't recur. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> --------- Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
1 parent 1b7d1a5 commit f2940ab

17 files changed

Lines changed: 271 additions & 135 deletions

File tree

.env.example

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,10 @@ DB_ROOT_PASSWORD=
3232
# -----------------------------------------------------------------------
3333
# Cache — Valkey (Linux Foundation Redis fork; speaks RESP, so redis:// works)
3434
# -----------------------------------------------------------------------
35+
# Put any auth in the DSN itself (redis://:pw@valkey:6379) AND add
36+
# --requirepass to the valkey command in compose.yml. The bundled valkey has
37+
# no password (reachable only on the internal network).
3538
GLPI_CACHE_DSN=redis://valkey:6379
36-
# Set only if you reconfigure the valkey service to require a password.
37-
REDIS_PASSWORD=
3839

3940
# -----------------------------------------------------------------------
4041
# GLPI runtime

.github/dependabot.yml

Lines changed: 0 additions & 53 deletions
This file was deleted.

.github/workflows/cleanup-packages.yml

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,23 @@ jobs:
3030
permissions:
3131
packages: write
3232
steps:
33-
- name: Delete old untagged versions
34-
uses: actions/delete-package-versions@e5bc658cc4c965c472efe991f8beea3981499c55 # v5.0.0
33+
# Manifest-list + referrer aware: actions/delete-package-versions is NOT,
34+
# so it would happily delete a per-arch CHILD manifest (or a cosign sig /
35+
# SLSA attestation referrer) of a LIVE tagged index once enough newer
36+
# untagged versions exist — silently breaking multi-arch pulls and
37+
# verification (the same failure mode that bricked ghcr's phpbu-docker).
38+
# dataaxiom/ghcr-cleanup-action understands manifest lists: it keeps the
39+
# children + referrers of every tagged image and only removes orphans.
40+
- name: Delete orphaned / partial versions (manifest-aware)
41+
uses: dataaxiom/ghcr-cleanup-action@d52806a0dc70b430571a37da1fde39733ffd640f # v1.2.2
3542
with:
36-
package-name: ${{ env.PACKAGE_NAME }}
37-
package-type: container
38-
# Keep the 10 most recent untagged versions as a rollback margin.
39-
min-versions-to-keep: 10
40-
# Only ever delete untagged versions — released tags are immutable here.
41-
delete-only-untagged-versions: true
43+
packages: ${{ env.PACKAGE_NAME }}
44+
delete-untagged: true # orphaned untagged manifests
45+
delete-partial-images: true # incomplete multi-arch pushes
46+
delete-ghost-images: true # tags whose children were GC'd
47+
keep-n-untagged: 10 # rollback margin
48+
# Children + cosign/SLSA referrers of TAGGED images are retained
49+
# automatically — tagged releases are never broken.
4250

4351
- name: Summary
4452
env:
@@ -49,5 +57,5 @@ jobs:
4957
echo ""
5058
echo "- Package: \`${PACKAGE_NAME}\` (container)"
5159
echo "- Kept minimum: 10 untagged versions"
52-
echo "- Deleted only untagged: true (tagged releases untouched)"
60+
echo "- Manifest-aware prune (children + sig/attestation referrers of tagged images kept)"
5361
} >> "$GITHUB_STEP_SUMMARY"

.github/workflows/release.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,26 @@ jobs:
6868
# rather than interpolating into the script body (sonarcloud
6969
# githubactions:S7630, shell injection).
7070
REF_NAME: ${{ github.ref_name }}
71+
REPO: ${{ github.repository }}
72+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
7173
run: |
7274
set -euo pipefail
7375
# Defensive 'v' strip even though the tag filter already excludes it.
7476
VERSION="${REF_NAME#v}"
77+
# Is this the newest semver tag in the repo? An out-of-order / backport
78+
# release must NOT move the GH "Latest" badge (and ideally not the
79+
# floating image tags) onto an older version.
80+
NEWEST=$(gh api "repos/${REPO}/git/refs/tags" \
81+
--jq '.[].ref | ltrimstr("refs/tags/")' 2>/dev/null \
82+
| sed 's/^v//' | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$' \
83+
| sort -V | tail -1)
84+
IS_NEWEST=true
85+
[ -n "$NEWEST" ] && [ "$VERSION" != "$NEWEST" ] && IS_NEWEST=false
7586
{
7687
echo "version=$VERSION"
7788
echo "major=$(echo "$VERSION" | cut -d. -f1)"
7889
echo "minor=$(echo "$VERSION" | cut -d. -f1-2)"
90+
echo "is_newest=$IS_NEWEST"
7991
echo "build_date=$(date -u +'%Y-%m-%dT%H:%M:%SZ')"
8092
echo "git_sha=$(git rev-parse --short HEAD)"
8193
} >> "$GITHUB_OUTPUT"
@@ -141,6 +153,10 @@ jobs:
141153
with:
142154
# Auto-categorized from .github/release.yml; prepend the usage block.
143155
generate_release_notes: true
156+
# Only the newest semver release gets the "Latest" badge — a backport
157+
# release of an older line must not steal it (GH marks latest by
158+
# creation time otherwise).
159+
make_latest: ${{ steps.version.outputs.is_newest }}
144160
body: |
145161
## Container image
146162

.github/workflows/smoke-test.yml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,26 @@ jobs:
131131
docker compose logs --tail=200
132132
exit 1
133133
134+
- name: Assert GLPI routes legacy .php entry points (regression guard)
135+
env:
136+
PORT: '8080'
137+
run: |
138+
set -eu
139+
# GLPI 11 routes /apirest.php, /front/*.php and /ajax/*.php through
140+
# public/index.php. A blanket nginx `.php` deny would 403 these even
141+
# though the login page at / still works (exactly the bug the serve
142+
# check above cannot see). Assert none of them is forbidden.
143+
rc=0
144+
for p in /apirest.php /front/central.php /ajax/dashboard.php; do
145+
code=$(curl -sS -o /dev/null -w '%{http_code}' "http://127.0.0.1:${PORT}${p}" || true)
146+
echo " ${p} -> HTTP ${code}"
147+
if [ "$code" = "403" ] || [ "$code" = "000" ]; then
148+
echo "::error::${p} returned ${code} — nginx is not routing a GLPI .php entry point"
149+
rc=1
150+
fi
151+
done
152+
[ "$rc" -eq 0 ] || { docker compose logs web --tail=50; exit 1; }
153+
134154
- name: Assert app + web report healthy
135155
run: |
136156
set -eu

Dockerfile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
# GLPI_VERSION — GLPI release (default 11.0.8 — keep in sync with .glpi-version)
2424
# GLPI_SHA256 — sha256 of glpi-${GLPI_VERSION}.tgz (supply-chain pin; "" skips)
2525

26+
# renovate: datasource=docker depName=php versioning=docker
2627
ARG PHP_VERSION=8.4
28+
# renovate: datasource=docker depName=alpine versioning=docker
2729
ARG ALPINE_VERSION=3.21
2830

2931
# =====================================================================

Makefile

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ backup: ## Run a backup now (requires `make backup-up`; normally ofelia at 03:00
8383
docker compose --profile backup exec -T backup phpbu --configuration=/config/backup.json
8484

8585
backup-list: ## List backup archives
86-
docker compose exec -T backup ls -lh /backups
86+
docker compose --profile backup exec -T backup ls -lh /backups
8787

8888
backup-verify: ## Sanity-check that last night's backup is on disk + non-zero
89-
@docker compose exec -T backup sh -c '\
89+
@docker compose --profile backup exec -T backup sh -c '\
9090
latest=$$(ls -t /backups/db/*.sql.gz 2>/dev/null | head -1); \
9191
if [ -z "$$latest" ]; then \
9292
echo "✗ no DB backups in /backups/db"; exit 1; \
@@ -168,6 +168,9 @@ lint: ## Run hadolint + shellcheck + yamllint via Docker (no local install)
168168
cytopia/yamllint:1 \
169169
-d "{extends: default, rules: {line-length: disable, document-start: disable, truthy: {check-keys: false}, comments: {min-spaces-from-content: 1}}}" \
170170
.github/workflows .hadolint.yaml compose.yml
171+
@printf '\033[1;34m[lint]\033[0m actionlint .github/workflows\n'
172+
docker run --rm -v $(CURDIR):/work -w /work \
173+
rhysd/actionlint:latest -color
171174

172175
# ────────────────────────────────────────────────────────────────────
173176
# Upgrade

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ each on its own named volume.
122122

123123
```bash
124124
make logs # tail all services
125-
make console -- glpi:cache:clear # run any bin/console command
125+
make console CMD="cache:clear" # run any bin/console command
126126
make ps # status
127127
make down # stop (keep volumes)
128128
```

compose.override.yml.example

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,55 +6,64 @@
66
# Copy to compose.override.yml and `docker compose up -d` will merge it.
77
#
88
# Adds:
9-
# - mailpit (SMTP sink + web UI on http://localhost:8025)
10-
# - exposes db port on host:3306
11-
# - exposes redis port on host:6379
12-
# - flips APP_DEBUG=true for the app
13-
# - opcache validate_timestamps=1 for hot-reload of code (only useful if
14-
# you bind-mount source from the host, not for the default image-baked code)
9+
# - mailpit (SMTP sink + web UI on http://127.0.0.1:8025); point GLPI at it
10+
# from Setup > Notifications (host mailpit, port 1025) — GLPI configures SMTP
11+
# in the UI, NOT via env.
12+
# - adminer (DB UI on http://127.0.0.1:8081)
13+
# - exposes db + valkey on LOOPBACK only for local inspection
14+
# - GLPI_ENVIRONMENT_TYPE=development (GLPI's real debug switch — shows the
15+
# debug toolbar + verbose errors; there is no Laravel APP_DEBUG in GLPI)
16+
# - opcache validate_timestamps=1 for hot-reload (only useful if you bind-mount
17+
# source over the image-baked code)
18+
#
19+
# All published ports are bound to 127.0.0.1 so a dev laptop on an untrusted
20+
# network never exposes the database / cache / mail UI off-host.
1521

1622
services:
1723
app:
1824
environment:
19-
APP_ENV: local
20-
APP_DEBUG: "true"
21-
MAIL_HOST: mailpit
22-
MAIL_PORT: 1025
23-
MAIL_ENCRYPTION: ""
24-
MAIL_USERNAME: ""
25-
MAIL_PASSWORD: ""
25+
GLPI_ENVIRONMENT_TYPE: development
2626

2727
db:
2828
ports:
29-
- "3306:3306"
29+
- "127.0.0.1:3306:3306"
3030

3131
valkey:
32-
# Heads-up: 6379 is the default Redis/Valkey port. If you already run a
33-
# local Redis/Valkey on the host, `docker compose up` will fail with
34-
# "bind: address already in use". Comment out the mapping below, or
35-
# remap to a free host port (e.g. "16379:6379") if you don't need
36-
# external access to this container's Valkey from the host.
3732
ports:
38-
- "6379:6379"
33+
- "127.0.0.1:6379:6379"
3934

4035
mailpit:
4136
image: axllent/mailpit:latest
4237
restart: unless-stopped
38+
logging:
39+
driver: json-file
40+
options:
41+
max-size: "10m"
42+
max-file: "5"
4343
ports:
44-
- "8025:8025" # web UI
45-
- "1025:1025" # SMTP (also reachable as mailpit:1025 from app)
44+
- "127.0.0.1:8025:8025" # web UI
45+
- "127.0.0.1:1025:1025" # SMTP (also reachable as mailpit:1025 from app)
4646
environment:
4747
MP_SMTP_AUTH_ACCEPT_ANY: "1"
4848
MP_SMTP_AUTH_ALLOW_INSECURE: "1"
49+
security_opt:
50+
- no-new-privileges:true
4951
networks:
5052
- glpi
5153

5254
adminer:
5355
image: adminer:latest
5456
restart: unless-stopped
57+
logging:
58+
driver: json-file
59+
options:
60+
max-size: "10m"
61+
max-file: "5"
5562
ports:
56-
- "8081:8080"
63+
- "127.0.0.1:8081:8080"
5764
environment:
5865
ADMINER_DEFAULT_SERVER: db
66+
security_opt:
67+
- no-new-privileges:true
5968
networks:
6069
- glpi

0 commit comments

Comments
 (0)