diff --git a/src/Ashlar.Manifest/Admission/GateStore.cs b/src/Ashlar.Manifest/Admission/GateStore.cs index 6fbafa0df..c9a56cd02 100644 --- a/src/Ashlar.Manifest/Admission/GateStore.cs +++ b/src/Ashlar.Manifest/Admission/GateStore.cs @@ -34,7 +34,7 @@ public sealed record GateRecord /// admitted and rejected records are immutable history, so there is no way to re-decide a /// refusal or quietly edit an admission, including for the vendor. /// -public sealed class GateStore +public sealed partial class GateStore { private static readonly JsonSerializerOptions Json = new() { @@ -194,7 +194,8 @@ public async Task DecideAsync(string proposalId, bool admit, string return decided; } - /// Fetches one record, or null. + /// Fetches one record, or null when absent. A file that exists but cannot be + /// read as a record is an error, never a null. public async Task GetAsync(string proposalId, CancellationToken ct = default) { var path = PathFor(proposalId); @@ -202,8 +203,7 @@ public async Task DecideAsync(string proposalId, bool admit, string { return null; } - await using var stream = File.OpenRead(path); - return await JsonSerializer.DeserializeAsync(stream, Json, ct).ConfigureAwait(false); + return await ReadRecordAsync(path, ct).ConfigureAwait(false); } /// Lists records, newest first, optionally filtered by state. @@ -212,9 +212,8 @@ public async Task> ListAsync(ProposalState? state = nu var records = new List(); foreach (var file in Directory.EnumerateFiles(_dir, "*.json")) { - await using var stream = File.OpenRead(file); - var record = await JsonSerializer.DeserializeAsync(stream, Json, ct).ConfigureAwait(false); - if (record is not null && (state is null || record.State == state)) + var record = await ReadRecordAsync(file, ct).ConfigureAwait(false); + if (state is null || record.State == state) { records.Add(record); } @@ -222,6 +221,35 @@ public async Task> ListAsync(ProposalState? state = nu return records.OrderByDescending(r => r.Proposal.ProposedAt).ToList(); } + /// + /// Reads one record file, FAIL-CLOSED. This used to skip records that deserialized to + /// null and let JsonException escape raw — and a corrupt HELD record silently vanishing + /// from the queue is an invisible pending decision, the worst possible failure shape + /// for an admission store. A store this class cannot fully read is a store it refuses + /// to summarize. + /// + private static async Task ReadRecordAsync(string path, CancellationToken ct) + { + try + { + await using var stream = File.OpenRead(path); + var record = await JsonSerializer.DeserializeAsync(stream, Json, ct).ConfigureAwait(false); + if (record is null) + { + throw new InvalidOperationException( + $"Corrupt gate record: {Path.GetFileName(path)} contains no record. " + + "Refusing to operate on a store that cannot be fully read — inspect or remove the file."); + } + return record; + } + catch (JsonException ex) + { + throw new InvalidOperationException( + $"Corrupt gate record: {Path.GetFileName(path)} is not valid JSON ({ex.Message}). " + + "Refusing to operate on a store that cannot be fully read — inspect or remove the file."); + } + } + /// /// How many extensions were admitted inside the budget window ending now. Drives the /// self-extending budget check. @@ -235,14 +263,41 @@ public async Task AdmittedInWindowAsync(TimeSpan window, DateTimeOffset now private string PathFor(string proposalId) { - // Fail closed on ids that would escape the store directory. - if (proposalId.Any(c => c == '/' || c == '\\' || c == '.')) + // ALLOWLIST, not blocklist. The old check blocked '/', '\' and '.' — and admitted + // the entire Windows hazard alphabet: reserved names (CON, NUL), trailing dots and + // spaces (Win32 strips them silently, so 'ext' and 'ext ' collide and append-once + // is bypassed), ':' (NTFS alternate data streams), and unicode confusables. Ids are + // machine-generated in this system; there is no reason to accept anything beyond + // this shape, so nothing beyond it is accepted. + if (!IdShape().IsMatch(proposalId)) + { + throw new ArgumentException( + $"Illegal proposal id '{proposalId}'. Ids are 1-64 characters of [A-Za-z0-9_-], starting alphanumeric.", + nameof(proposalId)); + } + // Win32 reserved device names are perfectly alphanumeric, so the shape check passes + // them — and they are denied on EVERY OS, not just Windows: a store written on Linux + // with a CON.json breaks the moment it syncs to a Windows machine. Portability means + // the same ids are legal everywhere. + if (Win32Reserved.Contains(proposalId)) { - throw new ArgumentException($"Illegal proposal id '{proposalId}'.", nameof(proposalId)); + throw new ArgumentException( + $"Illegal proposal id '{proposalId}': a Win32 reserved device name cannot be a store filename on any OS.", + nameof(proposalId)); } return Path.Combine(_dir, proposalId + ".json"); } + private static readonly HashSet Win32Reserved = new(StringComparer.OrdinalIgnoreCase) + { + "CON", "PRN", "AUX", "NUL", + "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", + "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", + }; + + [System.Text.RegularExpressions.GeneratedRegex("^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$")] + private static partial System.Text.RegularExpressions.Regex IdShape(); + private static async Task WriteAsync(string path, GateRecord record, CancellationToken ct) { // Write-then-move so a crash mid-write never leaves a truncated record. diff --git a/src/Ashlar.Manifest/ManifestLoader.cs b/src/Ashlar.Manifest/ManifestLoader.cs index d816b2471..789971283 100644 --- a/src/Ashlar.Manifest/ManifestLoader.cs +++ b/src/Ashlar.Manifest/ManifestLoader.cs @@ -60,6 +60,12 @@ public static bool TryLoad(string? yaml, out AshlarManifest? manifest, out strin return false; } + if (!YamlGuard.Check(yaml!, "manifest", out var guardReason)) + { + reason = guardReason; + return false; + } + // Read the top-level keys first, so a policy-owned key gets a precise explanation // rather than a generic schema error. Dictionary? raw; diff --git a/src/Ashlar.Manifest/PolicyLoader.cs b/src/Ashlar.Manifest/PolicyLoader.cs index ed5eb3ad4..85385cd23 100644 --- a/src/Ashlar.Manifest/PolicyLoader.cs +++ b/src/Ashlar.Manifest/PolicyLoader.cs @@ -62,6 +62,12 @@ public static bool TryLoad(string? yaml, out AshlarPolicy? policy, out string re return false; } + if (!YamlGuard.Check(yaml!, "policy", out var guardReason)) + { + reason = guardReason; + return false; + } + AshlarPolicy? parsed; try { diff --git a/src/Ashlar.Manifest/YamlGuard.cs b/src/Ashlar.Manifest/YamlGuard.cs new file mode 100644 index 000000000..bebad7ef4 --- /dev/null +++ b/src/Ashlar.Manifest/YamlGuard.cs @@ -0,0 +1,49 @@ +using System.Text.RegularExpressions; + +namespace Ashlar.Manifest; + +/// +/// Pre-parse guards both loaders apply before YAML ever reaches the deserializer. +/// +/// Two rules, both fail-closed. SIZE: a manifest or policy over 1 MB is rejected — +/// no honest configuration document is that large, and the cap bounds parser work on +/// garbage. ALIASES: YAML anchors/aliases are rejected outright, because YamlDotNet +/// expands them and a small document can be crafted to expand exponentially (the classic +/// billion-laughs shape); ashlar documents never need them. The scan is textual and +/// deliberately a little over-eager — a scalar that genuinely needs a token shaped like +/// &name or *name in anchor position does not belong in these files, and +/// the rejection says exactly what to change. +/// +public static partial class YamlGuard +{ + /// 1 MB. Configuration, not cargo. + public const int MaxBytes = 1024 * 1024; + + [GeneratedRegex(@"(^|[\s\[,{])[&*][A-Za-z0-9_]", RegexOptions.Multiline)] + private static partial Regex AnchorOrAlias(); + + /// + /// Returns false with a reason when the raw document violates a guard. + /// + public static bool Check(string yaml, string documentName, out string reason) + { + if (yaml.Length > MaxBytes) + { + reason = $"REJECTED: {documentName} is {yaml.Length:N0} characters; the limit is {MaxBytes:N0}. " + + "These are configuration documents — if something this large seems necessary, it belongs elsewhere."; + return false; + } + + var match = AnchorOrAlias().Match(yaml); + if (match.Success) + { + reason = $"REJECTED: {documentName} contains a YAML anchor or alias ('{match.Value.Trim()}…'). " + + "Anchors and aliases are not permitted — they enable exponential-expansion attacks and " + + "ashlar documents never need them. Write the value out literally."; + return false; + } + + reason = string.Empty; + return true; + } +} diff --git a/src/Ashlar.Tests.Kernel/AdmissionFuzzTests.cs b/src/Ashlar.Tests.Kernel/AdmissionFuzzTests.cs new file mode 100644 index 000000000..05587f409 --- /dev/null +++ b/src/Ashlar.Tests.Kernel/AdmissionFuzzTests.cs @@ -0,0 +1,193 @@ +using FluentAssertions; +using Ashlar.Manifest; +using Ashlar.Manifest.Admission; +using Xunit; + +namespace Ashlar.Tests.Kernel; + +/// +/// Fuzz-informed hardening tests (gold plan step 2): hostile YAML against the loaders, the +/// Windows path alphabet against proposal ids, and corruption against the store. The common +/// invariant: hostile input produces a REJECTION WITH A REASON — never a hang, an +/// exponential blowup, an unhandled exception, or (worst of all) a silently skipped record. +/// +public sealed class AdmissionFuzzTests : IDisposable +{ + private readonly string _dir; + + public AdmissionFuzzTests() + { + _dir = Path.Combine(Path.GetTempPath(), "fuzz-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_dir); + } + + public void Dispose() + { + if (Directory.Exists(_dir)) + { + Directory.Delete(_dir, recursive: true); + } + } + + private static readonly DateTimeOffset Now = new(2026, 8, 23, 12, 0, 0, TimeSpan.Zero); + + // ─────────────────────────── hostile YAML ─────────────────────────── + + [Fact] + public void Alias_bomb_is_rejected_before_it_can_expand() + { + // The classic billion-laughs shape: tiny document, exponential expansion if the + // parser follows the aliases. The guard rejects anchors outright — these documents + // never need them. + var bomb = """ + apiVersion: ashlar/v1 + kind: Policy + a: &a ["x","x","x","x","x","x","x","x"] + b: &b [*a,*a,*a,*a,*a,*a,*a,*a] + c: &c [*b,*b,*b,*b,*b,*b,*b,*b] + d: &d [*c,*c,*c,*c,*c,*c,*c,*c] + e: &e [*d,*d,*d,*d,*d,*d,*d,*d] + f: &f [*e,*e,*e,*e,*e,*e,*e,*e] + """; + + var sw = System.Diagnostics.Stopwatch.StartNew(); + PolicyLoader.TryLoad(bomb, out _, out var reason).Should().BeFalse(); + sw.Stop(); + + reason.Should().Contain("anchor"); + sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(2), + "the rejection must happen before expansion, not after surviving it"); + } + + [Fact] + public void Oversized_documents_are_rejected_by_size_not_parsed() + { + var giant = "apiVersion: ashlar/v1\nkind: Application\n# " + new string('x', YamlGuard.MaxBytes + 10); + + ManifestLoader.TryLoad(giant, out _, out var reason).Should().BeFalse(); + reason.Should().Contain("limit"); + } + + [Theory] + [InlineData("\0\x01\x02 binary garbage \xff")] + [InlineData("{{{{{{{{")] + [InlineData("apiVersion: [this, is, not, a, string]")] + [InlineData("kind: {nested: {absurdly: {deep: value}}}")] + [InlineData("apiVersion: ashlar/v1\nkind: Policy\nsandbox: \"not a map\"")] + [InlineData("apiVersion: ashlar/v1\nkind: Policy\nselfExtend:\n budget: \"words\"")] + public void Garbage_and_type_confusion_reject_with_reasons_never_escape_exceptions(string yaml) + { + // Both loaders, same invariant: TryLoad returns false with a REJECTED reason. + // An unhandled exception here becomes a CLI crash on hostile input. + var actPolicy = () => PolicyLoader.TryLoad(yaml, out _, out _); + var actManifest = () => ManifestLoader.TryLoad(yaml, out _, out _); + + actPolicy.Should().NotThrow().Which.Should().BeFalse(); + actManifest.Should().NotThrow().Which.Should().BeFalse(); + } + + [Fact] + public void An_honest_ampersand_in_prose_is_not_an_anchor() + { + // The guard is textual and deliberately eager, but plain prose must survive: + // '&' followed by whitespace is not YAML anchor syntax. + ProjectScaffold.TryScaffold("fish-and-chips", out var manifest, out _, out _).Should().BeTrue(); + var yaml = manifest.Replace( + "version: 0.1.0", + "version: 0.1.0\n # serves fish & chips daily"); + + ManifestLoader.TryLoad(yaml, out _, out var reason).Should().BeTrue(reason); + } + + // ─────────────────────── the Windows path alphabet ─────────────────────── + + public static TheoryData HostileIds => new() + { + "CON", "NUL", "PRN", "AUX", "COM1", // Win32 reserved names + "ext ", "ext.", // trailing space/dot: Win32 strips -> collisions + "a:b", "ext::$DATA", // NTFS alternate data streams + "..", "../x", "..\\x", "/etc/passwd", // traversal + "", " ", "-lead", "_lead", // empty / bad leading char + "éxt", "ｅxt", "ext​1", // non-ASCII and zero-width confusables + "id\twith\ttabs", "id\nnewline", + }; + + [Theory] + [MemberData(nameof(HostileIds))] + public async Task Hostile_proposal_ids_are_refused_by_the_allowlist(string id) + { + var store = new GateStore(_dir); + var proposal = new ExtensionProposal + { + Id = id, + Kind = "brick", + Summary = "hostile", + ProposedBy = "fuzzer", + ProposedAt = Now, + Courses = [new CourseResult { Name = "tests", Passed = true, Detail = "ok" }], + }; + + var act = () => store.RecordAsync(proposal, + new AdmissionOutcome { State = ProposalState.Held, Reason = "x" }, Now); + + await act.Should().ThrowAsync().WithMessage("*Illegal proposal id*"); + } + + [Theory] + [InlineData("a")] + [InlineData("ext-4f2a")] + [InlineData("A_1-b_2")] + [InlineData("0start")] + public async Task Legitimate_ids_still_pass(string id) + { + var store = new GateStore(_dir); + var proposal = new ExtensionProposal + { + Id = id, Kind = "brick", Summary = "ok", ProposedBy = "t", ProposedAt = Now, + Courses = [new CourseResult { Name = "tests", Passed = true, Detail = "ok" }], + }; + + var record = await store.RecordAsync(proposal, + new AdmissionOutcome { State = ProposalState.Held, Reason = "x" }, Now); + + record.Proposal.Id.Should().Be(id); + } + + // ─────────────────────────── store corruption ─────────────────────────── + + [Fact] + public async Task A_truncated_record_fails_the_listing_loudly_never_silently_skips() + { + // The worst failure shape for an admission store: a corrupt HELD record silently + // vanishing from the queue is an invisible pending decision. + var store = new GateStore(_dir); + await store.RecordAsync( + new ExtensionProposal + { + Id = "ext-c", Kind = "brick", Summary = "s", ProposedBy = "p", ProposedAt = Now, + Courses = [new CourseResult { Name = "tests", Passed = true, Detail = "ok" }], + }, + new AdmissionOutcome { State = ProposalState.Held, Reason = "held" }, Now); + + var file = Directory.GetFiles(Path.Combine(_dir, "gates"), "ext-c.json").Single(); + File.WriteAllText(file, File.ReadAllText(file)[..40]); // truncate mid-object + + var act = () => new GateStore(_dir).ListAsync(ProposalState.Held); + + await act.Should().ThrowAsync() + .WithMessage("*Corrupt gate record*ext-c*"); + } + + [Fact] + public async Task A_record_containing_null_fails_loudly_too() + { + var gatesDir = Path.Combine(_dir, "gates"); + Directory.CreateDirectory(gatesDir); + File.WriteAllText(Path.Combine(gatesDir, "ext-null.json"), "null"); + + var act = () => new GateStore(_dir).GetAsync("ext-null"); + + await act.Should().ThrowAsync() + .WithMessage("*contains no record*"); + } +}