From 80686d6ed1e6e2d2eebd2712648c2daeed3f01d5 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi <35780660+anakrish@users.noreply.github.com> Date: Tue, 20 Jan 2026 21:50:10 -0600 Subject: [PATCH] feat(ffi): unwind safety: shield FFI entrypoints with panic guard (#546) This PR implements widely accepted Rust programming practices for dealing with panics across ABI (programming language) boundaries. - Add panic_guard.rs to wrap FFI calls and prevent panic across FFI/ABI boundary (undefined behavior). - Capture per-thread backtraces via a temporary panic hook - After a panic, subsequent invocations are poisoned. - Integrate with_unwind_guard across the engine, schema registry, and target registry exportis Signed-off-by: Anand Krishnamoorthi --- .github/workflows/test-csharp.yml | 4 +- .../csharp/Regorus.Tests/PanicGuardTests.cs | 84 +++ .../csharp/Regorus.Tests/Regorus.Tests.csproj | 16 +- bindings/csharp/Regorus/AssemblyInfo.cs | 3 + bindings/csharp/Regorus/CompiledPolicy.cs | 3 +- bindings/csharp/Regorus/Compiler.cs | 2 +- bindings/csharp/Regorus/Engine.cs | 24 +- bindings/csharp/Regorus/NativeMethods.cs | 22 + bindings/csharp/Regorus/Regorus.csproj | 4 + bindings/csharp/Regorus/SchemaRegistry.cs | 6 +- bindings/csharp/Regorus/StatusExtensions.cs | 24 + bindings/csharp/Regorus/TargetRegistry.cs | 6 +- bindings/ffi/src/common.rs | 6 + bindings/ffi/src/compile.rs | 157 +++-- bindings/ffi/src/compiled_policy.rs | 45 +- bindings/ffi/src/effect_registry.rs | 157 ++--- bindings/ffi/src/engine.rs | 546 +++++++++++------- bindings/ffi/src/lib.rs | 1 + bindings/ffi/src/panic_guard.rs | 159 +++++ bindings/ffi/src/schema_registry.rs | 157 ++--- bindings/ffi/src/target_registry.rs | 83 +-- 21 files changed, 1008 insertions(+), 501 deletions(-) create mode 100644 bindings/csharp/Regorus.Tests/PanicGuardTests.cs create mode 100644 bindings/csharp/Regorus/AssemblyInfo.cs create mode 100644 bindings/csharp/Regorus/StatusExtensions.cs create mode 100644 bindings/ffi/src/panic_guard.rs diff --git a/.github/workflows/test-csharp.yml b/.github/workflows/test-csharp.yml index d0e2d43..f6b4c9b 100644 --- a/.github/workflows/test-csharp.yml +++ b/.github/workflows/test-csharp.yml @@ -144,11 +144,11 @@ jobs: path: ./bindings/csharp/regorus-nuget/ - name: Restore Regorus.Tests - run: dotnet restore /p:RestoreAdditionalProjectSources=../regorus-nuget + run: dotnet restore /p:RestoreAdditionalProjectSources=../regorus-nuget /p:UseLocalRegorus=false working-directory: ./bindings/csharp/Regorus.Tests - name: Run Regorus.Tests - run: dotnet test --no-restore + run: dotnet test --no-restore -p:UseLocalRegorus=false working-directory: ./bindings/csharp/Regorus.Tests - name: Restore TestApp diff --git a/bindings/csharp/Regorus.Tests/PanicGuardTests.cs b/bindings/csharp/Regorus.Tests/PanicGuardTests.cs new file mode 100644 index 0000000..3549afa --- /dev/null +++ b/bindings/csharp/Regorus.Tests/PanicGuardTests.cs @@ -0,0 +1,84 @@ +#if REGORUS_FFI_TEST_HOOKS +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Runtime.InteropServices; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Regorus.Internal; + +namespace Regorus.Tests; + +[TestClass] +public sealed class PanicGuardTests +{ + [TestInitialize] + public void Initialize() + { + API.regorus_engine_test_reset_poison(); + } + + [TestCleanup] + public void Cleanup() + { + API.regorus_engine_test_reset_poison(); + } + + [TestMethod] + public void Panic_produces_invalid_operation_exception() + { + var panic = Assert.ThrowsException(TriggerPanic); + StringAssert.Contains(panic.Message, "panicked", "panic message should capture payload"); + } + + [TestMethod] + public void Poison_flag_blocks_subsequent_calls() + { + _ = Assert.ThrowsException(TriggerPanic); + var poisoned = Assert.ThrowsException(TriggerPanic); + StringAssert.Contains(poisoned.Message, "poisoned", "poisoned message should explain guard state"); + } + + private static unsafe void TriggerPanic() + { + var result = API.regorus_engine_test_trigger_panic(); + try + { + if (result.status == RegorusStatus.Ok) + { + return; + } + + var message = PtrToStringUtf8((IntPtr)result.error_message); + throw result.status.CreateException(message); + } + finally + { + API.regorus_result_drop(result); + } + } + + private static string? PtrToStringUtf8(IntPtr ptr) + { +#if NETSTANDARD2_1 + return Marshal.PtrToStringUTF8(ptr); +#else + if (ptr == IntPtr.Zero) + { + return null; + } + + var len = 0; + while (Marshal.ReadByte(ptr, len) != 0) + { + len++; + } + + var buffer = new byte[len]; + Marshal.Copy(ptr, buffer, 0, buffer.Length); + return System.Text.Encoding.UTF8.GetString(buffer); +#endif + } +} + +#endif diff --git a/bindings/csharp/Regorus.Tests/Regorus.Tests.csproj b/bindings/csharp/Regorus.Tests/Regorus.Tests.csproj index 6130a90..a1add44 100644 --- a/bindings/csharp/Regorus.Tests/Regorus.Tests.csproj +++ b/bindings/csharp/Regorus.Tests/Regorus.Tests.csproj @@ -6,11 +6,17 @@ true true + true -$(VersionSuffix) + true + + + + $(DefineConstants);REGORUS_FFI_TEST_HOOKS @@ -21,7 +27,13 @@ - - + + + EnableRegorusTestHooks=true + + + + + \ No newline at end of file diff --git a/bindings/csharp/Regorus/AssemblyInfo.cs b/bindings/csharp/Regorus/AssemblyInfo.cs new file mode 100644 index 0000000..c6822d8 --- /dev/null +++ b/bindings/csharp/Regorus/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Regorus.Tests")] diff --git a/bindings/csharp/Regorus/CompiledPolicy.cs b/bindings/csharp/Regorus/CompiledPolicy.cs index 266afac..0673192 100644 --- a/bindings/csharp/Regorus/CompiledPolicy.cs +++ b/bindings/csharp/Regorus/CompiledPolicy.cs @@ -5,6 +5,7 @@ using System; using System.Text; using System.Text.Json; using System.Threading; +using Regorus.Internal; #nullable enable namespace Regorus @@ -165,7 +166,7 @@ namespace Regorus if (result.status != Internal.RegorusStatus.Ok) { var message = StringFromUTF8((IntPtr)result.error_message); - throw new Exception(message ?? "Unknown error occurred"); + throw result.status.CreateException(message); } return result.data_type switch diff --git a/bindings/csharp/Regorus/Compiler.cs b/bindings/csharp/Regorus/Compiler.cs index 3870bac..1c8c187 100644 --- a/bindings/csharp/Regorus/Compiler.cs +++ b/bindings/csharp/Regorus/Compiler.cs @@ -177,7 +177,7 @@ namespace Regorus if (result.status != Internal.RegorusStatus.Ok) { var message = StringFromUTF8((IntPtr)result.error_message); - throw new Exception(message ?? "Unknown compilation error occurred"); + throw result.status.CreateException(message); } if (result.data_type != Internal.RegorusDataType.Pointer || result.pointer_value == null) diff --git a/bindings/csharp/Regorus/Engine.cs b/bindings/csharp/Regorus/Engine.cs index 2d8ed11..7b84e0b 100644 --- a/bindings/csharp/Regorus/Engine.cs +++ b/bindings/csharp/Regorus/Engine.cs @@ -373,21 +373,21 @@ namespace Regorus string? CheckAndDropResult(Regorus.Internal.RegorusResult result) { - if (result.status != Regorus.Internal.RegorusStatus.Ok) + try { - var message = StringFromUTF8((IntPtr)result.error_message); - var ex = new Exception(message); - Regorus.Internal.API.regorus_result_drop(result); - throw ex; - } + if (result.status != Regorus.Internal.RegorusStatus.Ok) + { + var message = StringFromUTF8((IntPtr)result.error_message); + throw result.status.CreateException(message); + } - var resultString = ""; - if (result.output is not null) - { - resultString = StringFromUTF8((IntPtr)result.output); + var output = result.output is not null ? StringFromUTF8((IntPtr)result.output) : null; + return output ?? string.Empty; + } + finally + { + Regorus.Internal.API.regorus_result_drop(result); } - Regorus.Internal.API.regorus_result_drop(result); - return resultString; } private void ThrowIfDisposed() diff --git a/bindings/csharp/Regorus/NativeMethods.cs b/bindings/csharp/Regorus/NativeMethods.cs index a13bb77..f908740 100644 --- a/bindings/csharp/Regorus/NativeMethods.cs +++ b/bindings/csharp/Regorus/NativeMethods.cs @@ -217,6 +217,20 @@ namespace Regorus.Internal [DllImport(LibraryName, EntryPoint = "regorus_engine_compile_with_entrypoint", CallingConvention = CallingConvention.Cdecl, ExactSpelling = true)] internal static extern RegorusResult regorus_engine_compile_with_entrypoint(RegorusEngine* engine, byte* rule); + #if REGORUS_FFI_TEST_HOOKS + /// + /// Trigger a panic inside the engine for testing purposes. + /// + [DllImport(LibraryName, EntryPoint = "regorus_engine_test_trigger_panic", CallingConvention = CallingConvention.Cdecl, ExactSpelling = true)] + internal static extern RegorusResult regorus_engine_test_trigger_panic(); + + /// + /// Reset the engine poison flag for testing. + /// + [DllImport(LibraryName, EntryPoint = "regorus_engine_test_reset_poison", CallingConvention = CallingConvention.Cdecl, ExactSpelling = true)] + internal static extern void regorus_engine_test_reset_poison(); + #endif + #endregion #region Compilation Methods @@ -472,6 +486,14 @@ namespace Regorus.Internal /// Invalid policy content. /// InvalidPolicy, + /// + /// The engine panicked and cannot be reused until reset. + /// + Panic, + /// + /// The engine remains poisoned because a previous panic was detected. + /// + Poisoned, } /// diff --git a/bindings/csharp/Regorus/Regorus.csproj b/bindings/csharp/Regorus/Regorus.csproj index e496497..621acdc 100644 --- a/bindings/csharp/Regorus/Regorus.csproj +++ b/bindings/csharp/Regorus/Regorus.csproj @@ -17,6 +17,10 @@ + + $(DefineConstants);REGORUS_FFI_TEST_HOOKS + +