diff --git a/.env b/.env index c0b38c1..796b445 100644 --- a/.env +++ b/.env @@ -23,6 +23,3 @@ GOOGLE_ANALYTICS=foo REDIS_HOST=redis REDIS_PORT=6379 - -# Required for https redirects to correctly work behind a reverse proxy / load balancer -TRUSTED_PROXIES=127.0.0.1,10.0.0.0/8,172.16.0.0/12 diff --git a/.env.test b/.env.test index 9e7162f..9f7baf3 100644 --- a/.env.test +++ b/.env.test @@ -2,5 +2,8 @@ KERNEL_CLASS='App\Kernel' APP_SECRET='$ecretf0rt3st' SYMFONY_DEPRECATIONS_HELPER=999999 + +# Trusted proxy used by functional tests exercising forwarded client IPs. +SYMFONY_TRUSTED_PROXIES=192.0.2.0/24 PANTHER_APP_ENV=panther PANTHER_ERROR_SCREENSHOT_DIR=./var/error-screenshots diff --git a/.github/workflows/build-containers.yaml b/.github/workflows/build-containers.yaml index 2a087e7..904ff37 100644 --- a/.github/workflows/build-containers.yaml +++ b/.github/workflows/build-containers.yaml @@ -25,15 +25,18 @@ jobs: if: ${{ github.ref == 'refs/heads/master' }} runs-on: ubuntu-latest + permissions: + contents: read + steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4 - name: Login to DockerHub - uses: docker/login-action@v4 + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4 with: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} diff --git a/.github/workflows/cleanup-repo.yaml b/.github/workflows/cleanup-repo.yaml index 1a44be6..1bf2b70 100644 --- a/.github/workflows/cleanup-repo.yaml +++ b/.github/workflows/cleanup-repo.yaml @@ -11,10 +11,15 @@ jobs: cleanup-repository: runs-on: ubuntu-latest + permissions: + contents: write + issues: write + pull-requests: write + steps: # Mark issues and PRs with no activity as stale after a while, and close them after a while longer - - uses: actions/stale@v10 + - uses: actions/stale@1e223db275d687790206a7acac4d1a11bd6fe629 # v10 with: stale-issue-message: 'Marking issue as stale' stale-pr-message: 'Marking PR as stale' @@ -26,7 +31,7 @@ jobs: # Delete old, abandoned branches # See what constitutes an abandoned branch here: https://github.com/phpdocker-io/github-actions-delete-abandoned-branches - - uses: phpdocker-io/github-actions-delete-abandoned-branches@v1 + - uses: phpdocker-io/github-actions-delete-abandoned-branches@d8e3635360bd492315571df30f15f55c98a28434 # v1 with: github_token: ${{ github.token }} last_commit_age_days: 30 diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 9a5e4f7..3ffef61 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -15,17 +15,19 @@ jobs: tests: timeout-minutes: 10 runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Get Composer Cache Directory id: composer-cache run: | echo "dir=$(make composer-cache-dir)" >> $GITHUB_OUTPUT - - uses: actions/cache@v5 + - uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} diff --git a/Makefile b/Makefile index fbfa7ea..9e1fe6c 100644 --- a/Makefile +++ b/Makefile @@ -6,15 +6,39 @@ HOSTS_LOCATION=bin/hosts SITE_HOST=phpdocker.local PHP_RUN=docker compose run -e XDEBUG_MODE=coverage --rm php-fpm +# Integrity data for the binaries downloaded below. Upstream publishes no checksum +# for the mkcert v1.4.3 release assets (the GitHub release API reports +# "digest": null for each of them), so these SHA-256 values were computed locally +# from the assets downloaded over HTTPS from the official release URLs on +# 2026-09-25. The windows-amd64.exe value is independently corroborated by the +# mkcert 1.4.3 Chocolatey package (tools/mkcert.exe, published 2020-11-26), which +# contains the same bytes. hosts is fetched from the commit the 3.6.4 tag points at +# (9a929dc70fa11bfe6dc5b0f1d53aea442395edd3) and was hashed on 2026-09-25. If an +# asset is ever republished, update its URL and its hash in the same commit. +MKCERT_SHA256_linux-amd64=c2b0746528588d2a5dabe7c4394a848909da07e23ca3f2393375e9baa3931649 +MKCERT_SHA256_darwin-amd64=0b5bd40ea69ec34c567707249938bcd0502d2c3efc0137143a076a2b80d5e882 +MKCERT_SHA256_linux-arm=b982ade61b6781f17afc210914116d8078af3ebb631facd28a944033018d41d2 +MKCERT_SHA256_linux-arm64=43c4e3b9e7e6466d397b3d6e221788f83b5b91f826f1040240dbaddfc101ce33 +MKCERT_SHA256_windows-amd64.exe=9dc25f7d1ae0be93db81aa42f3abfd62d13725dfd48969c9fe94b6af57e5573c +HOSTS_COMMIT=9a929dc70fa11bfe6dc5b0f1d53aea442395edd3 +HOSTS_SHA256=eee51960ec8dd30e00090779ba79f11410396e69ac7812b0ad99f5b597c8c36e + +# sha256sum on Linux, shasum on macOS +SHA256_CMD=$(shell command -v sha256sum >/dev/null 2>&1 && echo sha256sum || echo 'shasum -a 256') + INFECTION_THREADS?=8 BUILD_TAG?:=$(shell date +'%Y-%m-%d-%H-%M-%S')-$(shell git rev-parse --short HEAD) -# linux-amd64, darwin-amd64, linux-arm +# linux-amd64, darwin-amd64, linux-arm, linux-arm64 # On windows, override with windows-amd64.exe ifndef BINARY_SUFFIX BINARY_SUFFIX:=$(shell [[ "`uname -s`" == "Linux" ]] && echo linux || echo darwin)-amd64 endif +# Resolved from BINARY_SUFFIX; an unsupported suffix yields an empty value, which +# fails verification instead of skipping it. +MKCERT_SHA256=$(MKCERT_SHA256_$(BINARY_SUFFIX)) + ifndef BUILD_TAG BUILD_TAG:=$(shell date +'%Y-%m-%d-%H-%M-%S')-$(shell git rev-parse --short HEAD) endif @@ -85,19 +109,57 @@ composer-update: $(PHP_RUN) composer update --no-scripts make composer-install -install-mkcert: - @echo "Installing mkcert for OS type ${BINARY_SUFFIX}" - @if [[ ! -f '$(MKCERT_LOCATION)' ]]; then curl -sL 'https://github.com/FiloSottile/mkcert/releases/download/$(MKCERT_VERSION)/mkcert-$(MKCERT_VERSION)-$(BINARY_SUFFIX)' -o $(MKCERT_LOCATION); chmod +x $(MKCERT_LOCATION); fi; +install-mkcert: verify-mkcert bin/mkcert -install -create-certs: +download-mkcert: + @echo "Installing mkcert for OS type ${BINARY_SUFFIX}" + @if [[ ! -f '$(MKCERT_LOCATION)' ]]; then \ + curl -fsSL --retry 3 -o '$(MKCERT_LOCATION).tmp' 'https://github.com/FiloSottile/mkcert/releases/download/$(MKCERT_VERSION)/mkcert-$(MKCERT_VERSION)-$(BINARY_SUFFIX)' || { rm -f '$(MKCERT_LOCATION).tmp'; exit 1; }; \ + mv '$(MKCERT_LOCATION).tmp' '$(MKCERT_LOCATION)'; \ + fi + +verify-mkcert: download-mkcert + @if [[ ! -f '$(MKCERT_LOCATION)' ]]; then \ + echo "Missing $(MKCERT_LOCATION); refusing to run it"; \ + exit 1; \ + fi; \ + if [[ -z '$(MKCERT_SHA256)' ]]; then \ + echo "No pinned SHA-256 for BINARY_SUFFIX '$(BINARY_SUFFIX)'; refusing to run $(MKCERT_LOCATION)"; \ + exit 1; \ + fi; \ + actual="$$($(SHA256_CMD) '$(MKCERT_LOCATION)' | awk '{print $$1}')"; \ + if [[ "$$actual" != '$(MKCERT_SHA256)' ]]; then \ + echo "SHA-256 mismatch for $(MKCERT_LOCATION): expected '$(MKCERT_SHA256)', got '$$actual'"; \ + exit 1; \ + fi + chmod +x $(MKCERT_LOCATION) + +create-certs: verify-mkcert bin/mkcert -cert-file=infrastructure/local/localhost.pem -key-file=infrastructure/local/localhost-key.pem $(SITE_HOST) -install-hosts: - @echo "Installing hosts script" - @if [[ ! -f '$(HOSTS_LOCATION)' ]]; then curl -sL 'https://raw.githubusercontent.com/xwmx/hosts/$(HOSTS_VERSION)/hosts' -o $(HOSTS_LOCATION); chmod +x $(HOSTS_LOCATION); fi; - -clean-hosts: +install-hosts: verify-hosts + +download-hosts: + @echo "Installing hosts script ($(HOSTS_VERSION))" + @if [[ ! -f '$(HOSTS_LOCATION)' ]]; then \ + curl -fsSL --retry 3 -o '$(HOSTS_LOCATION).tmp' 'https://raw.githubusercontent.com/xwmx/hosts/$(HOSTS_COMMIT)/hosts' || { rm -f '$(HOSTS_LOCATION).tmp'; exit 1; }; \ + mv '$(HOSTS_LOCATION).tmp' '$(HOSTS_LOCATION)'; \ + fi + +verify-hosts: download-hosts + @if [[ ! -f '$(HOSTS_LOCATION)' ]]; then \ + echo "Missing $(HOSTS_LOCATION); refusing to run it"; \ + exit 1; \ + fi; \ + actual="$$($(SHA256_CMD) '$(HOSTS_LOCATION)' | awk '{print $$1}')"; \ + if [[ "$$actual" != '$(HOSTS_SHA256)' ]]; then \ + echo "SHA-256 mismatch for $(HOSTS_LOCATION): expected '$(HOSTS_SHA256)', got '$$actual'"; \ + exit 1; \ + fi + chmod +x $(HOSTS_LOCATION) + +clean-hosts: verify-hosts sudo bin/hosts remove --force *$(SITE_HOST) > /dev/null 2>&1 || exit 0 init-hosts: clean-hosts diff --git a/composer.json b/composer.json index 8829307..5014e7a 100644 --- a/composer.json +++ b/composer.json @@ -14,9 +14,10 @@ "symfony/dotenv": "^8.0", "symfony/flex": "^2.4", "symfony/form": "^8.0", - "symfony/framework-bundle": "^8.0", + "symfony/framework-bundle": "^8.1", "symfony/mime": "^8.0", "symfony/monolog-bundle": "^4.0", + "symfony/rate-limiter": "^8.0", "symfony/runtime": "^8.0", "symfony/security-csrf": "^8.0", "symfony/twig-bundle": "^8.0", diff --git a/composer.lock b/composer.lock index b9bf1bc..dd3b76b 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "283de4b0f26712d589b3e88c11e08508", + "content-hash": "da92f28a7de2e1aa75b04e145ce31632", "packages": [ { "name": "michelf/php-markdown", @@ -3071,6 +3071,80 @@ ], "time": "2026-09-04T10:14:04+00:00" }, + { + "name": "symfony/rate-limiter", + "version": "v8.1.6", + "source": { + "type": "git", + "url": "https://github.com/symfony/rate-limiter.git", + "reference": "dee2fc983ca9944ac37af105a4940507ff5fc42c" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/rate-limiter/zipball/dee2fc983ca9944ac37af105a4940507ff5fc42c", + "reference": "dee2fc983ca9944ac37af105a4940507ff5fc42c", + "shasum": "" + }, + "require": { + "php": ">=8.4.1", + "symfony/options-resolver": "^7.4|^8.0" + }, + "require-dev": { + "psr/cache": "^1.0|^2.0|^3.0", + "symfony/lock": "^7.4|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\RateLimiter\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Wouter de Jong", + "email": "wouter@wouterj.nl" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides a Token Bucket implementation to rate limit input and output in your application", + "homepage": "https://symfony.com", + "keywords": [ + "limiter", + "rate-limiter" + ], + "support": { + "source": "https://github.com/symfony/rate-limiter/tree/v8.1.6" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-08-07T15:35:35+00:00" + }, { "name": "symfony/routing", "version": "v8.1.6", @@ -6855,5 +6929,5 @@ "ext-zip": "*" }, "platform-dev": {}, - "plugin-api-version": "2.9.0" + "plugin-api-version": "2.6.0" } diff --git a/config/packages/cache.yaml b/config/packages/cache.yaml index 9e407cc..20874f5 100644 --- a/config/packages/cache.yaml +++ b/config/packages/cache.yaml @@ -1,3 +1,14 @@ framework: cache: app: cache.adapter.apcu + default_redis_provider: 'redis://%env(REDIS_HOST)%:%env(REDIS_PORT)%' + pools: + rate_limiter.cache: + adapter: cache.adapter.redis + +when@test: + framework: + cache: + pools: + rate_limiter.cache: + adapter: cache.adapter.filesystem diff --git a/config/packages/framework.yaml b/config/packages/framework.yaml index bbd6107..549331f 100644 --- a/config/packages/framework.yaml +++ b/config/packages/framework.yaml @@ -13,6 +13,14 @@ framework: cookie_secure: auto cookie_samesite: strict + # Only these forwarded headers are honoured, and only when the request + # arrives from a proxy listed in SYMFONY_TRUSTED_PROXIES (Symfony's default + # env var, left unset here so no proxy is trusted by default). + trusted_headers: + - x-forwarded-for + - x-forwarded-proto + - x-forwarded-port + #esi: true #fragments: true php_errors: diff --git a/config/packages/rate_limiter.yaml b/config/packages/rate_limiter.yaml new file mode 100644 index 0000000..02a78c2 --- /dev/null +++ b/config/packages/rate_limiter.yaml @@ -0,0 +1,13 @@ +framework: + rate_limiter: + generator: + policy: 'fixed_window' + limit: 30 + interval: '1 minute' + cache_pool: 'rate_limiter.cache' + +when@test: + framework: + rate_limiter: + generator: + limit: 3 diff --git a/src/Assert/Path.php b/src/Assert/Path.php new file mode 100644 index 0000000..16ca3b5 --- /dev/null +++ b/src/Assert/Path.php @@ -0,0 +1,58 @@ + + */ + public static function getRegexes(): array + { + return [ + self::ABSOLUTE_DIR => '\A/[A-Za-z0-9._-]+(?:/[A-Za-z0-9._-]+)*/?\z', + self::RELATIVE_PHP => '\A[A-Za-z0-9._-]+(?:/[A-Za-z0-9._-]+)*\.php\z', + self::HOST_PATH => '\A[A-Za-z0-9._/-]*[A-Za-z0-9._-][A-Za-z0-9._/-]*\z', + ]; + } +} diff --git a/src/Assert/PathValidator.php b/src/Assert/PathValidator.php new file mode 100644 index 0000000..7072b0d --- /dev/null +++ b/src/Assert/PathValidator.php @@ -0,0 +1,51 @@ +type] ?? null; + $segments = explode('/', $value); + + if ($regex === null || in_array('..', $segments, true) || preg_match('#' . $regex . '#', $value) !== 1) { + $this + ->context + ->buildViolation($constraint->message) + ->setParameter('{{ value }}', $this->formatValue($value)) + ->addViolation(); + } + } +} diff --git a/src/Controller/GeneratorController.php b/src/Controller/GeneratorController.php index 8be26dc..9e31056 100644 --- a/src/Controller/GeneratorController.php +++ b/src/Controller/GeneratorController.php @@ -27,6 +27,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\ResponseHeaderBag; +use Symfony\Component\HttpKernel\Attribute\RateLimit; /** * Docker environment generator controller. @@ -43,6 +44,7 @@ public function __construct( /** * Form and form processor for creating a project. */ + #[RateLimit('generator', methods: ['POST'])] public function create(Request $request): BinaryFileResponse|Response { $form = $this->createForm(type: ProjectType::class, options: ['method' => Request::METHOD_POST]); diff --git a/src/Form/Generator/GlobalOptionsType.php b/src/Form/Generator/GlobalOptionsType.php index b639b6e..69e2259 100644 --- a/src/Form/Generator/GlobalOptionsType.php +++ b/src/Form/Generator/GlobalOptionsType.php @@ -3,9 +3,11 @@ namespace App\Form\Generator; +use App\Assert\Path; use Symfony\Component\Form\Extension\Core\Type\IntegerType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\Validator\Constraints\Length; use Symfony\Component\Validator\Constraints\NotBlank; use Symfony\Component\Validator\Constraints\Range; use Symfony\Component\Validator\Constraints\Type; @@ -26,12 +28,22 @@ public function buildForm(FormBuilderInterface $builder, array $options): void ], ]) ->add('appPath', TextType::class, [ - 'label' => 'Your source code\'s path', - 'data' => '.', + 'label' => 'Your source code\'s path', + 'data' => '.', + 'constraints' => [ + new NotBlank(), + new Length(max: 255), + new Path(type: Path::HOST_PATH), + ], ]) ->add('dockerWorkingDir', TextType::class, [ - 'label' => 'Containers workdir', - 'data' => '/application', + 'label' => 'Containers workdir', + 'data' => '/application', + 'constraints' => [ + new NotBlank(), + new Length(max: 255), + new Path(type: Path::ABSOLUTE_DIR), + ], ]); } diff --git a/src/Form/Generator/PhpType.php b/src/Form/Generator/PhpType.php index 462c6e0..0e196e6 100644 --- a/src/Form/Generator/PhpType.php +++ b/src/Form/Generator/PhpType.php @@ -19,6 +19,7 @@ namespace App\Form\Generator; +use App\Assert\Path; use App\PHPDocker\PhpExtension\AvailableExtensionsFactory; use App\PHPDocker\PhpExtension\PhpExtension; use App\PHPDocker\Project\ServiceOptions\Php; @@ -53,6 +54,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'constraints' => [ new NotBlank(), new Length(min: 2, max: 128), + new Path(type: Path::RELATIVE_PHP), ], ]) ->add('hasGit', CheckboxType::class, [ diff --git a/src/PHPDocker/Zip/Archiver.php b/src/PHPDocker/Zip/Archiver.php index 34ee572..1c318d9 100644 --- a/src/PHPDocker/Zip/Archiver.php +++ b/src/PHPDocker/Zip/Archiver.php @@ -32,13 +32,33 @@ final class Archiver /** * Initialise Zip File via the zip PECL extension into a temporary file on local storage. + * + * @throws Exception\ArchiveNotCreatedException */ public function __construct(private readonly string $baseFolder = '') { $this->zipFile = new ZipArchive(); $zipFilename = tempnam(sys_get_temp_dir(), str_replace('\\', '_', self::class)); - $this->zipFile->open(sprintf('%s.zip', $zipFilename), ZipArchive::CREATE); + + if ($zipFilename === false) { + throw new Exception\ArchiveNotCreatedException('Could not create temporary file for archive'); + } + + // tempnam() creates a zero-byte placeholder, but the archive lives at ${base}.zip, so remove it. + if (unlink($zipFilename) === false) { + throw new Exception\ArchiveNotCreatedException('Could not remove temporary file placeholder for archive'); + } + + $zipPath = sprintf('%s.zip', $zipFilename); + + if ($this->zipFile->open($zipPath, ZipArchive::CREATE) !== true) { + if (file_exists($zipPath)) { + unlink($zipPath); + } + + throw new Exception\ArchiveNotCreatedException('Archive creation failed for an unknown reason'); + } } /** diff --git a/tests/Functional/GeneratorTest.php b/tests/Functional/GeneratorTest.php index 8860639..194bbef 100644 --- a/tests/Functional/GeneratorTest.php +++ b/tests/Functional/GeneratorTest.php @@ -33,10 +33,24 @@ public function setUp(): void { parent::setUp(); - $this->client = static::createClient(options: [ - 'environment' => 'test', - 'debug' => false, - ]); + // The rate limiter keys requests by client IP, and its cache pool + // survives kernel resets. Give each test a unique, per-run IP so limiter + // state cannot leak between tests or test runs, and so tests never need + // to reach into the limiter's cache pool directly. + $clientIp = sprintf( + '10.%d.%d.%d', + random_int(0, 255), + random_int(0, 255), + random_int(1, 254), + ); + + $this->client = static::createClient( + options: [ + 'environment' => 'test', + 'debug' => false, + ], + server: ['REMOTE_ADDR' => $clientIp], + ); } #[Test] @@ -185,6 +199,148 @@ public function testPortOffsetsRespectCustomBasePort(): void self::assertStringContainsString('3004', $dockerCompose); // Postgres offset +4 } + #[Test] + public function testInvalidAppPathIsRejected(): void + { + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '8000', + 'project[globalOptions][appPath]' => '/var/www/my app', + ]); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('This value is not a valid path', (string) $this->client->getResponse()->getContent()); + } + + #[Test] + public function testSlashOnlyAppPathIsRejected(): void + { + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '8000', + 'project[globalOptions][appPath]' => '/', + ]); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('This value is not a valid path', (string) $this->client->getResponse()->getContent()); + } + + #[Test] + public function testInvalidDockerWorkingDirIsRejected(): void + { + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '8000', + 'project[globalOptions][dockerWorkingDir]' => 'relative/dir', + ]); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('This value is not a valid path', (string) $this->client->getResponse()->getContent()); + } + + #[Test] + public function testInvalidFrontControllerPathIsRejected(): void + { + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '8000', + 'project[phpOptions][frontControllerPath]' => 'public/index', + ]); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('This value is not a valid path', (string) $this->client->getResponse()->getContent()); + } + + #[Test] + public function testOversizedAppPathIsRejected(): void + { + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '8000', + 'project[globalOptions][appPath]' => str_repeat('a', 256), + ]); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('This value is too long', (string) $this->client->getResponse()->getContent()); + } + + #[Test] + public function testMaxLengthAppPathIsAccepted(): void + { + $this->generateAndGetZip([ + 'project[globalOptions][basePort]' => '8000', + 'project[globalOptions][appPath]' => str_repeat('a', 255), + ]); + } + + #[Test] + public function testMaxLengthDockerWorkingDirIsAccepted(): void + { + $this->generateAndGetZip([ + 'project[globalOptions][basePort]' => '8000', + 'project[globalOptions][dockerWorkingDir]' => '/' . str_repeat('a', 254), + ]); + } + + #[Test] + public function testOversizedDockerWorkingDirIsRejected(): void + { + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '8000', + 'project[globalOptions][dockerWorkingDir]' => '/' . str_repeat('a', 255), + ]); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('This value is too long', (string) $this->client->getResponse()->getContent()); + } + + #[Test] + public function testMaxLengthFrontControllerPathIsAccepted(): void + { + $this->generateAndGetZip([ + 'project[globalOptions][basePort]' => '8000', + 'project[phpOptions][frontControllerPath]' => str_repeat('a', 124) . '.php', + ]); + } + + #[Test] + public function testOversizedFrontControllerPathIsRejected(): void + { + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '8000', + 'project[phpOptions][frontControllerPath]' => str_repeat('a', 125) . '.php', + ]); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('This value is too long', (string) $this->client->getResponse()->getContent()); + } + + #[Test] + public function testPostGenerationIsRateLimited(): void + { + $this->client->disableReboot(); + + // The limiter counts every POST, valid or not, so keep the first three + // submissions invalid to avoid generating multiple archives with the + // shared, single-use Archiver instance. + for ($i = 1; $i <= 3; ++$i) { + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '', + ]); + self::assertResponseIsSuccessful(); + } + + $this->client->request('GET', '/'); + $this->client->submitForm('Generate project archive', [ + 'project[globalOptions][basePort]' => '8000', + ]); + + self::assertResponseStatusCodeSame(429); + } + private function generateAndGetZip(array $formData): void { $this->client->request('GET', '/'); diff --git a/tests/Functional/TrustedProxyTest.php b/tests/Functional/TrustedProxyTest.php new file mode 100644 index 0000000..a4f2851 --- /dev/null +++ b/tests/Functional/TrustedProxyTest.php @@ -0,0 +1,138 @@ +createClientWithRemoteAddr(self::TRUSTED_PEER); + + // The limiter cache pool survives kernel resets and test runs, so use + // unique client IPs to avoid collisions with other tests or runs. + $clientA = $this->randomClientIp(); + $clientB = $this->randomClientIp(); + self::assertNotSame($clientA, $clientB); + + for ($i = 1; $i <= 3; ++$i) { + $this->submitGeneration(clientIp: $clientA); + self::assertResponseIsSuccessful(); + } + + // Fourth request from the same forwarded client exceeds the limit. + $this->submitGeneration(clientIp: $clientA); + self::assertResponseStatusCodeSame(429); + + // A different forwarded client through the same trusted peer has its + // own bucket and is still allowed. + $this->submitGeneration(clientIp: $clientB); + self::assertResponseIsSuccessful(); + } + + #[Test] + public function untrustedPeerCannotSpoofForwardedClientToEvadeLimit(): void + { + $this->createClientWithRemoteAddr($this->randomUntrustedIp()); + + // Spoof a different X-Forwarded-For on every request. As the peer is not + // trusted, the limiter must key on REMOTE_ADDR, so all four requests + // share one bucket and the fourth is rejected. + for ($i = 1; $i <= 3; ++$i) { + $this->submitGeneration(clientIp: $this->randomClientIp()); + self::assertResponseIsSuccessful(); + } + + $this->submitGeneration(clientIp: $this->randomClientIp()); + self::assertResponseStatusCodeSame(429); + } + + #[Test] + public function onlyForwardedForProtoAndPortAreTrusted(): void + { + $this->createClientWithRemoteAddr(self::TRUSTED_PEER); + + self::assertSame(['192.0.2.0/24'], Request::getTrustedProxies()); + self::assertSame( + Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_PROTO | Request::HEADER_X_FORWARDED_PORT, + Request::getTrustedHeaderSet(), + ); + } + + private function createClientWithRemoteAddr(string $remoteAddr): void + { + $this->client = static::createClient( + options: [ + 'environment' => 'test', + 'debug' => false, + ], + server: ['REMOTE_ADDR' => $remoteAddr], + ); + } + + /** + * Performs a valid, CSRF-protected generation POST claiming the given client IP. + */ + private function submitGeneration(string $clientIp): void + { + $this->client->request('GET', '/'); + $this->client->submitForm( + 'Generate project archive', + ['project[globalOptions][basePort]' => '8000'], + serverParameters: ['HTTP_X_FORWARDED_FOR' => $clientIp], + ); + } + + private function randomClientIp(): string + { + return sprintf('198.51.100.%d', random_int(1, 254)); + } + + private function randomUntrustedIp(): string + { + return sprintf('203.0.113.%d', random_int(1, 254)); + } +} diff --git a/tests/Unit/Assert/PathValidatorTest.php b/tests/Unit/Assert/PathValidatorTest.php new file mode 100644 index 0000000..94d4f9c --- /dev/null +++ b/tests/Unit/Assert/PathValidatorTest.php @@ -0,0 +1,107 @@ + + */ +class PathValidatorTest extends ConstraintValidatorTestCase +{ + protected function createValidator(): ConstraintValidatorInterface + { + return new PathValidator(); + } + + /** + * @return array + */ + public static function validPathsProvider(): array + { + return [ + 'absolute dir' => [Path::ABSOLUTE_DIR, '/application'], + 'absolute nested' => [Path::ABSOLUTE_DIR, '/var/www/myapp'], + 'relative php' => [Path::RELATIVE_PHP, 'public/index.php'], + 'relative php nested' => [Path::RELATIVE_PHP, 'app/src/index.php'], + 'host path dot' => [Path::HOST_PATH, '.'], + 'host path relative' => [Path::HOST_PATH, 'src/app'], + 'host path absolute' => [Path::HOST_PATH, '/var/www'], + 'host path dots in name' => [Path::HOST_PATH, 'my..app'], + 'absolute trailing slash' => [Path::ABSOLUTE_DIR, '/srv/'], + ]; + } + + #[Test] + #[DataProvider('validPathsProvider')] + public function validPathsProduceNoViolations(string $type, string $value): void + { + $this->validator->validate($value, new Path(type: $type)); + $this->assertNoViolation(); + } + + /** + * @return array + */ + public static function invalidPathsProvider(): array + { + return [ + 'space in host path' => [Path::HOST_PATH, '/var/www/my app'], + 'traversal host path' => [Path::HOST_PATH, '../etc'], + 'traversal absolute' => [Path::ABSOLUTE_DIR, '/var/../etc'], + 'space absolute' => [Path::ABSOLUTE_DIR, '/var/www myapp'], + 'absolute without leading slash' => [Path::ABSOLUTE_DIR, 'var/www'], + 'absolute double trailing slash' => [Path::ABSOLUTE_DIR, '/srv//'], + 'php missing extension' => [Path::RELATIVE_PHP, 'public/index'], + 'php space' => [Path::RELATIVE_PHP, 'public/my index.php'], + 'php traversal' => [Path::RELATIVE_PHP, 'public/../index.php'], + 'slash only host path' => [Path::HOST_PATH, '/'], + 'double slash host path' => [Path::HOST_PATH, '//'], + 'triple slash host path' => [Path::HOST_PATH, '///'], + 'absolute newline injection' => [Path::ABSOLUTE_DIR, "/var/www\nfastcgi_pass evil;"], + 'absolute semicolon injection' => [Path::ABSOLUTE_DIR, '/var/www;'], + 'absolute hash injection' => [Path::ABSOLUTE_DIR, '/var/www#'], + 'absolute dollar injection' => [Path::ABSOLUTE_DIR, '/var/www$foo'], + 'php newline injection' => [Path::RELATIVE_PHP, "public/index.php\n"], + 'php semicolon injection' => [Path::RELATIVE_PHP, 'public/index.php;'], + 'php hash injection' => [Path::RELATIVE_PHP, 'public/index.php#'], + 'php dollar injection' => [Path::RELATIVE_PHP, 'public/index.php$'], + ]; + } + + #[Test] + #[DataProvider('invalidPathsProvider')] + public function invalidPathsProduceOneViolation(string $type, string $value): void + { + $constraint = new Path(type: $type); + $this->validator->validate($value, $constraint); + + $this->buildViolation($constraint->message) + ->setParameter('{{ value }}', '"' . $value . '"') + ->assertRaised(); + } + + #[Test] + public function nonStringValueProducesNoViolations(): void + { + $this->validator->validate(null, new Path(type: Path::HOST_PATH)); + $this->assertNoViolation(); + } + + #[Test] + public function emptyStringProducesOneViolation(): void + { + $constraint = new Path(type: Path::HOST_PATH); + $this->validator->validate('', $constraint); + + $this->buildViolation($constraint->message) + ->setParameter('{{ value }}', '""') + ->assertRaised(); + } +} diff --git a/tests/Unit/PHPDocker/Zip/ArchiverTest.php b/tests/Unit/PHPDocker/Zip/ArchiverTest.php index 9e6e63f..1b62372 100644 --- a/tests/Unit/PHPDocker/Zip/ArchiverTest.php +++ b/tests/Unit/PHPDocker/Zip/ArchiverTest.php @@ -42,6 +42,31 @@ public function generateArchiveReturnsArchiveWithCorrectFilename(): void self::assertSame('test.zip', $archive->getFilename()); } + #[Test] + public function constructorDoesNotLeaveOrphanTemporaryBaseFile(): void + { + $tempPrefix = sprintf('%s/%s*', sys_get_temp_dir(), str_replace('\\', '_', Archiver::class)); + $before = glob($tempPrefix) ?: []; + + try { + $archiver = new Archiver('phpdocker'); + $archiver->addFile($this->makeFile('placeholder.txt', 'content')); + $archive = $archiver->generateArchive('test.zip'); + + $created = array_diff(glob($tempPrefix) ?: [], $before); + $orphans = array_filter($created, static fn (string $path): bool => !str_ends_with($path, '.zip')); + + self::assertFileExists($archive->getTmpFilename()); + self::assertSame([], $orphans); + } finally { + foreach (array_diff(glob($tempPrefix) ?: [], $before) as $path) { + if (is_file($path)) { + unlink($path); + } + } + } + } + #[Test] public function generateArchiveReturnsTmpFilenamePointingToActualFile(): void {