From 396da106cdf1b7d913c73520c45661343fe66bae Mon Sep 17 00:00:00 2001 From: Aqua Date: Fri, 6 Nov 2026 18:29:25 +0000 Subject: [PATCH 1/5] One StoreFileName in Core: no id collisions, no '..' escape The daemon and native-runtime SafeFileName copies sanitised differently: the daemon mapped a/b, a b, a:b and a-b to the same name (a later patch overwrote an earlier artifact) and returned ".." unchanged, so a sessionId or runId of ".." resolved one directory above the store. Replace both with AquaSynth.Dsl.StoreFileName.From. Ids within [A-Za-z0-9._-] pass through; any other id is sanitised and suffixed with the first 8 hex of SHA-256 of the original. Empty, "." and all-dot ids are refused (ArgumentException). Co-Authored-By: Claude Sonnet 5.5 --- src/AquaSynth.Core/StoreFileName.cs | 38 +++++++++++ src/AquaSynth.Faust/AquaSynthDaemonService.cs | 25 ++----- src/AquaSynth.Faust/NativeFaustRuntime.cs | 10 +-- .../AquaSynth.Dsl.Tests/StoreFileNameTests.cs | 68 +++++++++++++++++++ 4 files changed, 114 insertions(+), 27 deletions(-) create mode 100644 src/AquaSynth.Core/StoreFileName.cs create mode 100644 tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs diff --git a/src/AquaSynth.Core/StoreFileName.cs b/src/AquaSynth.Core/StoreFileName.cs new file mode 100644 index 0000000..c18bda1 --- /dev/null +++ b/src/AquaSynth.Core/StoreFileName.cs @@ -0,0 +1,38 @@ +using System.Security.Cryptography; +using System.Text; + +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 "..". +/// +public static class StoreFileName +{ + public static string From(string id) + { + ArgumentNullException.ThrowIfNull(id); + if (id.Length == 0 || id.All(c => c == '.')) + { + throw new ArgumentException($"Id '{id}' cannot name a store file.", nameof(id)); + } + + var builder = new StringBuilder(id.Length); + foreach (var c in id) + { + 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('.')) + { + return safe; + } + + var hash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(id)))[..8].ToLowerInvariant(); + return $"{safe}-{hash}"; + } +} diff --git a/src/AquaSynth.Faust/AquaSynthDaemonService.cs b/src/AquaSynth.Faust/AquaSynthDaemonService.cs index 28ae62d..74f6830 100644 --- a/src/AquaSynth.Faust/AquaSynthDaemonService.cs +++ b/src/AquaSynth.Faust/AquaSynthDaemonService.cs @@ -556,9 +556,9 @@ public sealed class AquaSynthDaemonService : IDisposable session.Stream.ProcessBlock([], outputs, command.FrameCount, session.Controls); var mono = MixMono(outputs, command.FrameCount, command.Gain * session.Gain); var blockId = BlockId(command.CommandId); - var blockDir = Path.Combine(options.StoreRoot, "live", SafeFileName(command.SessionId)); + var blockDir = Path.Combine(options.StoreRoot, "live", StoreFileName.From(command.SessionId)); Directory.CreateDirectory(blockDir); - var floatPath = Path.Combine(blockDir, $"{SafeFileName(blockId)}.f32"); + var floatPath = Path.Combine(blockDir, $"{StoreFileName.From(blockId)}.f32"); await WriteFloat32Async(floatPath, mono).ConfigureAwait(false); var sequence = session.NextSequence(); @@ -703,8 +703,8 @@ public sealed class AquaSynthDaemonService : IDisposable var runId = $"{command.CommandId}-stream"; var verseId = "aquasynth.instrument"; - var audioStreamId = $"aquasynth.audio.{SafeFileName(command.CommandId)}"; - var controlStreamId = $"aquasynth.controls.{SafeFileName(command.CommandId)}"; + var audioStreamId = $"aquasynth.audio.{StoreFileName.From(command.CommandId)}"; + var controlStreamId = $"aquasynth.controls.{StoreFileName.From(command.CommandId)}"; if (!string.Equals(compile.Status, "succeeded", StringComparison.Ordinal)) { @@ -741,7 +741,7 @@ public sealed class AquaSynthDaemonService : IDisposable .GroupBy(frame => frame.Block) .ToDictionary(group => group.Key, group => NormalizeControls(group.Last().Controls), EqualityComparer.Default); - var streamDir = Path.Combine(options.StoreRoot, "streams", SafeFileName(runId)); + var streamDir = Path.Combine(options.StoreRoot, "streams", StoreFileName.From(runId)); Directory.CreateDirectory(streamDir); var packets = new List(); using (var stream = patch.CreateStreamingPatch()) @@ -918,7 +918,7 @@ public sealed class AquaSynthDaemonService : IDisposable private async Task WriteReceiptAsync(string family, string id, T document, bool global = false) where T : class { - var filePath = Path.Combine(options.StoreRoot, family, $"{SafeFileName(id)}.cc"); + var filePath = Path.Combine(options.StoreRoot, family, $"{StoreFileName.From(id)}.cc"); Directory.CreateDirectory(Path.GetDirectoryName(filePath)!); // Opening hydrates; a receipt file holds exactly one document, so everything else goes before the write. using var cache = CultCacheMessagePack.Create(filePath); @@ -1129,18 +1129,7 @@ public sealed class AquaSynthDaemonService : IDisposable private static string BlockId(string commandId) => $"{commandId}-block"; - private static string SessionId(string patchId, string compileKey) => $"{SafeFileName(patchId)}-{compileKey[..Math.Min(12, compileKey.Length)]}"; - - private static string SafeFileName(string value) - { - var builder = new StringBuilder(value.Length); - foreach (var c in value) - { - builder.Append(char.IsLetterOrDigit(c) || c is '-' or '_' or '.' ? c : '-'); - } - - return builder.ToString().Trim('-'); - } + private static string SessionId(string patchId, string compileKey) => $"{StoreFileName.From(patchId)}-{compileKey[..Math.Min(12, compileKey.Length)]}"; private static string ToArtifactUri(string path) => $"file:///{Path.GetFullPath(path).Replace('\\', '/')}"; diff --git a/src/AquaSynth.Faust/NativeFaustRuntime.cs b/src/AquaSynth.Faust/NativeFaustRuntime.cs index e6b44c4..99bc288 100644 --- a/src/AquaSynth.Faust/NativeFaustRuntime.cs +++ b/src/AquaSynth.Faust/NativeFaustRuntime.cs @@ -633,18 +633,10 @@ public sealed class AquaSynthNativeCompiler : IDisposable } Directory.CreateDirectory(outputRoot); - var path = Path.Combine(outputRoot, $"{SafeFileName(patchId)}.dsp"); + var path = Path.Combine(outputRoot, $"{StoreFileName.From(patchId)}.dsp"); File.WriteAllText(path, source); return path; } - - private static string SafeFileName(string value) - { - var invalid = Path.GetInvalidFileNameChars().ToHashSet(); - var chars = value.Select(ch => invalid.Contains(ch) ? '_' : ch).ToArray(); - var name = new string(chars).Trim(); - return string.IsNullOrWhiteSpace(name) ? "aquasynth_patch" : name; - } } public static class AquaSynthFaustToolchain diff --git a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs new file mode 100644 index 0000000..24ad104 --- /dev/null +++ b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs @@ -0,0 +1,68 @@ +using AquaSynth.Faust; + +namespace AquaSynth.Dsl.Tests; + +public sealed class StoreFileNameTests +{ + [Fact] + public void DistinctIdsGetDistinctNames() + { + var ids = new[] { "a/b", "a b", "a:b", "a-b", "a_b", "a\\b", "a?b" }; + var names = ids.Select(StoreFileName.From).ToArray(); + Assert.Equal(ids.Length, names.Distinct(StringComparer.OrdinalIgnoreCase).Count()); + } + + [Theory] + [InlineData("dings.warm-bell")] + [InlineData("demo-stream-block")] + [InlineData("A_b.c-1")] + public void CleanIdsPassThroughUnchanged(string id) => Assert.Equal(id, StoreFileName.From(id)); + + [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(); + Assert.Equal($"a_b-{expected}", StoreFileName.From("a b")); + } + + [Theory] + [InlineData(".")] + [InlineData("..")] + [InlineData("")] + public void DotsAndEmptyAreRefused(string id) => Assert.Throws(() => StoreFileName.From(id)); + + [Theory] + [InlineData("../..")] + [InlineData("../../etc/passwd")] + [InlineData("..\\..\\x")] + [InlineData("/abs/path")] + [InlineData("C:\\abs")] + [InlineData("a.")] + [InlineData("a/..")] + public void NoIdCanMakePathCombineEscapeTheRoot(string id) + { + var root = Path.Combine(Path.GetTempPath(), "store-root-probe", "store"); + var name = StoreFileName.From(id); + Assert.Equal(name, Path.GetFileName(name)); + Assert.NotEqual(".", name); + Assert.NotEqual("..", name); + var full = Path.GetFullPath(Path.Combine(root, name)); + Assert.Equal(Path.GetFullPath(root), Path.GetDirectoryName(full)); + } + + [Fact] + public async Task DaemonReceiptForTraversalCommandIdStaysInsideStore() + { + 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)); + + Assert.Empty(Directory.GetFiles(parent)); + Assert.Empty(Directory.GetDirectories(parent).Where(d => Path.GetFileName(d) != "store")); + Assert.NotEmpty(Directory.GetFiles(Path.Combine(storeRoot, "compile"))); + } +} -- 2.47.3 From abfc816804575cc2bbeb77ccf843e568b6e21635 Mon Sep 17 00:00:00 2001 From: Aqua Date: Fri, 6 Nov 2026 20:00:30 +0000 Subject: [PATCH 2/5] 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; } -- 2.47.3 From de754b5b8c8035300729d50a90e082a2796a74da Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 09:29:43 +0000 Subject: [PATCH 3/5] StoreFileName: route IpaTrialOrchestrator and FaustCompiler through it; daemon test uses ".." Nibu's review at abfc816: the daemon test passed against the old sanitizer because "../../escape" never escaped there; the bare ".." did. IpaTrialOrchestrator (SafeName, raw BatchId in Path.Combine) and FaustCompiler.SafeArtifactName were second owners of the same decision. Co-Authored-By: Claude Sonnet 5.5 --- src/AquaSynth.Faust/FaustCompiler.cs | 8 ++------ src/AquaSynth.Faust/IpaTrialOrchestrator.cs | 11 ++++------- tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs | 4 ++-- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/AquaSynth.Faust/FaustCompiler.cs b/src/AquaSynth.Faust/FaustCompiler.cs index b2b197d..94f363b 100644 --- a/src/AquaSynth.Faust/FaustCompiler.cs +++ b/src/AquaSynth.Faust/FaustCompiler.cs @@ -222,12 +222,8 @@ public static class FaustCompiler _ => throw new ArgumentOutOfRangeException(nameof(language), language, null) }; - private static string SafeArtifactName(string value) - { - var sanitized = new string(value.Select(character => - char.IsAsciiLetterOrDigit(character) || character is '-' or '_' ? character : '_').ToArray()); - return string.IsNullOrWhiteSpace(sanitized) ? "aquasynth_patch" : sanitized; - } + private static string SafeArtifactName(string value) => + string.IsNullOrWhiteSpace(value) ? "aquasynth_patch" : StoreFileName.From(value); private static async Task FaustArchDirAsync(string faustPath, CancellationToken cancellationToken) { diff --git a/src/AquaSynth.Faust/IpaTrialOrchestrator.cs b/src/AquaSynth.Faust/IpaTrialOrchestrator.cs index 4f33bf4..d28b020 100644 --- a/src/AquaSynth.Faust/IpaTrialOrchestrator.cs +++ b/src/AquaSynth.Faust/IpaTrialOrchestrator.cs @@ -124,7 +124,7 @@ public static class IpaTrialOrchestrator throw new ArgumentException("At least one IPA trial target set is required.", nameof(trialSets)); } - var batchDirectory = Path.Combine(artifactRoot, options.BatchId); + var batchDirectory = Path.Combine(artifactRoot, StoreFileName.From(options.BatchId)); Directory.CreateDirectory(batchDirectory); var storePath = Path.Combine(batchDirectory, options.TrialResultStoreFileName); var referenceRenderer = new PinkTromboneReferenceRenderer(); @@ -156,7 +156,7 @@ public static class IpaTrialOrchestrator var candidateDir = Path.Combine(round.RoundDirectory, "audio", candidate.Id); Directory.CreateDirectory(candidateDir); var script = await File.ReadAllTextAsync(candidate.ScriptPath, cancellationToken).ConfigureAwait(false); - var source = FaustEmitter.EmitScript(script, new FaustExportOptions(SafeName(candidate.Id))).Source; + var source = FaustEmitter.EmitScript(script, new FaustExportOptions(StoreFileName.From(candidate.Id))).Source; var dspPath = Path.Combine(candidateDir, "candidate.dsp"); await File.WriteAllTextAsync(dspPath, source, cancellationToken).ConfigureAwait(false); @@ -274,7 +274,7 @@ public static class IpaTrialOrchestrator throw new ArgumentException("At least one IPA script candidate is required.", nameof(candidates)); } - var batchDirectory = Path.Combine(artifactRoot, options.BatchId); + var batchDirectory = Path.Combine(artifactRoot, StoreFileName.From(options.BatchId)); var candidateRoot = Path.Combine(batchDirectory, "agent-candidates"); var timelineRoot = Path.Combine(batchDirectory, "agent-timelines"); Directory.CreateDirectory(candidateRoot); @@ -298,7 +298,7 @@ public static class IpaTrialOrchestrator var startedAt = DateTimeOffset.UtcNow; var startedStamp = Stopwatch.GetTimestamp(); - var safeCandidateId = SafeName(candidate.CandidateId); + var safeCandidateId = StoreFileName.From(candidate.CandidateId); var candidateDir = Path.Combine(candidateRoot, safeCandidateId); Directory.CreateDirectory(candidateDir); var audioDir = Path.Combine(candidateDir, "audio"); @@ -721,9 +721,6 @@ public static class IpaTrialOrchestrator return samples; } - private static string SafeName(string value) => - new(value.Select(character => char.IsLetterOrDigit(character) ? character : '_').ToArray()); - private static string Escape(string value) => value.Contains(',') || value.Contains('"') || value.Contains('\n') || value.Contains('\r') ? $"\"{value.Replace("\"", "\"\"", StringComparison.Ordinal)}\"" diff --git a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs index d8012ae..1efb857 100644 --- a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs +++ b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs @@ -93,7 +93,7 @@ public sealed class StoreFileNameTests } [Fact] - public async Task DaemonSampleFilesForTraversalCommandIdStayInsideStore() + public async Task DaemonSampleFilesForDotDotCommandIdStayInsideStore() { var parent = Path.Combine(Path.GetTempPath(), "aquasynth-sfn-" + Guid.NewGuid().ToString("N")); var storeRoot = Path.Combine(parent, "store"); @@ -101,7 +101,7 @@ public sealed class StoreFileNameTests // 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", + "..", "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)); -- 2.47.3 From afd0cc9ec34803fceae898a7d005230816fbff7b Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 09:54:00 +0000 Subject: [PATCH 4/5] StoreFileName: device stems no longer survive as the stem; IpaTrialWorker paths use it "nul.x" was rewritten to "nul.x-", whose stem is still NUL on Windows; rewritten device names now get a leading underscore, and the test asserts the stem is not reserved. IpaTrialWorker candidate directories go through StoreFileName.From; its remaining SafeName only slugs uuid-suffixed record ids. Co-Authored-By: Claude Sonnet 5.5 --- src/AquaSynth.Core/StoreFileName.cs | 6 ++++++ tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs | 2 ++ tools/IpaTrialWorker/Program.cs | 6 +++--- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/AquaSynth.Core/StoreFileName.cs b/src/AquaSynth.Core/StoreFileName.cs index de3d7cf..c52e883 100644 --- a/src/AquaSynth.Core/StoreFileName.cs +++ b/src/AquaSynth.Core/StoreFileName.cs @@ -38,6 +38,12 @@ public static class StoreFileName return safe; } + // "nul.x-" still has the device stem "nul"; the underscore moves the whole name off it. + if (IsDeviceStem(safe)) + { + safe = "_" + safe; + } + var hash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(id)))[..HashLength].ToLowerInvariant(); return $"{safe}-{hash}"; } diff --git a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs index 1efb857..0fd3e31 100644 --- a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs +++ b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs @@ -48,6 +48,8 @@ public sealed class StoreFileNameTests var name = StoreFileName.From(id); Assert.NotEqual(id.ToLowerInvariant(), name.ToLowerInvariant()); Assert.Matches("-[0-9a-f]{16}$", name); + var stem = name.Split('.')[0].TrimEnd(' '); + Assert.DoesNotMatch("^(?i:con|prn|aux|nul|com[0-9]|lpt[0-9])$", stem); } [Fact] diff --git a/tools/IpaTrialWorker/Program.cs b/tools/IpaTrialWorker/Program.cs index b463dec..23d7750 100644 --- a/tools/IpaTrialWorker/Program.cs +++ b/tools/IpaTrialWorker/Program.cs @@ -228,7 +228,7 @@ static async Task SongScoreAsync(Dictionary options) { var startedAt = DateTimeOffset.UtcNow; var startedStamp = Stopwatch.GetTimestamp(); - var safeCandidateId = SafeName(candidate.CandidateId); + var safeCandidateId = StoreFileName.From(candidate.CandidateId); var candidateDir = Path.Combine(candidateRoot, safeCandidateId); var audioDir = Path.Combine(candidateDir, "audio"); Directory.CreateDirectory(audioDir); @@ -2658,10 +2658,10 @@ static bool IsOfficialMusicArtifactPath(string path) } static string CandidatePatchPath(SongSummaryRow row) => - Path.Combine(Path.GetDirectoryName(row.SummaryPath)!, "song-candidates", SafeName(row.CandidateId), "candidate.aqua"); + Path.Combine(Path.GetDirectoryName(row.SummaryPath)!, "song-candidates", StoreFileName.From(row.CandidateId), "candidate.aqua"); static string CandidateAnalysisPath(SongSummaryRow row) => - Path.Combine(Path.GetDirectoryName(row.SummaryPath)!, "song-candidates", SafeName(row.CandidateId), "candidate-analysis.md"); + Path.Combine(Path.GetDirectoryName(row.SummaryPath)!, "song-candidates", StoreFileName.From(row.CandidateId), "candidate-analysis.md"); static string EvaluatorReportPath(SongSummaryRow row) => Path.Combine(Path.GetDirectoryName(row.SummaryPath)!, "evaluator-report.md"); -- 2.47.3 From 80ebf390339b66ae913538329c26af11a48acaf2 Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 10:33:47 +0000 Subject: [PATCH 5/5] StoreFileName.Combine: batch ids from the command line cannot leave the artifact root IpaTrialWorker song-score took --batch-id raw into Path.Combine; the orchestrator sites share the new Combine. Tests cover traversal and absolute batch ids and the underscore on device stems (nul.x, LPT1.x); drop the double From on the emitter name. Co-Authored-By: Claude Sonnet 5.5 --- src/AquaSynth.Core/StoreFileName.cs | 3 +++ src/AquaSynth.Faust/IpaTrialOrchestrator.cs | 6 +++--- .../AquaSynth.Dsl.Tests/StoreFileNameTests.cs | 19 +++++++++++++++++++ tools/IpaTrialWorker/Program.cs | 2 +- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/AquaSynth.Core/StoreFileName.cs b/src/AquaSynth.Core/StoreFileName.cs index c52e883..0de02d1 100644 --- a/src/AquaSynth.Core/StoreFileName.cs +++ b/src/AquaSynth.Core/StoreFileName.cs @@ -48,6 +48,9 @@ public static class StoreFileName return $"{safe}-{hash}"; } + /// A child of named for an external id; the result never leaves the root. + public static string Combine(string root, string id) => Path.Combine(root, From(id)); + private static bool IsFixedPoint(string id, string safe) { if (safe != id || id.EndsWith('.') || id.Any(char.IsAsciiLetterUpper) || IsDeviceStem(id) || EndsWithHashShape(id)) diff --git a/src/AquaSynth.Faust/IpaTrialOrchestrator.cs b/src/AquaSynth.Faust/IpaTrialOrchestrator.cs index d28b020..ef37d3d 100644 --- a/src/AquaSynth.Faust/IpaTrialOrchestrator.cs +++ b/src/AquaSynth.Faust/IpaTrialOrchestrator.cs @@ -124,7 +124,7 @@ public static class IpaTrialOrchestrator throw new ArgumentException("At least one IPA trial target set is required.", nameof(trialSets)); } - var batchDirectory = Path.Combine(artifactRoot, StoreFileName.From(options.BatchId)); + var batchDirectory = StoreFileName.Combine(artifactRoot, options.BatchId); Directory.CreateDirectory(batchDirectory); var storePath = Path.Combine(batchDirectory, options.TrialResultStoreFileName); var referenceRenderer = new PinkTromboneReferenceRenderer(); @@ -156,7 +156,7 @@ public static class IpaTrialOrchestrator var candidateDir = Path.Combine(round.RoundDirectory, "audio", candidate.Id); Directory.CreateDirectory(candidateDir); var script = await File.ReadAllTextAsync(candidate.ScriptPath, cancellationToken).ConfigureAwait(false); - var source = FaustEmitter.EmitScript(script, new FaustExportOptions(StoreFileName.From(candidate.Id))).Source; + var source = FaustEmitter.EmitScript(script, new FaustExportOptions(candidate.Id)).Source; var dspPath = Path.Combine(candidateDir, "candidate.dsp"); await File.WriteAllTextAsync(dspPath, source, cancellationToken).ConfigureAwait(false); @@ -274,7 +274,7 @@ public static class IpaTrialOrchestrator throw new ArgumentException("At least one IPA script candidate is required.", nameof(candidates)); } - var batchDirectory = Path.Combine(artifactRoot, StoreFileName.From(options.BatchId)); + var batchDirectory = StoreFileName.Combine(artifactRoot, options.BatchId); var candidateRoot = Path.Combine(batchDirectory, "agent-candidates"); var timelineRoot = Path.Combine(batchDirectory, "agent-timelines"); Directory.CreateDirectory(candidateRoot); diff --git a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs index 0fd3e31..3620874 100644 --- a/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs +++ b/tests/AquaSynth.Dsl.Tests/StoreFileNameTests.cs @@ -43,9 +43,12 @@ public sealed class StoreFileNameTests [InlineData("COM9.f32")] [InlineData("lpt0")] [InlineData("prn.cc")] + [InlineData("nul.x")] + [InlineData("LPT1.x")] public void WindowsDeviceStemsAreRewritten(string id) { var name = StoreFileName.From(id); + Assert.StartsWith("_", name); Assert.NotEqual(id.ToLowerInvariant(), name.ToLowerInvariant()); Assert.Matches("-[0-9a-f]{16}$", name); var stem = name.Split('.')[0].TrimEnd(' '); @@ -115,5 +118,21 @@ public sealed class StoreFileNameTests } } + [Theory] + [InlineData("../x")] + [InlineData("..\\x")] + [InlineData("/etc/x")] + [InlineData("C:\\x")] + public void BatchIdsFromTheCommandLineCannotLeaveTheArtifactRoot(string batchId) + { + var root = Path.Combine(Path.GetTempPath(), "artifact-root-probe", "artifacts"); + var full = Path.GetFullPath(StoreFileName.Combine(root, batchId)); + Assert.Equal(Path.GetFullPath(root), Path.GetDirectoryName(full)); + } + + [Fact] + public void BareDotDotBatchIdIsRefused() => + Assert.Throws(() => StoreFileName.Combine(Path.GetTempPath(), "..")); + private static string UriPath(string uri) => new Uri(uri).LocalPath; } diff --git a/tools/IpaTrialWorker/Program.cs b/tools/IpaTrialWorker/Program.cs index 23d7750..f7089d1 100644 --- a/tools/IpaTrialWorker/Program.cs +++ b/tools/IpaTrialWorker/Program.cs @@ -214,7 +214,7 @@ static async Task SongScoreAsync(Dictionary options) throw new InvalidDataException($"No .aqua song candidates found under `{patchRoot}`."); } - var batchDirectory = Path.Combine(artifactRoot, batchId); + var batchDirectory = StoreFileName.Combine(artifactRoot, batchId); var candidateRoot = Path.Combine(batchDirectory, "song-candidates"); var timelineRoot = Path.Combine(batchDirectory, "song-timelines"); Directory.CreateDirectory(candidateRoot); -- 2.47.3