Complete JVM and Kotlin binding evidence#199
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 42 minutes and 1 second. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (23)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR adds JVM/Kotlin Gradle binding package and promotes Kotlin/Native binding support. The implementation includes contract boundary adapters, Maven Central publication configuration, and comprehensive test coverage.
Critical Security Issues
Found path traversal vulnerabilities in the JVM file boundary validation that must be addressed before merge. The validation logic does not prevent malicious path traversal sequences in both input and output paths, allowing potential unauthorized file system access. These vulnerabilities could allow an attacker to read arbitrary files or create directories in unintended locations.
Additionally, the PII exposure in the build configuration should be addressed to avoid hardcoding personal information in the repository.
Validation
The PR includes comprehensive test coverage and Maven Central publication evidence as documented in the description.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| val parent = request.outputPath.parent | ||
| if (parent != null && !Files.exists(parent)) { | ||
| Files.createDirectories(parent) | ||
| } |
There was a problem hiding this comment.
🛑 Path Traversal Vulnerability: Validate that output path does not escape the intended directory before creating directories. An attacker could provide path traversal sequences (e.g., "../../../etc/passwd") causing Files.createDirectories to create directories in arbitrary locations on the filesystem.1
| val parent = request.outputPath.parent | |
| if (parent != null && !Files.exists(parent)) { | |
| Files.createDirectories(parent) | |
| } | |
| val parent = request.outputPath.parent | |
| if (parent != null) { | |
| val normalizedParent = parent.normalize() | |
| if (normalizedParent.startsWith("..") || normalizedParent.isAbsolute) { | |
| throw BoundaryValidationException("output_path must not contain path traversal or absolute paths: $parent") | |
| } | |
| if (!Files.exists(normalizedParent)) { | |
| Files.createDirectories(normalizedParent) | |
| } | |
| } |
Footnotes
-
CWE-22: Path Traversal - https://cwe.mitre.org/data/definitions/22.html ↩
| if (!Files.isRegularFile(request.inputPath)) { | ||
| throw BoundaryValidationException("input_path must reference an existing file: ${request.inputPath}") | ||
| } |
There was a problem hiding this comment.
🛑 Path Traversal Vulnerability: Validate that input path does not escape the intended directory. An attacker could provide path traversal sequences allowing unauthorized file system access.1
| if (!Files.isRegularFile(request.inputPath)) { | |
| throw BoundaryValidationException("input_path must reference an existing file: ${request.inputPath}") | |
| } | |
| val normalizedInputPath = request.inputPath.normalize() | |
| if (normalizedInputPath.startsWith("..") || normalizedInputPath.isAbsolute) { | |
| throw BoundaryValidationException("input_path must not contain path traversal or absolute paths: ${request.inputPath}") | |
| } | |
| if (!Files.isRegularFile(normalizedInputPath)) { | |
| throw BoundaryValidationException("input_path must reference an existing file: ${normalizedInputPath}") | |
| } |
Footnotes
-
CWE-22: Path Traversal - https://cwe.mitre.org/data/definitions/22.html ↩
| id.set("edithatogo") | ||
| name.set("Dylan Mordaunt") | ||
| name.set("edithatogo") | ||
| email.set("d.a.mordaunt@gmail.com") |
There was a problem hiding this comment.
🛑 Security Risk - PII Exposure: Replace the hardcoded personal email address with a generic organizational email address or variable reference. Hardcoding personal emails in build configurations exposes PII and makes it difficult to update when developer information changes.1
| email.set("d.a.mordaunt@gmail.com") | |
| email.set(providers.gradleProperty("developerEmail").orElse("contact@mchs.org").get()) |
Footnotes
-
CWE-359: Exposure of Private Personal Information - https://cwe.mitre.org/data/definitions/359.html ↩
| val parent = request.outputPath.parent | ||
| if (parent != null && !Files.exists(parent)) { | ||
| Files.createDirectories(parent) | ||
| } | ||
|
|
||
| if (parent != null && !Files.isDirectory(parent)) { | ||
| throw BoundaryValidationException("output_path parent is not a directory: $parent") | ||
| } |
There was a problem hiding this comment.
🛑 Logic Error: Check for parent directory after normalization to prevent potential race condition or incorrect validation. The code validates parent exists/isDirectory after creating it, but parent could be null or the check could fail if creation didn't complete properly.1
| val parent = request.outputPath.parent | |
| if (parent != null && !Files.exists(parent)) { | |
| Files.createDirectories(parent) | |
| } | |
| if (parent != null && !Files.isDirectory(parent)) { | |
| throw BoundaryValidationException("output_path parent is not a directory: $parent") | |
| } | |
| val parent = request.outputPath.parent | |
| if (parent != null) { | |
| val normalizedParent = parent.normalize() | |
| if (normalizedParent.startsWith("..") || normalizedParent.isAbsolute) { | |
| throw BoundaryValidationException("output_path must not contain path traversal or absolute paths: $parent") | |
| } | |
| if (!Files.exists(normalizedParent)) { | |
| Files.createDirectories(normalizedParent) | |
| } | |
| if (!Files.isDirectory(normalizedParent)) { | |
| throw BoundaryValidationException("output_path parent is not a directory: $normalizedParent") | |
| } | |
| } |
Footnotes
-
CWE-367: Time-of-check Time-of-use (TOCTOU) Race Condition - https://cwe.mitre.org/data/definitions/367.html ↩
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request completes the JVM Maven Central submission and Kotlin/Native binding tracks, transitioning them to completed status. It updates the JVM module to target Java 11, configures Maven Central publishing with in-memory PGP signing, and introduces a JvmFileBoundaryAdapter for path and calculator validation. Similarly, the Kotlin/Native module implements a FileBoundaryNativeBindingClient that performs envelope and path safety validation. The PR also records publication evidence for io.github.edithatogo:mchs-jvm-bindings:0.1.0 and adds msgpack as a dependency. Feedback on the changes suggests refactoring the validate method in JvmFileBoundaryAdapter to be side-effect free by moving directory creation to the execute method.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fun execute(request: JvmBindingRequest): JvmBindingResponse { | ||
| validate(request) | ||
| return sharedCoreAdapter.execute(request) | ||
| } | ||
|
|
||
| fun validate(request: JvmBindingRequest) { | ||
| if (request.calculatorId !in ContractBoundary.supportedCalculators) { | ||
| throw BoundaryValidationException("Unsupported calculator_id: ${request.calculatorId}") | ||
| } | ||
|
|
||
| if (request.pricingYear.isBlank()) { | ||
| throw BoundaryValidationException("pricing_year must not be blank") | ||
| } | ||
|
|
||
| if (!Files.isRegularFile(request.inputPath)) { | ||
| throw BoundaryValidationException("input_path must reference an existing file: ${request.inputPath}") | ||
| } | ||
|
|
||
| val parent = request.outputPath.parent | ||
| if (parent != null && !Files.exists(parent)) { | ||
| Files.createDirectories(parent) | ||
| } | ||
|
|
||
| if (parent != null && !Files.isDirectory(parent)) { | ||
| throw BoundaryValidationException("output_path parent is not a directory: $parent") | ||
| } | ||
| } |
There was a problem hiding this comment.
The validate method should be side-effect free. Currently, it creates directories on the filesystem via Files.createDirectories(parent) if they do not exist. This is unexpected behavior for a validation helper, which callers might invoke simply to check request pre-conditions without intending to modify the disk state.\n\nIt is highly recommended to keep validate side-effect free by only verifying that the parent is a directory if it already exists, and deferring the directory creation to the execute method.
fun execute(request: JvmBindingRequest): JvmBindingResponse {\n validate(request)\n val parent = request.outputPath.parent\n if (parent != null && !Files.exists(parent)) {\n Files.createDirectories(parent)\n }\n return sharedCoreAdapter.execute(request)\n }\n\n fun validate(request: JvmBindingRequest) {\n if (request.calculatorId !in ContractBoundary.supportedCalculators) {\n throw BoundaryValidationException(\"Unsupported calculator_id: ${request.calculatorId}\")\n }\n\n if (request.pricingYear.isBlank()) {\n throw BoundaryValidationException(\"pricing_year must not be blank\")\n }\n\n if (!Files.isRegularFile(request.inputPath)) {\n throw BoundaryValidationException(\"input_path must reference an existing file: ${request.inputPath}\")\n }\n\n val parent = request.outputPath.parent\n if (parent != null && Files.exists(parent) && !Files.isDirectory(parent)) {\n throw BoundaryValidationException(\"output_path parent is not a directory: $parent\")\n }\n }![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d4176f007
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| description = "Checks Maven Central Portal readiness without uploading or requiring credentials." | ||
|
|
||
| dependsOn( | ||
| "checkPomFileForMavenPublication", |
There was a problem hiding this comment.
When validateCentralPortalReadiness is run, Gradle tries to resolve checkPomFileForMavenPublication and fails before any readiness checks run; the Maven Publish plugin tasks documented for a Maven publication are generatePomFileFor..., publish..., and publishToMavenLocal, and a local gradle tasks --all check for maven-publish/signing also does not create a checkPomFile... task. This breaks the validation command recorded by this change (gradle -p bindings/jvm validateCentralPortalReadiness build) for the JVM binding.
Useful? React with 👍 / 👎.
| if (request.calculatorId != "nwau") { | ||
| add("calculatorId must be nwau.") | ||
| } |
There was a problem hiding this comment.
The Kotlin/Native contract is aligned to the public calculator contract and its passing fixture uses calculator_id: "acute", while the JVM boundary added in this same commit accepts the public calculator IDs such as acute, adjust, and subacute. With this hard-coded nwau check, any Kotlin/Native request shaped like the documented contract examples is returned as BLOCKED even when the rest of the envelope is valid.
Useful? React with 👍 / 👎.
1d4176f to
3a54fd3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Validation