From 674e960cea4db99e65f6b1a1701ced4380e21e6f Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:38:39 +0000 Subject: [PATCH] Protect host PHP controls from container writes --- .github/workflows/build.yml | 1 + docker/php-ci/README.md | 9 +++ scripts/ci/run-php-container.sh | 21 +++++- scripts/ci/setup-php-container.sh | 7 +- scripts/ci/test-build-workflow.py | 3 + scripts/ci/test-php-container-boundary.py | 80 +++++++++++++++++++++++ scripts/ci/test-php-container-launcher.py | 40 ++++++++++-- scripts/ci/test-php-container-tls.py | 2 +- 8 files changed, 152 insertions(+), 11 deletions(-) create mode 100644 scripts/ci/test-php-container-boundary.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4e6f678..bfd48b1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -46,6 +46,7 @@ jobs: test: name: Test (PHP ${{ matrix.php }}) + needs: [action-policy] if: ${{ github.server_url == 'https://github.com' }} runs-on: ubuntu-24.04 timeout-minutes: 15 diff --git a/docker/php-ci/README.md b/docker/php-ci/README.md index 7530d3e..681ad12 100644 --- a/docker/php-ci/README.md +++ b/docker/php-ci/README.md @@ -23,3 +23,12 @@ launchers. Those launchers forward the selected CA files as read-only mounts and keep unrelated host credentials outside the runtime. The caller's normal Composer verifier still checks the actual container PHAR before dependency requests and again before project scripts. + +The repository and launcher state are mounted read-only. Separate writable +mounts contain `vendor`, PHPUnit's result-cache data, the private home, +Composer state and temporary files. The launcher refuses symlinked writable +mount roots. Application code cannot replace a host wrapper or its source +through those mounts. A disposable-source control attempts direct launcher +writes, wrapper unlink/replacement and rename, then invokes the next host PHP +entry. The container test cells depend on the accepted central policy before +executing a changed runtime source. diff --git a/scripts/ci/run-php-container.sh b/scripts/ci/run-php-container.sh index f09cc5b..2ef254e 100755 --- a/scripts/ci/run-php-container.sh +++ b/scripts/ci/run-php-container.sh @@ -16,8 +16,25 @@ options=(--rm --init --interactive --name "cli-php-ci-${command}-${BASHPID}" --cap-drop ALL --security-opt no-new-privileges --user "$(id -u):$(id -g)" --workdir "$PWD" --env "HOME=$state/home" - --mount "type=bind,source=$root,target=$root" - --mount "type=bind,source=$state,target=$state") + --mount "type=bind,source=$root,target=$root,readonly" + --mount "type=bind,source=$state,target=$state,readonly") + +# Keep host-executed launchers and controls read-only. Only dependency data, +# test output and private runtime caches can be changed by application code. +for directory in "$root/vendor" "$state/home" "$state/tmp" "$state/composer-home"; do + if [[ ! -d "$directory" || -L "$directory" || "$(realpath "$directory")" != "$directory" ]]; then + echo 'PHP CI writable directories must be existing real paths.' >&2 + exit 1 + fi + options+=(--mount "type=bind,source=$directory,target=$directory") +done +for cache in "$root/.phpunit.result.cache" "$state/phpunit-result.cache"; do + if [[ ! -f "$cache" || -L "$cache" || "$(realpath "$cache")" != "$cache" ]]; then + echo 'PHP CI result cache must be an existing real file.' >&2 + exit 1 + fi +done +options+=(--mount "type=bind,source=$state/phpunit-result.cache,target=$root/.phpunit.result.cache") # The Composer verifier is a composite action outside the checkout. if [[ -n "${GITHUB_ACTION_PATH:-}" ]]; then diff --git a/scripts/ci/setup-php-container.sh b/scripts/ci/setup-php-container.sh index 38fb99c..9710357 100755 --- a/scripts/ci/setup-php-container.sh +++ b/scripts/ci/setup-php-container.sh @@ -38,11 +38,16 @@ docker run --rm --init --name "cli-php-ci-probe-${minor}-${GITHUB_RUN_ID:-local} printf 'php_ci_image=%s\n' "$runtime_id" state="$(mktemp -d "${RUNNER_TEMP:?RUNNER_TEMP is required}/cli-php-ci.XXXXXX")" -mkdir -p "$state/bin" "$state/home" "$state/tmp" "$state/composer-home/cache" +mkdir -p "$root/vendor" "$state/bin" "$state/home" "$state/tmp" "$state/composer-home/cache" +[[ ! -L "$root/vendor" && ! -L "$root/.phpunit.result.cache" ]] +[[ -e "$root/.phpunit.result.cache" ]] || touch "$root/.phpunit.result.cache" +touch "$state/phpunit-result.cache" ln -sf "$root/scripts/ci/run-php-container.sh" "$state/bin/php" ln -sf "$root/scripts/ci/run-php-container.sh" "$state/bin/composer" CLI_PHP_CI_TEST_IMAGE="$runtime_id" CLI_PHP_CI_ROOT="$root" CLI_PHP_CI_STATE="$state" \ python3 -I -S -B "$root/scripts/ci/test-php-container-tls.py" -v +CLI_PHP_CI_TEST_IMAGE="$runtime_id" CLI_PHP_CI_STATE="$state" \ + python3 -I -S -B "$root/scripts/ci/test-php-container-boundary.py" -v { echo "CLI_PHP_CI_IMAGE=$runtime_id" echo "CLI_PHP_CI_ROOT=$root" diff --git a/scripts/ci/test-build-workflow.py b/scripts/ci/test-build-workflow.py index 74974e6..ea9a4c7 100644 --- a/scripts/ci/test-build-workflow.py +++ b/scripts/ci/test-build-workflow.py @@ -26,6 +26,9 @@ def test_all_supported_php_versions_still_run_once(self): versions.append(setup['with']['php-version']) self.assertCountEqual(['8.2', '8.3', '8.4', '8.5'], versions) + def test_container_cells_require_accepted_policy_before_execution(self): + self.assertIn('action-policy', self.jobs['test']['needs']) + def test_required_gate_includes_every_job(self): self.assertEqual(set(self.jobs) - {'target-branch-qualification'}, set(self.gate['needs'])) guarded = set() diff --git a/scripts/ci/test-php-container-boundary.py b/scripts/ci/test-php-container-boundary.py new file mode 100644 index 0000000..b79d8ed --- /dev/null +++ b/scripts/ci/test-php-container-boundary.py @@ -0,0 +1,80 @@ +"""Exercise host-control replacement attempts in disposable source and state.""" +import hashlib +import json +import os +from pathlib import Path +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[2] +LAUNCHER = Path(os.environ.get('CLI_PHP_CI_TEST_LAUNCHER', ROOT / 'scripts/ci/run-php-container.sh')) + + +@unittest.skipUnless(os.environ.get('CLI_PHP_CI_TEST_IMAGE'), 'requires built PHP CI image') +class PhpContainerBoundaryTests(unittest.TestCase): + def test_application_cannot_replace_subsequent_host_controls(self): + for attack in ('launcher-overwrite', 'wrapper-unlink', 'wrapper-rename'): + with self.subTest(attack=attack), tempfile.TemporaryDirectory( + dir=Path(os.environ['CLI_PHP_CI_STATE']) / 'tmp') as directory: + temporary = Path(directory) + root, state = temporary / 'source', temporary / 'state' + scripts = root / 'scripts/ci' + scripts.mkdir(parents=True) + (root / 'vendor').mkdir() + (root / '.phpunit.result.cache').touch() + launcher = scripts / 'run-php-container.sh' + launcher.write_bytes(LAUNCHER.read_bytes()) + launcher.chmod(0o755) + subprocess.run(['git', 'init', '--quiet', str(root)], check=True) + for name in ('bin', 'home', 'tmp', 'composer-home/cache'): + (state / name).mkdir(parents=True, exist_ok=True) + (state / 'phpunit-result.cache').touch() + wrapper = state / 'bin/php' + wrapper.symlink_to(launcher) + digest = hashlib.sha256(launcher.read_bytes()).hexdigest() + environment = {'PATH': os.defpath, 'CLI_PHP_CI_ROOT': str(root), + 'CLI_PHP_CI_STATE': str(state), + 'CLI_PHP_CI_IMAGE': os.environ['CLI_PHP_CI_TEST_IMAGE'], + 'TMPDIR': str(state / 'tmp')} + code = r''' +$payload = "#!/bin/sh\nprintf '%s\\n' CLI_HOST_WRAPPER_REPLACED\n"; +[$unused, $attack, $launcher, $wrapper, $root, $state] = $argv; +if ($attack === 'launcher-overwrite') { + $changed = @file_put_contents($launcher, $payload) !== false; +} elseif ($attack === 'wrapper-unlink') { + $changed = @unlink($wrapper) && @file_put_contents($wrapper, $payload) !== false; +} else { + file_put_contents($state.'/tmp/replacement', $payload); + $changed = @rename($state.'/tmp/replacement', $wrapper); +} +if ($changed) { + @chmod($wrapper, 0755); +} +$writable = true; +foreach ([$root.'/vendor/output', $root.'/.phpunit.result.cache', + $state.'/home/output', $state.'/tmp/output', $state.'/composer-home/output'] as $path) { + $writable = $writable && file_put_contents($path, 'synthetic data') !== false; +} +echo json_encode(['control_replaced' => $changed, 'data_writable' => $writable]); +''' + attempt = subprocess.run([str(wrapper), '-r', code, '--', attack, str(launcher), + str(wrapper), str(root), str(state)], + cwd=root, env=environment, capture_output=True, text=True, timeout=30) + self.assertEqual(0, attempt.returncode, attempt.stderr) + result = json.loads(attempt.stdout) + # Invoke the next nominal host PHP entry even in the predecessor + # control, so wrapper substitution has an observable marker. + following = subprocess.run([str(wrapper), '-r', 'echo "CLI_CONTAINER_PHP";'], + cwd=root, env=environment, capture_output=True, text=True, timeout=30) + self.assertFalse(result['control_replaced'], + f'{attack}: next host entry returned {following.stdout!r}') + self.assertTrue(result['data_writable'], 'Necessary dependency/cache outputs must still work') + self.assertEqual(digest, hashlib.sha256(launcher.read_bytes()).hexdigest()) + self.assertTrue(wrapper.is_symlink()) + self.assertEqual(0, following.returncode, following.stderr) + self.assertEqual('CLI_CONTAINER_PHP', following.stdout) + + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/ci/test-php-container-launcher.py b/scripts/ci/test-php-container-launcher.py index 5a7b62c..a4c70fa 100644 --- a/scripts/ci/test-php-container-launcher.py +++ b/scripts/ci/test-php-container-launcher.py @@ -56,10 +56,24 @@ def test_workspace_python_imports_do_not_run_before_apt_source_guard(self): class PhpContainerLauncherTest(unittest.TestCase): - def invoke(self, command='php', arguments=(), **overrides): + def invoke(self, command='php', arguments=(), symlink_data=None, **overrides): with tempfile.TemporaryDirectory() as directory: state = Path(directory) + root = state / 'source' + (root / 'vendor').mkdir(parents=True) + (root / '.phpunit.result.cache').touch() + (state / 'phpunit-result.cache').touch() + subprocess.run(['git', 'init', '--quiet', str(root)], check=True) (state / 'bin').mkdir() + for name in ['home', 'tmp', 'composer-home']: + (state / name).mkdir() + if symlink_data: + selected = root / 'vendor' if symlink_data == 'vendor' else state / symlink_data + if selected.is_dir(): + selected.rmdir() + else: + selected.unlink() + selected.symlink_to(root) (state / 'bin' / command).symlink_to(LAUNCHER) docker = state / 'bin/docker' docker.write_text('#!/usr/bin/env python3\nimport json,sys\nprint(json.dumps(sys.argv[1:]))\n') @@ -68,7 +82,7 @@ def invoke(self, command='php', arguments=(), **overrides): bundle.write_text('synthetic mount only') environment = { 'PATH': str(state / 'bin') + os.pathsep + os.defpath, - 'CLI_PHP_CI_ROOT': str(ROOT), + 'CLI_PHP_CI_ROOT': str(root), 'CLI_PHP_CI_STATE': str(state), 'CLI_PHP_CI_IMAGE': 'sha256:' + 'f' * 64, 'SSL_CERT_FILE': str(bundle), @@ -85,10 +99,10 @@ def invoke(self, command='php', arguments=(), **overrides): arguments = [str(state / 'bin/composer') if x == '{composer}' else x for x in arguments] result = subprocess.run([str(state / 'bin' / command), *arguments], cwd=ROOT, env=environment, capture_output=True, text=True) - return result, str(bundle), str(state) + return result, str(bundle), str(state), str(root) def test_credentials_and_php_overrides_do_not_enter_runtime(self): - result, bundle, state = self.invoke(arguments=('-r', 'echo PHP_VERSION;')) + result, bundle, state, root = self.invoke(arguments=('-r', 'echo PHP_VERSION;')) self.assertEqual(0, result.returncode, result.stderr) invocation = json.loads(result.stdout) passed = [invocation[i + 1] for i, part in enumerate(invocation[:-1]) if part == '--env'] @@ -99,13 +113,18 @@ def test_credentials_and_php_overrides_do_not_enter_runtime(self): self.assertIn('HOME=' + state + '/home', passed) mounts = [invocation[i + 1] for i, part in enumerate(invocation[:-1]) if part == '--mount'] self.assertEqual(1, mounts.count(f'type=bind,source={bundle},target={bundle},readonly')) + self.assertIn(f'type=bind,source={root},target={root},readonly', mounts) + self.assertIn(f'type=bind,source={state},target={state},readonly', mounts) + self.assertIn(f'type=bind,source={root}/vendor,target={root}/vendor', mounts) + for directory in ['home', 'tmp', 'composer-home']: + self.assertIn(f'type=bind,source={state}/{directory},target={state}/{directory}', mounts) self.assertFalse(any('docker.sock' in mount for mount in mounts)) self.assertIn('--cap-drop', invocation) self.assertIn('no-new-privileges', invocation) self.assertEqual(['php', '-r', 'echo PHP_VERSION;'], invocation[-3:]) def test_composer_verifier_reads_container_phar(self): - result, _, _ = self.invoke(arguments=('/guard/verify.php', '{composer}', str(ROOT)), + result, _, _, _ = self.invoke(arguments=('/guard/verify.php', '{composer}', str(ROOT)), GITHUB_ACTION_PATH='/guard') self.assertEqual(0, result.returncode, result.stderr) invocation = json.loads(result.stdout) @@ -113,12 +132,19 @@ def test_composer_verifier_reads_container_phar(self): self.assertIn('type=bind,source=/guard,target=/guard,readonly', invocation) def test_composer_uses_container_executable(self): - result, _, _ = self.invoke(command='composer', arguments=('test',)) + result, _, _, _ = self.invoke(command='composer', arguments=('test',)) self.assertEqual(0, result.returncode, result.stderr) self.assertEqual(['composer', 'test'], json.loads(result.stdout)[-2:]) + def test_symlinked_writable_mounts_refuse_before_docker(self): + for selected in ['vendor', 'home', 'tmp', 'composer-home', 'phpunit-result.cache']: + with self.subTest(selected=selected): + result, _, _, _ = self.invoke(symlink_data=selected) + self.assertNotEqual(0, result.returncode) + self.assertEqual('', result.stdout) + def test_mutable_image_refuses_before_docker(self): - result, _, _ = self.invoke(CLI_PHP_CI_IMAGE='cli-php-ci:mutable') + result, _, _, _ = self.invoke(CLI_PHP_CI_IMAGE='cli-php-ci:mutable') self.assertNotEqual(0, result.returncode) self.assertIn('inspected immutable image config ID', result.stderr) self.assertEqual('', result.stdout) diff --git a/scripts/ci/test-php-container-tls.py b/scripts/ci/test-php-container-tls.py index 7095592..14eb684 100644 --- a/scripts/ci/test-php-container-tls.py +++ b/scripts/ci/test-php-container-tls.py @@ -31,7 +31,7 @@ def do_GET(self): def log_message(self, *_arguments): pass - with tempfile.TemporaryDirectory(dir=state) as directory: + with tempfile.TemporaryDirectory(dir=state / "tmp") as directory: project = pathlib.Path(directory) def openssl(*arguments):