From 023b67a6f55acaa0d6be0c9c6a2152fe667cf8c5 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 19 Aug 2026 14:39:12 -0500 Subject: [PATCH 1/2] Make fetched-content filenames collision-proof within one second WebFetchTool built each saved-content filename from a second-precision timestamp only. Two fetches of the same URL inside one second built the same filename. File.WriteAllBytes and File.WriteAllText both truncate an existing file, so the second fetch overwrote the first one with no error and no warning. Add a short random suffix to the filename. Keep the timestamp for a human to read. The random suffix gives real uniqueness, matching the pattern in WebhookExecutionService (a guid segment on the actor name). This bug is a sibling of the reminder execution child-name collision fixed in 30ff5d28: a millisecond timestamp collided when two fires landed inside one tick. Add a regression test with a frozen clock. Two fetches of the same URL at the same frozen second now produce two distinct files, and both keep their own content on disk. --- .../Tools/WebFetchToolTests.cs | 74 +++++++++++++++++++ src/Netclaw.Actors/Tools/WebFetchTool.cs | 9 ++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/Netclaw.Actors.Tests/Tools/WebFetchToolTests.cs b/src/Netclaw.Actors.Tests/Tools/WebFetchToolTests.cs index c6fcf0733..ed8cbdd2a 100644 --- a/src/Netclaw.Actors.Tests/Tools/WebFetchToolTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/WebFetchToolTests.cs @@ -4,6 +4,7 @@ // // ----------------------------------------------------------------------- using System.Text; +using Microsoft.Extensions.Time.Testing; using Netclaw.Actors.Tools; using Netclaw.Configuration; using Netclaw.Tests.Utilities; @@ -735,6 +736,79 @@ public async Task ExecuteAsync_body_under_cap_has_no_truncation_notice() Assert.DoesNotContain("content truncated", result); } + [Fact] + public async Task ExecuteAsync_same_url_same_frozen_second_saves_two_distinct_files() + { + // Regression for the filename collision bug: the saved-content + // filename used only second-precision time as its unique part. Two + // fetches of the same URL within one second built the same filename, + // and the second write silently overwrote the first (File.WriteAllText + // truncates an existing file; it does not throw). Freeze the clock at + // one second and fetch twice to prove both files now survive. + var frozenTime = new FakeTimeProvider(DateTimeOffset.Parse("2026-01-01T00:00:00Z")); + var handler = new SequencedHttpHandler( + ("First

First fetch content.

", "text/html"), + ("Second

Second fetch content.

", "text/html")); + var httpClient = new HttpClient(handler); + var tool = new WebFetchTool(httpClient: httpClient, fetchDirectory: _dir.Path, timeProvider: frozenTime); + + var firstResult = await tool.ExecuteAsync( + ToolInput.Create("Url", "https://example.com/same-page"), + TestToolExecutionContext.CreateUnbound(), + CancellationToken.None); + + var secondResult = await tool.ExecuteAsync( + ToolInput.Create("Url", "https://example.com/same-page"), + TestToolExecutionContext.CreateUnbound(), + CancellationToken.None); + + var files = Directory.GetFiles(_dir.Path, "*.html"); + Assert.Equal(2, files.Length); + + var firstPath = ExtractSavedPath(firstResult); + var secondPath = ExtractSavedPath(secondResult); + Assert.NotEqual(firstPath, secondPath); + Assert.True(File.Exists(firstPath)); + Assert.True(File.Exists(secondPath)); + + var firstContent = await File.ReadAllTextAsync(firstPath, TestContext.Current.CancellationToken); + var secondContent = await File.ReadAllTextAsync(secondPath, TestContext.Current.CancellationToken); + Assert.Contains("First fetch content.", firstContent); + Assert.Contains("Second fetch content.", secondContent); + } + + private static string ExtractSavedPath(string toolResult) + { + const string marker = "Saved to: "; + var start = toolResult.IndexOf(marker, StringComparison.Ordinal) + marker.Length; + var end = toolResult.IndexOf(" (", start, StringComparison.Ordinal); + return toolResult[start..end]; + } + + private sealed class SequencedHttpHandler : HttpMessageHandler + { + private readonly (byte[] Bytes, string ContentType)[] _responses; + private int _index; + + public SequencedHttpHandler(params (string Content, string ContentType)[] responses) + { + _responses = [.. responses.Select(r => (Encoding.UTF8.GetBytes(r.Content), r.ContentType))]; + } + + protected override Task SendAsync( + HttpRequestMessage request, CancellationToken ct) + { + var (bytes, contentType) = _responses[Math.Min(_index++, _responses.Length - 1)]; + var content = new ByteArrayContent(bytes); + content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue(contentType); + var response = new HttpResponseMessage(System.Net.HttpStatusCode.OK) + { + Content = content + }; + return Task.FromResult(response); + } + } + private sealed class CountingHttpHandler(string content, string contentType) : HttpMessageHandler { public int Requests; diff --git a/src/Netclaw.Actors/Tools/WebFetchTool.cs b/src/Netclaw.Actors/Tools/WebFetchTool.cs index 684b10b7c..c571505d5 100644 --- a/src/Netclaw.Actors/Tools/WebFetchTool.cs +++ b/src/Netclaw.Actors/Tools/WebFetchTool.cs @@ -266,7 +266,14 @@ private string BuildFilePath(Uri uri, string directory, string extension) { Directory.CreateDirectory(directory); var sanitized = SanitizeForFilename(uri); - var filename = $"{sanitized}-{_timeProvider.GetUtcNow().ToUnixTimeSeconds()}{extension}"; + + // The second-precision timestamp is for a human to read, not for + // uniqueness. Two fetches of the same URL within one second gave the + // same filename, and File.WriteAllBytes/WriteAllText overwrote the + // first fetch with no warning. The guid segment gives real + // uniqueness; the timestamp stays for readability. + var uniqueSuffix = Guid.NewGuid().ToString("N")[..8]; + var filename = $"{sanitized}-{_timeProvider.GetUtcNow().ToUnixTimeSeconds()}-{uniqueSuffix}{extension}"; return Path.Combine(directory, filename); } From 0d352715cc97bcf42999a33746d95da2e7767b23 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 19 Aug 2026 15:05:35 -0500 Subject: [PATCH 2/2] Use a millisecond timestamp in fetched-content filenames The guid suffix owns uniqueness. The timestamp's only job is human readability and name-sort order, and at second precision two fetches in the same second sort by random guid instead of fetch order. --- src/Netclaw.Actors/Tools/WebFetchTool.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Netclaw.Actors/Tools/WebFetchTool.cs b/src/Netclaw.Actors/Tools/WebFetchTool.cs index c571505d5..3912422c4 100644 --- a/src/Netclaw.Actors/Tools/WebFetchTool.cs +++ b/src/Netclaw.Actors/Tools/WebFetchTool.cs @@ -267,13 +267,14 @@ private string BuildFilePath(Uri uri, string directory, string extension) Directory.CreateDirectory(directory); var sanitized = SanitizeForFilename(uri); - // The second-precision timestamp is for a human to read, not for - // uniqueness. Two fetches of the same URL within one second gave the - // same filename, and File.WriteAllBytes/WriteAllText overwrote the - // first fetch with no warning. The guid segment gives real - // uniqueness; the timestamp stays for readability. + // The timestamp is for a human to read and sort, not for uniqueness. + // Two fetches of the same URL within one second gave the same + // filename, and File.WriteAllBytes/WriteAllText overwrote the first + // fetch with no warning. The guid segment gives real uniqueness; + // millisecond precision keeps same-second fetches in fetch order + // when the directory is sorted by name. var uniqueSuffix = Guid.NewGuid().ToString("N")[..8]; - var filename = $"{sanitized}-{_timeProvider.GetUtcNow().ToUnixTimeSeconds()}-{uniqueSuffix}{extension}"; + var filename = $"{sanitized}-{_timeProvider.GetUtcNow().ToUnixTimeMilliseconds()}-{uniqueSuffix}{extension}"; return Path.Combine(directory, filename); }