Skip to content

Feature: Add Shell-compatible file transfer - #17

Closed
0x5bfa wants to merge 1 commit into
mainfrom
code-quality/ban-direct-file-system-access
Closed

Feature: Add Shell-compatible file transfer#17
0x5bfa wants to merge 1 commit into
mainfrom
code-quality/ban-direct-file-system-access

Conversation

@0x5bfa

@0x5bfa 0x5bfa commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Enforce storage boundaries in Files, Files.Controls, and Files.Operations with Banned API analyzers, while keeping stream contracts, I/O exceptions, and lexical Path operations available.
  • Remove WinRT StorageFile / StorageFolder path conversion from clipboard and drag-and-drop code.
  • Create the same native Shell IDataObject selection used by Explorer, including Shell namespace and virtual items, and preserve preferred drop effects and asynchronous clipboard behavior.
  • Keep WinRT only at the WinUI event / DataPackage bridge; forward the raw native data object through Shell IDropTarget sessions for folder views, item handlers, the sidebar, and breadcrumbs.
  • Support copy, move, link, keyboard modifiers, item-specific drop handlers, folder-background fallback, and right-button drag/drop.
  • Delegate paste acceptance and execution to the destination Shell folder's canonical paste command so delayed, virtual, and handler-specific formats are not filtered by the app.
  • Isolate explicitly requested item-properties metadata and attribute work behind a narrow filesystem adapter, route rename through IStorageOperationService, and remove the operation server's automatic debug_server.log write.

Resolved / Related Issues

  • No linked issue; this implements the requested storage boundary and Explorer-compatible file transfer behavior.

Steps used to test these changes

  1. Built the complete solution for Debug x64, including the native launcher, benchmarks, and all test projects; also built the main app and operation server for Release x64.
  2. Built the main app and operation server for Debug and Release ARM64, plus the ARM64 unit, WinUI, control, and accessibility test projects. The only unavailable ARM64 solution component was the native launcher because this machine does not have its v145/v143 ARM64 C++ toolsets installed.
  3. Ran all x64 unit tests: 281 total, 278 passed, 0 failed, and 3 clipboard tests were inconclusive because this Windows session denied OpenClipboard with error 5.
  4. Repeated the native Shell drag/drop integration matrix ten times: 90/90 passed. This exercises real file and folder copy/move, same- and mixed-parent selections, IDropTarget lifecycle and modifier negotiation, preferred effects, async data-object behavior, and the inbox ZIP Shell extension.
  5. Repeated the real WinUI DataPackage to native Shell IDataObject/IDropTarget bridge ten times: 10/10 passed.
  6. Ran 91/91 functional WinUI tests, 3/3 safe stress tests, and 1/1 synthetic performance test successfully.
  7. Added temporary negative probes for File, Directory, StorageFile.GetFileFromPathAsync, and StorageFolder.GetFolderFromPathAsync in each enforced project. All 12 usages were rejected with RS0030; the probes were then removed.
  8. Verified x64/ARM64 operation-server output contains its .exe, .dll, and .winmd, and completed final static checks for CRLF, line length, XML documentation, conflict markers, banned API leakage, and git diff --check.

The packaged-app, cross-process Explorer interaction matrix still requires an interactive Windows session with package registration enabled. In particular, right-drag menus, drag cancellation/hover behavior, elevated targets, OneDrive/MTP/network locations, and third-party Shell extensions remain manual end-to-end checks.

Copilot AI lite review requested due to automatic review settings September 4, 2026 03:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

ItemPropertiesViewModel.CanRename can report rename support even when no IStorageOperationService is available, leading to a predictable apply-time failure instead of disabling rename up front.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens the “storage boundary” in the WinUI app layers by preventing direct physical filesystem probing (e.g., File.Exists, Directory.Exists) in Files, Files.Controls, and Files.Operations, pushing that access behind storage abstractions/adapters and enforcing the rule via Banned API analyzers.

Changes:

  • Introduces repo-wide Banned API enforcement (RS0030 as error) for selected projects using Microsoft.CodeAnalysis.BannedApiAnalyzers + BannedSymbols.txt.
  • Refactors item properties to isolate filesystem metadata/attribute work behind IItemPropertiesFileSystem and routes rename through IStorageOperationService.
  • Removes/avoids direct filesystem probes in clipboard prep, sidebar drag setup, command-state evaluation, and removes Files.Operations’s on-disk debug logging; ensures Files.Operations is built in CI.
File summaries
File Description
tests/Files.UITests/PresentationTests/ItemPropertiesViewModelTests.cs Updates rename test to assert rename is routed via IStorageOperationService.
src/Files/ViewModels/FolderBrowserViewModel.cs Reworks copy/paste and “current folder” checks to avoid filesystem probing; removes byte-count probing.
src/Files/MainWindow.xaml.cs Wires IStorageOperationService into ItemPropertiesService.
src/Files/ItemProperties/ItemPropertiesWindow.xaml.cs Passes storage ops + filesystem adapter into the view model.
src/Files/ItemProperties/ItemPropertiesViewModel.cs Uses filesystem adapter for metadata/attribute operations and storage ops for rename.
src/Files/ItemProperties/ItemPropertiesService.cs Stores and supplies storage ops + filesystem adapter for item properties windows.
src/Files/ItemProperties/ItemPropertiesFileSystem.cs New adapter encapsulating the (now-isolated) direct filesystem/Win32 work for item properties.
src/Files/Infrastructure/FileClipboard.cs Changes clipboard API to accept typed sources (path + folder flag) instead of probing existence.
src/Files.Operations/Program.cs Removes automatic debug_server.log write; uses Debug.WriteLine instead.
src/Files.Operations/Helpers.cs Adds missing System using (consistency/compilation hygiene).
src/Files.Operations/AppInstanceMonitor.cs Adds missing System/Threading usings.
src/Files.Controls/Sidebar/SidebarItem.cs Removes directory probing before drag storage item setup.
Directory.Packages.props Adds Microsoft.CodeAnalysis.BannedApiAnalyzers package version.
Directory.Build.props Enables storage-boundary enforcement for selected projects and adds analyzer configuration.
BannedSymbols.txt Defines banned filesystem APIs and guidance messages.
.github/workflows/ci.yml Adds a build step for Files.Operations to ensure enforcement applies in CI.
Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +324 to +326
public bool HasFolders => _hasFolders;

public bool IsSingleFile => _fileSystemPaths.Count is 1 && File.Exists(_fileSystemPaths[0]);
public bool IsSingleFile => _isSingleFile;
@0x5bfa 0x5bfa changed the title Code Quality: Enforce storage-boundary file system access Feature: Add Shell-compatible file transfer Sep 4, 2026
@0x5bfa
0x5bfa force-pushed the code-quality/ban-direct-file-system-access branch from e6c131e to 989fcfd Compare September 4, 2026 14:17
@0x5bfa
0x5bfa force-pushed the code-quality/ban-direct-file-system-access branch from 989fcfd to d910434 Compare September 4, 2026 14:21
@0x5bfa 0x5bfa closed this Sep 4, 2026
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