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
34 changes: 28 additions & 6 deletions src/SharpFM/Models/ClipRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -234,19 +234,41 @@ private static IReadOnlyList<string> GetRelativeFolderSegments(string root, stri
return decoded;
}

// Reject traversal segments (security) and percent-encode any character
// the filesystem rejects so FileMaker names containing '/' or ':' survive
// the round-trip instead of being silently dropped.
private static IReadOnlyList<string> SanitizeFolderPath(IReadOnlyList<string> segments)
/// <summary>
/// Raised when <see cref="SanitizeFolderPath"/> drops one or more segments
/// (empty, whitespace, or traversal). Hosts can subscribe to surface a
/// non-blocking notification so plugin-driven placements that landed
/// somewhere other than requested are visible to the user.
/// </summary>
public event EventHandler<FolderPathSanitizedEventArgs>? FolderPathSanitized;

// Empty / "." / ".." segments are dropped (not encoded) — the call is
// best-effort, not a hard error, but the dropped segments are reported via
// Log.Warn and FolderPathSanitized so the caller can surface them.
private IReadOnlyList<string> SanitizeFolderPath(IReadOnlyList<string> segments)
{
if (segments is null || segments.Count == 0) return [];
var safe = new List<string>(segments.Count);
var dropped = new List<string>();
foreach (var raw in segments)
{
if (string.IsNullOrWhiteSpace(raw)) continue;
if (raw == "." || raw == "..") continue;
if (string.IsNullOrWhiteSpace(raw) || raw == "." || raw == "..")
{
dropped.Add(raw ?? "");
continue;
}
safe.Add(EncodeName(raw));
}

if (dropped.Count > 0)
{
Log.Warn(
"Sanitized folder path: dropped [{Dropped}]; kept [{Kept}].",
string.Join(", ", dropped),
string.Join("/", safe));
FolderPathSanitized?.Invoke(this, new FolderPathSanitizedEventArgs(segments, safe, dropped));
}

return safe;
}

Expand Down
26 changes: 26 additions & 0 deletions src/SharpFM/Models/FolderPathSanitizedEventArgs.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using System;
using System.Collections.Generic;

namespace SharpFM.Models;

/// <summary>
/// Payload for <see cref="ClipRepository.FolderPathSanitized"/>. Reports the
/// original folder path, the path that was actually used, and the segments
/// that were dropped during sanitization.
/// </summary>
public sealed class FolderPathSanitizedEventArgs : EventArgs
{
public IReadOnlyList<string> Original { get; }
public IReadOnlyList<string> Sanitized { get; }
public IReadOnlyList<string> DroppedSegments { get; }

public FolderPathSanitizedEventArgs(
IReadOnlyList<string> original,
IReadOnlyList<string> sanitized,
IReadOnlyList<string> droppedSegments)
{
Original = original;
Sanitized = sanitized;
DroppedSegments = droppedSegments;
}
}
28 changes: 23 additions & 5 deletions src/SharpFM/ViewModels/MainWindowViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,32 @@ public async Task SaveClipsStorageAsync()
})
.ToList();

// Folders persist first so the orphan sweep in SaveClipsAsync
// sees up-to-date marker files when reclaiming empty directories.
await _repository.SaveFoldersAsync(Folders.ToList());
await _repository.SaveClipsAsync(clipData);
var sanitized = 0;
void OnSanitized(object? _, FolderPathSanitizedEventArgs __) => sanitized++;
var fileRepo = _repository as ClipRepository;
if (fileRepo is not null) fileRepo.FolderPathSanitized += OnSanitized;

try
{
// Folders persist first so the orphan sweep in SaveClipsAsync
// sees up-to-date marker files when reclaiming empty directories.
await _repository.SaveFoldersAsync(Folders.ToList());
await _repository.SaveClipsAsync(clipData);
}
finally
{
if (fileRepo is not null) fileRepo.FolderPathSanitized -= OnSanitized;
}

foreach (var c in FileMakerClips) c.MarkSaved();

ShowStatus($"Saved {clipData.Count} clip(s) to {_repository.CurrentLocation}");
var hadSanitization = sanitized > 0;
var suffix = hadSanitization
? $"; sanitized {sanitized} folder path(s) — see log for dropped segments"
: "";
ShowStatus(
$"Saved {clipData.Count} clip(s) to {_repository.CurrentLocation}{suffix}",
isError: hadSanitization);
}
catch (Exception e)
{
Expand Down
95 changes: 95 additions & 0 deletions tests/SharpFM.Tests/Models/ClipRepositoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -502,4 +502,99 @@ await repo.SaveClipsAsync([new("Evil", "Mac-XMSS", "<x/>")
if (Directory.Exists(sibling)) Directory.Delete(sibling, true);
}
}

[Fact]
public async Task SaveClipsAsync_TraversalSegment_RaisesFolderPathSanitizedEvent()
{
var dir = CreateTempDir();
try
{
var repo = new ClipRepository(dir);
FolderPathSanitizedEventArgs? captured = null;
repo.FolderPathSanitized += (_, e) => captured = e;

await repo.SaveClipsAsync([new("X", "Mac-XMSS", "<x/>")
{ FolderPath = new[] { "..", "Real" } }]);

Assert.NotNull(captured);
Assert.Equal(new[] { "..", "Real" }, captured!.Original);
Assert.Equal(new[] { "Real" }, captured.Sanitized);
Assert.Equal(new[] { ".." }, captured.DroppedSegments);
}
finally { Directory.Delete(dir, true); }
}

[Fact]
public async Task SaveClipsAsync_EmptySegment_RaisesEventNamingDropped()
{
var dir = CreateTempDir();
try
{
var repo = new ClipRepository(dir);
FolderPathSanitizedEventArgs? captured = null;
repo.FolderPathSanitized += (_, e) => captured = e;

await repo.SaveClipsAsync([new("X", "Mac-XMSS", "<x/>")
{ FolderPath = new[] { "Real", " " } }]);

Assert.NotNull(captured);
Assert.Equal(new[] { "Real" }, captured!.Sanitized);
Assert.Single(captured.DroppedSegments);
}
finally { Directory.Delete(dir, true); }
}

[Fact]
public async Task SaveClipsAsync_AllValidSegments_DoesNotRaiseEvent()
{
var dir = CreateTempDir();
try
{
var repo = new ClipRepository(dir);
var fired = 0;
repo.FolderPathSanitized += (_, _) => fired++;

await repo.SaveClipsAsync([new("X", "Mac-XMSS", "<x/>")
{ FolderPath = new[] { "Group", "Sub" } }]);

Assert.Equal(0, fired);
}
finally { Directory.Delete(dir, true); }
}

[Fact]
public async Task SaveClipsAsync_PercentEncodedOnly_DoesNotRaiseEvent()
{
var dir = CreateTempDir();
try
{
var repo = new ClipRepository(dir);
var fired = 0;
repo.FolderPathSanitized += (_, _) => fired++;

await repo.SaveClipsAsync([new("X", "Mac-XMSS", "<x/>")
{ FolderPath = new[] { "Date/Time" } }]);

Assert.Equal(0, fired);
}
finally { Directory.Delete(dir, true); }
}

[Fact]
public async Task SaveFoldersAsync_TraversalSegment_RaisesFolderPathSanitizedEvent()
{
var dir = CreateTempDir();
try
{
var repo = new ClipRepository(dir);
FolderPathSanitizedEventArgs? captured = null;
repo.FolderPathSanitized += (_, e) => captured = e;

await repo.SaveFoldersAsync([new(new[] { "..", "Real" })]);

Assert.NotNull(captured);
Assert.Equal(new[] { ".." }, captured!.DroppedSegments);
}
finally { Directory.Delete(dir, true); }
}
}
28 changes: 28 additions & 0 deletions tests/SharpFM.Tests/ViewModels/MainWindowViewModelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,34 @@ public async Task CopySelectedToClip_NoSelection_ShowsStatus()
Assert.Equal("No clip selected", vm.StatusMessage);
}

[Fact]
public async Task SaveClips_SanitizationDropsSegment_StatusIncludesSanitizedSuffix()
{
var vm = CreateVm();
ResetRepoState(vm);
var clip = Clip.FromXml("Test", "Mac-XMSC", "<x/>");
vm.FileMakerClips.Add(new ClipViewModel(clip) { FolderPath = new[] { "..", "Real" } });

await vm.SaveClipsStorageAsync();

Assert.Contains("Saved", vm.StatusMessage);
Assert.Contains("sanitized 1 folder path", vm.StatusMessage);
}

[Fact]
public async Task SaveClips_NoSanitization_StatusOmitsSuffix()
{
var vm = CreateVm();
ResetRepoState(vm);
var clip = Clip.FromXml("Test", "Mac-XMSC", "<x/>");
vm.FileMakerClips.Add(new ClipViewModel(clip));

await vm.SaveClipsStorageAsync();

Assert.Contains("Saved", vm.StatusMessage);
Assert.DoesNotContain("sanitized", vm.StatusMessage, StringComparison.OrdinalIgnoreCase);
}

[Fact]
public async Task PasteFileMakerClipData_NoFormats_ShowsStatus()
{
Expand Down
Loading