Skip to content

fix(logging): remove duplicate Commons Logging runtime binding - #340

Closed
seonghobae wants to merge 4 commits into
mainfrom
fix/single-commons-logging-binding-20260810
Closed

fix(logging): remove duplicate Commons Logging runtime binding#340
seonghobae wants to merge 4 commits into
mainfrom
fix/single-commons-logging-binding-20260810

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Objective

Advance issue #320 by removing the duplicate Commons Logging runtime provider while preserving Spring's spring-jcl bridge, representative PDFBox behavior, exact coverage, public Javadocs, SBOM-sensitive dependency policy, and normal repository protection.

Fresh exact state — 2026-08-10

  • exact source head: 523dd8403ad1ccfcc88ed2fe7226c90d7f02c7b5;
  • independently resolved protected main: 55d7ae8647208e301f282350f076eeddaba61d11;
  • clean three-file slice: pom.xml, the runtime-provider Java regression, and one repository dependency-intent contract;
  • open, Ready, mergeable;
  • exact-head CI 31394251655: success;
  • exact-head Security Scan 31394253262: success;
  • exact-head SAST Semgrep 31394251794: success;
  • exact-head fuzz 31394251697: success;
  • current GHAS review state is COMMENTED, not approval; both Semgrep threads created on the RED predecessor are resolved and outdated after the fix;
  • no automated check, comment, status, or model output is qualifying independent approval.

Test-first RCA and narrow remedies

The focused Java CommonsLoggingRuntimeBindingTest enumerates every runtime resource for org/apache/commons/logging/LogFactory.class, requires exactly one provider, and requires that provider to be Spring's spring-jcl bridge. The production change excludes only commons-logging:commons-logging from Clearfolio's direct org.apache.pdfbox:pdfbox dependency.

A second deterministic repository contract now protects the dependency declaration itself: PDFBox must retain the standalone Commons Logging exclusion, the project must not directly reintroduce commons-logging:commons-logging, and the intentional Log4j2 backend remains declared. This catches future dependency-authority drift without depending only on runtime classpath enumeration.

SAST RED → GREEN

Test head 1eeeb5fbfc09b0feb3bda257d08ad10c9fd92e23 initially parsed pom.xml with Python's native xml.etree.ElementTree. CI, Security Scan and fuzz succeeded, but SAST Semgrep run 31393873748 correctly failed with two python.lang.security.use-defused-xml-parse.use-defused-xml-parse findings because the parser class is unsafe for untrusted XML/XXE boundaries.

Rather than suppressing the finding or adding a new parser dependency for a tiny trusted repository contract, current head 523dd8403ad1ccfcc88ed2fe7226c90d7f02c7b5 rewrites the check as bounded UTF-8 text plus regular-expression matching of Maven dependency blocks. The two GHAS findings became resolved/outdated, and exact-head SAST is GREEN. No production behavior or security gate was weakened.

Current exact-head Maven/CI acceptance also proves the PDFBox exclusion preserves representative PDF parsing/generation behavior, zero-missed owned production line/branch coverage, warning-free public Javadocs, packaging, and the existing Java/script contract suite.

Scope and follow-through

Changed files:

  1. pom.xml
  2. src/test/java/com/clearfolio/viewer/config/CommonsLoggingRuntimeBindingTest.java
  3. scripts/test_commons_logging_dependency_contract.py

This slice removes one duplicate runtime binding and protects its dependency intent. It does not claim that every future packaging profile is automatically clean. Release acceptance must still verify the exact packaged dependency graph, startup diagnostics, CycloneDX SBOM, license policy, third-party attribution and provenance after later dependency changes. PR #305 remains the canonical documentation and release-traceability writer.

Merge gate

Keep this exact head unchanged. Merge only after live repository protection still sees every applicable required check passing, zero valid unresolved findings, and the counted approving review from a qualifying independent reviewer with write access. GHAS COMMENTED, automated checks and model output are not approval.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 38 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fd2c543e-305e-4685-9dee-3832ceb71018

📥 Commits

Reviewing files that changed from the base of the PR and between 55d7ae8 and 523dd84.

📒 Files selected for processing (3)
  • pom.xml
  • scripts/test_commons_logging_dependency_contract.py
  • src/test/java/com/clearfolio/viewer/config/CommonsLoggingRuntimeBindingTest.java

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 10, 2026 13:14
@seonghobae
seonghobae enabled auto-merge (squash) August 10, 2026 13:16

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently review exact current head c5d2c40809cf6ede5d411dcd1a9f38e993a48e0c against protected main 55d7ae8647208e301f282350f076eeddaba61d11. Exact-head CI 31391274361, Security Scan 31391275148, SAST 31391274360, and fuzz 31391274615 are successful. Verify the runtime resource test requires exactly one LogFactory provider from spring-jcl, the production change excludes only PDFBox's standalone commons-logging, representative PDFBox behavior and the 100%/Javadoc gates remain intact, and no warning suppression or redundant bridge was added. Advisory review only; do not mutate, count as human approval, merge, or weaken gates.

Comment thread scripts/test_commons_logging_dependency_contract.py Fixed
Comment thread scripts/test_commons_logging_dependency_contract.py Fixed
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