fix(cli): read OPENCODE_SERVER_USERNAME in attach command#18646
fix(cli): read OPENCODE_SERVER_USERNAME in attach command#18646kevinWangSheng wants to merge 1 commit intoanomalyco:devfrom
Conversation
The attach command hardcoded "opencode" as the Basic auth username, ignoring the OPENCODE_SERVER_USERNAME environment variable. This broke authentication for any server configured with a custom username. Fixes anomalyco#18611
|
The following comment was made by an LLM, it may be inaccurate: Found 1 potential duplicate:
These PRs are likely duplicates or one may have been created before the other was merged. You should check which one is more recent or if one has already been merged to |
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
atharvau
left a comment
There was a problem hiding this comment.
Code Review for PR #18646: CLI Username Fix
✅ Overall Assessment: APPROVED
This is a small but important fix that makes authentication more configurable by allowing custom usernames via environment variables.
🔍 Changes Summary
File: packages/opencode/src/cli/cmd/tui/attach.ts
- Adds support for
OPENCODE_SERVER_USERNAMEenvironment variable - Falls back to "opencode" as the default username (preserving existing behavior)
✅ What's Good
- Backwards Compatible: Uses "opencode" as fallback, maintaining existing behavior
- Simple & Clear: Single-purpose change that's easy to understand
- Consistent: Follows same pattern as
OPENCODE_SERVER_PASSWORD - Security Conscious: Allows custom usernames without hardcoding
🔧 Code Quality
- Clean Implementation: Uses nullish coalescing operator appropriately
- No Side Effects: Change is isolated to authentication logic
- Proper Scoping: Variable is declared in appropriate scope
📋 Minor Suggestions (Optional)
Consider adding this to documentation if there's a CLI reference guide, so users know about the environment variable option.
🚀 Recommendation: APPROVE
This is a good, focused improvement that adds flexibility without breaking existing functionality. The implementation is clean and follows established patterns.
Rating: ⭐⭐⭐⭐⭐
atharvau
left a comment
There was a problem hiding this comment.
Code Review Summary
Overall Assessment: ✅ APPROVE
Simple security improvement for configurable authentication.
✅ Strengths:
- Security Enhancement: Allows custom username via environment variable
- Backward Compatible: Falls back to "opencode" if not specified
- Clean Implementation: Minimal, focused change
🟡 Observations:
- Identical implementation to PR #18641 (duplicate fix?)
- No validation on username format/content
- Environment variable not documented in the change
Security Analysis:
- ✅ No injection vulnerabilities (Base64 encoding used)
- ✅ Secure fallback behavior
- ✅ Standard Basic Auth pattern
Minor Suggestion: Consider adding input validation for if it accepts user input.
Recommendation: Ready to merge
atharvau
left a comment
There was a problem hiding this comment.
Code Review Summary
Overall Assessment: ✅ APPROVE
Simple security improvement for configurable authentication.
✅ Strengths:
- Security Enhancement: Allows custom username via environment variable
- Backward Compatible: Falls back to 'opencode' if not specified
- Clean Implementation: Minimal, focused change
🟡 Observations:
- Identical implementation to PR #18641 (duplicate fix?)
- No validation on username format/content
- Environment variable not documented in the change
Security Analysis:
- ✅ No injection vulnerabilities (Base64 encoding used)
- ✅ Secure fallback behavior
- ✅ Standard Basic Auth pattern
Minor Suggestion: Consider adding input validation for username if it accepts user input.
Recommendation: Ready to merge
|
Any news on this? Any way to help this along? I am running a server with custom username so having this would be amazing. Changes look good to me, but if there is more that needs to be done I am happy to help out. |
Issue for this PR
Closes #18611
Type of change
What does this PR do?
opencode attachhardcodes"opencode"as the Basic auth username, ignoring theOPENCODE_SERVER_USERNAMEenvironment variable. This causes silent authentication failure when the server is configured with a custom username.The fix reads
process.env.OPENCODE_SERVER_USERNAMEwith"opencode"as fallback, matching the existing pattern inrun.ts,worker.ts, andplugin/index.ts.How did you verify your code works?
run.ts:659,worker.ts:154, andplugin/index.ts:65Screenshots / recordings
N/A — 1-line code change
Checklist