Repository navigation
feat: harden TypeScript server runtime and proxy websocket access through port 5173 - #1
Conversation
…lation fix: seperate browser and node targets for linting
There was a problem hiding this comment.
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 installtonpm cifor 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.
|
@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. |
Co-authored-by: cooperj <28831674+cooperj@users.noreply.github.com>
Co-authored-by: cooperj <28831674+cooperj@users.noreply.github.com>
Implemented in commits c890aad and 39b96cc. This now enforces Node >=22.16.0 <23 (including Docker pinning), adds |
|
@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>
Implemented in commit Updated runtime pinning to Node 24.11.1 across |
| "engines": { | ||
| "node": ">=24.11.1 <25" | ||
| }, |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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:
server.jstoserver.ts, updating all imports to usenode:specifiers and adding type annotations for better type safety and compatibility with Node.js ESM modules. [1] [2]server.tsinstead ofserver.js, including CMake install targets and npm scripts. [1] [2]package.jsonengines and pinned runtime tooling (.nvmrcand Docker image setup) to avoid ambiguous--experimental-strip-typesbehavior across environments.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:
npm installtonpm ciin both GitHub Actions workflow and CMake build process to ensure reproducible and faster installs in CI and local builds. [1] [2]@types/nodetodevDependenciesto support Node.js type definitions in TypeScript.Linting and Formatting Improvements:
eslint.config.mjstoeslint.config.tsand updated ESLint configuration to better target TypeScript files and Node.js scripts, improving linting accuracy and performance. [1] [2]--experimental-strip-typesNode.js flag, allowing ESLint to process TypeScript files directly.Websocket Proxying and Port Exposure Reduction:
/capability) instead of directly exposing port 9000.UI and Runtime Behavior Updates:
?sound=offto disable rendering of the audio player component.Static File Serving Security Hardening:
These changes collectively improve maintainability, runtime reliability, security posture, and deployment ergonomics of the frontend panel.