Repository navigation
Expand APPSEC integration testing: Symfony - #4268
Open
estringana wants to merge 9 commits into
Open
estringana wants to merge 9 commits into
estringana wants to merge 9 commits into
Conversation
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 3 Pipeline jobs failed
ℹ️ InfoNo other issues found (see more)❄️ No new flaky tests detected 🎯 Code Coverage (details) Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 556192a | Docs | View more details | Give us feedback! |
estringana
marked this pull request as ready for review
October 5, 2026 13:20
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6110d0811
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation is coherent, with only a non-blocking inaccurate documentation statement identified.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Expands AppSec Symfony integration coverage across supported fixture versions using a shared test suite.
Changes:
- Adds Symfony 4.4, 5.2, 7.3, and latest integration suites.
- Extracts shared AppSec assertions from the Symfony 6.2 suite.
- Adds fixture routes, Apache rewrites, SQLite initialization, and endpoint probes.
| File | Description |
|---|---|
tests/Frameworks/Symfony/Version_7_3/src/Controller/HomeController.php |
Adds test routes. |
tests/Frameworks/Symfony/Version_7_3/public/outside_of_framework.php |
Adds endpoint-state probe. |
tests/Frameworks/Symfony/Version_7_3/public/.htaccess |
Enables front-controller routing. |
tests/Frameworks/Symfony/Version_7_3/docker-init.sh |
Initializes the SQLite fixture. |
tests/Frameworks/Symfony/Version_7_3/composer.json |
Limits mocks to development autoloading. |
tests/Frameworks/Symfony/Version_5_2/src/Controller/HomeController.php |
Adds test routes. |
tests/Frameworks/Symfony/Version_5_2/public/outside_of_framework.php |
Adds endpoint-state probe. |
tests/Frameworks/Symfony/Version_5_2/public/.htaccess |
Enables front-controller routing. |
tests/Frameworks/Symfony/Version_5_2/docker-init.sh |
Initializes the SQLite fixture. |
tests/Frameworks/Symfony/Version_4_4/src/Controller/HomeController.php |
Adds version-compatible test routes. |
tests/Frameworks/Symfony/Version_4_4/public/outside_of_framework.php |
Adds endpoint-state probe. |
tests/Frameworks/Symfony/Version_4_4/public/.htaccess |
Enables front-controller routing. |
tests/Frameworks/Symfony/Version_4_4/docker-init.sh |
Initializes the SQLite fixture. |
tests/Frameworks/Symfony/Latest/src/Controller/HomeController.php |
Adds current Symfony test routes. |
tests/Frameworks/Symfony/Latest/public/outside_of_framework.php |
Adds endpoint-state probe. |
tests/Frameworks/Symfony/Latest/public/.htaccess |
Enables front-controller routing. |
tests/Frameworks/Symfony/Latest/docker-init.sh |
Initializes the SQLite fixture. |
tests/Frameworks/Symfony/Latest/composer.json |
Limits mocks to development autoloading. |
appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/SymfonyLatestTests.groovy |
Adds latest-version coverage. |
appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Symfony73Tests.groovy |
Adds Symfony 7.3 coverage. |
appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Symfony62Tests.groovy |
Adopts the shared suite. |
appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Symfony52Tests.groovy |
Adds Symfony 5.2 coverage. |
appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Symfony44Tests.groovy |
Adds Symfony 4.4 coverage. |
appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/AbstractSymfonyAppsecTests.groovy |
Defines shared AppSec assertions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
2 tasks
cataphract
approved these changes
Oct 8, 2026
cataphract
left a comment
cataphract
left a comment
Contributor
There was a problem hiding this comment.
Approved with two comments
Comment on lines
+45
to
+52
| /** Status code the fixture returns after a successful form_login. */ | ||
| int expectedLoginSuccessStatus() { 302 } | ||
|
|
||
| /** Status code the fixture returns after a failed form_login. */ | ||
| int expectedLoginFailureStatus() { 302 } | ||
|
|
||
| /** Status code the fixture returns after a successful signup. */ | ||
| int expectedSignupStatus() { 302 } |
Contributor
There was a problem hiding this comment.
Wouldn't it be better to unify the several versions' implementations?
Comment on lines
+11
to
+54
| export DATABASE_URL="sqlite:////var/www/var/app.db" | ||
| export APP_ENV=prod | ||
|
|
||
| # The committed config/packages/doctrine.yaml hard-codes a mysql URL (shared with | ||
| # trace integration tests that run against a mysql service). Drop a prod-env | ||
| # override so DATABASE_URL is honoured and the fixture can run on sqlite. | ||
| mkdir -p config/packages/prod | ||
| cat > config/packages/prod/doctrine_appsec.yaml << 'YAMLEOF' | ||
| doctrine: | ||
| dbal: | ||
| url: '%env(resolve:DATABASE_URL)%' | ||
| server_version: ~ | ||
| YAMLEOF | ||
| mark "wrote doctrine_appsec.yaml" | ||
|
|
||
| composer config optimize-autoloader false | ||
| mark "composer config done" | ||
| if [[ -f composer.lock ]]; then | ||
| composer install --no-dev --no-scripts | ||
| else | ||
| composer update --no-dev --no-scripts | ||
| fi | ||
| mark "composer install/update done" | ||
|
|
||
| mkdir -p var | ||
| # Nuke any residual state from a cached volume: Symfony prod cache AND the | ||
| # sqlite database. Starting from a clean DB file lets `doctrine:schema:create` | ||
| # succeed without needing a separate `doctrine:database:drop` step (which has | ||
| # been observed to hang silently under some SSI setups). | ||
| rm -rf var/cache/* var/app.db | ||
| mark "cleaned var/" | ||
|
|
||
| php bin/console doctrine:schema:create | ||
| mark "schema:create done" | ||
|
|
||
| php << 'PHPEOF' | ||
| <?php | ||
| // Symfony 7+ ships a `MakerBundle`-generated User with an extra NOT-NULL | ||
| // `is_verified` column. We seed it explicitly so SQLite doesn't reject the | ||
| // INSERT (which `OR IGNORE` would silently swallow). | ||
| $db = new PDO('sqlite:/var/www/var/app.db'); | ||
| $stmt = $db->prepare('INSERT OR IGNORE INTO "user" (email, password, roles, is_verified) VALUES (?, ?, ?, ?)'); | ||
| $stmt->execute(['test-user@email.com', '$2y$13$WNnAxSuifzgXGx9kYfFr.eMaXzE50MmrMnXxmrlZqxSa21oiMyy0i', '[]', 1]); | ||
| PHPEOF |
Contributor
There was a problem hiding this comment.
You can also use a mysql container (see org.testcontainers.mysql.MySQLContainer). That's probably the best option if it doesn't slow down the tests too much
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.
Description
Expand appsec integration testing to all supported versions of Symfony
Right now only one version of symfony was tested. This PR expands the testing to all supported versions
Reviewer checklist