Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/GregModmanager.Core/Models/AppJsonContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ namespace GregModmanager.Models;
[JsonSerializable(typeof(int))]
[JsonSerializable(typeof(RalphTaskStatus))]
[JsonSerializable(typeof(AssetModMetadata))]
[JsonSerializable(typeof(TokenExchangeRequest))]
[JsonSerializable(typeof(TokenExchangeResponse))]
[JsonSerializable(typeof(object))]
public partial class AppJsonContext : JsonSerializerContext
{
/// <summary>
Expand Down
4 changes: 1 addition & 3 deletions src/GregModmanager.Core/Services/TelemetryService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ public async Task TrackEventAsync(string eventName, object payload, Dictionary<s
var message = payload switch
{
SyncCollectionEvent sync => JsonSerializer.Serialize(sync, AppJsonContext.Default.SyncCollectionEvent),
DebugLogPayload debug => JsonSerializer.Serialize(debug, AppJsonContext.Default.DebugLogPayload),
string s => s,
_ => "{}"
_ => JsonSerializer.Serialize((object)payload, AppJsonContext.Default.Object)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 HIGH RISK

Serializing using AppJsonContext.Default.Object will result in an empty JSON object {} for any runtime type because properties of derived classes are not included in the object metadata. This effectively breaks telemetry for any payload that isn't a base object. To fix this, you should explicitly handle known payload types in the switch expression or update AppJsonContext to include them via [JsonSerializable(typeof(YourType))].

};

await PushToLokiAsync(eventName, message, labels);
Expand Down
3 changes: 3 additions & 0 deletions tests/GregModmanager.Tests/GregModmanager.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
<ItemGroup>
<ProjectReference Include="..\..\src\GregModmanager.Core\GregModmanager.Core.csproj" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\src\GregModmanager.Avalonia\GregModmanager.Avalonia.csproj" />
</ItemGroup>
</Project>


150 changes: 150 additions & 0 deletions tests/GregModmanager.Tests/SubDirectoryFixerInstallerServiceTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
using GregModmanager.Avalonia.Services;
using System.Security.Cryptography;

namespace GregModmanager.Tests;

public class SubDirectoryFixerInstallerServiceTests : IDisposable
{
private readonly string _tempGameRoot;

public SubDirectoryFixerInstallerServiceTests()
{
_tempGameRoot = Path.Combine(Path.GetTempPath(), "GregModmanager_Tests_" + Guid.NewGuid().ToString());

// Note: The service uses Path.Combine(AppContext.BaseDirectory, "SubDirectoryFixer\\SubDirectoryFixer.dll").
// On Linux, Path.Combine does not translate '\' to '/', so it attempts to find a file literally named
// "SubDirectoryFixer\SubDirectoryFixer.dll" in the BaseDirectory.
// We create it here to ensure the tests pass on Linux when run in the CI/headless environment.
var payloadPath = Path.Combine(AppContext.BaseDirectory, "SubDirectoryFixer\\SubDirectoryFixer.dll");
var dirPath = Path.GetDirectoryName(payloadPath);
if (!string.IsNullOrEmpty(dirPath))
{
Directory.CreateDirectory(dirPath);
}
if (!File.Exists(payloadPath))
{
File.WriteAllText(payloadPath, "dummy-payload-content");
}
Comment on lines +18 to +27

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

Using a literal backslash in a path string passed to Path.Combine results in a filename containing a backslash on Linux rather than a directory structure. Update the production code and this test to use multiple arguments to Path.Combine to ensure the correct directory separator is used for the host OS.

Suggested change
var payloadPath = Path.Combine(AppContext.BaseDirectory, "SubDirectoryFixer\\SubDirectoryFixer.dll");
var dirPath = Path.GetDirectoryName(payloadPath);
if (!string.IsNullOrEmpty(dirPath))
{
Directory.CreateDirectory(dirPath);
}
if (!File.Exists(payloadPath))
{
File.WriteAllText(payloadPath, "dummy-payload-content");
}
var payloadPath = Path.Combine(AppContext.BaseDirectory, "SubDirectoryFixer", "SubDirectoryFixer.dll");

}

public void Dispose()
{
if (Directory.Exists(_tempGameRoot))
{
try
{
Directory.Delete(_tempGameRoot, true);
}
catch
{
// Ignore cleanup errors
}
}
}

[Theory]
[InlineData(null)]
[InlineData("")]
[InlineData(" ")]
public async Task EnsureInstalledAsync_WithInvalidGameRoot_ReturnsSkippedNoGameRoot(string? gameRoot)
{
var result = await SubDirectoryFixerInstallerService.EnsureInstalledAsync(gameRoot);

Assert.Equal(SubDirectoryFixerInstallStatus.SkippedNoGameRoot, result.Status);
}

[Fact]
public async Task EnsureInstalledAsync_WithNonExistentGameRoot_ReturnsSkippedNoGameRoot()
{
var result = await SubDirectoryFixerInstallerService.EnsureInstalledAsync(_tempGameRoot);

Assert.Equal(SubDirectoryFixerInstallStatus.SkippedNoGameRoot, result.Status);
}

[Fact]
public async Task EnsureInstalledAsync_WithValidGameRoot_InstallsSuccessfully()
{
// Arrange
Directory.CreateDirectory(_tempGameRoot);

// Act
var result = await SubDirectoryFixerInstallerService.EnsureInstalledAsync(_tempGameRoot);

// Assert
Assert.Equal(SubDirectoryFixerInstallStatus.Installed, result.Status);

var pluginsDir = Path.Combine(_tempGameRoot, "Plugins");
var targetFile = Path.Combine(pluginsDir, "SubDirectoryFixer.dll");
var markerFile = Path.Combine(pluginsDir, ".gregmodmanager-subdirfixer.sha256");

Assert.True(File.Exists(targetFile));
Assert.True(File.Exists(markerFile));
}

[Fact]
public async Task EnsureInstalledAsync_AlreadyInstalled_ReturnsAlreadyInstalled()
{
// Arrange
Directory.CreateDirectory(_tempGameRoot);

// First install
await SubDirectoryFixerInstallerService.EnsureInstalledAsync(_tempGameRoot);

// Act - Second install
var result = await SubDirectoryFixerInstallerService.EnsureInstalledAsync(_tempGameRoot);

// Assert
Assert.Equal(SubDirectoryFixerInstallStatus.AlreadyInstalled, result.Status);
}

[Fact]
public async Task EnsureInstalledAsync_MarkerHashMismatch_UpdatesSuccessfully()
{
// Arrange
Directory.CreateDirectory(_tempGameRoot);

// First install
await SubDirectoryFixerInstallerService.EnsureInstalledAsync(_tempGameRoot);

// Tamper with marker file
var pluginsDir = Path.Combine(_tempGameRoot, "Plugins");
var markerFile = Path.Combine(pluginsDir, ".gregmodmanager-subdirfixer.sha256");

// Make sure Plugins exists in case the first call failed
Directory.CreateDirectory(pluginsDir);
await File.WriteAllTextAsync(markerFile, "invalid-hash");

// Act - Update
var result = await SubDirectoryFixerInstallerService.EnsureInstalledAsync(_tempGameRoot);

// Assert
Assert.Equal(SubDirectoryFixerInstallStatus.Installed, result.Status);
var newMarkerHash = await File.ReadAllTextAsync(markerFile);
Assert.NotEqual("invalid-hash", newMarkerHash);
}

[Fact]
public async Task EnsureInstalledAsync_FileLocked_ReturnsFailedStatus()
{
// Arrange
Directory.CreateDirectory(_tempGameRoot);

// Create the directory structure where it will be installed
var pluginsDir = Path.Combine(_tempGameRoot, "Plugins");
Directory.CreateDirectory(pluginsDir);

// Create a dummy file and lock it
var targetFile = Path.Combine(pluginsDir, "SubDirectoryFixer.dll");
await File.WriteAllTextAsync(targetFile, "dummy");

// Lock the file for exclusive access
using var stream = new FileStream(targetFile, FileMode.Open, FileAccess.ReadWrite, FileShare.None);

// Act
var result = await SubDirectoryFixerInstallerService.EnsureInstalledAsync(_tempGameRoot);

// Assert
Assert.Equal(SubDirectoryFixerInstallStatus.Failed, result.Status);
Assert.Contains("install failed", result.Message, StringComparison.OrdinalIgnoreCase);
}
}
Loading