Skip to content

Conversation

@xingyaoww
Copy link
Collaborator

@xingyaoww xingyaoww commented Nov 13, 2025

Description

This PR implements secure token-based authentication for VNC/noVNC access to address the security vulnerability identified in #972. The implementation follows the same pattern as VSCode's token-based authentication.

Changes Made

1. Token Generation and Management

  • Added connection_token attribute to DesktopService
  • Token can be provided via session_api_keys or auto-generated (64-char hex)
  • Token stored in ~/.vnc/websockify-tokens.conf with proper file permissions (0o600)

2. WebSocket Authentication

  • Updated noVNC proxy startup to use websockify's TokenFile plugin
  • Token format: <token>: 127.0.0.1:5901\n
  • Websockify validates tokens before allowing VNC access
  • VNC server continues to listen only on localhost for additional security

3. URL Generation with Token

  • Updated get_vnc_url() to include token in URL parameters
  • URL format: <base>/vnc.html?path=<host>/websockify&token=<token>&autoconnect=1&resize=remote
  • Similar to VSCode's connection token approach

4. Integration with Config

  • Updated get_desktop_service() to pass token from session_api_keys
  • Maintains consistency with VSCode authentication pattern

5. Comprehensive Testing

  • Added tests for token initialization (with and without provided token)
  • Added tests for token generation during service start
  • Added tests for token file creation with correct format
  • Added tests for URL generation with token
  • Added tests for integration with session_api_keys
  • All tests passing (33/33)

Security Considerations

  • Token-based authentication: Only clients with valid tokens can access VNC
  • Localhost binding: VNC server listens only on 127.0.0.1
  • File permissions: Token file created with mode 0o600 (read/write for owner only)
  • Token rotation: New tokens generated per session when not explicitly provided
  • Kept -SecurityTypes None: Since VNC only listens on localhost, authentication is enforced by websockify at the proxy layer

Testing

All existing tests pass plus new authentication tests:

uv run pytest tests/agent_server/test_desktop_service.py -xvs
# 33 passed, 2 warnings in 0.09s

Pre-commit hooks all pass:

uv run pre-commit run --files openhands-agent-server/openhands/agent_server/desktop_service.py tests/agent_server/test_desktop_service.py
# Format YAML files....Skipped
# Ruff format..........Passed
# Ruff lint............Passed
# PEP8 style check.....Passed
# Type check...........Passed

Related Issues

Fixes #972

Co-authored-by: openhands [email protected]

@xingyaoww can click here to continue refining the PR


Agent Server images for this PR

GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Architectures Base Image Docs / Tags
java amd64, arm64 eclipse-temurin:17-jdk Link
python amd64, arm64 nikolaik/python-nodejs:python3.12-nodejs22 Link
golang amd64, arm64 golang:1.21-bookworm Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:bccd57f-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-bccd57f-python \
  ghcr.io/openhands/agent-server:bccd57f-python

All tags pushed for this build

ghcr.io/openhands/agent-server:bccd57f-golang-amd64
ghcr.io/openhands/agent-server:bccd57f-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:bccd57f-golang-arm64
ghcr.io/openhands/agent-server:bccd57f-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:bccd57f-java-amd64
ghcr.io/openhands/agent-server:bccd57f-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:bccd57f-java-arm64
ghcr.io/openhands/agent-server:bccd57f-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:bccd57f-python-amd64
ghcr.io/openhands/agent-server:bccd57f-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-amd64
ghcr.io/openhands/agent-server:bccd57f-python-arm64
ghcr.io/openhands/agent-server:bccd57f-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-arm64
ghcr.io/openhands/agent-server:bccd57f-golang
ghcr.io/openhands/agent-server:bccd57f-java
ghcr.io/openhands/agent-server:bccd57f-python

About Multi-Architecture Support

  • Each variant tag (e.g., bccd57f-python) is a multi-arch manifest supporting both amd64 and arm64
  • Docker automatically pulls the correct architecture for your platform
  • Individual architecture tags (e.g., bccd57f-python-amd64) are also available if needed

This commit implements secure token-based authentication for VNC/noVNC access, similar to the VSCode authentication pattern:

- Add token generation and management to DesktopService
  - Token can be provided via session_api_keys or auto-generated
  - Token stored in websockify-tokens.conf file with proper permissions

- Update noVNC proxy startup to use websockify TokenFile plugin
  - Tokens are validated by websockify before allowing VNC access
  - VNC server continues to listen only on localhost for security

- Update get_vnc_url() to include token in URL parameters
  - Token passed via 'token' query parameter
  - Similar to VSCode's connection token approach

- Add comprehensive tests for token authentication
  - Test token generation and file creation
  - Test URL generation with token
  - Test integration with session_api_keys

Fixes #972

Co-authored-by: openhands <[email protected]>
@github-actions
Copy link
Contributor

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands-agent-server/openhands/agent_server
   desktop_service.py12210216%21–24, 28–30, 33–39, 42–47, 50–52, 55–56, 59–64, 67–70, 76–79, 82, 84, 93–94, 96–97, 101, 116–118, 122–123, 132–133, 135–137, 139–142, 150, 154, 168–170, 172, 178, 181–183, 185–186, 190–200, 202, 206–207, 210–211, 214–216, 227–231, 253–258
TOTAL11985552453% 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VNC password authentication

3 participants