Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 31747311 | Triggered | Generic High Entropy Secret | fffa946 | Tests/Resgrid.Tests/Config/PaymentProviderConfigTests.cs | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
📝 WalkthroughWalkthroughIntroduces Paddle configuration validation methods with normalization logic to ensure environment and client-token values are properly formatted. Updates the subscription controller to validate Paddle configuration and gate checkout initialization. Extends view models with Paddle readiness flags. Modifies subscription views to conditionally initialize Paddle based on validated configuration. Updates Kubernetes deployment manifest with enhanced security and storage configurations. Changes
Sequence DiagramsequenceDiagram
participant User
participant View as Subscription View
participant Controller as SubscriptionController
participant Config as PaymentProviderConfig
participant Client as Browser/JS
User->>View: Request subscription page
View->>Controller: GET request
Controller->>Config: GetPaddleEnvironment()
Config-->>Controller: Normalized env value
Controller->>Config: GetPaddleClientToken()
Config-->>Controller: Normalized token
Controller->>Config: IsValidPaddleEnvironment(env)
Config-->>Controller: Validation result
Controller->>Config: IsValidPaddleClientToken(token)
Config-->>Controller: Validation result
Controller->>Controller: Set CanInitializePaddleCheckout flag
Controller-->>View: Render with validation flags
View->>Client: Serialize config as JSON
Client->>Client: Check CanInitializePaddleCheckout?
alt Valid Configuration
Client->>Client: Initialize Paddle SDK
else Invalid Configuration
Client->>User: Show "Checkout Unavailable" alert
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Web/Resgrid.Web/Areas/User/Views/Subscription/Index.cshtml (1)
297-308:⚠️ Potential issue | 🟡 MinorConsider surfacing the Paddle configuration error inline on this page, consistent with
SelectRegistrationPlan.cshtml.
SelectRegistrationPlan.cshtml(lines 135-140) renders a visiblealert alert-dangerwhenModel.IsPaddleDepartment && !Model.CanInitializePaddleCheckout && PaddleConfigurationErroris populated, butIndex.cshtmlonly reveals the error via a Swal dialog after the user clicks Buy Yearly/Buy Monthly. For a department admin who is actively troubleshooting a misconfiguration, an inline banner (placed alongside the existingSubscriptionErrorMessageblock) provides faster, self-service diagnosis and keeps the two subscription surfaces consistent.💡 Suggested placement
`@if` (ViewBag.SubscriptionErrorMessage != null) { <div class="row"> <div class="col-xs-12"> <div class="alert alert-danger alert-block"> <h4 class="alert alert-heading">@localizer["Warning"]</h4> `@ViewBag.SubscriptionErrorMessage` </div> </div> </div> } + `@if` (Model.IsPaddleDepartment && !Model.CanInitializePaddleCheckout && !string.IsNullOrWhiteSpace(Model.PaddleConfigurationError)) + { + <div class="row"> + <div class="col-xs-12"> + <div class="alert alert-danger alert-block"> + <strong>Paddle Checkout Unavailable:</strong> `@Model.PaddleConfigurationError` + </div> + </div> + </div> + } <hr>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Subscription/Index.cshtml` around lines 297 - 308, Add an inline alert that surfaces the Paddle configuration error on the subscription Index page similar to SelectRegistrationPlan.cshtml: inside Index.cshtml, next to the existing ViewBag.SubscriptionErrorMessage block, add a conditional block that checks Model.IsPaddleDepartment && !Model.CanInitializePaddleCheckout && !String.IsNullOrEmpty(Model.PaddleConfigurationError) and renders a <div class="alert alert-danger"> showing Model.PaddleConfigurationError (optionally with `@localizer`["Warning"] heading) so admins see the misconfiguration banner without invoking the Swal dialog when clicking Buy Yearly/Buy Monthly.
🧹 Nitpick comments (2)
Tests/Resgrid.Tests/Config/PaymentProviderConfigTests.cs (1)
41-43: Use atest_-prefixed token in this fixture to avoid secret-scanner false positives.The value is Paddle's public documentation example and not a real credential, but using a
live_prefix in test data trips Betterleaks (high severity in CI) and similar scanners. Since the regex treatstest_andlive_equivalently, switching to atest_token keeps coverage identical while silencing the finding and making the intent "this is a test fixture" clearer.♻️ Proposed change
- PaymentProviderConfig.PaddleProductionClientToken = " live_7d279f61a3499fed520f7cd8c08 "; + PaymentProviderConfig.PaddleProductionClientToken = " test_7d279f61a3499fed520f7cd8c08 "; - PaymentProviderConfig.GetPaddleClientToken().Should().Be("live_7d279f61a3499fed520f7cd8c08"); + PaymentProviderConfig.GetPaddleClientToken().Should().Be("test_7d279f61a3499fed520f7cd8c08");The same applies to line 52 (
[TestCase("live_7d279f61a3499fed520f7cd8c08", true)]) if you want to fully silence the scanner across the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/Resgrid.Tests/Config/PaymentProviderConfigTests.cs` around lines 41 - 43, Replace the test data Paddle token strings that use a "live_" prefix with a clearly non-secret "test_" prefixed token to avoid secret-scanner false positives; specifically update the PaddleProductionClientToken assignment (PaymentProviderConfig.PaddleProductionClientToken) and any related test case entries that pass "live_7d279f61a3499fed520f7cd8c08" (e.g., the TestCase that calls GetPaddleClientToken) to use "test_7d279f61a3499fed520f7cd8c08" so assertions against PaymentProviderConfig.GetPaddleClientToken() remain identical but scanners are silenced.Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs (1)
81-99: LGTM — clean extraction into a pure static helper.Centralizing the Paddle configuration/validation into a pure tuple-returning helper is a nice fit with the codebase's functional leanings, and the early-return for non-Paddle departments keeps the happy path flat. Reuse between
SelectRegistrationPlanandIndexeliminates duplication.Minor nit: the
isPaddleDepartment && !canInitializePaddleCheckoutcheck on line 96 is redundant —isPaddleDepartmentis already guaranteedtrueat that point due to the early return on line 83-84. You can simplify to!canInitializePaddleCheckout ? GetPaddleConfigurationError(...) : null. Not a functional issue.As per coding guidelines: "Prefer pure methods over methods with side effects".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs` around lines 81 - 99, GetPaddleCheckoutConfiguration contains a redundant check: the ternary uses "isPaddleDepartment && !canInitializePaddleCheckout" but isPaddleDepartment is already true due to the early return; simplify the ternary to check only "!canInitializePaddleCheckout" and return GetPaddleConfigurationError(paddleEnvironment, paddleClientToken) when that is true, otherwise null, keeping the rest of the tuple and method logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Web/Resgrid.Web.Tts/k8s/deployment.yaml`:
- Around line 174-197: The Ingress resource (metadata name resgrid-tts) must
declare an ingress class and enable TLS: add spec.ingressClassName: nginx to
ensure an nginx controller will reconcile it (don't rely solely on nginx.*
annotations), and add a spec.tls block with hosts: ["tts.example.com"] and a
secretName (e.g., resgrid-tts-tls) that will hold the certificate (use your
cert-manager-issued secret name if you use cert-manager). Ensure the host under
spec.rules still matches tts.example.com and that the secretName is
created/managed by your certificate issuer.
---
Outside diff comments:
In `@Web/Resgrid.Web/Areas/User/Views/Subscription/Index.cshtml`:
- Around line 297-308: Add an inline alert that surfaces the Paddle
configuration error on the subscription Index page similar to
SelectRegistrationPlan.cshtml: inside Index.cshtml, next to the existing
ViewBag.SubscriptionErrorMessage block, add a conditional block that checks
Model.IsPaddleDepartment && !Model.CanInitializePaddleCheckout &&
!String.IsNullOrEmpty(Model.PaddleConfigurationError) and renders a <div
class="alert alert-danger"> showing Model.PaddleConfigurationError (optionally
with `@localizer`["Warning"] heading) so admins see the misconfiguration banner
without invoking the Swal dialog when clicking Buy Yearly/Buy Monthly.
---
Nitpick comments:
In `@Tests/Resgrid.Tests/Config/PaymentProviderConfigTests.cs`:
- Around line 41-43: Replace the test data Paddle token strings that use a
"live_" prefix with a clearly non-secret "test_" prefixed token to avoid
secret-scanner false positives; specifically update the
PaddleProductionClientToken assignment
(PaymentProviderConfig.PaddleProductionClientToken) and any related test case
entries that pass "live_7d279f61a3499fed520f7cd8c08" (e.g., the TestCase that
calls GetPaddleClientToken) to use "test_7d279f61a3499fed520f7cd8c08" so
assertions against PaymentProviderConfig.GetPaddleClientToken() remain identical
but scanners are silenced.
In `@Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs`:
- Around line 81-99: GetPaddleCheckoutConfiguration contains a redundant check:
the ternary uses "isPaddleDepartment && !canInitializePaddleCheckout" but
isPaddleDepartment is already true due to the early return; simplify the ternary
to check only "!canInitializePaddleCheckout" and return
GetPaddleConfigurationError(paddleEnvironment, paddleClientToken) when that is
true, otherwise null, keeping the rest of the tuple and method logic unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ccb917b4-a6be-414c-abcd-d58d3a8b538c
📒 Files selected for processing (8)
Core/Resgrid.Config/PaymentProviderConfig.csTests/Resgrid.Tests/Config/PaymentProviderConfigTests.csWeb/Resgrid.Web.Tts/k8s/deployment.yamlWeb/Resgrid.Web/Areas/User/Controllers/SubscriptionController.csWeb/Resgrid.Web/Areas/User/Models/Subscription/SelectRegistrationPlanView.csWeb/Resgrid.Web/Areas/User/Models/Subscription/SubscriptionView.csWeb/Resgrid.Web/Areas/User/Views/Subscription/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml
| --- | ||
| apiVersion: networking.k8s.io/v1 | ||
| kind: Ingress | ||
| metadata: | ||
| name: resgrid-tts | ||
| labels: | ||
| app.kubernetes.io/name: resgrid-tts | ||
| app.kubernetes.io/part-of: resgrid | ||
| app.kubernetes.io/managed-by: fleet | ||
| annotations: | ||
| nginx.ingress.kubernetes.io/proxy-read-timeout: "60" | ||
| nginx.ingress.kubernetes.io/proxy-send-timeout: "60" | ||
| spec: | ||
| rules: | ||
| - host: tts.example.com | ||
| http: | ||
| paths: | ||
| - path: / | ||
| pathType: Prefix | ||
| backend: | ||
| service: | ||
| name: resgrid-tts | ||
| port: | ||
| number: 80 |
There was a problem hiding this comment.
Ingress is missing ingressClassName and TLS.
Two gaps worth addressing before this rolls out:
- No
ingressClassName. The nginx-specific annotations alone are not sufficient on clusters without a defaultIngressClass; the resource will be created but never reconciled by any controller. Prefer the modernspec.ingressClassName: nginxfield over the deprecatedkubernetes.io/ingress.classannotation. - No TLS. The service exposes
StaticPromptAdminKey-guarded admin endpoints and playback URLs. Serving over plain HTTP leaks the admin key and any request payloads. Add atls:block (e.g., cert-manager-issued) fortts.example.com.
🛡️ Suggested change
spec:
+ ingressClassName: nginx
+ tls:
+ - hosts:
+ - tts.example.com
+ secretName: resgrid-tts-tls
rules:
- host: tts.example.com
http:
paths:
- path: /
pathType: Prefix
backend:
service:
name: resgrid-tts
port:
number: 80🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web.Tts/k8s/deployment.yaml` around lines 174 - 197, The Ingress
resource (metadata name resgrid-tts) must declare an ingress class and enable
TLS: add spec.ingressClassName: nginx to ensure an nginx controller will
reconcile it (don't rely solely on nginx.* annotations), and add a spec.tls
block with hosts: ["tts.example.com"] and a secretName (e.g., resgrid-tts-tls)
that will hold the certificate (use your cert-manager-issued secret name if you
use cert-manager). Ensure the host under spec.rules still matches
tts.example.com and that the secretName is created/managed by your certificate
issuer.
|
Approve |
|
Approve |
|
Approve |
Summary by CodeRabbit
Bug Fixes
Tests
Chores