Skip to content

feat: harden TypeScript server runtime and proxy websocket access through port 5173 - #1

Merged
cooperj merged 9 commits into
mainfrom
typescript-rewrite
May 28, 2026
Merged

cooperj merged 9 commits into
mainfrom
typescript-rewrite

Conversation

@cooperj

@cooperj cooperj commented May 27, 2026 •

Copy link
Copy Markdown
Member

This pull request updates the frontend panel to improve TypeScript support, streamline dependency management, and enhance tooling configurations. The most significant changes include migrating the server from JavaScript to TypeScript, updating build and install processes to use more reliable dependency management, refining linting and formatting workflows, and addressing follow-up runtime, networking, and security feedback.

TypeScript Migration and Node.js Compatibility:

  • Migrated server.js to server.ts, updating all imports to use node: specifiers and adding type annotations for better type safety and compatibility with Node.js ESM modules. [1] [2]
  • Updated all scripts and install processes to reference server.ts instead of server.js, including CMake install targets and npm scripts. [1] [2]
  • Added explicit Node.js runtime requirements via package.json engines and pinned runtime tooling (.nvmrc and Docker image setup) to avoid ambiguous --experimental-strip-types behavior across environments.
  • Follow-up update: bumped runtime pinning from Node 22 to Node 24.11.1 (including engines, .nvmrc, Dockerfile, and README runtime note) to resolve CI/devcontainer build failures tied to the previous Node 22 pin while preserving the same TypeScript runtime execution approach.

Dependency Management and Build Process:

  • Switched from npm install to npm ci in both GitHub Actions workflow and CMake build process to ensure reproducible and faster installs in CI and local builds. [1] [2]
  • Added @types/node to devDependencies to support Node.js type definitions in TypeScript.

Linting and Formatting Improvements:

  • Renamed eslint.config.mjs to eslint.config.ts and updated ESLint configuration to better target TypeScript files and Node.js scripts, improving linting accuracy and performance. [1] [2]
  • Updated lint and lint:fix npm scripts to use the --experimental-strip-types Node.js flag, allowing ESLint to process TypeScript files directly.
  • Expanded formatting scripts to include new TypeScript configuration and server files.

Websocket Proxying and Port Exposure Reduction:

  • Added robust websocket proxy handling so browser connections use the UI origin on port 5173 (/capability) instead of directly exposing port 9000.
  • Updated Vite development proxy and server-side websocket upgrade proxying to support the same endpoint path.
  • Removed public port 9000 exposure references from compose/devcontainer/runtime docs and configuration.
  • Added Docker-friendly runtime configuration for websocket endpoint path so remote access works without client IP/host rewrites.

UI and Runtime Behavior Updates:

  • Added support for ?sound=off to disable rendering of the audio player component.
  • Documented this new sound-disable option in the README.

Static File Serving Security Hardening:

  • Strengthened static file resolution and reads to mitigate traversal and unexpected file access risks (including stricter path normalization, decode validation, and realpath boundary checks before file reads).

These changes collectively improve maintainability, runtime reliability, security posture, and deployment ergonomics of the frontend panel.

@cooperj
cooperj requested a review from Copilot May 27, 2026 23:12
@cooperj cooperj self-assigned this May 27, 2026

Copilot AI 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.

Pull request overview

This pull request migrates the frontend panel’s lightweight static server and tooling to TypeScript, while tightening CI/build reproducibility via npm ci and updating lint/format workflows to operate on TypeScript configuration directly.

Changes:

  • Migrates the UI static server to TypeScript and updates runtime/start scripts accordingly.
  • Switches dependency installation in CI and CMake builds from npm install to npm ci for reproducible installs.
  • Updates ESLint configuration to a TypeScript-based flat config and adjusts lint/format scripts to include new TS config/server files.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/fre_information_panel/server.ts Converts server imports to node: specifiers and introduces TypeScript typing/formatting adjustments while serving the Vite dist/ output.
src/fre_information_panel/package.json Updates scripts to run/lint TS directly using --experimental-strip-types, expands prettier targets, and adds @types/node.
src/fre_information_panel/package-lock.json Locks the newly added Node typings and associated dependency graph changes.
src/fre_information_panel/eslint.config.ts Refines ESLint targeting for browser React sources vs Node-side TS scripts/config files.
src/fre_information_panel/CMakeLists.txt Uses npm ci and installs server.ts instead of server.js for the packaged artifact.
.github/workflows/copilot-setup-steps.yml Updates workflow dependency installation to npm ci.
Files not reviewed (1)
  • src/fre_information_panel/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)

src/fre_information_panel/server.ts:69

  • The SPA fallback currently triggers for any fs.readFile error (including EACCES/EISDIR), returning 200 with index.html. Consider only falling back when the requested file is genuinely missing (err.code === 'ENOENT'), and return 500 for other errors to avoid masking real deployment issues. Optionally also gate the SPA fallback on Accept: text/html so missing static assets return 404 instead of index.html.
    src/fre_information_panel/server.ts:52
  • The path traversal check is based on string-prefix comparison of the resolved path. This prevents ".." traversal, but it does not prevent escaping dist/ via symlinks that live inside dist/ (a symlinked file can point outside while still having a path that starts with DIST_DIR). If you want this server to be robust, consider resolving real paths (fs.realpath) for both DIST_DIR and the requested file and comparing those, or explicitly rejecting symlinks (lstat + isSymbolicLink).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/fre_information_panel/package.json
@cooperj

cooperj commented May 28, 2026

Copy link
Copy Markdown
Member Author

@copilot implement the following changes:

start:ui (and lint) now relies on Node's --experimental-strip-types to execute TypeScript directly. To avoid confusing runtime failures on systems where Node 22 is present but the specific minor version lacks/changes this flag. We should declare this explicit Node version requirement (e.g., via package.json engines.node and ensuring the Dockerfile is strictly configured).

Add an option for a user to go to http://localhost:5173/?sound=off which disables all rendering of the Audio Player component. Add a single bullet point to the read me explaining this functionality.

Implement a proxy for the websocket functionality from rclnodejs. Allowing for only the main 5173 port to be exposed, closing up port 9000. You should do this using the correct Vite proxy in a robust manner. Then update the readme and compose files to ensure that any reference to port 9000 is removed.

Improve the security of the implemented fs.fileRead. It seems likely that it could be exploited to get access to files which aren't expected to be served.

The finally; review the project for logistical and security concerns.

Copilot AI and others added 2 commits May 28, 2026 06:24
Co-authored-by: cooperj <28831674+cooperj@users.noreply.github.com>
Co-authored-by: cooperj <28831674+cooperj@users.noreply.github.com>

Copilot AI commented May 28, 2026

Copy link
Copy Markdown

@copilot implement the following changes:

start:ui (and lint) now relies on Node's --experimental-strip-types to execute TypeScript directly. To avoid confusing runtime failures on systems where Node...

Implemented in commits c890aad and 39b96cc. This now enforces Node >=22.16.0 <23 (including Docker pinning), adds ?sound=off to disable Audio Player rendering, proxies websocket traffic through port 5173 with port 9000 exposure removed from compose/devcontainer/docs, hardens static file reads against traversal/symlink escape, and makes the websocket endpoint runtime-configurable for Docker while keeping host/IP automatic for remote access.

Copilot AI changed the title chore: rewrite server components to use typescript feat: harden TypeScript server runtime and proxy websocket access through port 5173 May 28, 2026
@cooperj

cooperj commented May 28, 2026

Copy link
Copy Markdown
Member Author

@copilot the build is failing due to the way that the we are using NodeJS 22.

I think it would be wise to upgrade to Node version 24, but only if we can make the package.json work in the same way with typescript types.

Co-authored-by: cooperj <28831674+cooperj@users.noreply.github.com>

Copilot AI commented May 28, 2026

Copy link
Copy Markdown

@copilot the build is failing due to the way that the we are using NodeJS 22.

I think it would be wise to upgrade to Node version 24, but only if we can make the package.json work in the same way wi...

Implemented in commit f268b46.

Updated runtime pinning to Node 24.11.1 across .devcontainer/Dockerfile, src/fre_information_panel/.nvmrc, and src/fre_information_panel/package.json (engines.node now >=24.11.1 <25), and updated README runtime docs to match. This keeps the TypeScript execution flow the same (--experimental-strip-types) while resolving the CI build failure caused by the previous Node 22.16.0 tarball fetch.

@cooperj
cooperj requested a review from Copilot May 28, 2026 09:28
@cooperj
cooperj marked this pull request as ready for review May 28, 2026 09:30

Copilot AI 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.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.

Files not reviewed (1)
  • src/fre_information_panel/package-lock.json: Language not supported

Comment thread src/fre_information_panel/server.ts
Comment thread src/fre_information_panel/server.ts
Comment thread src/fre_information_panel/server.ts Outdated
Comment on lines +7 to +9
"engines": {
"node": ">=24.11.1 <25"
},
cooperj and others added 2 commits May 28, 2026 10:35
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@cooperj
cooperj merged commit 6000f06 into main May 28, 2026
3 checks passed
@cooperj
cooperj deleted the typescript-rewrite branch May 28, 2026 10:00
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.

3 participants