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.
This commit is contained in:
cproudlock
2026-08-14 13:46:53 -04:00
parent d830dd49a9
commit c7dffce81e
12 changed files with 229 additions and 31 deletions

View File

@@ -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

View File

@@ -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',

View File

@@ -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/<path:filename>', 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('/<int:app_id>/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('/<int:app_id>/package', methods=['DELETE'])

View File

@@ -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/<path:filename>', 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('/<int:model_id>/image', methods=['DELETE'])

View File

@@ -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 []

46
shopdb/utils/uploads.py Normal file
View File

@@ -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 <script>, and it is wanted
for floor-plan maps and branding, where vector is the right format. Loaded
through an <img> tag a script inside it never runs, so the tiles and maps in
this app were never the risk. Opening the file's own URL is - and the
application image route is public, so the URL is reachable without a session.
Rather than banning the format that has a real use, every uploaded file is
served with headers that make the document case inert:
Content-Security-Policy: default-src 'none'; sandbox
No script, no fetch, no plugin, and a unique opaque origin. This is what
stops an SVG (or an HTML file that talked its way past an allowlist)
executing in the site's origin and reading a token out of localStorage.
X-Content-Type-Options: nosniff
A file whose extension and bytes disagree is not re-typed by the browser
into something executable.
Both are cheap and apply to every upload route, so a new one added later starts
from the same place instead of repeating the reasoning.
"""
from flask import send_from_directory
# 'sandbox' with no allow-* tokens: opaque origin, scripts blocked, forms and
# popups blocked. default-src 'none' is belt and braces for a UA that ignores
# sandbox in a header.
UPLOAD_CSP = "default-src 'none'; sandbox"
def harden_upload_response(response):
"""Apply the inert-document headers to a response serving an upload."""
response.headers['Content-Security-Policy'] = UPLOAD_CSP
response.headers['X-Content-Type-Options'] = 'nosniff'
return response
def send_upload(directory, filename, **kwargs):
"""send_from_directory, with the upload headers applied.
Any keyword send_from_directory takes is passed through, so a route that
already sends as an attachment keeps doing so.
"""
return harden_upload_response(send_from_directory(directory, filename, **kwargs))