Skip to content

Redact Private Key from CreateWallet API Response#521

Closed
Sankara-Jefferson wants to merge 42 commits intomainfrom
pr-517
Closed

Redact Private Key from CreateWallet API Response#521
Sankara-Jefferson wants to merge 42 commits intomainfrom
pr-517

Conversation

@Sankara-Jefferson
Copy link
Contributor

Summary

This PR redacts the PrivateKey field from the response of the /wallet/create API endpoint to prevent accidental key exposure.

What Was Done

  • CreateHandler: removed PrivateKey from response payload by explicitly clearing the field before return.
  • Added a test case CreateWallet_privateKeyShouldBeRedacted to assert that PrivateKey is empty in the response.
  • Verified that all tests pass locally via go test ./api -v.

Rationale

Previously, the PrivateKey was included in plaintext in API responses. While this endpoint is useful for testing, returning secrets by default is a major security risk. This patch minimizes attack surface and sets groundwork for further key safety improvements.

Follow-ups (to be tracked)

  • Encrypt private keys at rest (symmetric key or KMS)
  • Add secure export flow (wallet export)
  • Audit other wallet-related endpoints for similar redaction
  • Add key handling docs to GitBook

Status

Ready for merge after CI passes.

Copy link
Collaborator

@ianconsolata ianconsolata left a comment

Choose a reason for hiding this comment

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

LGTM, though it seems like there are some merge conflicts to resolve

zachfedor and others added 15 commits October 28, 2025 13:48
- Add init function to create full current schema on brand new clean
databases and run missing migrations on any databases using old
auto-migrate strategy with existing data.
- Add CLI commands to migrate up, down, or to a specified version by ID
- Add utility functions to get list of migration IDs ran on current
database, check if migration has run by ID, etc.
Fix struct tag ordering in handler/storage/types_gen.go to match
the current code generator output. This resolves CI failures where
go generate was detecting differences between committed and generated files.
Merging this now — all tests have passed.
Add Wallet Create Handler + Migrate to Numeric Wallet ID.

Merging as all local test passed. The go version error will be addressed on a separate issue.
@parkan
Copy link
Collaborator

parkan commented Oct 28, 2025

this, in addition to not having a sane history, is only relevant with wallet creation functionality

closing this and will be mindful of this issue when re-implementing wallets

@parkan parkan closed this Oct 28, 2025
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.

9 participants