Skip to content

Commit 91477d1

Browse files
authored
feat(build-tests): add Phase 1 dry-run regression suite for demo_build.sh (#146) (#147)
* feat(build-tests): add Phase 1 dry-run regression suite for demo_build.sh Phase 1 of issue #146: per-PR regression coverage that catches the PR #142 (printf '-' dash) and PR #145 (SC2115 rm -fr footguns) class of bug at PR time in seconds, rather than waiting for the nightly host reset to surface them. Three pieces: - demo_build.sh: additive --dry-run flag + ACTION_LOG emitter (run_action / run_action_sh / run_action_capture helpers). With no flag and no ACTION_LOG env, helpers degrade to plain exec -- production behavior is unchanged. - tools/build-tests/: fixture-test harness mirroring the tools/auto-derive/fixtures-and-tests/ pattern. 6 scenarios cover branch-pinned/tag-pinned/release-packageserve/fork-URL/ light-reset/multi-demo paths. - .github/workflows/build-tests.yml: runs the suite on every PR that touches demo_build.sh or tools/build-tests/**. Phase 2 (separate PR) will add a slower nightly job that actually executes demo_build.sh end-to-end against a compose stack, reusing the same fixtures + action-log emitter, and asserting the dry-run log matches what live execution produces -- the integrity check that keeps Phase 1 honest. Refs: #146 Assisted-by: Claude Code * feat(build-tests): address rabbit review + add capsule-path-traversal fixture Rabbit-flagged fixes: - .github/workflows/build-tests.yml: drop the stale "includes derive-ip-map.yml" comment that didn't match the actual paths filter (and didn't need to -- changes to that workflow file don't affect build behavior). - demo_build.sh: add a _emit_action_log helper that redacts `-p<password>` (mariadb root pass flag) and `rootpass=<password>` from action-log lines before append. Best-effort; documented that ACTION_LOG should not be enabled in production without further review. Phase 2 live-run work will extend this with composer auth tokens and Authorization headers when those become real values rather than dry-run stubs. - demo_build.sh: document the trust assumption on run_action_sh (wrapped strings come from ip_map_branch.txt + docker env, not user input). Eval is intentional for pipes/redirects; refactor is out of scope for this PR. - demo_build.sh: unwrap two `mkdir` calls in the packageServe block so subsequent `cd` commands work in dry-run, where the intervening rsync is skipped. Same pattern already used for $WEB/log and $OPENEMR earlier in the script. New fixture: - capsule-path-traversal: synthetic ip_map row with `capsule` column = "../etc", exercising the case statement guard PR #145 added (demo_build.sh lines ~617-622). Pins the rejection so a future regression that drops the guard fails CI -- otherwise it would silently re-enable path traversal in dry code, since no production row currently sets `capsule`. - tools/build-tests/test.sh: extends harness with the `expected/fail.txt` pattern (mirrors auto-derive harness): substring check on stderr/stdout + assert non-zero exit. All 7 fixtures pass. Verified negative test: stripping the case guard from demo_build.sh fails ONLY capsule-path-traversal. Refs: #146 (issue), #147 (this PR) Assisted-by: Claude Code * fix(build-tests): rabbit-review-2 fixes for test.sh harness Two findings from rabbit's second review pass: - Fail-expected check used `cat $stdout $stderr | grep -qF`, which under `set -o pipefail` (enabled at the top of the script) can flip a real PASS into a false FAIL: grep closes stdin on the first match, cat dies on SIGPIPE, pipeline exits non-zero. Read the files directly with `grep -qF -- $msg $stdout $stderr` — no pipe, no false negative. - normalize_action_log was called unconditionally even when the script produced no action log. If a future regression makes demo_build.sh exit 0 without writing ACTION_LOG, `sed -i` on the missing file would trip `set -e` and abort the whole harness mid-run, killing later fixtures and the summary. Added an explicit `[[ -f $action_log ]]` guard that fails just the affected fixture with a clear message and lets the run finish. Refs: #146 (issue), #147 (this PR) Assisted-by: Claude Code * fix(build-tests): redaction false-positive on --unsafe-perm The `-p<pass>` redaction regex matched anywhere in a line, including inside `--unsafe-perm` (npm flag), turning legitimate command args into `--unsafe-p<REDACTED>` in the action log. Surfaced by an in-progress Phase 2 smoke run — first time live mode exercised the npm install branch end-to-end. Anchor the pattern to start-of-line or whitespace so it only matches the standalone mariadb `-p<password>` flag. Regen the 6 goldens that contained the false-positive form. Refs: #146 (issue), #147 (this PR) Assisted-by: Claude Code * fix(build-tests): extend redaction + fix fixture-extras path docstring Address two rabbit findings (PR #148, applies to Phase 1): * Latent secret leak in live mode: _emit_action_log only redacted -p<pass> and rootpass=<pass>. The script also emits the github api token via `curl -H "Authorization: token $GITHUB_KEY_COMPOSER"` (rate-limit probe) and `composer config --auth github-oauth.github.com $KEY`. In dry-run those values are stubbed (placeholders have <> which %q-escapes to \<\>, dodging the new pattern's [A-Za-z0-9_]+ body match), but Phase 2 live runs with a real /home/openemr/ github-key mounted would otherwise commit the raw token to ACTION_LOG. Added two more sed patterns to cover both shapes. * test.sh docstring example was stale: the harness does `cp -a "$extra_dir/." "$work/"` which mirrors the work-tree layout. Fixtures need to put version.php at extra/web/openemr/version.php, not extra/openemr/version.php (which silently lands at the wrong root and never overrides $WEB/openemr/version.php). Phase 1 goldens are unchanged — the new redaction patterns don't fire against the dry-run stubs. Refs: #146 (issue), #147 (this PR), #148 (Phase 2) Assisted-by: Claude Code
1 parent 000d131 commit 91477d1

27 files changed

Lines changed: 973 additions & 114 deletions

File tree

.github/workflows/build-tests.yml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
name: build-tests (dry-run)
2+
3+
# Phase 1 of issue #146: per-PR regression coverage for demo_build.sh.
4+
#
5+
# Runs `demo_build.sh --dry-run` against each fixture in
6+
# tools/build-tests/fixtures/ and diffs the captured action log against
7+
# the golden. Catches the PR #142 / #145 class of bug (control-flow,
8+
# field-index, quoting) at PR time, in seconds rather than minutes.
9+
#
10+
# Phase 2 (separate PR, future) will add a slower nightly job that
11+
# actually executes the script end-to-end against a compose stack;
12+
# this fast job is the per-PR signal.
13+
14+
on:
15+
pull_request:
16+
paths:
17+
- demo_build.sh
18+
- tools/build-tests/**
19+
- .github/workflows/build-tests.yml
20+
workflow_dispatch:
21+
22+
jobs:
23+
dry-run:
24+
runs-on: ubuntu-24.04
25+
permissions:
26+
contents: read
27+
steps:
28+
- name: Checkout
29+
uses: actions/checkout@v7
30+
with:
31+
persist-credentials: false
32+
33+
- name: Run fixture test suite
34+
run: |
35+
set -euo pipefail
36+
./tools/build-tests/test.sh

.shellcheckrc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ disable=SC3014 # `==` in place of `=`
3333
disable=SC3028 # `RANDOM` undefined
3434
disable=SC3030 # arrays undefined
3535
disable=SC3037 # `echo` flags undefined
36+
disable=SC3043 # `local` undefined -- used by the build-tests helpers added in #146
37+
disable=SC3050 # `printf %q` undefined -- used by the build-tests action-log emitter (#146)
3638
disable=SC3054 # array references undefined
3739

3840
# Pre-existing bash warnings deferred to follow-up cleanup PRs.

demo_build.sh

100644100755
Lines changed: 245 additions & 114 deletions
Large diffs are not rendered by default.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
ACTION: apk add --no-cache postfix stunnel
2+
ACTION_SH: echo 'error_log = <WORK>/web/log/logPhp.txt' > /etc/php85/conf.d/99-demo-farm-error-log.ini
3+
ACTION: git clone --branch master --depth 1 https://github.com/openemr/openemr.git
4+
ACTION_SH: rm -fr "<WORK>/web/openemr"/*
5+
ACTION_SH: rsync --recursive --exclude .git <WORK>/git/openemr/* <WORK>/web/openemr/
6+
ACTION: rm -fr <WORK>/git/openemr
7+
ACTION: chmod 666 <WORK>/web/openemr/sites/default/sqlconf.php
8+
ACTION: chmod -R a+w <WORK>/web/openemr/sites/default/documents
9+
ACTION_CAPTURE: cat /home/openemr/github-key
10+
ACTION_CAPTURE: curl -H Authorization:\ token\ \<DRY_RUN_GITHUB_KEY\> https://api.github.com/rate_limit
11+
ACTION_SH: composer install --no-dev &>> <WORK>/web/log/logSetup.txt
12+
ACTION_SH: npm install --unsafe-perm &>> <WORK>/web/log/logSetup.txt
13+
ACTION_SH: npm run build &>> <WORK>/web/log/logSetup.txt
14+
ACTION_SH: npm install --unsafe-perm &>> <WORK>/web/log/logSetup.txt
15+
ACTION_SH: composer global require phing/phing &>> <WORK>/web/log/logSetup.txt
16+
ACTION_SH: /root/.composer/vendor/bin/phing vendor-clean &>> <WORK>/web/log/logSetup.txt
17+
ACTION_SH: /root/.composer/vendor/bin/phing assets-clean &>> <WORK>/web/log/logSetup.txt
18+
ACTION_SH: composer global remove phing/phing &>> <WORK>/web/log/logSetup.txt
19+
ACTION_SH: composer dump-autoload -o &>> <WORK>/web/log/logSetup.txt
20+
ACTION_SH: sed -e 's@^exit;@ @' <<WORK>/web/openemr/contrib/util/installScripts/InstallerAuto.php ><WORK>/web/openemr/contrib/util/installScripts/InstallerAutoTemp.php
21+
ACTION_SH: su -p -s /bin/sh "root" -c "php -f <WORK>/web/openemr/contrib/util/installScripts/InstallerAutoTemp.php development_translations=yes rootpass=<REDACTED> server=mysql loginhost=% login=two pass=two dbname=two" >> <WORK>/web/log/logSetup.txt
22+
ACTION: rm -f <WORK>/web/openemr/contrib/util/installScripts/InstallerAutoTemp.php
23+
ACTION: mariadb --skip-ssl -h mysql -u root -p<REDACTED> -e UPDATE\ two.globals\ SET\ gl_value=\'https://two.openemr.io\'\ WHERE\ gl_name=\'site_addr_oath\'
24+
ACTION: mariadb --skip-ssl -h mysql -u root -p<REDACTED> -e UPDATE\ two.globals\ SET\ gl_value=\'\<RANDOM_THEME_2\>\'\ WHERE\ gl_name=\'css_header\'
25+
ACTION: mariadb --skip-ssl -h mysql -u root -p<REDACTED> -e $'\n UPDATE two.globals SET gl_value=\'1\' WHERE gl_name=\'portal_onsite_two_enable\';\n UPDATE two.globals SET gl_value=\'1\' WHERE gl_name=\'rest_api\';\n UPDATE two.globals SET gl_value=\'1\' WHERE gl_name=\'rest_fhir_api\';\n UPDATE two.globals SET gl_value=\'1\' WHERE gl_name=\'rest_portal_api\';\n UPDATE two.globals SET gl_value=\'3\' WHERE gl_name=\'oauth_password_grant\';\n UPDATE two.globals SET gl_value=\'1\' WHERE gl_name=\'rest_system_scopes_api\';\n UPDATE two.globals SET gl_value=\'3\' WHERE gl_name=\'ccda_alt_service_enable\';\n '
26+
ACTION: chmod 644 <WORK>/web/openemr/sites/default/sqlconf.php
27+
ACTION: mkdir <WORK>/web/openemr/sites/default/procedure_results
28+
ACTION: chmod -R a+w <WORK>/web/openemr/sites/default/procedure_results
29+
ACTION: sed -i s@/apis/default/@/openemr/apis/default/@ <WORK>/web/openemr/swagger/openemr-api.yaml
30+
ACTION: sed -i s@/oauth2/default/authorize@/openemr/oauth2/default/authorize@ <WORK>/web/openemr/swagger/openemr-api.yaml
31+
ACTION: sed -i s@/oauth2/default/token@/openemr/oauth2/default/token@ <WORK>/web/openemr/swagger/openemr-api.yaml
32+
ACTION: rm -f <WORK>/web/openemr/library/openflashchart/php-ofc-library/ofc_upload_image.php
33+
ACTION_SH: stunnel /etc/stunnel/stunnel.conf >> <WORK>/web/log/logSetup.txt
34+
ACTION_SH: postfix start >> <WORK>/web/log/logSetup.txt
35+
ACTION: cp <WORK>/git/demo_farm_openemr/openemr-alpine.conf /etc/apache2/conf.d/openemr.conf
36+
ACTION_SH: httpd -k start >> <WORK>/web/log/logSetup.txt
37+
ACTION: tail -F -n0 /etc/hosts
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# Scenario: branch-pinned (master) cluster with development translations,
2+
# patient-portal/API globals, and random theme (funStuff=2 -> ThemeTwo).
3+
# Exercises: git clone --branch master, translationsDevelopment branch,
4+
# random theme branch, ppapi globals branch.
5+
DOCKERDEMO=two
6+
DOCKERMYSQLHOST=mysql
7+
PHP_VERSION_ABBR=85
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
docker_number openemr_repo branch serve_development_translations(not used) use_development_translations serve_packages legacy_patching(not used) demo_data demo_ssh(not used) patient_portals_and_api external_link root_sql_pass branch_tag(not used) demo_data_upgrade_from fun_stuff pass_reset capsule description
2+
two https://github.com/openemr/openemr.git master 0 1 0 0 0 0 1 two.openemr.io hey 0 0 2 0 0 Branch-pinned master demo (with udt, ppapi, random theme)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
invalid useCapsuleFile value
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Scenario: useCapsuleFile column carries "../etc" (path-traversal attempt).
2+
# The case statement guard added by PR #145 (demo_build.sh lines ~617-622)
3+
# must reject this with a non-zero exit. Without the guard, the later
4+
# `rm -fr "${OPENEMR:?}/${useCapsuleFile}"` and `cp $CAPSULES/${useCapsuleFile}.tgz`
5+
# would traverse outside $OPENEMR.
6+
#
7+
# This is a fail-expected fixture (expected/fail.txt instead of action-log.txt):
8+
# the test asserts exit != 0 AND a specific error substring in script output.
9+
DOCKERDEMO=malcap
10+
DOCKERMYSQLHOST=mysql
11+
PHP_VERSION_ABBR=85
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
docker_number openemr_repo branch serve_development_translations(not used) use_development_translations serve_packages legacy_patching(not used) demo_data demo_ssh(not used) patient_portals_and_api external_link root_sql_pass branch_tag(not used) demo_data_upgrade_from fun_stuff pass_reset capsule description
2+
malcap https://github.com/openemr/openemr.git master 0 0 0 0 0 0 1 malcap.openemr.io hey 0 0 0 0 ../etc Synthetic row exercising the useCapsuleFile path-traversal guard
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
ACTION: apk add --no-cache postfix stunnel
2+
ACTION_SH: echo 'error_log = <WORK>/web/log/logPhp.txt' > /etc/php85/conf.d/99-demo-farm-error-log.ini
3+
ACTION: git clone --branch master --depth 1 https://github.com/openemr/openemr.git
4+
ACTION_SH: rm -fr "<WORK>/web/a/openemr"/*
5+
ACTION_SH: rsync --recursive --exclude .git <WORK>/git/openemr/* <WORK>/web/a/openemr/
6+
ACTION: rm -fr <WORK>/git/openemr
7+
ACTION: chmod 666 <WORK>/web/a/openemr/sites/default/sqlconf.php
8+
ACTION: chmod -R a+w <WORK>/web/a/openemr/sites/default/documents
9+
ACTION_CAPTURE: cat /home/openemr/github-key
10+
ACTION_CAPTURE: curl -H Authorization:\ token\ \<DRY_RUN_GITHUB_KEY\> https://api.github.com/rate_limit
11+
ACTION_SH: composer install --no-dev &>> <WORK>/web/log/logSetup.txt
12+
ACTION_SH: npm install --unsafe-perm &>> <WORK>/web/log/logSetup.txt
13+
ACTION_SH: npm run build &>> <WORK>/web/log/logSetup.txt
14+
ACTION_SH: npm install --unsafe-perm &>> <WORK>/web/log/logSetup.txt
15+
ACTION_SH: composer global require phing/phing &>> <WORK>/web/log/logSetup.txt
16+
ACTION_SH: /root/.composer/vendor/bin/phing vendor-clean &>> <WORK>/web/log/logSetup.txt
17+
ACTION_SH: /root/.composer/vendor/bin/phing assets-clean &>> <WORK>/web/log/logSetup.txt
18+
ACTION_SH: composer global remove phing/phing &>> <WORK>/web/log/logSetup.txt
19+
ACTION_SH: composer dump-autoload -o &>> <WORK>/web/log/logSetup.txt
20+
ACTION_SH: sed -e 's@^exit;@ @' <<WORK>/web/a/openemr/contrib/util/installScripts/InstallerAuto.php ><WORK>/web/a/openemr/contrib/util/installScripts/InstallerAutoTemp.php
21+
ACTION_SH: su -p -s /bin/sh "root" -c "php -f <WORK>/web/a/openemr/contrib/util/installScripts/InstallerAutoTemp.php development_translations=yes rootpass=<REDACTED> server=mysql loginhost=% login=two_a pass=two_a dbname=two_a" >> <WORK>/web/log/logSetup.txt
22+
ACTION: rm -f <WORK>/web/a/openemr/contrib/util/installScripts/InstallerAutoTemp.php
23+
ACTION: mariadb --skip-ssl -h mysql -u root -p<REDACTED> -e UPDATE\ two_a.globals\ SET\ gl_value=\'https://two.openemr.io\'\ WHERE\ gl_name=\'site_addr_oath\'
24+
ACTION: mariadb --skip-ssl -h mysql -u root -p<REDACTED> -e $'\n UPDATE two_a.globals SET gl_value=\'1\' WHERE gl_name=\'portal_onsite_two_enable\';\n UPDATE two_a.globals SET gl_value=\'1\' WHERE gl_name=\'rest_api\';\n UPDATE two_a.globals SET gl_value=\'1\' WHERE gl_name=\'rest_fhir_api\';\n UPDATE two_a.globals SET gl_value=\'1\' WHERE gl_name=\'rest_portal_api\';\n UPDATE two_a.globals SET gl_value=\'3\' WHERE gl_name=\'oauth_password_grant\';\n UPDATE two_a.globals SET gl_value=\'1\' WHERE gl_name=\'rest_system_scopes_api\';\n UPDATE two_a.globals SET gl_value=\'3\' WHERE gl_name=\'ccda_alt_service_enable\';\n '
25+
ACTION: chmod 644 <WORK>/web/a/openemr/sites/default/sqlconf.php
26+
ACTION: mkdir <WORK>/web/a/openemr/sites/default/procedure_results
27+
ACTION: chmod -R a+w <WORK>/web/a/openemr/sites/default/procedure_results
28+
ACTION: sed -i s@/apis/default/@/a/openemr/apis/default/@ <WORK>/web/a/openemr/swagger/openemr-api.yaml
29+
ACTION: sed -i s@/oauth2/default/authorize@/a/openemr/oauth2/default/authorize@ <WORK>/web/a/openemr/swagger/openemr-api.yaml
30+
ACTION: sed -i s@/oauth2/default/token@/a/openemr/oauth2/default/token@ <WORK>/web/a/openemr/swagger/openemr-api.yaml
31+
ACTION: rm -f <WORK>/web/a/openemr/library/openflashchart/php-ofc-library/ofc_upload_image.php

0 commit comments

Comments
 (0)