security: harden credential handling, SSRF protection, and key storage#1
Open
orsharon7 wants to merge 1 commit into
Open
security: harden credential handling, SSRF protection, and key storage#1orsharon7 wants to merge 1 commit into
orsharon7 wants to merge 1 commit into
Conversation
- Mask password fields in GET /api/integrations/[provider] so decrypted credentials are never returned over the wire; edit form uses blank=keep logic that was already in the POST handler - Update bank edit form to allow blank password fields in edit mode and show "Leave blank to keep current" placeholder - Validate provider against BANK_PROVIDERS in POST /api/setup/bank to reject unknown provider strings before touching the DB - Validate ollamaUrl is a localhost URL in both PUT /api/settings and POST /api/setup/ai to block SSRF via a crafted Ollama URL - Move encryption key from data/.encryption-key to ~/.config/spent/.encryption-key so a copy of the data directory alone cannot decrypt stored credentials; migrates existing keys automatically - Warn in the Show Browser setting that verbose logs may contain bank credentials Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
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.
Summary
This PR fixes several security issues identified during a review of the credential handling flow. The app stores bank credentials and API keys, so these are high-priority hardening items.
GET /api/integrations/[provider]was returning fully decrypted credentials including passwords. Now returns empty strings for password-type fields. The edit form already had blank=keep logic in the POST handler, so no functionality is lost. Password inputs show "Leave blank to keep current" as placeholder in edit mode, andallValidallows blank passwords when editing.POST /api/setup/banknow rejects unknownprovidervalues (400) before touching the database.PUT /api/settingsandPOST /api/setup/ainow validate thatollamaUrlis a localhost URL before storing it. Previously any URL could be saved and the server would make outbound requests to it.data/.encryption-key, the same directory asdata/spent.db. Anyone with a copy of that directory could decrypt all credentials. The key now lives at~/.config/spent/.encryption-key. Existing keys are migrated automatically on first startup.Test plan
/api/setup/bankwith an unknown provider name -- should return 400 "Unknown provider"/api/settingswithollamaUrlpointing to a non-localhost address -- should return 400~/.config/spent/.encryption-keyis created on first run (mode 600) anddata/.encryption-keyis absent🤖 Generated with Claude Code