Gate operator reports behind their own grant
Some checks failed
CI / backend (push) Has been cancelled
CI / naming (push) Has been cancelled
CI / gates (push) Has been cancelled
CI / frontend (push) Has been cancelled
CI / lean-build (push) Has been cancelled

Operator report screenshots are an uncontrolled capture of whatever was
on the operator's screen when they filed the report, so they can carry
part numbers, customer names, or other shopfloor content. The rest of
endpoints.view is uniform machine-collected telemetry. Reading a PC's
CPU load does not imply a right to read someone's screen, so the reports
and their images move to endpoints.operatorreports.

The image route is the real control point, not the panel: the panel only
decides whether a tab is drawn, while the bytes stay reachable by URL to
anyone who can guess an id. Both GET routes now require the new grant.

Migration 7d47 backfills the grant to everyone holding endpoints.view so
nobody loses a panel they were already using; tightening from there is a
per-account decision in the user editor. The grant is in the scope
denylist, so it is a per-user direct grant and Reporter cannot hold it.

Also adds test_user_editor_checkboxes_match_the_server_action_list. The
SCOPE_ENDPOINT_ACTIONS comment claimed a sync guard kept the Vue
checkbox list aligned with the Python tuple, but no such test existed.
The lists had already drifted: endpoints.operatorreports reached a
running container with no way to grant it through the UI. The guard
catches drift in both directions - a name only in Python ships the
feature dark, and a name only in Vue is worse, since the admin ticks a
box and believes access was granted while the server refuses to store it.
This commit is contained in:
benjamin.baker
2026-09-08 07:36:19 -04:00
parent 2cf46da55e
commit 68ddf87d3b
8 changed files with 280 additions and 10 deletions

View File

@@ -366,15 +366,18 @@ const userForm = reactive({
unlock: false
})
// The four endpoint actions the shop gates per person rather than per policy.
// The endpoint actions the shop gates per person rather than per policy.
// Named here rather than derived from the permission catalog so the user editor
// stays a short, meaningful list instead of the full permission grid, which
// belongs to the role editor.
// belongs to the role editor. Mirrors SCOPE_ENDPOINT_ACTIONS in
// shopdb/core/models/user.py; adding one here without adding it there leaves a
// checkbox the server will refuse to store.
const endpointActionPermissions = [
{ name: 'endpoints.restart', label: 'Remote Restart' },
{ name: 'endpoints.runscript', label: 'Script Push' },
{ name: 'endpoints.updateapp', label: 'Application Update' },
{ name: 'endpoints.corptools', label: 'Corporate Tool Use' }
{ name: 'endpoints.corptools', label: 'Corporate Tool Use' },
{ name: 'endpoints.operatorreports', label: 'Operator Reports' }
]
// What each scope means, in the words shown at the point of use.

View File

@@ -0,0 +1,97 @@
"""Split operator report viewing out of endpoints.view.
Operator reports carry a screenshot of whatever was on the operator's screen
when they filed the report. That is an uncontrolled human capture - it can
include part numbers, customer names, email, or anything else that happened to
be visible - whereas the rest of endpoints.view is uniform machine-collected
telemetry. Reading a PC's CPU load does not imply a right to read someone's
screen, so the reports and their images move to their own grant.
Backfill preserves access rather than revoking it: every user who can see
endpoints today gets the new grant, so nobody loses a panel they were already
using. Tightening from there is an administrative decision made per account in
the user editor, not something this migration should force. The grant is a
per-user direct permission (endpoints.operatorreports is in the scope
denylist), so the backfill writes userpermissions rows, not role rows.
Revision ID: 7d47_endpoints_operatorreports
Revises: 7d46_nest_app_packages
"""
from alembic import op
import sqlalchemy as sa
revision = '7d47_endpoints_operatorreports'
down_revision = '7d46_nest_app_packages'
branch_labels = None
depends_on = None
NEW_PERMISSION = (
'endpoints.operatorreports',
'View operator-submitted reports and their screenshots',
'endpoints',
)
SOURCE_PERMISSION = 'endpoints.view'
def _tables_present(bind):
inspector = sa.inspect(bind)
tables = set(inspector.get_table_names())
return {'users', 'roles', 'permissions', 'rolepermissions',
'userpermissions', 'userroles'} <= tables
def upgrade():
bind = op.get_bind()
if not _tables_present(bind):
return
name, description, category = NEW_PERMISSION
# Guarded in Python so the statement stays portable: MySQL's FROM DUAL
# idiom does not run on SQLite, which the test suite uses.
exists = bind.execute(sa.text(
'SELECT 1 FROM permissions WHERE name = :name'),
{'name': name}).first()
if not exists:
bind.execute(sa.text(
'INSERT INTO permissions (name, description, category) '
'VALUES (:name, :description, :category)'),
{'name': name, 'description': description, 'category': category})
# Preserve access for anyone who reaches operator reports today, which is
# anyone holding endpoints.view through a role. NOT EXISTS keeps the
# migration re-runnable and avoids duplicate rows for users in two
# qualifying roles.
bind.execute(sa.text(
'INSERT INTO userpermissions (userid, permissionid) '
'SELECT DISTINCT ur.userid, new.permissionid '
'FROM userroles ur '
'JOIN rolepermissions rp ON rp.roleid = ur.roleid '
'JOIN permissions old ON old.permissionid = rp.permissionid '
'JOIN permissions new ON new.name = :newname '
'WHERE old.name = :oldname '
'AND NOT EXISTS (SELECT 1 FROM userpermissions existing '
' WHERE existing.userid = ur.userid '
' AND existing.permissionid = new.permissionid)'),
{'newname': name, 'oldname': SOURCE_PERMISSION})
def downgrade():
bind = op.get_bind()
if not _tables_present(bind):
return
name = NEW_PERMISSION[0]
bind.execute(sa.text(
'DELETE FROM userpermissions WHERE permissionid IN '
'(SELECT permissionid FROM permissions WHERE name = :name)'),
{'name': name})
bind.execute(sa.text(
'DELETE FROM rolepermissions WHERE permissionid IN '
'(SELECT permissionid FROM permissions WHERE name = :name)'),
{'name': name})
bind.execute(sa.text(
'DELETE FROM permissions WHERE name = :name'), {'name': name})

View File

@@ -829,6 +829,13 @@ const hasCloudIdentity = computed(() => portalTemplatesLoaded.value && Boolean(
// during setup, which reads canDeploy before this line would otherwise run.
const canDeploy = computed(() => authStore.hasPermission('endpoints.corptools'))
// Operator report screenshots are an uncontrolled capture of whatever was on
// the operator's screen, so they carry their own grant rather than riding
// endpoints.view. Declared alongside canDeploy for the same reason: detailtabs
// reads it during setup.
const canViewOperatorReports = computed(
() => authStore.hasPermission('endpoints.operatorreports'))
const detailtabs = computed(() => {
const filled = assetPanels.value?.populatedGroups ?? new Set()
const has = (key) => filled.has ? filled.has(key) : false
@@ -844,7 +851,12 @@ const detailtabs = computed(() => {
// Operator Reports earns a tab rather than a card in System Monitors: it is
// what a person reported, not what the agent measured, and the thumbnail
// grid is much bulkier than the fact cards around it.
if (has('reports')) tabs.push({ key: 'reports', label: 'Operator Reports' })
// Permission-gated as well as data-gated: the screenshots can carry shopfloor
// content that endpoints.view does not imply a right to see. The panel and
// image routes both re-check server-side; this only hides the tab.
if (has('reports') && canViewOperatorReports.value) {
tabs.push({ key: 'reports', label: 'Operator Reports' })
}
if (has('software') || installedApps.value.length > 0) {
tabs.push({ key: 'software', label: 'Software', count: installedApps.value.length || null })
}

View File

@@ -3036,7 +3036,7 @@ def upload_operator_report():
@endpoints_bp.route('/panel/<int:asset_id>/operatorreports', methods=['GET'])
@require_permission('endpoints.view')
@require_permission('endpoints.operatorreports')
def panel_operator_reports(asset_id: int):
"""Operator Reports panel: this PC's recent operator issue reports.
@@ -3058,9 +3058,16 @@ def panel_operator_reports(asset_id: int):
@endpoints_bp.route('/operatorreports/<int:operatorreportid>/image',
methods=['GET'])
@require_permission('endpoints.view')
@require_permission('endpoints.operatorreports')
def serve_operator_report_image(operatorreportid: int):
"""Serve one operator report's screenshot (user JWT + endpoints.view)."""
"""Serve one operator report's screenshot.
Requires a user JWT plus endpoints.operatorreports.
This route, not the panel, is what actually protects the image: the panel
only decides whether a tab is drawn, while the bytes are reachable by URL to
anyone who can guess an id. Screenshots are an uncontrolled capture of the
operator's screen, so this must never widen back to endpoints.view.
"""
from shopdb.api import send_upload
report = db.session.get(OperatorReport, operatorreportid)

View File

@@ -177,6 +177,16 @@ class EndpointsPlugin(BasePlugin):
# shop-floor PC is a heavier grant than reading its telemetry.
('endpoints.command',
'Send allowlisted commands to endpoint agents', 'endpoints'),
# Operator-submitted issue reports and their screenshots. Separate
# from endpoints.view because the screenshot is whatever was on the
# operator's screen when they hit report: it can carry part numbers,
# customer names or other shopfloor content that reading telemetry
# does not imply a right to see. Telemetry is machine-collected and
# uniform; this is an uncontrolled human capture, so it is its own
# grant rather than a facet of endpoints.view.
('endpoints.operatorreports',
'View operator-submitted reports and their screenshots',
'endpoints'),
# Elevated grant for destructive endpoint actions (system_restart).
# Required IN ADDITION to endpoints.command by the enqueue route, so a
# remote reboot is a strictly heavier grant than a routine toggle.
@@ -702,6 +712,11 @@ class EndpointsPlugin(BasePlugin):
'empty': 'No operator reports for this endpoint.',
'position': 18,
'group': 'reports',
# Not endpoints.view: the screenshots can carry shopfloor
# content that reading telemetry does not imply a right to see.
# The panel route and the image route re-check this server-side;
# withholding the declaration only keeps the tab from appearing.
'permission': 'endpoints.operatorreports',
},
]

View File

@@ -263,6 +263,10 @@ SCOPE_NEVER_GRANT = {
'endpoints.runscript',
'endpoints.updateapp',
'endpoints.corptools',
# Operator report screenshots are an uncontrolled capture of the operator's
# screen and may carry shopfloor content, so who sees them is a per-account
# decision rather than something a scope confers.
'endpoints.operatorreports',
# Fleet-wide and agent-distribution capabilities, admin-only by nature.
'endpoints.fleet',
'endpoints.agentpackage',
@@ -278,6 +282,7 @@ SCOPE_ENDPOINT_ACTIONS = (
'endpoints.runscript',
'endpoints.updateapp',
'endpoints.corptools',
'endpoints.operatorreports',
)

View File

@@ -18,6 +18,37 @@ def _core_catalog():
return list(Permission.CORE_PERMISSIONS)
def test_user_editor_checkboxes_match_the_server_action_list():
"""The Vue checkbox list must match SCOPE_ENDPOINT_ACTIONS exactly.
These two lists are maintained by hand in different languages, so they drift
silently and in both directions. A name in Python but not in Vue is a grant
no admin can actually give - the feature ships dark. A name in Vue but not in
Python is worse: the checkbox appears, the admin ticks it believing access
was granted, and the server refuses to store it.
This is the guard the SCOPE_ENDPOINT_ACTIONS comment claims exists. It did
not, which is exactly how endpoints.operatorreports reached a running
container with no way to grant it.
"""
import re
from pathlib import Path
source = Path(__file__).resolve().parents[2].joinpath(
'frontend', 'src', 'views', 'settings', 'UsersList.vue').read_text(
encoding='utf-8')
block = re.search(
r'const endpointActionPermissions = \[(.*?)\]', source, re.S)
assert block, 'endpointActionPermissions not found in UsersList.vue'
infrontend = set(re.findall(r"name:\s*'([^']+)'", block.group(1)))
assert infrontend == set(SCOPE_ENDPOINT_ACTIONS), (
'User editor checkboxes and SCOPE_ENDPOINT_ACTIONS disagree. '
f'Only in Vue: {sorted(infrontend - set(SCOPE_ENDPOINT_ACTIONS))}. '
f'Only in Python: {sorted(set(SCOPE_ENDPOINT_ACTIONS) - infrontend)}.')
# ---------------------------------------------------------------------------
# Derived grant lists
# ---------------------------------------------------------------------------

View File

@@ -3,9 +3,11 @@
The tray captures a screenshot and a free-text note; the agent posts them to
POST /api/endpoints/operatorreports on the collector key. The bytes land on
disk under the operator-reports uploads dir and the row holds metadata. The
human-facing panel and image-serve routes are JWT + endpoints.view gated, and
the serve route carries the inert-upload headers. A retention sweep purges rows
and files older than the configured window.
human-facing panel and image-serve routes are JWT + endpoints.operatorreports
gated - deliberately NOT endpoints.view, because the screenshot is an
uncontrolled capture of the operator's screen and reading telemetry does not
imply a right to see it - and the serve route carries the inert-upload headers.
A retention sweep purges rows and files older than the configured window.
"""
import io
@@ -283,3 +285,101 @@ def test_upload_refuses_an_image_over_the_cap(client, db, app):
assert 'limit' in response.get_json()['data']['error']['message']
assert OperatorReport.query.filter_by(
reportedhostname='OPREPORT-HUGE').first() is None
def _viewer_headers(client, db, permissions, username):
"""Log in a user holding exactly `permissions` as direct grants.
Deliberately direct grants rather than a role: endpoints.operatorreports is
in the scope denylist, so a role can never confer it and a role-based
fixture could not express the case these tests need.
"""
from shopdb.core.models import Permission, User
from werkzeug.security import generate_password_hash
user = User(username=username, email=f'{username}@test.local',
passwordhash=generate_password_hash('testpass'))
for name in permissions:
perm = Permission.query.filter_by(name=name).first()
if perm is None:
perm = Permission(name=name, description=name,
category='endpoints')
db.session.add(perm)
db.session.flush()
user.directpermissions.append(perm)
db.session.add(user)
db.session.commit()
response = client.post('/api/auth/login',
json={'username': username, 'password': 'testpass'})
assert response.status_code == 200, response.get_json()
token = response.get_json()['data']['access_token']
return {'Authorization': f'Bearer {token}'}
def _report_for(client, db, app, hostname):
"""Upload one report on the collector key and return its row."""
from plugins.endpoints.models import OperatorReport
asset = _computer(db, hostname)
client.post(UPLOAD_URL,
data={'hostname': hostname, 'details': 'x',
'screenshot': (io.BytesIO(_png_bytes()), 'shot.png')},
content_type='multipart/form-data',
headers=_collector_headers(app))
return OperatorReport.query.filter_by(assetid=asset.assetid).first()
def test_endpoints_view_alone_cannot_fetch_a_report_screenshot(
client, db, app):
"""The whole point of the split: telemetry access is not screenshot access.
The screenshot is an uncontrolled capture of the operator's screen, so a
user who can read a PC's telemetry must NOT get the image for free. If this
regresses, sensitive shopfloor content is exposed to every endpoint viewer.
"""
report = _report_for(client, db, app, 'OPREPORT-VIEWONLY')
headers = _viewer_headers(client, db, ['endpoints.view'], 'telemetryonly')
response = client.get(IMAGE_URL.format(report.operatorreportid),
headers=headers)
assert response.status_code == 403
def test_endpoints_view_alone_cannot_list_operator_reports(client, db, app):
"""The panel route leaks the report text and filenames, so it is gated too.
Hiding the tab in the UI is not a control; the route must refuse directly.
"""
report = _report_for(client, db, app, 'OPREPORT-PANELDENY')
headers = _viewer_headers(client, db, ['endpoints.view'], 'panelviewonly')
response = client.get(PANEL_URL.format(report.assetid), headers=headers)
assert response.status_code == 403
def test_the_operatorreports_grant_opens_both_routes(client, db, app):
"""The positive case: the dedicated grant is sufficient on its own."""
report = _report_for(client, db, app, 'OPREPORT-GRANTED')
headers = _viewer_headers(
client, db, ['endpoints.view', 'endpoints.operatorreports'],
'reportviewer')
image = client.get(IMAGE_URL.format(report.operatorreportid),
headers=headers)
assert image.status_code == 200
panel = client.get(PANEL_URL.format(report.assetid), headers=headers)
assert panel.status_code == 200
reports = panel.get_json()['data']['reports']
assert len(reports) == 1
def test_operator_reports_panel_declares_its_permission():
"""The panel carries the grant so the tab is not advertised without it."""
from plugins.endpoints.plugin import EndpointsPlugin
panels = EndpointsPlugin().get_asset_panels()
panel = next(p for p in panels
if p.get('render') == 'operatorreports')
assert panel['permission'] == 'endpoints.operatorreports'