fix: add default lmitation on vulnerable template options

docs: improve comments
chore: address warnings/info

Author: JeremyMelton

src/Transmitly.TemplateEngine.Scriban/ScribanOptions.cs

@@ -15,16 +15,63 @@
 using SB = Scriban;
 namespace Transmitly.TemplateEngine.Scriban
 {
+	/// <summary>
+	/// Configures how Scriban templates are parsed and rendered by the Transmitly template engine.
+	/// </summary>
 	public sealed class ScribanOptions
 	{
+		public const int DefaultExpressionDepthLimit = 250;
+		public const int DefaultObjectRecursionLimit = 20;
+		public const int DefaultLimitToString = 1024 * 1024;
+
 		/// <summary>
-		/// Whether to force the use of the Liquid template parser. (Default=false)
+		/// Gets or sets a value indicating whether templates should be parsed using Scriban's Liquid-compatible parser.
 		/// </summary>
 		public bool UseLiquidTemplates { get; set; }
+
+		/// <summary>
+		/// Gets or sets a value indicating whether parse errors should throw a <see cref="ScribanTemplateEngineException"/> instead of returning <see langword="null"/>.
+		/// </summary>
 		public bool ThrowIfTemplateError { get; set; } = true;
+
+		/// <summary>
+		/// Gets or sets the maximum nested expression depth allowed during parsing.
+		/// Set to <see langword="null"/> to disable the limit.
+		/// </summary>
+		public int? ExpressionDepthLimit { get; set; } = DefaultExpressionDepthLimit;
+
+		/// <summary>
+		/// Gets or sets the maximum recursion depth allowed when Scriban converts rendered objects to strings.
+		/// Set to <c>0</c> to disable the limit.
+		/// </summary>
+		public int ObjectRecursionLimit { get; set; } = DefaultObjectRecursionLimit;
+
+		/// <summary>
+		/// Gets or sets the maximum number of characters produced while Scriban converts rendered objects to strings.
+		/// Set to <c>0</c> to disable the limit.
+		/// </summary>
+		public int LimitToString { get; set; } = DefaultLimitToString;
+
+		/// <summary>
+		/// Gets or sets the lexer options used when tokenizing template content.
+		/// When <see langword="null"/>, Scriban uses its default lexer behavior.
+		/// </summary>
 		public SB.Parsing.LexerOptions? LexerOptions { get; set; }
-		public SB.Parsing.ParserOptions ParserOptions { get; set; }
+
+		/// <summary>
+		/// Gets or sets the base parser options used when parsing templates.
+		/// The engine still applies <see cref="ExpressionDepthLimit"/> when creating the effective parser options.
+		/// </summary>
+		public SB.Parsing.ParserOptions ParserOptions { get; set; } = new();
+
+		/// <summary>
+		/// Gets or sets the delegate used to rename imported .NET members before they are exposed to templates.
+		/// </summary>
 		public SB.Runtime.MemberRenamerDelegate? MemberRenamer { get; set; }
+
+		/// <summary>
+		/// Gets or sets the delegate used to filter which imported .NET members are exposed to templates.
+		/// </summary>
 		public SB.Runtime.MemberFilterDelegate? MemberFilterDelegate { get; set; }
 	}
 }

src/Transmitly.TemplateEngine.Scriban/ScribanTemplateEngine.cs

@@ -14,6 +14,7 @@
 
 using System;
 using System.Threading.Tasks;
+using Scriban.Runtime;
 using Transmitly.Template.Configuration;
 using Transmitly.Util;
 using SB = Scriban;
@@ -36,22 +37,55 @@ internal sealed class ScribanTemplateEngine(ScribanOptions options) : ITemplateE
 			if (template.HasErrors)
 			{
 				var messages = string.Join(Environment.NewLine, template.Messages);
-				if (options.ThrowIfTemplateError)
+				if (_options.ThrowIfTemplateError)
 				{
 					throw new ScribanTemplateEngineException($"Provided template has errors. Pipeline: '{context.PipelineIntent}'.{Environment.NewLine}{messages}");
 				}
 				System.Diagnostics.Debug.WriteLine($"{nameof(ScribanTemplateEngine)} {string.Join(";", messages)}");
 				return null;
 			}
-			var result = await template.RenderAsync(model, _options.MemberRenamer, _options.MemberFilterDelegate);
-			return result?.ToString();
+
+			var renderContext = CreateTemplateContext(model);
+			try
+			{
+				return await template.RenderAsync(renderContext);
+			}
+			finally
+			{
+				renderContext.PopGlobal();
+			}
+		}
+
+		internal SB.Parsing.ParserOptions CreateParserOptions()
+		{
+			var parserOptions = _options.ParserOptions;
+			parserOptions.ExpressionDepthLimit = _options.ExpressionDepthLimit;
+			return parserOptions;
+		}
+
+		internal SB.TemplateContext CreateTemplateContext(object? model)
+		{
+			var scriptObject = new ScriptObject();
+			if (model != null)
+			{
+				scriptObject.Import(model, renamer: _options.MemberRenamer, filter: _options.MemberFilterDelegate);
+			}
+
+			var renderContext = _options.UseLiquidTemplates ? new SB.LiquidTemplateContext() : new SB.TemplateContext();
+			renderContext.MemberRenamer = _options.MemberRenamer;
+			renderContext.MemberFilter = _options.MemberFilterDelegate;
+			renderContext.ObjectRecursionLimit = _options.ObjectRecursionLimit;
+			renderContext.LimitToString = _options.LimitToString;
+			renderContext.PushGlobal(scriptObject);
+			return renderContext;
 		}
 
 		private SB.Template Parse(string? content)
 		{
+			var parserOptions = CreateParserOptions();
 			if (_options.UseLiquidTemplates)
-				return SB.Template.ParseLiquid(content, parserOptions: _options.ParserOptions, lexerOptions: _options.LexerOptions);
-			return SB.Template.Parse(content, parserOptions: _options.ParserOptions, lexerOptions: _options.LexerOptions);
+				return SB.Template.ParseLiquid(content, parserOptions: parserOptions, lexerOptions: _options.LexerOptions);
+			return SB.Template.Parse(content, parserOptions: parserOptions, lexerOptions: _options.LexerOptions);
 		}
 
 	}

src/Transmitly.TemplateEngine.Scriban/ScribanTemplateEngineException.cs

@@ -13,21 +13,40 @@
 //  limitations under the License.
 
 using System;
+#if NETFRAMEWORK
 using System.Runtime.Serialization;
+#endif
 
 namespace Transmitly.TemplateEngine.Scriban
 {
+#if NETFRAMEWORK
 	[Serializable]
+	/// <summary>
+	/// Represents an error raised by the Scriban template engine while validating or rendering template content.
+	/// </summary>
 	public sealed class ScribanTemplateEngineException : Exception
 	{
 		public ScribanTemplateEngineException(string message) : base(message)
 		{
 
 		}
 
-		protected ScribanTemplateEngineException(SerializationInfo info, StreamingContext context) : base(info, context)
+		private ScribanTemplateEngineException(SerializationInfo info, StreamingContext context) : base(info, context)
 		{
 
 		}
+
+		public override void GetObjectData(SerializationInfo info, StreamingContext context)
+		{
+			base.GetObjectData(info, context);
+		}
+	}
+#else
+	/// <summary>
+	/// Represents an error raised by the Scriban template engine while validating or rendering template content.
+	/// </summary>
+	public sealed class ScribanTemplateEngineException(string message) : Exception(message)
+	{
 	}
+#endif
 }

src/Transmitly.TemplateEngine.Scriban/ScribanTemplateEngineExtensions.cs

@@ -19,18 +19,33 @@
 
 namespace Transmitly
 {
+	/// <summary>
+	/// Extensions for registering and referencing the Scriban template engine within a Transmitly configuration.
+	/// </summary>
 	public static class ScribanTemplateEngineExtensions
 	{
 		private const string ScribanId = "Scriban";
 
-
+		/// <summary>
+		/// Gets the identifier used to reference the Scriban template engine configurations.
+		/// </summary>
+		/// <param name="templateEngines">The template engine identifier source.</param>
+		/// <param name="providerId">An optional provider-specific suffix used to create a distinct engine identifier.</param>
+		/// <returns>The resolved template engine identifier.</returns>
 		public static string Scriban(this TemplateEngines templateEngines, string? providerId = null)
 		{
 			Guard.AgainstNull(templateEngines);
 
 			return templateEngines.GetId(ScribanId, providerId);
 		}
 
+		/// <summary>
+		/// Registers the Scriban template engine using the supplied configuration callback.
+		/// </summary>
+		/// <param name="templateConfiguration">The template configuration builder that will receive the engine registration.</param>
+		/// <param name="options">A callback that configures <see cref="ScribanOptions"/> before registration.</param>
+		/// <param name="templateEngineId">An optional explicit template engine identifier.</param>
+		/// <returns>The parent communications client builder.</returns>
 		public static CommunicationsClientBuilder AddScribanTemplateEngine(this TemplateConfigurationBuilder templateConfiguration, Action<ScribanOptions> options, string? templateEngineId = null)
 		{
 			Guard.AgainstNull(templateConfiguration);
@@ -41,16 +56,33 @@ public static CommunicationsClientBuilder AddScribanTemplateEngine(this Template
 			return templateConfiguration.Add(new ScribanTemplateEngine(opts), Id.TemplateEngines.Scriban(templateEngineId));
 		}
 
+		/// <summary>
+		/// Registers the Scriban template engine using the default <see cref="ScribanOptions"/>.
+		/// </summary>
+		/// <param name="templateConfiguration">The template configuration builder that will receive the engine registration.</param>
+		/// <param name="templateEngineId">An optional explicit template engine identifier.</param>
+		/// <returns>The parent communications client builder.</returns>
 		public static CommunicationsClientBuilder AddScribanTemplateEngine(this TemplateConfigurationBuilder templateConfiguration, string? templateEngineId = null)
 		{
 			return AddScribanTemplateEngine(templateConfiguration, (opts) => { }, templateEngineId);
 		}
 
+		/// <summary>
+		/// Registers the Scriban template engine on a communications client using the default <see cref="ScribanOptions"/>.
+		/// </summary>
+		/// <param name="communicationsClientBuilder">The communications client builder to configure.</param>
+		/// <returns>The configured communications client builder.</returns>
 		public static CommunicationsClientBuilder AddScribanTemplateEngine(this CommunicationsClientBuilder communicationsClientBuilder)
 		{
 			return AddScribanTemplateEngine(communicationsClientBuilder.TemplateEngine, (opts) => { });
 		}
 
+		/// <summary>
+		/// Registers the Scriban template engine on a communications client using the supplied configuration callback.
+		/// </summary>
+		/// <param name="communicationsClientBuilder">The communications client builder to configure.</param>
+		/// <param name="options">A callback that configures <see cref="ScribanOptions"/> before registration.</param>
+		/// <returns>The configured communications client builder.</returns>
 		public static CommunicationsClientBuilder AddScribanTemplateEngine(this CommunicationsClientBuilder communicationsClientBuilder, Action<ScribanOptions> options)
 		{
 			return AddScribanTemplateEngine(communicationsClientBuilder.TemplateEngine, options, Id.TemplateEngines.Scriban());

tests/Transmitly.TemplateEngine.Scriban.Tests/TemplateEngineTests.cs

@@ -78,7 +78,7 @@ public async Task ShouldThrowWithInvalidTemplateByDefault()
 			context.Setup(s => s.ContentModel).Returns(model.Object);
 			var engine = new Scriban.ScribanTemplateEngine(new ScribanOptions { });
 
-			await Assert.ThrowsExceptionAsync<ScribanTemplateEngineException>(() => engine.RenderAsync(template.Object, context.Object));
+			await Assert.ThrowsExactlyAsync<ScribanTemplateEngineException>(() => engine.RenderAsync(template.Object, context.Object));
 		}
 
 		[TestMethod]
@@ -100,5 +100,77 @@ public async Task ShouldReturnNullWithInvalidtemplate()
 
 			Assert.IsNull(result);
 		}
+
+		[TestMethod]
+		public void ShouldApplySecureDefaultParserLimit()
+		{
+			var engine = new Scriban.ScribanTemplateEngine(new ScribanOptions());
+
+			var parserOptions = engine.CreateParserOptions();
+
+			Assert.AreEqual(ScribanOptions.DefaultExpressionDepthLimit, parserOptions.ExpressionDepthLimit);
+		}
+
+		[TestMethod]
+		public void ShouldApplySecureDefaultRenderLimits()
+		{
+			var engine = new Scriban.ScribanTemplateEngine(new ScribanOptions());
+			var renderContext = engine.CreateTemplateContext(model: null);
+
+			try
+			{
+				Assert.AreEqual(ScribanOptions.DefaultObjectRecursionLimit, renderContext.ObjectRecursionLimit);
+				Assert.AreEqual(ScribanOptions.DefaultLimitToString, renderContext.LimitToString);
+			}
+			finally
+			{
+				renderContext.PopGlobal();
+			}
+		}
+
+		[TestMethod]
+		public async Task ShouldThrowWhenExpressionDepthLimitExceeded()
+		{
+			var nestedExpression = new string('(', ScribanOptions.DefaultExpressionDepthLimit + 1) + "1" + new string(')', ScribanOptions.DefaultExpressionDepthLimit + 1);
+			var templateContent = $"{{{{ {nestedExpression} }}}}";
+			var template = new Mock<IContentTemplateRegistration>();
+			template.Setup(s => s.GetContentAsync(It.IsAny<IDispatchCommunicationContext>())).Returns(Task.FromResult<string?>(templateContent));
+			var model = new Mock<IContentModel>();
+#pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type.
+			model.Setup(s => s.Model).Returns(null);
+#pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type.
+			var context = new Mock<IDispatchCommunicationContext>();
+			context.Setup(s => s.ContentModel).Returns(model.Object);
+			var engine = new Scriban.ScribanTemplateEngine(new ScribanOptions());
+
+			var exception = await Assert.ThrowsExactlyAsync<ScribanTemplateEngineException>(() => engine.RenderAsync(template.Object, context.Object));
+
+			StringAssert.Contains(exception.Message, "errors");
+		}
+
+		[TestMethod]
+		public async Task ShouldThrowWhenObjectRecursionLimitExceeded()
+		{
+			var recursiveObject = new global::Scriban.Runtime.ScriptObject();
+			recursiveObject["self"] = recursiveObject;
+			var model = new global::Scriban.Runtime.ScriptObject
+			{
+				["a"] = recursiveObject
+			};
+			var engine = new Scriban.ScribanTemplateEngine(new ScribanOptions());
+			var template = global::Scriban.Template.Parse("{{ a }}");
+			var renderContext = engine.CreateTemplateContext(model);
+
+			try
+			{
+				var exception = await Assert.ThrowsExactlyAsync<global::Scriban.Syntax.ScriptRuntimeException>(async () => await template.RenderAsync(renderContext));
+
+				StringAssert.Contains(exception.Message, "deeply nested");
+			}
+			finally
+			{
+				renderContext.PopGlobal();
+			}
+		}
 	}
-}
+}