Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Refitter.Core/Generation/InterfaceGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ private IEnumerable<GeneratedCode> GenerateMultipleInterface(
code.AppendLine($"{Separator}{Separator}{attribute}");
}

code.AppendLine($"{Separator}{Separator}[{verb}(\"{op.Path}\")]")
code.AppendLine($"{Separator}{Separator}[{verb}(\"{ParameterShared.EscapeString(op.Path)}\")]")
.AppendLine($"{Separator}{Separator}{returnType} {methodName}({parametersString});")
.AppendLine();

Expand All @@ -263,7 +263,7 @@ private IEnumerable<GeneratedCode> GenerateMultipleInterface(
", ",
parameters.Where(parameter => !parameter.Contains("?")));

code.AppendLine($"{Separator}{Separator}[{verb}(\"{op.Path}\")]")
code.AppendLine($"{Separator}{Separator}[{verb}(\"{ParameterShared.EscapeString(op.Path)}\")]")
.AppendLine($"{Separator}{Separator}{returnType} {methodName}({nonOptionalParametersString});")
.AppendLine();
}
Expand Down
4 changes: 2 additions & 2 deletions src/Refitter.Core/Generation/MethodAttributeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public string[] Generate(OpenApiOperation operation, CSharpOperationModel operat

if (uniqueContentTypes.Any())
{
headers.Add($"\"Accept: {string.Join(", ", uniqueContentTypes)}\"");
headers.Add($"\"Accept: {string.Join(", ", uniqueContentTypes.Select(ParameterShared.EscapeString))}\"");
}
}

Expand All @@ -54,7 +54,7 @@ public string[] Generate(OpenApiOperation operation, CSharpOperationModel operat

if (!string.IsNullOrWhiteSpace(contentType) && !operationModel.Consumes.Contains("multipart/form-data"))
{
headers.Add($"\"Content-Type: {contentType}\"");
headers.Add($"\"Content-Type: {ParameterShared.EscapeString(contentType)}\"");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public IEnumerable<string> Extract(
.Select(p =>
{
var variableName = ParameterShared.GetVariableName(p);
return $"{ParameterShared.JoinAttributes($"Header(\"{p.Name}\")")}{ParameterShared.GetParameterType(p, settings)} {variableName}";
return $"{ParameterShared.JoinAttributes($"Header(\"{ParameterShared.EscapeString(p.Name)}\")")}{ParameterShared.GetParameterType(p, settings)} {variableName}";
})
.ToList();
}
Expand All @@ -47,7 +47,7 @@ public IEnumerable<string> Extract(
&& securityScheme.In == OpenApiSecurityApiKeyLocation.Header
&& !operationModel.Parameters.Any(p => p.Kind == OpenApiParameterKind.Header && p.IsHeader && p.Name == securityScheme.Name))
{
headerParameters.Add($"[Header(\"{securityScheme.Name}\")] string {ParameterShared.ReplaceUnsafeCharacters(securityScheme.Name)}");
headerParameters.Add($"[Header(\"{ParameterShared.EscapeString(securityScheme.Name)}\")] string {ParameterShared.ReplaceUnsafeCharacters(securityScheme.Name)}");
}
else if (securityScheme is { Type: OpenApiSecuritySchemeType.Http }
&& string.Equals(securityScheme.Scheme, "bearer", StringComparison.OrdinalIgnoreCase))
Expand Down
118 changes: 118 additions & 0 deletions src/Refitter.Core/Validation/AttributeStringValidator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
using System.Linq;
using Microsoft.OpenApi;
using Microsoft.OpenApi.Reader;

namespace Refitter.Core.Validation;

/// <summary>
/// Rejects OpenAPI paths, header names, and content-type keys containing characters that could break
/// out of the generated Refit attribute string literals (e.g. quotes, backslashes, newlines). Generated
/// code escapes these characters, but rejecting them gives a clear error unless validation is skipped.
/// See GHSA-3fhm-p725-h3g3 (path), GHSA-58x9-vjvp-6mx8 (header name), GHSA-p32v-8v8j-j534 (content-type).
/// </summary>
internal static class AttributeStringValidator
{
internal static bool ContainsUnsafeCharacters(string? value) =>
value != null && value.Any(c => c is '"' or '\\' || char.IsControl(c));

internal static void Validate(OpenApiDocument? document, OpenApiDiagnostic diagnostic)

Check failure on line 18 in src/Refitter.Core/Validation/AttributeStringValidator.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 35 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=christianhelle_refitter&issues=AZ8YeY0PtSRAbF5ZlDbP&open=AZ8YeY0PtSRAbF5ZlDbP&pullRequest=1178
{
if (document == null)
return;

// Validate security scheme names for API-key schemes with header location
if (document.Components?.SecuritySchemes != null)
{
foreach (var securityScheme in document.Components.SecuritySchemes)
{
if (securityScheme.Value?.Type == SecuritySchemeType.ApiKey
&& securityScheme.Value.In == ParameterLocation.Header
&& ContainsUnsafeCharacters(securityScheme.Value.Name))
{
diagnostic.Errors.Add(new OpenApiError(
securityScheme.Key,
$"Security scheme '{securityScheme.Key}' has header name '{securityScheme.Value.Name}' containing illegal characters and is rejected to prevent code injection into Refit attributes. Use --skip-validation to bypass."));
}
}
}

if (document.Paths == null)
return;

// Accept/Content-Type headers are only emitted from content map keys for OpenAPI 3.0+,
// so only reject unsafe content-type keys for those documents to avoid Swagger 2.0 false positives.
var validateContentTypes = diagnostic.SpecificationVersion != OpenApiSpecVersion.OpenApi2_0;

foreach (var path in document.Paths)
{
if (ContainsUnsafeCharacters(path.Key))
{
diagnostic.Errors.Add(new OpenApiError(
path.Key,
$"Path '{path.Key}' contains illegal characters (quotes, backslashes, or control characters) and is rejected to prevent code injection into Refit attributes. Use --skip-validation to bypass."));
}

if (path.Value?.Operations == null)
continue;

foreach (var operation in path.Value.Operations.Values)
{
if (operation == null)
continue;

if (operation.Parameters != null)
{
foreach (var parameter in operation.Parameters)
{
if (parameter.In == ParameterLocation.Header && ContainsUnsafeCharacters(parameter.Name))
{
diagnostic.Errors.Add(new OpenApiError(
parameter.Name ?? string.Empty,
$"Header parameter name '{parameter.Name}' contains illegal characters and is rejected to prevent code injection into Refit attributes. Use --skip-validation to bypass."));
}
}
}

if (validateContentTypes)
{
ValidateContentTypeKeys(operation, diagnostic);
}
}
}
}

private static void ValidateContentTypeKeys(OpenApiOperation operation, OpenApiDiagnostic diagnostic)
{
if (operation.RequestBody?.Content != null)
{
foreach (var contentType in operation.RequestBody.Content.Keys)
{
AddContentTypeErrorIfUnsafe(contentType, diagnostic);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (operation.Responses == null)
return;

foreach (var response in operation.Responses.Values)
{
if (response?.Content == null)
continue;

foreach (var contentType in response.Content.Keys)
{
AddContentTypeErrorIfUnsafe(contentType, diagnostic);
}
}
}

private static void AddContentTypeErrorIfUnsafe(string contentType, OpenApiDiagnostic diagnostic)
{
if (ContainsUnsafeCharacters(contentType))
{
diagnostic.Errors.Add(new OpenApiError(
contentType,
$"Content type '{contentType}' contains illegal characters and is rejected to prevent code injection into Refit attributes. Use --skip-validation to bypass."));
}
}
}
4 changes: 4 additions & 0 deletions src/Refitter.Core/Validation/OpenApiValidator.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.OpenApi;
using Microsoft.OpenApi.Reader;

Expand Down Expand Up @@ -26,6 +28,8 @@ public static async Task<OpenApiValidationResult> Validate(
var walker = new OpenApiWalker(statsVisitor);
walker.Walk(result.OpenApiDocument);

AttributeStringValidator.Validate(result.OpenApiDocument, result.OpenApiDiagnostic);

return new(
result.OpenApiDiagnostic,
statsVisitor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Text.Json.Serialization;
using System.Threading;
using System.Threading.Tasks;
using System.Net.Http;

using Refitter.Tests.AdditionalFiles.SingeInterface;

Expand Down Expand Up @@ -512,8 +511,9 @@ public partial interface IDeleteUserEndpoint
namespace Refitter.Tests.AdditionalFiles.ByEndpoint
{
using System;
using Microsoft.Extensions.DependencyInjection;
using Refit;
using System.Net.Http;
using Microsoft.Extensions.DependencyInjection;
using Refit;
Comment thread
christianhelle marked this conversation as resolved.

/// <summary>
/// Extension methods for configuring Refit clients in the service collection.
Expand All @@ -536,6 +536,7 @@ public static IServiceCollection ConfigureRefitClients(
{
var clientBuilderIUpdatePetEndpoint = services
.AddRefitClient<IUpdatePetEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
Comment thread
christianhelle marked this conversation as resolved.
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -544,6 +545,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIAddPetEndpoint = services
.AddRefitClient<IAddPetEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -552,6 +554,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIFindPetsByStatusEndpoint = services
.AddRefitClient<IFindPetsByStatusEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -560,6 +563,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIFindPetsByTagsEndpoint = services
.AddRefitClient<IFindPetsByTagsEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -568,6 +572,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIGetPetByIdEndpoint = services
.AddRefitClient<IGetPetByIdEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -576,6 +581,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIUpdatePetWithFormEndpoint = services
.AddRefitClient<IUpdatePetWithFormEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -584,6 +590,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIDeletePetEndpoint = services
.AddRefitClient<IDeletePetEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -592,6 +599,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIUploadFileEndpoint = services
.AddRefitClient<IUploadFileEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -600,6 +608,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIGetInventoryEndpoint = services
.AddRefitClient<IGetInventoryEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -608,6 +617,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIPlaceOrderEndpoint = services
.AddRefitClient<IPlaceOrderEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -616,6 +626,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIGetOrderByIdEndpoint = services
.AddRefitClient<IGetOrderByIdEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -624,6 +635,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIDeleteOrderEndpoint = services
.AddRefitClient<IDeleteOrderEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -632,6 +644,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderICreateUserEndpoint = services
.AddRefitClient<ICreateUserEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -640,6 +653,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderICreateUsersWithListInputEndpoint = services
.AddRefitClient<ICreateUsersWithListInputEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -648,6 +662,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderILoginUserEndpoint = services
.AddRefitClient<ILoginUserEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -656,6 +671,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderILogoutUserEndpoint = services
.AddRefitClient<ILogoutUserEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -664,6 +680,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIGetUserByNameEndpoint = services
.AddRefitClient<IGetUserByNameEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -672,6 +689,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIUpdateUserEndpoint = services
.AddRefitClient<IUpdateUserEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand All @@ -680,6 +698,7 @@ public static IServiceCollection ConfigureRefitClients(

var clientBuilderIDeleteUserEndpoint = services
.AddRefitClient<IDeleteUserEndpoint>(settings)

.ConfigureHttpClient(c => c.BaseAddress = baseUrl)
.AddHttpMessageHandler<EmptyMessageHandler>()
.AddHttpMessageHandler<AnotherEmptyMessageHandler>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Collections.Generic;
using System.Text.Json.Serialization;
using System.Threading.Tasks;
using System.Net.Http;

#nullable enable annotations

Expand Down Expand Up @@ -743,6 +742,7 @@ public FileParameter(System.IO.Stream data, string fileName, string contentType)
namespace Refitter.Tests.AdditionalFiles.SingeInterfaceWithHttpResilience
{
using System;
using System.Net.Http;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Http.Resilience;
using Refit;
Expand All @@ -766,6 +766,7 @@ public static IServiceCollection ConfigureRefitClients(
{
var clientBuilderISwaggerPetstoreInterfaceWithHttpResilience = services
.AddRefitClient<ISwaggerPetstoreInterfaceWithHttpResilience>(settings)

.ConfigureHttpClient(c => c.BaseAddress = new Uri("https://petstore3.swagger.io/api/v3"));
Comment thread
christianhelle marked this conversation as resolved.

clientBuilderISwaggerPetstoreInterfaceWithHttpResilience
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Collections.Generic;
using System.Text.Json.Serialization;
using System.Threading.Tasks;
using System.Net.Http;

#nullable enable annotations

Expand Down Expand Up @@ -453,6 +452,7 @@ public FileParameter(System.IO.Stream data, string fileName, string contentType)
namespace Refitter.Tests.AdditionalFiles.SingeInterface
{
using System;
using System.Net.Http;
using Microsoft.Extensions.DependencyInjection;
using Polly;
using Polly.Contrib.WaitAndRetry;
Expand Down
Loading
Loading