From 405a2ef919c04f13bb1de7d765e5fd896d0044d1 Mon Sep 17 00:00:00 2001
From: cproudlock <cproudlock@localhost>
Date: Thu, 3 Sep 2026 08:38:20 -0400
Subject: [PATCH] Cap request bodies, close a hole in the gate, fix to_dict and
 two settings pages

Six fixes that were ours regardless of what the sister site does. Four came out
of our own reviews; two you reported from the settings pages.

MAX_CONTENT_LENGTH was set nowhere. Every upload route caps its own payload, but
each of those checks runs after the body has arrived: the first touch of
request.form or request.files makes Werkzeug parse the whole multipart and spool
the file to a temp file, so a route can reject the content and still have paid
for the transfer and the disk. Three separate reviews found that shape in three
different routes, which makes it a property of the app rather than of any
handler. 128 MB, clearing the 64 MB agent binary with room for the envelope.

The naming gate excluded devnotes with --exclude-dir, which matches a directory
of that NAME at any depth - so a force-added plugins/<plugin>/devnotes/ was
invisible to every check in the file, ADR-015 included. Replaced with a path
anchored post-filter. Verified by planting the same site literal in root
devnotes/ AND in plugins/endpoints/devnotes/: the gate now reports exactly one,
the nested one. Before, it reported none.

BaseModel.to_dict walked __table__.columns and published every one, so a model
without an override published whatever it carried - including columns added
later by someone not thinking about this method. User has no override, so
User.to_dict() returned its bcrypt hash. Nothing calls it in a response path
today, which is luck rather than design. A blanket allowlist would empty the
output of all 34 models relying on the default, so this pins a floor instead:
named secret-bearing columns are never emitted, and a model may extend the set.
Eight tests, including a sweep over every mapped model.

ActionPicker's Esc handlers had .prevent but not .stop, so the event bubbled to
the wrapper and closed the whole picker instead of returning to the monitor list.

The branding page reported "error loading image" on the site and badge logos.
Two bugs stacked. The load failure was real: the preview used the stored value
as a raw src, and that value is root-relative, so under /shopdb or /ops it
resolved against the SERVER root and 404d - siteSettings.js wraps every logo in
withBase() and this page was the one place that did not. With that fixed two of
three were still invisible, because the shipped GE mark is fill="white" for the
dark sidebar and was rendering white-on-white. The preview tile now sits on a
mid-tone checker so artwork of any colour reads against it.

The floor-map levels table ran off the right with its cells crushed - nine
columns of inputs, no scroll wrapper, and .data-table is styled nowhere. My
first attempt reached for the global .table-container and made it WORSE: that
class also sets white-space: nowrap, and a full sentence of hint text lives in
one of the cells, so nowrap forced the table wider still. It now scrolls in a
local wrapper, the hint wraps, and the name input has a width instead of
collapsing to one character.

Suite 2377, frontend 324, gate green.

Scope: product
---
 frontend/src/assets/style.css                 | 24 ++++++-
 .../src/views/settings/BrandingSettings.vue   |  8 ++-
 .../src/views/settings/FloorMapSettings.vue   | 22 +++++-
 .../computers/frontend/views/ActionPicker.vue |  4 +-
 scripts/check-naming-and-style.sh             | 24 ++++---
 shopdb/config.py                              | 16 +++++
 shopdb/core/models/base.py                    | 29 +++++++-
 tests/test_core/test_to_dict_floor.py         | 69 +++++++++++++++++++
 8 files changed, 180 insertions(+), 16 deletions(-)
 create mode 100644 tests/test_core/test_to_dict_floor.py

diff --git a/frontend/src/assets/style.css b/frontend/src/assets/style.css
index 18bd2c44..30b8d035 100644
--- a/frontend/src/assets/style.css
+++ b/frontend/src/assets/style.css
@@ -1323,13 +1323,33 @@ td.actions .btn + .btn {
   gap: 0.75rem;
   margin-top: 0.4rem;
 }
+/* A branding preview has to show a logo whose colour we do not control. The
+   shipped GE mark is fill="white" because it lives on the dark sidebar, so on a
+   plain white tile it rendered white-on-white and read as a failed load. A
+   checker ground makes any logo visible - light, dark or transparent - and is
+   the usual convention for previewing artwork with alpha. */
 .map-thumb {
   height: 40px;
+  min-width: 40px;
   border: 1px solid var(--border);
   border-radius: 4px;
-  background: #fff;
+  /* MID-TONE, not white and not near-white. The tile has to show artwork whose
+     colour we do not control: the shipped GE mark is fill="white" for the dark
+     sidebar, the monogram is fill="black". A light ground hides the first, a
+     dark ground hides the second, so the checker sits either side of mid grey
+     and both read against it. */
+  background-color: #9aa1ad;
+  background-image:
+    linear-gradient(45deg, #6f7783 25%, transparent 25%, transparent 75%, #6f7783 75%),
+    linear-gradient(45deg, #6f7783 25%, transparent 25%, transparent 75%, #6f7783 75%);
+  background-size: 12px 12px;
+  background-position: 0 0, 6px 6px;
+  padding: 2px;
+}
+.map-thumb-dark {
+  background-color: #222;
+  background-image: none;
 }
-.map-thumb-dark { background: #222; }
 .settings-success {
   margin-top: 1rem;
   padding: 0.75rem;
diff --git a/frontend/src/views/settings/BrandingSettings.vue b/frontend/src/views/settings/BrandingSettings.vue
index bf455bc2..dcb6294b 100644
--- a/frontend/src/views/settings/BrandingSettings.vue
+++ b/frontend/src/views/settings/BrandingSettings.vue
@@ -24,7 +24,7 @@
             >
             <div class="map-upload-row">
               <input type="file" :accept="logo.accept" @change="uploadLogo(logo.kind, logo.key, $event)" :disabled="brandingUploading" />
-              <img v-if="settings[logo.key]" :src="settings[logo.key]" class="map-thumb" :alt="logo.label" />
+              <img v-if="settings[logo.key]" :src="withBase(settings[logo.key])" class="map-thumb" :alt="logo.label" />
             </div>
             <small class="input-hint">{{ logo.hint }}</small>
           </label>
@@ -132,6 +132,12 @@
 <script setup>
 import { onMounted } from 'vue'
 import { useSystemSettings, brandingLogos } from '../../composables/systemSettings'
+// The stored value is root-relative ('/ge-aerospace-logo.svg'). This app is
+// mounted under a base path at every real deployment (/shopdb, /ops), so a raw
+// src resolves against the SERVER root and 404s - which is what "error loading
+// image" was. siteSettings.js already wraps every logo this way for the rest of
+// the app; this preview was the one place that did not.
+import { withBase } from '@/utils/basePath'
 
 const {
   settings, saving, brandingUploading, error, success,
diff --git a/frontend/src/views/settings/FloorMapSettings.vue b/frontend/src/views/settings/FloorMapSettings.vue
index fba81339..c134bcbd 100644
--- a/frontend/src/views/settings/FloorMapSettings.vue
+++ b/frontend/src/views/settings/FloorMapSettings.vue
@@ -26,6 +26,14 @@
         <button class="btn btn-small" @click="startAddLevel(building)">Add level</button>
       </div>
 
+      <!-- Scroll rather than squeeze: nine columns of inputs do not fit a
+           narrow window, and without a wrapper the table ran off the right of
+           the page with the cells crushed out of shape.
+           NOT the global .table-container - that also sets white-space: nowrap,
+           and the sizing hint below is a full sentence, so nowrap made the
+           table WIDER instead of letting the paragraph wrap. Scroll here, wrap
+           inside the cells. -->
+      <div class="level-table-scroll">
       <table class="data-table">
         <thead>
           <tr>
@@ -46,7 +54,7 @@
             <td>
               <input
                 v-model="level.levelname"
-                class="form-control"
+                class="form-control level-name-input"
                 @blur="renameLevel(level)"
                 :disabled="saving"
               />
@@ -123,6 +131,7 @@
           </tr>
         </tbody>
       </table>
+      </div>
     </div>
 
     <!-- Add building -->
@@ -437,6 +446,17 @@ async function remove(level) {
 }
 
 .mono { font-family: monospace; }
+/* Wide enough that every column keeps a usable width; past this the wrapper
+   scrolls instead of the inputs collapsing to a few pixels. */
+.level-table-scroll { overflow-x: auto; }
+.data-table { min-width: 58rem; }
+/* The sizing note is prose and must wrap; it is the reason the shared
+   table-container wrapper is wrong for this table. */
+.data-table .input-hint { white-space: normal; display: block; max-width: 22rem; }
+.data-table td, .data-table th { vertical-align: top; }
+/* A bare form-control in a table cell collapses to whatever width is left over,
+   which put "Ground floor" in a box one character wide. */
+.level-name-input { width: 11rem; }
 .order-input { width: 4.5rem; }
 .size-input { width: 6rem; }
 .size-fields { display: flex; align-items: center; gap: 0.35rem; }
diff --git a/plugins/computers/frontend/views/ActionPicker.vue b/plugins/computers/frontend/views/ActionPicker.vue
index 7111d195..f7b42572 100644
--- a/plugins/computers/frontend/views/ActionPicker.vue
+++ b/plugins/computers/frontend/views/ActionPicker.vue
@@ -63,7 +63,7 @@
             :placeholder="createkind === 'service' ? 'e.g. Spooler' : 'e.g. agent.exe'"
             v-model="createname"
             @keydown.enter.prevent="submitCreate"
-            @keydown.esc.prevent="cancelCreate"
+            @keydown.esc.prevent.stop="cancelCreate"
           />
           <template v-if="createkind === 'process'">
             <label class="action-create-label">
@@ -75,7 +75,7 @@
               placeholder="e.g. C:\Program Files (x86)\Microsoft\Edge\Application\msedge.exe"
               v-model="createlaunchpath"
               @keydown.enter.prevent="submitCreate"
-              @keydown.esc.prevent="cancelCreate"
+              @keydown.esc.prevent.stop="cancelCreate"
             />
           </template>
           <div class="action-create-actions">
diff --git a/scripts/check-naming-and-style.sh b/scripts/check-naming-and-style.sh
index 26e63818..b251c412 100755
--- a/scripts/check-naming-and-style.sh
+++ b/scripts/check-naming-and-style.sh
@@ -35,12 +35,18 @@ EXCLUDES=(
     --exclude-dir=.git
     --exclude-dir=versions
     --exclude-dir=staticdocs
-    # Local, gitignored scratch notes (see .gitignore devnotes/). Not product
-    # source, never committed, and free to hold pasted logs full of em-dashes
-    # and arrows - grep does not consult .gitignore, so exclude it explicitly.
-    --exclude-dir=devnotes
 )
 
+# Root-level gitignored scratch: review writeups and chat handoffs, free to hold
+# pasted logs full of em-dashes and arrows. grep does not consult .gitignore, so
+# it has to be dropped here.
+#
+# A POST-FILTER, not --exclude-dir=devnotes. That flag matches a directory of
+# that NAME at any depth, so a force-added plugins/<plugin>/devnotes/ would be
+# invisible to every check in this file, ADR-015 included - a hole in the gate
+# that nothing would report. This drops only the path we mean.
+drop_scratch() { grep -v '^\./devnotes/' || true; }
+
 INCLUDES_CODE=(
     --include='*.py'
     --include='*.vue'
@@ -64,7 +70,7 @@ INCLUDES_ALL=(
 # and did, including in files this repo's own convention forbids them in.
 # Markdown, JSON and YAML are now covered too.
 echo "==> Checking for non-ASCII characters..."
-NON_ASCII=$(grep -rPn '[^\x00-\x7F]' "${EXCLUDES[@]}" "${INCLUDES_ALL[@]}" . 2>/dev/null || true)
+NON_ASCII=$(grep -rPn '[^\x00-\x7F]' "${EXCLUDES[@]}" "${INCLUDES_ALL[@]}" . 2>/dev/null | drop_scratch || true)
 if [ -n "$NON_ASCII" ]; then
     echo "FAIL: non-ASCII characters found (em-dashes, smart quotes, arrows, emojis):"
     echo "$NON_ASCII"
@@ -89,7 +95,7 @@ for word in cfg ctx mgr req res; do
 done
 
 echo "==> Checking for snake_case DB tablenames..."
-SNAKE_TABLES=$(grep -rPn "__tablename__\s*=\s*['\"][^'\"]*_" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null || true)
+SNAKE_TABLES=$(grep -rPn "__tablename__\s*=\s*['\"][^'\"]*_" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null | drop_scratch || true)
 if [ -n "$SNAKE_TABLES" ]; then
     echo "FAIL: snake_case __tablename__ found (must be lowercase concatenated):"
     echo "$SNAKE_TABLES"
@@ -98,7 +104,7 @@ if [ -n "$SNAKE_TABLES" ]; then
 fi
 
 echo "==> Checking for snake_case DB column attrs..."
-SNAKE_COLS=$(grep -rPn "^\s+[a-z]+_[a-z_]+\s*=\s*db\.Column" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null || true)
+SNAKE_COLS=$(grep -rPn "^\s+[a-z]+_[a-z_]+\s*=\s*db\.Column" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null | drop_scratch || true)
 if [ -n "$SNAKE_COLS" ]; then
     echo "FAIL: snake_case db.Column attribute found (must match column name, no underscores):"
     echo "$SNAKE_COLS"
@@ -107,7 +113,7 @@ if [ -n "$SNAKE_COLS" ]; then
 fi
 
 echo "==> Checking for snake_case ForeignKey targets..."
-SNAKE_FK=$(grep -rPn "ForeignKey\(['\"][^'\"]*_[^'\"]*['\"]" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null || true)
+SNAKE_FK=$(grep -rPn "ForeignKey\(['\"][^'\"]*_[^'\"]*['\"]" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null | drop_scratch || true)
 if [ -n "$SNAKE_FK" ]; then
     echo "FAIL: snake_case ForeignKey target found:"
     echo "$SNAKE_FK"
@@ -116,7 +122,7 @@ if [ -n "$SNAKE_FK" ]; then
 fi
 
 echo "==> Checking for snake_case API params in frontend (DB-mirrored fields)..."
-SNAKE_FE=$(grep -rPn "params\.(machine_id|location_id|vendor_id|type_id|business_unit_id|model_id|status_id|operating_system_id|asset_id|user_id|is_active|is_shopfloor)" "${EXCLUDES[@]}" --include='*.vue' --include='*.js' --include='*.ts' . 2>/dev/null || true)
+SNAKE_FE=$(grep -rPn "params\.(machine_id|location_id|vendor_id|type_id|business_unit_id|model_id|status_id|operating_system_id|asset_id|user_id|is_active|is_shopfloor)" "${EXCLUDES[@]}" --include='*.vue' --include='*.js' --include='*.ts' . 2>/dev/null | drop_scratch || true)
 if [ -n "$SNAKE_FE" ]; then
     echo "FAIL: snake_case API params in frontend (must match DB column names without underscores):"
     echo "$SNAKE_FE"
diff --git a/shopdb/config.py b/shopdb/config.py
index 061ac7bf..662937f8 100644
--- a/shopdb/config.py
+++ b/shopdb/config.py
@@ -134,6 +134,22 @@ class Config:
     DEFAULT_PAGE_SIZE = 20
     MAX_PAGE_SIZE = 100
 
+    # Hard ceiling on a request body, enforced by Werkzeug before a view runs.
+    #
+    # Upload routes cap their own payloads, but every one of those checks runs
+    # AFTER the body has arrived: the first touch of request.form or
+    # request.files makes Werkzeug parse the whole multipart and spool the file
+    # part to a temp file, so a route can reject the content and still have paid
+    # for the transfer and the disk. Three separate reviews found that shape in
+    # three different routes, which makes it a property of the app rather than
+    # of any handler.
+    #
+    # 128 MB clears the largest thing anyone legitimately sends - the agent
+    # binary upload caps itself at 64 MB - with room for the multipart envelope.
+    # Over this, Werkzeug answers 413 without reading the body.
+    MAX_CONTENT_LENGTH = int(
+        os.environ.get('MAX_CONTENT_LENGTH', 128 * 1024 * 1024))
+
 
 class DevelopmentConfig(Config):
     """Development configuration."""
diff --git a/shopdb/core/models/base.py b/shopdb/core/models/base.py
index 9349418a..18e577b3 100644
--- a/shopdb/core/models/base.py
+++ b/shopdb/core/models/base.py
@@ -29,10 +29,37 @@ class BaseModel(db.Model):
     )
     isactive = db.Column(db.Boolean, default=True, nullable=False)
 
+    # Columns the default to_dict NEVER emits, whatever model carries them.
+    #
+    # The default serialiser walks __table__.columns and publishes every one, so
+    # a model that does not override it publishes whatever it has - including
+    # columns added to it later, by someone who was not thinking about this
+    # method. User is the live example: it does not override to_dict, so
+    # User.to_dict() returns passwordhash. Nothing calls it in a response path
+    # today, which is luck rather than design.
+    #
+    # A blanket allowlist would be the stronger rule, but it would also silently
+    # empty the output of all 34 models that rely on this default. This is the
+    # part that can be fixed without breaking anything: name the columns that
+    # must never be published, and let a model add its own.
+    #
+    # A model with something sensitive should still override to_dict and say
+    # exactly what it emits. This is a floor, not permission to skip that.
+    HIDDEN_COLUMNS = frozenset({
+        'passwordhash',
+        'tokenhash',
+        'apikey',
+        'clientsecret',
+        'collectorkey',
+    })
+
     def to_dict(self):
-        """Convert model to dictionary."""
+        """Convert model to dictionary, minus anything secret-bearing."""
+        hidden = self.HIDDEN_COLUMNS
         result = {}
         for c in self.__table__.columns:
+            if c.name in hidden:
+                continue
             value = getattr(self, c.name)
             if isinstance(value, datetime):
                 value = value.isoformat() + 'Z'
diff --git a/tests/test_core/test_to_dict_floor.py b/tests/test_core/test_to_dict_floor.py
new file mode 100644
index 00000000..1ca415b9
--- /dev/null
+++ b/tests/test_core/test_to_dict_floor.py
@@ -0,0 +1,69 @@
+"""BaseModel.to_dict never publishes a secret-bearing column.
+
+The default serialiser walks __table__.columns and emits every one, so a model
+that does not override it publishes whatever it happens to carry - including
+columns added later by someone not thinking about this method. User is the live
+example: it has no override, so User.to_dict() returned its bcrypt hash. Nothing
+called it in a response path, which was luck.
+
+A blanket allowlist is the stronger rule and would empty the output of every
+model relying on the default, so this pins the floor instead: named columns are
+never emitted, and a model may extend the set.
+"""
+
+import pytest
+
+from shopdb.core.models import User
+from shopdb.core.models.base import BaseModel
+
+
+def test_the_default_serialiser_hides_a_password_hash(app):
+    with app.app_context():
+        user = User(username='floor', email='floor@example.invalid',
+                    passwordhash='$2b$12$notarealhashbutlookslikeone')
+        data = user.to_dict()
+        assert 'passwordhash' not in data
+        # Still a useful dict - this is a filter, not a mute button.
+        assert data['username'] == 'floor'
+        assert data['email'] == 'floor@example.invalid'
+
+
+def test_every_hidden_name_is_actually_dropped(app):
+    """Each entry earns its place: drop one and the column comes back."""
+    with app.app_context():
+        user = User(username='floor2', email='f2@example.invalid',
+                    passwordhash='x')
+        assert 'passwordhash' in {c.name for c in User.__table__.columns}
+        assert 'passwordhash' in BaseModel.HIDDEN_COLUMNS
+        assert 'passwordhash' not in user.to_dict()
+
+
+def test_a_model_can_extend_the_hidden_set(app):
+    """A model with its own secret adds to the floor rather than replacing
+    to_dict wholesale."""
+    with app.app_context():
+        class Extended(User):
+            __abstract__ = True
+            HIDDEN_COLUMNS = BaseModel.HIDDEN_COLUMNS | {'email'}
+
+        assert 'passwordhash' in Extended.HIDDEN_COLUMNS
+        assert 'email' in Extended.HIDDEN_COLUMNS
+
+
+@pytest.mark.parametrize('name', sorted(BaseModel.HIDDEN_COLUMNS))
+def test_no_model_publishes_a_hidden_column_through_the_default(name, app):
+    """Sweep every mapped model: if it carries a hidden column AND relies on the
+    default to_dict, the column must not come out."""
+    with app.app_context():
+        from shopdb.extensions import db
+
+        for mapper in db.Model.registry.mappers:
+            model = mapper.class_
+            table = getattr(model, '__table__', None)
+            if table is None or name not in table.columns:
+                continue
+            if 'to_dict' in vars(model):
+                continue  # overrides the default; its own business
+            assert name in getattr(model, 'HIDDEN_COLUMNS', frozenset()), (
+                '%s carries %s and uses the default to_dict, so it would be '
+                'published' % (model.__name__, name))