feat(wallet): migrate Uphold OAuth secrets to gitignored Uphold-Info.plist#798
feat(wallet): migrate Uphold OAuth secrets to gitignored Uphold-Info.plist#798romchornyi wants to merge 5 commits into
Conversation
…ret-scan guardrails Remove the redundant Coinbase CLIENT_ID/CLIENT_SECRET from project.pbxproj build settings and both Info.plist files. These were never read at runtime (Coinbase reads gitignored Coinbase-Info.plist) yet sat committed as a leak vector. Add defense-in-depth against future secret leaks: - CI secret scan (.github/workflows/gitleaks.yml) — non-bypassable backstop - local pre-commit hook (.githooks/pre-commit) — opt-in via core.hooksPath - gitleaks config/allowlist (.gitleaks.toml, .gitleaksignore) - Secrets Management rule documented in CLAUDE.md Note: leaked credentials already in git history must still be rotated; this only stops re-introduction.
…fo.plist Removes the Release build-phase swap that copied DWUpholdMainnetConstants__Release.m over the tracked constants file — the root cause of the PR-797 Uphold credential leak. Mainnet client_id / client_secret / authorize URL now read from a gitignored Uphold-Info.plist at runtime (same pattern as Coinbase / ZenLedger / Topper). The previously hardcoded sandbox client_secret is likewise moved out of source into the plist under SANDBOX_CLIENT_SECRET. No secret value remains in any tracked file. - Remove "Uphold Constants" / "Cleanup Uphold Constants" run-script build phases from both dashwallet and dashpay targets - Remove the clang-format special-case for DWUpholdMainnetConstants.m - Delete the now-unused DWUpholdMainnetConstants__Empty.m - Add **/Uphold-Info.plist to .gitignore; add the plist to Copy Bundle Resources - DWUpholdMainnetConstants.m + DWUpholdConstants.m read credentials from the plist - Update CLAUDE.md Secrets Management to reference Uphold-Info.plist Credentials were rotated during incident response; this removes the mechanism so the leak cannot recur.
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change adds local and CI gitleaks enforcement, removes tracked Uphold credentials, loads runtime values from ChangesSecret handling and Uphold configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant DWUpholdConstants
participant UpholdInfoPlist
App->>DWUpholdConstants: request client ID or secret
DWUpholdConstants->>UpholdInfoPlist: read keyed value from bundle
UpholdInfoPlist-->>DWUpholdConstants: return plist string or empty string
DWUpholdConstants-->>App: return configuration value
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/gitleaks.yml:
- Around line 17-20: Update the actions/checkout@v4 step to explicitly disable
credential persistence by setting persist-credentials to false, while preserving
fetch-depth: 0 for the full-history secret scan.
In @.gitleaks.toml:
- Around line 22-27: Update the hardcoded-hex-secret-literal regex in
.gitleaks.toml to use the inline case-insensitive flag (?i), while preserving
the existing 40–64 character length bounds and assignment/return patterns.
🪄 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: d7b143ab-0b00-4321-a688-3e54c7d714d1
📒 Files selected for processing (12)
.githooks/pre-commit.github/workflows/gitleaks.yml.gitignore.gitleaks.toml.gitleaksignoreCLAUDE.mdDWUpholdMainnetConstants__Empty.mDashPay/dashpay-info.plistDashWallet.xcodeproj/project.pbxprojDashWallet/Info.plistDashWallet/Sources/Models/Uphold/DWUpholdConstants.mDashWallet/Sources/Models/Uphold/DWUpholdMainnetConstants.m
💤 Files with no reviewable changes (3)
- DashWallet/Info.plist
- DashPay/dashpay-info.plist
- DWUpholdMainnetConstants__Empty.m
…nning
The gitleaks/gitleaks-action@v2 wrapper now requires a paid license for organization repos, so on the dashpay org it failed on every run ("missing gitleaks license") without ever scanning. Remove the CI workflow and rely on the local .githooks/pre-commit gitleaks hook instead.
- Delete .github/workflows/gitleaks.yml
- Update CLAUDE.md Secrets Management to document local-only enforcement (pre-commit hook), noting the CI scan was intentionally dropped
…tive
The hardcoded-hex-secret-literal rule matched only lowercase [0-9a-f], so an uppercase hex secret (e.g. return @"DA72F…") would slip through undetected. Add the (?i) flag so hex literals are caught regardless of casing. 24-char pbxproj UUIDs remain excluded by the {40} / {40,64} length bounds, not the character class (verified: uppercase and lowercase 40-hex literals flagged, 24-char UUID not).
|
@coderabbitai review |
✅ Action performedReview finished.
|
3 participants
Issue being fixed or feature implemented
This change removes the mechanism that caused the Uphold credential leak in PR-797.
The old Release build-phase swap copied
DWUpholdMainnetConstants__Release.mover the trackedDWUpholdMainnetConstants.m, which meant a Release build could rewrite the source file with real secrets. This migration moves Uphold OAuth credentials to a gitignored runtime plist so the tracked source no longer contains or receives secret values.What was done?
DWUpholdMainnetConstants.mso Uphold mainnet client_id, client_secret, and authorizeURLFormat are read from a gitignored Uphold-Info.plist at runtime.How Has This Been Tested?
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Security
Build & Configuration