Skip to content

SYN-648: Bump guzzle, phpunit and symfony/process to clear High alerts - #193

Merged
vmangwani merged 3 commits into
mainfrom
SYN-648-bump-high-cve-sdk-deps
Oct 2, 2026
Merged

vmangwani merged 3 commits into
mainfrom
SYN-648-bump-high-cve-sdk-deps

Conversation

@vmangwani

@vmangwani vmangwani commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Description

Clears the 3 open High Dependabot alerts on lob-php (guzzlehttp/guzzle #53, phpunit/phpunit #25, symfony/process #24) by regenerating composer.lock. composer.json is unchanged because its ranges already allowed the patched versions.

Package Before After Required
guzzlehttp/guzzle 7.4.5 7.15.5 >= 7.15.2
phpunit/phpunit (dev) 9.5.20 9.6.37 >= 9.6.33
symfony/process (dev) 5.4.8 5.4.51 >= 5.4.46

Files: composer.lock, .github/workflows/run_tests.yml, test/Unit/USVerificationsApiUnitTest.php.

CI fix: run_tests.yml:20 ran --group unit--coverage-text (missing space), so CI ran zero unit tests. It now runs --group unit --coverage-text, so CI actually runs the unit suite from this PR on.

Hidden test failure fixed (operator-approved): once CI actually ran the unit suite, 9 tests in test/Unit/USVerificationsApiUnitTest.php errored on Linux with Class "OpenAPI\Client\Api\USVerificationsApi" not found. The generated class is UsVerificationsApi (lib/Api/UsVerificationsApi.php), and PSR-4 autoloading is case-sensitive on Linux. The test has referenced the wrong case since #189 (2c235e4). It stayed hidden because the --group unit--coverage-text typo matched zero tests, and case-insensitive macOS filesystems resolve it anyway. The fix changes only the test's use/new references to UsVerificationsApi. Generated lib/ and file names are untouched. Verified 357/357 on a case-sensitive filesystem.

Transitive major bumps in the lock (pulled in by guzzle 7.15 / phpunit 9.6 via --with-all-dependencies, not hand-picked):

  • guzzlehttp/promises 1.5.1 -> 2.5.3 and psr/http-message 1.0.1 -> 2.0 (runtime deps of guzzle). lib/ doesn't implement PSR-7 interfaces or call removed promise functions; only Psr7\Utils::tryFopen is used.
  • guzzlehttp/psr7 2.4.0 -> 2.13.1 (same major).
  • Dev only: doctrine/instantiator 1 -> 2, nikic/php-parser 4 -> 5; phpspec/prophecy, phpdocumentor/* and webmozart/assert dropped.
  • SDK consumers are unaffected by the lock; they resolve their own versions from the unchanged composer.json ranges.

PHP version: the lock was regenerated and tested on PHP 8.1 (php:8.1-cli), the lowest 8.x in composer.json's ^7.3 || ^8.1. The existing base lock couldn't install on PHP >= 8.2 (prophecy), and the lock's dev set now effectively needs PHP >= 8.1 (doctrine/instantiator 2). The tech-lead review also ran the suite on PHP 8.3 (matching ubuntu-latest).

Supersedes Dependabot PR #171 (guzzlehttp/psr7 2.4.5, Medium), since psr7 is now 2.13.1. Close it after this merges.

Story

SYN-648 (parent SYN-624, Sync High CVE Remediation 2026-08)

Related PR's

Verify

  • Code runs without errors
  • Tests pass with >=85% line coverage

Gate results (run in Docker, php:8.1-cli + Composer 2):

  • vendor/bin/phpunit --group unit --coverage-text --configuration=phpunit.xml.dist: base 357 tests / 542 assertions OK; after 357 / 542 OK (also run on a case-sensitive filesystem copy, matching Linux CI).
  • composer install from a clean vendor/: OK. composer show confirms the versions above.
  • Integration suite (test/Integration, needs live Lob API keys): not run. It's an optional manual check.

To test: composer install && vendor/bin/phpunit --group unit --configuration=phpunit.xml.dist on PHP 8.1+.

Review

1 round: the lob-php specialist and the tech lead both approved. 1 minor finding (flag the transitive major bumps) is addressed above.

Acceptance criteria

AC Status Evidence
1. lob-php: guzzlehttp/guzzle resolves to >= 7.15.2 in composer.lock and the composer.json constr ✅ ✅ unit: lob-php test/Unit/*ApiUnitTest.php (full @group unit suite, run via vendor/bin/phpunit --group unit --coverage-text --configuration=phpunit.xml.dist) plus composer install/show (gates green)
2. lob-php: phpunit/phpunit resolves to >= 9.6.33 and symfony/process to >= 5.4.46 in `composer.loc ✅ ✅ unit: lob-php test/Unit/*ApiUnitTest.php (full @group unit suite, run via vendor/bin/phpunit --group unit --coverage-text --configuration=phpunit.xml.dist) (gates green)
3. lob-java: org.json:json resolves to >= 20231013 and com.squareup.okhttp3:okhttp to >= 4.9.2 in t ✅ ✅ integration: lob-java mvn dependency:tree -Dincludes=org.json:json,com.squareup.okhttp3 (build/resolution check, not a JUnit/TestNG test; confirms org.json:json:20231013 and okhttp:4.9.2 in the effective tree) plus mvn clean compile (gates green)
4. lob-java: the existing unit test suite passes after the bumps. Any API changes from the okhttp or or ✅ ✅ unit: lob-java tests/Api/*ApiTest.java (full suite, run via mvn test "-Dtest=%regex[.ApiTest.]") and tests/Model/*Test.java (full suite, run via mvn test "-Dtest=%regex[.Model.]") (gates green)
5. Before and after High counts are recorded on the ticket per repo. Before: lob-php 3 (live Dependabot ⏳ ⏳ manual-operator: pending AC5-manual-operator

Pending checks (the PR stays draft until these pass)

  • AC5-manual-operator AC5 · manual-operator · owner: operator

Risks (every review round)

  • lob-php: --with-all-dependencies pulled transitive major bumps into composer.lock: guzzlehttp/promises 1.5.1 -> 2.5.3, psr/http-message 1.0.1 -> 2.0 (runtime), nikic/php-parser v4 -> v5 and doctrine/instantiator 1 -> 2 (dev). phpspec/prophecy, phpdocumentor/* and webmozart/assert were dropped. lib/ doesn't implement any PSR-7 interface and doesn't call removed promise functions. Only Psr7\Utils::tryFopen is used. composer.json ranges are unchanged, so SDK consumers resolve their own versions. (r1)
  • lob-php: guzzle resolves to 7.15.5, phpunit to 9.6.37 and symfony/process to v5.4.51, all above the minimums. The regenerated lock makes lob-php#171 (psr7 2.4.5, Medium) redundant: psr7 is now 2.13.1. (r1)
  • lob-php: the lock's dev set is effectively PHP >= 8.1 (doctrine/instantiator 2.0 requires ^8.1, and symfony/deprecation-contracts v3 already required >= 8.1 before this change). composer.json still says ^7.3 || ^8.1. (r1)
  • lob-php: the run_tests.yml:20 typo fix means CI runs the unit suite for the first time: 357 tests pass on PHP 8.1 (implementer) and on PHP 8.3 (tech-lead, php:8.3-cli, matching ubuntu-latest). (r1)
  • lob-java: org.json jumps 20220320 -> 20231013. ApiClient.java:803-806 (CreativeResponse: JSONObject.put of a bean, then toString) and :1041-1043 (error parsing) aren't unit-covered, so only the live *SpecTest CI run exercises them. Check that the PR CI run is green before merge. (r1)
  • lob-java follow-up: build.gradle:109-110 and build.sbt:13-14 still pin okhttp 4.9.1 (not read by Dependabot or CI). (r1)
  • lob-java: the build was proven with mvn clean compile, not install -DskipTests, because the maven-gpg-plugin is bound at verify (pom.xml:223-238) and needs a signing key. This predates the change. (r1)
  • AC5 is operator-only: after merge, confirm 0 open High alerts per repo and post the before/after table. Close lob-java#351 and #338, and post the SUP-1322 note. (r1)

Follow-ups

🤖 Generated with Claude Code

Regenerate composer.lock so guzzlehttp/guzzle (7.15.5), phpunit/phpunit
(9.6.37) and symfony/process (5.4.51) are at patched versions, clearing
the 3 open High Dependabot alerts. composer.json is unchanged. Also fix
the missing space in run_tests.yml so CI actually runs the unit suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test referenced USVerificationsApi, but the generated class is
UsVerificationsApi, so PSR-4 autoloading fails on case-sensitive
filesystems. This was hidden because CI ran zero unit tests until the
run_tests.yml fix in this branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vmangwani
vmangwani marked this pull request as ready for review September 28, 2026 17:28
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Patch high-risk PHP dependencies and restore CI unit-test coverage

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Refresh the Composer lockfile to resolve three High Dependabot alerts without changing package
 requirements.
• Fix the CI command so it runs the unit suite instead of selecting zero tests.
• Correct API class casing in nine tests so they load on case-sensitive filesystems.
Diagram

graph TD
  M["composer.json"] --> L[("composer.lock")] --> G["Guzzle 7.15"] --> P["HTTP dependencies"]
  L --> U["PHPUnit 9.6"] --> T["Unit tests"] --> A["UsVerificationsApi"]
  W["CI workflow"] --> U --> T
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Refresh only the affected dependency trees
  • ➕ Could reduce unrelated lockfile churn and simplify compatibility review.
  • ➖ Guzzle's patched release still requires transitive updates, including a new promises major version.
  • ➖ May leave the older development dependency set difficult to install on newer PHP versions.

Recommendation: Keep the lockfile refresh and the two focused test fixes: the patched packages fit existing manifest ranges, and CI can now validate them. Review the transitive major upgrades and note that this development lockfile requires PHP 8.1 or newer even though composer.json also permits PHP 7.3.

Files changed (3) +436 / -654

Bug fix (2) +11 / -11
run_tests.ymlRestore unit-test selection in CI +1/-1

Restore unit-test selection in CI

• Separates '--group unit' from '--coverage-text'. The previous combined argument matched no unit tests; CI will now execute the suite.

.github/workflows/run_tests.yml

USVerificationsApiUnitTest.phpMatch generated API class casing in unit tests +10/-10

Match generated API class casing in unit tests

• Changes the import and nine constructor calls from 'USVerificationsApi' to the generated 'UsVerificationsApi' class. This fixes PSR-4 autoloading on case-sensitive filesystems without changing generated SDK code.

test/Unit/USVerificationsApiUnitTest.php

Other (1) +425 / -643
composer.lockLock patched Guzzle, PHPUnit, and Symfony Process releases +425/-643

Lock patched Guzzle, PHPUnit, and Symfony Process releases

• Moves guzzlehttp/guzzle to 7.15.5, phpunit/phpunit to 9.6.37, and symfony/process to 5.4.51, alongside transitive dependency updates. These include runtime major upgrades to guzzlehttp/promises 2 and psr/http-message 2, plus development-only dependency changes. The refreshed development lockfile includes doctrine/instantiator 2, which requires PHP 8.1 or newer.

composer.lock

@qodo-code-review

qodo-code-review Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. CI skips the card idempotency test ✓ Resolved
Description
The corrected PHPUnit command selects only @group unit, but testCreateWithIdempotency is tagged
@group units. When CI runs the unit suite, it excludes that test without failing or reporting the
omission.
Code

.github/workflows/run_tests.yml[20]

+        run: vendor/bin/phpunit --group unit --coverage-text --coverage-clover=coverage/coverage.xml --configuration=phpunit.xml.dist
Evidence
The changed CI command filters for unit, while the card idempotency test declares only units and
cards; neighboring card tests declare unit.

.github/workflows/run_tests.yml[19-20]
test/Unit/CardsApiUnitTest.php[150-154]
test/Unit/CardsApiUnitTest.php[174-176]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CI unit-test command excludes the card idempotency test because its group tag is plural.

## Fix Focus Areas
- test/Unit/CardsApiUnitTest.php[150-153]
- .github/workflows/run_tests.yml[19-20]

## Recommended Fix
Change the idempotency test's `@group units` annotation to `@group unit` so the corrected CI command runs it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 1 rule
✅ Web pages:
  +11 more
✅ Cross-repo context — repo relationships
Review mode: ⚖️ Balanced: This is a security-motivated dependency and CI/test behavior change with broad lockfile transitive updates, but the actual logic is not dense enough to warrant extended review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/run_tests.yml
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vmangwani
vmangwani merged commit cfda064 into main Oct 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants