Skip to content

SYN-648: Bump org.json and okhttp to clear High alerts - #352

Open
vmangwani wants to merge 2 commits into
mainfrom
SYN-648-bump-high-cve-sdk-deps
Open

vmangwani wants to merge 2 commits into
mainfrom
SYN-648-bump-high-cve-sdk-deps

Conversation

@vmangwani

@vmangwani vmangwani commented Sep 26, 2026 •

Copy link
Copy Markdown

What problem are you trying to solve?

How did you solve this problem?

  • org.json:json 20220320 -> 20231013 (pom.xml:454).
  • <okhttp-version> 4.9.1 -> 4.9.2 (pom.xml:476). logging-interceptor shares the property and moves with it.
  • CI fix (operator-approved): .github/workflows/run_tests.yml:27 actions/cache@v2 -> @v4. GitHub auto-fails jobs that use cache@v2, so run_tests never started on this PR (Dependabot chore(deps): bump com.squareup.okhttp3:okhttp from 4.9.1 to 4.9.2 #351 fails the same way).
  • No source changes were needed. ApiClient.java only uses org.json/okhttp APIs that didn't change across these versions.

Important notes

CI status

  • With the cache fix, run_tests runs: 1189 tests, 11 failures, all in the live Integration.*SpecTest suite, 39 skipped (run 36204693707). Every Api/Model unit test passes.
  • The 11 failures come from the CI Lob account and keys, not this change: "This endpoint requires live mode, but test mode was used" (Campaigns, Creatives, Uploads), "Print & Mail Edition does not allow you to add more Scheduled Mailings" (Letters), "Your API key is not valid" message mismatch (IdentityValidation), "check not found" (Check), and a self-mailer render trigger failure. These failures were already there before this PR; main's CI hasn't run since cache@v2 started being auto-failed.

Test plan

Run in Docker (maven:3-eclipse-temurin-17; CI uses JDK 14):

  1. mvn -B dependency:tree -Dincludes=org.json:json,com.squareup.okhttp3 shows okhttp 4.9.2, logging-interceptor 4.9.2 and json 20231013.
  2. mvn -B clean compile: BUILD SUCCESS.
  3. mvn -B test "-Dtest=%regex[.*ApiTest.*]": base 156 tests, 0 failures; after 156, 0 failures.
  4. mvn -B test "-Dtest=%regex[.*Model.*]": base 956 tests, 0 failures; after 956, 0 failures.
  5. The live *SpecTest suite (needs repo secrets) runs in PR CI.

Review: 1 round. The lob-java specialist and the tech lead both approved, with no findings on this repo.

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 chore(deps): bump org.json:json from 20220320 to 20231013 #338, and post the SUP-1322 note. (r1)

Follow-ups

  • The operator accepted the 11 live *SpecTest failures for this PR. Follow-up: fix the CI Lob account and keys used by the live SpecTests. That means live-mode access for Campaigns, Creatives and Uploads, the scheduled-mailing plan limit, the IdentityValidation API-key message, the Check fixture, and the self-mailer render.
  • Follow-up: add synchronize to run_tests.yml's pull_request types (in lob-java and lob-php), so pushes to an open PR run CI.
  • build.gradle:109-110 and build.sbt:13-14 still pin okhttp 4.9.1. Neither Dependabot nor CI reads them; left unchanged on purpose.
  • After merge: confirm 0 open High alerts on lob-java and record the after-count on SYN-648 (AC5). Close chore(deps): bump com.squareup.okhttp3:okhttp from 4.9.1 to 4.9.2 #351 and chore(deps): bump org.json:json from 20220320 to 20231013 #338.

🤖 Generated with Claude Code

Bump org.json:json 20220320 -> 20231013 and okhttp-version 4.9.1 ->
4.9.2 (logging-interceptor follows the shared property), clearing the 3
open High Dependabot alerts on pom.xml. No source changes needed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub auto-fails jobs that use the deprecated actions/cache@v2, so
run_tests never started on this branch (or on main's Dependabot PRs).

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

Bump org.json and OkHttp; restore CI test execution

⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Upgrade org.json and OkHttp to address three high-severity Dependabot alerts.
• Keep the OkHttp logging interceptor aligned through its shared Maven version property.
• Upgrade actions/cache to v4 so CI can run; pre-existing live integration failures remain.
Diagram

graph TD
  W["Run Tests workflow"] --> C["Cache v4"] --> T["Maven tests"] --> P["pom.xml"] --> O["OkHttp stack"] --> A["ApiClient"]
  P --> J["org.json"] --> A
Loading
High-Level Assessment

Direct Maven version bumps are appropriate: the existing shared property keeps OkHttp and its interceptor aligned, and no source migration is needed. Upgrading the blocked cache action separately restores CI execution. Check dependency resolution and unit-test results; the reported live integration failures require separate investigation.

Files changed (2) +3 / -3

Other (2) +3 / -3
run_tests.ymlReplace deprecated Maven cache action +1/-1

Replace deprecated Maven cache action

• Moves actions/cache from v2 to v4 so GitHub no longer rejects the test job before Maven starts.

.github/workflows/run_tests.yml

pom.xmlUpgrade JSON and OkHttp dependencies +2/-2

Upgrade JSON and OkHttp dependencies

• Raises org.json:json from 20220320 to 20231013 and the shared OkHttp version from 4.9.1 to 4.9.2. The shared property also upgrades logging-interceptor.

pom.xml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Some users retain vulnerable OkHttp 🐞 Bug ⛨ Security
Description
The Maven property now selects OkHttp 4.9.2, but the Gradle and SBT builds and both documented
Spring dependency overrides still select 4.9.1. Gradle or SBT builds retain the old version, and
Spring users following the instructions override the updated SDK dependency back to it.
Code

pom.xml[476]

+        <okhttp-version>4.9.2</okhttp-version>
Evidence
Both Maven OkHttp artifacts use the changed property, while the alternate build files explicitly
retain 4.9.1. The Spring instructions explicitly add dependency management and a direct dependency
at 4.9.1, which take precedence over the SDK's transitive Maven version.

pom.xml[411-418]
pom.xml[476-476]
build.gradle[109-110]
build.sbt[13-14]
README.md[60-76]
MIGRATION.md[28-44]

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 Maven OkHttp bump does not reach the alternate builds, and the documented Spring overrides force consumers back to 4.9.1.

## Fix Focus Areas
- pom.xml[476-476]
- build.gradle[109-110]
- build.sbt[13-14]
- README.md[60-76]
- MIGRATION.md[28-44]

## Recommended Fix
Update both OkHttp artifacts in the Gradle and SBT builds to 4.9.2, and change both Spring dependency examples in each guide to 4.9.2 so they no longer override the SDK's updated version.

ⓘ 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
✅ Cross-repo context — repo relationships
  Explored: repo: lob/lob-sdk-demo (sha: b6d6ebcd) — View relationship
Review mode: ⚖️ Balanced: This is a localized dependency-remediation and CI configuration change with security-related impact, so it warrants a careful single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread pom.xml
<gson-fire-version>1.8.5</gson-fire-version>
<swagger-core-version>1.6.2</swagger-core-version>
<okhttp-version>4.9.1</okhttp-version>
<okhttp-version>4.9.2</okhttp-version>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Some users retain vulnerable okhttp 🐞 Bug ⛨ Security

The Maven property now selects OkHttp 4.9.2, but the Gradle and SBT builds and both documented
Spring dependency overrides still select 4.9.1. Gradle or SBT builds retain the old version, and
Spring users following the instructions override the updated SDK dependency back to it.
Agent Prompt
## Issue description
The Maven OkHttp bump does not reach the alternate builds, and the documented Spring overrides force consumers back to 4.9.1.

## Fix Focus Areas
- pom.xml[476-476]
- build.gradle[109-110]
- build.sbt[13-14]
- README.md[60-76]
- MIGRATION.md[28-44]

## Recommended Fix
Update both OkHttp artifacts in the Gradle and SBT builds to 4.9.2, and change both Spring dependency examples in each guide to 4.9.2 so they no longer override the SDK's updated version.

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

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.

1 participant