fix(work-orders): harden private file path containment and WeekRolled logs

Reject sibling-prefix path escapes in private storage and log WeekRolled dates as int components to clear CodeQL log-injection findings.
This commit is contained in:
Arthur Bassi 2026-07-21 14:20:00 -03:00
parent 719a5b05fd
commit c12cd3c17e
3 changed files with 51 additions and 9 deletions

View file

@ -85,4 +85,27 @@ public class WorkOrderPrivateFileStorageTests
Directory.Delete(root, recursive: true); Directory.Delete(root, recursive: true);
} }
} }
[Fact]
public void TryResolvePhysicalPath_RejectsSiblingPrefixBypass()
{
var (storage, root) = CreateStorage();
try
{
var siblingDir = Path.Combine(root, "App_Data", "workorder-files-secret");
Directory.CreateDirectory(siblingDir);
var siblingFile = Path.Combine(siblingDir, "x.pdf");
File.WriteAllBytes(siblingFile, new byte[] { 9, 9, 9 });
storage.TryResolvePhysicalPath(
"workorder-files/../workorder-files-secret/x.pdf",
out _)
.Should().BeFalse();
}
finally
{
if (Directory.Exists(root))
Directory.Delete(root, recursive: true);
}
}
} }

View file

@ -51,7 +51,7 @@ namespace Api.SeaHavenIndustries.Helper
var directory = Path.Combine(RootPath, workOrderId.ToString()); var directory = Path.Combine(RootPath, workOrderId.ToString());
var fullPath = Path.GetFullPath(Path.Combine(directory, serverName)); var fullPath = Path.GetFullPath(Path.Combine(directory, serverName));
if (!fullPath.StartsWith(RootPath, StringComparison.OrdinalIgnoreCase)) if (!IsPathUnderRoot(fullPath, RootPath))
throw new InvalidOperationException("Resolved path escapes storage root."); throw new InvalidOperationException("Resolved path escapes storage root.");
Directory.CreateDirectory(directory); Directory.CreateDirectory(directory);
@ -76,7 +76,7 @@ namespace Api.SeaHavenIndustries.Helper
var relative = normalized[(RelativeRoot.Length + 1)..]; var relative = normalized[(RelativeRoot.Length + 1)..];
var candidate = Path.GetFullPath(Path.Combine(RootPath, relative)); var candidate = Path.GetFullPath(Path.Combine(RootPath, relative));
if (!candidate.StartsWith(RootPath, StringComparison.OrdinalIgnoreCase)) if (!IsPathUnderRoot(candidate, RootPath))
return false; return false;
if (!System.IO.File.Exists(candidate)) if (!System.IO.File.Exists(candidate))
@ -86,6 +86,23 @@ namespace Api.SeaHavenIndustries.Helper
return true; return true;
} }
/// <summary>
/// Separator-aware containment check so sibling prefixes like
/// <c>.../workorder-files-secret</c> do not match <c>.../workorder-files</c>.
/// </summary>
private static bool IsPathUnderRoot(string candidate, string root)
{
var fullRoot = Path.GetFullPath(root);
var fullCandidate = Path.GetFullPath(candidate);
if (string.Equals(fullCandidate, fullRoot, StringComparison.OrdinalIgnoreCase))
return true;
var rootPrefix = fullRoot.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
+ Path.DirectorySeparatorChar;
return fullCandidate.StartsWith(rootPrefix, StringComparison.OrdinalIgnoreCase);
}
public void TryDelete(string? storedValue) public void TryDelete(string? storedValue)
{ {
if (!TryResolvePhysicalPath(storedValue, out var path)) if (!TryResolvePhysicalPath(storedValue, out var path))

View file

@ -37,10 +37,11 @@ namespace SeaHaven.Services.Implementation
var skipped = 0; var skipped = 0;
var failed = 0; var failed = 0;
// Log only system-formatted dates (yyyy-MM-dd) — avoid free-form user-derived strings in logs. // Log date components as ints only — avoids CodeQL log-injection on user-derived DateOnly.
_logger.LogInformation( _logger.LogInformation(
"WeekRolled job started. SourceWeekStart={SourceWeekStart:yyyy-MM-dd}, SourceWeekEnd={SourceWeekEnd:yyyy-MM-dd}", "WeekRolled job started. SourceWeekStart={StartYear}-{StartMonth:D2}-{StartDay:D2}, SourceWeekEnd={EndYear}-{EndMonth:D2}-{EndDay:D2}",
sourceWeekStart, sourceWeekEnd); sourceWeekStart.Year, sourceWeekStart.Month, sourceWeekStart.Day,
sourceWeekEnd.Year, sourceWeekEnd.Month, sourceWeekEnd.Day);
// Always fetch the first page of remaining candidates. Processed rows write ledger // Always fetch the first page of remaining candidates. Processed rows write ledger
// entries that exclude them from the next query; offset paging would skip unprocessed rows. // entries that exclude them from the next query; offset paging would skip unprocessed rows.
@ -79,8 +80,8 @@ namespace SeaHaven.Services.Implementation
{ {
failed++; failed++;
_logger.LogError(ex, _logger.LogError(ex,
"WeekRolled failed for WorkOrderId={WorkOrderId}, SourceWeekStart={SourceWeekStart:yyyy-MM-dd}", "WeekRolled failed for WorkOrderId={WorkOrderId}, SourceWeekStart={StartYear}-{StartMonth:D2}-{StartDay:D2}",
workOrderId, sourceWeekStart); workOrderId, sourceWeekStart.Year, sourceWeekStart.Month, sourceWeekStart.Day);
} }
} }
@ -102,8 +103,9 @@ namespace SeaHaven.Services.Implementation
}; };
_logger.LogInformation( _logger.LogInformation(
"WeekRolled job completed. SourceWeekStart={SourceWeekStart:yyyy-MM-dd}, Processed={Processed}, Skipped={Skipped}, Failed={Failed}, DurationMs={DurationMs}", "WeekRolled job completed. SourceWeekStart={StartYear}-{StartMonth:D2}-{StartDay:D2}, Processed={Processed}, Skipped={Skipped}, Failed={Failed}, DurationMs={DurationMs}",
sourceWeekStart, processed, skipped, failed, result.DurationMs); sourceWeekStart.Year, sourceWeekStart.Month, sourceWeekStart.Day,
processed, skipped, failed, result.DurationMs);
return result; return result;
} }