Skip to content

Allow macos to have vhd/x as a supported file type#7990

Merged
bcarrier merged 2 commits intosleuthkit:developfrom
markmckinnon:Allow-macos-to-have-vhd/x-as-a-supported-file-type
Apr 6, 2026
Merged

Allow macos to have vhd/x as a supported file type#7990
bcarrier merged 2 commits intosleuthkit:developfrom
markmckinnon:Allow-macos-to-have-vhd/x-as-a-supported-file-type

Conversation

@markmckinnon
Copy link
Copy Markdown
Contributor

@markmckinnon markmckinnon commented Nov 25, 2025

Summary by CodeRabbit

  • Improvements

    • Virtual machine support is now consistently available across all operating systems.
  • Chores

    • Updated internal dependencies and code structure for better maintainability.

remove check if a mac to not add virtual machine extensions.
Change deprecated methods to supported methods
@bcarrier
Copy link
Copy Markdown
Member

bcarrier commented Apr 6, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 6, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 6, 2026

📝 Walkthrough

Walkthrough

Modified ImageDSProcessor to unconditionally enable virtual machine support detection (previously gated to non-macOS systems) and refactored password defaulting logic from StringUtils to Objects utility methods, while updating associated imports.

Changes

Cohort / File(s) Summary
Virtual Machine Support & Password Handling
Core/src/org/sleuthkit/autopsy/casemodule/ImageDSProcessor.java
Removed OS-specific gating that conditionally added virtual machine filter only on non-macOS systems; replaced StringUtils.defaultString() with Objects.toString() across three call sites for password resolution; updated imports from StringUtils to Objects.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 VM support now hops freely, no OS walls remain,
Objects.toString() replaces StringUtils with care,
Imports cleaned up, the code flows plain—
One hopping change, light and fair! 🌿

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main change: enabling macOS to support VHD/VHDX file types by removing the OS-based gating that previously excluded these formats on macOS.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PMD (7.23.0)
Core/src/org/sleuthkit/autopsy/casemodule/ImageDSProcessor.java

[ERROR] Cannot load ruleset rulesets/java/basic.xml/SimplifiedTernary: Cannot resolve rule/ruleset reference 'rulesets/java/basic.xml/SimplifiedTernary'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath.
[WARN] Progressbar rendering conflicts with reporting to STDOUT. No progressbar will be shown. Try running with argument -r to output the report to a file instead.


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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
Core/src/org/sleuthkit/autopsy/casemodule/ImageDSProcessor.java (1)

580-588: Consider applying the same null-defaulting pattern here for consistency.

Lines 215, 325, and 548 use Objects.toString(password, this.password) to preserve the existing password when the parameter is null. However, line 587 uses direct assignment this.password = password;, which would overwrite any existing password with null.

If this difference is intentional (e.g., this method should explicitly clear the password when null is passed), consider adding a comment to clarify. Otherwise, applying the same pattern would ensure consistent behavior across all entry points.

♻️ Suggested change for consistency
     public IngestStream processWithIngestStream(String deviceId, Path dataSourcePath, String password, Host host, IngestJobSettings settings, DataSourceProcessorProgressMonitor progressMonitor, DataSourceProcessorCallback callBack) {
         // this method does not use the config panel
         this.deviceId = deviceId;
         this.imagePath = dataSourcePath.toString();
         this.sectorSize = 0;
         this.timeZone = Calendar.getInstance().getTimeZone().getID();
         this.host = host;
-        this.password = password;
+        this.password = Objects.toString(password, this.password);
         this.ignoreFatOrphanFiles = false;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Core/src/org/sleuthkit/autopsy/casemodule/ImageDSProcessor.java` around lines
580 - 588, The processWithIngestStream method currently assigns this.password =
password which will overwrite an existing password with null; change it to use
the same null-defaulting pattern used elsewhere (e.g.,
Objects.toString(password, this.password)) so that a null parameter preserves
the existing this.password, or add a clarifying comment in
processWithIngestStream if the intent is to explicitly clear the password;
locate the password assignment in processWithIngestStream and update it to the
Objects.toString(...) pattern (or comment the explicit-null behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@Core/src/org/sleuthkit/autopsy/casemodule/ImageDSProcessor.java`:
- Around line 580-588: The processWithIngestStream method currently assigns
this.password = password which will overwrite an existing password with null;
change it to use the same null-defaulting pattern used elsewhere (e.g.,
Objects.toString(password, this.password)) so that a null parameter preserves
the existing this.password, or add a clarifying comment in
processWithIngestStream if the intent is to explicitly clear the password;
locate the password assignment in processWithIngestStream and update it to the
Objects.toString(...) pattern (or comment the explicit-null behavior).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b3bc1427-bd4f-4a62-9b1f-1d2e4da2cc04

📥 Commits

Reviewing files that changed from the base of the PR and between 1775aa0 and 85eb7a4.

📒 Files selected for processing (1)
  • Core/src/org/sleuthkit/autopsy/casemodule/ImageDSProcessor.java

@bcarrier bcarrier merged commit 3d0f2b8 into sleuthkit:develop Apr 6, 2026
1 check passed
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