fix: apply second review feedback

Agent-Logs-Url: https://github.com/OpenLoco/ObjectEditor/sessions/4e0b3e6d-0b3f-4ddf-a12e-36820d17d4dd

Co-authored-by: LeftofZen <7483209+LeftofZen@users.noreply.github.com>

Author: Copilot

Gui/Models/FileSystemItems.cs

@@ -38,6 +38,10 @@ public bool CanDownload
 	public bool CanOpenFolder
 		=> FileLocation == FileLocationKind.Local && IsLeafNode && !string.IsNullOrEmpty(FileName);
 
+	[JsonIgnore]
+	public bool HasContextActions
+		=> CanOpenFolder || CanDownload;
+
 	public uint? DatChecksum { get; init; }
 
 	public ulong? xxHash3 { get; init; }

Gui/ViewModels/FolderTreeViewModel.cs

@@ -779,6 +779,45 @@ static FileSystemItem CreateOnlineScenarioFileSystemItem(DtoScenarioEntry item)
 			OnlineApiEndpointGroup = OnlineApiEndpointGroup.Scenarios,
 		};
 
+	static string SanitizeDownloadName(string? name, bool stripDirectoryComponents, bool stripExtension, string fallbackName)
+	{
+		var sanitizedName = name ?? string.Empty;
+		if (stripDirectoryComponents)
+		{
+			sanitizedName = sanitizedName.Split(['/', '\\'], StringSplitOptions.RemoveEmptyEntries).LastOrDefault() ?? string.Empty;
+		}
+
+		if (stripExtension)
+		{
+			sanitizedName = Path.GetFileNameWithoutExtension(sanitizedName);
+		}
+
+		sanitizedName = Path.GetInvalidFileNameChars().Aggregate(sanitizedName, (current, c) => current.Replace(c, '_'));
+		sanitizedName = sanitizedName
+			.Replace('/', '_')
+			.Replace('\\', '_')
+			.Replace("..", "__", StringComparison.Ordinal);
+
+		return string.IsNullOrWhiteSpace(sanitizedName) ? fallbackName : sanitizedName;
+	}
+
+	async Task ShowDownloadFailureDialogAsync(string contentMessage)
+	{
+		var box = MessageBoxManager.GetMessageBoxStandard(new MessageBoxStandardParams
+		{
+			ContentTitle = "Download failed",
+			ContentMessage = contentMessage,
+			ButtonDefinitions = ButtonEnum.Ok,
+			Icon = Icon.Warning,
+			WindowStartupLocation = WindowStartupLocation.CenterOwner,
+			Topmost = true,
+			ShowInCenter = true,
+			SizeToContent = SizeToContent.WidthAndHeight,
+			MinHeight = 170,
+		});
+		_ = await box.ShowAsync();
+	}
+
 	async Task DownloadOnlineItemAsync(FileSystemItem item)
 	{
 		if (EditorContext.ObjectServiceClient == null || item.Id == null)
@@ -800,10 +839,24 @@ async Task DownloadOnlineItemAsync(FileSystemItem item)
 		}
 
 		var extension = item.OnlineApiEndpointGroup == OnlineApiEndpointGroup.Scenarios ? ".SC5" : ".dat";
-		var safeName = Path.GetInvalidFileNameChars().Aggregate(item.DisplayName, (current, c) => current.Replace(c, '_'));
+		var safeName = SanitizeDownloadName(item.DisplayName, stripDirectoryComponents: true, stripExtension: true, fallbackName: "download");
 		var filename = Path.Combine(EditorContext.Settings.DownloadFolder, $"{safeName}-{item.Id}{extension}");
-		await File.WriteAllBytesAsync(filename, fileBytes);
-		EditorContext.Logger.Info($"Downloaded \"{item.DisplayName}\" to \"{filename}\"");
+		try
+		{
+			await using var outputStream = new FileStream(filename, FileMode.CreateNew, FileAccess.Write, FileShare.None, 4096, useAsync: true);
+			await outputStream.WriteAsync(fileBytes);
+			EditorContext.Logger.Info($"Downloaded \"{item.DisplayName}\" to \"{filename}\"");
+		}
+		catch (IOException ex)
+		{
+			EditorContext.Logger.Error($"Failed to download \"{item.DisplayName}\" to \"{filename}\"", ex);
+			await ShowDownloadFailureDialogAsync($"Could not create:\n{filename}\n\nThe destination file may already exist, be locked by another process, or the download folder may be unavailable.");
+		}
+		catch (UnauthorizedAccessException ex)
+		{
+			EditorContext.Logger.Error($"Failed to download \"{item.DisplayName}\" to \"{filename}\" due to insufficient permissions", ex);
+			await ShowDownloadFailureDialogAsync($"You do not have permission to write to:\n{filename}\n\nPlease check folder permissions or choose a different download folder.");
+		}
 	}
 
 	async Task DownloadOnlinePackAsync(OnlineItemPackBrowseResult pack)
@@ -826,7 +879,7 @@ async Task DownloadOnlinePackAsync(OnlineItemPackBrowseResult pack)
 			return;
 		}
 
-		var safePackName = Path.GetInvalidFileNameChars().Aggregate(pack.Name, (current, c) => current.Replace(c, '_'));
+		var safePackName = SanitizeDownloadName(pack.Name, stripDirectoryComponents: false, stripExtension: false, fallbackName: "pack");
 		var filename = Path.Combine(EditorContext.Settings.DownloadFolder, $"{safePackName}-{pack.Id}.zip");
 		try
 		{
@@ -837,19 +890,7 @@ async Task DownloadOnlinePackAsync(OnlineItemPackBrowseResult pack)
 		catch (IOException ex)
 		{
 			EditorContext.Logger.Error($"Failed to download pack \"{pack.Name}\" to \"{filename}\"", ex);
-			var box = MessageBoxManager.GetMessageBoxStandard(new MessageBoxStandardParams
-			{
-				ContentTitle = "Download failed",
-				ContentMessage = $"Could not create:\n{filename}\n\nA file may already exist or be locked by another process.",
-				ButtonDefinitions = ButtonEnum.Ok,
-				Icon = Icon.Warning,
-				WindowStartupLocation = WindowStartupLocation.CenterOwner,
-				Topmost = true,
-				ShowInCenter = true,
-				SizeToContent = SizeToContent.WidthAndHeight,
-				MinHeight = 170,
-			});
-			_ = await box.ShowAsync();
+			await ShowDownloadFailureDialogAsync($"Could not create:\n{filename}\n\nThe destination file may already exist, be locked by another process, or the download folder may be unavailable.");
 		}
 	}
 }

Gui/Views/FolderTreeView.axaml

@@ -158,7 +158,7 @@
 									<materialIcons:MaterialIcon Kind="{Binding DisplayName, Converter={StaticResource EnumToMaterialIconConverter}}" Width="24" Height="24" Margin="2"/>
 									<TextBlock VerticalAlignment="Center" Text="{Binding NameComputed}"/>
 									<StackPanel.ContextMenu>
-										<ContextMenu>
+										<ContextMenu IsVisible="{Binding HasContextActions}" IsEnabled="{Binding HasContextActions}">
 											<MenuItem Header="Open folder" IsVisible="{Binding CanOpenFolder}" Command="{Binding $parent[TreeDataGrid].((vm:FolderTreeViewModel)DataContext).OpenFolderFor}" CommandParameter="{Binding}" />
 											<MenuItem Header="Download" IsVisible="{Binding CanDownload}" Command="{Binding $parent[TreeDataGrid].((vm:FolderTreeViewModel)DataContext).DownloadOnlineItemCommand}" CommandParameter="{Binding}" />
 										</ContextMenu>
@@ -171,7 +171,7 @@
 									<materialIcons:MaterialIcon Kind="{Binding DisplayName, Converter={StaticResource EnumToMaterialIconConverter}}" Width="24" Height="24" Margin="2" />
 									<TextBlock VerticalAlignment="Center" Text="{Binding NameComputed}"/>
 									<StackPanel.ContextMenu>
-										<ContextMenu>
+										<ContextMenu IsVisible="{Binding HasContextActions}" IsEnabled="{Binding HasContextActions}">
 											<MenuItem Header="Open folder" IsVisible="{Binding CanOpenFolder}" Command="{Binding $parent[TreeDataGrid].((vm:FolderTreeViewModel)DataContext).OpenFolderFor}" CommandParameter="{Binding}" />
 											<MenuItem Header="Download" IsVisible="{Binding CanDownload}" Command="{Binding $parent[TreeDataGrid].((vm:FolderTreeViewModel)DataContext).DownloadOnlineItemCommand}" CommandParameter="{Binding}" />
 										</ContextMenu>

ObjectService/RouteHandlers/RouteHelpers.cs

@@ -2,7 +2,7 @@ namespace ObjectService.RouteHandlers;
 
 public static class RouteHelpers
 {
-	static readonly char[] PathSeparators = [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar];
+	static readonly char[] PathSeparators = ['/', '\\'];
 
 	public static string MakeNicePlural(string name)
 		=> $"{name.Replace("RouteHandler", string.Empty)}s";
@@ -37,7 +37,7 @@ public static bool TryGetSafeRelativePathUnderRoot(string rootPath, string? rela
 		}
 
 		fullPath = candidateFullPath;
-		normalizedRelativePath = relativePath.Replace('\\', '/');
+		normalizedRelativePath = string.Join('/', segments);
 		return true;
 	}
 }

Tests/ObjectServiceIntegrationTests/DownloadRoutesTest.cs

@@ -75,6 +75,7 @@ public async Task GetSC5FilePackFileAsync_ReturnsZipWithOnlySafeScenarioEntries(
 
 		var outsidePath = Path.Combine(rootFolder, "outside.SC5");
 		await File.WriteAllBytesAsync(outsidePath, [7, 7, 7]);
+		await File.WriteAllBytesAsync(Path.Combine(sfm.ScenariosFolder, @"..\outside-windows.SC5"), [6, 6, 6]);
 
 		var pack = new TblSC5FilePack
 		{
@@ -84,6 +85,7 @@ public async Task GetSC5FilePackFileAsync_ReturnsZipWithOnlySafeScenarioEntries(
 			[
 				new TblSC5File { Id = 1, Name = safeRelativePath },
 				new TblSC5File { Id = 2, Name = Path.Combine("..", "outside.SC5") },
+				new TblSC5File { Id = 3, Name = @"..\outside-windows.SC5" },
 			],
 		};
 
@@ -123,9 +125,11 @@ public async Task GetObjectPackFileAsync_ReturnsZipWithOnlySafeIndexedObjectEntr
 
 		var outsidePath = Path.Combine(rootFolder, "outside.dat");
 		await File.WriteAllBytesAsync(outsidePath, [8, 8, 8]);
+		await File.WriteAllBytesAsync(Path.Combine(sfm.ObjectsFolder, @"..\outside-windows.dat"), [5, 5, 5]);
 
 		sfm.ObjectIndex.Objects.Add(new ObjectIndexEntry("SAFEOBJ", safeRelativePath, null, 111, null, ObjectType.Vehicle, ObjectSource.Custom, null, null));
 		sfm.ObjectIndex.Objects.Add(new ObjectIndexEntry("UNSAFEOBJ", Path.Combine("..", "outside.dat"), null, 222, null, ObjectType.Vehicle, ObjectSource.Custom, null, null));
+		sfm.ObjectIndex.Objects.Add(new ObjectIndexEntry("WINDOWSOBJ", @"..\outside-windows.dat", null, 333, null, ObjectType.Vehicle, ObjectSource.Custom, null, null));
 
 		var obj = new TblObject
 		{
@@ -139,6 +143,7 @@ public async Task GetObjectPackFileAsync_ReturnsZipWithOnlySafeIndexedObjectEntr
 			[
 				new TblDatObject { Id = 1, DatName = "SAFEOBJ", DatChecksum = 111, xxHash3 = 1, ObjectId = 1 },
 				new TblDatObject { Id = 2, DatName = "UNSAFEOBJ", DatChecksum = 222, xxHash3 = 2, ObjectId = 1 },
+				new TblDatObject { Id = 3, DatName = "WINDOWSOBJ", DatChecksum = 333, xxHash3 = 3, ObjectId = 1 },
 			],
 		};