chore: update passphrase to file rather than string in cli#146
Conversation
WalkthroughReplaces direct CLI passphrase arguments with file-based passphrases across evstack and evmsingle nodes: adds passphraseFilePath() helpers, writes configured passphrases to Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Harness
participant Node as Node (evstack / evmsingle)
participant FS as Filesystem
participant Container as evnode Container
Note over Node,FS: Init / initContainer flow (new)
Test->>Node: call Init/initContainer(...)
Node->>FS: write "config/passphrase.txt" with passphrase
alt write succeeds
Node->>Container: start with --*.signer.passphrase_file=/path/config/passphrase.txt
Container-->>Node: started
Node-->>Test: success
else write fails
FS-->>Node: error
Node-->>Test: return error (formatted)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)framework/docker/evstack/evmsingle/node.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (4)
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. Comment |
![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 (1)
framework/docker/evstack/node.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
framework/docker/evstack/node.go (2)
framework/docker/evstack/reth/node.go (1)
Node(23-31)framework/docker/container/node.go (1)
Node(20-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
framework/docker/evstack/node.go (3)
96-99: LGTM! Clean abstraction for passphrase file path.The helper method provides a consistent, cross-platform path for the passphrase file across both initialization and startup commands.
115-115: LGTM! Correct flag usage with proper sequencing.The passphrase file flag correctly references the file path, and the preceding error handling ensures the file exists before this command executes.
159-159: <!-- [scratchpad_start]
[task review_file_1/1 review_segment_4/4]
Checking Build() and Chain.Start() to see if Init() is called[observations]
- Build() method calls initializeNodes() -> newNode() but NO Init() call
- Build() only creates nodes and sets up volumes (CreateAndSetupVolume)
- Init() is NOT called during Build()
- Need to check Chain.Start() to see if it calls node.Init()
[analysis]
- Build() does not initialize nodes
- The concern is getting stronger - need to verify if Chain.Start() calls node.Init()
- If Chain.Start() does NOT call node.Init(), then the review comment is correct
[pending]
- Find Chain.Start() method
- Verify if it calls node.Init() before starting nodes
[actions]
Search for Chain.Start() method in evstack package
| if err := n.WriteFile(ctx, filepath.Join("config", "passphrase.txt"), []byte(n.cfg.AggregatorPassphrase)); err != nil { | ||
| return fmt.Errorf("failed to write passphrase file: %w", err) | ||
| } |
There was a problem hiding this comment.
Set restrictive file permissions when writing the passphrase.
The passphrase file should be created with restrictive permissions (e.g., 0600) to prevent unauthorized access, even in test environments. Currently, the file is written with default permissions which may allow other users or processes to read the sensitive passphrase.
Check if the WriteFile method supports setting permissions. If it does, apply this pattern:
- if err := n.WriteFile(ctx, filepath.Join("config", "passphrase.txt"), []byte(n.cfg.AggregatorPassphrase)); err != nil {
+ if err := n.WriteFile(ctx, filepath.Join("config", "passphrase.txt"), []byte(n.cfg.AggregatorPassphrase), 0600); err != nil {
return fmt.Errorf("failed to write passphrase file: %w", err)
}If WriteFile doesn't support permission parameters, consider using a different method or ensuring the container's umask is set appropriately.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In framework/docker/evstack/node.go around lines 108 to 110, the passphrase file
is written with default permissions which may be world-readable; change the
write to create the file with restrictive permissions (0600). First check if
n.WriteFile accepts a file mode argument and, if so, pass os.FileMode(0o600). If
n.WriteFile does not support permissions, replace the call with a write that
allows setting mode (e.g., os.WriteFile or creating the file with os.OpenFile
and os.FileMode(0o600)), or explicitly set the file mode right after writing
using os.Chmod(ctxPath, 0o600); ensure the final file is created with permission
0600 to protect the passphrase.
2 participants
Overview
The test will fail in this repo until evstack/ev-node#2764 is merged, however I ran locally and have passing tests in both repos with this fix.
Summary by CodeRabbit