Feature/g UI#1
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
📝 WalkthroughWalkthroughAdds a web-based terminal shell feature: a mongoose-based server (web_main.c) bridging a PTY shell to browsers via WebSocket, embedded static assets (HTML/CSS/JS) generated via a new Perl script, Makefile/run.sh/VSCode config wiring, plus standalone mongoose socket test files and minor gitignore/test file additions. ChangesWeb Terminal Feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant WebMain as web_main.c (mongoose)
participant PTY as PTY/./shell
Browser->>WebMain: HTTP GET /
WebMain-->>Browser: embedded index.html
Browser->>WebMain: WS upgrade /ws
WebMain->>PTY: flush buffered PTY output
WebMain-->>Browser: buffered PTY bytes
Browser->>WebMain: WS message {cols, rows}
WebMain->>PTY: ioctl TIOCSWINSZ resize
Browser->>WebMain: WS message (keystrokes)
WebMain->>PTY: write bytes to PTY master
loop every 50ms
WebMain->>PTY: poll PTY master fd
PTY-->>WebMain: output bytes
WebMain-->>Browser: send bytes over WS
end
Browser->>WebMain: WS close
WebMain->>WebMain: clear tracked connection
Related PRs: None. Suggested labels: feature, build, gui Suggested reviewers: None specified. 🐰 A shell now lives behind glass and glow, 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
lib/mongoose/socktest2.c (1)
1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStandalone scratch test file.
Ad-hoc mongoose smoke test unrelated to the web terminal feature; not wired into the Makefile per the PR summary. Recommend removing before merge or relocating outside the tracked
lib/mongoosesources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mongoose/socktest2.c` around lines 1 - 19, The standalone scratch test in mg_mgr_init/mg_http_listen under the tracked lib/mongoose sources should not be part of this PR. Remove this ad-hoc socktest2.c file from the repository, or move the smoke test outside the tracked mongoose source tree so it is not shipped or built with the library.lib/mongoose/socktest.c (1)
1-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStandalone scratch test file.
Same pattern as
errtest.c/socktest2.c— an ad-hoc diagnostic program with no build integration. Recommend removing before merge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mongoose/socktest.c` around lines 1 - 18, This is a standalone diagnostic scratch file with no build or test integration; remove the ad-hoc socktest main program from the tree before merge. Delete the temporary test code in the socktest.c entry point and keep only integrated test sources such as errtest.c or socktest2.c if they are part of the build.lib/mongoose/errtest.c (1)
1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDebug scratch file committed to repo.
This is a standalone diagnostic snippet unrelated to the web terminal feature described in the PR. Consider removing it or moving it to a
tools//scratch location excluded from version control.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mongoose/errtest.c` around lines 1 - 8, The committed debug scratch snippet in main is unrelated to the feature and should not live in the repo. Remove this standalone diagnostic program from lib/mongoose/errtest.c, or relocate it into a tools/ or scratch area that is excluded from version control so it does not ship with the application..vscode/c_cpp_properties.json (1)
4-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDon’t make one local Cygwin install the only shared workspace config.
This now assumes
C:/cygwin64/bin/gcc.exeand rewrites PATH for every contributor. Anyone on Linux, WSL, MSYS2, or a different Cygwin location loses working IntelliSense out of the box. Keep Cygwin as an additional configuration, or parameterize the path instead of replacing the shared default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.vscode/c_cpp_properties.json around lines 4 - 20, The shared workspace Cygwin configuration is hardcoding a local compiler path and PATH override, which breaks IntelliSense for contributors using other environments. Update the Cygwin entry in c_cpp_properties.json so it is an additional, opt-in configuration or make the compiler path/environment configurable instead of replacing the shared default; keep the existing workspace-wide setup intact for other setups. Focus on the Cygwin config block fields compilerPath and env inside the Cygwin configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@file`:
- Line 1: The committed stray file named file is accidental and should be
removed from the change set before merging. Delete the artifact entirely so it
is not included in the repository, and verify no references remain to this
unintended file.
In `@Makefile`:
- Around line 29-32: The web build target only produces moss-web, but web_main.c
launches ./shell at runtime, so make web must also build the shell binary.
Update the Makefile’s web target/dependencies so it triggers the shell build as
part of the same invocation, using the existing web and shell build rules or by
adding the shell artifact as a prerequisite of web. Locate the fix in the web
and WEB_OUT targets so the built web server always has shell available after
make clean && make web.
In `@src/gui/web_main.c`:
- Around line 95-96: The /ws upgrade path in web_main.c is currently open to any
client that can reach the server, which exposes the PTY unauthenticated. Update
the websocket handling around mg_match and mg_ws_upgrade to require an explicit
auth and/or origin check before upgrading, and make the server bind to loopback
by default unless an override is configured. Apply the same protection to the
other websocket handling block referenced in the review so both entry points
enforce the same access control.
- Around line 49-52: The PTY output frame type is inconsistent between the live
send path in the websocket handling logic and the backlog flush on connect, so
align them to the same frame type. Update the websocket send calls in the PTY
output path and the connect/backlog flush logic in the web_main flow to use a
consistent frame type (referencing s_ws_conn and mg_ws_send) so the browser
client receives data in the expected format for term.write().
- Around line 44-60: In pty_poll_cb(), the PTY read loop only stops on EOF, so a
permanent read failure can leave the timer running forever on a dead shell
session. Update the read handling to check errno after read() in pty_poll_cb and
treat only EAGAIN/EWOULDBLOCK as transient; for any other error, stop the
session by clearing s_running (and any related state if needed). Keep the
existing EOF path intact and make the fix within the pty_poll_cb symbol.
In `@src/gui/web/index.html`:
- Around line 11-17: The web UI markup is missing the footer/dims elements that
`sendSize()` in `main.js` and the existing `#footer`/`#dims` rules in
`styles.css` depend on, so either restore those elements in `index.html` or
update `sendSize()`/the related CSS to stop referencing them. Make sure the
socket-open and resize paths in `sendSize()` safely handle the target element
before writing `textContent`, and keep the `#term` layout working after the
change.
- Around line 7-8: The xterm assets referenced in the web shell page are loaded
from unversioned jsDelivr URLs, which leaves the UI dependent on a third party
and vulnerable to unexpected remote changes. Update the asset references in
index.html to use locally served copies or pin them to exact xterm versions, and
if any remote loading remains, add SRI so the shell UI can be located reliably
via the xterm link/script tags.
In `@src/gui/web/main.js`:
- Line 39: The WebSocket setup in the main UI bootstrap uses a hard-coded ws://
URL, which breaks on secure origins and forces cleartext transport. Update the
WebSocket construction in main.js to derive the scheme from location.protocol so
it uses the secure WebSocket variant when the page is served over HTTPS and the
non-secure variant otherwise, keeping the existing location.host and /ws path
unchanged.
---
Nitpick comments:
In @.vscode/c_cpp_properties.json:
- Around line 4-20: The shared workspace Cygwin configuration is hardcoding a
local compiler path and PATH override, which breaks IntelliSense for
contributors using other environments. Update the Cygwin entry in
c_cpp_properties.json so it is an additional, opt-in configuration or make the
compiler path/environment configurable instead of replacing the shared default;
keep the existing workspace-wide setup intact for other setups. Focus on the
Cygwin config block fields compilerPath and env inside the Cygwin configuration.
In `@lib/mongoose/errtest.c`:
- Around line 1-8: The committed debug scratch snippet in main is unrelated to
the feature and should not live in the repo. Remove this standalone diagnostic
program from lib/mongoose/errtest.c, or relocate it into a tools/ or scratch
area that is excluded from version control so it does not ship with the
application.
In `@lib/mongoose/socktest.c`:
- Around line 1-18: This is a standalone diagnostic scratch file with no build
or test integration; remove the ad-hoc socktest main program from the tree
before merge. Delete the temporary test code in the socktest.c entry point and
keep only integrated test sources such as errtest.c or socktest2.c if they are
part of the build.
In `@lib/mongoose/socktest2.c`:
- Around line 1-19: The standalone scratch test in mg_mgr_init/mg_http_listen
under the tracked lib/mongoose sources should not be part of this PR. Remove
this ad-hoc socktest2.c file from the repository, or move the smoke test outside
the tracked mongoose source tree so it is not shipped or built with the library.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1529cadc-e69d-4938-836c-f9486c956e54
📒 Files selected for processing (16)
.gitignore.vscode/c_cpp_properties.jsonMakefilefilelib/mongoose/errtest.clib/mongoose/mongoose.clib/mongoose/mongoose.hlib/mongoose/socktest.clib/mongoose/socktest2.clib/mongoose/socktest3.crun.shscripts/embed.plsrc/gui/web/index.htmlsrc/gui/web/main.jssrc/gui/web/styles.csssrc/gui/web_main.c
| @@ -0,0 +1 @@ | |||
| test | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Accidental stray file committed.
A file named file containing only test appears to be an unintentional commit artifact. Remove it before merging.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@file` at line 1, The committed stray file named file is accidental and should
be removed from the change set before merging. Delete the artifact entirely so
it is not included in the repository, and verify no references remain to this
unintended file.
| web: $(WEB_OUT) | ||
|
|
||
| $(WEB_OUT): $(WEB_SRC) $(WEB_HDRS) | ||
| $(CC) $(CFLAGS) $(WEB_SRC) lib/mongoose/mongoose.c -o $(WEB_OUT) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make web build the shell binary too.
src/gui/web_main.c starts ./shell on launch, but make web only builds moss-web. After make clean && make web, the web server can start up only to fail its PTY exec path immediately.
Suggested fix
-web: $(WEB_OUT)
+web: $(OUT) $(WEB_OUT)
-$(WEB_OUT): $(WEB_SRC) $(WEB_HDRS)
+$(WEB_OUT): $(OUT) $(WEB_SRC) $(WEB_HDRS)
$(CC) $(CFLAGS) $(WEB_SRC) lib/mongoose/mongoose.c -o $(WEB_OUT)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| web: $(WEB_OUT) | |
| $(WEB_OUT): $(WEB_SRC) $(WEB_HDRS) | |
| $(CC) $(CFLAGS) $(WEB_SRC) lib/mongoose/mongoose.c -o $(WEB_OUT) | |
| web: $(OUT) $(WEB_OUT) | |
| $(WEB_OUT): $(OUT) $(WEB_SRC) $(WEB_HDRS) | |
| $(CC) $(CFLAGS) $(WEB_SRC) lib/mongoose/mongoose.c -o $(WEB_OUT) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` around lines 29 - 32, The web build target only produces moss-web,
but web_main.c launches ./shell at runtime, so make web must also build the
shell binary. Update the Makefile’s web target/dependencies so it triggers the
shell build as part of the same invocation, using the existing web and shell
build rules or by adding the shell artifact as a prerequisite of web. Locate the
fix in the web and WEB_OUT targets so the built web server always has shell
available after make clean && make web.
| static void pty_poll_cb(void *arg) { | ||
| (void)arg; | ||
| char buf[4096]; | ||
| ssize_t n = read(s_master_fd, buf, sizeof(buf)); | ||
|
|
||
| if (n > 0) { | ||
| if (s_ws_conn) { | ||
| mg_ws_send(s_ws_conn, buf, (size_t)n, WEBSOCKET_OP_TEXT); | ||
| } | ||
| else if (s_pty_buf_len + (size_t)n < sizeof(s_pty_buf)) { | ||
| memcpy(s_pty_buf + s_pty_buf_len, buf, (size_t)n); | ||
| s_pty_buf_len += (size_t)n; | ||
| } | ||
| } | ||
|
|
||
| if (n == 0) | ||
| s_running = 0; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle fatal PTY read errors instead of polling a dead fd forever.
pty_poll_cb() only stops on EOF. A permanent read failure keeps the timer alive with a broken shell session and no recovery path. Treat only EAGAIN/EWOULDBLOCK as transient.
Suggested fix
static void pty_poll_cb(void *arg) {
(void)arg;
char buf[4096];
ssize_t n = read(s_master_fd, buf, sizeof(buf));
@@
- if (n == 0)
+ if (n == 0)
s_running = 0;
+ else if (n < 0 && errno != EAGAIN && errno != EWOULDBLOCK)
+ s_running = 0;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static void pty_poll_cb(void *arg) { | |
| (void)arg; | |
| char buf[4096]; | |
| ssize_t n = read(s_master_fd, buf, sizeof(buf)); | |
| if (n > 0) { | |
| if (s_ws_conn) { | |
| mg_ws_send(s_ws_conn, buf, (size_t)n, WEBSOCKET_OP_TEXT); | |
| } | |
| else if (s_pty_buf_len + (size_t)n < sizeof(s_pty_buf)) { | |
| memcpy(s_pty_buf + s_pty_buf_len, buf, (size_t)n); | |
| s_pty_buf_len += (size_t)n; | |
| } | |
| } | |
| if (n == 0) | |
| s_running = 0; | |
| static void pty_poll_cb(void *arg) { | |
| (void)arg; | |
| char buf[4096]; | |
| ssize_t n = read(s_master_fd, buf, sizeof(buf)); | |
| if (n > 0) { | |
| if (s_ws_conn) { | |
| mg_ws_send(s_ws_conn, buf, (size_t)n, WEBSOCKET_OP_TEXT); | |
| } | |
| else if (s_pty_buf_len + (size_t)n < sizeof(s_pty_buf)) { | |
| memcpy(s_pty_buf + s_pty_buf_len, buf, (size_t)n); | |
| s_pty_buf_len += (size_t)n; | |
| } | |
| } | |
| if (n == 0) | |
| s_running = 0; | |
| else if (n < 0 && errno != EAGAIN && errno != EWOULDBLOCK) | |
| s_running = 0; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gui/web_main.c` around lines 44 - 60, In pty_poll_cb(), the PTY read loop
only stops on EOF, so a permanent read failure can leave the timer running
forever on a dead shell session. Update the read handling to check errno after
read() in pty_poll_cb and treat only EAGAIN/EWOULDBLOCK as transient; for any
other error, stop the session by clearing s_running (and any related state if
needed). Keep the existing EOF path intact and make the fix within the
pty_poll_cb symbol.
| if (n > 0) { | ||
| if (s_ws_conn) { | ||
| mg_ws_send(s_ws_conn, buf, (size_t)n, WEBSOCKET_OP_TEXT); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a consistent frame type for PTY output.
The live path sends PTY data as text frames, but the backlog flush on connect uses binary. The current browser client writes e.data straight into term.write(), so the initial prompt/output can arrive in the wrong type and not render.
Suggested fix
- mg_ws_send(c, s_pty_buf, s_pty_buf_len, WEBSOCKET_OP_BINARY);
+ mg_ws_send(c, s_pty_buf, s_pty_buf_len, WEBSOCKET_OP_TEXT);Also applies to: 110-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gui/web_main.c` around lines 49 - 52, The PTY output frame type is
inconsistent between the live send path in the websocket handling logic and the
backlog flush on connect, so align them to the same frame type. Update the
websocket send calls in the PTY output path and the connect/backlog flush logic
in the web_main flow to use a consistent frame type (referencing s_ws_conn and
mg_ws_send) so the browser client receives data in the expected format for
term.write().
| if (mg_match(hm->uri, mg_str("/ws"), NULL)) | ||
| mg_ws_upgrade(c, hm, NULL); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Don’t expose an unauthenticated shell on all interfaces.
Any host that can reach this port can connect to /ws and drive the PTY. With the default bind on 0.0.0.0, this is remote command execution by default. Bind to loopback unless explicitly overridden, and require an auth/origin check before upgrading the socket.
Also applies to: 173-182
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gui/web_main.c` around lines 95 - 96, The /ws upgrade path in web_main.c
is currently open to any client that can reach the server, which exposes the PTY
unauthenticated. Update the websocket handling around mg_match and mg_ws_upgrade
to require an explicit auth and/or origin check before upgrading, and make the
server bind to loopback by default unless an override is configured. Apply the
same protection to the other websocket handling block referenced in the review
so both entry points enforce the same access control.
| <link rel='stylesheet' href='https://cdn.jsdelivr.net/npm/xterm/css/xterm.min.css'/> | ||
| <link rel='stylesheet' href='/styles.css'/> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant web UI files and inspect the HTML plus server-side asset handling.
git ls-files 'src/gui/web/index.html' 'src/gui/web_main.c' 'src/gui/web*' | sed 's#^`#FILE`: #'
printf '\n--- index.html outline/size ---\n'
wc -l src/gui/web/index.html
sed -n '1,120p' src/gui/web/index.html
printf '\n--- web_main.c outline/size ---\n'
wc -l src/gui/web_main.c
sed -n '1,260p' src/gui/web_main.cRepository: MM120-i/MOSS-Shell
Length of output: 6369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether xterm assets are vendored locally and how the page loads them.
printf '\n--- search for xterm assets and references ---\n'
rg -n "xterm|xterm-addon-fit|cdn.jsdelivr|integrity=|crossorigin=" src/gui -S
printf '\n--- list web directory ---\n'
find src/gui/web -maxdepth 2 -type f | sortRepository: MM120-i/MOSS-Shell
Length of output: 1088
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the HTML around the referenced lines and the server route for the page.
nl -ba src/gui/web/index.html | sed -n '1,120p'
printf '\n--- web_main.c excerpts with relevant symbols ---\n'
rg -n "index.html|web/|styles.css|xterm|addon-fit|sendfile|mmap|Content-Type|Content-Security-Policy" src/gui/web_main.c -n -SRepository: MM120-i/MOSS-Shell
Length of output: 196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant file and locate every external asset reference.
nl -ba src/gui/web/index.html | sed -n '1,80p'Repository: MM120-i/MOSS-Shell
Length of output: 196
Vendor or pin the xterm assets in src/gui/web/index.html. The page pulls xterm CSS/JS from unversioned jsDelivr URLs, so the shell UI depends on a third party and executes remote code in the same page as live terminal I/O. Serve local copies, or at least pin exact versions and add SRI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gui/web/index.html` around lines 7 - 8, The xterm assets referenced in
the web shell page are loaded from unversioned jsDelivr URLs, which leaves the
UI dependent on a third party and vulnerable to unexpected remote changes.
Update the asset references in index.html to use locally served copies or pin
them to exact xterm versions, and if any remote loading remains, add SRI so the
shell UI can be located reliably via the xterm link/script tags.
| <div id="frame"> | ||
| <div id="header"> | ||
| <span id="logo">MOSS</span> | ||
| <span id="status"></span> | ||
| </div> | ||
| <div id="term"></div> | ||
| </div> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restore the footer/dims markup or remove the JS/CSS dependency on it.
sendSize() in src/gui/web/main.js Line 44 writes to #dims, and src/gui/web/styles.css already styles #footer/#dims, but this HTML never renders either element. The first successful socket open will throw on null.textContent, which aborts the rest of the handler and leaves resize events throwing too.
Proposed fix
<div id="frame">
<div id="header">
<span id="logo">MOSS</span>
<span id="status"></span>
</div>
<div id="term"></div>
+ <div id="footer">
+ <span id="dims"></span>
+ </div>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div id="frame"> | |
| <div id="header"> | |
| <span id="logo">MOSS</span> | |
| <span id="status"></span> | |
| </div> | |
| <div id="term"></div> | |
| </div> | |
| <div id="frame"> | |
| <div id="header"> | |
| <span id="logo">MOSS</span> | |
| <span id="status"></span> | |
| </div> | |
| <div id="term"></div> | |
| <div id="footer"> | |
| <span id="dims"></span> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gui/web/index.html` around lines 11 - 17, The web UI markup is missing
the footer/dims elements that `sendSize()` in `main.js` and the existing
`#footer`/`#dims` rules in `styles.css` depend on, so either restore those
elements in `index.html` or update `sendSize()`/the related CSS to stop
referencing them. Make sure the socket-open and resize paths in `sendSize()`
safely handle the target element before writing `textContent`, and keep the
`#term` layout working after the change.
| term.loadAddon(fitAddon); | ||
| term.open(document.getElementById('term')); | ||
|
|
||
| const ws = new WebSocket('ws://' + location.host + '/ws'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Derive the WebSocket scheme from the page protocol.
Hard-coding ws:// will fail as mixed content once this UI is served over HTTPS, and it also forces cleartext transport when a secure origin is available.
Proposed fix
-const ws = new WebSocket('ws://' + location.host + '/ws');
+const wsScheme = location.protocol === 'https:' ? 'wss://' : 'ws://';
+const ws = new WebSocket(wsScheme + location.host + '/ws');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const ws = new WebSocket('ws://' + location.host + '/ws'); | |
| const wsScheme = location.protocol === 'https:' ? 'wss://' : 'ws://'; | |
| const ws = new WebSocket(wsScheme + location.host + '/ws'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gui/web/main.js` at line 39, The WebSocket setup in the main UI bootstrap
uses a hard-coded ws:// URL, which breaks on secure origins and forces cleartext
transport. Update the WebSocket construction in main.js to derive the scheme
from location.protocol so it uses the secure WebSocket variant when the page is
served over HTTPS and the non-secure variant otherwise, keeping the existing
location.host and /ws path unchanged.
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit
New Features
Bug Fixes