Security: enforce safe hosted controls for demo UI (#88)#123
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChangesDemo UI security
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RateLimiter
participant SecurityMiddleware
participant ExampleEndpoint
Client->>RateLimiter: Send request
RateLimiter-->>SecurityMiddleware: Permit or HTTP 429
SecurityMiddleware->>SecurityMiddleware: Validate API key, CSRF, and live-mode policy
SecurityMiddleware->>ExampleEndpoint: Forward permitted request
ExampleEndpoint-->>Client: Return response
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)PayBridge.SDK.Example/appsettings.Development.jsonFile contains syntax errors that prevent linting: Line 2: JSON standard does not allow comments.; Line 3: End of file expected; Line 3: End of file expected; Line 3: End of file expected; Line 8: End of file expected; Line 10: End of file expected; Line 10: End of file expected; Line 10: End of file expected; Line 19: End of file expected; Line 21: End of file expected; Line 21: End of file expected; Line 21: End of file expected; Line 42: End of file expected PayBridge.SDK.Example/appsettings.jsonFile contains syntax errors that prevent linting: Line 2: JSON standard does not allow comments.; Line 18: End of file expected; Line 18: End of file expected; Line 18: End of file expected; Line 23: End of file expected; Line 24: End of file expected; Line 24: End of file expected; Line 24: End of file expected; Line 24: End of file expected; Line 26: JSON standard does not allow comments.; Line 31: End of file expected; Line 31: End of file expected; Line 31: End of file expected; Line 31: End of file expected; Line 32: End of file expected; Line 32: End of file expected; Line 32: End of file expected; Line 34: End of file expected; Line 36: End of file expected; Line 36: End of file expected; Line 36: End of file expected; Line 45: End of file expected; Line 47: End of file expected; Line 47: End of file expected; Line 47: End of file expected; Line 49: JSON standard does not allow comments.; Line 53: End of file expected; Line 53: End of file expected; Line 53: End of file expected ... [truncated 1548 characters] ... file expected; Line 124: End of file expected; Line 124: End of file expected; Line 128: End of file expected; Line 130: JSON standard does not allow comments.; Line 132: End of file expected; Line 132: End of file expected; Line 132: End of file expected; Line 138: End of file expected; Line 140: End of file expected; Line 140: End of file expected; Line 140: End of file expected; Line 145: End of file expected; Line 147: End of file expected; Line 147: End of file expected; Line 147: End of file expected; Line 152: End of file expected; Line 154: End of file expected; Line 154: End of file expected; Line 154: End of file expected; Line 158: End of file expected; Line 160: End of file expected; Line 160: End of file expected; Line 160: End of file expected; Line 164: End of file expected Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces a server-enforced security policy for the PayBridge SDK Example “demo UI” by adding validated DemoUiSecurityOptions, enforcing hosted-only API key + CSRF checks for demo API routes, and applying security headers plus global rate limiting.
Changes:
- Added
DemoUiSecurityOptionsand a startup validator (IValidateOptions) with unit tests. - Implemented hosted
/apirequest gating (API key + CSRF) with webhook/verify route exemptions. - Added secure response headers and a global ASP.NET Core rate limiter.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| PayBridge.SDK.Test/Unit/DemoUiSecurityOptionsValidatorTests.cs | Adds unit tests for options validation failure/success cases. |
| PayBridge.SDK.Example/Services/DemoUiSecurityOptionsValidator.cs | Validates Demo UI security configuration on startup. |
| PayBridge.SDK.Example/Program.cs | Wires options + validator, adds global rate limiting, security headers, and hosted /api gating. |
| PayBridge.SDK.Example/Models/DemoUiSecurityOptions.cs | Defines security policy options (mode, API key, CSRF, throttling). |
| PayBridge.SDK.Example/appsettings.json | Provides default (non-dev) Demo UI security configuration values. |
| PayBridge.SDK.Example/appsettings.Development.json | Provides relaxed development defaults for Demo UI security configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| builder.Services.AddRateLimiter(options => | ||
| { | ||
| options.GlobalLimiter = PartitionedRateLimiter.Create<HttpContext, string>(_ => | ||
| RateLimitPartition.GetFixedWindowLimiter( | ||
| partitionKey: "global", | ||
| factory: _ => new FixedWindowRateLimiterOptions | ||
| { | ||
| PermitLimit = 60, | ||
| Window = TimeSpan.FromMinutes(1), | ||
| QueueLimit = 0, | ||
| AutoReplenishment = true | ||
| })); | ||
| options.RejectionStatusCode = StatusCodes.Status429TooManyRequests; | ||
| }); |
| if (options.RequireAuthenticatedAccess && string.IsNullOrWhiteSpace(options.ApiKey)) | ||
| { | ||
| failures.Add("DemoUiSecurity:ApiKey is required when DemoUiSecurity:RequireAuthenticatedAccess is true."); | ||
| } | ||
|
|
||
| if (options.RequireCsrfHeader) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(options.CsrfHeaderName)) | ||
| { | ||
| failures.Add("DemoUiSecurity:CsrfHeaderName is required when DemoUiSecurity:RequireCsrfHeader is true."); | ||
| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(options.CsrfHeaderValue)) | ||
| { | ||
| failures.Add("DemoUiSecurity:CsrfHeaderValue is required when DemoUiSecurity:RequireCsrfHeader is true."); | ||
| } | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
PayBridge.SDK.Example/Program.cs (1)
214-219: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant runtime validation block.
This block evaluates condition logic that is already strictly enforced by
DemoUiSecurityOptionsValidatorviaValidateOnStart()(line 77). BecausedemoSecurityis loaded as a singleton value at startup (line 152), the conditionMode == Live && !AllowLiveModewill never evaluate to true in this middleware.♻️ Proposed fix
- if (isHosted && demoSecurity.Mode == DemoRuntimeMode.Live && !demoSecurity.AllowLiveMode) - { - context.Response.StatusCode = StatusCodes.Status403Forbidden; - await context.Response.WriteAsJsonAsync(new { error = "live_mode_blocked" }); - return; - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PayBridge.SDK.Example/Program.cs` around lines 214 - 219, Remove the redundant live-mode validation block from the middleware around the isHosted request handling, including its 403 response and early return. Rely on DemoUiSecurityOptionsValidator with ValidateOnStart() to enforce the DemoRuntimeMode.Live and AllowLiveMode configuration before the singleton demoSecurity value is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PayBridge.SDK.Example/appsettings.json`:
- Around line 38-43: Replace the non-empty placeholder values for ApiKey and
CsrfHeaderValue with empty strings in PayBridge.SDK.Example/appsettings.json
lines 38-43 and PayBridge.SDK.Example/appsettings.Development.json lines 12-17,
ensuring DemoUiSecurityOptionsValidator and ValidateOnStart() require real
secrets supplied securely before startup.
In `@PayBridge.SDK.Example/Program.cs`:
- Around line 42-55: The global rate limiter configuration in AddRateLimiter
must use the validated RequestsPerMinute setting for
FixedWindowRateLimiterOptions.PermitLimit instead of the hardcoded value 60.
Reuse the existing configuration/options symbol that provides RequestsPerMinute,
while preserving the current global partition and other limiter settings.
- Around line 193-199: Replace the string.Equals check in the API-key validation
block with a constant-time comparison, converting the supplied and expected key
spans via MemoryMarshal.AsBytes without allocating byte arrays. Preserve ordinal
comparison semantics and the existing 401 response and early return for
mismatches.
- Around line 202-212: Update the CSRF validation block in Program.cs to cover
all state-changing HTTP methods (POST, PUT, PATCH, and DELETE) instead of only
POST, and replace string.Equals with the existing constant-time token comparison
approach used for the API key. Preserve the current invalid-token response and
early return behavior.
---
Nitpick comments:
In `@PayBridge.SDK.Example/Program.cs`:
- Around line 214-219: Remove the redundant live-mode validation block from the
middleware around the isHosted request handling, including its 403 response and
early return. Rely on DemoUiSecurityOptionsValidator with ValidateOnStart() to
enforce the DemoRuntimeMode.Live and AllowLiveMode configuration before the
singleton demoSecurity value is used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7019e43-4910-4719-b8e2-6c41525b7468
📒 Files selected for processing (6)
PayBridge.SDK.Example/Models/DemoUiSecurityOptions.csPayBridge.SDK.Example/Program.csPayBridge.SDK.Example/Services/DemoUiSecurityOptionsValidator.csPayBridge.SDK.Example/appsettings.Development.jsonPayBridge.SDK.Example/appsettings.jsonPayBridge.SDK.Test/Unit/DemoUiSecurityOptionsValidatorTests.cs
Summary
Why
Issue #88 requires preventing credential exposure and unsafe hosted console behavior by default. This change moves enforcement to server-side policy and startup validation rather than browser trust.
Validation
Notes
Closes #88
Summary by CodeRabbit
DemoUiSecurityconfiguration section in app settings.