Remove committed payment credentials#98
Conversation
📝 WalkthroughWalkthroughThe pull request replaces tracked sample credentials with placeholders, binds PayBridge setup from configuration, documents .NET user-secrets usage, adds repository placeholder validation, and introduces a gitleaks GitHub Actions workflow. ChangesCredential-safe configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 28: 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 37: JSON standard does not allow comments.; Line 41: End of file expected; Line 41: End of file expected; Line 41: End of file expected; Line 41: End of file expected; Line 43: JSON standard does not allow comments.; Line 48: End of file expected; Line 48: En ... [truncated 1415 characters] ... file expected; Line 108: End of file expected; Line 108: End of file expected; Line 112: End of file expected; Line 114: JSON standard does not allow comments.; Line 116: End of file expected; Line 116: End of file expected; Line 116: End of file expected; Line 122: End of file expected; Line 124: End of file expected; Line 124: End of file expected; Line 124: End of file expected; Line 129: End of file expected; Line 131: End of file expected; Line 131: End of file expected; Line 131: End of file expected; Line 136: End of file expected; Line 138: End of file expected; Line 138: End of file expected; Line 138: End of file expected; Line 142: End of file expected; Line 144: End of file expected; Line 144: End of file expected; Line 144: End of file expected; Line 148: 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 removes committed payment/database credentials from tracked configuration and documentation, shifts local secret setup to the .NET configuration stack (user-secrets/env/secret stores), and adds automated guardrails (unit test + CI secret scan) to prevent regressions.
Changes:
- Replaced hard-coded gateway activation and committed
appsettings*.jsoncredential values with placeholders and configuration binding. - Added a repository security unit test to detect committed credentials in tracked
appsettings*.jsonfiles. - Introduced a GitHub Actions workflow to scan PRs and key branches with Gitleaks.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.nuget.md | Updates NuGet quickstart to configure PayBridge via configuration and adds guidance to avoid committing credentials. |
| README.md | Updates setup docs to use user-secrets for connection strings and gateway credentials. |
| PayBridge.SDK.Test/Unit/RepositorySecurityTests.cs | Adds regression test that scans tracked appsettings*.json for committed credential values. |
| PayBridge.SDK.Presentation/Program.cs | Removes hard-coded gateway enablement and binds PayBridge from configuration. |
| PayBridge.SDK.Presentation/appsettings.json | Replaces committed DB/gateway values with placeholders and clears enabled gateways. |
| PayBridge.SDK.Example/README.md | Updates example instructions to store credentials outside the repo (user-secrets/env). |
| PayBridge.SDK.Example/Program.cs | Refreshes example comments to align with configuration-stack usage. |
| PayBridge.SDK.Example/PayBridge.SDK.Example.csproj | Adds UserSecretsId for local secrets support. |
| PayBridge.SDK.Example/appsettings.json | Converts example configuration to schema/placeholder-only tracked settings. |
| PayBridge.SDK.Example/appsettings.Development.json | Keeps dev settings tracked but credential-free with placeholders. |
| .github/workflows/secret-scanning.yml | Adds Gitleaks scanning for PRs and security-related branches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Passkey", | ||
| "Password", | ||
| "PrivateKey", | ||
| "SecretKey", | ||
| "TerminalResourceKey" |
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <RootNamespace>PayBridge.SDK.Example</RootNamespace> | ||
| <AssemblyName>PayBridge.SDK.Example</AssemblyName> | ||
| <UserSecretsId>PayBridge.SDK.Example</UserSecretsId> |
| For local development, prefer .NET user-secrets; for deployments, use environment | ||
| variables or a managed secret store. | ||
|
|
||
| ```json | ||
| { |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.nuget.md (1)
94-110: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign configuration placeholders with the new standard.
The example configuration block still uses a concrete connection string, an integer array for
EnabledGateways, and test keys (likesk_test_xxxx). To maintain consistency with the updatedappsettings.jsontemplates and to ensure the SDK's auto-ignore logic works reliably (which expects theYOUR_prefix), consider updating these values to the new placeholder format.♻️ Proposed update
"ConnectionStrings": { - "PayBridgeDbContext": "Server=localhost;Database=PayBridgeDB;Trusted_Connection=True;TrustServerCertificate=true;" + "PayBridgeDbContext": "YOUR_PAYBRIDGE_CONNECTION_STRING" }, "PaymentGatewayConfig": { - "EnabledGateways": [ 1, 2, 3 ], + "EnabledGateways": [], "FlutterwaveConfig": { - "PublicKey": "FLWPUBK-xxxx", - "SecretKey": "FLWSECK-xxxx", - "EncryptionKey": "xxxx" + "PublicKey": "YOUR_FLUTTERWAVE_PUBLIC_KEY", + "SecretKey": "YOUR_FLUTTERWAVE_SECRET_KEY", + "EncryptionKey": "YOUR_FLUTTERWAVE_ENCRYPTION_KEY" }, "Paystack": { - "PublicKey": "pk_test_xxxx", - "SecretKey": "sk_test_xxxx" + "PublicKey": "YOUR_PAYSTACK_PUBLIC_KEY", + "SecretKey": "YOUR_PAYSTACK_SECRET_KEY" }, "Stripe": { - "SecretKey": "sk_test_xxxx" + "SecretKey": "YOUR_STRIPE_SECRET_KEY" }🤖 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 `@README.nuget.md` around lines 94 - 110, Update the README configuration example to match the current appsettings.json placeholder conventions: replace the concrete PayBridgeDbContext connection string, integer EnabledGateways values, and test gateway keys with the standardized YOUR_ placeholders expected by the SDK auto-ignore logic. Keep the existing configuration structure and gateway entries unchanged.
🤖 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 @.github/workflows/secret-scanning.yml:
- Around line 24-25: Update the actions/checkout step in the secret-scanning
workflow to explicitly disable credential persistence with the supported
checkout configuration, while preserving the existing repository checkout
behavior.
In `@PayBridge.SDK.Test/Unit/RepositorySecurityTests.cs`:
- Around line 30-34: Update the file-filtering flow in the repository security
test to compute each file’s path relative to repositoryRoot before passing it to
IsGeneratedPath. Keep FindCredentialViolations using the original file path, and
ensure generated-directory detection only examines repository-relative segments.
---
Outside diff comments:
In `@README.nuget.md`:
- Around line 94-110: Update the README configuration example to match the
current appsettings.json placeholder conventions: replace the concrete
PayBridgeDbContext connection string, integer EnabledGateways values, and test
gateway keys with the standardized YOUR_ placeholders expected by the SDK
auto-ignore logic. Keep the existing configuration structure and gateway entries
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 216b5636-c133-4f93-8f9e-6cb780bf15d8
📒 Files selected for processing (11)
.github/workflows/secret-scanning.ymlPayBridge.SDK.Example/PayBridge.SDK.Example.csprojPayBridge.SDK.Example/Program.csPayBridge.SDK.Example/README.mdPayBridge.SDK.Example/appsettings.Development.jsonPayBridge.SDK.Example/appsettings.jsonPayBridge.SDK.Presentation/Program.csPayBridge.SDK.Presentation/appsettings.jsonPayBridge.SDK.Test/Unit/RepositorySecurityTests.csREADME.mdREADME.nuget.md
| - name: Checkout repository | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Prevent credential persistence in the checkout step.
As flagged by static analysis, configuring persist-credentials: false prevents the GITHUB_TOKEN from being persisted in the local Git config, mitigating the risk of credential exposure if subsequent steps are compromised or if the .git directory is archived. Since this workflow only performs scanning and does not push changes, it does not need the token to persist.
🛡️ Proposed fix to explicitly disable credential persistence
- name: Checkout repository
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout repository | |
| uses: actions/checkout@v4 | |
| - name: Checkout repository | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 24-25: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/secret-scanning.yml around lines 24 - 25, Update the
actions/checkout step in the secret-scanning workflow to explicitly disable
credential persistence with the supported checkout configuration, while
preserving the existing repository checkout behavior.
Source: Linters/SAST tools
| var violations = Directory | ||
| .EnumerateFiles(repositoryRoot, "appsettings*.json", SearchOption.AllDirectories) | ||
| .Where(path => !IsGeneratedPath(path)) | ||
| .SelectMany(path => FindCredentialViolations(repositoryRoot, path)) | ||
| .ToList(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent false-positive path filtering due to repository location.
Passing the absolute file path to IsGeneratedPath means that if the repository itself is cloned into a directory structure containing a folder named bin or obj (for example, /home/user/bin/PayBridge.SDK/), IsGeneratedPath will inadvertently return true for all files. This effectively skips the entire credential scan test.
Evaluate the path relative to the repository root before checking for generated directories.
🐛 Proposed fix to evaluate the relative path
var violations = Directory
.EnumerateFiles(repositoryRoot, "appsettings*.json", SearchOption.AllDirectories)
- .Where(path => !IsGeneratedPath(path))
+ .Where(path => !IsGeneratedPath(Path.GetRelativePath(repositoryRoot, path)))
.SelectMany(path => FindCredentialViolations(repositoryRoot, path))
.ToList();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var violations = Directory | |
| .EnumerateFiles(repositoryRoot, "appsettings*.json", SearchOption.AllDirectories) | |
| .Where(path => !IsGeneratedPath(path)) | |
| .SelectMany(path => FindCredentialViolations(repositoryRoot, path)) | |
| .ToList(); | |
| var violations = Directory | |
| .EnumerateFiles(repositoryRoot, "appsettings*.json", SearchOption.AllDirectories) | |
| .Where(path => !IsGeneratedPath(Path.GetRelativePath(repositoryRoot, path))) | |
| .SelectMany(path => FindCredentialViolations(repositoryRoot, path)) | |
| .ToList(); |
🤖 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.Test/Unit/RepositorySecurityTests.cs` around lines 30 - 34,
Update the file-filtering flow in the repository security test to compute each
file’s path relative to repositoryRoot before passing it to IsGeneratedPath.
Keep FindCredentialViolations using the original file path, and ensure
generated-directory detection only examines repository-relative segments.
Summary
TDD
RepositorySecurityTests.Sample_configuration_must_not_contain_committed_credentials; it failed first on the Presentation sample and then exposed the same defect in the Example configurationSenior .NET review
UserSecretsIdto the Example projectProvider guidance reviewed
Validation
dotnet build PayBridge.SDK.sln --configuration Release(passes; one pre-existing nullable warning tracked in build: resolve remaining nullable warning and enforce warning-free builds #70)dotnet test PayBridge.SDK.sln --configuration Release --no-restore(91 passed, 2 skipped)dotnet format ... whitespace --verify-no-changesgit diff --checkSecurity follow-up
This PR removes credentials from the current tree and prevents recurrence. It cannot rotate provider credentials or safely purge shared Git history. A maintainer with Paystack, Flutterwave, and Stripe dashboard access must rotate the exposed credentials immediately; history cleanup should be coordinated separately because it rewrites shared commits.
Refs #60
Summary by CodeRabbit
Security
Configuration
Documentation
Tests