Repository navigation
Chore/security ci hardening - #7
Open
davidjbrossard wants to merge 6 commits into
Open
davidjbrossard wants to merge 6 commits into
davidjbrossard wants to merge 6 commits into
Conversation
…ependencies Client fixes: - JaxRsAuthZClient shared one Invocation.Builder across calls, which is not thread-safe: under concurrent use requests failed with duplicated Content-Type headers or ProcessingExceptions. It now builds a request per call from a shared WebTarget. The Invocation.Builder constructor and field are kept but deprecated; subclasses can override newRequest() to add headers. The client is now AutoCloseable. - DefaultClientConfiguration (and its builder) no longer print the password in toString(). - Both clients now reject a username without a password instead of silently sending no credentials (Feign) or an empty password (JAX-RS). - pdpUrl() is marked @deprecated; the builder error message names the correct method. Dependencies: - Import the Jackson and Jersey BOMs. Without them Maven's nearest-wins resolution gave consumers jackson-databind 2.10.1 (JAX-RS) or 2.12.3 (Feign) instead of the pinned version. - Replace springfox-swagger2 with swagger-annotations: only the annotations were used, and springfox pulled Spring 4.0.9 and Guava 20 onto every consumer's classpath. - Remove the unused commons-io dependency. - jackson 2.22.3, jersey 2.48, feign 13.15, slf4j 1.7.36, junit 4.13.2, and current Maven plugin versions. All remain Java 8 compatible; compile with --release 8 on JDK 9+. - Resolved runtime classpath goes from 50+ known advisories (OSV) to none. Tests: - Add tests for client-core, client-feign and client-jaxrs, run against an in-process stub PDP shared through a client-core test-jar, including a 400-call concurrency test that fails on the previous JAX-RS implementation. Build metadata: point url/scm to the axiomatics repository, add license and issue management, inherit module versions from the parent, and skip deploying the example modules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- ci.yml: build and test on Java 8, 11, 17, 21 and 25 for pushes to master and pull requests. - codeql.yml: CodeQL (java-kotlin, security-extended) on pushes, pull requests and weekly. - dependabot.yml: weekly grouped Maven and GitHub Actions updates; Jersey 3+ (jakarta namespace, Java 11) and slf4j 2.x majors are ignored. - release.yml: pushing a v<major>.<minor>.<patch> tag sets that version, publishes the SDK modules (with sources and javadoc) to GitHub Packages and creates a GitHub release with generated notes. - Maven wrapper 3.3.4 pinned to Maven 3.9.16 with a checksum. - README: thread-safety, build and release instructions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Compile with --release 17 (a single java.version.minimum property) and fail the build early on older JDKs with the enforcer plugin. - CI matrix: 17, 21 and 25. - README and comments updated; StubPdp uses InputStream.readAllBytes(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Jersey 2.48 -> 3.1.12 (Jakarta REST 3.1 / Jakarta EE 10). JaxRsAuthZClient now uses the jakarta.ws.rs namespace: a breaking change for code that passes a WebTarget or Invocation.Builder to it. - slf4j-api 1.7.36 -> 2.0.20. Applications need an slf4j 2.x provider (e.g. logback 1.3+ or slf4j-simple 2.x) to see the SDK's log output. - Dependabot no longer ignores Jersey and slf4j major updates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A null Invocation.Builder passed into the deprecated JaxRsAuthZClient(Invocation.Builder) constructor can still trigger a runtime NPE, and should be guarded with an explicit null check.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
This PR hardens the build/release/CI setup for the Java JSON PEP SDK while modernizing the codebase to Java 17+ and newer dependency versions (Jakarta/Jersey 3, updated Jackson/Feign/SLF4J), alongside improving client thread-safety guidance and adding regression tests.
Changes:
- Upgrade baseline to Java 17 (enforced via Maven Enforcer) and introduce Maven Wrapper for reproducible builds.
- Modernize dependencies and Jakarta namespace usage (Jersey 3.x, Jackson BOM import, Feign upgrades), plus tighten credential handling (mask passwords in
toString()and validate username/password pairing). - Add CI/release automation (CI matrix, CodeQL, Dependabot, release workflow) and add client test utilities + new test suites.
| File | Description |
|---|---|
| README.md | Adds thread-safety/closing guidance and build/release instructions. |
| pom.xml | Java 17+ enforcement, BOM-based dependency management, plugin/version modernization, metadata cleanup. |
| mvnw.cmd | Adds Maven Wrapper (Windows). |
| mvnw | Adds Maven Wrapper (POSIX). |
| .mvn/wrapper/maven-wrapper.properties | Pins wrapper and Maven distribution + SHA-256 checksum. |
| models/pom.xml | Removes old deps and aligns versions via parent/BOM; updates description text. |
| client-core/pom.xml | Publishes test-jar for shared test utilities. |
| client-core/src/main/java/io/xacml/pep/json/client/DefaultClientConfiguration.java | Masks passwords in toString(), improves error messages, deprecates pdpUrl(...). |
| client-core/src/test/java/io/xacml/pep/json/client/StubPdp.java | Adds in-process PDP stub for integration-style client tests. |
| client-core/src/test/java/io/xacml/pep/json/client/ConcurrentCalls.java | Adds concurrency runner for thread-safety tests. |
| client-core/src/test/java/io/xacml/pep/json/client/DefaultClientConfigurationTest.java | Adds tests for password masking and URL behavior. |
| client-jaxrs/pom.xml | Aligns module versions and pulls in shared test utilities. |
| client-jaxrs/src/main/java/io/xacml/pep/json/client/jaxrs/JaxRsAuthZClient.java | Switches to Jakarta client types, improves thread-safety by rebuilding requests per call, adds close() for owned client. |
| client-jaxrs/src/test/java/io/xacml/pep/json/client/jaxrs/JaxRsAuthZClientTest.java | Adds coverage for auth behavior, parsing, error handling, concurrency, and header overrides. |
| client-jaxrs-example/pom.xml | Skips deploy for examples; aligns module dependency version. |
| client-jaxrs-example/src/main/java/io/xacml/pep/json/client/jaxrs/AuthZClientExample.java | Uses try-with-resources for JaxRsAuthZClient and documents lifecycle. |
| client-feign/pom.xml | Aligns module versions and pulls in shared test utilities. |
| client-feign/src/main/java/io/xacml/pep/json/client/feign/FeignAuthZClient.java | Enforces username/password pairing for basic auth. |
| client-feign/src/main/java/io/xacml/pep/json/client/feign/PDPFeignClient.java | Minor import cleanup. |
| client-feign/src/test/java/io/xacml/pep/json/client/feign/FeignAuthZClientTest.java | Adds Feign client behavioral and concurrency tests. |
| client-feign-example/pom.xml | Skips deploy for examples; aligns module dependency version. |
| client-feign-example/src/main/java/io/xacml/pep/json/client/feign/AuthZClientExample.java | Fixes incorrect example comment (Feign vs JAX-RS). |
| .github/workflows/ci.yml | Adds CI workflow (Java 17/21/25 matrix). |
| .github/workflows/codeql.yml | Adds CodeQL scanning workflow. |
| .github/workflows/release.yml | Adds tag-driven release publishing + GitHub release creation. |
| .github/dependabot.yml | Adds Dependabot configuration for Maven + Actions updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Ensure requestInvocationBuilder is non-null in constructor. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The previous commit's null-check edit removed the constructor's original closing brace but left an extra one behind, breaking compilation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
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.
I used Claude to inspect our Java PEP SDK, upgrade to Java 17, and move to newer versions of dependencies.