{{ message }}
Repository navigation
feat(installer): add --symfony-docker-ref, keep the pinned default - #3013
Merged
Merged
Conversation
(cherry picked from commit fc625ba) Adapted: the original also floated SYMFONY_DOCKER_REF to `main`. That part is dropped — the reviewed SHA stays the default and this option overrides it per install.
A 40-char SHA is no proof of review: GitHub serves a fork network from a single object store, so a shallow fetch against dunglas/symfony-docker resolves any commit pushed to any fork of it, including an unmerged pull request head. Verified against the head of dunglas/symfony-docker#948, which fetches cleanly from the canonical URL. Format validation would therefore buy nothing, so the option accepts any ref and says plainly what the user is opting into.
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
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.

Follow-up to #3012, taking the good half of @ili101's #3011.
--symfony-docker-refis cherry-picked from ili101'sfc625bawith authorship intact.What's taken, and what isn't
#3011 bundled two separable changes:
SYMFONY_DOCKER_REF = 'main'— the default floats.--symfony-docker-ref— a per-install override.Only (2) is taken. The security and reproducibility properties live in the default, because that is what runs for everyone who types
api-platform new myappand nothing else. An escape hatch for the few who pass the flag doesn't compensate for floating the many who don't. And the rot that motivated #3008 is already handled by #3012: the pin now gets a validated bump PR the week upstream touches a file we copy.So the default stays the reviewed SHA, and the flag overrides it per install.
testSymfonyDockerRefIsPinnedToFullCommitShais kept rather than renamed totestSymfonyDockerRefDefaultsToMain— that test is the guard rail that stops the default drifting back to a branch name.Also dropped from the cherry-pick: everything touching
--with-database, which is #3009's content and belongs in its own review.Why the flag accepts any ref, with a warning instead of validation
The obvious hardening — "require a 40-character SHA" — buys nothing. GitHub serves an entire fork network from one object store, so a shallow fetch against the canonical repo resolves commits that were never merged, or never even opened as a PR. Verified with the exact command
setupDockerruns, against the head of an unmerged PR from a fork (dunglas/symfony-docker#948, branch owned byenginydigital):A hostile SHA would therefore look more trustworthy than a branch name — it points at the official repository and reads like a reproducibility measure. Format validation would be theatre, and refusing branch names would block the one case the flag exists for ("I need the fix that's on main today").
An attacker who can dictate your command line has already won, so this isn't a reason to drop the flag. It is a reason to say plainly what you're opting into:
The warning goes to stderr, and only fires when the ref will actually be fetched — passing it alongside
--no-with-dockerstays silent, since nothing is fetched.Commits
147d4e0f469017Verification
123 tests / 239 assertions green, PHPStan clean, warning rendering checked against the real CLI.