Skip to content

Full security and code quality audit fixes#733

Open
GaltRanch wants to merge 2 commits intocurly60e:masterfrom
GaltRanch:audit/full-security-quality-fix
Open

Full security and code quality audit fixes#733
GaltRanch wants to merge 2 commits intocurly60e:masterfrom
GaltRanch:audit/full-security-quality-fix

Conversation

@GaltRanch
Copy link
Copy Markdown
Contributor

Summary

  • Eliminate ~95 command injection vectors (shell=True with user input/interpolation) across ppi.py and SPV/spvblock.py, replacing with requests library or safe list-form subprocess
  • Fix 63 bare except clauses with specific exception types + structured logging across 10 files
  • Harden crypto operations: random.randint/choice β†’ secrets module; add input validation (fiat allowlist, IP validation); path traversal prevention in config; remove token exposure from stdout
  • Fix resource leaks & race conditions: context managers for file handles, threading.Lock for shared state in clock/data.py, timeout=10 on ~50 requests calls, bounded list growth
  • Deduplicate 102 repeated code blocks: _load_macaroon() (69x in PyBlock.py), _load_lnd_config() (33x in nodeconnection.py + 2x in PyBlock.py)
  • Add .conf.example templates for safe onboarding without credential exposure

Files changed: 21 | +1057 / -725 lines

Test plan

  • Verify python3 -m py_compile passes on all 18 modified .py files (verified locally)
  • Test Bitcoin Core RPC connection via pyblock main menu
  • Test LND gRPC connection and channel listing
  • Test SPV mode startup and API calls
  • Test AI assistant chat flow (Astrolexis)
  • Test rateSXList / rateSXGraph with valid fiat code (e.g., USD)
  • Test LNBits/OpenNode payment flows in SPV
  • Verify clock/data.py background fetches still work without race conditions

πŸ€– Generated with Claude Code

Security (Critical):
- Eliminate all shell=True command injection vectors (~95 instances in ppi.py, spvblock.py)
- Replace subprocess curl calls with requests library
- Add input validation (fiat code allowlist, IP address validation)
- Replace weak random.randint/choice with secrets module for crypto ops
- Remove token/credential exposure from print statements
- Add path traversal prevention in config.py
- Create .conf.example templates, scrub local credentials

Stability:
- Replace 63 bare except clauses with specific exceptions + logging
- Fix file handle leaks with context managers (lnd.py, apisnd.py)
- Add threading.Lock for race conditions in clock/data.py
- Cap unbounded list growth (MAX_HISTORY_LEN=50)
- Add timeout=10 to ~50 requests calls missing timeouts

Maintainability:
- Extract _load_macaroon() helper (dedup 69 instances in PyBlock.py)
- Extract _load_lnd_config() helper (dedup 33 instances in nodeconnection.py)
- Normalize json import (simplejson with stdlib fallback)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @GaltRanch, your pull request is larger than the review limit of 150000 diff characters

…conditions

- Replace all shell=True subprocess calls with Python-native processing
  (nodeconnection.py, SPV/nodeconnection.py, SPV/ppi.py)
- Mask sensitive inputs (private keys, passwords, tokens) with getpass
- Add threading.Lock to block_explorer.py shared state
- Use json.loads() instead of fragile string splitting in apisnd.py
- Add path validation before file open in apisnd.py
- Replace random.randint with secrets.randbelow for mining nonces
- Fix destructive exception handlers in clone.py and feed.py
- Replace bare except clauses with specific exceptions + logging
- Remove unused imports (psutil, xmltodict, block_visualizer, base64, say)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant