Split the fleet roster from per-endpoint reads, and stop offering controls nobody can use
All checks were successful
CI / naming (push) Successful in 3s
CI / frontend (push) Successful in 14s
CI / migrations-mysql (push) Successful in 45s
CI / backend (push) Successful in 4m34s

Twelve endpoints routes shared one permission, which made a reasonable policy
impossible to express. A site that wants signed-out visitors to see live status
on a PC page had to hand them the entire fleet - including every logged-in
username on the floor - because the per-asset panels and the fleet-wide list
were gated on the same string.

endpoints.fleet now gates the list and the three dashboard routes.
endpoints.view keeps the per-asset reads: status, services, processes,
interfaces, events, command history.

The name is load-bearing. guest_grantable_permissions() opens any '.view' or
'.read' permission to the guest role, so calling this one 'endpoints.fleet'
rather than 'endpoints.fleetview' is what keeps the roster off the public
surface while per-asset telemetry stays grantable. Verified both ways.

Also: the services panel emitted Start/Stop row actions to every reader,
because row_actions_for() never consulted the caller. Clicking was always
refused server-side, so nothing was exploitable, but showing a read-only viewer
controls over a shop-floor PC is the wrong signal - and it stops being cosmetic
the moment a site grants that panel to guests.

That needed a contract addition rather than a local check: caller_holds() on
shopdb.api answers whether THIS caller holds a permission without refusing the
request, which is the complement to require_permission for a surface that
serves everyone but should only offer what the caller can act on. Guests are
included, since a signed-out visitor holds whatever the guest role was granted.
Reaching into shopdb.utils.authz instead would repeat the ADR-002 violation the
contract gate caught earlier in this branch. Contract 0.24.0 -> 0.25.0.

DEPLOYMENT: endpoints.fleet is a new permission. Run 'flask seed permissions'
and grant it to the roles that should keep seeing the Fleet page. Until then
that page is admin-only, since admins bypass every check - a safe direction,
but not an automatic one.

Full suite 2124 passed, naming and contract gates green, map and spec
regenerated.
This commit is contained in:
cproudlock
2026-08-28 21:14:27 -04:00
parent 4ef22b6579
commit 54f0857e13
7 changed files with 72 additions and 13 deletions

View File

@@ -44,6 +44,7 @@ Recorded in the comment block in `shopdb/__init__.py`:
| 0.22.0 | Added the `register_import_hook` surface: a plugin owns a VIRTUAL COLUMN on an importable CSV table - a header the bulk importer accepts but never writes - plus a post-write row hook that turns the value into a plugin concept. The computers plugin uses it for `machineassetnumber` on `computers.csv`, which builds the PC->machine controls link, so a site writes the machine number in the PC's own row instead of hand-building a separate relationships sheet. Core never learns what the column means. `RowWarning` is exported with it so a hook can report a soft failure without aborting the import | additive surface (minor) | | 0.22.0 | Added the `register_import_hook` surface: a plugin owns a VIRTUAL COLUMN on an importable CSV table - a header the bulk importer accepts but never writes - plus a post-write row hook that turns the value into a plugin concept. The computers plugin uses it for `machineassetnumber` on `computers.csv`, which builds the PC->machine controls link, so a site writes the machine number in the PC's own row instead of hand-building a separate relationships sheet. Core never learns what the column means. `RowWarning` is exported with it so a hook can report a soft failure without aborting the import | additive surface (minor) |
| 0.23.0 | Added `mint_service_token`, `list_service_tokens` and `revoke_service_tokens` so a plugin can provision, list and revoke a single-scope managed service token without importing core token internals. The endpoints plugin uses them for a UI-managed, regenerable agent key scoped to `collector.ingest.endpoints` alone - narrow on purpose, so rotating or revoking it cannot disturb the computers or GE-Enforce collectors, and revocation is an instant kill switch on agent ingest | additive surface (minor) | | 0.23.0 | Added `mint_service_token`, `list_service_tokens` and `revoke_service_tokens` so a plugin can provision, list and revoke a single-scope managed service token without importing core token internals. The endpoints plugin uses them for a UI-managed, regenerable agent key scoped to `collector.ingest.endpoints` alone - narrow on purpose, so rotating or revoking it cannot disturb the computers or GE-Enforce collectors, and revocation is an instant kill switch on agent ingest | additive surface (minor) |
| 0.24.0 | Added `collector_authorized(pluginname)` so a plugin can gate an agent-facing route that lives OUTSIDE the generic collector blueprint on the same per-plugin key and kill switch the ingest route uses. The endpoints command channel needs it: the agent long-polls `/api/endpoints/commands/next` and posts results back, and those are agent callbacks rather than JWT routes. Without it a plugin would reach into core collector internals to authorize them | additive surface (minor) | | 0.24.0 | Added `collector_authorized(pluginname)` so a plugin can gate an agent-facing route that lives OUTSIDE the generic collector blueprint on the same per-plugin key and kill switch the ingest route uses. The endpoints command channel needs it: the agent long-polls `/api/endpoints/commands/next` and posts results back, and those are agent callbacks rather than JWT routes. Without it a plugin would reach into core collector internals to authorize them | additive surface (minor) |
| 0.25.0 | Added `caller_holds(permission)`, which answers whether the current caller holds a permission WITHOUT refusing the request - the complement to `require_permission` for a route that serves everyone but should only offer what the caller can act on. The endpoints services panel is the case: it emitted Start/Stop row actions to every reader, because the emission never consulted the caller. Clicking them was always refused server-side, so nothing was exploitable, but offering a read-only viewer controls over a shop-floor PC is the wrong signal, and it becomes a real problem once a site grants that panel to guests. Guests are included in the answer, since a signed-out visitor holds whatever the guest role was granted | additive surface (minor) |
The source comment block documents 0.3.0, 0.4.0, 0.6.0, 0.7.0, 0.9.0, 0.10.0, The source comment block documents 0.3.0, 0.4.0, 0.6.0, 0.7.0, 0.9.0, 0.10.0,
0.11.0, 0.16.0, 0.18.0 and 0.19.0; 0.12.0 through 0.15.0 and 0.17.0 are recorded 0.11.0, 0.16.0, 0.18.0 and 0.19.0; 0.12.0 through 0.15.0 and 0.17.0 are recorded

View File

@@ -12,7 +12,7 @@ never by editing this file.
| series | value | governed by | | series | value | governed by |
|---|---|---| |---|---|---|
| product `__version__` | `0.12.0` | ADR-007 | | product `__version__` | `0.12.0` | ADR-007 |
| plugin contract `__contract_version__` | `0.24.0` | ADR-002 | | plugin contract `__contract_version__` | `0.25.0` | ADR-002 |
They move independently. A contract bump is not a release. They move independently. A contract bump is not a release.
@@ -90,6 +90,6 @@ Manifest-less directories under `plugins/` are core frontend surface and always
## Size ## Size
- test functions defined: **1448** (parametrised cases collect higher) - test functions defined: **1449** (parametrised cases collect higher)
- documented API paths: **331** (`docs/openapi.json`, regenerate with `scripts/gen_openapi.py`) - documented API paths: **331** (`docs/openapi.json`, regenerate with `scripts/gen_openapi.py`)

View File

@@ -16,7 +16,8 @@ from flask_jwt_extended import current_user
from shopdb.api import ( from shopdb.api import (
db, Asset, Setting, success_response, error_response, paginated_response, db, Asset, Setting, success_response, error_response, paginated_response,
ErrorCodes, get_pagination_params, paginate_query, require_permission, ErrorCodes, get_pagination_params, paginate_query, require_permission,
require_role, mint_service_token, list_service_tokens, revoke_service_tokens, require_role, caller_holds, mint_service_token, list_service_tokens,
revoke_service_tokens,
AuditLog, collector_authorized, AuditLog, collector_authorized,
) )
@@ -72,7 +73,7 @@ def _format_uptime(seconds):
# ============================================================================= # =============================================================================
@endpoints_bp.route('', methods=['GET']) @endpoints_bp.route('', methods=['GET'])
@require_permission('endpoints.view') @require_permission('endpoints.fleet')
def list_endpoints(): def list_endpoints():
"""List reported endpoint states. """List reported endpoint states.
@@ -408,6 +409,8 @@ def panel_services(asset_id: int):
if not state or not state.servicesjson: if not state or not state.servicesjson:
return success_response([]) return success_response([])
maycommand = caller_holds('endpoints.command')
try: try:
services = json.loads(state.servicesjson) services = json.loads(state.servicesjson)
except (TypeError, ValueError): except (TypeError, ValueError):
@@ -429,7 +432,11 @@ def panel_services(asset_id: int):
'statelabel': 'Running' if running else (raw.title() or 'Unknown'), 'statelabel': 'Running' if running else (raw.title() or 'Unknown'),
'statecolor': 'green' if running else ('red' if raw else 'gray'), 'statecolor': 'green' if running else ('red' if raw else 'gray'),
} }
targets = row_actions_for(app, kind, name) # Only offer Start/Stop to a caller who could actually run it. The
# emission never consulted the caller, so a read-only viewer was shown
# controls on a shop-floor PC that 403 on click - wrong signal, and it
# gets worse the moment guests can read this panel.
targets = row_actions_for(app, kind, name) if maycommand else None
if targets: if targets:
row['action'] = { row['action'] = {
'running': running, 'running': running,
@@ -512,7 +519,7 @@ def panel_interfaces(asset_id: int):
# ============================================================================= # =============================================================================
@endpoints_bp.route('/dashboard/summary', methods=['GET']) @endpoints_bp.route('/dashboard/summary', methods=['GET'])
@require_permission('endpoints.view') @require_permission('endpoints.fleet')
def dashboard_summary(): def dashboard_summary():
"""High-level fleet counts for the endpoints dashboard.""" """High-level fleet counts for the endpoints dashboard."""
total = db.session.query(EndpointState).join( total = db.session.query(EndpointState).join(
@@ -551,7 +558,7 @@ def dashboard_summary():
@endpoints_bp.route('/dashboard/quiet', methods=['GET']) @endpoints_bp.route('/dashboard/quiet', methods=['GET'])
@require_permission('endpoints.view') @require_permission('endpoints.fleet')
def dashboard_quiet(): def dashboard_quiet():
"""Endpoints that reported before, then went quiet past the window. """Endpoints that reported before, then went quiet past the window.
@@ -586,7 +593,7 @@ def dashboard_quiet():
@endpoints_bp.route('/dashboard/stability', methods=['GET']) @endpoints_bp.route('/dashboard/stability', methods=['GET'])
@require_permission('endpoints.view') @require_permission('endpoints.fleet')
def dashboard_stability(): def dashboard_stability():
"""Endpoints with recent crash/shutdown/corruption events. """Endpoints with recent crash/shutdown/corruption events.

View File

@@ -103,6 +103,22 @@ class EndpointsPlugin(BasePlugin):
""" """
return [ return [
('endpoints.view', 'View endpoint telemetry', 'endpoints'), ('endpoints.view', 'View endpoint telemetry', 'endpoints'),
# The FLEET-WIDE list and dashboards, separate from endpoints.view,
# which covers reading ONE endpoint that the caller already reached
# through its PC page.
#
# They were one permission, and that made a reasonable policy
# impossible to express: a site that wants signed-out visitors to
# see live status on a PC page had to hand them the whole fleet,
# including every logged-in username on the floor, because the
# panels and the list were gated on the same string.
#
# It is deliberately NOT named '.view'. guest_grantable_permissions
# opens any '.view' or '.read' permission to the guest role, so this
# name is what keeps the fleet roster off the public surface while
# per-asset telemetry can still be granted.
('endpoints.fleet',
'View the fleet-wide endpoint list and dashboards', 'endpoints'),
# Issue an allowlisted command to an endpoint agent (for example the # Issue an allowlisted command to an endpoint agent (for example the
# MTConnect toggle). Separate from endpoints.view because acting on a # MTConnect toggle). Separate from endpoints.view because acting on a
# shop-floor PC is a heavier grant than reading its telemetry. # shop-floor PC is a heavier grant than reading its telemetry.
@@ -189,7 +205,7 @@ class EndpointsPlugin(BasePlugin):
'icon': 'activity', 'icon': 'activity',
'route': '/endpoints', 'route': '/endpoints',
'position': 16, 'position': 16,
'permission': 'endpoints.view', 'permission': 'endpoints.fleet',
'section': 'assets', 'section': 'assets',
}, },
] ]
@@ -340,7 +356,7 @@ class EndpointsPlugin(BasePlugin):
'endpoint': '/api/endpoints/dashboard/quiet', 'endpoint': '/api/endpoints/dashboard/quiet',
'render': 'exceptions', 'render': 'exceptions',
'severity': 'warning', 'severity': 'warning',
'permission': 'endpoints.view', 'permission': 'endpoints.fleet',
'empty': 'hide', 'empty': 'hide',
'position': 22, 'position': 22,
'map': { 'map': {
@@ -358,7 +374,7 @@ class EndpointsPlugin(BasePlugin):
'endpoint': '/api/endpoints/dashboard/stability', 'endpoint': '/api/endpoints/dashboard/stability',
'render': 'exceptions', 'render': 'exceptions',
'severity': 'critical', 'severity': 'critical',
'permission': 'endpoints.view', 'permission': 'endpoints.fleet',
'empty': 'hide', 'empty': 'hide',
'position': 16, 'position': 16,
'map': { 'map': {

View File

@@ -81,7 +81,14 @@ from .plugins import plugin_manager
# endpoints command claim/result long-poll callbacks) on the same per-plugin # endpoints command claim/result long-poll callbacks) on the same per-plugin
# collector key + managed-token check the ingest route uses, without importing # collector key + managed-token check the ingest route uses, without importing
# core collector internals. Additive name on the surface, minor bump. # core collector internals. Additive name on the surface, minor bump.
__contract_version__ = '0.24.0' # 0.25.0: added caller_holds(permission) to shopdb.api so a route can ask
# whether THIS caller holds a permission without refusing the request. It is for
# a surface that serves everyone but should only OFFER what the caller can use:
# the endpoints services panel emitted Start/Stop to every reader because the
# emission never consulted the caller, so a read-only viewer was shown controls
# over a shop-floor PC that 403 on click. Guests are included, since a signed-out
# visitor holds whatever the guest role was granted. Additive name, minor bump.
__contract_version__ = '0.25.0'
# Product release version (see ADR-007). The product version and the # Product release version (see ADR-007). The product version and the
# plugin-contract version above are distinct series with independent # plugin-contract version above are distinct series with independent

View File

@@ -68,7 +68,8 @@ from shopdb.utils.responses import (
from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.pagination import get_pagination_params, paginate_query
# Authorization decorators for gating plugin write routes # Authorization decorators for gating plugin write routes
from shopdb.utils.authz import require_permission, require_any_permission, require_role from shopdb.utils.authz import (require_permission, require_any_permission,
require_role, caller_holds)
# Service-token authorization for unattended plugin endpoints (collector, # Service-token authorization for unattended plugin endpoints (collector,
# GE-Enforce fetch, ...): checks a scoped managed token without exposing token # GE-Enforce fetch, ...): checks a scoped managed token without exposing token
@@ -331,6 +332,7 @@ __all__ = [
# Authorization decorators # Authorization decorators
'require_permission', 'require_permission',
'require_any_permission', 'require_any_permission',
'caller_holds',
'require_role', 'require_role',
'service_token_authorized', 'service_token_authorized',
'authorized_service_token', 'authorized_service_token',

View File

@@ -86,6 +86,32 @@ def guestholds(permission_name: str) -> bool:
return any(p.name == permission_name for p in role.permissions) return any(p.name == permission_name for p in role.permissions)
def caller_holds(permission_name: str) -> bool:
"""Whether THIS caller holds a permission, without refusing the request.
require_permission answers the same question by returning 401/403. This
answers it in the body, for the case where a route serves everyone but
should only OFFER something to the callers who can use it.
The endpoints services panel is the example that prompted it: it emitted
Start/Stop buttons for every reader, because the emission never consulted
the caller. Clicking them 403s, so nothing was exploitable, but showing a
read-only viewer controls over a shop-floor PC is the wrong signal - and it
gets worse the moment a site grants that panel to guests.
Guests are included on purpose: a signed-out visitor holds whatever the
guest role was granted, so the answer must come from the same place
require_permission gets it.
"""
user = currentuser()
if user is None:
return guestholds(permission_name)
try:
return bool(user.haspermission(permission_name))
except Exception:
return False
def require_permission(permission_name: str): def require_permission(permission_name: str):
"""Gate a route behind a single permission. Admin role bypasses. """Gate a route behind a single permission. Admin role bypasses.