Skip to content

Replace calling reg tool with FFI - #233

Merged
infeo merged 1 commit into
developfrom
feature/winfsp-util-ffi
Sep 28, 2026
Merged

infeo merged 1 commit into
developfrom
feature/winfsp-util-ffi

Conversation

@infeo

@infeo infeo commented Sep 28, 2026

Copy link
Copy Markdown
Member

Accessing Windows reg tool might be blocked by group policies, thus WinFSP detection can fail. This PR replaces the reg call with native calls.

@infeo infeo self-assigned this Sep 28, 2026
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 79954d3e-9f9a-4552-af6c-dc1c380d1068

📥 Commits

Reviewing files that changed from the base of the PR and between af8d9fa and c7ee9b6.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • README.md
  • src/main/java/org/cryptomator/frontend/fuse/mount/WinFspUtil.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

WinFspUtil now reads the WinFsp installation directory from the Windows registry with RegGetValueW through FFM, instead of launching reg.exe. If a registry call or another operation fails, the method returns FALLBACK_PATH. The README and changelog document the lookup and the required Java native-access flags.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to c7ee9

The WinFsp lookup change is mergeable after normal checks; no demonstrated installation-detection regression remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c7ee9

The lookup has fixed registry inputs and retains the existing DLL path selection, so no new attacker-controlled route to native code is established. Windows deployments still need to account for the new native-access requirement: if the lookup is denied, mounting can proceed using the fallback location.

Retained concerns

  • Low · security · inferred: If a Windows runtime denies the newly needed native call, the lookup silently selects the existing fallback directory. If its DLL exists, support detection can succeed and mounting can use that DLL without a successful registry lookup. Whether production launchers permit the call, or the fallback differs from the intended installation, is unverified.
Security review details

Security Blast Radius

  • inferred — The evidenced exposure is the local Windows mount process’s selection of a WinFsp native DLL. The changed lookup adds no caller-supplied registry key or library name and shows no new cross-service or tenant path.

Security Findings and Attack Paths

  • inferred — No new attacker-controlled path to registry writes or an arbitrary native symbol is established. A denied native lookup can increase use of the pre-existing fallback DLL path, but an unsafe DLL or affected production deployment has not been demonstrated.

Trust Boundaries and Controls

  • observed — Native access moves from a separate reg.exe process into the JVM module. The Windows registry value remains the installation-path source; the provider’s existence check does not verify DLL provenance or that the registry query succeeded.

Resilience and Maintainability Implications

  • inferred — Because the selected path is cached and lookup failures only produce debug logging, a native-access denial can remain indistinguishable to the provider from an ordinary fallback installation for the process lifetime.

Hardening Proposals

  • proposed — Verify the documented module-scoped native-access setting in consuming Windows launchers, and make native-access denial distinguishable from an ordinary missing registry value before relying on a fallback DLL.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: replacing the Windows reg tool call with FFI for WinFsp detection.
Description check ✅ Passed The description explains the motivation and the implemented change. It directly relates to the pull request changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@infeo infeo added this to the next milestone Sep 28, 2026
@infeo
infeo merged commit cdccce4 into develop Sep 28, 2026
12 checks passed
@infeo
infeo deleted the feature/winfsp-util-ffi branch September 30, 2026 13:09
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.

1 participant