From 54f0857e13b13d70fdba29d04f13a429e4337435 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Fri, 28 Aug 2026 21:14:27 -0400 Subject: [PATCH] Split the fleet roster from per-endpoint reads, and stop offering controls nobody can use 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. --- docs/CONTRACT-STABILITY.md | 1 + docs/PROJECT-MAP.md | 4 ++-- plugins/endpoints/api/routes.py | 19 +++++++++++++------ plugins/endpoints/plugin.py | 22 +++++++++++++++++++--- shopdb/__init__.py | 9 ++++++++- shopdb/api/__init__.py | 4 +++- shopdb/utils/authz.py | 26 ++++++++++++++++++++++++++ 7 files changed, 72 insertions(+), 13 deletions(-) diff --git a/docs/CONTRACT-STABILITY.md b/docs/CONTRACT-STABILITY.md index 64d8e560..899ee32d 100644 --- a/docs/CONTRACT-STABILITY.md +++ b/docs/CONTRACT-STABILITY.md @@ -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.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.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, 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 diff --git a/docs/PROJECT-MAP.md b/docs/PROJECT-MAP.md index ee167dc1..17687f93 100644 --- a/docs/PROJECT-MAP.md +++ b/docs/PROJECT-MAP.md @@ -12,7 +12,7 @@ never by editing this file. | series | value | governed by | |---|---|---| | 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. @@ -90,6 +90,6 @@ Manifest-less directories under `plugins/` are core frontend surface and always ## 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`) diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index bb05248d..68c8b69a 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -16,7 +16,8 @@ from flask_jwt_extended import current_user from shopdb.api import ( db, Asset, Setting, success_response, error_response, paginated_response, 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, ) @@ -72,7 +73,7 @@ def _format_uptime(seconds): # ============================================================================= @endpoints_bp.route('', methods=['GET']) -@require_permission('endpoints.view') +@require_permission('endpoints.fleet') def list_endpoints(): """List reported endpoint states. @@ -408,6 +409,8 @@ def panel_services(asset_id: int): if not state or not state.servicesjson: return success_response([]) + maycommand = caller_holds('endpoints.command') + try: services = json.loads(state.servicesjson) except (TypeError, ValueError): @@ -429,7 +432,11 @@ def panel_services(asset_id: int): 'statelabel': 'Running' if running else (raw.title() or 'Unknown'), '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: row['action'] = { 'running': running, @@ -512,7 +519,7 @@ def panel_interfaces(asset_id: int): # ============================================================================= @endpoints_bp.route('/dashboard/summary', methods=['GET']) -@require_permission('endpoints.view') +@require_permission('endpoints.fleet') def dashboard_summary(): """High-level fleet counts for the endpoints dashboard.""" total = db.session.query(EndpointState).join( @@ -551,7 +558,7 @@ def dashboard_summary(): @endpoints_bp.route('/dashboard/quiet', methods=['GET']) -@require_permission('endpoints.view') +@require_permission('endpoints.fleet') def dashboard_quiet(): """Endpoints that reported before, then went quiet past the window. @@ -586,7 +593,7 @@ def dashboard_quiet(): @endpoints_bp.route('/dashboard/stability', methods=['GET']) -@require_permission('endpoints.view') +@require_permission('endpoints.fleet') def dashboard_stability(): """Endpoints with recent crash/shutdown/corruption events. diff --git a/plugins/endpoints/plugin.py b/plugins/endpoints/plugin.py index ed012b37..348c157b 100644 --- a/plugins/endpoints/plugin.py +++ b/plugins/endpoints/plugin.py @@ -103,6 +103,22 @@ class EndpointsPlugin(BasePlugin): """ return [ ('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 # MTConnect toggle). Separate from endpoints.view because acting on a # shop-floor PC is a heavier grant than reading its telemetry. @@ -189,7 +205,7 @@ class EndpointsPlugin(BasePlugin): 'icon': 'activity', 'route': '/endpoints', 'position': 16, - 'permission': 'endpoints.view', + 'permission': 'endpoints.fleet', 'section': 'assets', }, ] @@ -340,7 +356,7 @@ class EndpointsPlugin(BasePlugin): 'endpoint': '/api/endpoints/dashboard/quiet', 'render': 'exceptions', 'severity': 'warning', - 'permission': 'endpoints.view', + 'permission': 'endpoints.fleet', 'empty': 'hide', 'position': 22, 'map': { @@ -358,7 +374,7 @@ class EndpointsPlugin(BasePlugin): 'endpoint': '/api/endpoints/dashboard/stability', 'render': 'exceptions', 'severity': 'critical', - 'permission': 'endpoints.view', + 'permission': 'endpoints.fleet', 'empty': 'hide', 'position': 16, 'map': { diff --git a/shopdb/__init__.py b/shopdb/__init__.py index 7226817a..b61894e6 100644 --- a/shopdb/__init__.py +++ b/shopdb/__init__.py @@ -81,7 +81,14 @@ from .plugins import plugin_manager # endpoints command claim/result long-poll callbacks) on the same per-plugin # collector key + managed-token check the ingest route uses, without importing # 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 # plugin-contract version above are distinct series with independent diff --git a/shopdb/api/__init__.py b/shopdb/api/__init__.py index c7c2baf9..a2b61390 100644 --- a/shopdb/api/__init__.py +++ b/shopdb/api/__init__.py @@ -68,7 +68,8 @@ from shopdb.utils.responses import ( from shopdb.utils.pagination import get_pagination_params, paginate_query # 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, # GE-Enforce fetch, ...): checks a scoped managed token without exposing token @@ -331,6 +332,7 @@ __all__ = [ # Authorization decorators 'require_permission', 'require_any_permission', + 'caller_holds', 'require_role', 'service_token_authorized', 'authorized_service_token', diff --git a/shopdb/utils/authz.py b/shopdb/utils/authz.py index 2a267643..e9bb4284 100644 --- a/shopdb/utils/authz.py +++ b/shopdb/utils/authz.py @@ -86,6 +86,32 @@ def guestholds(permission_name: str) -> bool: 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): """Gate a route behind a single permission. Admin role bypasses.