diff --git a/src/DevSecOpsSentinel.Api/Endpoints/WorkflowEndpoints.cs b/src/DevSecOpsSentinel.Api/Endpoints/WorkflowEndpoints.cs index 4fa5205..1559312 100644 --- a/src/DevSecOpsSentinel.Api/Endpoints/WorkflowEndpoints.cs +++ b/src/DevSecOpsSentinel.Api/Endpoints/WorkflowEndpoints.cs @@ -26,6 +26,11 @@ public static WebApplication MapWorkflowEndpoints(this WebApplication app, int m IWorkflowAnalysisService service, CancellationToken cancellationToken) => { + if (request is null) + { + return MissingOrInvalidRequest(); + } + IResult? validationFailure = ValidateWorkflowRequest( request, @@ -39,7 +44,7 @@ public static WebApplication MapWorkflowEndpoints(this WebApplication app, int m WorkflowAnalysisResult result = await service.AnalyzeAsync( new WorkflowDocument( - request!.FileName, + request.FileName, request.Content), cancellationToken); @@ -70,6 +75,11 @@ await service.AnalyzeAsync( IRemediationReportService service, CancellationToken cancellationToken) => { + if (request is null) + { + return MissingOrInvalidRequest(); + } + IResult? validationFailure = ValidateWorkflowRequest( request, @@ -83,7 +93,7 @@ await service.AnalyzeAsync( RemediationReport report = await service.BuildAsync( new WorkflowDocument( - request!.FileName, + request.FileName, request.Content), cancellationToken); @@ -107,6 +117,11 @@ await service.BuildAsync( IRemediationReportService service, CancellationToken cancellationToken) => { + if (request is null) + { + return MissingOrInvalidRequest(); + } + IResult? validationFailure = ValidateWorkflowRequest( request, @@ -120,7 +135,7 @@ await service.BuildAsync( RemediationReport report = await service.BuildAsync( new WorkflowDocument( - request!.FileName, + request.FileName, request.Content), cancellationToken); @@ -184,13 +199,16 @@ await service.BuildAsync( CallerAuthentication caller, CancellationToken cancellationToken) => { + if (request is null) + { + return MissingOrInvalidRequest(); + } + IResult? validationFailure = ValidateWorkflowRequest( - request is null - ? null - : new AnalyzeWorkflowRequest( - request.FileName, - request.Content), + new AnalyzeWorkflowRequest( + request.FileName, + request.Content), maximumWorkflowCharacters); if (validationFailure is not null) @@ -201,7 +219,7 @@ request is null WorkflowExplanationResult result = await service.ExplainAsync( new WorkflowDocument( - request!.FileName, + request.FileName, request.Content), request.UseAi, caller.AiAccess == AiAccess.Full @@ -232,20 +250,28 @@ await service.ExplainAsync( return app; } + /// + /// A missing body and an invalid one produce the same problem response, but they are + /// checked in different places: the null test lives at each call site so the compiler — + /// and the analyzer — can see the proof, instead of a null-forgiving operator asserting + /// what a helper established somewhere else. + /// + private static IResult MissingOrInvalidRequest() => + Results.BadRequest(new ProblemDetails + { + Title = "Invalid workflow request", + Detail = "Both fileName and content are required.", + Status = StatusCodes.Status400BadRequest + }); + private static IResult? ValidateWorkflowRequest( - AnalyzeWorkflowRequest? request, + AnalyzeWorkflowRequest request, int maximumCharacters) { - if (request is null || - string.IsNullOrWhiteSpace(request.FileName) || + if (string.IsNullOrWhiteSpace(request.FileName) || string.IsNullOrWhiteSpace(request.Content)) { - return Results.BadRequest(new ProblemDetails - { - Title = "Invalid workflow request", - Detail = "Both fileName and content are required.", - Status = StatusCodes.Status400BadRequest - }); + return MissingOrInvalidRequest(); } if (request.Content.Length > maximumCharacters) diff --git a/src/DevSecOpsSentinel.Api/Security/ApiKeyAuthenticationMiddleware.cs b/src/DevSecOpsSentinel.Api/Security/ApiKeyAuthenticationMiddleware.cs index 4d271c5..5264081 100644 --- a/src/DevSecOpsSentinel.Api/Security/ApiKeyAuthenticationMiddleware.cs +++ b/src/DevSecOpsSentinel.Api/Security/ApiKeyAuthenticationMiddleware.cs @@ -60,14 +60,10 @@ private bool IsOpenRequest(PathString path, ApiSecurityOptions options) return true; } - if (environment.IsDevelopment() || - environment.IsEnvironment("Testing")) + if ((environment.IsDevelopment() || environment.IsEnvironment("Testing")) && + (path.StartsWithSegments("/openapi") || path.StartsWithSegments("/scalar"))) { - if (path.StartsWithSegments("/openapi") || - path.StartsWithSegments("/scalar")) - { - return true; - } + return true; } // Everything the deterministic engine serves. It parses text and diff --git a/src/DevSecOpsSentinel.Infrastructure/Ai/OpenAiWorkflowAiProvider.cs b/src/DevSecOpsSentinel.Infrastructure/Ai/OpenAiWorkflowAiProvider.cs index b3586af..cfa45c2 100644 --- a/src/DevSecOpsSentinel.Infrastructure/Ai/OpenAiWorkflowAiProvider.cs +++ b/src/DevSecOpsSentinel.Infrastructure/Ai/OpenAiWorkflowAiProvider.cs @@ -30,6 +30,12 @@ public async Task ExplainAsync( string sanitizedContent, CancellationToken cancellationToken) { + // The file name arrives in the request, and a value containing a line break would + // let a caller forge extra lines in any text log sink. Structured logging keeps it a + // property in JSON sinks, but the console rendering is still a text line. One control + // character is enough to matter; none survive this. + string safeFileName = new([.. analysis.FileName.Where(c => !char.IsControl(c))]); + if (_client is null) { return AiExplanationFactory.CreateFallback( @@ -113,7 +119,7 @@ public async Task ExplainAsync( _logger.LogWarning( exception, "OpenAI request timed out for workflow {FileName}.", - analysis.FileName); + safeFileName); return AiExplanationFactory.CreateFallback( analysis, @@ -125,7 +131,7 @@ public async Task ExplainAsync( _logger.LogWarning( exception, "OpenAI request failed for workflow {FileName}.", - analysis.FileName); + safeFileName); return AiExplanationFactory.CreateFallback( analysis, diff --git a/tests/DevSecOpsSentinel.Evals/ContainmentReplayEval.cs b/tests/DevSecOpsSentinel.Evals/ContainmentReplayEval.cs index 27cda96..b13e43e 100644 --- a/tests/DevSecOpsSentinel.Evals/ContainmentReplayEval.cs +++ b/tests/DevSecOpsSentinel.Evals/ContainmentReplayEval.cs @@ -131,15 +131,15 @@ public void Write_replay_scoreboard() + $"| {verdict(accepted)} | {(accepted == entry.ShouldBeAccepted ? "pass" : "**FAIL**")} |"); } - File.WriteAllLines(Path.Combine(AppContext.BaseDirectory, "replay-scoreboard.md"), lines); + File.WriteAllLines(Path.Join(AppContext.BaseDirectory, "replay-scoreboard.md"), lines); Assert.True(true); } - private static string ResponsesDirectory => Path.Combine(AppContext.BaseDirectory, "Responses"); + private static string ResponsesDirectory => Path.Join(AppContext.BaseDirectory, "Responses"); private static OpenAiWorkflowAiProvider.OpenAiExplanationPayload Load(string responseFile) { - string json = File.ReadAllText(Path.Combine(ResponsesDirectory, responseFile)); + string json = File.ReadAllText(Path.Join(ResponsesDirectory, responseFile)); return JsonSerializer.Deserialize(json, JsonOptions) ?? throw new InvalidOperationException($"{responseFile} did not deserialise."); } diff --git a/tests/DevSecOpsSentinel.Evals/CorpusEval.cs b/tests/DevSecOpsSentinel.Evals/CorpusEval.cs index ab304b6..d6522f4 100644 --- a/tests/DevSecOpsSentinel.Evals/CorpusEval.cs +++ b/tests/DevSecOpsSentinel.Evals/CorpusEval.cs @@ -89,7 +89,7 @@ public void Corpus_entries_all_have_a_file_on_disk() { string[] missing = [.. GoldenCorpus.Entries .Select(entry => entry.FileName) - .Where(name => !File.Exists(Path.Combine(CorpusDirectory, name)))]; + .Where(name => !File.Exists(Path.Join(CorpusDirectory, name)))]; Assert.True(missing.Length == 0, $"Declared but absent from Corpus/: {Join(missing)}"); } @@ -140,12 +140,12 @@ public void Write_scoreboard() lines.Add($"| {rule.RuleId} {rule.Title} | {(count == 0 ? "**none**" : count.ToString())} |"); } - string path = Path.Combine(AppContext.BaseDirectory, "scoreboard.md"); + string path = Path.Join(AppContext.BaseDirectory, "scoreboard.md"); File.WriteAllLines(path, lines); Assert.True(File.Exists(path)); } - internal static string CorpusDirectory => Path.Combine(AppContext.BaseDirectory, "Corpus"); + internal static string CorpusDirectory => Path.Join(AppContext.BaseDirectory, "Corpus"); /// /// The scan a recorded reply is measured against. Shared with the replay eval so both @@ -153,7 +153,7 @@ public void Write_scoreboard() /// internal static WorkflowAnalysisResult AnalyzeForReplay(string fileName) { - string content = File.ReadAllText(Path.Combine(CorpusDirectory, fileName)); + string content = File.ReadAllText(Path.Join(CorpusDirectory, fileName)); WorkflowParseResult parsed = Parser.Parse(new WorkflowDocument(fileName, content)); return new WorkflowAnalysisResult( @@ -166,7 +166,7 @@ internal static WorkflowAnalysisResult AnalyzeForReplay(string fileName) private static string[] Scan(string fileName) { - string content = File.ReadAllText(Path.Combine(CorpusDirectory, fileName)); + string content = File.ReadAllText(Path.Join(CorpusDirectory, fileName)); WorkflowParseResult parsed = Parser.Parse(new WorkflowDocument(fileName, content)); Assert.True( diff --git a/tests/DevSecOpsSentinel.Infrastructure.Tests/GitHubPrivateKeySourceTests.cs b/tests/DevSecOpsSentinel.Infrastructure.Tests/GitHubPrivateKeySourceTests.cs index 83e36b8..21f6764 100644 --- a/tests/DevSecOpsSentinel.Infrastructure.Tests/GitHubPrivateKeySourceTests.cs +++ b/tests/DevSecOpsSentinel.Infrastructure.Tests/GitHubPrivateKeySourceTests.cs @@ -22,7 +22,7 @@ private static string CreatePem() private readonly string _pem = CreatePem(); - private readonly string _keyPath = Path.Combine( + private readonly string _keyPath = Path.Join( Path.GetTempPath(), $"sentinel-key-{Guid.NewGuid():N}.pem"); @@ -122,7 +122,7 @@ public void A_missing_file_is_reported_as_unavailable() { GitHubPrivateKeySource source = new(new GitHubOptions { - PrivateKeyPath = Path.Combine(Path.GetTempPath(), "does-not-exist.pem") + PrivateKeyPath = Path.Join(Path.GetTempPath(), "does-not-exist.pem") }); Assert.False(source.IsAvailable); diff --git a/tests/DevSecOpsSentinel.Infrastructure.Tests/RepositoryWorkflowsTests.cs b/tests/DevSecOpsSentinel.Infrastructure.Tests/RepositoryWorkflowsTests.cs index 4a4aad3..4d1e78f 100644 --- a/tests/DevSecOpsSentinel.Infrastructure.Tests/RepositoryWorkflowsTests.cs +++ b/tests/DevSecOpsSentinel.Infrastructure.Tests/RepositoryWorkflowsTests.cs @@ -59,7 +59,7 @@ private static string WorkflowDirectory() DirectoryInfo? directory = new(AppContext.BaseDirectory); while (directory is not null) { - string candidate = Path.Combine(directory.FullName, ".github", "workflows"); + string candidate = Path.Join(directory.FullName, ".github", "workflows"); if (Directory.Exists(candidate)) { return candidate; @@ -96,7 +96,7 @@ public void There_are_workflows_to_check() [MemberData(nameof(WorkflowFiles))] public void Our_own_workflows_pass_our_own_rules(string fileName) { - string path = Path.Combine(WorkflowDirectory(), fileName); + string path = Path.Join(WorkflowDirectory(), fileName); WorkflowParseResult result = _parser.Parse( new WorkflowDocument(fileName, File.ReadAllText(path)));