Project homepage Mailing List  Warmcat.com  API Docs  Github Mirror 
    npro  
 Modern all-safe Rust Network Protocol library supporting h1, h2, h3, ws, wt sans-IO and with socket IO + tls
git clone https://npro.rs/repo/npro
Author[]Andy Green <andy@warmcat.com> 2026-09-06 07:57 UTC
Committer[]Andy Green <andy@warmcat.com> 2026-09-06 07:57 UTC
Tree4887359006d7df1778bcbff23909bb2485bb42f9   Raw Patch
 
server: one shell id contract (SAI_SHELLID_LEN, 32 hex), fixes F-013
server: one shell id contract (SAI_SHELLID_LEN, 32 hex), fixes F-013

Interactive-shell ids had three disagreeing shapes: sai.js minted
16-hex-char ids for Open Shell, the server's empty-uuid fallback minted
32 (sai_uuid16_create), and the PTYDATA and CLOSESHELL handlers
validated for exactly 64 alnum (SAI_TASKID_LEN, a task uuid length no
shell ever has).  So the admin's shell keystrokes and closes were
always silently rejected: the shell never received input, and because
closeshell is the only cleanup for shell sessions (there is no
reaper), every Open Shell click leaked a server-side session struct
and a builder-side shell.  OPENSHELL itself accepted any non-empty id,
which is how the unclosable sessions got created in the first place.

Define SAI_SHELLID_LEN (32, the sai_uuid16_create shape) and validate
shell ids consistently: sai.js mints 16 random bytes -> 32 hex chars,
OPENSHELL rejects a supplied id that is not exactly that shape (the
server still mints when none is supplied), and PTYDATA / CLOSESHELL
validate against SAI_SHELLID_LEN instead of SAI_TASKID_LEN.  The
builder matches shell ids by exact string compare, so it needs no
change.
diff --git a/assets/sai.js b/assets/sai.js index c7e02c4..072056d 100644 --- a/assets/sai.js +++ b/assets/sai.js @@ -2483,7 +2483,9 @@ function createBuilderDiv(plat) { menuItems.push({ label: "<span class='builder-shell-btn'>Open Shell</span>", callback: () => { - const task_uuid = Array.from(crypto.getRandomValues(new Uint8Array(8))) + /* 16 random bytes -> 32 hex chars: the shell id shape + * the server validates (SAI_SHELLID_LEN) */ + const task_uuid = Array.from(crypto.getRandomValues(new Uint8Array(16))) .map(b => b.toString(16).padStart(2, '0')).join(''); const msg = { diff --git a/src/server/s-private.h b/src/server/s-private.h index ea0688b..765d440 100644 --- a/src/server/s-private.h +++ b/src/server/s-private.h @@ -26,6 +26,14 @@ #define SAI_EVENTID_LEN 32 #define SAI_TASKID_LEN 64 +/* + * Ad-hoc interactive shell session ids (com.warmcat.sai.openshell et al): + * the browser mints one when opening the shell and reuses it in ptydata / + * closeshell; the server mints the same kind when the browser supplied + * none. Same shape as an event id: 32 hex chars. + */ +#define SAI_SHELLID_LEN 32 + struct sai_plat; /* lws_wsmsg_ array for different sources */ diff --git a/src/server/s-ws-web.c b/src/server/s-ws-web.c index 4a784df..b7cf3f6 100644 --- a/src/server/s-ws-web.c +++ b/src/server/s-ws-web.c @@ -714,7 +714,18 @@ websrvss_ws_rx(void *userobj, const uint8_t *buf, size_t len, int flags) lwsl_notice("%s: OPENSHELL received from web for %s, passing to builder\n", __func__, os->builder_name); - if (!os->task_uuid[0]) + /* + * The shell id must be a shell-shaped id if given, so the + * session we create can always be addressed (and closed) by + * the ptydata/closeshell validation below. + */ + if (os->task_uuid[0]) { + if (sais_validate_id(os->task_uuid, SAI_SHELLID_LEN)) { + lwsl_notice("%s: OPENSHELL bad shell id\n", + __func__); + break; + } + } else sai_uuid16_create(m->vhd->context, os->task_uuid); /* Add it to in-memory shell sessions list */ @@ -752,8 +763,8 @@ websrvss_ws_rx(void *userobj, const uint8_t *buf, size_t len, int flags) { sai_closeshell_t *cs = (sai_closeshell_t *)a.dest; - if (sais_validate_id(cs->task_uuid, SAI_TASKID_LEN)) { - lwsl_notice("%s: CLOSESHELL bad task_uuid\n", __func__); + if (sais_validate_id(cs->task_uuid, SAI_SHELLID_LEN)) { + lwsl_notice("%s: CLOSESHELL bad shell id\n", __func__); break; } @@ -788,9 +799,9 @@ websrvss_ws_rx(void *userobj, const uint8_t *buf, size_t len, int flags) * Validate ids (not pd->data, which is opaque pty payload that * must pass through to the builder's shell). */ - if (sais_validate_id(pd->task_uuid, SAI_TASKID_LEN) || + if (sais_validate_id(pd->task_uuid, SAI_SHELLID_LEN) || sais_validate_builder_name(pd->builder_name)) { - lwsl_notice("%s: PTYDATA bad task_uuid/builder\n", + lwsl_notice("%s: PTYDATA bad shell id/builder\n", __func__); break; }
Page fetched 0s ago, creation time: 5ms (vhost etag hits: 0%, cache hits: 0%)