Summary
The reusable workflows integration.yaml and the proposed e2e.yaml (#313) share ~90% of their step logic (services, PHP/Composer setup, Magento project creation, fixups, MySQL 5.7 downgrade, etc.). This duplication makes bugs harder to fix and increases maintenance burden as more workflow types are added.
We should extract the shared setup into a composite action (e.g. setup-magento-test-env) and have both workflows call it, fixing several long-standing issues along the way.
Issues to address during extraction
- Shell injection in
run: steps. Multiple steps interpolate ${{ }} expressions directly into shell commands (e.g. composer create-project --repository-url="${{ inputs.magento_repository }}", echo ${{ steps.supported-version.outputs.matrix }}). These should use env: blocks and reference shell variables instead, per GitHub's security hardening guide.
- Unsigned MySQL 5.7 binary download. The MySQL 5.7 client downgrade step downloads
.deb packages via wget --quiet with no SHA256 checksum verification. Should pin expected hashes.
- Internal actions pinned to
@main. mage-os/github-actions/get-magento-version@main should be pinned to a release tag or SHA.
Possible bugs
- Hardcoded artifact upload path. Both workflows use
path: /home/runner/work/infrastructure/magento2/dev/tests/… which is specific to a single repo and will be wrong for any external consumer. Should be derived from inputs.magento_directory.
$GITHUB_WORKSPACE as input default. Workflow input defaults are literal strings, not shell-expanded. $GITHUB_WORKSPACE works in run: steps but would break if used in non-shell contexts (e.g. if: expressions or action with: blocks).- Missing
db-name sed replacement in e2e.yaml. The MySQL service creates magento_e2e_tests but the sed block doesn't update the database name in install-config-mysql.php.dist.
Documentation
- README examples reference
actions/checkout@v2 — should be @v4.
e2e-README.md documents test_command default as "../../../vendor/bin/phpunit" but the workflow defaults to "".
Proposed approach
-
Create a new composite action (e.g. setup-magento-test-env/action.yml) that handles:
- PHP + Composer setup
- Composer cache
- Magento project creation
- Version-specific fixups (Monolog, Dotmailer, Composer plugins, prestissimo)
- Package require + install
- Config sed replacements (parameterized for integration vs e2e paths)
- MySQL 5.7 client downgrade (with checksum)
-
Refactor integration.yaml and e2e.yaml to call the composite action, adding only their test-type-specific steps (phpunit vs Playwright, working directories, etc.).
-
Fix all security and bug issues listed above in the shared action.
This also aligns with the direction discussed in #140 (reducing public workflow surface area).
Related
Summary
The reusable workflows
integration.yamland the proposede2e.yaml(#313) share ~90% of their step logic (services, PHP/Composer setup, Magento project creation, fixups, MySQL 5.7 downgrade, etc.). This duplication makes bugs harder to fix and increases maintenance burden as more workflow types are added.We should extract the shared setup into a composite action (e.g.
setup-magento-test-env) and have both workflows call it, fixing several long-standing issues along the way.Issues to address during extraction
run:steps. Multiple steps interpolate${{ }}expressions directly into shell commands (e.g.composer create-project --repository-url="${{ inputs.magento_repository }}",echo ${{ steps.supported-version.outputs.matrix }}). These should useenv:blocks and reference shell variables instead, per GitHub's security hardening guide..debpackages viawget --quietwith no SHA256 checksum verification. Should pin expected hashes.@main.mage-os/github-actions/get-magento-version@mainshould be pinned to a release tag or SHA.Possible bugs
path: /home/runner/work/infrastructure/magento2/dev/tests/…which is specific to a single repo and will be wrong for any external consumer. Should be derived frominputs.magento_directory.$GITHUB_WORKSPACEas input default. Workflow input defaults are literal strings, not shell-expanded.$GITHUB_WORKSPACEworks inrun:steps but would break if used in non-shell contexts (e.g.if:expressions or actionwith:blocks).db-namesed replacement in e2e.yaml. The MySQL service createsmagento_e2e_testsbut the sed block doesn't update the database name ininstall-config-mysql.php.dist.Documentation
actions/checkout@v2— should be@v4.e2e-README.mddocumentstest_commanddefault as"../../../vendor/bin/phpunit"but the workflow defaults to"".Proposed approach
Create a new composite action (e.g.
setup-magento-test-env/action.yml) that handles:Refactor
integration.yamlande2e.yamlto call the composite action, adding only their test-type-specific steps (phpunit vs Playwright, working directories, etc.).Fix all security and bug issues listed above in the shared action.
This also aligns with the direction discussed in #140 (reducing public workflow surface area).
Related