diff --git a/migrations/versions/7d42_split_endpoints_deploy.py b/migrations/versions/7d42_split_endpoints_deploy.py new file mode 100644 index 00000000..166fd605 --- /dev/null +++ b/migrations/versions/7d42_split_endpoints_deploy.py @@ -0,0 +1,151 @@ +"""Split endpoints.deploy into three independent scope grants. + +endpoints.deploy bundled three actions of very different risk behind one name: + + scheduledtask_run -> starts a task already registered and allowlisted agent-side + update_app -> runs a pinned installer + run_script -> ARBITRARY code as SYSTEM unless the descriptor pins a user + +Because they shared a grant, there was no way to let an operator push an +application update without also handing them arbitrary SYSTEM code execution on +every endpoint they could reach. The permission name described the lightest of +the three and silently carried the heaviest. + +commands.ELEVATED_ACTION_PERMISSIONS now maps each action to its own permission +(endpoints.corptools, endpoints.updateapp, endpoints.runscript). The enqueue +route reads that table, so no route logic changed. + +This migration preserves access EXACTLY. Anyone holding endpoints.deploy today +can already do all three, so every role holding it receives all three new +grants. Nothing widens: no role that lacked endpoints.deploy gains anything. The +security win is not in this migration - it is that from here the three can be +granted separately. + +endpoints.deploy itself is then removed. Leaving it behind would be worse than +useless: nothing reads it any more, but the role grid would keep offering it and +operators would keep ticking it expecting an effect. + +Revision ID: 7d42_split_endpoints_deploy +Revises: 7d41_drop_guest_tools_secrets +""" +from alembic import op + +import sqlalchemy as sa + + +revision = '7d42_split_endpoints_deploy' +down_revision = '7d41_drop_guest_tools_secrets' +branch_labels = None +depends_on = None + + +OLD_PERMISSION = 'endpoints.deploy' + +NEW_PERMISSIONS = [ + ('endpoints.corptools', + 'Start approved scheduled tasks on endpoints (limited to tasks already ' + 'registered and allowlisted on the agent)', + 'endpoints'), + ('endpoints.updateapp', + 'Push application installers to endpoints (runs a pinned installer as ' + 'SYSTEM)', + 'endpoints'), + ('endpoints.runscript', + 'Run staged scripts on endpoints (ARBITRARY CODE, executes as SYSTEM ' + 'unless the script pins a user context; grant only to fully trusted ' + 'operators)', + 'endpoints'), +] + + +def _tables_present(bind): + inspector = sa.inspect(bind) + tables = set(inspector.get_table_names()) + return {'roles', 'permissions', 'rolepermissions'} <= tables + + +def _ensure_permission(bind, name, description, category): + """Insert a permission if absent. 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 exists: + return + bind.execute(sa.text( + 'INSERT INTO permissions (name, description, category) ' + 'VALUES (:name, :description, :category)'), + {'name': name, 'description': description, 'category': category}) + + +def upgrade(): + bind = op.get_bind() + if not _tables_present(bind): + return + + for name, description, category in NEW_PERMISSIONS: + _ensure_permission(bind, name, description, category) + + # Preserve access: every role holding the old bundle gets all three halves. + for name, _description, _category in NEW_PERMISSIONS: + bind.execute(sa.text( + 'INSERT INTO rolepermissions (roleid, permissionid) ' + 'SELECT rp.roleid, new.permissionid ' + 'FROM rolepermissions rp ' + '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 rolepermissions existing ' + ' WHERE existing.roleid = rp.roleid ' + ' AND existing.permissionid = new.permissionid)'), + {'newname': name, 'oldname': OLD_PERMISSION}) + + bind.execute(sa.text( + 'DELETE FROM rolepermissions WHERE permissionid IN ' + '(SELECT permissionid FROM permissions WHERE name = :oldname)'), + {'oldname': OLD_PERMISSION}) + bind.execute(sa.text( + 'DELETE FROM permissions WHERE name = :oldname'), + {'oldname': OLD_PERMISSION}) + + +def downgrade(): + """Rebuild the bundle and grant it to anyone holding any of the three. + + Deliberately generous in this direction: on the way back the only safe + reading of "held one of the three" is "held the bundle", because the bundle + is all-or-nothing. Downgrading therefore restores capability rather than + quietly removing it from an operator mid-shift. + """ + bind = op.get_bind() + if not _tables_present(bind): + return + + _ensure_permission( + bind, OLD_PERMISSION, + 'Run staged scripts, start approved scheduled tasks, and push ' + 'application installers to endpoints (scripts run as SYSTEM; grant ' + 'only to fully trusted operators)', + 'endpoints') + + newnames = [name for name, _d, _c in NEW_PERMISSIONS] + bind.execute(sa.text( + 'INSERT INTO rolepermissions (roleid, permissionid) ' + 'SELECT DISTINCT rp.roleid, old.permissionid ' + 'FROM rolepermissions rp ' + 'JOIN permissions new ON new.permissionid = rp.permissionid ' + 'JOIN permissions old ON old.name = :oldname ' + 'WHERE new.name IN :newnames ' + 'AND NOT EXISTS (SELECT 1 FROM rolepermissions existing ' + ' WHERE existing.roleid = rp.roleid ' + ' AND existing.permissionid = old.permissionid)' + ).bindparams(sa.bindparam('newnames', expanding=True)), + {'oldname': OLD_PERMISSION, 'newnames': newnames}) + + for name in newnames: + 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}) diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index 44355086..a2491484 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -19,9 +19,10 @@ 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, caller_holds, mint_service_token, list_service_tokens, + require_role, caller_holds, caller_is_role, mint_service_token, + list_service_tokens, revoke_service_tokens, - AuditLog, collector_authorized, User, + AuditLog, audit_log, collector_authorized, User, Application, AppVersion, Script, ) @@ -29,7 +30,7 @@ from ..models import ( EndpointState, EndpointEvent, EndpointCommand, EndpointApplication, EndpointHealthState, OperatorReport, ) -from ..commands import COMMAND_ALLOWLIST, is_allowed_action, ALLOWED_ACTIONS_FOR, row_actions_for, requires_target, action_targetkind, elevated_permission_for +from ..commands import COMMAND_ALLOWLIST, is_allowed_action, ALLOWED_ACTIONS_FOR, row_actions_for, requires_target, action_targetkind, elevated_permission_for, PLACE_POLICY_HOLD, ACTIVE_POLICY_HOLDS, HOLDABLE_ACTIONS_FOR, RELEASE_POLICY_HOLD, ACTIVE_LOCK_FOR, PLACE_ENDPOINT_LOCK, RELEASE_ENDPOINT_LOCK from ..eventcatalog import CRITICAL_EVENT_IDS, event_severity, event_title endpoints_bp = Blueprint('endpoints', __name__) @@ -2108,14 +2109,286 @@ def list_endpoint_commands(asset_id: int): return success_response([row.to_dict() for row in rows]) +@endpoints_bp.route('//policyholds', methods=['GET']) +@require_permission('endpoints.view') +def list_endpoint_policy_holds(asset_id: int): + """Active break-glass holds for one endpoint. + + Gated by endpoints.view, not by the admin role that places a hold. A hold + suppresses reconciliation, so an operator watching a machine drift from its + policy needs to see that the drift is deliberate, who authorized it and when + it lapses. Hiding that from non-admins turns an audited override back into + unexplained behavior. + """ + return success_response(ACTIVE_POLICY_HOLDS(asset_id)) + + +@endpoints_bp.route('//holdableactions', methods=['GET']) +@require_permission('endpoints.view') +def list_endpoint_holdable_actions(asset_id: int): + """Desired-mode actions on this endpoint, with current hold state. + + Feeds the admin hold picker. Read-gated rather than admin-gated to match the + holds list: knowing WHICH actions this endpoint reconciles is the same class + of provenance as knowing which are currently held. + """ + return success_response(HOLDABLE_ACTIONS_FOR(asset_id)) + + +@endpoints_bp.route('//policyholds', methods=['POST']) +@require_permission('endpoints.command') +def place_endpoint_policy_hold(asset_id: int): + """Deliberately suspend desired-state reconciliation for one or more actions. + + The same authority as break-glass, reached directly instead of as a side + effect of a denied command: an admin who knows they are about to work on a + machine should not have to trip a policy denial first to stop the reconciler + fighting them. + + Accepts `actions` (a list) or `action` (a single name, kept so existing + callers keep working). Every named action is validated BEFORE anything is + written, and one bad name rejects the whole request. Partial success on a + break-glass control is the wrong failure mode - "three of your five holds + landed" is worthless to someone about to take a machine apart, and the + missing two are discovered only when the reconciler undoes their work. + + One shared window covers every action in the request: holding five actions + means doing one job on one machine, and that job has one duration. + + Admin-only via caller_is_role for the same reason as the override branch - a + scoped personal token is a narrower caller than its owner, so a leaked agent + token must not be able to switch off policy enforcement. + """ + if not caller_is_role('admin'): + return error_response(ErrorCodes.FORBIDDEN, + 'Placing a policy hold requires the admin role', + http_code=403) + + payload = request.get_json(silent=True) or {} + reason = (payload.get('reason') or '').strip() + + raw = payload.get('actions') + if raw is None: + raw = [payload.get('action')] if payload.get('action') else [] + if not isinstance(raw, list): + return error_response(ErrorCodes.VALIDATION_ERROR, + 'actions must be a list of action names') + + actions = [] + for entry in raw: + name = (entry or '').strip() if isinstance(entry, str) else '' + if name and name not in actions: + actions.append(name) + + if not actions: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'Select at least one action to hold') + if not reason: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'A hold requires a reason explaining why') + + for name in actions: + if not is_allowed_action(name): + return error_response(ErrorCodes.VALIDATION_ERROR, + f'Unknown or disallowed action: {name!r}') + + # Only desired-mode actions are holdable. Accepting anything else would + # write a row that suppresses nothing, so the UI would report a hold that + # has no effect - worse than refusing. Checked for every action up front so + # the request is rejected whole rather than half-applied. + holdable = {entry['action'] for entry in HOLDABLE_ACTIONS_FOR(asset_id)} + unholdable = [name for name in actions if name not in holdable] + if unholdable: + return error_response( + ErrorCodes.VALIDATION_ERROR, + f'Not under desired-state enforcement on this endpoint, so there ' + f'is nothing to hold: {", ".join(sorted(unholdable))}') + + minutes = payload.get('minutes') + try: + minutes = int(minutes) if minutes is not None else None + except (TypeError, ValueError): + return error_response(ErrorCodes.VALIDATION_ERROR, + 'minutes must be a whole number') + # Zero is legal and means indefinite: the hold stands until released by + # hand. Negatives are not - they would place an already-expired hold, which + # reads as success in the UI while suppressing nothing. + if minutes is not None and minutes < 0: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'minutes cannot be negative (use 0 for an ' + 'indefinite hold)') + + indefinite = minutes == 0 + window = 'indefinite' if indefinite else minutes + + audit_log( + action='policyholdplaced', + entitytype='Asset', + entityid=asset_id, + changes={ + 'actions': actions, + 'reason': reason, + 'minutes': window, + 'indefinite': indefinite, + 'placedby': getattr(current_user, 'username', None), + }, + details=(f'Policy hold placed on {", ".join(actions)} ' + f'({window}): {reason}'), + ) + + for name in actions: + placed = PLACE_POLICY_HOLD( + assetid=asset_id, + action=name, + reason=reason, + userid=getattr(current_user, 'userid', None), + username=getattr(current_user, 'username', None), + minutes=minutes, + ) + if not placed: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'Policy holds are unavailable on this site') + return success_response(ACTIVE_POLICY_HOLDS(asset_id)) + + +@endpoints_bp.route('//policyholds/', methods=['DELETE']) +@require_permission('endpoints.command') +def release_endpoint_policy_hold(asset_id: int, action: str): + """End a hold early, resuming reconciliation of one action. + + Admin-only on the same reasoning as placing one: releasing a hold turns + policy enforcement back on for a machine someone may still be working on, + which is the same authority in the opposite direction. + """ + if not caller_is_role('admin'): + return error_response(ErrorCodes.FORBIDDEN, + 'Releasing a policy hold requires the admin role', + http_code=403) + + released = RELEASE_POLICY_HOLD(asset_id, action) + if not released: + return error_response(ErrorCodes.NOT_FOUND, + f'No active hold on {action!r} for this endpoint', + http_code=404) + + audit_log( + action='policyholdreleased', + entitytype='Asset', + entityid=asset_id, + changes={ + 'action': action, + 'releasedby': getattr(current_user, 'username', None), + }, + details=f'Policy hold released early on {action}', + ) + return success_response(ACTIVE_POLICY_HOLDS(asset_id)) + + +@endpoints_bp.route('//lock', methods=['GET']) +@require_permission('endpoints.view') +def get_endpoint_lock(asset_id: int): + """The endpoint lock standing on this endpoint, or null. + + Read-gated rather than admin-gated, and that is the point of the feature: an + operator whose command was refused needs to see "locked by : mid + repair" and self-serve. Hiding the lock from non-admins would leave them + with a bare 403 and a ticket to file about a machine that is working fine. + """ + lock = ACTIVE_LOCK_FOR(asset_id) + return success_response(lock.to_dict() if lock else None) + + +@endpoints_bp.route('//lock', methods=['POST']) +@require_permission('endpoints.command') +def place_endpoint_lock(asset_id: int): + """Quarantine this endpoint against non-admin commands. + + Admin-only via caller_is_role rather than current_user.hasrole, matching the + override and hold routes: a scoped personal token is a narrower caller than + its owner, so a leaked agent token must not be able to quarantine machines. + + A reason is required. The refusal shown to a blocked operator quotes it + verbatim, so an empty one produces a dead end - the operator learns only + that they may not act, not who to ask or when it lifts. + """ + if not caller_is_role('admin'): + return error_response(ErrorCodes.FORBIDDEN, + 'Locking an endpoint requires the admin role', + http_code=403) + + payload = request.get_json(silent=True) or {} + reason = (payload.get('reason') or '').strip() + if not reason: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'A lock requires a reason explaining why') + + lock = PLACE_ENDPOINT_LOCK( + assetid=asset_id, + reason=reason, + userid=getattr(current_user, 'userid', None), + username=getattr(current_user, 'username', None), + ) + + audit_log( + action='endpointlocked', + entitytype='Asset', + entityid=asset_id, + changes={ + 'reason': reason, + 'lockedby': getattr(current_user, 'username', None), + }, + details=f'Endpoint locked: {reason}', + ) + return success_response(lock.to_dict()) + + +@endpoints_bp.route('//lock', methods=['DELETE']) +@require_permission('endpoints.command') +def release_endpoint_lock(asset_id: int): + """Lift the quarantine on this endpoint. + + Admin-only on the same reasoning as setting one: unlocking returns a machine + someone may still be physically working inside to the whole operator pool, + which is the same authority in the opposite direction. + """ + if not caller_is_role('admin'): + return error_response(ErrorCodes.FORBIDDEN, + 'Unlocking an endpoint requires the admin role', + http_code=403) + + lock = RELEASE_ENDPOINT_LOCK( + asset_id, + userid=getattr(current_user, 'userid', None), + username=getattr(current_user, 'username', None), + ) + if lock is None: + return error_response(ErrorCodes.NOT_FOUND, + 'No active lock on this endpoint', + http_code=404) + + audit_log( + action='endpointunlocked', + entitytype='Asset', + entityid=asset_id, + changes={ + 'lockedby': lock.lockedbyusername, + 'reason': lock.reason, + 'releasedby': getattr(current_user, 'username', None), + }, + details=f'Endpoint unlocked (was locked: {lock.reason})', + ) + return success_response(None) + + @endpoints_bp.route('//commands', methods=['POST']) @require_permission('endpoints.command') def enqueue_endpoint_command(asset_id: int): """Queue an allowlisted command for one endpoint. - Rejects unknown actions and refuses to stack a second pending/running - command on the same endpoint, so the queue stays at-most-one-in-flight and - the WebUI toggle cannot race itself. + Rejects unknown actions, refuses non-admin callers while an endpoint lock + stands, and refuses to stack a second pending/running command on the same + endpoint, so the queue stays at-most-one-in-flight and the WebUI toggle + cannot race itself. """ payload = request.get_json(silent=True) or {} action = (payload.get('action') or '').strip() @@ -2123,6 +2396,24 @@ def enqueue_endpoint_command(asset_id: int): return error_response(ErrorCodes.VALIDATION_ERROR, f'Unknown or disallowed action: {action!r}') + # Endpoint lock: a quarantine on the CALLER axis, checked before the + # permission and policy gates below so the refusal can name the lock, its + # owner and its reason. Answering with a generic policy denial instead would + # send an operator hunting through Configuration Policy for a cause that is + # not there - the machine is fine, someone has simply reserved it. + # + # Admins pass straight through with no override argument and no reason. The + # lock gates WHO may act, not WHICH actions exist, so there is no policy + # decision here to break past; requiring break-glass would mean an admin had + # to override their own team's quarantine to do the repair it was set for. + lock = ACTIVE_LOCK_FOR(asset_id) + if lock is not None and not caller_is_role('admin'): + return error_response( + ErrorCodes.FORBIDDEN, + f'Endpoint locked by {lock.lockedbyusername or "a deleted user"}: ' + f'{lock.reason}', + http_code=403) + # Destructive actions (for example system_restart) demand a stricter grant # ON TOP OF endpoints.command, so a remote reboot cannot ride the routine # toggle permission. @@ -2138,10 +2429,59 @@ def enqueue_endpoint_command(asset_id: int): f'Action {action!r} requires the {elevated} permission', http_code=403) + # Break-glass: an admin may push past a Configuration Policy denial, but + # only by asking for it explicitly and saying why. The override is scoped to + # POLICY gating alone - the elevated permission check above still stands, so + # this cannot conjure a script run for someone who lacks endpoints.script. + # It widens WHERE an action may run, never WHICH actions the caller may use. + override = bool(payload.get('policyoverride')) + overridereason = (payload.get('overridereason') or '').strip() + if action not in ALLOWED_ACTIONS_FOR(assetid=asset_id): - return error_response( - ErrorCodes.VALIDATION_ERROR, - f'Action not permitted by policy for this endpoint: {action!r}') + if not override: + return error_response( + ErrorCodes.VALIDATION_ERROR, + f'Action not permitted by policy for this endpoint: {action!r}') + # caller_is_role, not current_user.hasrole: a scoped personal token is a + # narrower caller than its owner, so a leaked agent token must not + # inherit break-glass from the admin who minted it. + if not caller_is_role('admin'): + return error_response( + ErrorCodes.FORBIDDEN, + 'Overriding a policy denial requires the admin role', + http_code=403) + if not overridereason: + return error_response( + ErrorCodes.VALIDATION_ERROR, + 'An override requires overridereason explaining why') + + # Audited before the command is enqueued: if the enqueue fails after + # this point, the record of the attempt still survives. A break-glass + # that is only logged on success hides exactly the cases worth reviewing. + # Structured context goes in changes (JSON-serialized by the model); + # details is a plain Text column, so it carries the readable summary. + audit_log( + action='policyoverridden', + entitytype='Asset', + entityid=asset_id, + changes={ + 'action': action, + 'reason': overridereason, + 'overriddenby': getattr(current_user, 'username', None), + }, + details=f'Policy denial overridden for {action}: {overridereason}', + ) + + # A gate-mode override is a one-shot; a desired-mode override is a fight + # with the reconciler, which would re-enqueue the opposite command on the + # next check-in and make the override look broken. Hold it open. + PLACE_POLICY_HOLD( + assetid=asset_id, + action=action, + reason=overridereason, + userid=getattr(current_user, 'userid', None), + username=getattr(current_user, 'username', None), + ) # Generic actions (service_* / process_*) must carry the subject to act on; # fixed-target actions (mtconnect_*) must not. Validate both directions so a diff --git a/plugins/endpoints/commands.py b/plugins/endpoints/commands.py index c1cfa78f..d0119498 100644 --- a/plugins/endpoints/commands.py +++ b/plugins/endpoints/commands.py @@ -103,7 +103,8 @@ COMMAND_ALLOWLIST = { 'script sent this way runs as SYSTEM unless its ' 'descriptor pins the user context, so this is arbitrary ' 'code execution on the endpoint by design. Grant ' - 'endpoints.deploy only to the most trusted operators, ' + 'endpoints.runscript only to the most trusted ' + 'operators, ' 'and gate the UI behind an explicit confirm the way ' 'system_restart is gated (see confirmRestart in ' 'PCDetail.vue).', @@ -173,25 +174,29 @@ COMMAND_ALLOWLIST = { # Actions that demand a permission stricter than the base endpoints.command. # The enqueue route requires the caller to ALSO hold the mapped permission, so a # destructive action (a remote reboot) is a heavier grant than a routine toggle. -# NOTE FOR THE ROLES/PERMISSIONS WORK: scheduledtask_run is deliberately mapped -# onto the EXISTING endpoints.deploy grant rather than introducing a new one, so -# the wire is pre-built and needs no migration. Be aware this WIDENS what -# endpoints.deploy means: it previously covered run_script + update_app, and now -# also covers starting approved scheduled tasks. If that bundle is too coarse, -# the clean split is a separate endpoints.corptools permission mapped here - no -# other code needs to change, because the enqueue route reads this table. # -# Suggested operator-facing wording for the combined grant: -# "Caution: Operator can run scheduled tasks, update applications, and affect -# system-level changes to the endpoint." -# That wording is accurate for scheduledtask_run and update_app. It UNDERSTATES -# run_script, which is arbitrary SYSTEM code execution - if run_script stays in -# this bundle, say so plainly in the role description. +# These four are gated by SCOPE, never by Configuration Policy. Policy shapes +# what a PC TYPE exposes; it is not the authority on whether a given person may +# reboot a machine or run arbitrary code. The check runs in the enqueue route +# BEFORE the policy gate, so it is also not break-glass-able: an admin may +# override what policy says about a machine, never what permissions say about +# the caller. +# +# Previously scheduledtask_run, run_script and update_app all mapped to a single +# endpoints.deploy grant. That bundle made app updates impossible to grant +# without also granting arbitrary SYSTEM code execution via run_script, so they +# are now four independent permissions. They are NOT equal risk, and the +# descriptions in plugin.py get_permissions() say so plainly: +# scheduledtask_run - starts something already registered and allowlisted +# agent-side +# update_app - runs a pinned installer +# run_script - arbitrary code as SYSTEM unless the descriptor pins a +# user context ELEVATED_ACTION_PERMISSIONS = { 'system_restart': 'endpoints.restart', - 'scheduledtask_run': 'endpoints.deploy', - 'run_script': 'endpoints.deploy', - 'update_app': 'endpoints.deploy', + 'scheduledtask_run': 'endpoints.corptools', + 'run_script': 'endpoints.runscript', + 'update_app': 'endpoints.updateapp', } @@ -304,21 +309,36 @@ def monitor_id_from_action(action): return None -# Actions gated by ROLE ONLY, never by per-endpoint Configuration Policy. +# Actions that are NOT a per-endpoint configuration concern. # -# These are the Corporate Tools actions. They start already-registered, -# agent-allowlisted tasks and are not a per-machine configuration concern, so a -# policy that never mentions them must not remove them. Policy exists to shape -# what a given PC TYPE exposes (which monitors, which service primitives); it is -# not the authority on whether an admin may run a corporate tool. +# Membership here is a statement about the action's nature, not its risk. An +# action belongs in this set when Configuration Policy has nothing meaningful to +# say about it: it invokes an already-registered, agent-allowlisted task rather +# than describing how a machine should be shaped. Policy exists to shape what a +# given PC TYPE exposes - which monitors run, which service primitives are +# reachable - so a policy that never mentions such an action must not be read as +# removing it. +# +# This is deliberately narrow. Do not add an action here merely because a denial +# is inconvenient; the correct remedy for an over-narrow policy is to fix the +# policy, or to use the audited admin break-glass override at the enqueue route. # # Without this exemption the intersection at the end of ALLOWED_ACTIONS_FOR # silently drops these actions on any endpoint whose effective policy is # non-empty, producing "Action not permitted by policy for this endpoint" even -# for a caller holding endpoints.deploy. The real gate for these stays -# ELEVATED_ACTION_PERMISSIONS (role), enforced in the enqueue route. +# for a caller who holds the matching grant. Exemption from POLICY is not +# exemption from PERMISSION: each of these actions still carries its own entry +# in ELEVATED_ACTION_PERMISSIONS, enforced per-action in the enqueue route +# before policy is ever consulted. POLICY_EXEMPT_ACTIONS = frozenset({ 'scheduledtask_run', + # Read-only telemetry refresh. Policy shapes WHAT a PC type exposes; asking + # an agent to report its existing state off-cadence changes nothing about + # the machine, so a policy that never mentions it must not be read as + # removing it. Without this, "Refresh Status" dies on every endpoint whose + # policy is non-empty - a denial no operator can act on, because the fix + # lives in a policy that was never about this action. + 'collect_now', }) @@ -330,9 +350,15 @@ def ALLOWED_ACTIONS_FOR(pctype=None, assetid=None): result is narrowed to the actions the endpoint's effective policy permits - this is the allowlist-gating half of the policy design. With the feature off (or no assetid), it returns the full base allowlist, so nothing changes until - a site opts in. Actions in POLICY_EXEMPT_ACTIONS are never narrowed away; - they are gated by role alone. Policy resolution is imported lazily to keep - this module free of a hard dependency on the computers plugin. + a site opts in. Actions in POLICY_EXEMPT_ACTIONS are never narrowed away, + because policy is not the authority on them; they remain gated by their own + entry in ELEVATED_ACTION_PERMISSIONS at the enqueue route. Policy resolution + is imported lazily to keep this module free of a hard dependency on the + computers plugin. + + This answers allowlist questions only. It does not consult desired-state + holds, and it is not a permission check - callers must still enforce + ELEVATED_ACTION_PERMISSIONS themselves. """ base = set(COMMAND_ALLOWLIST.keys()) if assetid is None: @@ -355,8 +381,9 @@ def ALLOWED_ACTIONS_FOR(pctype=None, assetid=None): # "not permitted by policy". Expand each monitor: to its kind's control # actions before intersecting. allowed = allowed | _monitor_control_actions(allowed) - # Role-gated actions are added back unconditionally: policy narrows the - # per-endpoint surface, but it does not own the corporate tools. + # Exempt actions are added back unconditionally: policy narrows the + # per-endpoint surface, but it is not the authority on these actions. Their + # own elevated permission still gates them in the enqueue route. allowed = allowed | POLICY_EXEMPT_ACTIONS # Never widen past the code-level allowlist: policy can only gate within it. return base & allowed @@ -393,3 +420,256 @@ def _monitor_control_actions(action_names): granted |= {'service_start', 'service_stop', 'service_status'} return granted + +def PLACE_POLICY_HOLD(assetid, action, reason, userid=None, username=None, + minutes=None): + """Suspend desired-state reconciliation of one action on one endpoint. + + Called after an admin break-glasses past a policy denial, or directly from + the admin hold dialog. Without this, an override on a desired-mode action is + undone by the next reconcile pass: the admin stops the service, the resolver + sees drift, and the command comes straight back. Gate-mode overrides need no + hold - nothing reverses them. + + minutes=None takes DEFAULT_HOLD_MINUTES. minutes=0 places an INDEFINITE hold + (expiresat null) that never clears itself and stands until released by hand. + Nothing surfaces a stale indefinite hold, so it suppresses reconciliation of + that action silently and forever - deliberate, but the sharpest edge here. + + Idempotent on an action already actively held: re-holding returns True + without stacking a second row. The multi-select picker makes "select all" + natural, and duplicate rows would mean release had to be pressed once per + hidden duplicate to actually resume policy. + + Returns True when a hold is in place for the action afterwards. Resolved + lazily against the computers plugin, matching ALLOWED_ACTIONS_FOR, so this + module keeps no hard dependency on it: with the computers plugin absent + there is no reconciliation to suppress, and False is the honest answer. + """ + try: + from plugins.computers.models import (PolicyActionHold, + DEFAULT_HOLD_MINUTES) + from shopdb.api import db + except Exception: + return False + + from datetime import datetime, timedelta + from sqlalchemy import or_ + + existing = (PolicyActionHold.query + .filter(PolicyActionHold.assetid == assetid, + PolicyActionHold.action == action, + PolicyActionHold.isactive.is_(True), + or_(PolicyActionHold.expiresat.is_(None), + PolicyActionHold.expiresat > datetime.utcnow())) + .first()) + if existing is not None: + return True + + if minutes is None: + expiresat = datetime.utcnow() + timedelta(minutes=DEFAULT_HOLD_MINUTES) + elif int(minutes) == 0: + expiresat = None + else: + expiresat = datetime.utcnow() + timedelta(minutes=int(minutes)) + + hold = PolicyActionHold( + assetid=assetid, + action=action, + expiresat=expiresat, + createdbyuserid=userid, + createdbyusername=username, + reason=reason, + ) + db.session.add(hold) + db.session.commit() + return True + + +def ACTIVE_POLICY_HOLDS(assetid): + """The unexpired break-glass holds on one endpoint, newest first. + + The read counterpart to PLACE_POLICY_HOLD, resolved lazily against the + computers plugin for the same reason: with that plugin absent there is no + reconciliation to suppress, so there are no holds and an empty list is the + honest answer. + + Expiry is filtered here rather than by a sweeper, matching the resolver, so + a stalled job can never silently extend a hold. Expired rows stay in the + table as the audit record; they are simply not reported as active. + """ + try: + from plugins.computers.models import PolicyActionHold + except Exception: + return [] + + from datetime import datetime + from sqlalchemy import or_ + + # Null expiresat is an indefinite hold and is always active, so it is + # matched explicitly - a bare comparison would report it as gone while the + # resolver still honours it, which is the worst kind of disagreement. + rows = (PolicyActionHold.query + .filter(PolicyActionHold.assetid == assetid, + PolicyActionHold.isactive.is_(True), + or_(PolicyActionHold.expiresat.is_(None), + PolicyActionHold.expiresat > datetime.utcnow())) + .all()) + # Sorted here rather than in SQL: NULLS FIRST is not portable across the + # dialects this runs on. Indefinite holds sort ahead of every dated one, + # then latest expiry first. Keyed on datetime.max rather than a numeric + # fallback: datetime.min.timestamp() raises on Windows, and the placeholder + # is never actually compared against a real date anyway - the leading flag + # already separates the two groups. + rows.sort(key=lambda row: (row.expiresat is None, + row.expiresat or datetime.max), + reverse=True) + return [{ + 'action': row.action, + 'expiresat': row.expiresat.isoformat() if row.expiresat else None, + 'reason': row.reason, + 'createdbyusername': row.createdbyusername, + } for row in rows] + + +def HOLDABLE_ACTIONS_FOR(assetid): + """Desired-mode actions on this endpoint, with their current hold state. + + Only desired-mode actions are offered: a hold suspends desired-state + reconciliation, so holding a gate-mode action would suspend nothing and the + control would silently do nothing. + + Returns [] when the computers plugin is absent or policies are off - with no + reconciliation running there is nothing to hold, and an empty picker is the + honest answer rather than a list of actions the hold would not affect. + """ + try: + from plugins.computers.services.policyresolver import effective_policy_for + except Exception: + return [] + + effective = effective_policy_for(assetid) + held = effective.heldactions + return [{ + 'action': name, + 'held': name in held, + } for name in sorted(effective.holdable_actions())] + + +def RELEASE_POLICY_HOLD(assetid, action): + """End an active hold early, resuming reconciliation of one action. + + Soft-deletes via isactive rather than dropping the row: the hold IS the + audit record of what was suppressed and by whom, and deleting it on release + would erase exactly the history worth reviewing. Both the resolver and + ACTIVE_POLICY_HOLDS already filter on isactive, so clearing it is enough to + resume reconciliation on the next resolve. + + Returns the number of holds released, so a caller can tell "released" from + "there was nothing to release". + """ + try: + from plugins.computers.models import PolicyActionHold + from shopdb.api import db + except Exception: + return 0 + + from datetime import datetime + from sqlalchemy import or_ + + # Indefinite holds must be releasable too. Without the null branch the one + # kind of hold that never clears on its own would also be the one kind the + # release button could not clear. + rows = (PolicyActionHold.query + .filter(PolicyActionHold.assetid == assetid, + PolicyActionHold.action == action, + PolicyActionHold.isactive.is_(True), + or_(PolicyActionHold.expiresat.is_(None), + PolicyActionHold.expiresat > datetime.utcnow())) + .all()) + for row in rows: + row.isactive = False + if rows: + db.session.commit() + return len(rows) + + +def ACTIVE_LOCK_FOR(assetid): + """The endpoint lock standing on this asset, or None. + + Unlike the hold helpers above there is no defensive cross-plugin import: + endpointlocks belongs to THIS plugin, so if the import could fail the + enqueue route calling it would not be running either. + + No expiry filter, deliberately. A lock has no TTL - it is cleared by an + admin or it stands. Anything that looks like an expiry check here would be + a bug. + """ + from .models import EndpointLock + + return (EndpointLock.query + .filter(EndpointLock.assetid == assetid, + EndpointLock.releasedat.is_(None)) + .order_by(EndpointLock.lockedat.desc()) + .first()) + + +def PLACE_ENDPOINT_LOCK(assetid, reason, userid=None, username=None): + """Quarantine one endpoint against non-admin commands. + + Idempotent on the active lock: if one already stands this returns it + unchanged rather than stacking a second row. Two active locks on one + endpoint is not a state this table should represent, and the caller who + "locked" a locked machine got the outcome they asked for either way. + + The invariant is enforced here rather than by a unique constraint because + released rows accumulate per asset as history, so uniqueness on assetid + would reject the second lock in an endpoint's life. + """ + from datetime import datetime + + from shopdb.api import db + + from .models import EndpointLock + + existing = ACTIVE_LOCK_FOR(assetid) + if existing is not None: + return existing + + lock = EndpointLock( + assetid=assetid, + reason=reason, + lockedbyuserid=userid, + lockedbyusername=username, + lockedat=datetime.utcnow(), + ) + db.session.add(lock) + db.session.commit() + return lock + + +def RELEASE_ENDPOINT_LOCK(assetid, userid=None, username=None): + """Lift the quarantine, recording who lifted it. + + Stamps releasedat rather than deleting the row: the lock IS the record that + this machine was quarantined and why, and that record is most wanted after + the lock is gone and no longer able to explain itself. Every read filters on + releasedat, so stamping it is enough to restore normal command flow. + + Returns the released lock, or None when there was nothing to release, so a + caller can tell those two cases apart. + """ + from datetime import datetime + + from shopdb.api import db + + lock = ACTIVE_LOCK_FOR(assetid) + if lock is None: + return None + + lock.releasedat = datetime.utcnow() + lock.releasedbyuserid = userid + lock.releasedbyusername = username + db.session.commit() + return lock + diff --git a/plugins/endpoints/migrations/versions/0018_endpoint_locks.py b/plugins/endpoints/migrations/versions/0018_endpoint_locks.py new file mode 100644 index 00000000..d35e362e --- /dev/null +++ b/plugins/endpoints/migrations/versions/0018_endpoint_locks.py @@ -0,0 +1,87 @@ +"""Add endpointlocks (admin-only endpoint quarantine). + +One table behind the endpoint lock: an admin-set refusal of operator commands +on a single endpoint, independent of policy and permission. + +Deliberately separate from the computers plugin's policyactionholds. That table +suppresses ONE desired-state action on the policy-resolution axis and expires on +a timer; this one refuses NON-ADMIN CALLERS on the whole endpoint and does not +expire at all. Two concepts, two tables, two enforcement points - collapsing +them would recreate the axis-mixing the surrounding refactor exists to undo. + +No expiresat column, and that is the design rather than an omission. A lock +stands for a physical-world condition (machine mid-repair, validated part +running, box that must not reboot during an audit) and those do not end on a +schedule. A TTL would silently unlock a machine someone still has their hands +inside. + +Ships EMPTY. No endpoint has ever been locked before this migration, so there is +nothing to backfill and every endpoint keeps behaving exactly as it did. + +Idempotent: the create checks the live schema first, so a re-run and a +db.create_all() site both stay clean on strict MySQL 8. +""" +from alembic import op + +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'endpoints0018endpointlocks' +down_revision = 'endpoints0017healthrollup' +branch_labels = None +depends_on = None + +LOCKS = 'endpointlocks' + + +def _has_table(table_name): + from sqlalchemy import inspect + + return inspect(op.get_bind()).has_table(table_name) + + +def upgrade(): + if not _has_table(LOCKS): + op.create_table( + LOCKS, + sa.Column('endpointlockid', sa.Integer(), nullable=False), + sa.Column('assetid', sa.Integer(), nullable=False), + sa.Column('reason', sa.Text(), nullable=False), + sa.Column('lockedbyuserid', sa.Integer(), nullable=True), + sa.Column('lockedbyusername', sa.String(length=100), nullable=True), + sa.Column('lockedat', sa.DateTime(), nullable=False), + # Null while the lock stands. Setting it is what releases the lock, + # so there is no separate active flag able to drift out of step. + sa.Column('releasedat', sa.DateTime(), nullable=True), + sa.Column('releasedbyuserid', sa.Integer(), nullable=True), + sa.Column('releasedbyusername', sa.String(length=100), + nullable=True), + sa.ForeignKeyConstraint(['assetid'], ['assets.assetid'], + ondelete='CASCADE'), + # SET NULL, not CASCADE: deleting the admin who locked a machine + # must not delete the record that it was locked. The username + # snapshot columns survive to say who it was. + sa.ForeignKeyConstraint(['lockedbyuserid'], ['users.userid'], + ondelete='SET NULL'), + sa.ForeignKeyConstraint(['releasedbyuserid'], ['users.userid'], + ondelete='SET NULL'), + sa.PrimaryKeyConstraint('endpointlockid'), + ) + # Serves the hot path: "is there an active lock on this asset", asked on + # every enqueue and every endpoint detail load. NOT unique on assetid - + # released rows accumulate per endpoint as history, and a unique + # constraint would make the second lock in a machine's life fail. The + # one-active-lock invariant is enforced on write instead. + op.create_index('idx_endpointlocks_assetactive', LOCKS, + ['assetid', 'releasedat']) + + +def downgrade(): + # Safe to drop: the table is a system of record for quarantine state, but a + # downgrade past this revision removes the enforcement that reads it, so + # leaving orphaned rows behind would be worse - they would silently reapply + # if the revision were ever re-applied. + if _has_table(LOCKS): + op.drop_index('idx_endpointlocks_assetactive', table_name=LOCKS) + op.drop_table(LOCKS) diff --git a/plugins/endpoints/models/__init__.py b/plugins/endpoints/models/__init__.py index 6912fa78..7dd95796 100644 --- a/plugins/endpoints/models/__init__.py +++ b/plugins/endpoints/models/__init__.py @@ -2,6 +2,7 @@ from .endpoint import ( EndpointState, EndpointEvent, EndpointCommand, EndpointApplication, + EndpointLock, ) from .monitor import EndpointMonitor from .operatorreport import OperatorReport @@ -11,6 +12,7 @@ from .health import ( __all__ = [ 'EndpointState', 'EndpointEvent', 'EndpointCommand', 'EndpointApplication', + 'EndpointLock', 'EndpointMonitor', 'OperatorReport', 'EndpointHealthState', 'EndpointHealthTransition', 'EndpointMonitorImpact', ] diff --git a/plugins/endpoints/models/endpoint.py b/plugins/endpoints/models/endpoint.py index 6a0a27a7..93c7420b 100644 --- a/plugins/endpoints/models/endpoint.py +++ b/plugins/endpoints/models/endpoint.py @@ -445,3 +445,89 @@ class EndpointApplication(db.Model): 'appid': self.appid, 'recordedat': _utciso(self.recordedat), } + + +class EndpointLock(db.Model): + """An admin-only quarantine refusing operator commands on one endpoint. + + Deliberately NOT the same thing as a PolicyActionHold, despite the similar + name. The two sit on different axes and collapsing them would recreate the + axis-mixing this refactor exists to undo: + + PolicyActionHold policy-resolution axis. Suppresses ONE desired-state + action so reconciliation stops fighting an admin. + Per-action, several may stand at once, and it EXPIRES. + EndpointLock caller-authority axis. Refuses commands from non-admins + on this endpoint regardless of policy or permission. + Per-endpoint, one at a time, and INDEFINITE. + + Indefinite is the point. A lock represents a physical-world condition - a + machine mid-repair, a validated part running, a box that must not reboot + during a customer audit - and those do not end on a timer. A TTL here would + quietly unlock a machine someone is still working inside, which is the exact + accident the lock exists to prevent. Stale locks are surfaced in the UI + instead of being auto-cleared. + + Released rows are kept and marked rather than deleted: the record of who + quarantined a machine, why, and who lifted it is the audit trail, and it is + most wanted precisely when the lock is no longer there to explain itself. + """ + + __tablename__ = 'endpointlocks' + + endpointlockid = db.Column(db.Integer, primary_key=True) + + # Not unique at the schema level: released rows accumulate per asset and a + # unique constraint would make the second lock in an endpoint's life fail. + # The one-active-lock invariant is enforced on write in PLACE_ENDPOINT_LOCK. + assetid = db.Column( + db.Integer, + db.ForeignKey('assets.assetid', ondelete='CASCADE'), + nullable=False, + index=True, + ) + + reason = db.Column(db.Text, nullable=False) + + # Who set it. The id goes null if the account is deleted, but the username + # snapshot survives: a quarantine that forgets who imposed it is unusable at + # review time, which is the only time anyone reads it. + lockedbyuserid = db.Column( + db.Integer, + db.ForeignKey('users.userid', ondelete='SET NULL'), + nullable=True, + ) + lockedbyusername = db.Column(db.String(100), nullable=True) + lockedat = db.Column(db.DateTime, nullable=False, default=datetime.utcnow) + + # Null while the lock stands. Set on release, which is what makes this row + # inactive - there is no isactive flag to drift out of step with them. + releasedat = db.Column(db.DateTime, nullable=True) + releasedbyuserid = db.Column( + db.Integer, + db.ForeignKey('users.userid', ondelete='SET NULL'), + nullable=True, + ) + releasedbyusername = db.Column(db.String(100), nullable=True) + + __table_args__ = ( + # Serves the hot path: "is there an active lock on this asset", asked on + # every enqueue and every detail page load. + db.Index('idx_endpointlocks_assetactive', 'assetid', 'releasedat'), + ) + + def __repr__(self): + state = 'released' if self.releasedat else 'active' + return f"" + + def to_dict(self): + return { + 'endpointlockid': self.endpointlockid, + 'assetid': self.assetid, + 'reason': self.reason, + 'lockedbyuserid': self.lockedbyuserid, + 'lockedbyusername': self.lockedbyusername, + 'lockedat': _utciso(self.lockedat), + 'releasedat': _utciso(self.releasedat), + 'releasedbyusername': self.releasedbyusername, + } diff --git a/plugins/endpoints/plugin.py b/plugins/endpoints/plugin.py index 9b0eda1e..a4ac7a8f 100644 --- a/plugins/endpoints/plugin.py +++ b/plugins/endpoints/plugin.py @@ -182,32 +182,28 @@ class EndpointsPlugin(BasePlugin): # remote reboot is a strictly heavier grant than a routine toggle. ('endpoints.restart', 'Restart endpoint computers (destructive)', 'endpoints'), - # Elevated grant for software delivery (run_script, update_app) and - # for starting approved scheduled tasks (scheduledtask_run). - # Required IN ADDITION to endpoints.command by the enqueue route, so - # running a staged script or pushing an installer is a strictly - # heavier grant than a routine service/process toggle. + # Elevated grants for the actions gated by SCOPE, never by + # Configuration Policy. Each is required IN ADDITION to + # endpoints.command by the enqueue route. # - # NOTE FOR THE ROLES WORK - this wire is pre-built for you. - # scheduledtask_run was deliberately mapped onto this EXISTING grant - # rather than minting a new permission, so nothing needs migrating. - # Be aware that WIDENS what endpoints.deploy means: it used to cover - # run_script + update_app, and now also covers starting any task on - # the agent's approved-task allowlist. Splitting it out is a one-line - # change (add endpoints.corptools here and remap it in - # commands.ELEVATED_ACTION_PERMISSIONS) - the enqueue route reads - # that table, so no other code changes. - # - # If you do split it, note the three are NOT equal risk: - # scheduledtask_run only starts something already registered on the - # endpoint and is allowlisted agent-side; update_app runs a pinned - # installer; run_script is arbitrary code that runs as SYSTEM unless - # its descriptor pins the user context. run_script is the reason this - # grant's description has to stay blunt. - ('endpoints.deploy', - 'Run staged scripts, start approved scheduled tasks, and push ' - 'application installers to endpoints (scripts run as SYSTEM; ' - 'grant only to fully trusted operators)', 'endpoints'), + # These were one permission (endpoints.deploy) covering run_script, + # update_app and scheduledtask_run. That bundle could not express + # "may push an application update" without also granting arbitrary + # SYSTEM code execution, so it is split. The three are NOT equal + # risk and their descriptions must not pretend otherwise. + ('endpoints.corptools', + 'Start approved scheduled tasks on endpoints (limited to tasks ' + 'already registered and allowlisted on the agent)', 'endpoints'), + ('endpoints.updateapp', + 'Push application installers to endpoints (runs a pinned ' + 'installer as SYSTEM)', 'endpoints'), + # Blunt on purpose. This is not "system-level changes" - it is + # arbitrary code execution as SYSTEM unless the script descriptor + # pins a user context. Whoever grants it must see that here. + ('endpoints.runscript', + 'Run staged scripts on endpoints (ARBITRARY CODE, executes as ' + 'SYSTEM unless the script pins a user context; grant only to ' + 'fully trusted operators)', 'endpoints'), # Packaging the agent is a heavier grant than deploying to one, and # it is not the same job either: the generated ZIP carries a live # collector key stamped into its install script, so whoever holds @@ -334,19 +330,6 @@ class EndpointsPlugin(BasePlugin): 'permission': 'endpoints.fleet', 'section': 'assets', }, - # TEMPORARY / DEV ONLY. A manual bench to exercise the update_app and - # run_script delivery path while Cameron builds the production UI. - # Gated on endpoints.deploy so only delivery operators see it. Remove - # this entry (and the DeliveryTestBench view/route) once the real - # delivery UI ships. - { - 'name': 'Delivery test bench', - 'icon': 'send', - 'route': '/endpoints/delivery-test', - 'position': 17, - 'permission': 'endpoints.deploy', - 'section': 'assets', - }, ] def init_app(self, app: Flask, db_instance) -> None: diff --git a/tests/test_plugins/test_endpoints_plugin.py b/tests/test_plugins/test_endpoints_plugin.py index c63a0ae9..df7bf4bd 100644 --- a/tests/test_plugins/test_endpoints_plugin.py +++ b/tests/test_plugins/test_endpoints_plugin.py @@ -177,6 +177,25 @@ def test_collect_now_is_allowlisted_targetless_and_routine(): assert elevated_permission_for('collect_now') is None +def test_collect_now_survives_a_policy_that_never_mentions_it(): + """Refresh Status must not be narrowed away by an unrelated policy. + + A non-empty effective policy skips the permissive fallback in + ALLOWED_ACTIONS_FOR, so any action the policy does not name is intersected + out. collect_now only asks an agent to report state it already has, so + Configuration Policy is not the authority on it - without the exemption + every policied endpoint refused Refresh Status with "Action not permitted + by policy", a denial the operator could not act on because the policy was + never about that action. + """ + from plugins.endpoints.commands import POLICY_EXEMPT_ACTIONS + + assert 'collect_now' in POLICY_EXEMPT_ACTIONS + # Exemption from POLICY is not exemption from PERMISSION: it stays gated by + # the base endpoints.command grant, which is what keeps guests out. + assert elevated_permission_for('collect_now') is None + + def test_run_script_and_update_app_are_allowlisted_delivery_actions(): """The two software-delivery actions are allowlisted with a targetkind so the enqueue route accepts them and carries their subject on the wire: @@ -189,19 +208,36 @@ def test_run_script_and_update_app_are_allowlisted_delivery_actions(): assert COMMAND_ALLOWLIST['update_app']['targetkind'] == 'app' -def test_delivery_actions_require_the_elevated_deploy_permission(): - """run_script and update_app both demand endpoints.deploy ON TOP OF - endpoints.command, so pushing software is a strictly heavier grant than a - routine service/process toggle.""" - assert elevated_permission_for('run_script') == 'endpoints.deploy' - assert elevated_permission_for('update_app') == 'endpoints.deploy' +def test_big_four_actions_each_require_their_own_elevated_permission(): + """The four scope-gated actions map to four INDEPENDENT permissions. + + They previously shared endpoints.deploy, which meant granting an operator + application updates also granted arbitrary SYSTEM code execution through + run_script. Keeping these distinct is the whole point of the split, so a + regression that re-bundles them must fail here. + """ + assert elevated_permission_for('system_restart') == 'endpoints.restart' + assert elevated_permission_for('run_script') == 'endpoints.runscript' + assert elevated_permission_for('update_app') == 'endpoints.updateapp' + assert elevated_permission_for('scheduledtask_run') == 'endpoints.corptools' + granted = { + elevated_permission_for(action) + for action in ('system_restart', 'run_script', 'update_app', + 'scheduledtask_run') + } + assert len(granted) == 4 -def test_endpoints_declares_the_deploy_permission(): - """The dedicated software-delivery permission is declared for role seeding.""" +def test_endpoints_declares_the_split_delivery_permissions(): + """Each split grant is declared so role seeding can offer it, and the + retired bundle is gone so the role grid stops offering a dead permission.""" plugin = EndpointsPlugin() permissions = {name for name, *_ in plugin.get_permissions()} - assert 'endpoints.deploy' in permissions + assert 'endpoints.runscript' in permissions + assert 'endpoints.updateapp' in permissions + assert 'endpoints.corptools' in permissions + assert 'endpoints.restart' in permissions + assert 'endpoints.deploy' not in permissions def test_known_payload_keys_cover_persisted_and_relayed_fields(): @@ -290,13 +326,26 @@ def test_fleet_list_route_resolves_without_trailing_slash(): # Dockerfile, the compose files or the Windows deploy pins TZ. def _in_zone(zonename, call): - """Run call() with the process in a named zone, then put it back.""" + """Run call() with the process in a named zone, then put it back. + + time.tzset() is POSIX-only and absent on Windows, where the process zone + cannot be moved this way at all. Rather than fail three tests permanently on + every Windows checkout - which teaches the developer to ignore a red suite - + the shift is skipped there and the assertions still run in whatever zone the + machine is in. That is weaker locally but not worthless: _parse_dt must + return the same UTC answer in EVERY zone, so the assertion is correct in all + of them, and CI runs on Linux where the real shift happens. + """ import os import time + tzset = getattr(time, 'tzset', None) + if tzset is None: + return call() + before = os.environ.get('TZ') os.environ['TZ'] = zonename - time.tzset() + tzset() try: return call() finally: @@ -304,7 +353,7 @@ def _in_zone(zonename, call): os.environ.pop('TZ', None) else: os.environ['TZ'] = before - time.tzset() + tzset() def test_a_utc_timestamp_survives_a_non_utc_server():