Skip to content

fix(security): bump jackson-databind to 2.18.8 - #160

Open
alexgomezlf wants to merge 18 commits into
v2from
alex/fix-jackson-databind-cve-2026-54512-54513
Open

fix(security): bump jackson-databind to 2.18.8#160
alexgomezlf wants to merge 18 commits into
v2from
alex/fix-jackson-databind-cve-2026-54512-54513

Conversation

@alexgomezlf

@alexgomezlf alexgomezlf commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Bumps the shared jackson-version property from 2.18.2 to 2.18.8 to remediate two Veracode-flagged CVEs in jackson-databind:

  • CVE-2026-54512 — PolymorphicTypeValidator bypass via generic type parameters
  • CVE-2026-54513 — PolymorphicTypeValidator bypass via array component types (allowIfSubTypeIsArray)

Both are fixed upstream in 2.18.8.

Note: the v1 branch has the same vulnerable pin but is out of scope here since the flagged Veracode build corresponds to v2. A tracking issue for the v1 fix will be filed separately.

Fixes #679086
Fixes #679087

Test / CI changes

Alongside the jackson bump, this PR also includes integration-test and workflow fixes needed to get the suite green against the current dev/CA repository:

  • Audit-reason handling for delete (BaseTest, EntriesClientTest, TasksClientTest): the dev/CA repo now enforces an audit reason on DeleteEntry (errorCode 216), unlike the old dedicated test repo. Added getDeleteAuditReasonId()/newDeleteEntryRequest() helpers that resolve and cache the delete audit reason (mirroring the existing export audit-reason lookup) and swapped all bare new StartDeleteEntryRequest() call sites over to it.
  • Fail-fast task polling (BaseTest.waitUntilTaskEnds): previously a task ending in a non-IN_PROGRESS, non-success state would fall through as if it succeeded. Now a terminal status other than COMPLETED throws immediately with the task's ProblemDetails (title/detail/status/errorCode/errorSource) surfaced via a new formatErrors helper, instead of masking the failure or requiring a timeout to notice.
  • Richer failure diagnostics (ExportDocumentApiTest, ImportUploadedPartsApiTest): wrapped exportEntry/multipart-upload/task-list calls so an ApiException's status code, headers, and problem details show up in the surefire report — we don't have access to the raw CI job log to see the actual HTTP response otherwise.
  • REPOSITORY_ID secret swap (.github/workflows/main.yml): integration tests now use DEV_CA_PUBLIC_USE_REPOSITORY_ID_1 instead of ..._2, and the workflow grants checks: write / pull-requests: write so the test-results action can publish/update its PR comment.

Happy to split the test/CI changes into a separate PR if preferred — flagging here since they're bundled with the security fix in this diff.

alexgomezlf and others added 4 commits July 8, 2026 14:48
…54512 and CVE-2026-54513

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion can post check runs

Default GITHUB_TOKEN permissions are read-only, so EnricoMi/publish-unit-test-result-action
was failing with 403 Resource not accessible by integration on every PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…R comment

checks:write alone got the check runs created, but the action's default
comment_mode also posts/updates a PR comment, which needs pull-requests:write.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

unit-test-results

25 tests  ±0   25 ✅ ±0   0s ⏱️ ±0s
 3 suites ±0    0 💤 ±0 
 3 files   ±0    0 ❌ ±0 

Results for commit 690efa9. ± Comparison against base commit 50913c6.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

integration-test-results-cloud

77 tests  ±0   76 ✅ +3   2m 20s ⏱️ - 1m 54s
16 suites ±0    1 💤 ±0 
16 files   ±0    0 ❌  - 2 

Results for commit 690efa9. ± Comparison against base commit 50913c6.

♻️ This comment has been updated with latest results.

alexgomezlf and others added 13 commits July 9, 2026 18:03
…lder

Access denied errors trace back to the service principal not having access
to whatever repo DEV_CA_PUBLIC_USE_REPOSITORY_ID_2 points to (confirmed via
RepositoriesClientTest.listRepositoriesWorks: listRepositories() succeeds but
that repo ID isn't in the returned list). Trying REPOSITORY_ID_1 (used by the
dotnet/JS clients) with a hardcoded folder ID to see what actually fails now
that auth itself isn't the blocker. Not a real fix - just a diagnostic.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@bzajzon-laserfiche bzajzon-laserfiche left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Jackson bump itself looks good (same as lf-api-client-core-java #86).

Requesting changes for one blocker:

  • Remove the [RETRY-DEBUG] System.out.println calls in ApiClientUtils.sendRequestWithRetry — debug scaffolding that would ship in the published client, printing request URLs/statuses to consumers' stdout.

Minor:

  • PR body only mentions the jackson bump — please also describe the test/CI changes (audit-reason handling, fail-fast task polling, REPOSITORY_ID secret swap), or split them out.
  • Confirm a tracking issue exists for the v1 branch's same vulnerable pin.

Debug scaffolding left in sendRequestWithRetry would have shipped in the
published client, printing request URLs/statuses to consumers' stdout.
@alexgomezlf

alexgomezlf commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review!

  • Blocker ([RETRY-DEBUG] printlns): removed all three call sites in ApiClientUtils.sendRequestWithRetry in 690efa9. That was left-over debug scaffolding from tracking down a flaky retry in the integration suite and shouldn't have been in the diff — good catch.
  • PR body / test-CI description: updated the PR description to call out the audit-reason handling, fail-fast task polling, and REPOSITORY_ID secret swap explicitly, rather than splitting into a separate PR, since they were needed to get this branch's integration suite green. Let me know if you'd still prefer them split out.

Commit 690efa9 has the fix pushed.

@bzajzon-laserfiche bzajzon-laserfiche left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All feedback addressed — thanks:

  • [RETRY-DEBUG] printlns removed; the diff no longer touches production source at all (test + CI + pom/CHANGELOG only).
  • PR body now documents the test/CI changes and their rationale; fine to keep them bundled given they're test-only and needed for a green suite.

One follow-up: please do file the v1 tracking issue mentioned in the description so the same vulnerable pin there doesn't get lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants