From cd07ef703d2e3f096e1ef6515a37e0e3e949c5bc Mon Sep 17 00:00:00 2001 From: Arkadiusz Biel - bielu <2244074+bielu@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:22:51 +0100 Subject: [PATCH] fix: report invalid SQS/SNS policy values as diagnostics StringOrStringList, Principal and Condition threw ArgumentException from their Parse methods. The reader only catches AsyncApiException, so a malformed policy statement (e.g. `action` given as a map) escaped ReadFragment/Read instead of becoming a diagnostic. An empty principal map also threw InvalidOperationException from First(). Parse now throws AsyncApiReaderException with the parsing context, and an empty principal map is rejected through the same path. Co-Authored-By: Claude Opus 5.5 --- .../Sns/Condition.cs | 13 ++-- .../Sns/Principal.cs | 23 ++++--- .../Sqs/Condition.cs | 13 ++-- .../Sqs/Principal.cs | 23 ++++--- .../StringOrStringList.cs | 7 +- .../Bindings/AwsPolicyParsing_Should.cs | 68 +++++++++++++++++++ 6 files changed, 121 insertions(+), 26 deletions(-) create mode 100644 test/ByteBard.AsyncAPI.Tests/Bindings/AwsPolicyParsing_Should.cs diff --git a/src/ByteBard.AsyncAPI.Bindings/Sns/Condition.cs b/src/ByteBard.AsyncAPI.Bindings/Sns/Condition.cs index 7a82aa4..c4dd759 100644 --- a/src/ByteBard.AsyncAPI.Bindings/Sns/Condition.cs +++ b/src/ByteBard.AsyncAPI.Bindings/Sns/Condition.cs @@ -4,6 +4,7 @@ namespace ByteBard.AsyncAPI.Bindings.Sns; using System.Collections.Generic; using System.Linq; using ByteBard.AsyncAPI.Models.Interfaces; +using ByteBard.AsyncAPI.Readers.Exceptions; using ByteBard.AsyncAPI.Readers.ParseNodes; using ByteBard.AsyncAPI.Writers; @@ -49,8 +50,10 @@ public static Condition Parse(ParseNode node) .ToDictionary(x => x.Key, x => x.Value))); break; default: - throw new ArgumentException($"An error occured while parsing a {nameof(Condition)} node. " + - $"AWS condition values should be one or more key value pairs."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Condition)} node. " + + $"AWS condition values should be one or more key value pairs.", + node.Context); } } @@ -58,8 +61,10 @@ public static Condition Parse(ParseNode node) } default: - throw new ArgumentException($"An error occured while parsing a {nameof(Condition)} node. " + - $"Node should contain a collection of condition types."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Condition)} node. " + + $"Node should contain a collection of condition types.", + node.Context); } } } \ No newline at end of file diff --git a/src/ByteBard.AsyncAPI.Bindings/Sns/Principal.cs b/src/ByteBard.AsyncAPI.Bindings/Sns/Principal.cs index aff3fd2..39a1bea 100644 --- a/src/ByteBard.AsyncAPI.Bindings/Sns/Principal.cs +++ b/src/ByteBard.AsyncAPI.Bindings/Sns/Principal.cs @@ -6,6 +6,7 @@ namespace ByteBard.AsyncAPI.Bindings.Sns; using System.Text.Json; using System.Text.Json.Nodes; using ByteBard.AsyncAPI.Models.Interfaces; +using ByteBard.AsyncAPI.Readers.Exceptions; using ByteBard.AsyncAPI.Readers.ParseNodes; using ByteBard.AsyncAPI.Writers; @@ -21,19 +22,23 @@ public static Principal Parse(ParseNode node) var nodeValue = node.GetScalarValue(); if (!IsStarString(nodeValue)) { - throw new ArgumentException($"An error occured while parsing a {nameof(Principal)} node. " + - $"Principal value without a property name can only be a string value of '*'."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Principal)} node. " + + $"Principal value without a property name can only be a string value of '*'.", + node.Context); } return new PrincipalStar(); case MapNode mapNode: { - var propertyNode = mapNode.First(); - if (!IsValidPrincipalProperty(propertyNode.Name)) + var propertyNode = mapNode.FirstOrDefault(); + if (propertyNode == null || !IsValidPrincipalProperty(propertyNode.Name)) { - throw new ArgumentException($"An error occured while parsing a {nameof(Principal)} node. " + - $"Node should contain a valid AWS principal property name."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Principal)} node. " + + $"Node should contain a valid AWS principal property name.", + node.Context); } var principalValue = new KeyValuePair( @@ -44,8 +49,10 @@ public static Principal Parse(ParseNode node) } default: - throw new ArgumentException($"An error occured while parsing a {nameof(Principal)} node. " + - $"Node should contain a string value of '*' or a valid AWS principal property."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Principal)} node. " + + $"Node should contain a string value of '*' or a valid AWS principal property.", + node.Context); } } diff --git a/src/ByteBard.AsyncAPI.Bindings/Sqs/Condition.cs b/src/ByteBard.AsyncAPI.Bindings/Sqs/Condition.cs index 2a70d63..4fdbf32 100644 --- a/src/ByteBard.AsyncAPI.Bindings/Sqs/Condition.cs +++ b/src/ByteBard.AsyncAPI.Bindings/Sqs/Condition.cs @@ -4,6 +4,7 @@ namespace ByteBard.AsyncAPI.Bindings.Sqs; using System.Collections.Generic; using System.Linq; using ByteBard.AsyncAPI.Models.Interfaces; +using ByteBard.AsyncAPI.Readers.Exceptions; using ByteBard.AsyncAPI.Readers.ParseNodes; using ByteBard.AsyncAPI.Writers; @@ -49,8 +50,10 @@ public static Condition Parse(ParseNode node) .ToDictionary(x => x.Key, x => x.Value))); break; default: - throw new ArgumentException($"An error occured while parsing a {nameof(Condition)} node. " + - $"AWS condition values should be one or more key value pairs."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Condition)} node. " + + $"AWS condition values should be one or more key value pairs.", + node.Context); } } @@ -58,8 +61,10 @@ public static Condition Parse(ParseNode node) } default: - throw new ArgumentException($"An error occured while parsing a {nameof(Condition)} node. " + - $"Node should contain a collection of AWS condition types."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Condition)} node. " + + $"Node should contain a collection of AWS condition types.", + node.Context); } } } \ No newline at end of file diff --git a/src/ByteBard.AsyncAPI.Bindings/Sqs/Principal.cs b/src/ByteBard.AsyncAPI.Bindings/Sqs/Principal.cs index 9c0832f..65f0834 100644 --- a/src/ByteBard.AsyncAPI.Bindings/Sqs/Principal.cs +++ b/src/ByteBard.AsyncAPI.Bindings/Sqs/Principal.cs @@ -6,6 +6,7 @@ namespace ByteBard.AsyncAPI.Bindings.Sqs; using System.Text.Json; using System.Text.Json.Nodes; using ByteBard.AsyncAPI.Models.Interfaces; +using ByteBard.AsyncAPI.Readers.Exceptions; using ByteBard.AsyncAPI.Readers.ParseNodes; using ByteBard.AsyncAPI.Writers; @@ -21,19 +22,23 @@ public static Principal Parse(ParseNode node) var nodeValue = node.GetScalarValue(); if (!IsStarString(nodeValue)) { - throw new ArgumentException($"An error occured while parsing a {nameof(Principal)} node. " + - $"Principal value without a property name can only be a string value of '*'."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Principal)} node. " + + $"Principal value without a property name can only be a string value of '*'.", + node.Context); } return new PrincipalStar(); case MapNode mapNode: { - var propertyNode = mapNode.First(); - if (!IsValidPrincipalProperty(propertyNode.Name)) + var propertyNode = mapNode.FirstOrDefault(); + if (propertyNode == null || !IsValidPrincipalProperty(propertyNode.Name)) { - throw new ArgumentException($"An error occured while parsing a {nameof(Principal)} node. " + - $"Node should contain a valid AWS principal property name."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Principal)} node. " + + $"Node should contain a valid AWS principal property name.", + node.Context); } var principalValue = new KeyValuePair( @@ -44,8 +49,10 @@ public static Principal Parse(ParseNode node) } default: - throw new ArgumentException($"An error occured while parsing a {nameof(Principal)} node. " + - $"Node should contain a string value of '*' or a valid AWS principal property."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(Principal)} node. " + + $"Node should contain a string value of '*' or a valid AWS principal property.", + node.Context); } } diff --git a/src/ByteBard.AsyncAPI.Bindings/StringOrStringList.cs b/src/ByteBard.AsyncAPI.Bindings/StringOrStringList.cs index 8bf5d1b..8b48642 100644 --- a/src/ByteBard.AsyncAPI.Bindings/StringOrStringList.cs +++ b/src/ByteBard.AsyncAPI.Bindings/StringOrStringList.cs @@ -6,6 +6,7 @@ namespace ByteBard.AsyncAPI.Bindings using System.Text.Json.Nodes; using ByteBard.AsyncAPI.Models; using ByteBard.AsyncAPI.Models.Interfaces; + using ByteBard.AsyncAPI.Readers.Exceptions; using ByteBard.AsyncAPI.Readers.ParseNodes; public class StringOrStringList : IAsyncApiElement @@ -40,8 +41,10 @@ public static StringOrStringList Parse(ParseNode node) } default: - throw new ArgumentException($"An error occured while parsing a {nameof(StringOrStringList)} node. " + - $"Node should contain a string value or a list of strings."); + throw new AsyncApiReaderException( + $"An error occured while parsing a {nameof(StringOrStringList)} node. " + + $"Node should contain a string value or a list of strings.", + node.Context); } } diff --git a/test/ByteBard.AsyncAPI.Tests/Bindings/AwsPolicyParsing_Should.cs b/test/ByteBard.AsyncAPI.Tests/Bindings/AwsPolicyParsing_Should.cs new file mode 100644 index 0000000..22713c7 --- /dev/null +++ b/test/ByteBard.AsyncAPI.Tests/Bindings/AwsPolicyParsing_Should.cs @@ -0,0 +1,68 @@ +namespace ByteBard.AsyncAPI.Tests.Bindings +{ + using FluentAssertions; + using ByteBard.AsyncAPI.Models; + using ByteBard.AsyncAPI.Readers; + using NUnit.Framework; + using BindingsCollection = ByteBard.AsyncAPI.Bindings.BindingsCollection; + + internal class AwsPolicyParsing_Should : TestBase + { + [TestCase("action:\n foo: bar", "StringOrStringList")] + [TestCase("principal: someone", "Principal")] + [TestCase("principal:\n Unknown: arn", "Principal")] + [TestCase("principal: {}", "Principal")] + [TestCase("principal:\n - '*'", "Principal")] + [TestCase("condition: invalid", "Condition")] + [TestCase("condition:\n StringEquals: invalid", "Condition")] + public void SqsChannelBinding_WithInvalidPolicyStatement_ReportsDiagnosticInsteadOfThrowing(string statementField, string expectedType) + { + var yaml = + "bindings:\n" + + " sqs:\n" + + " queue:\n" + + " name: myQueue\n" + + " policy:\n" + + " statements:\n" + + " - effect: Allow\n" + + Indent(statementField, 12); + + var settings = new AsyncApiReaderSettings { Bindings = BindingsCollection.Sqs }; + + AsyncApiDiagnostic diagnostic = null; + var act = () => new AsyncApiStringReader(settings).ReadFragment(yaml, AsyncApiVersion.AsyncApi2_0, out diagnostic); + + act.Should().NotThrow(); + diagnostic.Errors.Should().ContainSingle().Which.Message.Should().Contain(expectedType); + } + + [TestCase("action:\n foo: bar", "StringOrStringList")] + [TestCase("principal: someone", "Principal")] + [TestCase("condition: invalid", "Condition")] + public void SnsChannelBinding_WithInvalidPolicyStatement_ReportsDiagnosticInsteadOfThrowing(string statementField, string expectedType) + { + var yaml = + "bindings:\n" + + " sns:\n" + + " name: myTopic\n" + + " policy:\n" + + " statements:\n" + + " - effect: Allow\n" + + Indent(statementField, 10); + + var settings = new AsyncApiReaderSettings { Bindings = BindingsCollection.Sns }; + + AsyncApiDiagnostic diagnostic = null; + var act = () => new AsyncApiStringReader(settings).ReadFragment(yaml, AsyncApiVersion.AsyncApi2_0, out diagnostic); + + act.Should().NotThrow(); + diagnostic.Errors.Should().ContainSingle().Which.Message.Should().Contain(expectedType); + } + + private static string Indent(string value, int spaces) + { + var prefix = new string(' ', spaces); + return prefix + value.Replace("\n", "\n" + prefix) + "\n"; + } + } +}