fix(security): remediate CodeQL HIGH access-control, IDOR, and log-forging findings#126
Merged
Merged
Conversation
…rging findings ## Access control & IDOR (HIGH, alerts #4 and #5) - Add [Authorize] to IntegrationsController to enforce authentication on all management endpoints, including the unprotected DELETE at line 171 that triggered both cs/web/missing-function-level-access-control and cs/web/insecure-direct-object-reference. - Register authentication middleware (UseAuthentication) in the pipeline so the [Authorize] attribute is honoured at runtime. - Wire JWT Bearer auth when Auth:Authority is configured; fall back to a dev-only no-op handler (DevNoOpAuthHandler) that must be replaced before exposing to untrusted networks. - Add Microsoft.AspNetCore.Authentication.JwtBearer package reference. ## Log-forging (MEDIUM, alerts #1 #2 #152-155 #156-162 #197) - Sanitize all user-controlled strings before they reach log calls by stripping CR/LF characters (SanitizeForLog helper added to each affected class): - QuickApiMapper.Web/Program.cs: inputBody passed to LogDebug - Management.Api/Controllers/MessagesController.cs: messageId in LogWarning - Demo.JsonApi/Program.cs: statusUpdate.Notes in LogInformation - Demo.JsonApi/Services/InMemoryOrderService.cs: orderId x2 - Demo.SoapApi/Services/WarehouseService.cs: OrderNumber / ConfirmationNumber x4 - Demo.SoapApi/Storage/InMemoryFulfillmentRepository.cs: ConfirmationNumber / OrderNumber - QuickApiMapper.Designer.Web/Services/IntegrationApiClient.cs: url ## CodeQL workflow - Switch queries from security-and-quality to security-extended to eliminate the 14 medium + 57 warning + 177 note quality-rule noise from future scans. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Dependency ReviewThe following issues were found:
License Issuessrc/QuickApiMapper.Management.Api/QuickApiMapper.Management.Api.csproj
OpenSSF Scorecard
Scanned Files
|
Contributor
Contributor
🔍 PR Validation ResultsVersion: 📦 Detected NuGet Packages (17)
🚀 Detected Executables (2)
✅ Validation Steps
📊 ArtifactsDry-run artifacts have been uploaded and will be available for 7 days. This comment was automatically generated by the PR validation workflow. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| { | ||
| logger.LogInformation("Order {OrderId} status updated to {Status}. Notes: {Notes}", | ||
| id, statusUpdate.Status, statusUpdate.Notes); | ||
| id, statusUpdate.Status, SanitizeForLog(statusUpdate.Notes)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Access control & IDOR (HIGH, alerts #4 and #5)
management endpoints, including the unprotected DELETE at line 171 that triggered
both cs/web/missing-function-level-access-control and
cs/web/insecure-direct-object-reference.
[Authorize] attribute is honoured at runtime.
dev-only no-op handler (DevNoOpAuthHandler) that must be replaced before
exposing to untrusted networks.
Log-forging (MEDIUM, alerts #1 #2 #152-155 #156-162 #197)
CR/LF characters (SanitizeForLog helper added to each affected class):
CodeQL workflow
the 14 medium + 57 warning + 177 note quality-rule noise from future scans.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com