| Author | Andy Green <andy@warmcat.com> 2026-09-06 07:51 UTC | | Committer | Andy Green <andy@warmcat.com> 2026-09-06 07:51 UTC | | Tree | a8bfe6865a9ca46a9eab458bafa929eb1a268aa8 Raw Patch | | | web: deliver shell ptydata only to the browser that opened the shell, fixes F-010 | web: deliver shell ptydata only to the browser that opened the shell, fixes F-010
com.warmcat.sai.ptydata (builder interactive-shell I/O) was forwarded via
saiw_ws_broadcast_browsers_REQUIRES_LWS_PRE() to every connected browser
on both the fragment and completion paths in saiw_lp_rx(), with no auth
or ownership filter -- the tx side had no gate at all, while the rx side
(openshell/closeshell/ptydata) is admin-gated. A passive unauthenticated
listener on /sai/browse received the whole privileged session: commands
the admin types and everything the builder prints. sai.js even
auto-creates a terminal window for unknown ptydata task_uuids, so the
session popped up on every viewer's screen.
Track shell ownership per browser connection: the (already admin-gated)
OPENSHELL rx records the shell task_uuid on the pss (up to
SAIW_MAX_SHELLS, closeshell removes it), and sai-web queues server-side
ptydata only to connections that own the shell, mirroring the per-pss
filtering the LOADREPORT forward already does. A ptydata message with no
owning connection, or that did not parse to an object with a task_uuid,
is dropped.
Because the shell task_uuid is only trustworthy once the whole message
has parsed, ptydata fragments are no longer forwarded as they arrive:
they are reassembled (bounded at 64KiB, sai-server sends these in one
<=2KiB piece) and delivered complete to the owner. TASKACTIVITY stays a
broadcast, it is ordinary public build status.
Behaviour note: a browser page reload creates a fresh ws connection with
no shell ownership, so output from shells opened before the reload is no
longer restreamed to it (previously it reached every viewer); the admin
reopens the shell. Non-owner admin conns no longer see other admins'
shells either, which is the point.
|
diff --git a/src/web/w-private.h b/src/web/w-private.h
index a0a32c1..cb1c24c 100644
--- a/src/web/w-private.h
+++ b/src/web/w-private.h
@@ -25,6 +25,14 @@
#define SAIW_API_VERSION 4
+/*
+ * How many builder shells one browser connection may have open and so have
+ * tracked for ptydata delivery. Further openshell forwards still work (it
+ * is the server that opens the shell), but ptydata for shells past this
+ * many is not delivered back to this connection.
+ */
+#define SAIW_MAX_SHELLS 8
+
struct sai_plat;
typedef struct sai_platm {
@@ -110,6 +118,14 @@ struct pss { struct vhd *vhd;
char selected_project[65];
char selected_ref[65];
+ /*
+ * task_uuids of the builder shells this browser itself opened with
+ * com.warmcat.sai.openshell; shell ptydata from the server is only
+ * queued to connections that own the shell.
+ */
+ char shell_task_uuid[SAIW_MAX_SHELLS][65];
+ unsigned int shell_count;
+
sqlite3 *pdb_artifact;
sqlite3_blob *blob_artifact;
@@ -195,12 +211,24 @@ struct vhd {
};
typedef struct saiw_websrv {
- struct lws_ss_handle *ss;
- void *opaque_data;
+ struct lws_ss_handle *ss;
+ void *opaque_data;
+
+ lws_struct_args_t a;
+ struct lejp_ctx ctx;
+ struct lws_buflist *wbltx;
- lws_struct_args_t a;
- struct lejp_ctx ctx;
- struct lws_buflist *wbltx;
+ /*
+ * ptydata rx reassembly (see w-ws-server.c): fragments are buffered
+ * until the message completes, because only the parsed members say
+ * which browser owns the shell, and shell output must not be sent to
+ * anyone else meanwhile. pty_dropped marks a message whose
+ * reassembly was abandoned (oversize / oom): nothing is forwarded
+ * for it.
+ */
+ uint8_t *pty_accum; /* content at + LWS_PRE */
+ size_t pty_accum_len;
+ unsigned int pty_dropped:1;
} saiw_websrv_t;
@@ -271,6 +299,15 @@ void
saiw_browser_state_changed(struct pss *pss, int established);
void
+saiw_pss_shell_open(struct pss *pss, const char *task_uuid);
+
+void
+saiw_pss_shell_close(struct pss *pss, const char *task_uuid);
+
+int
+saiw_pss_owns_shell(struct pss *pss, const char *task_uuid);
+
+void
saiw_update_viewer_count(struct vhd *vhd);
int
diff --git a/src/web/w-ws-browser.c b/src/web/w-ws-browser.c
index 9eba68a..12fa11e 100644
--- a/src/web/w-ws-browser.c
+++ b/src/web/w-ws-browser.c
@@ -1126,7 +1126,20 @@ saiw_ws_json_rx_browser(struct vhd *vhd, struct pss *pss, uint8_t *buf,
break;
case SAIM_WS_BROWSER_RX_OPENSHELL:
+ /*
+ * This browser opened a shell; only it will be sent the
+ * shell's ptydata. The forward to sai-server happens with
+ * the rest below.
+ */
+ saiw_pss_shell_open(pss,
+ ((sai_openshell_t *)a.dest)->task_uuid);
+ break;
+
case SAIM_WS_BROWSER_RX_CLOSESHELL:
+ saiw_pss_shell_close(pss,
+ ((sai_closeshell_t *)a.dest)->task_uuid);
+ break;
+
case SAIM_WS_BROWSER_RX_PTYDATA:
break;
@@ -2169,6 +2182,65 @@ saiw_browser_state_changed(struct pss *pss, int established)
saiw_update_viewer_count(pss->vhd);
}
+/*
+ * Track which builder shells this browser opened itself (the rx side only
+ * lets admins send openshell/closeshell). Shell ptydata coming back from
+ * the server is delivered only to connections on this list.
+ */
+void
+saiw_pss_shell_open(struct pss *pss, const char *task_uuid)
+{
+ unsigned int n;
+
+ if (!task_uuid[0])
+ return;
+
+ for (n = 0; n < pss->shell_count; n++)
+ if (!strcmp(pss->shell_task_uuid[n], task_uuid))
+ return;
+
+ if (pss->shell_count >= SAIW_MAX_SHELLS) {
+ lwsl_wsi_notice(pss->wsi,
+ "shell tracking full, ptydata for %s will "
+ "not be delivered here", task_uuid);
+ return;
+ }
+
+ lws_strncpy(pss->shell_task_uuid[pss->shell_count], task_uuid,
+ sizeof(pss->shell_task_uuid[0]));
+ pss->shell_count++;
+}
+
+void
+saiw_pss_shell_close(struct pss *pss, const char *task_uuid)
+{
+ unsigned int n;
+
+ for (n = 0; n < pss->shell_count; n++) {
+ if (!strcmp(pss->shell_task_uuid[n], task_uuid)) {
+ memmove(&pss->shell_task_uuid[n],
+ &pss->shell_task_uuid[n + 1],
+ (pss->shell_count - n - 1) *
+ sizeof(pss->shell_task_uuid[0]));
+ pss->shell_count--;
+
+ return;
+ }
+ }
+}
+
+int
+saiw_pss_owns_shell(struct pss *pss, const char *task_uuid)
+{
+ unsigned int n;
+
+ for (n = 0; n < pss->shell_count; n++)
+ if (!strcmp(pss->shell_task_uuid[n], task_uuid))
+ return 1;
+
+ return 0;
+}
+
diff --git a/src/web/w-ws-server.c b/src/web/w-ws-server.c
index fd55d6b..5d7a2c3 100644
--- a/src/web/w-ws-server.c
+++ b/src/web/w-ws-server.c
@@ -85,6 +85,67 @@ enum {
* This may come in chunks and is statefully parsed
* so it's not directly sensitive to size or fragmentation
*/
+
+/*
+ * Reassemble ptydata rx until the message completes: the members saying
+ * which browser owns the shell are only trusted once the whole message has
+ * parsed, and the shell output must not be queued to anyone else in the
+ * meantime. sai-server sends these in one piece (its own serialization
+ * buffer is 2KiB), so this is normally a single append. The cap just
+ * bounds what a broken or hostile peer can make us hold.
+ */
+#define SAIW_PTY_ACCUM_MAX (64 * 1024)
+
+static void
+saiw_pty_accum_reset(saiw_websrv_t *m)
+{
+ free(m->pty_accum);
+ m->pty_accum = NULL;
+ m->pty_accum_len = 0;
+ m->pty_dropped = 0;
+}
+
+static void
+saiw_pty_accum_drop(saiw_websrv_t *m)
+{
+ free(m->pty_accum);
+ m->pty_accum = NULL;
+ m->pty_accum_len = 0;
+ m->pty_dropped = 1;
+}
+
+static int
+saiw_pty_accum(saiw_websrv_t *m, const uint8_t *frag, size_t len)
+{
+ uint8_t *na;
+
+ if (m->pty_dropped)
+ return 0;
+
+ if (m->pty_accum_len + len > SAIW_PTY_ACCUM_MAX) {
+ lwsl_notice("%s: ptydata reassembly over size, dropping msg\n",
+ __func__);
+ saiw_pty_accum_drop(m);
+
+ return 0;
+ }
+
+ na = realloc(m->pty_accum, LWS_PRE + m->pty_accum_len + len);
+ if (!na) {
+ lwsl_notice("%s: ptydata reassembly oom, dropping msg\n",
+ __func__);
+ saiw_pty_accum_drop(m);
+
+ return 0;
+ }
+
+ m->pty_accum = na;
+ memcpy(m->pty_accum + LWS_PRE + m->pty_accum_len, frag, len);
+ m->pty_accum_len += len;
+
+ return 0;
+}
+
static int
saiw_lp_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
{
@@ -100,6 +161,7 @@ saiw_lp_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
if (is_start) {
/* First frag of a new message. Clear old parse results and init */
lwsac_free(&m->a.ac);
+ saiw_pty_accum_reset(m);
memset(&m->a, 0, sizeof(m->a));
m->a.map_st[0] = lsm_schema_json_map;
m->a.map_entries_st[0] = LWS_ARRAY_SIZE(lsm_schema_json_map);
@@ -129,7 +191,6 @@ saiw_lp_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
*/
switch (m->a.top_schema_index) {
case SAIS_WS_WEBSRV_RX_TASKACTIVITY:
- case SAIS_WS_WEBSRV_RX_PTYDATA:
{
uint8_t *tmp = malloc(LWS_PRE + rem);
if (tmp) {
@@ -142,6 +203,14 @@ saiw_lp_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
}
break;
}
+ case SAIS_WS_WEBSRV_RX_PTYDATA:
+ /*
+ * Hold the fragment until the message
+ * completes; it goes out only to the
+ * shell's owner once we know who that is
+ */
+ saiw_pty_accum(m, p, rem);
+ break;
case SAIS_WS_WEBSRV_RX_LOADREPORT:
{
uint8_t *tmp = malloc(LWS_PRE + rem);
@@ -174,7 +243,6 @@ saiw_lp_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
case SAIS_WS_WEBSRV_RX_TASKCHANGE:
case SAIS_WS_WEBSRV_RX_EVENTCHANGE:
case SAIS_WS_WEBSRV_RX_TASKACTIVITY:
- case SAIS_WS_WEBSRV_RX_PTYDATA:
{
uint8_t *tmp = malloc(LWS_PRE + consumed);
if (tmp) {
@@ -187,6 +255,42 @@ saiw_lp_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
}
break;
}
+ case SAIS_WS_WEBSRV_RX_PTYDATA:
+ {
+ /*
+ * Shell output is private to the admin session that
+ * opened the shell: queue it only to browsers that
+ * sent openshell for this task_uuid. Reassemble the
+ * whole message first so we know the parsed members
+ * are complete and honest.
+ */
+ sai_ptydata_t *pd = (sai_ptydata_t *)m->a.dest;
+
+ saiw_pty_accum(m, p, consumed);
+
+ if (m->pty_accum && pd && pd->task_uuid[0]) {
+ lws_start_foreach_dll(struct lws_dll2 *, pt,
+ vhd->browsers.head) {
+ struct pss *pss = lws_container_of(
+ pt, struct pss, same);
+
+ if (saiw_pss_owns_shell(pss,
+ pd->task_uuid))
+ saiw_ws_browser_queue_REQUIRES_LWS_PRE(
+ pss,
+ m->pty_accum + LWS_PRE,
+ m->pty_accum_len,
+ lws_write_ws_flags(
+ LWS_WRITE_TEXT,
+ 1, 1));
+ } lws_end_foreach_dll(pt);
+ } else
+ lwsl_notice("%s: ptydata for unowned shell,"
+ " not forwarded\n", __func__);
+
+ saiw_pty_accum_reset(m);
+ break;
+ }
case SAIS_WS_WEBSRV_RX_LOADREPORT:
{
uint8_t *tmp = malloc(LWS_PRE + consumed);
@@ -363,6 +467,7 @@ saiw_lp_state(void *userobj, void *sh, lws_ss_constate_t state,
switch (state) {
case LWSSSCS_DESTROYING:
+ saiw_pty_accum_reset(m);
break;
case LWSSSCS_CONNECTED:
|