Feature/ipc hardened draft final

[primeinc]

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

• Closes #

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

1. Opened Files ...
2. ...

[primeinc]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive and well-designed Inter-Process Communication (IPC) system. The architecture is robust, featuring support for both WebSocket and Named Pipe transports, strong security measures like DPAPI-encrypted token storage and per-user ACLs, and resilience features such as rate limiting and lossy message queues. The code is well-structured with clear separation of concerns between transport services, the central coordinator, and shell adapters. The addition of a unified Python test suite is also a great step towards ensuring correctness and parity between transports. My review has identified a couple of minor areas for improvement, mainly related to code cleanup and best practices in the test script. Overall, this is an excellent and significant contribution.

[gemini-code-assist]

Code Review

This is an impressive and substantial pull request that introduces a well-designed and robust IPC subsystem. The implementation demonstrates a strong focus on security, resource management, and correctness, with support for both WebSocket and Named Pipe transports. The addition of a comprehensive Python test suite and detailed documentation is excellent. My review includes a few minor suggestions to further improve code clarity and maintainability.

[primeinc]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive IPC (Inter-Process Communication) system to the Files app, focusing on hardening security, improving resource management, and ensuring correctness. The changes include adding new files and modifying existing ones to implement features like JSON-RPC 2.0 validation, encrypted token storage, rate limiting, and UI operation serialization. The review focuses on identifying potential security vulnerabilities, areas for optimization, and maintainability concerns within the introduced code, particularly in the new communication and IPC-related files.

[primeinc]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive and well-designed Inter-Process Communication (IPC) subsystem. The architecture is robust, featuring strong security measures like token-based authentication, per-user ACLs, and input sanitization. The implementation is clean, with good separation of concerns and helpful documentation in the code. The addition of a unified test suite is also a great step towards ensuring the feature's stability. I've identified a few areas for improvement, including a potential SDK version downgrade and some minor refinements in the test script and exception handling.

[primeinc]

/gemini review

[gemini-code-assist]

Code Review

This is an extensive and well-executed pull request that introduces a robust, secure, and well-documented IPC subsystem. The architecture is thoughtfully designed with clear separation of concerns, and security considerations are evident throughout the implementation, from DPAPI-encrypted token storage to detailed exception sanitization. The inclusion of a comprehensive Python test suite is commendable and adds a great deal of confidence in the solution's correctness. The changes to existing code also improve robustness and maintainability. I have a couple of minor suggestions regarding potential data loss from type casting, but overall this is an excellent contribution.

[primeinc]

/gemini review

[gemini-code-assist]

Code Review

This is an impressive pull request that introduces a comprehensive and well-designed IPC subsystem. The architecture is robust, with strong considerations for security, performance, and correctness. The code is of high quality, with good separation of concerns and detailed comments explaining design decisions. The addition of a unified test script is also a great practice. My main feedback is a suggestion to refactor some duplicated code in the IpcActionAdapter to improve maintainability.

[primeinc]

/gemini review

[gemini-code-assist]

Code Review

This is a substantial and well-executed pull request that introduces a robust, secure, and feature-rich IPC subsystem. The architecture is thoughtfully designed with clear separation of concerns, and security considerations are evident throughout the implementation. The addition of a comprehensive test suite is also commendable. My review includes a high-severity fix for a potential test hang in the Python test script and a medium-severity suggestion for improving consistency in the C# code. Overall, excellent work.

[gemini-code-assist]

The timeout implementation in this loop is not effective. The win32file.ReadFile call is blocking, which means if the server doesn't send any data, the test will hang indefinitely instead of timing out after 5 seconds. The while time.time() - start_time < timeout condition is only checked after a message is received.

To ensure the timeout is respected, you could use win32pipe.PeekNamedPipe to check for available data before calling ReadFile, and add a short sleep to prevent busy-waiting.

            # Check if data is available before blocking to make the timeout effective
            bytes_avail, _, _ = win32pipe.PeekNamedPipe(self.pipe_handle, 0)
            if bytes_avail < 4: # Not enough data for length prefix
                time.sleep(0.01)  # Avoid busy-waiting
                continue

            # Read length
            _, length_data = win32file.ReadFile(self.pipe_handle, 4)
            if not length_data:
                continue # Pipe might have been closed
            length = struct.unpack('<I', length_data)[0]
            
            # Read payload
            _, payload_data = win32file.ReadFile(self.pipe_handle, length)
            response = json.loads(payload_data.decode('utf-8'))
            
            # Skip notifications
            if response.get("IsNotification") or response.get("id") is None:
                continue
            
            if response.get("id") == msg_id:
                if "error" in response and response["error"]:
                    raise RuntimeError(f"RPC error: {response['error']}")
                return response.get("result")

[gemini-code-assist]

For consistency with IpcCoordinator.cs and for improved performance, consider converting AppLifecycleHelper to a partial class and using source-generated regexes for the sanitization logic in SanitizeExceptionMessage and SanitizeStackTrace. This avoids runtime regex compilation and keeps the implementation approach consistent across the codebase.

[Copilot]

Pull Request Overview

This PR implements a comprehensive hardened IPC (Inter-Process Communication) system for Files that enables remote control functionality via both WebSocket and Named Pipe transports. The system provides secure, rate-limited access to navigation, file operations, and UI actions through a JSON-RPC 2.0 API.

Key changes include:

• Complete IPC framework with dual transport support (WebSocket + Named Pipes)
• Security hardening with DPAPI token encryption, rate limiting, and strict validation
• Settings UI for remote control management with token rotation capabilities

Reviewed Changes

Copilot reviewed 50 out of 52 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/Files.App/Communication/*	Core IPC framework including transport services, client management, and security
src/Files.App/Views/Settings/IpcPage.*	Settings UI for enabling/disabling remote control and managing tokens
src/Files.App/ViewModels/Settings/IpcViewModel.cs	ViewModel for IPC settings page with token management
src/Files.App/ViewModels/ShellIpcAdapter.cs	Adapter bridging IPC requests to shell operations with safety controls
src/Files.App/Views/Shells/ModernShellPage.xaml.cs	Integration of IPC bootstrapper with shell lifecycle
tests/Files.InteractionTests/Ipc/EphemeralHelpersTests.cs	Unit tests for IPC helper functions
scripts/test_ipc_unified.py	Comprehensive test suite for both transport protocols

[Copilot]

The Win32 API declarations and RECT struct are duplicated functionality that likely exists elsewhere in the codebase. Consider extracting these to a shared Win32Interop class or reusing existing declarations to avoid code duplication.

[Copilot]

The local function SetFocusWithEventAsync is complex (35+ lines) and used twice. Consider extracting it to a private method or helper class to improve readability and testability.

[Copilot]

The method recreates the entire queue using O(n) operations. While the comment acknowledges this trade-off, consider using a more efficient approach like tracking method counts separately and using a timestamp-based cleanup strategy to avoid frequent queue rebuilding.

[Copilot]

Multiple Regex.Replace calls in sanitization methods should use compiled regex patterns or source-generated regex (available in .NET 7+) for better performance, especially since these methods may be called frequently during error handling.

[Copilot]

The catch block should specify the expected exception type (COMException) rather than catching all exceptions. This prevents masking other unexpected exceptions that might indicate real bugs.

[Copilot]

The Win32 error code constants should be extracted to a shared constants class rather than being redefined here. This improves maintainability and prevents magic numbers from being scattered throughout the codebase.