Skip to content

feat: Add RuleSync .NET SDK with Source Generators#1304

Closed
rudironsoni wants to merge 7 commits intodyoshikawa:mainfrom
rudironsoni:feat/rulesync-dotnet-sdk
Closed

feat: Add RuleSync .NET SDK with Source Generators#1304
rudironsoni wants to merge 7 commits intodyoshikawa:mainfrom
rudironsoni:feat/rulesync-dotnet-sdk

Conversation

@rudironsoni
Copy link
Contributor

@rudironsoni rudironsoni commented Mar 10, 2026

This PR introduces a comprehensive .NET SDK for RuleSync, enabling developers to generate AI tool configurations programmatically from .NET applications.

Features

  • Multi-targeting support: Builds for netstandard2.1, net6.0, and net8.0
  • Source Generators: Automatically generates strongly-typed C# bindings from TypeScript source files (ToolTarget, Feature enums, Options classes)
  • AOT Compatible: Fully compatible with Native AOT for .NET 8.0
  • Result Pattern: Functional error handling with Result type
  • Async/Await: Full async support with cancellation tokens

SDK Usage

Generate AI Tool Configurations

var client = new RulesyncClient();
var result = await client.GenerateAsync(new GenerateOptions
{
    Targets = [ToolTarget.ClaudeCode, ToolTarget.Cline],
    Features = [Feature.Rules, Feature.Commands],
    DryRun = true
});

if (result.IsSuccess)
{
    Console.WriteLine(result.Value.GeneratedFiles);
}

Import from Existing Tools

var result = await client.ImportAsync(new ImportOptions
{
    Target = ToolTarget.Cursor,
    Features = [Feature.Rules, Feature.Skills]
});

Security Fixes (Addressed from Review)

  • Command Injection: Fixed argument building to use List with ArgumentList instead of string concatenation/splitting
  • Input Validation: Added Enum.IsDefined validation for ImportOptions.Target
  • Race Condition: Fixed race condition in ProcessExtensions polyfill by reordering event subscription

Configuration Updates

  • Fixed repository URL to point to dyoshikawa/rulesync
  • Fixed Dependabot configuration for NuGet packages (/sdk/dotnet)
  • Fixed tag validation timing in publish.yml (validate before use)

Package Details

  • Package ID: RuleSync.Sdk.DotNet
  • Version: Synced with main project version
  • License: MIT

Checklist

  • Source generators for type-safe enums and options
  • Multi-target framework support
  • AOT compatibility enabled
  • Comprehensive unit tests (xUnit)
  • NuGet package configuration
  • GitHub Actions workflow for publishing
  • Dependabot configuration
  • Security review feedback addressed

@github-actions

This comment has been minimized.

@rudironsoni rudironsoni force-pushed the feat/rulesync-dotnet-sdk branch from 4e9529c to 93c28ec Compare March 10, 2026 17:44
@rudironsoni
Copy link
Contributor Author

Thank you for your contribution! Unfortunately, this PR has 2236 added lines, which exceeds the limit of 1000 lines for external contributors.

Please split your changes into smaller PRs. See CONTRIBUTING.md for details.

Actually, it's not feasible to do so.

@rudironsoni rudironsoni force-pushed the feat/rulesync-dotnet-sdk branch 2 times, most recently from 67c4083 to 93c28ec Compare March 10, 2026 18:02
@dyoshikawa-claw

This comment has been minimized.

@github-actions

This comment has been minimized.

@rudironsoni rudironsoni force-pushed the feat/rulesync-dotnet-sdk branch 3 times, most recently from 3e5964b to 3322b0a Compare March 10, 2026 18:25
@dyoshikawa-claw

This comment has been minimized.

@github-actions

This comment has been minimized.

@rudironsoni rudironsoni force-pushed the feat/rulesync-dotnet-sdk branch 2 times, most recently from 446e0d3 to 2294d31 Compare March 10, 2026 20:44
@dyoshikawa-claw

This comment has been minimized.

@rudironsoni rudironsoni force-pushed the feat/rulesync-dotnet-sdk branch from 2294d31 to d9b4cb8 Compare March 10, 2026 21:03
@github-actions

This comment has been minimized.

@rudironsoni rudironsoni force-pushed the feat/rulesync-dotnet-sdk branch from d9b4cb8 to 194988c Compare March 10, 2026 21:34
@rudironsoni
Copy link
Contributor Author

PR #91 Review: RuleSync .NET SDK with Source Generators

Mergeability Verdict: NOT MERGEABLE

The PR contains 3 critical and 6 high-severity issues that must be fixed before merging. These will cause runtime failures, security vulnerabilities, or build crashes.

Code Review Findings

🔴 Critical

  1. AOT Compatibility: Reflection-based JSON Serialization (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncJsonContext.cs:30-33, 44-47)

    • Uses JsonSerializer.Deserialize<T>() with reflection, which is NOT AOT-compatible
    • Will fail at runtime when used in Native AOT scenarios
    • Fix: Use JsonSerializerContext-derived class with [JsonSerializable] attributes
  2. Source Generator Throws Exception During Compilation (sdk/dotnet/src/RuleSync.Sdk.DotNet.SourceGenerators/RulesyncIncrementalGenerator.cs:355-359)

    • Throws InvalidOperationException during code generation
    • Will crash the compiler in a non-recoverable way
    • Fix: Use context.ReportDiagnostic() instead of throwing

🟠 High

  1. Missing JsonStringEnumConverter Registration (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncJsonContext.cs:15-19)

    • Enums serialize as integers (0, 1, 2) instead of strings ("rules", "ignore")
    • Will break API compatibility with rulesync CLI
    • Fix: Add Converters = { new JsonStringEnumConverter(JsonNamingPolicy.CamelCase) }
  2. Process Argument Building - Enum String Mismatch (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:181-189)

    • Enum values converted via ToString() without validation
    • If source generator produces values that don't match CLI expectations, commands fail
    • Fix: Add [Description("cli-value")] attributes or mapping method
  3. Incomplete Enum Validation (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:164-175)

    • Enum.IsDefined() may not catch default(ToolTarget) (value 0) scenarios
    • Fix: Add explicit check for uninitialized enums
  4. Race Condition in Output Reading (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:305-323)

    • stdoutLength check and increment are not atomic
    • Multiple threads could exceed MaxOutputSize limit
    • Fix: Use Interlocked.Add or lock

🟡 Medium

  1. Missing XML Documentation on Generated Types (sdk/dotnet/src/RuleSync.Sdk.DotNet.SourceGenerators/RulesyncIncrementalGenerator.cs:286-310)
  2. Regex Patterns May Miss TypeScript Edge Cases (sdk/dotnet/src/RuleSync.Sdk.DotNet.SourceGenerators/RulesyncIncrementalGenerator.cs:20-36)
  3. No Integration Tests (sdk/dotnet/tests/RuleSync.Sdk.DotNet.Tests/ClientTests.cs)
  4. Hardcoded Version in Source Generator Project (sdk/dotnet/src/RuleSync.Sdk.DotNet.SourceGenerators/RuleSync.Sdk.DotNet.SourceGenerators.csproj:17)
  5. Missing ConfigureAwait(false) Consistency (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs)

🔵 Low

  1. EditorConfig: Inconsistent Naming Convention (sdk/dotnet/.editorconfig:82-86)
  2. Missing Package Icon and Readme (sdk/dotnet/src/RuleSync.Sdk.DotNet/RuleSync.Sdk.DotNet.csproj)
  3. Test Project Only Targets net8.0 (sdk/dotnet/tests/RuleSync.Sdk.DotNet.Tests/RuleSync.Sdk.DotNet.Tests.csproj:4)
  4. Unused sourcePath Parameter (sdk/dotnet/src/RuleSync.Sdk.DotNet.SourceGenerators/RulesyncIncrementalGenerator.cs:75)
  5. README Security Concern (sdk/dotnet/README.md:23-38)

Security Review Findings

🔴 Critical

  1. GitHub Actions: Script Injection via REPO_OWNER (.github/workflows/publish.yml:83-90)
  • REPO_OWNER is interpolated directly into URL without validation
  • Could lead to credential exfiltration or arbitrary command execution
  • Fix: Add validation: if ! echo "$REPO_OWNER" | grep -qE '^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?$'

🟠 High

  1. Untrusted nodeExecutablePath and rulesyncPath (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:38-46, 273-297)
  • SDK accepts arbitrary paths without validation
  • Could lead to arbitrary code execution if library consumers pass untrusted input
  • Fix: Add path validation for null bytes and require absolute paths
  1. NuGet Package Signing: Certificate Exposure (.github/workflows/publish.yml:68-81)
  • Certificate password passed as command-line argument (visible in process listings)
  • Certificate written to disk without restricted permissions
  • Fix: Use set +x and temp directory with restricted permissions

🟡 Medium

  1. Path Traversal: Partial Protection in ConfigPath (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:253-265)

    • No restriction on which directories can be accessed
    • Fix: Add working directory restriction
  2. JSON Deserialization: No Max Depth Limit (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:267)

    • Malformed JSON could cause memory exhaustion
    • Fix: Add MaxDepth = 64 to JsonSerializerOptions
  3. Source Generator: Regex DoS Potential (sdk/dotnet/src/RuleSync.Sdk.DotNet.SourceGenerators/RulesyncIncrementalGenerator.cs:20-36)

    • [^\]]* pattern could cause catastrophic backtracking
    • Fix: Add length limit and timeout to regex

🔵 Low

  1. Missing Content Validation on Downloaded NuGet Packages (.github/workflows/publish.yml:60-66)
  2. Timeout Race Condition in ProcessExtensions (sdk/dotnet/src/RuleSync.Sdk.DotNet/Polyfills/ProcessExtensions.cs:37-48)

✅ Verified Security Fixes

  1. Command Injection Fix - CORRECT: Uses ArgumentList instead of string concatenation
  2. Input Validation Fix - CORRECT: Enum.IsDefined() validates enum values
  3. Race Condition Fix - CORRECT: Proper event subscription order in ProcessExtensions

Summary

Severity Code Review Security Review Total
Critical 2 1 3
High 4 2 6
Medium 5 3 8
Low 6 2 8
Required Before Merge:

github run

All findings resolved.

@dyoshikawa-claw

This comment has been minimized.

@github-actions

This comment has been minimized.

@dyoshikawa-claw

This comment has been minimized.

@github-actions

This comment has been minimized.

@cm-dyoshikawa
Copy link
Collaborator

cm-dyoshikawa commented Mar 11, 2026

@rudironsoni Thank you for the PR. However I don't have experiences about the .NET ecosystem, so I'm not sure if I can maintain it in into the future.

So can you create your repository for it? If so, I can link to your repository from the documentation of this repository.

By the way, I'm sorry about this situation, so I would appreciate it if you could open an issue in advance for any new feature suggestions next time.

@dyoshikawa-claw

This comment has been minimized.

@github-actions

This comment has been minimized.

Add comprehensive test suite for RuleSync .NET SDK:

- ClientSecurityTests.cs: Path validation security tests (12 tests)
- ClientArgumentBuilderTests.cs: Enum validation and CLI argument tests (30+ tests)
- ClientAsyncTests.cs: Async operation, disposal, and concurrent tests
- JsonContextTests.cs: JSON serialization/deserialization tests
- ResultEdgeCaseTests.cs: Result<T> pattern edge case tests
- AssemblyInfo.cs: InternalsVisibleTo for testing internal APIs
- ModelTests.cs: Extended with complete enum coverage (all 24 ToolTarget values)

Results:
- Tests: 31 → 232 (7.5x increase)
- Coverage: ~25% → 90.05% (exceeds 80% target)
- Line coverage: 335/372 lines
- Branch coverage: 115/130 branches

All priority areas covered: security, async operations,
argument building, models, JSON context, and edge cases.
@dyoshikawa-claw

This comment has been minimized.

@github-actions

This comment has been minimized.

Add sdk/dotnet/.gitignore to exclude:
- coverage-report/ (generated test coverage)
- TestResults/ (dotnet test output)
- bin/ and obj/ (build artifacts)
- IDE files and user-specific settings
@dyoshikawa-claw

This comment has been minimized.

@github-actions
Copy link
Contributor


PR #91 Review: RuleSync .NET SDK with Source Generators

Mergeability Verdict: NOT MERGEABLE

The PR contains 3 high-severity issues that must be fixed before merging. Additionally, the repository maintainer has indicated this PR will not be merged due to lack of .NET ecosystem experience and maintainability concerns (see comment).


Code Review Findings

🟠 HIGH Severity

  1. AOT Compatibility Claim is Misleading (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncJsonContext.cs:13-16)

    • Claims "Native AOT compatibility" but uses reflection-based JSON serialization with RequiresUnreferencedCode suppression
    • True AOT compatibility requires source-generated JSON serialization using [JsonSerializable]
    • Fix: Remove AOT claims from documentation or implement true source-generated serialization
  2. Missing Feature Validation in ImportAsync (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:247-251)

    • BuildGenerateArgs validates both ToolTarget and Feature enums
    • BuildImportArgs only validates ToolTarget, allowing invalid Feature values
    • Fix: Add Enum.IsDefined() validation for Feature enum in BuildImportArgs
  3. Incorrect CLI Value Conversion for Hyphenated Enums (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:314-342)

    • ToCliValue converts AugmentCodeLegacyaugment-code-legacy but CLI expects augmentcode-legacy
    • Source generator has correct mappings but ToCliValue doesn't use them
    • Fix: Create reverse lookup dictionary or use the same mapping approach

🟡 MEDIUM Severity

  1. Output Size Limit Race Condition (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:386-399)

    • Condition allows exceeding MaxOutputSize by dataLength
    • Multiple threads could pass check simultaneously
  2. Result.Map Constraint Limits Value Types (sdk/dotnet/src/RuleSync.Sdk.DotNet/Result.cs:69)

    • where TResult : class constraint prevents mapping to value types
  3. Missing CancellationToken Registration Cleanup (sdk/dotnet/src/RuleSync.Sdk.DotNet/Polyfills/ProcessExtensions.cs:39-47)

    • Registration only disposed if task completes, not if process exits first

🔵 LOW Severity

  1. Inconsistent test namespace (RuleSync.Sdk.Tests vs RuleSync.Sdk.DotNet.Tests)
  2. Potential null reference in ToCliValue returns empty string instead of throwing
  3. Missing unit tests for ToCliValue method

Security Review Findings

🟡 MEDIUM Severity

  1. Duplicate Dependabot Configuration (.github/dependabot-dotnet.yml)

    • GitHub only reads .github/dependabot.yml, so NuGet config is ignored
    • Fix: Merge into main dependabot.yml and delete duplicate file
  2. Shell Metacharacters Not Blocked in Executable Path (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:287-309)

    • Paths like "node; rm -rf /" pass validation
    • UseShellExecute = false prevents shell interpretation but path used directly as FileName
    • Fix: Reject paths containing ;, $, `, |, &, etc.

🔵 LOW Severity

  1. Config Path Traversal Without Warning (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncClient.cs:270-282)

    • Acceptable for CLI tool but could add optional base directory validation
  2. JSON Deserialization Missing Type Validation (sdk/dotnet/src/RuleSync.Sdk.DotNet/RulesyncJsonContext.cs:36-38)

    • Acceptable for trusted CLI source with existing mitigations

✅ Verified Security Fixes

  1. Script injection via REPO_OWNER - FIXED with regex validation
  2. Certificate exposure in publish.yml - FIXED with set +x and secure temp directory
  3. Command injection via argument concatenation - FIXED using ArgumentList
  4. Race condition in ProcessExtensions - FIXED with proper event subscription order
  5. Enum validation for ImportOptions.Target - FIXED with Enum.IsDefined
  6. Tag validation timing - FIXED by validating before use

Summary

Severity Code Review Security Review Total
Critical 0 0 0
High 3 0 3
Medium 3 2 5
Low 3 2 5

Required Before Merge:

Important: The maintainer has requested the contributor create a separate repository for this SDK. Consider forking to a dedicated rulesync-dotnet repository.

github run

Remove invalid Assert.NotNull() calls on ValueTask<T> and Result<T>
value types to fix xUnit2002 analyzer warnings treated as errors in CI.

Changes:
- ClientArgumentBuilderTests.cs: Replace var task + Assert.NotNull(task)
  with discard pattern _ = method call
- ClientAsyncTests.cs: Replace Assert.All(results, r => Assert.NotNull(r))
  with Assert.All(results, r => Assert.True(r.IsSuccess || r.IsFailure))
- ClientSecurityTests.cs: Same discard pattern fix

CI was failing with:
- Do not use Assert.NotNull() on value type 'ValueTask<Result<...>>'
- Do not use Assert.NotNull() on value type 'Result<...>'

Build: 0 warnings, 0 errors
- Remove all vacuous assertions (IsSuccess || IsFailure) from test files
- Remove silent timeout handling that masked performance issues
- Remove no-op Assert.True(true) assertions
- Fix tool target naming: augmentcode-legacy and claudecode-legacy
- Add missing cspell dictionary entries for security test strings
- Add meaningful comments explaining test validation intent
@rudironsoni
Copy link
Contributor Author

@rudironsoni Thank you for the PR. However I don't have experiences about the .NET ecosystem, so I'm not sure if I can maintain it in into the future.

So can you create your repository for it? If so, I can link to your repository from the documentation of this repository.

By the way, I'm sorry about this situation, so I would appreciate it if you could open an issue in advance for any new feature suggestions next time.

I can be the maintainer of the dotnet sdk, I'm totally willing to contribute to your project.

If you feel comfortable with it being a separate project, I'm ok. Your call!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants