fix(core): resolve path traversal, process launch safety, null-safety, OOM guard, and IPC key validation (#516)

- C1: GetTempDirectory() use StartsWith+Substring instead of string.Replace
  for path computation to prevent incorrect path resolution when targetPath
  is a substring of another path component (potential path traversal).
- H1: DiffPipelineOptions.MaxInputFileSize guard — configurable file size
  limit checked before invoking the binary differ, preventing OOM on
  oversized files. Default 0 = no limit (backward compatible).
- H2: Validate Process.Start return value across all strategy files
  (Windows/Linux/Mac/Client/Abstract). Log PID on success, throw on null.
- H3: UnixPermissionHooks uses ArgumentList { "+x", mainApp } on .NET 6+
  to avoid shell injection; netstandard2.0 fallback retains quoting.
- H4: SafeOnBeforeUpdateAsync returns false on hook exception so a faulty
  hook does not silently allow the update to proceed.
- L4: Environments.Set/GetEnvironmentVariable validates key contains only
  alphanumeric, underscore, hyphen, and dot chars to prevent path traversal.
- M2: HttpClientProvider sets 5-minute hard upper bound timeout as safety
  net instead of InfiniteTimeSpan. Improve comment clarity.
- M3: StorageManager.GetAllFiles and CopyDirectory use dirName.StartsWith
  instead of dirName.Contains for directory skipping.
- M4: GetTempDirectory and GetBackupDirectoryName use DateTime.UtcNow.
- M6: SilentPollOrchestrator dispatches ExceptionEventArgs via EventManager.
- M7: DefaultDirtyMatcher.Match adds ArgumentNullException guard.
- M8: GracefulExit.ShutdownAsync adds XML doc explaining CloseMainWindow()
  behavior for console / headless processes.
- L1: CopyUnknownFiles skips files outside patch directory instead of
  using the full absolute path. Fix comment wording.
- L2: AbstractStrategy: patchRoot in finally; sanitize version.Name as
  subdirectory key (null-safe, strip path separators, guard . / ..).
- L5: Remove redundant File.Delete in EncryptedFileProcessContractProvider.
- L7: CallSmallBowlHomeAsync logs a warning when Bowl name is empty.

Co-authored-by: Claude <noreply@anthropic.com>

Author: JusterZhu

src/c#/GeneralUpdate.Core/Bootstrap/GeneralUpdateBootstrap.cs

@@ -577,7 +577,11 @@ private void InitBlackPolicy()
 
     private async Task CallSmallBowlHomeAsync(string processName)
     {
-        if (string.IsNullOrWhiteSpace(processName)) return;
+        if (string.IsNullOrWhiteSpace(processName))
+        {
+            GeneralTracer.Warn("CallSmallBowlHomeAsync: Bowl process name is empty or whitespace, skipping shutdown.");
+            return;
+        }
         try
         {
             var processes = Process.GetProcessesByName(processName);

src/c#/GeneralUpdate.Core/Configuration/Environments.cs

@@ -1,4 +1,6 @@
+using System;
 using System.IO;
+using System.Linq;
 using System.Security.Cryptography;
 using System.Text;
 using GeneralUpdate.Core.Ipc;
@@ -17,6 +19,11 @@ public static class Environments
         .ComputeHash(Encoding.UTF8.GetBytes("GeneralUpdate.IPC.EnvironmentProvider.v1"));
     private static readonly byte[] _aesIV = new byte[16] { 0x47, 0x55, 0x50, 0x44, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
 
+    /// <summary>
+    /// Allowed characters for IPC key names: alphanumeric, underscore, hyphen, dot.
+    /// </summary>
+    private static readonly char[] KeyAllowedChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_-.".ToCharArray();
+
     private static string IpcDir
     {
         get
@@ -29,15 +36,27 @@ private static string IpcDir
 
     public static void SetEnvironmentVariable(string key, string value)
     {
+        ValidateKey(key);
         var filePath = Path.Combine(IpcDir, $"{key}.enc");
         var plainBytes = Encoding.UTF8.GetBytes(value);
         IpcEncryption.EncryptToFile(plainBytes, filePath, _aesKey, _aesIV);
     }
 
     public static string GetEnvironmentVariable(string key)
     {
+        ValidateKey(key);
         var filePath = Path.Combine(Path.GetTempPath(), "GeneralUpdate", "ipc", $"{key}.enc");
         var plainBytes = IpcEncryption.DecryptFromFile(filePath, _aesKey, _aesIV);
         return plainBytes != null ? Encoding.UTF8.GetString(plainBytes) : string.Empty;
     }
+
+    private static void ValidateKey(string key)
+    {
+        if (string.IsNullOrWhiteSpace(key))
+            throw new ArgumentException("IPC key must not be null or whitespace.", nameof(key));
+        if (key.Any(c => !KeyAllowedChars.Contains(c)))
+            throw new ArgumentException(
+                $"IPC key '{key}' contains invalid characters. Only alphanumeric, underscore, hyphen, and dot are allowed.",
+                nameof(key));
+    }
 }

src/c#/GeneralUpdate.Core/Differential/DefaultDirtyMatcher.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -33,6 +34,9 @@ public class DefaultDirtyMatcher : IDirtyMatcher
     /// <inheritdoc/>
     public FileInfo? Match(FileInfo oldFile, IEnumerable<FileInfo> patchFiles)
     {
+        if (oldFile is null) throw new ArgumentNullException(nameof(oldFile));
+        if (patchFiles is null) throw new ArgumentNullException(nameof(patchFiles));
+
         var findFile = patchFiles.FirstOrDefault(f =>
         {
             var name = Path.GetFileNameWithoutExtension(f.Name);

src/c#/GeneralUpdate.Core/FileSystem/StorageManager.cs

@@ -199,7 +199,7 @@ public static void CreateJson<T>(string targetPath, T obj, JsonTypeInfo<T>? type
         /// </remarks>
         public static string GetTempDirectory(string name)
         {
-            var path = $"generalupdate_{DateTime.Now:yyyy-MM-dd-HHmmss-fff}_{System.Diagnostics.Process.GetCurrentProcess().Id}_{name}";
+            var path = $"generalupdate_{DateTime.UtcNow:yyyy-MM-dd-HHmmss-fff}_{System.Diagnostics.Process.GetCurrentProcess().Id}_{name}";
             var tempDir = Path.Combine(Path.GetTempPath(), path);
             if (!Directory.Exists(tempDir))
             {
@@ -217,7 +217,7 @@ public static string GetTempDirectory(string name)
         /// Timestamp naming ensures each backup is unique and naturally sortable by creation time.
         /// Used by <see cref="Backup"/> to create version-independent backup directory names.
         /// </remarks>
-        public static string GetBackupDirectoryName() => $"{DirectoryName}{DateTime.Now:yyyyMMddHHmmss}";
+        public static string GetBackupDirectoryName() => $"{DirectoryName}{DateTime.UtcNow:yyyyMMddHHmmss}";
 
         /// <summary>
         /// Finds the most recent backup directory by scanning for backup directories
@@ -317,7 +317,11 @@ public static List<FileInfo> GetAllFiles(string path, List<string> skipDirectory
                     bool shouldSkip = false;
                     foreach (var notBackup in skipDirectorys)
                     {
-                        if (dic.FullName.Contains(notBackup))
+                        // Use prefix matching instead of substring Contains to avoid
+                        // false-positive skips (e.g. a directory named "myapp-backup-config"
+                        // should not be skipped just because "backup-" appears in its name).
+                        var dirName = dic.Name;
+                        if (dirName.StartsWith(notBackup, StringComparison.OrdinalIgnoreCase))
                         {
                             shouldSkip = true;
                             break;
@@ -425,7 +429,7 @@ private static void CopyDirectory(string sourceDir, string targetDir, IReadOnlyL
             foreach (string dirPath in Directory.GetDirectories(sourceDir, "*", SearchOption.TopDirectoryOnly))
             {
                 var dirName = Path.GetFileName(dirPath);
-                if (!directoryNames.Any(name => dirName.Contains(name)))
+                if (!directoryNames.Any(name => dirName.StartsWith(name, StringComparison.OrdinalIgnoreCase)))
                 {
                     string newTargetDir = Path.Combine(targetDir, Path.GetFileName(dirPath));
                     Directory.CreateDirectory(newTargetDir);

src/c#/GeneralUpdate.Core/GracefulExit.cs

@@ -9,11 +9,32 @@ public static class GracefulExit
     /// <summary>
     /// Attempt a graceful shutdown via CloseMainWindow(), fall back to Kill() after timeout.
     /// </summary>
+    /// <remarks>
+    /// <para>
+    /// <see cref="Process.CloseMainWindow"/> only works for GUI processes with a main window.
+    /// For console applications or headless processes, it returns <c>false</c> immediately.
+    /// In that case the method waits <paramref name="timeoutMs"/> and then falls back to
+    /// <see cref="Process.Kill"/> as a last resort.
+    /// </para>
+    /// <para>
+    /// When <c>CloseMainWindow</c> succeeds (returns <c>true</c>), the close message was
+    /// sent to the main window's message queue. The process is given <paramref name="timeoutMs"/>
+    /// to exit before falling back to <c>Kill</c>.
+    /// </para>
+    /// </remarks>
     public static async Task ShutdownAsync(Process? process, int timeoutMs = 3000)
     {
         if (process == null || process.HasExited) return;
-        if (!process.CloseMainWindow())
+        if (process.CloseMainWindow())
+        {
+            // CloseMainWindow sent a WM_CLOSE — give the process time to shut down.
             await Task.Delay(timeoutMs).ConfigureAwait(false);
+        }
+        else
+        {
+            // Process has no main window (e.g. console app) — still give it time.
+            await Task.Delay(timeoutMs).ConfigureAwait(false);
+        }
         if (!process.HasExited)
             process.Kill(); // Last resort
     }

src/c#/GeneralUpdate.Core/Hooks/IUpdateHooks.cs

@@ -48,8 +48,37 @@ public class UnixPermissionHooks : IUpdateHooks
     public async Task OnBeforeStartAppAsync(HookContext ctx)
     {
         var mainApp = Path.Combine(ctx.InstallPath, ctx.UpdateAppName);
-        if (File.Exists(mainApp))
-            await Task.Run(() => Process.Start("chmod", $"+x \"{mainApp}\"").WaitForExit());
+        if (!File.Exists(mainApp)) return;
+
+        var exitCode = -1;
+#if NET6_0_OR_GREATER
+        // Use ArgumentList to avoid shell injection via user-controlled path.
+        // NOTE: when ArgumentList is non-empty, ProcessStartInfo.Arguments is
+        // ignored, so the mode "+x" must be added to ArgumentList as well.
+        var psi = new ProcessStartInfo("chmod")
+        {
+            ArgumentList = { "+x", mainApp },
+            RedirectStandardOutput = true,
+            RedirectStandardError = true
+        };
+        using var proc = Process.Start(psi);
+        if (proc == null)
+        {
+            GeneralTracer.Warn($"UnixPermissionHooks: chmod process could not be started for {mainApp}.");
+            return;
+        }
+        await Task.Run(() => proc.WaitForExit());
+        exitCode = proc.ExitCode;
+#else
+        // netstandard2.0 fallback: path is double-quoted to mitigate injection risk.
+        // ArgumentList is unavailable, so we rely on the quoting and the fact that
+        // chmod +x with an extra argument is not exploitable beyond a file-not-found error.
+        using var proc = Process.Start("chmod", $"+x \"{mainApp}\"");
+        await Task.Run(() => proc.WaitForExit());
+        exitCode = proc.ExitCode;
+#endif
+        if (exitCode != 0)
+            GeneralTracer.Warn($"UnixPermissionHooks: chmod for {mainApp} exited with code {exitCode}.");
     }
     public Task<bool> OnBeforeUpdateAsync(HookContext ctx) => Task.FromResult(true);
     public Task OnDownloadCompletedAsync(DownloadContext ctx) => Task.CompletedTask;

src/c#/GeneralUpdate.Core/Ipc/IProcessInfoProvider.cs

@@ -79,8 +79,8 @@ public void Send(ProcessContract info)
         var plain = IpcEncryption.DecryptFromFile(_filePath, Key, IV);
         if (plain == null) return null;
 
-        try { File.Delete(_filePath); }
-        catch { /* best-effort cleanup */ }
+        // Note: IpcEncryption.DecryptFromFile deletes the file in its finally block,
+        // so no additional cleanup is needed here.
 
         var json = Encoding.UTF8.GetString(plain);
         return JsonSerializer.Deserialize(json, ProcessContractJsonContext.Default.ProcessContract);

src/c#/GeneralUpdate.Core/Network/HttpClientProvider.cs

@@ -34,11 +34,13 @@ static HttpClientProvider()
             _sslPolicy.ValidateCertificate(cert, chain, errors);
         _shared = new HttpClient(handler, disposeHandler: false)
         {
-            // Let per-request CancellationTokenSource.CancelAfter (set by
-            // HttpDownloadExecutor & VersionService) control timeout — the
-            // global HttpClient.Timeout (default ~100s) would otherwise cap
-            // requests before a caller-configured longer DownloadTimeout.
-            Timeout = System.Threading.Timeout.InfiniteTimeSpan
+            // Default 5-minute hard upper bound as a safety net. Per-request
+            // CancellationTokenSource.CancelAfter (set by HttpDownloadExecutor
+            // and VersionService) provides the primary timeout — this value
+            // only catches leaked requests where no per-request timeout was set.
+            // Callers requiring transfers longer than 5 minutes must increase
+            // this timeout or pass a larger value via e.g. UpdateRequest.
+            Timeout = TimeSpan.FromMinutes(5)
         };
     }
 

src/c#/GeneralUpdate.Core/Pipeline/DiffPipeline.cs

@@ -239,6 +239,7 @@ public async Task CleanAsync(
                 {
                     if (!StorageManager.HashEquals(oldFile.FullName, file.FullName))
                     {
+                        ValidateFileSize(oldFile.FullName, file.FullName);
                         var tempPatchPath = Path.Combine(tempDir, $"{file.Name}{PatchExtension}");
                         await _binaryDiffer.CleanAsync(oldFile.FullName, file.FullName, tempPatchPath, cancellationToken);
                     }
@@ -522,14 +523,20 @@ private static Task CopyUnknownFiles(string appPath, string patchPath)
 
                 // Compute the relative path by stripping the patch directory prefix.
                 // Using StartsWith + Substring instead of string.Replace to avoid
-                // incorrect replacements when appPath is a substring of patchPath.
+                // incorrect replacements when the prefix appears as a substring of
+                // another path component (e.g. "C:\target" vs "C:\target-extra").
                 var patchPrefix = patchPath.TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar;
                 var comparison = RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
                     ? StringComparison.OrdinalIgnoreCase
                     : StringComparison.Ordinal;
                 var relativePart = file.FullName.StartsWith(patchPrefix, comparison)
                     ? file.FullName.Substring(patchPrefix.Length)
-                    : file.FullName;
+                    : null;
+                if (relativePart == null)
+                {
+                    GeneralTracer.Warn($"CopyUnknownFiles: file {file.FullName} is outside patch directory {patchPath}, skipping.");
+                    continue;
+                }
                 var targetPath = Path.Combine(appPath, relativePart);
                 var parentFolder = Directory.GetParent(targetPath);
                 if (parentFolder?.Exists == false)
@@ -572,10 +579,18 @@ private static Task CopyUnknownFiles(string appPath, string patchPath)
     /// </remarks>
     private static string GetTempDirectory(FileNode file, string targetPath, string patchPath)
     {
-        var tempPath = file.FullName
-            .Replace(targetPath, "")
-            .Replace(Path.GetFileName(file.FullName), "")
-            .Trim(Path.DirectorySeparatorChar);
+        // Use StartsWith + Substring instead of string.Replace to avoid incorrect
+        // replacements when targetPath is a substring of another path component.
+        // Same approach as CopyUnknownFiles which documents this rationale.
+        var prefix = targetPath.TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar;
+        var comparison = RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
+            ? StringComparison.OrdinalIgnoreCase
+            : StringComparison.Ordinal;
+        var tempPath = file.FullName.StartsWith(prefix, comparison)
+            ? file.FullName.Substring(prefix.Length)
+            : string.Empty;
+        if (!string.IsNullOrEmpty(tempPath))
+            tempPath = tempPath.Replace(Path.GetFileName(file.FullName), "").Trim(Path.DirectorySeparatorChar);
         var tempDir = string.IsNullOrEmpty(tempPath) ? patchPath : Path.Combine(patchPath, tempPath);
         Directory.CreateDirectory(tempDir);
         return tempDir;
@@ -604,4 +619,26 @@ private static void ValidateDirectories(string sourcePath, string targetPath, st
         if (!Directory.Exists(targetPath))
             throw new DirectoryNotFoundException($"Target directory not found: {targetPath}");
     }
+
+    /// <summary>
+    /// Validates that both source and target files do not exceed the configured <see cref="DiffPipelineOptions.MaxInputFileSize"/>.
+    /// The BSDIFF algorithm loads entire files into memory, so this guard prevents <see cref="OutOfMemoryException"/>
+    /// when processing oversized files.
+    /// </summary>
+    private void ValidateFileSize(string oldFilePath, string newFilePath)
+    {
+        if (_options.MaxInputFileSize <= 0) return;
+
+        var oldInfo = new FileInfo(oldFilePath);
+        if (oldInfo.Length > _options.MaxInputFileSize)
+            throw new InvalidOperationException(
+                $"File too large for binary differ: {oldFilePath} ({oldInfo.Length} bytes > {_options.MaxInputFileSize} bytes limit). " +
+                $"Increase {nameof(DiffPipelineOptions.MaxInputFileSize)} or switch to a streaming differ.");
+
+        var newInfo = new FileInfo(newFilePath);
+        if (newInfo.Length > _options.MaxInputFileSize)
+            throw new InvalidOperationException(
+                $"File too large for binary differ: {newFilePath} ({newInfo.Length} bytes > {_options.MaxInputFileSize} bytes limit). " +
+                $"Increase {nameof(DiffPipelineOptions.MaxInputFileSize)} or switch to a streaming differ.");
+    }
 }

src/c#/GeneralUpdate.Core/Pipeline/DiffPipelineOptions.cs

@@ -14,6 +14,7 @@ namespace GeneralUpdate.Core.Pipeline;
 /// <list type="bullet">
 ///   <item><description><see cref="MaxDegreeOfParallelism"/> = 2 — Processes 2 files concurrently.</description></item>
 ///   <item><description><see cref="StopOnFirstError"/> = <c>false</c> — Continues processing other files on error.</description></item>
+///   <item><description><see cref="MaxInputFileSize"/> = 0 — No limit (backward compatible).</description></item>
 /// </list>
 /// </para>
 /// </remarks>
@@ -63,4 +64,27 @@ public sealed class DiffPipelineOptions
     /// </para>
     /// </remarks>
     public bool StopOnFirstError { get; set; } = false;
+
+    /// <summary>
+    /// Gets or sets the maximum allowed file size (in bytes) for files processed by the binary differ.
+    /// Files larger than this threshold cause an error rather than being loaded entirely into memory.
+    /// </summary>
+    /// <value>
+    /// The maximum file size in bytes. The default value is 0, meaning no limit (backward compatible).
+    /// Set to a positive value to reject oversized files before the differ allocates memory.
+    /// Example: 512MB = 512 * 1024 * 1024.
+    /// </value>
+    /// <remarks>
+    /// <para>
+    /// The BSDIFF algorithm reads entire files into memory (old + new + up to 3x new file size in
+    /// working buffers). For large files this can cause <see cref="OutOfMemoryException"/>.
+    /// When <see cref="MaxInputFileSize"/> is set, the pipeline checks both old and new file sizes
+    /// before invoking the differ and reports a clear error for oversized files.
+    /// </para>
+    /// <para>
+    /// For incremental updates of very large files, consider splitting the file into smaller chunks
+    /// or using an alternative differ with streaming support.
+    /// </para>
+    /// </remarks>
+    public long MaxInputFileSize { get; set; } = 0;
 }