Skip to content

feat: add optional VNC password authentication - #1072

Merged
mdean808 merged 5 commits into
mainfrom
feat/vnc-password-auth
Feb 26, 2026
Merged

feat: add optional VNC password authentication#1072
mdean808 merged 5 commits into
mainfrom
feat/vnc-password-auth

Conversation

@r33drichards

@r33drichards r33drichards commented Feb 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds optional VNC password authentication to the XFCE container via VNC_PW environment variable
  • When VNC_PW is set, the VNC server uses VncAuth with a password file; otherwise, it runs without authentication (preserving existing behavior)
  • Password file is created with restricted permissions (600) for security

Test plan

  • Verify container starts without VNC_PW set (no-auth mode, existing behavior)
  • Verify container starts with VNC_PW=somepassword and requires password to connect
  • Verify VNC client can connect with correct password
  • Verify VNC client is rejected with incorrect password

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • VNC authentication is now configurable, supporting optional password protection via environment variable with graceful fallback to unsecured mode when not configured
  • Updates

    • Refined message processing in Copilot Kit API to consistently include all messages in analytics and conversation tracking

r33drichards and others added 2 commits February 10, 2026 11:30
Removed filterEmptyMessages call and use messages directly. This fixes
the build error where filteredMessages was undefined after previous
partial changes.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When VNC_PW is set, VNC server uses password authentication instead of
running with no security. This allows securing VNC connections in
environments where the server is exposed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Feb 26, 2026 7:33pm

Request Review

@coderabbitai

coderabbitai Bot commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This pull request removes empty assistant message filtering from the Copilot API route to use unfiltered messages directly throughout, and adds conditional VNC authentication to the startup script that applies password-based auth when the VNC_PW environment variable is set.

Changes

Cohort / File(s) Summary
Copilot API Route
docs/src/app/api/copilotkit/route.ts
Removed empty assistant message filtering; the unfiltered messages array is now used directly for message selection, counting in analytics, and user message extraction.
VNC Authentication
libs/xfce/src/scripts/start-vnc.sh
Added conditional VNC authentication based on VNC_PW environment variable; security arguments are now dynamically set to either password-authenticated or insecure mode instead of hard-coded insecure configuration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 A filter falls away, messages flow free,
While VNC now guards its door with a key,
When passwords are set, security's tight,
These small but sturdy changes get it right! 🔐

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: add optional VNC password authentication' accurately describes the main change in the changeset—adding optional VNC password authentication via the VNC_PW environment variable in the start-vnc.sh file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/vnc-password-auth

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

📦 Publishable packages changed

  • docker/xfce

Add release:<service> labels to auto-release on merge (+ optional bump:minor or bump:major, default is patch).
Or add no-release to skip.

@github-actions

Copy link
Copy Markdown
Contributor

📦 Publishable packages changed

  • docker/xfce

Add release:<service> labels to auto-release on merge (+ optional bump:minor or bump:major, default is patch).
Or add no-release to skip.

@github-actions

Copy link
Copy Markdown
Contributor

📦 Publishable packages changed

  • docker/xfce

Add release:<service> labels to auto-release on merge (+ optional bump:minor or bump:major, default is patch).
Or add no-release to skip.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
docs/src/app/api/copilotkit/route.ts (1)

216-220: Update the class contract after removing filtering.

Line 216 removes filtering, but the class comment and filterEmptyMessages helper still describe the old behavior. Please align docs/implementation to avoid future regressions and confusion.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/src/app/api/copilotkit/route.ts` around lines 216 - 220, Update the
documentation and helper to match the new behavior where messages are no longer
filtered: revise the class/file comment that describes message filtering to
reflect that empty messages are passed through to CopilotKit/Anthropic, and
either remove or repurpose the filterEmptyMessages helper (or update its JSDoc
and name) so it no longer claims to be used for automatic empty-message removal;
ensure references to symbols like filterEmptyMessages, messages, and
modifiedInput in route.ts and any related comments match this new contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/xfce/src/scripts/start-vnc.sh`:
- Around line 8-12: After writing the VNC password file, immediately remove the
secret from the environment to prevent inheritance by child processes: in
start-vnc.sh, after creating /home/cua/.vnc/passwd, setting permissions, and
assigning SECURITY_ARGS, call unset VNC_PW (or otherwise remove it from the
environment) so the VNC_PW variable is no longer exported or visible to
children.
- Around line 10-12: In start-vnc.sh the VNC_PW assigned to vncpasswd can be
silently truncated to 8 characters; add a guard that checks the length of the
VNC_PW variable before calling vncpasswd (referencing VNC_PW and the vncpasswd
invocation that writes /home/cua/.vnc/passwd) and either reject/exit with a
clear error telling the user the 8-char limit or explicitly truncate and log a
warning, then continue to generate the passwd file and set SECURITY_ARGS as
before; ensure the message mentions the 8-character TigerVNC/vncpasswd limit so
users are informed.

---

Nitpick comments:
In `@docs/src/app/api/copilotkit/route.ts`:
- Around line 216-220: Update the documentation and helper to match the new
behavior where messages are no longer filtered: revise the class/file comment
that describes message filtering to reflect that empty messages are passed
through to CopilotKit/Anthropic, and either remove or repurpose the
filterEmptyMessages helper (or update its JSDoc and name) so it no longer claims
to be used for automatic empty-message removal; ensure references to symbols
like filterEmptyMessages, messages, and modifiedInput in route.ts and any
related comments match this new contract.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb7e40a and 386a4d5.

📒 Files selected for processing (2)
  • docs/src/app/api/copilotkit/route.ts
  • libs/xfce/src/scripts/start-vnc.sh

Comment thread libs/xfce/src/scripts/start-vnc.sh
Comment thread libs/xfce/src/scripts/start-vnc.sh
@mdean808 mdean808 added the release:docker/xfce Release docker/xfce on merge label Feb 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📦 Publishable packages changed

  • docker/xfce — will auto-release on merge

@github-actions

Copy link
Copy Markdown
Contributor

📦 Publishable packages changed

  • docker/xfce — will auto-release on merge

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

Labels

release:docker/xfce Release docker/xfce on merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants