Skip to content

Conversation

@BuilderFred
Copy link
Contributor

Implements a data export tool for network analytics and reporting. Supports CSV, JSON, and JSONL formats for miners, epochs, balances, and attestations.

Copy link
Owner

@Scottcjn Scottcjn left a comment

Choose a reason for hiding this comment

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

🔧 Changes Requested - Data Export Pipeline

@BuilderFred Good feature but security concerns need addressing.

Critical Security Issues:

  1. No Authentication

    • Export tool can dump all wallet balances with no auth
    • Add admin key requirement:
    admin_key = os.environ.get("RC_ADMIN_KEY")
    if not admin_key:
        print("Error: RC_ADMIN_KEY required for export")
        sys.exit(1)
  2. Unencrypted Output

    • Financial data (balances, ledger) written as plaintext
    • Add --encrypt flag using AES-256-GCM (like wallet does)
  3. Memory Issue with Large Datasets

    • writer.writerows(data) loads all into memory
    • Use streaming for CSV:
    for row in cursor:
        writer.writerow(row)
  4. CSV Formula Injection

    • Malicious wallet IDs like =cmd|'/c calc' could exploit Excel
    • Add: csv.QUOTE_ALL or sanitize fields starting with =+-@
  5. Privacy: Hardware Fingerprints Exposed

    • Exports device_arch, device_family linked to miner IDs
    • Add --anonymize flag to hash miner IDs

Recommended:

  1. Add Audit Logging

    • Log who exported what, when
  2. API Timeout

    • 10-second timeout may be too short for large exports
    • Make configurable: --timeout 60

Homework:

  • Add RC_ADMIN_KEY authentication
  • Add --encrypt flag for output
  • Fix memory issue (streaming writes)
  • Sanitize CSV fields for formula injection
  • Add --anonymize flag
  • Add audit logging
  • Make timeout configurable

The export functionality is useful - just needs security hardening for production use.

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 3, 2026

@BuilderFred - Security issues still not addressed:

❌ No authentication (RC_ADMIN_KEY check)
❌ No encryption option for output
❌ No memory streaming for large datasets
❌ No CSV injection protection

This exports sensitive financial data - security is mandatory. Push fixes and ping when ready.

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 3, 2026

👋 @BuilderFred — Data export PR needs security fixes before merge:

  1. ❌ Add RC_ADMIN_KEY authentication check
  2. ❌ Add encryption option for sensitive exports
  3. ❌ Stream large datasets (memory safety)
  4. ❌ CSV injection protection (escape =, +, -, @ prefixes)

These are security requirements — let me know if you need code examples!

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.

2 participants