-
Notifications
You must be signed in to change notification settings - Fork 19
Remove private keys from tests #833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
btw, had a quick look and don't think this is going on the right track.. we're changing a lot of things unrelated with private keys... hard coding RPCS for instance, is one.. but there are others... |
Ok thank you Paulo, but the purpose was to no expose private keys in the code, now we place them in a separate file... it is a bit confusing, I remeber Alex said to not share private keys anywhere, we can have them as env vars, but it is not clear the scope of the issue then. |
alexcos20
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AI automated code review (Gemini 3).
Overall risk: medium
Summary:
This pull request significantly enhances the security of the test suite by removing numerous hardcoded private keys directly embedded within various integration and unit test files. The approach now leverages environment variables (e.g., process.env.ANOTHER_WALLET_PRIVATE_KEY) and a Blockchain abstraction for signer management, centralizing and making key usage more secure. Test environment configuration is also streamlined through TEST_ENV_CONFIG_FILE updates. While the primary goal of removing hardcoded keys from test logic has been successfully achieved, a new private key was added to the src/test/.env.test file, continuing an existing pattern of storing test keys within the repository.
Comments:
• [WARNING][security] While this PR successfully removes hardcoded private keys from test files, the addition of ANOTHER_WALLET_PRIVATE_KEY to src/test/.env.test continues a pattern of committing private keys (even for test purposes) to the repository. It's generally best practice to avoid committing any private keys to version control. For test environments, consider generating ephemeral keys on the fly, or ensuring all private keys are sourced from truly external, .gitignored files, or securely managed CI secrets. This helps maintain a strict 'no private keys in repo' policy.
• [INFO][other] Noticed the change in the RPCS environment variable for the CI workflow. It now points only to the development chain (8996). This change helps to simplify the CI setup for tests, which is a good operational improvement.
Fixes #402 .
Changes proposed in this PR: