Next patch release (v2.1.2) - #355
Conversation
Bumps `jacksonVersion` from 2.22.0 to 2.22.1. Updates `com.fasterxml.jackson.core:jackson-databind` from 2.22.0 to 2.22.1 - [Commits](https://github.com/FasterXML/jackson/commits) Updates `com.fasterxml.jackson.datatype:jackson-datatype-jsr310` from 2.22.0 to 2.22.1 --- updated-dependencies: - dependency-name: com.fasterxml.jackson.core:jackson-databind dependency-version: 2.22.1 dependency-type: direct:production - dependency-name: com.fasterxml.jackson.datatype:jackson-datatype-jsr310 dependency-version: 2.22.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…ndows" This reverts commit e75c9b2.
…ot-read-back-in fix: encoded paths are not read back
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughWriters now decode entity IDs for folder and ZIP filenames. ChangesEncoded filename resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CrateReader
participant IdentifierUtils
participant CrateFolder
participant WriteFolderStrategy
CrateReader->>IdentifierUtils: Decode entity ID
IdentifierUtils-->>CrateReader: Decoded candidate
CrateReader->>CrateFolder: Check decoded and original filenames
CrateFolder-->>CrateReader: Existing in-folder path
WriteFolderStrategy->>IdentifierUtils: Decode entity ID for output
IdentifierUtils-->>WriteFolderStrategy: Decoded filename
WriteFolderStrategy->>CrateFolder: Copy entity to decoded filename
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
…ut-7.x chore(deps): update actions/checkout action to v7.0.1
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java (1)
145-150: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove debug
System.out.println.This runs for every resolved data entity on every crate read and writes to stdout unconditionally. A
loggeris already available on this class (logger.debug(...)), or this line should simply be dropped before release.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java` around lines 145 - 150, Remove the unconditional System.out.println from the checkFolderHasFile callback in CrateReader; rely on the existing logger.debug only if this diagnostic is still needed, while preserving the usedFiles and builder updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java`:
- Around line 72-82: Validate decoded destination names before writing in both
strategies: in WriteFolderStrategy.saveToFile, normalize the resolved path under
the destination folder and reject it if it does not remain contained, mirroring
CrateReader.checkFolderHasFile; in WriteZipStreamStrategy at the decoded
filename handling, reject or safely strip absolute markers and traversal
segments before constructing the zip entry name. Apply the changes to
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java lines
72-82 and
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.java
lines 152-171.
---
Outside diff comments:
In `@src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java`:
- Around line 145-150: Remove the unconditional System.out.println from the
checkFolderHasFile callback in CrateReader; rely on the existing logger.debug
only if this diagnostic is still needed, while preserving the usedFiles and
builder updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db7edca6-8df9-430c-8400-2cc6285c7098
📒 Files selected for processing (6)
src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataEntity.javasrc/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.javasrc/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.javasrc/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.javasrc/test/java/edu/kit/datamanager/ro_crate/crate/ReadAndWriteTest.javasrc/test/java/edu/kit/datamanager/ro_crate/writer/CommonWriterTest.java
| private void saveToFile(DataEntity entity, File file) throws IOException { | ||
| if (entity.getPath() != null) { | ||
| String id = entity.getId(); | ||
| String filename = IdentifierUtils.decode(id).orElse(id); | ||
| if (entity.getPath().toFile().isDirectory()) { | ||
| FileUtils.copyDirectory(entity.getPath().toFile(), file.toPath().resolve(entity.getId()).toFile()); | ||
| FileUtils.copyDirectory(entity.getPath().toFile(), file.toPath().resolve(filename).toFile()); | ||
| } else { | ||
| FileUtils.copyFile(entity.getPath().toFile(), file.toPath().resolve(entity.getId()).toFile()); | ||
| FileUtils.copyFile(entity.getPath().toFile(), file.toPath().resolve(filename).toFile()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Unvalidated decoded id used to build output paths in both writer strategies. Both writers decode the entity id and use it directly to construct a destination location, without the containment check that CrateReader.checkFolderHasFile applies on the read side for the same reason ("defence-in-depth: ensure we are still inside the crate folder"). A decoded id that is absolute, or contains ../, escapes the intended output location (arbitrary file write on folder-write, Zip Slip on zip-write).
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java#L72-L82: after computingfilename, normalize/resolve against the destination folder and reject (or throw) if the result doesn't stay under it, mirroring the reader'sstartsWithguard.src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.java#L152-L171: reject (or strip) decoded filenames containing traversal segments/absolute markers before using them to build the zip entry name.
📍 Affects 2 files
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java#L72-L82(this comment)src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.java#L152-L171
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java`
around lines 72 - 82, Validate decoded destination names before writing in both
strategies: in WriteFolderStrategy.saveToFile, normalize the resolved path under
the destination folder and reject it if it does not remain contained, mirroring
CrateReader.checkFolderHasFile; in WriteZipStreamStrategy at the decoded
filename handling, reject or safely strip absolute markers and traversal
segments before constructing the zip entry name. Apply the changes to
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java lines
72-82 and
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.java
lines 152-171.
…onVersion-2.22.1 chore(deps): bump jacksonVersion from 2.22.0 to 2.22.1
…action-4.x chore(deps): update github/codeql-action action to v4.37.3
…k-monorepo fix(deps): update dependency org.junit:junit-bom to v6.1.2
…java-5.x chore(deps): update actions/setup-java action to v5.6.0
1 participant
Summary by CodeRabbit
Bug Fixes
Documentation