-
Notifications
You must be signed in to change notification settings - Fork 169
feat(terminal): Migrate ttyd authentication from shell script to HTTP Basic Auth (-c parameter)
#119
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
…ion URL param - Use ttyd's native HTTP Basic Auth (-c parameter) instead of shell script auth - Support ?authorization=base64(user:pass) URL param for seamless auth (no browser popup) - Increase token length from 12 to 24 chars (~143 bits entropy) - Rename ttyd-auth.sh to ttyd-startup.sh (only handles session tracking now) - Update frontend to parse authorization param and send AuthToken in WebSocket JSON - Use labring/ttyd fork which supports ?authorization= feature - Add set -euo pipefail for stricter error handling in entrypoint.sh - Update documentation (TTYD_AUTHENTICATION.md, runtime.md, RUNTIME_WORKFLOW.md, TECHNICAL_DOCUMENTATION.md) BREAKING CHANGE: URL format changed from ?arg=TOKEN&arg=SESSION_ID to ?authorization=base64(user:pass)&arg=SESSION_ID
✅ PR Check Results: PassedBuild Checks
✨ Great work!All checks passed successfully. Your PR is ready for review. Details:
🔗 View Details: |
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.
Pull request overview
This PR migrates ttyd authentication from shell script validation to HTTP Basic Auth using ttyd's -c parameter, improving security and user experience by eliminating browser authentication popups.
Key Changes:
- Authentication moved from shell script to HTTP layer using
-c user:password - Token length increased from 12 to 24 characters (~143 bits entropy)
- URL format changed from
?arg=TOKEN&arg=SESSION_IDto?authorization=base64&arg=SESSION_ID - ttyd source switched from tsl0922/ttyd to labring/ttyd (supports
?authorization=parameter)
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
sandbox/ttyd-startup.sh |
Renamed from ttyd-auth.sh; removed authentication logic, now only handles session tracking |
sandbox/ttyd-auth.sh |
Deleted (functionality replaced by ttyd's HTTP Basic Auth) |
sandbox/entrypoint.sh |
Updated to use -c parameter with HTTP Basic Auth credentials |
sandbox/Dockerfile |
Changed ttyd source to labring/ttyd fork with authorization parameter support |
lib/k8s/sandbox-manager.ts |
Build ttydUrl with ?authorization=base64(user:token) parameter |
app/api/projects/route.ts |
Increased token length from 12 to 24 characters for improved security |
components/terminal/xterm-terminal.tsx |
Updated to parse authorization param and send AuthToken in WebSocket JSON |
docs/technical-notes/* |
Comprehensive documentation updates explaining new authentication architecture |
app/projects/[id]/database/page.tsx |
Removed unused variable (unrelated cleanup) |
pnpm-lock.yaml |
Dependency updates (unrelated) |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * - Session ID passed to ttyd-auth.sh via ?arg= for file upload cwd detection | ||
| * - ttyd-auth.sh stores shell PID in /tmp/.terminal-session-{SESSION_ID} |
Copilot
AI
Dec 17, 2025
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.
The comment mentions 'ttyd-auth.sh' which has been renamed to 'ttyd-startup.sh' in this PR. This should be updated to reflect the new script name for consistency with the actual implementation.
| let ttydUrl = baseTtydUrl | ||
| if (ttydAccessToken) { | ||
| const credentials = `user:${ttydAccessToken}` | ||
| const authBase64 = Buffer.from(credentials).toString('base64') | ||
| ttydUrl = `${baseTtydUrl}?authorization=${authBase64}` | ||
| } |
Copilot
AI
Dec 17, 2025
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.
This change introduces a breaking change for the ttyd-exec utility (lib/util/ttyd-exec.ts). The ttydUrl now includes the authorization parameter, but ttyd-exec.ts still expects to receive a base URL and constructs the authentication via ?arg= parameters. This will break existing functionality that uses ttyd-exec, such as repo initialization and exec-test pages. The ttyd-exec.ts buildWsUrl function needs to be updated in a separate PR or included in this PR to maintain compatibility.
| # Download and install ttyd | ||
| wget -q -O /usr/local/bin/ttyd https://github.com/tsl0922/ttyd/releases/latest/download/ttyd.x86_64; \ | ||
| # Download and install ttyd (using labring fork with ?authorization= support) | ||
| wget -q -O /usr/local/bin/ttyd https://github.com/labring/ttyd/releases/download/1.7.7/ttyd.x86_64; \ |
Copilot
AI
Dec 17, 2025
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.
The Dockerfile downloads a prebuilt ttyd binary with wget -q -O /usr/local/bin/ttyd https://github.com/labring/ttyd/releases/download/1.7.7/ttyd.x86_64 and executes it in the runtime image without any checksum or signature verification. If the labring/ttyd release asset is ever compromised or replaced, new builds will silently include a backdoored ttyd with access to sandbox shells and secrets. To reduce this supply chain risk, pin to an immutable artifact with a verified SHA (or signature) or vendor/build ttyd from a trusted source checked into this repository.
| 3. **URL Format**: `?authorization=base64(user:password)` | ||
|
|
||
| ```bash | ||
| docker run -it --rm \ | ||
| -e DISABLE_TTYD=true \ | ||
| fullstackagent/fullstack-web-runtime:latest | ||
| # Generate authorization parameter | ||
| TOKEN="your-secret-token" | ||
| AUTH=$(echo -n "user:$TOKEN" | base64) | ||
| echo "Access URL: http://localhost:7681?authorization=$AUTH" |
Copilot
AI
Dec 17, 2025
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.
Using the ?authorization=base64(user:password) query parameter to carry the HTTP Basic Auth secret means the ttyd credentials will appear in full in request URLs, which are typically logged by ingress/access logs and stored in browser history. Anyone with access to these logs or shared URLs can base64-decode the value and gain shell access to the sandbox. To reduce this exposure, avoid putting credentials in query parameters (prefer Authorization headers or short‑lived one‑time tokens) and ensure ttydUrl values that are logged or displayed never contain the raw authentication secret.
Summary
-cparameter)?authorization=base64(user:pass)URL parameter for seamless authentication without browser popupChanges
Authentication Architecture
?arg=TOKEN&arg=SESSION_ID?authorization=base64&arg=SESSION_IDsleep infinity?authorization=)Files Modified
Core Changes:
app/api/projects/route.ts- Token length 12 → 24 charslib/k8s/sandbox-manager.ts- Build ttydUrl with?authorization=paramsandbox/entrypoint.sh- Add-cparameter, useset -euo pipefailsandbox/ttyd-auth.sh→sandbox/ttyd-startup.sh- Renamed, simplified to session tracking onlysandbox/Dockerfile- Use labring/ttyd forkcomponents/terminal/xterm-terminal.tsx- Parse authorization, send AuthToken in JSONDocumentation:
docs/technical-notes/TTYD_AUTHENTICATION.md- Complete rewrite for new architecturedocs/technical-notes/runtime.md- Updated ttyd environment variablesdocs/technical-notes/RUNTIME_WORKFLOW.md- Updated startup scripts sectiondocs/technical-notes/TECHNICAL_DOCUMENTATION.md- Updated ttyd integration section