Fix Open Container file copy crashes in Wine - #1784
Conversation
📝 WalkthroughWalkthroughThe change adds native Win32 copying to WFM, hardens path and filesystem handling, adds conflict policies, and installs the bundled patched ChangesWFM copy deployment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant XServerScreen
participant WfmInstaller
participant WfmAsset
participant ContainerWindows
XServerScreen->>WfmInstaller: Install WFM before selecting a WFM command
WfmInstaller->>ContainerWindows: Check installed wfm.exe hash
WfmInstaller->>WfmAsset: Read bundled wfm.exe
WfmInstaller->>ContainerWindows: Verify and atomically replace wfm.exe
WfmInstaller-->>XServerScreen: Installation result
XServerScreen-->>XServerScreen: Show failure and reject WFM startup when installation fails
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app/src/main/assets/wfm/native-copy.patch (1)
1-9: 📐 Maintainability & Code Quality | 🔵 TrivialDocument the reproducible build procedure for the bundled
wfm.exe.
WfmInstaller.ktpinswfm.exeto the SHA-256cc7b5b77.... These patch assets record the upstream commit but not the toolchain, compiler flags, or patch application order needed to reproduce that exact binary. Without that, no reviewer can verify the shipped executable matches the patches, and the next hash update becomes guesswork.Add a short README in
app/src/main/assets/wfm/that lists the patch order (native-copy.patch,replace-all-conflicts.patch,copy-hardening.patch), thegit applyflags for each, and the exact build command.🤖 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 `@app/src/main/assets/wfm/native-copy.patch` around lines 1 - 9, Add a short README in the wfm assets directory documenting reproducible wfm.exe creation: identify the pinned upstream commit, apply native-copy.patch, replace-all-conflicts.patch, and copy-hardening.patch in that order with the required git apply flags for each, and provide the exact toolchain/compiler build command needed to produce the SHA-256 pinned by WfmInstaller.kt.app/src/main/assets/wfm/copy-hardening.patch (1)
264-272: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRegenerate this patch with context lines instead of
--unidiff-zero.This hunk closes a reparse-point check and the next hunk appends
else createdDestination = true;to theif (!CreateDirectory(...))block. Neither hunk carries context lines, sogit apply --unidiff-zeroplaces them purely by absolute line number. Those line numbers depend onnative-copy.patchandreplace-all-conflicts.patchapplying at exactly the expected offsets.If any earlier patch shifts by one line, these hunks land at the wrong place and the brace nesting changes without a conflict being reported. The build product is pinned by SHA-256 in
WfmInstaller.kt, so a silent misapply would still be shipped.Regenerate all three patches with standard 3-line context, or vendor the patched
file_actions.cdirectly so the result is reviewable.🤖 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 `@app/src/main/assets/wfm/copy-hardening.patch` around lines 264 - 272, Regenerate copy-hardening.patch and the related native-copy.patch and replace-all-conflicts.patch hunks with standard three-line context instead of --unidiff-zero, ensuring the reparse-point check and the createdDestination assignment remain attached to their intended blocks. Alternatively, vendor the fully patched file_actions.c directly so patch placement and brace nesting are reviewable.
🤖 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 `@app/src/main/assets/wfm/copy-hardening.patch`:
- Around line 361-363: Remove the no-op trailing hunk after
createDesktopShortcuts, including the duplicated unchanged closing brace and its
associated absolute-line dependency; leave the meaningful patch hunks unchanged.
- Around line 342-349: The reparse-point branch in copyPathNative must skip only
the unsupported entry instead of aborting the entire paste. Introduce or reuse a
distinct skipped-result status, propagate it through copyDirectoryNative so
enumeration continues, and ensure fileActionTask continues processing remaining
items while accumulating skipped entries. Report a single summary after the
operation completes, while preserving failure handling for genuine copy errors.
- Around line 68-76: Replace swprintf_s in joinCopyPath with _snwprintf_s using
the _TRUNCATE mode and preserve the existing negative-result failure check.
Apply the same formatting change to the message call at
app/src/main/assets/wfm/copy-hardening.patch lines 47-55; both sites should
treat any negative return as truncation without invoking the invalid parameter
handler.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 4014-4017: Ensure WfmInstaller.install(context, container) runs
before every wfm.exe launch path, including Custom Games without a detectable
executable and Steam games with no executablePath, rather than only when
bootToContainer is true. Move the installation call to the shared pre-launch
flow or invoke it unconditionally, while preserving the existing failure
logging.
In `@app/src/main/java/app/gamenative/utils/WfmInstaller.kt`:
- Around line 47-86: Update the existing fast-path condition in installPayload
so it also requires destination.canExecute() to be true, alongside
destination.isFile and the matching SHA-256 hash. This ensures matching files
with missing executable permissions proceed through the reinstall and chmod
path.
---
Nitpick comments:
In `@app/src/main/assets/wfm/copy-hardening.patch`:
- Around line 264-272: Regenerate copy-hardening.patch and the related
native-copy.patch and replace-all-conflicts.patch hunks with standard three-line
context instead of --unidiff-zero, ensuring the reparse-point check and the
createdDestination assignment remain attached to their intended blocks.
Alternatively, vendor the fully patched file_actions.c directly so patch
placement and brace nesting are reviewable.
In `@app/src/main/assets/wfm/native-copy.patch`:
- Around line 1-9: Add a short README in the wfm assets directory documenting
reproducible wfm.exe creation: identify the pinned upstream commit, apply
native-copy.patch, replace-all-conflicts.patch, and copy-hardening.patch in that
order with the required git apply flags for each, and provide the exact
toolchain/compiler build command needed to produce the SHA-256 pinned by
WfmInstaller.kt.
🪄 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: 55b43aa3-e022-42fd-9a43-01262cdfbd4b
⛔ Files ignored due to path filters (1)
app/src/main/assets/wfm/wfm.exeis excluded by!**/*.exe
📒 Files selected for processing (7)
app/src/main/assets/wfm/LICENSE.WFM.txtapp/src/main/assets/wfm/NOTICE.txtapp/src/main/assets/wfm/copy-hardening.patchapp/src/main/assets/wfm/native-copy.patchapp/src/main/assets/wfm/replace-all-conflicts.patchapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/WfmInstaller.kt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/src/main/assets/wfm/copy-hardening.patch`:
- Around line 99-129: Update isCopyDestinationDescendant to resolve mapped-drive
aliases before comparing paths, using resolved filesystem identities or final
paths for the destination’s nearest existing ancestor. Ensure a destination that
maps inside the source is rejected even when source and destination use
different drive letters, while preserving the existing descendant and
path-validation behavior.
🪄 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: d0f5e887-a205-4efd-98a7-b9d721aa3f2d
⛔ Files ignored due to path filters (1)
app/src/main/assets/wfm/wfm.exeis excluded by!**/*.exe
📒 Files selected for processing (8)
app/src/main/assets/wfm/NOTICE.txtapp/src/main/assets/wfm/README.mdapp/src/main/assets/wfm/copy-hardening.patchapp/src/main/assets/wfm/native-copy.patchapp/src/main/assets/wfm/replace-all-conflicts.patchapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/WfmInstaller.ktapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/assets/wfm/NOTICE.txt
- app/src/main/java/app/gamenative/utils/WfmInstaller.kt
- app/src/main/assets/wfm/replace-all-conflicts.patch
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/utils/WfmInstaller.kt (1)
65-78: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFall back on atomic move failure for an existing target.
With
StandardCopyOption.ATOMIC_MOVE, option dependencies likeREPLACE_EXISTINGare ignored. Ifdestinationexists and supports atomic moves, thisFiles.movecan throwIOExceptionwithout reaching the atomic-move fallback. Catch the atomic attempt’sIOException, then retry withREPLACE_EXISTING.🤖 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 `@app/src/main/java/app/gamenative/utils/WfmInstaller.kt` around lines 65 - 78, Update the move logic in WfmInstaller to catch the atomic attempt’s IOException, not only AtomicMoveNotSupportedException, so existing destinations can fall back to the non-atomic move with REPLACE_EXISTING. Preserve the current fallback move options and avoid swallowing failures from the retry.
🤖 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.
Outside diff comments:
In `@app/src/main/java/app/gamenative/utils/WfmInstaller.kt`:
- Around line 65-78: Update the move logic in WfmInstaller to catch the atomic
attempt’s IOException, not only AtomicMoveNotSupportedException, so existing
destinations can fall back to the non-atomic move with REPLACE_EXISTING.
Preserve the current fallback move options and avoid swallowing failures from
the retry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a4eb1d36-ecd5-4ead-aaa4-939aee87d62c
⛔ Files ignored due to path filters (1)
app/src/main/assets/wfm/wfm.exeis excluded by!**/*.exe
📒 Files selected for processing (6)
app/src/main/assets/wfm/NOTICE.txtapp/src/main/assets/wfm/README.mdapp/src/main/assets/wfm/copy-hardening.patchapp/src/main/assets/wfm/native-copy.patchapp/src/main/assets/wfm/replace-all-conflicts.patchapp/src/main/java/app/gamenative/utils/WfmInstaller.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/main/assets/wfm/native-copy.patch
- app/src/main/assets/wfm/replace-all-conflicts.patch
- app/src/main/assets/wfm/NOTICE.txt
- app/src/main/assets/wfm/README.md
- app/src/main/assets/wfm/copy-hardening.patch
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/assets/wfm/copy-hardening.patch">
<violation number="1" location="app/src/main/assets/wfm/copy-hardening.patch:99">
P2: Copies through a long resolved junction/mapped-drive path can bypass the self-descendant check and recurse into the source tree. Size the final-path buffer from `GetFinalPathNameByHandle`’s required length (or fail closed) instead of treating paths over 1024 characters as unrelated.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| + return length >= 0 && (size_t)length < destinationSize; | ||
| +} | ||
| + | ||
| +#define COPY_RESOLVED_PATH_SIZE 1024 |
There was a problem hiding this comment.
P2: Copies through a long resolved junction/mapped-drive path can bypass the self-descendant check and recurse into the source tree. Size the final-path buffer from GetFinalPathNameByHandle’s required length (or fail closed) instead of treating paths over 1024 characters as unrelated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/assets/wfm/copy-hardening.patch, line 99:
<comment>Copies through a long resolved junction/mapped-drive path can bypass the self-descendant check and recurse into the source tree. Size the final-path buffer from `GetFinalPathNameByHandle`’s required length (or fail closed) instead of treating paths over 1024 characters as unrelated.</comment>
<file context>
@@ -96,6 +96,82 @@ index 45a2c60..d657bd8 100644
+ return length >= 0 && (size_t)length < destinationSize;
+}
+
++#define COPY_RESOLVED_PATH_SIZE 1024
++
++static bool isCopyPathSeparator(wchar_t value) {
</file context>
1 participant
Description
Fix file copy crashes in the Open Container file manager when copying between Wine-mapped drives.
The first file conflict now also offers Replace all, avoiding repeated confirmation prompts when installing mods containing many existing files.
The issue I and another user came across:
Open Container launches Winlator File Manager inside the game's Wine container. WFM delegates copy operations to shell32's
SHFileOperation.On some Wine/Proton configurations, pasting files between mapped drives for example, from D: to A: causes WFM to terminate instead of displaying the expected replacement prompt. This was reproduced with games including Unreal Tournament GOTY and Fields of Mistria.
Recording
VN20260730_215630.mp4
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Fixes Open Container copy crashes under Wine by switching WFM copy to native Win32 and auto-installing the patched
wfm.exe. Resolves mapped-drive aliases to prevent copying folders into their own descendants, adds a first-conflict “Replace all,” and improves handling for symlinks, read-only files, long paths, and errors.Bug Fixes
SHFileOperationwith nativeCopyFileforACTION_COPYto stop crashes between mapped drives.New Features
WfmInstallerinstalls the patchedwfm.exeat container boot with SHA-256 verification; Open Container launch is gated and shows a snackbar if installation fails. IncludesLICENSE.WFM.txt,NOTICE.txt, andREADME.mdwith reproducible build steps.Written for commit ec11ed8. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation