From abfc816804575cc2bbeb77ccf843e568b6e21635 Mon Sep 17 00:00:00 2001 From: Aqua Date: Fri, 6 Nov 2026 20:00:30 +0000 Subject: [PATCH] StoreFileName: close review findings (collisions, case, device stems, length, renderId) - Pass-through ids may not end in "-<16 hex>", so a rewritten name fed back as an id cannot alias it; hash widened 32 -> 64 bits. - Ids with uppercase are rewritten (case-insensitive filesystems). - Windows device stems (CON PRN AUX NUL COM0-9 LPT0-9, any extension) are rewritten. - Stem capped at 100 chars; longer ids carry the hash. - Daemon samples/ write now goes through StoreFileName.From(renderId). - Tests: rewritten-as-id, case-only, device stems, overlong; daemon test uses a compiling patch so the samples/ write is reached when Faust is present. Co-Authored-By: Claude Sonnet 5.5 --- src/AquaSynth.Core/StoreFileName.cs | 59 +++++++++++++++--- src/AquaSynth.Faust/AquaSynthDaemonService.cs | 5 +- .../AquaSynth.Dsl.Tests/StoreFileNameTests.cs | 61 +++++++++++++++++-- 3 files changed, 109 insertions(+), 16 deletions(-) diff --git a/src/AquaSynth.Core/StoreFileName.cs b/src/AquaSynth.Core/StoreFileName.cs index c18bda1..de3d7cf 100644 --- a/src/AquaSynth.Core/StoreFileName.cs +++ b/src/AquaSynth.Core/StoreFileName.cs @@ -5,12 +5,19 @@ namespace AquaSynth.Dsl; /// /// The one mapping from a caller-chosen id to a single file-name component in a store. -/// Ids already within [A-Za-z0-9._-] pass through unchanged. Anything else is sanitised and -/// suffixed with the first 8 hex of SHA-256 of the original id, so distinct ids stay distinct. -/// The result is never a path: no separators, never "." or "..". +/// An id passes through unchanged only if it is already a fixed point: ASCII [A-Za-z0-9._-], +/// all lowercase (stores live on case-insensitive filesystems), at most long, +/// no trailing dot, not a Windows device stem (CON, NUL, COM1, ...), and not ending in a +/// "-" plus 16 hex digits. Every other id is sanitised and suffixed with "-" plus the first +/// 16 hex of SHA-256 of the original id. Passed-through names never end in the hash shape and +/// rewritten names always do, so the two sets cannot meet; distinct ids stay distinct up to a 64-bit +/// hash. The result is never a path: no separators, never "." or "..". /// public static class StoreFileName { + private const int MaxStem = 100; + private const int HashLength = 16; + public static string From(string id) { ArgumentNullException.ThrowIfNull(id); @@ -19,20 +26,56 @@ public static class StoreFileName throw new ArgumentException($"Id '{id}' cannot name a store file.", nameof(id)); } - var builder = new StringBuilder(id.Length); - foreach (var c in id) + var builder = new StringBuilder(Math.Min(id.Length, MaxStem)); + foreach (var c in id.Take(MaxStem)) { builder.Append(c is >= 'a' and <= 'z' or >= 'A' and <= 'Z' or >= '0' and <= '9' or '-' or '_' or '.' ? c : '_'); } var safe = builder.ToString(); - // A trailing dot is dropped by Windows, so "a." would alias "a"; hash it like any other rewrite. - if (safe == id && !id.EndsWith('.')) + if (IsFixedPoint(id, safe)) { return safe; } - var hash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(id)))[..8].ToLowerInvariant(); + var hash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(id)))[..HashLength].ToLowerInvariant(); return $"{safe}-{hash}"; } + + private static bool IsFixedPoint(string id, string safe) + { + if (safe != id || id.EndsWith('.') || id.Any(char.IsAsciiLetterUpper) || IsDeviceStem(id) || EndsWithHashShape(id)) + { + return false; + } + + return true; + } + + // Windows resolves CON, PRN, AUX, NUL, COM0-9, LPT0-9 as devices whatever the extension. + private static bool IsDeviceStem(string id) + { + var dot = id.IndexOf('.'); + var stem = (dot < 0 ? id : id[..dot]).TrimEnd(' '); + return stem.Length switch + { + 3 => stem.Equals("con", StringComparison.OrdinalIgnoreCase) + || stem.Equals("prn", StringComparison.OrdinalIgnoreCase) + || stem.Equals("aux", StringComparison.OrdinalIgnoreCase) + || stem.Equals("nul", StringComparison.OrdinalIgnoreCase), + 4 => char.IsAsciiDigit(stem[3]) + && (stem.StartsWith("com", StringComparison.OrdinalIgnoreCase) || stem.StartsWith("lpt", StringComparison.OrdinalIgnoreCase)), + _ => false, + }; + } + + private static bool EndsWithHashShape(string id) + { + if (id.Length <= HashLength || id[^(HashLength + 1)] != '-') + { + return false; + } + + return id[^HashLength..].All(c => c is >= '0' and <= '9' or >= 'a' and <= 'f'); + } } diff --git a/src/AquaSynth.Faust/AquaSynthDaemonService.cs b/src/AquaSynth.Faust/AquaSynthDaemonService.cs index 74f6830..0021895 100644 --- a/src/AquaSynth.Faust/AquaSynthDaemonService.cs +++ b/src/AquaSynth.Faust/AquaSynthDaemonService.cs @@ -661,8 +661,9 @@ public sealed class AquaSynthDaemonService : IDisposable var renderId = RenderId(command.CommandId); var sampleDir = Path.Combine(options.StoreRoot, "samples"); Directory.CreateDirectory(sampleDir); - var floatPath = Path.Combine(sampleDir, $"{renderId}.f32"); - var wavPath = Path.Combine(sampleDir, $"{renderId}.wav"); + var sampleStem = StoreFileName.From(renderId); + var floatPath = Path.Combine(sampleDir, $"{sampleStem}.f32"); + var wavPath = Path.Combine(sampleDir, $"{sampleStem}.wav"); await WriteFloat32Async(floatPath, samples).ConfigureAwait(false); await WriteWavAsync(wavPath, samples, patch.Manifest.SampleRate).ConfigureAwait(false); diff --git a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs index 24ad104..d8012ae 100644 --- a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs +++ b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs @@ -15,14 +15,55 @@ public sealed class StoreFileNameTests [Theory] [InlineData("dings.warm-bell")] [InlineData("demo-stream-block")] - [InlineData("A_b.c-1")] + [InlineData("a_b.c-1")] + [InlineData("conx")] + [InlineData("com10")] public void CleanIdsPassThroughUnchanged(string id) => Assert.Equal(id, StoreFileName.From(id)); + [Fact] + public void RewrittenIdOutputAsAnIdDoesNotCollideWithTheOriginal() + { + var rewritten = StoreFileName.From("a b"); + Assert.NotEqual(rewritten, StoreFileName.From(rewritten)); + Assert.NotEqual(StoreFileName.From("a_b"), rewritten); + } + + [Fact] + public void IdsDifferingOnlyByCaseGetDistinctNames() + { + var names = new[] { "A", "a", "Ab", "aB", "ab" }.Select(StoreFileName.From).ToArray(); + Assert.Equal(names.Length, names.Distinct(StringComparer.OrdinalIgnoreCase).Count()); + } + + [Theory] + [InlineData("CON")] + [InlineData("nul")] + [InlineData("Aux.txt")] + [InlineData("com1")] + [InlineData("COM9.f32")] + [InlineData("lpt0")] + [InlineData("prn.cc")] + public void WindowsDeviceStemsAreRewritten(string id) + { + var name = StoreFileName.From(id); + Assert.NotEqual(id.ToLowerInvariant(), name.ToLowerInvariant()); + Assert.Matches("-[0-9a-f]{16}$", name); + } + + [Fact] + public void OverlongIdsAreBoundedAndStayDistinct() + { + var a = StoreFileName.From(new string('x', 400)); + var b = StoreFileName.From(new string('x', 399) + "y"); + Assert.True(a.Length + ".cc".Length <= 255); + Assert.NotEqual(a, b); + } + [Fact] public void RewrittenIdsCarryHashOfTheOriginal() { // sha256("a b") begins c2a5..., checked independently of the implementation. - var expected = Convert.ToHexString(System.Security.Cryptography.SHA256.HashData("a b"u8.ToArray()))[..8].ToLowerInvariant(); + var expected = Convert.ToHexString(System.Security.Cryptography.SHA256.HashData("a b"u8.ToArray()))[..16].ToLowerInvariant(); Assert.Equal($"a_b-{expected}", StoreFileName.From("a b")); } @@ -52,17 +93,25 @@ public sealed class StoreFileNameTests } [Fact] - public async Task DaemonReceiptForTraversalCommandIdStaysInsideStore() + public async Task DaemonSampleFilesForTraversalCommandIdStayInsideStore() { var parent = Path.Combine(Path.GetTempPath(), "aquasynth-sfn-" + Guid.NewGuid().ToString("N")); var storeRoot = Path.Combine(parent, "store"); using var service = new AquaSynthDaemonService(new AquaSynthDaemonOptions(storeRoot)); - await service.SampleAsync(new AquaSynthInstrumentSampleCommand( - "../../escape", "test.invalid", "test_invalid", "this is not aqua syntax", DurationSeconds: 0.1f)); + // A patch that compiles reaches the samples/ write; without native Faust only the compile-failed branch runs. + var result = await service.SampleAsync(new AquaSynthInstrumentSampleCommand( + "../../escape", "test.sine", "test_sine", + "voice wave=sine freq=440 gain=.2 attack=.001 sustain=.05 decay=.05", DurationSeconds: 0.1f)); Assert.Empty(Directory.GetFiles(parent)); Assert.Empty(Directory.GetDirectories(parent).Where(d => Path.GetFileName(d) != "store")); - Assert.NotEmpty(Directory.GetFiles(Path.Combine(storeRoot, "compile"))); + if (string.Equals(result.RenderReceipt.Status, "succeeded", StringComparison.Ordinal)) + { + Assert.Equal(storeRoot, Path.GetDirectoryName(Path.GetDirectoryName(UriPath(result.RenderReceipt.Float32Uri)))); + Assert.Equal(storeRoot, Path.GetDirectoryName(Path.GetDirectoryName(UriPath(result.RenderReceipt.WavUri)))); + } } + + private static string UriPath(string uri) => new Uri(uri).LocalPath; }