Commit c56d164
authored
feat(devtools): drop privileges to apache for OpenEMR CLI invocations (#743)
## Summary
openemr/openemr#12267 introduces a `RootCliGuard` that refuses to let
OpenEMR CLI scripts run as root: anything that loads
`interface/globals.php`, runs through `bin/console`, or calls into the
`Installer` class aborts with a `RuntimeException`. The failure it
catches is the one that produced the original `/tmp/openemr-smarty`
issue — PHP run as root creates files (cache dirs, generated keys,
Smarty compile output) the web server later cannot read, surfacing as
opaque fatal errors deep in template rendering or OAuth.
This PR is openemr-devops's companion: it makes every OpenEMR CLI
invocation across **flex (dev container)**, **8.1.1 (production)**, and
**binary (rolling production)** drop privileges to the apache user
before the script runs — including the container entrypoint's first-boot
install path and the fsupgrade chain that runs when a container starts
on an image with a newer docker-version than the persistent volume.
Without this PR landed and the images rebuilt against an openemr master
containing #12267, every fresh container would brick on first-boot
(`auto_configure.php` hits the new Installer gate at startup), every
devtools install / upgrade / multisite / API-docs command would throw,
and every customer container crossing into a guard-bearing release would
fail mid-upgrade.
## What changes
Four classes of changes across each of the three images:
### 1. New `su-exec` dependency (all three Dockerfiles)
`apk add --no-cache su-exec` (alpine main, 14 KB static binary). su-exec
exec's a target program directly under a different uid/gid +
supplementary groups with **no intervening shell** — argv reaches the
target program verbatim, env is preserved automatically. It's what
official postgres/mariadb/etc. Docker images use for the same job.
`shadow` is already present in all three Dockerfiles, but alpine
deliberately excludes shadow's `/usr/bin/su` to avoid clashing with
busybox `/bin/su`. Busybox `su` itself can't be used in argv-passthrough
mode (`su -c 'exec "$@"' arg0 …`) because its option parser rescans the
entire arg list, absorbing inner `-c`, `-f`, `-s` flags as su's own —
that's why we need a dedicated tool.
### 2. `utilities/devtoolsLibrary.source` — manual-invocation paths
New helper:
```bash
run_php_as_apache() {
su-exec apache "$@"
}
```
Affected `devtools` commands:
| devtools command | Underlying script | Function |
|---|---|---|
| `dev-install` / `dev-reset-install` / `dev-reset-install-demodata` /
`dev-sqldrive` / `dev-reset-install-sqldrive` | `php auto_configure.php`
| `installOpenemr()` |
| `upgrade <version>` | `php sql_upgrade.php` | `upgradeOpenEMR()` |
| `dev-sqldrive` / `dev-reset-install-sqldrive` | `php import_ccda.php`
| `demoDataDrive()` |
| `generate-multisite-bank` | `php InstallerAuto.php` |
`multiSiteBank()` |
| `set-swagger-to-multisite` | `php bin/command-runner` | (flex
devtools) |
| `build-api-docs` | `php bin/console openemr:create-api-documentation`
| (flex devtools) |
### 3. `openemr.sh` — first-boot install path (all three images)
`openemr.sh:run_auto_configure()` invokes `auto_configure.php` directly
at container startup. `auto_configure.php` instantiates the `Installer`
class and calls `quick_install()` — which is now gated. **Without this
fix, every fresh container would fail to install and the web UI would
never come up.**
Three coordinated adjustments per entrypoint:
- Wrap the `php auto_configure.php …` invocation in `su-exec apache …`.
Argv-passthrough means `${CONFIGURATION}` / `${config_args[@]}` reach
php verbatim — values containing quotes, `$`, `;`, backticks, or control
characters in (e.g.) `MYSQL_ROOT_PASS` pass through unmolested.
- `chmod 0644 auto_configure.php` immediately before invocation. The
Dockerfile sets the file to mode 000 as a safety measure (root could
read it regardless); apache cannot, so a brief loosen-then-run sequence
is needed. The file is `rm -f`'d at the end of the entrypoint either
way.
- `chown apache:apache` + `chmod 0700` the opcache file-cache dir
(`/tmp/php-file-cache`) after `mkdir -p`. apache is the only writer
(under su-exec). Cache dir is `rm -rf`'d at the end of the entrypoint
either way.
### 4. `upgrade/fsupgrade-*.sh` — automatic upgrade path on container
restart (8.1.1 + binary only)
18 scripts (10 in 8.1.1, 8 in binary). Each builds a
`TEMPsql_upgrade.php` from the on-disk `sql_upgrade.php` and runs it via
`php -f` to migrate the database. `openemr.sh:run_upgrade()` invokes
these on container startup whenever the persisted `docker-version` is
older than the image's — without the wrap, an existing customer
container starting on a guard-bearing release would abort partway
through the upgrade chain. Each script's `php -f TEMPsql_upgrade.php` is
wrapped inline in `su-exec apache php -f …`.
## Scoping
Only flex, 8.1.1, and binary are patched. 7.0.4, 8.0.0, and 8.1.0 are
intentionally out of scope per discussion — those aren't expected to be
rebuilt against a guard-containing openemr release.
## Iteration history (what local rebuild caught)
This PR went through several iterations driven by local-rebuild
validation against an openemr master containing #12267. Each iteration's
bug was invisible from PR diff review alone but immediately apparent on
container boot:
1. `openemr.sh:run_auto_configure` ran auto_configure.php as root →
tripped the new Installer gate. Fixed: wrap in privilege drop.
2. Dockerfile's `chmod 000 auto_configure.php` safety measure left the
script apache-unreadable. Fixed: `chmod 0644` immediately before the
invocation.
3. `mkdir -p /tmp/php-file-cache` default mode 0755 → apache couldn't
write opcache files. Fixed: chown apache + chmod 0700 (originally `chmod
0777`; tightened to least-privilege after Copilot review).
4. Copilot review flagged shell-injection / quoting risk in the `su -c
"…"` interpolation pattern (real concern: passwords with `"` or `$`
would mis-parse). Multiple attempts at the `su -c 'exec "$@"' …`
argv-passthrough form failed against busybox `su`'s option rescanning.
Resolved by adopting `su-exec` (14 KB alpine package) for direct
argv-passthrough with no shell parsing at all.
## Test plan
End-to-end **local rebuild validation** completed against both image
classes after each substantive iteration:
### flex (dev) — gate-firing scenario
Built `openemr/openemr:flex` locally from this branch; ran against an
openemr master containing openemr/openemr#12267 via the dev-easy stack
(volume-mounted source provides the new gate). Recycled the stack with
volumes removed.
- Container boot: install completed cleanly through `su-exec`. Zero
`RootCliGuard` / `RuntimeException` in container logs. Login page
returns HTTP 200.
- Devtools end-to-end:
- `build-api-docs` → swagger yaml generated (exercises
`bin/command-runner` + `bin/console openemr:create-api-documentation`).
- `upgrade 7.0.0` → all schema migrations 7.0.0 → current complete
(exercises `sql_upgrade.php`).
- `generate-multisite-bank 1` → multisite `run1` created (exercises
`InstallerAuto.php`).
- Negative test: bare `php bin/console list` and bare `php
sql_upgrade.php …` as root both throw the expected `RootCliGuard`
`RuntimeException` — confirms the gate still bites for unwrapped
invocations.
- Post-test web UI + swagger UI both still HTTP 200; container log scan
over the test window: zero `permission denied` / `fatal` errors.
### 8.1.1 (production) — backward-compatibility scenario
Built `openemr/openemr:8.1.1` locally from this branch; ran via
`docker/production/` compose against the image's baked-in 8.1.1 openemr
source (which predates the gate). Tore down with `--volumes` after.
- Container boot: install completed, status `running (health: healthy)`,
login page returns HTTP 200.
- Log scan: zero `RootCliGuard` / `RuntimeException` /
`opcache.file_cache` / install errors.
- Confirms the openemr.sh + devtools changes don't regress current
production-image behavior. When 8.1.1 is eventually rebuilt against an
openemr master containing #12267, the changes become load-bearing.
### binary (rolling production) — not directly tested
Structurally identical to 8.1.1 (just `php` → `/usr/local/bin/php`).
Changes mirror 8.1.1's; relies on that symmetry rather than a separate
build cycle.
## Related
- **openemr/openemr#12267** — establishes the `RootCliGuard`. Required
for this PR's wraps to be load-bearing rather than hygiene.
- **openemr/demo_farm_openemr#109** (merged) — same privilege-drop
pattern in `demo_build.sh`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)1 parent 55e402c commit c56d164
28 files changed
Lines changed: 256 additions & 49 deletions
File tree
- docker/openemr
- 8.1.1
- upgrade
- utilities
- binary
- upgrade
- utilities
- flex
- utilities
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
50 | 53 | | |
51 | 54 | | |
52 | 55 | | |
| |||
68 | 71 | | |
69 | 72 | | |
70 | 73 | | |
| 74 | + | |
71 | 75 | | |
72 | 76 | | |
73 | 77 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
578 | 578 | | |
579 | 579 | | |
580 | 580 | | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
581 | 586 | | |
582 | 587 | | |
583 | 588 | | |
| |||
596 | 601 | | |
597 | 602 | | |
598 | 603 | | |
599 | | - | |
600 | | - | |
601 | | - | |
602 | | - | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
603 | 616 | | |
604 | | - | |
| 617 | + | |
| 618 | + | |
605 | 619 | | |
606 | 620 | | |
607 | 621 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
57 | | - | |
| 57 | + | |
| 58 | + | |
58 | 59 | | |
59 | 60 | | |
60 | 61 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
36 | | - | |
| 36 | + | |
| 37 | + | |
37 | 38 | | |
38 | 39 | | |
39 | 40 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
0 commit comments