Harden generator input handling and build supply chain - #399
Merged
Merged
Conversation
tempnam() created a zero-byte placeholder, then ZipArchive opened
${base}.zip, leaking the base file. Unlink the placeholder before
opening the archive and throw ArchiveNotCreatedException when tempnam
returns false. Add a focused regression test asserting no non-.zip
temporary file remains after construction and generation.
Validate generated paths against strict grammars and reject traversal, and rate limit POST generation using a Redis-backed production pool with isolated test state.
…sions Pin every uses: in build-containers.yaml and cleanup-repo.yaml to full commit SHAs (version comments kept). Add explicit job-level GITHUB_TOKEN permissions: build needs only contents:read (pushes to DockerHub, not GH packages); cleanup needs contents/issues/pull-requests write for stale handling and branch deletion.
Verify every downloaded executable before it is chmod'ed or run: - curl -fsSL --retry 3: HTTP/network errors now abort the target instead of falling through to chmod + execution. Downloads go to a temp file and are moved into place, so a failed fetch leaves no partial cache. - Pin the xwmx/hosts source to the commit the 3.6.4 tag points at (9a929dc70fa11bfe6dc5b0f1d53aea442395edd3) instead of a mutable tag. - Add verify-mkcert/verify-hosts targets that check the SHA-256 of both cached and freshly downloaded files. verify-mkcert precedes install-mkcert and create-certs; verify-hosts precedes install-hosts, clean-hosts and init-hosts (the sudo paths). Upstream publishes no checksum for the mkcert v1.4.3 assets, so the values were computed from the assets downloaded over HTTPS from the official release URLs on 2026-09-25; windows-amd64.exe is corroborated by the mkcert 1.4.3 Chocolatey package. Provenance is documented next to the values. An unsupported BINARY_SUFFIX fails closed instead of skipping verification.
Throw ArchiveNotCreatedException when the tempnam placeholder cannot
be unlinked or when ZipArchive::open fails, removing any partially
created ${base}.zip first so no temporary artefact is left behind.
The regression test now cleans up the zip it generates and asserts
both that the archive file exists and that no non-.zip orphan remains.
Allow a single optional trailing slash for absolute dirs, reject only real .. path segments, and require framework-bundle ^8.1 for the RateLimit attribute wiring.
Verify every downloaded executable before it is chmod'ed or run: - curl -fsSL --retry 3: HTTP/network errors now abort the target instead of falling through to chmod + execution. Downloads go to a temp file and are moved into place, so a failed fetch leaves no partial cache. - Pin the xwmx/hosts source to the commit the 3.6.4 tag points at (9a929dc70fa11bfe6dc5b0f1d53aea442395edd3) instead of a mutable tag. - verify-mkcert/verify-hosts check the SHA-256 of cached and freshly downloaded files, refuse to pass when the expected executable is missing, and fail closed on an unsupported BINARY_SUFFIX. Each verify target depends on its download target, so make -j still downloads before verifying. verify-mkcert precedes install-mkcert and create-certs (which therefore downloads and verifies on a clean tree); verify-hosts precedes install-hosts, clean-hosts and init-hosts (the sudo paths). Upstream publishes no checksum for the mkcert v1.4.3 assets, so the values were computed from the assets downloaded over HTTPS from the official release URLs on 2026-09-25; windows-amd64.exe is corroborated by the mkcert 1.4.3 Chocolatey package. Provenance is documented next to the values.
GeneratorTest::setUp() retrieved the rate_limiter.cache pool directly to clear limiter state between tests. That service is only present in a freshly compiled test container; with debug=false and a stale var/cache/test it is missing, so every GeneratorTest test errored in setUp before issuing a request. The limiter keys requests by client IP (plus method and path). Give each test a unique, per-run IP instead, which isolates limiter buckets between tests and test runs without ever touching the cache pool. Production Redis limiter and the test filesystem pool override are unchanged.
Audit SA-004: actions/checkout and actions/cache were referenced by mutable major-version tags. Pin both to immutable 40-char commit SHAs with version comments, matching build-containers.yaml.
verify-mkcert/verify-hosts now chmod +x the binary immediately after the SHA-256 comparison succeeds, so every consumer (create-certs, init-hosts, install-mkcert, clean-hosts) gets an executable binary. Removes the redundant chmod from install-mkcert/install-hosts that left create-certs and init-hosts failing standalone on fresh checkouts and raced under make -j. Mismatch still exits before any chmod.
HOST_PATH now requires at least one non-slash character so values like '/', '//' no longer validate and cannot collapse to an empty DockerCompose volume. Adds unit cases for slash-only and nginx/newline/semicolon/#/$ payloads, plus functional length boundaries for appPath/dockerWorkingDir (255/256) and frontControllerPath (128/129).
Configure framework.trusted_headers to x-forwarded-for/proto/port so no other forwarded header is honoured, and rely on Symfony's default SYMFONY_TRUSTED_PROXIES env var (unset in .env) for a no-trust fallback. Remove the obsolete TRUSTED_PROXIES line that no code read. .env.test points SYMFONY_TRUSTED_PROXIES at a documentation-only range and TrustedProxyTest proves the rate limiter buckets forwarded clients separately through a trusted peer while an untrusted peer cannot spoof X-Forwarded-For.
Audit SA-004: tests.yaml had no explicit permissions, so the job ran with default token permissions. Scope the job to contents: read.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses included findings from
.steiner/security/2026-09-25_1102_repo.md.mkcertandhoststools before chmod, execution, or sudo use.Excluded by request
phpdockerio/php:8.5-fpmremains unchanged.Verification
118 tests, 551 assertions20 tests, 30 assertions7 tests, 10 assertionsNotes
mkcertv1.4.3 does not publish upstream checksums. Recorded hashes were computed from official release assets; unsupported binary suffixes fail closed.