From c7dffce81eb953ddb8ea8c56730a39b4cc455264 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Fri, 14 Aug 2026 13:46:53 -0400 Subject: [PATCH] Serve an uploaded file as data, not as a document that can run An SVG is an XML document that may carry a script, and it is an accepted image type because floor-plan maps and branding genuinely want vector. Loaded through an img tag that script never runs, so the tiles and maps were never the risk. Opening the file's own URL is - and the application image route is public, so that URL needs no session. Every route that serves an upload now goes through one helper that sends Content-Security-Policy: default-src 'none'; sandbox, and nosniff. Seven routes across core and five plugins, so a new one added later starts from the same place rather than repeating the reasoning. Banning the format instead would have cost the maps their only sensible one. The app also sent no security headers at all. It now sets nosniff, frame-ancestors self (as X-Frame-Options too, for the display bays' browsers) and a referrer policy. Deliberately NOT a page-wide CSP: this serves an SPA with inline styles, so a real script-src policy is a change worth making with the frontend in front of you, and a permissive header claiming one would be worse than having none. Contract 0.19.0. send_upload is on the shopdb.api surface, because a plugin serving user-supplied bytes should not have to remember these headers. The same bump records that get_dashboard_widgets has taken data and shape rather than a component name since the dashboard was rebuilt - that shipped without a bump, while BasePlugin and PLUGIN-HOOKS.md both still documented the shape nothing renders, which is how five plugins came to declare widgets pointing at components nobody had written. --- docs/PLUGIN-HOOKS.md | 35 +++++++++++---- plugins/employees/api/routes.py | 5 ++- plugins/printedparts/api/routes.py | 7 ++- plugins/slides/api/routes.py | 5 ++- plugins/warranty/api/routes.py | 5 ++- shopdb/__init__.py | 46 ++++++++++++++++++- shopdb/api/__init__.py | 8 ++++ shopdb/core/api/applications.py | 7 +-- shopdb/core/api/models.py | 5 ++- shopdb/plugins/base.py | 34 +++++++++++--- shopdb/utils/uploads.py | 46 +++++++++++++++++++ tests/test_core/test_security_headers.py | 57 ++++++++++++++++++++++++ 12 files changed, 229 insertions(+), 31 deletions(-) create mode 100644 shopdb/utils/uploads.py create mode 100644 tests/test_core/test_security_headers.py diff --git a/docs/PLUGIN-HOOKS.md b/docs/PLUGIN-HOOKS.md index 0540b74..fcf2522 100644 --- a/docs/PLUGIN-HOOKS.md +++ b/docs/PLUGIN-HOOKS.md @@ -9,7 +9,7 @@ The contract is locked in [ADR-001](../docs/adr/ADR-001-asset-as-platform-contra The framework declares its contract version in `shopdb/__init__.py`: ```python -__contract_version__ = '0.18.0' +__contract_version__ = '0.19.0' ``` Each plugin's `manifest.json` declares the range of contract versions it supports: @@ -154,20 +154,39 @@ class PrintersPlugin(BasePlugin): ### `get_dashboard_widgets() -> List[Dict]` -Returns dashboard widget definitions for the home page. +Returns dashboard card definitions for the home page. + +A card declares DATA AND SHAPE, never a component name. Core owns a small set of +generic renderers and draws the card; the plugin says what to show, where it +comes from, and how to link it. + +**Changed in contract 0.19.0.** The previous shape named a Vue component per +widget (`'component': 'NotificationsWidget'`). That cannot survive a lean build, +because a plugin's component may never be staged into the frontend bundle +(ADR-013), and in practice five plugins declared widgets pointing at components +nobody had written - so they rendered as nothing. A card using the old shape is +ignored. This is the same correction ADR-010 already made for asset panels. ```python -class NotificationsPlugin(BasePlugin): +class GeEnforcePlugin(BasePlugin): def get_dashboard_widgets(self): return [{ - 'name': 'recent_notifications', - 'component': 'NotificationsWidget', - 'endpoint': '/api/notifications/recent', - 'size': 'medium', - 'position': 1, + 'id': 'geenforce-failures', # stable, unique across plugins + 'title': 'Enforcement failures', + 'endpoint': '/api/geenforce/dashboard/failures', + 'render': 'exceptions', # a core renderer, not a component + 'severity': 'critical', # orders cards on the page + 'permission': 'geenforce.manage', # hidden without it + 'empty': 'hide', # say nothing when there is nothing + 'position': 10, + 'viewall': '/geenforce', # optional link behind the heading + 'map': {'title': 'hostname', 'detail': 'entryname'}, }] ``` +`empty: 'hide'` is not cosmetic. A card that reports "nothing wrong" every day +teaches people to stop reading the page. + Consumed by `GET /api/dashboard/widgets`, which merges widgets from all enabled plugins sorted by `position` (disabled plugins are skipped; a broken plugin is isolated in prod, re-raised in dev/test). diff --git a/plugins/employees/api/routes.py b/plugins/employees/api/routes.py index 72162e1..92d1736 100644 --- a/plugins/employees/api/routes.py +++ b/plugins/employees/api/routes.py @@ -13,7 +13,7 @@ import io import logging import os -from flask import Blueprint, request, current_app, send_from_directory +from flask import Blueprint, request, current_app from flask_jwt_extended import jwt_required from werkzeug.utils import secure_filename @@ -26,6 +26,7 @@ from shopdb.api import ( require_role, ) from shopdb.api import Setting +from shopdb.api import send_upload from ..models import DirectoryEmployee @@ -612,7 +613,7 @@ def upload_employee_photo(sso): @employees_bp.route('/photo/', methods=['GET']) def serve_employee_photo(filename): """Serve an uploaded employee photo (public - kiosk cards read it).""" - return send_from_directory(_employeephoto_dir(), filename) + return send_upload(_employeephoto_dir(), filename) @employees_bp.route('//photo', methods=['DELETE']) diff --git a/plugins/printedparts/api/routes.py b/plugins/printedparts/api/routes.py index 877244e..0c0ac30 100644 --- a/plugins/printedparts/api/routes.py +++ b/plugins/printedparts/api/routes.py @@ -236,8 +236,7 @@ def upload_item_image(item_id: int): @printedparts_bp.route('/image/', methods=['GET']) def serve_item_image(filename): """Serve an uploaded item image (public - kiosk and list read it).""" - from flask import send_from_directory - return send_from_directory(_imagedir(), filename) + return send_upload(_imagedir(), filename) @printedparts_bp.route('/items//image', methods=['DELETE']) @@ -643,6 +642,7 @@ def report_by_person(): # --- print files: append-only revisions per item ------------------------------ from flask_jwt_extended import get_jwt_identity +from shopdb.api import send_upload from ..models import PrintedItemFile @@ -737,12 +737,11 @@ def upload_item_file(item_id: int): @jwt_required(optional=True) def download_item_file(file_id: int): """Download a revision under its original filename.""" - from flask import send_from_directory record = db.session.get(PrintedItemFile, file_id) if not record: return error_response(ErrorCodes.NOT_FOUND, 'File not found', http_code=404) - return send_from_directory(_filedir(), record.storedfilename, + return send_upload(_filedir(), record.storedfilename, as_attachment=True, download_name=record.filename) diff --git a/plugins/slides/api/routes.py b/plugins/slides/api/routes.py index 40c5ccd..dc42a99 100644 --- a/plugins/slides/api/routes.py +++ b/plugins/slides/api/routes.py @@ -9,12 +9,13 @@ have it by default). import os import re -from flask import Blueprint, request, current_app, jsonify, send_from_directory +from flask import Blueprint, request, current_app, jsonify from flask_jwt_extended import jwt_required from werkzeug.utils import secure_filename from shopdb.api import (db, success_response, error_response, ErrorCodes, require_permission) +from shopdb.api import send_upload from ..models import TvSlide @@ -81,7 +82,7 @@ def serve_image(surface, filename): directory = _surface_dir(surface) if not os.path.isfile(os.path.join(directory, safe)): return error_response(ErrorCodes.NOT_FOUND, 'Slide not found', http_code=404) - return send_from_directory(directory, safe) + return send_upload(directory, safe) # ============================================================================= diff --git a/plugins/warranty/api/routes.py b/plugins/warranty/api/routes.py index 11011a4..dd8ef34 100644 --- a/plugins/warranty/api/routes.py +++ b/plugins/warranty/api/routes.py @@ -9,11 +9,12 @@ import glob import os from datetime import date, datetime, timedelta, timezone -from flask import Blueprint, request, current_app, send_from_directory +from flask import Blueprint, request, current_app from flask_jwt_extended import jwt_required from sqlalchemy.orm import joinedload from werkzeug.utils import secure_filename +from shopdb.api import send_upload from shopdb.api import ( db, Asset, success_response, error_response, ErrorCodes, @@ -559,7 +560,7 @@ def serve_proof(filename): Warranty.proofurl == f'{PROOF_URL_PREFIX}{filename}').first() downloadname = (warranty.prooffilename if warranty and warranty.prooffilename else filename) - return send_from_directory(_proof_dir(), filename, as_attachment=True, + return send_upload(_proof_dir(), filename, as_attachment=True, download_name=downloadname) diff --git a/shopdb/__init__.py b/shopdb/__init__.py index 5e6d6cc..d3db59a 100644 --- a/shopdb/__init__.py +++ b/shopdb/__init__.py @@ -48,7 +48,16 @@ from .plugins import plugin_manager # the literal values of C:\Enrollment\display-type.txt - so a plugin holding its # own copy of that map (geenforce did) can read core's instead of drifting from # it. Additive names on the import surface, minor bump. -__contract_version__ = '0.18.0' +# 0.19.0: get_dashboard_widgets returns DATA AND SHAPE, not a component name. +# The old shape ('name' + 'component' + 'size') named a Vue component per +# widget, which cannot survive a lean build - a plugin's component may never be +# staged into the frontend bundle (ADR-013) - and five plugins were declaring +# widgets that pointed at components nobody had written. Core now owns the +# renderers and a card declares id / title / endpoint / render / severity / +# permission / empty / position. BREAKING for any plugin still using the old +# shape, which is why it is recorded here: the change itself shipped earlier +# without a bump, and a contract that changes silently is not a contract. +__contract_version__ = '0.19.0' # Product release version (see ADR-007). The product version and the # plugin-contract version above are distinct series with independent @@ -134,6 +143,41 @@ def create_app(config_name: str = None) -> Flask: identity = jwt_data["sub"] return db.session.get(User, int(identity)) + @app.after_request + def apply_security_headers(response): + """Baseline response headers. The app shipped with none of these. + + Deliberately the three that cost nothing and break nothing: + + nosniff a response whose bytes and declared type disagree is + not re-typed by the browser into something executable. + This is the general form of the upload problem that + shopdb/utils/uploads.py addresses per-file. + frame-ancestors + same-origin only. The kiosks open routes directly + rather than framing them, so this costs the fleet + nothing and stops the UI being framed elsewhere and + clicked through. Sent as X-Frame-Options too, because + the display bays run browsers old enough to want it. + Referrer-Policy + an asset id or a hostname in a path is not handed to + whatever a user clicks through to. + + NOT a full page CSP. This app serves an SPA with inline styles, so a + real script-src policy is a change worth making on its own with the + frontend in front of you - claiming one here by adding a permissive + header would be worse than having none. + """ + response.headers.setdefault('X-Content-Type-Options', 'nosniff') + response.headers.setdefault('X-Frame-Options', 'SAMEORIGIN') + response.headers.setdefault('Referrer-Policy', + 'strict-origin-when-cross-origin') + # setdefault: an upload response has already declared its own, stricter + # Content-Security-Policy, and this must not weaken it. + response.headers.setdefault('Content-Security-Policy', + "frame-ancestors 'self'") + return response + return app diff --git a/shopdb/api/__init__.py b/shopdb/api/__init__.py index a4df277..a196f28 100644 --- a/shopdb/api/__init__.py +++ b/shopdb/api/__init__.py @@ -77,6 +77,11 @@ from shopdb.utils.apitoken_auth import ( service_token_authorized, authorized_service_token, ) +# Serving an uploaded file safely. A plugin that serves user-supplied bytes must +# not have to remember the headers that keep an SVG or an HTML file from running +# as script in this origin - see shopdb/utils/uploads.py. +from shopdb.utils.uploads import send_upload, harden_upload_response + # Import-mode helpers: preserve legacy timestamps during a bulk data import from shopdb.utils.import_mode import ( apply_import_timestamps, @@ -283,6 +288,9 @@ __all__ = [ 'service_token_authorized', 'authorized_service_token', 'SupportTeam', + # Serving uploads + 'send_upload', + 'harden_upload_response', # Import-mode helpers 'apply_import_timestamps', 'import_mode_active', diff --git a/shopdb/core/api/applications.py b/shopdb/core/api/applications.py index 54d859c..20ded43 100644 --- a/shopdb/core/api/applications.py +++ b/shopdb/core/api/applications.py @@ -3,7 +3,7 @@ import glob import os -from flask import Blueprint, request, current_app, send_from_directory +from flask import Blueprint, request, current_app from flask_jwt_extended import jwt_required from werkzeug.utils import secure_filename @@ -112,6 +112,7 @@ def _require_computer_models(): from shopdb.utils.authz import require_permission +from shopdb.utils.uploads import send_upload applications_bp = Blueprint('applications', __name__) @@ -550,7 +551,7 @@ def upload_application_image(app_id: int): @applications_bp.route('/image/', methods=['GET']) def serve_application_image(filename): """Serve an uploaded application image (public - lists and tiles read it).""" - return send_from_directory(_appimage_dir(), filename) + return send_upload(_appimage_dir(), filename) @applications_bp.route('//image', methods=['DELETE']) @@ -635,7 +636,7 @@ def serve_application_package(filename): open URL would publish it to anything that can reach the site. Always sent as an attachment so a browser saves it rather than trying to render it. """ - return send_from_directory(_apppackage_dir(), filename, as_attachment=True) + return send_upload(_apppackage_dir(), filename, as_attachment=True) @applications_bp.route('//package', methods=['DELETE']) diff --git a/shopdb/core/api/models.py b/shopdb/core/api/models.py index e3688bd..7eac480 100644 --- a/shopdb/core/api/models.py +++ b/shopdb/core/api/models.py @@ -3,7 +3,7 @@ import glob import os -from flask import Blueprint, request, current_app, send_from_directory +from flask import Blueprint, request, current_app from flask_jwt_extended import jwt_required from werkzeug.utils import secure_filename @@ -19,6 +19,7 @@ from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.authz import require_role from shopdb.utils.import_mode import apply_import_timestamps +from shopdb.utils.uploads import send_upload models_bp = Blueprint('models', __name__) @@ -228,7 +229,7 @@ def upload_model_image(model_id: int): @models_bp.route('/image/', methods=['GET']) def serve_model_image(filename): """Serve an uploaded model image (public - asset detail pages read it).""" - return send_from_directory(_modelimage_dir(), filename) + return send_upload(_modelimage_dir(), filename) @models_bp.route('//image', methods=['DELETE']) diff --git a/shopdb/plugins/base.py b/shopdb/plugins/base.py index bb4ebc2..4843f4b 100644 --- a/shopdb/plugins/base.py +++ b/shopdb/plugins/base.py @@ -205,15 +205,35 @@ class BasePlugin(ABC): def get_dashboard_widgets(self) -> List[Dict]: """ - Return dashboard widget definitions. + Return dashboard card definitions. - Each widget: { - 'name': str, - 'component': str, # Frontend component name - 'endpoint': str, # API endpoint for data - 'size': str, # 'small', 'medium', 'large' - 'position': int # Order on dashboard + DATA AND SHAPE, not a component name. The card is drawn by one of core's + generic renderers; the plugin says what to show, where it comes from and + how to link it. + + Contract 0.19.0 replaced a shape that named a Vue component per widget. + That could not survive a lean build - a plugin's component may never be + staged into the frontend bundle (ADR-013) - and in practice five plugins + declared widgets pointing at components nobody had written, which + rendered as nothing at all. It is the same correction ADR-010 already + made for asset panels. + + Each card: { + 'id': str, # stable, unique across plugins + 'title': str, # heading + 'endpoint': str, # API path the card fetches its own data from + 'render': str, # core renderer: 'exceptions' | 'list' | 'stat' + 'severity': str, # 'critical' | 'warning' | 'info' - orders cards + 'permission': str, # card is hidden without it + 'empty': str, # 'hide' when nothing to report, or empty text + 'position': int, # order within a severity + 'viewall': str, # optional route behind the card heading + 'map': dict, # optional field mapping for the renderer, + # e.g. {'title': 'hostname', 'detail': 'entryname'} } + + `empty: 'hide'` is not cosmetic. A card that reports "nothing wrong" + every day teaches people to stop reading the page. """ return [] diff --git a/shopdb/utils/uploads.py b/shopdb/utils/uploads.py new file mode 100644 index 0000000..bce1707 --- /dev/null +++ b/shopdb/utils/uploads.py @@ -0,0 +1,46 @@ +"""Serving user-uploaded files without handing the browser a script. + +Uploads reach these routes from people, and one of the accepted image types is +not inert: an SVG is an XML document that may carry ') + path = directory / 'probe-headers.svg' if hasattr(directory, '__truediv__') \ + else None + if path is None: + import os + path = os.path.join(directory, 'probe-headers.svg') + with open(path, 'w') as handle: + handle.write(svg) + else: + path.write_text(svg) + + try: + resp = client.get('/api/applications/image/probe-headers.svg') + assert resp.status_code == 200 + assert resp.headers.get('Content-Security-Policy') == UPLOAD_CSP + assert resp.headers.get('X-Content-Type-Options') == 'nosniff' + finally: + import os + os.remove(path)