feat: Add Temporary Key Caching System with Read-Only Client Support#241
Conversation
WalkthroughThis change introduces a read-only mode for account retrieval and client instantiation, refactors key management to cache decrypted keys in temporary files, and adds robust temporary file lifecycle management. It updates the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI_Command
participant BaseAction
participant ConfigFileManager
participant GenLayerClient
User->>CLI_Command: Initiate contract call (read-only)
CLI_Command->>BaseAction: getClient(rpc, readOnly=true)
BaseAction->>ConfigFileManager: getTempFile(TEMP_KEY_FILENAME)
alt Temp file exists and valid
ConfigFileManager-->>BaseAction: Return cached private key
else
BaseAction->>ConfigFileManager: Read keystore file
alt readOnly
BaseAction-->>CLI_Command: Return address only
else
BaseAction->>User: Prompt for password
BaseAction->>ConfigFileManager: storeTempFile(TEMP_KEY_FILENAME, privateKey)
end
end
BaseAction->>GenLayerClient: Instantiate client (readOnly)
GenLayerClient-->>CLI_Command: Ready for use
Estimated code review effort3 (~45 minutes) Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/commands/contracts/call.ts(1 hunks)src/lib/actions/BaseAction.ts(4 hunks)src/lib/config/ConfigFileManager.ts(3 hunks)tests/actions/appeal.test.ts(1 hunks)tests/actions/call.test.ts(1 hunks)tests/actions/deploy.test.ts(1 hunks)tests/actions/receipt.test.ts(1 hunks)tests/actions/write.test.ts(1 hunks)tests/libs/baseAction.test.ts(5 hunks)tests/libs/configFileManager.test.ts(2 hunks)
🧬 Code Graph Analysis (2)
tests/libs/configFileManager.test.ts (1)
src/lib/config/ConfigFileManager.ts (1)
ConfigFileManager(10-124)
src/lib/actions/BaseAction.ts (2)
src/lib/config/ConfigFileManager.ts (1)
ConfigFileManager(10-124)src/lib/interfaces/KeystoreData.ts (1)
KeystoreData(1-5)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/libs/configFileManager.test.ts (1)
src/lib/config/ConfigFileManager.ts (1)
ConfigFileManager(10-124)
src/lib/actions/BaseAction.ts (2)
src/lib/config/ConfigFileManager.ts (1)
ConfigFileManager(10-124)src/lib/interfaces/KeystoreData.ts (1)
KeystoreData(1-5)
🔇 Additional comments (28)
src/lib/config/ConfigFileManager.ts (8)
5-8: LGTM: Clean interface design for temporary file data.The
TempFileDatainterface appropriately encapsulates the content and timestamp needed for expiration tracking.
13-14: LGTM: Well-defined constants for temporary file management.The temp folder path using OS temp directory and 5-minute expiration time are appropriate choices for balancing security and usability.
19-22: LGTM: Proper initialization of temporary folder system.The constructor correctly initializes the temp folder path and ensures folder existence. Good integration with existing initialization flow.
37-41: LGTM: Secure temporary folder creation with owner-only permissions.The method correctly creates the temp folder with restrictive permissions (0o700) to prevent unauthorized access. Good security practice.
67-75: LGTM: Secure temporary file storage implementation.The method properly:
- Ensures temp folder exists
- Creates timestamped data structure
- Uses secure file permissions (0o600) for owner-only access
- Handles JSON serialization correctly
77-93: LGTM: Robust temporary file retrieval with expiration handling.The method correctly:
- Handles non-existent files gracefully
- Parses JSON data safely
- Implements expiration logic with automatic cleanup
- Returns null for expired files
95-97: LGTM: Simple and efficient existence check.The method leverages the existing
getTempFilemethod, which already handles expiration checking.
99-104: LGTM: Safe temporary file cleanup.The method properly checks file existence before attempting deletion to avoid errors.
tests/actions/write.test.ts (1)
22-22: LGTM: Test correctly updated to reflect BaseAction API refactor.The change from mocking
getPrivateKeytogetAccountaligns with the refactor in BaseAction where account retrieval now returns an object containing the private key rather than the key directly.tests/actions/appeal.test.ts (1)
24-24: LGTM: Consistent test update for BaseAction API refactor.The mock update properly reflects the change from
getPrivateKeytogetAccountmethod, maintaining test functionality while adapting to the new API.tests/actions/receipt.test.ts (1)
26-26: LGTM: Test properly aligned with BaseAction API changes.The mock update maintains consistency with the refactored BaseAction class interface while preserving test functionality.
tests/actions/deploy.test.ts (1)
29-29: LGTM: Final test file consistently updated for API refactor.The mock change completes the consistent update across all action test files to work with the new
getAccountmethod signature.src/commands/contracts/call.ts (1)
24-24: LGTM: Correctly implements read-only client for contract calls.This change appropriately sets the
readOnlyparameter totruefor contract call operations, which eliminates unnecessary password prompts since read operations don't require private key access.tests/actions/call.test.ts (1)
24-24: LGTM: Test correctly updated to reflect BaseAction refactor.The change from mocking
getPrivateKeytogetAccountwith an object return value properly aligns with the BaseAction refactor wheregetPrivateKeywas replaced bygetAccountreturning an account object.tests/libs/baseAction.test.ts (5)
9-22: LGTM: Comprehensive mock setup for new dependencies.The addition of mocks for
fs,os, andgenlayer-jsproperly supports testing the new temporary file handling and OS path functionality introduced in the BaseAction refactor.
60-90: LGTM: Well-structured temp file method mocking.The mock setup for temporary file methods (
storeTempFile,getTempFile,clearTempFile,cleanupExpiredTempFiles) provides comprehensive coverage for testing the new caching functionality.
252-268: LGTM: Test correctly updated for getAccount method.The test properly verifies the new
getAccountmethod returns an account object with a private key, and validates the expected file system interactions.
262-268: LGTM: Excellent test coverage for read-only functionality.This test correctly verifies that when
getAccountis called withreadOnly=true, it returns only the address without requiring private key decryption, which is the core functionality for eliminating password prompts on read operations.
293-301: LGTM: Critical test for caching functionality.This test verifies that cached keys are properly retrieved and reused, avoiding repeated password prompts. The test correctly mocks
getTempFileand verifies thatinquirer.promptis not called when a cached key exists.tests/libs/configFileManager.test.ts (1)
113-290: LGTM: Comprehensive test coverage for temporary file functionality.The new test suite thoroughly covers all aspects of the temporary file system:
- File creation with proper timestamps and permissions (mode 0o600)
- Temp folder creation with secure permissions (mode 0o700)
- Expiration logic with 5-minute timeout
- Retrieval of valid and expired files
- Cleanup of expired files while preserving valid ones
- Edge cases for non-existent files and folders
The tests properly mock Date.now() to control timestamp behavior and verify the security model with owner-only file permissions.
src/lib/actions/BaseAction.ts (8)
14-17: LGTM: Well-defined constants improve maintainability.The introduction of constants for default paths, limits, and filenames eliminates magic numbers and makes the codebase more maintainable and configurable.
25-25: LGTM: Proactive cleanup on initialization.Calling
cleanupExpiredTempFiles()in the constructor ensures stale temporary files are removed when the CLI starts, maintaining good hygiene for the temporary storage system.
36-37: LGTM: Secure temporary key caching implementation.The caching of decrypted private keys after successful decryption is well-implemented. The temporary file system uses secure permissions (0o600) and 5-minute expiration as defined in ConfigFileManager, balancing security with user convenience.
64-76: LGTM: Clean read-only client implementation.The
getClientmethod properly handles the newreadOnlyparameter and delegates account retrieval to the newgetAccountmethod. The logic maintains backward compatibility with the defaultreadOnly=falseparameter.
78-108: LGTM: Robust account retrieval with caching and read-only support.The
getAccountmethod excellently implements the core functionality:
- Read-only optimization: Returns only the address for read operations (lines 99-101)
- Smart caching: Checks for cached decrypted keys before prompting for passwords (lines 104-106)
- Fallback handling: Creates new keypairs when files are missing or invalid
- Address validation: Ensures cached keys match the current keystore address through the validation in ConfigFileManager
The method properly balances security (temporary file expiration, secure permissions) with user experience (eliminating redundant password prompts).
110-112: LGTM: Simple and effective address extraction.The
getAddresshelper method provides a clean way to extract the address from keystore data for read-only operations, with proper type casting to theAddresstype.
149-149: LGTM: Proper cache invalidation on keypair creation.Clearing the temporary decrypted key file when creating a new keypair ensures that stale cached keys don't persist after keypair changes, maintaining security and consistency.
133-135: LGTM: Consistent use of constants for validation.Using the
MIN_PASSWORD_LENGTHconstant in both the validation logic and error message ensures consistency and makes the password policy easily configurable.
cristiam86
left a comment
cristiam86
left a comment
There was a problem hiding this comment.
@epsjunior minor comment about the permissions in the temp file
2 participants
Add Temporary Key Caching System with Read-Only Client Support
Summary
Implements a temporary file caching system for decrypted private keys and introduces read-only client support to eliminate unnecessary password prompts. This enhancement addresses the inconvenience of requiring keystore passwords for read operations and provides automatic key caching for write operations, significantly improving developer experience and workflow efficiency.
Changes
🚀 New Features
readOnlyparameter togetClient()to support contract reads without password prompts🎯 Problem Solved
Issue: The GenLayer CLI was asking for keystore password to unlock the wallet for reading contracts, which is inconvenient and should only be required when calling write operations.
Solution:
🔧 Infrastructure Updates
os.tmpdir()for temporary file storage following OS conventions🏗️ Implementation Details
📁 File Structure
🔐 Security Features
0o600(owner read/write only)🧪 Testing
ConfigFileManager Tests (
tests/libs/configFileManager.test.ts):BaseAction Tests (
tests/libs/baseAction.test.ts):Action Tests (Various action test files):
getAccountmethodcreateAccountfunction🔧 Usage
Read Operations (NEW - No Password Required)
Write Operations (First Time - Password Required)
Session Management (Auto-Expiration)
✨ Code Quality
🎯 Authentication Flow
getClient(rpcUrl, readOnly: true)→ Address-only client → No password neededgetClient(rpcUrl, readOnly: false)→ Full client → Password/cached key required📊 Performance Benefits
🛡️ Security Considerations
🔗 Dependencies
ConfigFileManagerandBaseActionpatternsos.tmpdir()for cross-platform supportfsmodule operations with proper error handling🛡️ Backward Compatibility
Testing: ✅ All tests pass with 100% coverage on new code
Security: ✅ Secure file permissions, address validation, and read-only safety
Performance: ✅ Zero authentication for reads, reduced authentication for writes
Cross-Platform: ✅ Works on Unix, macOS, and Windows
User Experience: ✅ Major workflow improvement eliminating unnecessary password prompts
Future-Proof: ✅ Generic temp file system for extensibility
Summary by CodeRabbit
New Features
Improvements
Bug Fixes