diff --git a/plugins/computers/api/policy_routes.py b/plugins/computers/api/policy_routes.py index 65592e4e..3e6346d2 100644 --- a/plugins/computers/api/policy_routes.py +++ b/plugins/computers/api/policy_routes.py @@ -14,7 +14,7 @@ allowlist, so a bad tree or an unknown action never reaches the database. from flask import request from shopdb.api import (db, AuditLog, success_response, error_response, - ErrorCodes, require_permission) + ErrorCodes, require_permission, caller_holds) from .routes import computers_bp from ..models import (ConfigurationPolicy, ConfigurationPolicyAction, @@ -26,6 +26,77 @@ from ..services.policyscope import validate_scope, ScopeError _MODES = {ACTION_MODE_DESIRED, ACTION_MODE_GATE} +def _elevated_denial(actionnames): + """Refuse unless the caller may author every elevated action in actionnames. + + A policy is a deferred instruction to run something. Gating only the enqueue + route left authoring wide open: computers.edit alone could author a policy + carrying run_script and let the agent execute it, which is the whole point of + splitting the big four undone. The permission to CAUSE an action must be the + same permission required to RUN it. + + caller_holds, not current_user.haspermission: a scoped personal token is a + narrower caller than its owner, and asking the owner is the exact bypass + require_permission exists to prevent. + """ + from plugins.endpoints.commands import elevated_permission_for + + missing = [] + for name in sorted(set(actionnames)): + elevated = elevated_permission_for(name) + if elevated and not caller_holds(elevated) and elevated not in missing: + missing.append(elevated) + if not missing: + return None + return error_response( + ErrorCodes.FORBIDDEN, + 'This policy carries actions you may not author. Missing: ' + + ', '.join(missing), + http_code=403) + + +def _requested_actionnames(actions): + return [(row.get('action') or '').strip() for row in actions or []] + + +def _policy_actionnames(policy): + return [a.action for a in policy.actions] + + +def _composed_actionnames(configpolicyids): + """Every action reachable through a set of configuration policies. + + Composition is an authoring act: adding a configuration policy to a business + policy causes its actions to run on everything the business policy targets, + without the composer ever naming those actions. + """ + names = [] + for cpid in configpolicyids: + policy = db.session.get(ConfigurationPolicy, cpid) + if policy: + names.extend(_policy_actionnames(policy)) + return names + + +def _changes_effect(data, policy=None): + """Whether this payload changes WHAT a policy does, rather than how it reads. + + Scope widening, reactivation, priority and composition all change the effect + of an already-authored action without touching the action list, so gating the + action payload alone would leave every one of them open. Renaming a policy or + editing its description changes nothing that runs, so it stays ungated. + """ + for field in ('actions', 'items', 'scopejson', 'priority'): + if field in data: + return True + if 'isactive' in data: + # Only reactivation matters. Deactivating a policy cannot widen anything, + # and refusing it would strand a live policy nobody present can switch off. + was = getattr(policy, 'isactive', False) if policy is not None else False + return bool(data['isactive']) and not was + return False + + def _feature_gate(): """Return a 404 response when the policy feature is off, else None.""" if not policy_flag_enabled(): @@ -155,6 +226,10 @@ def create_configuration_policy(): return error_response(ErrorCodes.VALIDATION_ERROR, f'Invalid scope: {exc}') + denial = _elevated_denial(_requested_actionnames(data.get('actions'))) + if denial: + return denial + policy = ConfigurationPolicy( name=name, description=data.get('description'), @@ -189,6 +264,17 @@ def update_configuration_policy(policy_id: int): http_code=409) data = request.get_json() or {} + # Both sides. The incoming actions are what the caller is asking to cause; + # the existing ones are what a scope widening or reactivation would newly + # unleash on machines that were never in range before. + if _changes_effect(data, policy): + names = list(_policy_actionnames(policy)) + if 'actions' in data: + names.extend(_requested_actionnames(data['actions'])) + denial = _elevated_denial(names) + if denial: + return denial + if 'name' in data: newname = (data['name'] or '').strip() if newname and newname != policy.name: @@ -242,6 +328,11 @@ def delete_configuration_policy(policy_id: int): ErrorCodes.CONFLICT, f'Cannot delete: used by {inuse} business policy item(s)', http_code=409) + # Deleting a gate policy REMOVES a restriction, so it changes what endpoints + # will run just as surely as authoring one does. + denial = _elevated_denial(_policy_actionnames(policy)) + if denial: + return denial db.session.delete(policy) AuditLog.log('deleted', 'ConfigurationPolicy', entityname=policy.name) db.session.commit() @@ -324,6 +415,12 @@ def create_business_policy(): priority=data.get('priority', 100), source=POLICY_SOURCE_LOCAL, ) + requested = [row.get('configurationpolicyid') + for row in data.get('items') or []] + denial = _elevated_denial(_composed_actionnames( + [cpid for cpid in requested if cpid is not None])) + if denial: + return denial err = _apply_items(policy, data.get('items')) if err: return err @@ -349,6 +446,20 @@ def update_business_policy(policy_id: int): 'Policy is managed externally and is read-only', http_code=409) data = request.get_json() or {} + + # Composition is the quiet escalation: attaching a configuration policy runs + # its actions everywhere this business policy reaches, without the composer + # naming a single one of them. + if _changes_effect(data, policy): + cpids = [it.configurationpolicyid for it in policy.items] + for row in data.get('items') or []: + cpid = row.get('configurationpolicyid') + if cpid is not None: + cpids.append(cpid) + denial = _elevated_denial(_composed_actionnames(cpids)) + if denial: + return denial + if 'name' in data: newname = (data['name'] or '').strip() if newname and newname != policy.name: @@ -387,6 +498,10 @@ def delete_business_policy(policy_id: int): return error_response(ErrorCodes.CONFLICT, 'Policy is managed externally and is read-only', http_code=409) + denial = _elevated_denial(_composed_actionnames( + [it.configurationpolicyid for it in policy.items])) + if denial: + return denial db.session.delete(policy) AuditLog.log('deleted', 'BusinessPolicy', entityname=policy.name) db.session.commit() @@ -405,16 +520,29 @@ def list_policy_actions(): Returns command actions tagged kind='command'. Monitor attachments are not listed here: the UI builds that branch from the endpoints monitor catalog (endpointsApi.monitors) so newly created monitors appear without a reload. + + Actions the caller may not author are marked rather than hidden. Hiding them + would make the editor look like the capability does not exist; marking them + says it exists and is not yours, which is the difference between a confusing + UI and an honest one. The authoring gate is enforced server-side regardless. """ gate = _feature_gate() if gate: return gate - from plugins.endpoints.commands import COMMAND_ALLOWLIST - return success_response([ - {'action': name, 'label': meta.get('label'), - 'description': meta.get('description'), 'kind': 'command'} - for name, meta in COMMAND_ALLOWLIST.items() - ]) + from plugins.endpoints.commands import (COMMAND_ALLOWLIST, + elevated_permission_for) + catalog = [] + for name, meta in COMMAND_ALLOWLIST.items(): + elevated = elevated_permission_for(name) + catalog.append({ + 'action': name, + 'label': meta.get('label'), + 'description': meta.get('description'), + 'kind': 'command', + 'requiredpermission': elevated, + 'mayauthor': not elevated or caller_holds(elevated), + }) + return success_response(catalog) @computers_bp.route('/policies/effective/', methods=['GET']) diff --git a/plugins/computers/frontend/views/ActionPicker.vue b/plugins/computers/frontend/views/ActionPicker.vue index f7b42572..9f5b3647 100644 --- a/plugins/computers/frontend/views/ActionPicker.vue +++ b/plugins/computers/frontend/views/ActionPicker.vue @@ -17,9 +17,13 @@ v-for="opt in filteredCommands" :key="opt.action" class="action-option" + :class="{ 'action-locked': opt.mayauthor === false }" @mousedown.prevent="selectCommand(opt)" > {{ opt.label || opt.action }} + + needs {{ opt.requiredpermission }} +
+ + +
+ LOCKED + + by {{ endpointlock.lockedbyusername || 'a deleted user' }} + - {{ endpointlock.reason }} + + +
+ + +
+
+ {{ hold.action }} + + policy enforcement paused by + {{ hold.createdbyusername || 'a deleted user' }} + until {{ holdExpiryLabel(hold) }} - {{ hold.reason }} + + + +
+
+ + @@ -470,6 +522,163 @@

Computer not found

+ + + +
+ What this does +
+ + + +

Lock

+

+ Locking is separate, and it is about PEOPLE rather than policy. A + hold stops ShopDB correcting the machine; a lock stops other users + touching it. While a lock stands, nobody but an admin can run any + action against this computer - they are refused and shown your + reason instead. +

+

+ A lock never expires, so use it when the machine must be left alone + for reasons ShopDB cannot see - mid-repair, a validated part + running, an audit in progress. It stays until an admin unlocks it + from this page. Agent check-in and monitoring keep working + throughout; you do not go blind on the machine. +

+
+
+ + +
+

+ Nothing on this computer is under automatic policy enforcement right + now, so there is nothing for a hold to pause. You can still lock it + below. +

+
+ + + + +
+ + + + Recorded against your account and shown to anyone viewing this + computer. + +
+ + + + +
+ + + Blocks other users from running any action on this computer until an + admin unlocks it. They are shown your reason above. This does not + expire on its own. + +
+ + + @@ -486,6 +695,7 @@ import AssetTriageBand from '@/components/AssetTriageBand.vue' import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import AssetDetailTabs from '@/components/AssetDetailTabs.vue' import AssetRelationships from '@/components/AssetRelationships.vue' +import Modal from '@/components/Modal.vue' import { useIdentifierFlags } from '@/composables/identifierSettings' import { Copy } from 'lucide-vue-next' import { getEntraDeviceUrls } from '@/utils/siteSettings' @@ -611,12 +821,13 @@ const hasCloudIdentity = computed(() => portalTemplatesLoaded.value && Boolean( // populatedGroups comes from PluginAssetPanels, which knows which panels // actually returned data; Software is additionally true when core has an // installed-applications list of its own. -// Gates the Corporate Tools tab. endpoints.deploy currently bundles -// scheduledtask_run with run_script/update_app - see the note beside the -// permission declaration in plugins/endpoints/plugin.py if that gets split. +// Gates the Corporate Tools tab, which runs scheduledtask_run. That action now +// has its own grant (endpoints.corptools) instead of riding the old +// endpoints.deploy bundle, so seeing this tab no longer implies the ability to +// push installers or run arbitrary scripts. // Declared ahead of detailtabs because the watch below evaluates that computed // during setup, which reads canDeploy before this line would otherwise run. -const canDeploy = computed(() => authStore.hasPermission('endpoints.deploy')) +const canDeploy = computed(() => authStore.hasPermission('endpoints.corptools')) const detailtabs = computed(() => { const filled = assetPanels.value?.populatedGroups ?? new Set() @@ -640,7 +851,7 @@ const detailtabs = computed(() => { if (has('activity')) tabs.push({ key: 'activity', label: 'Activity' }) // Corporate Tools is permission-gated rather than data-gated: it is a set of // fixed actions, not a rendering of collected telemetry, so there is no - // populatedGroup to key off. Hidden without endpoints.deploy because every + // populatedGroup to key off. Hidden without endpoints.corptools because every // button here is an elevated command; the server re-checks regardless. if (canDeploy.value && liveness.value) { tabs.push({ key: 'corptools', label: 'Corporate Tools' }) @@ -662,6 +873,214 @@ const refreshing = ref(false) const canCommand = computed(() => authStore.hasPermission('endpoints.command')) const canRestart = computed(() => authStore.hasPermission('endpoints.restart')) +// Break-glass is admin-only and re-checked server-side against the CALLER, so a +// scoped token cannot inherit it from the admin who minted it. This flag only +// decides whether we offer the prompt. +const canOverridePolicy = computed(() => authStore.isAdmin) + +// Active holds are shown to everyone with endpoints.view: a held action means +// this PC is deliberately not being reconciled, and an operator who cannot see +// that is left debugging drift that someone already authorized. +const policyholds = ref([]) + +// Placing or releasing a hold is the same authority as break-glass, so it is +// admin-gated here and re-checked server-side against the CALLER. +const canHoldPolicy = computed(() => authStore.isAdmin) + +const showholddialog = ref(false) +const holdableactions = ref([]) +const holdactions = ref([]) +const holdreason = ref('') +const holdminutes = ref(240) +const holdbusy = ref(false) +const holdlockendpoint = ref(false) + +// Drives the warning panel. Guards against the empty input, which v-model.number +// yields as '' rather than 0 - treating that as indefinite would warn while the +// field looks blank, and worse, submit as indefinite. +const holdisindefinite = computed(() => holdminutes.value === 0) + +// Null when unlocked. Loaded for EVERY viewer, not just admins, so a blocked +// operator can read who locked the box instead of guessing at a refusal. +const endpointlock = ref(null) +const lockbusy = ref(false) + +// The dialog does two independent jobs, and on an endpoint with no policy +// enforcement only the lock half is available. Submitting needs a reason plus +// at least one job selected - otherwise the button would post nothing. +const canSubmitHold = computed(() => { + if (holdbusy.value || !holdreason.value.trim()) return false + // An empty duration is not a valid hold: it is neither a window nor an + // explicit indefinite choice, so block rather than guess which was meant. + if (holdactions.value.length && !Number.isInteger(holdminutes.value)) { + return false + } + return holdactions.value.length > 0 || holdlockendpoint.value +}) + +// Mirrors canSubmitHold in the same order, so the message always names the +// first thing actually standing in the way. Empty when the form is submittable. +const holdBlockedReason = computed(() => { + if (holdbusy.value) return '' + if (!holdreason.value.trim()) return 'Enter a reason to continue.' + if (!holdactions.value.length && !holdlockendpoint.value) { + return holdableactions.value.length + ? 'Select an action to pause, or tick "Also lock this endpoint".' + : 'Tick "Also lock this endpoint" to continue.' + } + if (holdactions.value.length && !Number.isInteger(holdminutes.value)) { + return 'Enter a duration in minutes, or 0 for indefinite.' + } + return '' +}) + +const holdSubmitLabel = computed(() => { + if (holdbusy.value) return 'Working...' + const count = holdactions.value.length + if (count && holdlockendpoint.value) { + return count > 1 ? `Place ${count} holds and lock` : 'Place hold and lock' + } + if (holdlockendpoint.value) return 'Lock endpoint' + return count > 1 ? `Place ${count} holds` : 'Place hold' +}) + +// Named for what the dialog can actually do here. Calling it "Place admin hold" +// on an endpoint with nothing to hold reads as a dead end. +const holdDialogTitle = computed(() => + holdableactions.value.length ? 'Place admin hold' : 'Lock endpoint') + +async function openHoldDialog() { + holdactions.value = [] + holdreason.value = '' + holdminutes.value = 240 + holdableactions.value = [] + // Default off: locking everyone else out is the bigger side effect of the two + // and should be a deliberate tick, not something inherited from last time. + holdlockendpoint.value = false + showholddialog.value = true + try { + const response = await endpointsApi.listHoldableActions(computer.value.assetid) + holdableactions.value = response.data?.data || [] + } catch (error) { + // Leave the list empty: the dialog then explains there is nothing under + // enforcement, which is the same thing the user needs to know either way. + holdableactions.value = [] + } +} + +async function submitHold() { + if (!canSubmitHold.value) return + const reason = holdreason.value.trim() + const actions = [...holdactions.value] + const wantshold = actions.length > 0 + const wantslock = holdlockendpoint.value + // Phrased once here so every branch below agrees on how the holds are named. + const summary = actions.length === 1 + ? actions[0] + : `${actions.length} actions` + holdbusy.value = true + try { + if (wantshold) { + // One call for every action: the server applies them atomically, so a + // rejected name leaves no partial set of holds behind. + await endpointsApi.placePolicyHold( + computer.value.assetid, actions, reason, holdminutes.value) + } + // Sequenced after the hold, not in parallel: if the lock call fails the + // hold still stands and the banner says so. Firing both at once could + // leave the page reporting a lock whose hold never landed. + if (wantslock) { + await endpointsApi.placeEndpointLock(computer.value.assetid, reason) + } + if (wantshold && wantslock) { + toast.success( + `Policy enforcement paused for ${summary} and endpoint locked`) + } else if (wantslock) { + toast.success('Endpoint locked') + } else { + toast.success(`Policy enforcement paused for ${summary}`) + } + showholddialog.value = false + await loadPolicyHolds() + await loadEndpointLock() + } catch (error) { + toast.error(apiError(error, wantshold + ? 'Failed to place hold' + : 'Failed to lock endpoint')) + } finally { + holdbusy.value = false + } +} + +async function confirmReleaseHold(hold) { + if (!hold) return + if (!confirm(`Resume automatic policy enforcement for ${hold.action}?\n\n` + + 'ShopDB will go back to correcting this action automatically, ' + + 'which may undo changes made while the hold was in place.')) return + holdbusy.value = true + try { + await endpointsApi.releasePolicyHold(computer.value.assetid, hold.action) + toast.success(`Policy enforcement resumed for ${hold.action}`) + await loadPolicyHolds() + } catch (error) { + toast.error(apiError(error, 'Failed to release hold')) + } finally { + holdbusy.value = false + } +} + +async function loadPolicyHolds() { + if (!computer.value?.assetid) return + try { + const response = await endpointsApi.listPolicyHolds(computer.value.assetid) + policyholds.value = response.data?.data || [] + } catch { + // Non-fatal: the holds banner is context, not a gate. A failure here must + // not blank the page or block the actions below it. + policyholds.value = [] + } +} + +async function loadEndpointLock() { + if (!computer.value?.assetid) return + try { + const response = await endpointsApi.getEndpointLock(computer.value.assetid) + endpointlock.value = response.data?.data || null + } catch { + // Same reasoning as the holds banner: context, not a gate. The server is + // the real enforcement point, so a failed read here cannot let anyone past + // a lock - it only means this page does not explain one. + endpointlock.value = null + } +} + +async function confirmReleaseLock() { + if (!endpointlock.value) return + if (!confirm('Unlock this endpoint?\n\n' + + 'Other users will be able to run actions against it again. ' + + 'Any policy holds you placed stay in force until they expire ' + + 'or you release them separately.')) return + lockbusy.value = true + try { + await endpointsApi.releaseEndpointLock(computer.value.assetid) + toast.success('Endpoint unlocked') + await loadEndpointLock() + } catch (error) { + toast.error(apiError(error, 'Failed to unlock endpoint')) + } finally { + lockbusy.value = false + } +} + +function holdExpiryLabel(hold) { + if (!hold.expiresat) return 'unknown' + // Server sends naive UTC isoformat; mark it as UTC so it renders in local time + // rather than being read as local and shifting by the offset. + const raw = hold.expiresat.endsWith('Z') ? hold.expiresat : `${hold.expiresat}Z` + const when = new Date(raw) + return Number.isNaN(when.getTime()) ? 'unknown' : when.toLocaleString() +} + // Warranty fetch + hero badge (shared across asset detail pages). const { heroWarranty, warrantyDate: formatWarrantyDate } = useWarrantyBadge(() => computer.value?.assetid) @@ -725,6 +1144,8 @@ onMounted(async () => { } catch (appError) { } loadLiveness(computer.value?.assetid) + loadPolicyHolds() + loadEndpointLock() // Warranties load via useWarrantyBadge (watches computer.assetid). } catch (error) { console.error('Error loading computer:', error) @@ -836,6 +1257,33 @@ async function waitForCommand(commandId, { attempts = 15, delayMs = 1000 } = {}) return 'timeout' } +// A policy denial is the server telling us this action is not exposed on this +// endpoint. For an admin that is recoverable, but only deliberately: we surface +// what was refused, demand a written reason, and send it as an explicit +// override. Returns true when the retry succeeded. +async function retryWithOverride(assetid, action, target, error, label) { + if (!canOverridePolicy.value) return false + const message = apiError(error, '') + if (!message.includes('not permitted by policy')) return false + + const reason = prompt( + `Policy does not permit ${action} on ${label}.\n\n` + + 'As an admin you may override this. The override is recorded against ' + + 'your account, and desired-state enforcement for this action is paused ' + + 'until the hold expires.\n\n' + + 'Reason for the override:') + // Cancel returns null; an empty or whitespace reason is refused server-side + // anyway, so do not spend a round trip on it. + if (!reason || !reason.trim()) return false + + await endpointsApi.enqueueCommand(assetid, action, target, { + policyoverride: true, + overridereason: reason.trim(), + }) + await loadPolicyHolds() + return true +} + // Destructive: queues a system_restart that reboots the PC immediately // (shutdown /r /t 0 on the agent). The confirm gate is the last stop before an // unrecoverable action; the server re-checks endpoints.restart regardless. @@ -849,7 +1297,17 @@ async function confirmRestart() { await endpointsApi.enqueueCommand(computer.value.assetid, 'system_restart') toast.success(`Restart queued for ${label}`) } catch (error) { - toast.error(apiError(error, 'Failed to queue restart')) + try { + const overridden = await retryWithOverride( + computer.value.assetid, 'system_restart', null, error, label) + if (overridden) { + toast.success(`Restart queued for ${label} (policy overridden)`) + return + } + toast.error(apiError(error, 'Failed to queue restart')) + } catch (retryError) { + toast.error(apiError(retryError, 'Failed to queue restart')) + } } finally { restarting.value = false } @@ -909,7 +1367,7 @@ const firingTask = ref('') // Elevated: queues a scheduledtask_run. The confirm gate mirrors // confirmRestart - these start real work on a live machine and some of them -// change system state - and the server re-checks endpoints.deploy regardless. +// change system state - and the server re-checks endpoints.corptools regardless. // // Tools marked destructive get a TYPED confirmation rather than an OK button, // for the same reason Restart has its own gate: applying firmware updates can @@ -1110,6 +1568,214 @@ async function confirmRetire() { justify-content: flex-end; gap: 0.5rem; } +/* Warning-toned, not error-toned: a hold is a deliberate, audited state, not a + fault. It should read as "someone did this on purpose" at a glance. */ +.policy-holds { + display: flex; + flex-direction: column; + gap: 0.35rem; + margin-top: 0.6rem; +} +/* Heavier than .policy-hold on purpose. A lock outranks a hold - it stops every + non-admin from acting at all - so it must not read as one more amber note in + a stack of them. */ +.endpoint-lock { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 0.5rem; + margin-top: 0.6rem; + padding: 0.45rem 0.7rem; + font-size: 0.85rem; + border-left: 3px solid var(--danger); + background: color-mix(in srgb, var(--danger) 12%, transparent); + border-radius: 3px; +} +.endpoint-lock-tag { + font-family: monospace; + font-weight: 700; + letter-spacing: 0.04em; + color: var(--danger); +} +.endpoint-lock-text { + color: var(--text); +} +.endpoint-lock-release { + margin-left: auto; + padding: 0.1rem 0.5rem; + font-size: 0.78rem; + color: var(--danger); + background: transparent; + border: 1px solid var(--danger); + border-radius: 3px; + cursor: pointer; +} +.endpoint-lock-release:hover:not(:disabled) { + background: color-mix(in srgb, var(--danger) 18%, transparent); +} +.endpoint-lock-release:disabled { + opacity: 0.55; + cursor: default; +} +/* Scrolls rather than growing the dialog: endpoints under broad policy can + carry a long action list, and the reason/duration fields below must stay + reachable without hunting for them. */ +.hold-action-list { + display: flex; + flex-direction: column; + gap: 0.15rem; + max-height: 13rem; + overflow-y: auto; + padding: 0.4rem 0.5rem; + border: 1px solid var(--border); + border-radius: 4px; +} +.hold-action-item { + display: flex; + align-items: center; + gap: 0.5rem; + padding: 0.25rem 0; + font-weight: 400; + cursor: pointer; +} +.hold-action-item input { + width: auto; + margin: 0; +} +.hold-action-item.is-held { + opacity: 0.6; + cursor: default; +} +.hold-action-name { + font-family: var(--font-mono, monospace); + font-size: 0.9rem; +} +.hold-action-note { + font-size: 0.78rem; + color: var(--text-light); + font-style: italic; +} +/* Deliberately loud. An indefinite hold is the one setting here with no + self-correcting behaviour behind it. + Tinted from --warning via colour-mix rather than a fixed pale hex: a light + cream panel keeps its dark body text in dark mode and turns unreadable. */ +.hold-indefinite-warn { + margin: 0.5rem 0 0; + padding: 0.5rem 0.65rem; + border-left: 3px solid var(--warning); + background: color-mix(in srgb, var(--warning) 12%, transparent); + font-size: 0.82rem; + line-height: 1.45; + color: var(--text); +} +/* Separated from the fields above it: this is a second, heavier decision than + the hold being configured, not another attribute of it. */ +.hold-lock-option { + padding-top: 0.75rem; + border-top: 1px solid var(--border); +} +.hold-lock-check { + display: flex; + align-items: center; + gap: 0.45rem; + font-weight: 600; + cursor: pointer; +} +.hold-lock-check input { + width: auto; + margin: 0; +} +.policy-hold { + display: flex; + flex-wrap: wrap; + align-items: baseline; + gap: 0.4rem; + font-size: 0.85rem; + padding: 0.35rem 0.6rem; + border-left: 3px solid var(--warning); + background: color-mix(in srgb, var(--warning) 10%, transparent); + border-radius: 3px; +} +.policy-hold-action { + font-family: monospace; + font-weight: 600; +} +.policy-hold-text { + color: var(--text); +} +/* Pushed to the row end so the release affordance reads as belonging to that + specific hold rather than to the banner as a whole. */ +.policy-hold-release { + margin-left: auto; + padding: 0.1rem 0.5rem; + font-size: 0.78rem; + color: var(--warning); + background: transparent; + border: 1px solid var(--warning); + border-radius: 3px; + cursor: pointer; +} +.policy-hold-release:hover:not(:disabled) { + background: color-mix(in srgb, var(--warning) 18%, transparent); +} +.policy-hold-release:disabled { + opacity: 0.55; + cursor: default; +} +.hold-explain { + margin-bottom: 1rem; + padding: 0.5rem 0.75rem; + background: var(--bg); + border: 1px solid var(--border); + border-radius: 4px; +} +.hold-explain summary { + cursor: pointer; + font-weight: 600; + font-size: 0.9rem; +} +.hold-explain-body { + margin-top: 0.5rem; + font-size: 0.875rem; + line-height: 1.5; +} +.hold-explain-body p { + margin: 0 0 0.6rem; +} +.hold-explain-body p:last-child { + margin-bottom: 0; +} +/* Labels the two halves apart. Without them the lock paragraphs read as more + detail about the hold, which is the exact conflation to avoid. */ +.hold-explain-head { + margin-bottom: 0.25rem !important; + font-weight: 700; + text-transform: uppercase; + font-size: 0.75rem; + letter-spacing: 0.05em; + color: var(--text-light); +} +.hold-explain-head:not(:first-child) { + margin-top: 0.9rem !important; +} +.hold-empty { + font-size: 0.9rem; + color: var(--text-light); +} +/* Sits to the left of the footer buttons, next to the control it explains. */ +.hold-blocked-reason { + margin-right: auto; + align-self: center; + text-align: left; + font-size: 0.8rem; + color: var(--text-light); +} +.form-hint { + display: block; + margin-top: 0.25rem; + font-size: 0.8rem; + color: var(--text-light); +} /* The chip styles live in AssetTriageBand's scoped block, so they do not reach this copy of the markup. Kept in step with that component deliberately. */ .triage-chip { diff --git a/plugins/computers/migrations/versions/0005_policyactionholds.py b/plugins/computers/migrations/versions/0005_policyactionholds.py new file mode 100644 index 00000000..56ff1f27 --- /dev/null +++ b/plugins/computers/migrations/versions/0005_policyactionholds.py @@ -0,0 +1,68 @@ +"""Add policyactionholds (break-glass desired-state suppression). + +Backs the admin override: when an admin break-glasses past a policy denial on a +desired-mode action, a hold row suspends reconciliation of that action on that +endpoint until expiresat, so the override is not immediately undone by the next +drift-correcting enqueue. + +Rows are never swept: expiry is evaluated at resolve time against expiresat, and +an expired row stays as the audit record of what was suppressed and why. + +Idempotent: the create_table is skipped when the table already exists (e.g. a +test DB built by db.create_all()). + +Revision ID: computers0005policyactionholds +Revises: computers0004policies +""" +from alembic import op +import sqlalchemy as sa + + +revision = 'computers0005policyactionholds' +down_revision = 'computers0004policies' +branch_labels = None +depends_on = None + + +def upgrade(): + bind = op.get_bind() + insp = sa.inspect(bind) + existing = set(insp.get_table_names()) + + if 'policyactionholds' not in existing: + op.create_table( + 'policyactionholds', + sa.Column('policyactionholdid', sa.Integer(), nullable=False), + sa.Column('assetid', sa.Integer(), nullable=False), + sa.Column('action', sa.String(length=100), nullable=False), + sa.Column('expiresat', sa.DateTime(), nullable=False), + sa.Column('createdbyuserid', sa.Integer(), nullable=True), + sa.Column('createdbyusername', sa.String(length=100), nullable=True), + sa.Column('reason', sa.Text(), nullable=False), + sa.Column('createddate', sa.DateTime(), nullable=False), + sa.Column('modifieddate', sa.DateTime(), nullable=False), + sa.Column('isactive', sa.Boolean(), nullable=False, + server_default=sa.true()), + sa.PrimaryKeyConstraint('policyactionholdid'), + sa.ForeignKeyConstraint(['assetid'], ['assets.assetid'], + ondelete='CASCADE'), + sa.ForeignKeyConstraint(['createdbyuserid'], ['users.userid'], + ondelete='SET NULL'), + ) + op.create_index('idx_policyactionhold_asset', 'policyactionholds', + ['assetid']) + op.create_index('idx_policyactionhold_expiry', 'policyactionholds', + ['expiresat']) + + +def downgrade(): + bind = op.get_bind() + insp = sa.inspect(bind) + existing = set(insp.get_table_names()) + + if 'policyactionholds' in existing: + op.drop_index('idx_policyactionhold_expiry', + table_name='policyactionholds') + op.drop_index('idx_policyactionhold_asset', + table_name='policyactionholds') + op.drop_table('policyactionholds') diff --git a/plugins/computers/migrations/versions/0006_policyactionhold_indefinite.py b/plugins/computers/migrations/versions/0006_policyactionhold_indefinite.py new file mode 100644 index 00000000..bbb88f56 --- /dev/null +++ b/plugins/computers/migrations/versions/0006_policyactionhold_indefinite.py @@ -0,0 +1,85 @@ +"""Allow indefinite policy action holds (policyactionholds.expiresat nullable). + +0005 created expiresat NOT NULL because every hold was time-boxed: a hold that +nobody remembered to clear would expire on its own and policy would resume. That +self-healing property was the point of the column. + +Admins now need to hold an action for work that cannot be bounded up front, so a +null expiresat is permitted and means indefinite - the hold stands until it is +released by hand. Everything reading this table must test +`expiresat IS NULL OR expiresat > now`; testing only the comparison silently +drops indefinite holds and lets policy resume against an admin who asked it not +to. + +No backfill: existing rows all carry a real expiry and keep it. Widening +NOT NULL to NULL rewrites no data. + +Idempotent: the alter is skipped when the column is already nullable, so a test +DB built by db.create_all() from the current model is left alone. + +Revision ID: computers0006holdindefinite +Revises: computers0005policyactionholds + +The id is kept short deliberately: alembic_version.version_num is VARCHAR(32), +and a longer id applies the DDL and then fails on the stamp, leaving the DB +migrated but recorded as un-migrated. +""" +from alembic import op +import sqlalchemy as sa + + +revision = 'computers0006holdindefinite' +down_revision = 'computers0005policyactionholds' +branch_labels = None +depends_on = None + + +def _expiresat_is_nullable(bind): + """True when policyactionholds.expiresat already permits null. + + Returns None when the table is absent entirely, which lets both directions + no-op rather than raising on a DB where the computers chain has not built + the table yet. + """ + insp = sa.inspect(bind) + if 'policyactionholds' not in set(insp.get_table_names()): + return None + for column in insp.get_columns('policyactionholds'): + if column['name'] == 'expiresat': + return bool(column.get('nullable')) + return None + + +def upgrade(): + bind = op.get_bind() + nullable = _expiresat_is_nullable(bind) + if nullable is None or nullable: + return + + op.alter_column('policyactionholds', 'expiresat', + existing_type=sa.DateTime(), + nullable=True) + + +def downgrade(): + """Re-impose NOT NULL, giving any indefinite hold a bounded expiry. + + A straight alter would fail on the rows this feature exists to create, so + indefinite holds are first stamped with the current time - expiring them + rather than destroying them. That is the conservative direction: on + downgrade, policy resuming is safer than reconciliation staying suppressed + by a row the older code cannot represent. The audit row survives either way. + """ + bind = op.get_bind() + nullable = _expiresat_is_nullable(bind) + if nullable is None or not nullable: + return + + op.execute(sa.text( + 'UPDATE policyactionholds ' + 'SET expiresat = CURRENT_TIMESTAMP ' + 'WHERE expiresat IS NULL')) + + op.alter_column('policyactionholds', 'expiresat', + existing_type=sa.DateTime(), + nullable=False) diff --git a/plugins/computers/models/__init__.py b/plugins/computers/models/__init__.py index bb66dc4d..45560a21 100644 --- a/plugins/computers/models/__init__.py +++ b/plugins/computers/models/__init__.py @@ -16,6 +16,8 @@ from .policy import ( POLICY_SOURCE_SHIPPED, ACTION_MODE_DESIRED, ACTION_MODE_GATE, + PolicyActionHold, + DEFAULT_HOLD_MINUTES, ) __all__ = [ @@ -32,4 +34,6 @@ __all__ = [ 'POLICY_SOURCE_SHIPPED', 'ACTION_MODE_DESIRED', 'ACTION_MODE_GATE', + 'PolicyActionHold', + 'DEFAULT_HOLD_MINUTES', ] diff --git a/plugins/computers/models/policy.py b/plugins/computers/models/policy.py index 47de49cc..90318733 100644 --- a/plugins/computers/models/policy.py +++ b/plugins/computers/models/policy.py @@ -202,3 +202,82 @@ class BusinessPolicyItem(BaseModel): def __repr__(self): return (f"") + + +# Default life of a break-glass suppression hold, used when the caller names no +# window. Long enough to comfortably outlast one maintenance window, short +# enough that nobody has to remember to clear it: policy resumes on its own. +# +# A caller MAY name any window instead, including none at all (expiresat NULL, +# an indefinite hold). That is a deliberate escape hatch for work that cannot be +# bounded up front, and it forfeits the self-healing this default exists to +# provide - see the class docstring. +DEFAULT_HOLD_MINUTES = 240 + + +class PolicyActionHold(BaseModel): + """A time-boxed suspension of desired-state reconciliation for one action. + + An admin break-glass override on a `gate`-mode action is a one-shot - policy + only said "not exposed", and nothing reverses it. An override on a + `desired`-mode action is a fight: the admin stops a service and the + reconciliation loop restarts it on the next check-in, so the override looks + broken in production. A hold records that fight's outcome server-side, and + the resolver drops the held action out of the desired set until it expires. + + Expiry is evaluated at RESOLVE time against expiresat, never by a sweeper + job. A sweeper that stalls would silently extend every hold it failed to + clear, which is policy disabled by accident - the failure mode this design + exists to avoid. Rows are left behind after expiry deliberately: an expired + hold is the audit record of what was suppressed, by whom, and why. + + expiresat is NULLABLE, and null means indefinite: the hold applies until an + admin releases it by hand. This is the one case where the self-healing + property above does not hold, so treat it as the sharp edge it is - an + indefinite hold silently disables reconciliation of that action forever, and + the endpoint will keep reporting healthy while it drifts. Nothing in the + system surfaces a stale hold; the only guard is the admin who placed it. + + Every read of this table must therefore test `expiresat IS NULL OR + expiresat > now`. Testing only the comparison drops indefinite holds, which + would let policy quietly resume against an admin who asked it not to. + """ + __tablename__ = 'policyactionholds' + + policyactionholdid = db.Column(db.Integer, primary_key=True) + + assetid = db.Column( + db.Integer, + db.ForeignKey('assets.assetid', ondelete='CASCADE'), + nullable=False, + ) + + # Allowlisted action name, same vocabulary as ConfigurationPolicyAction. + action = db.Column(db.String(100), nullable=False) + + # Naive UTC, matching BaseModel timestamps. Null means indefinite - the + # hold stands until released by hand. + expiresat = db.Column(db.DateTime, nullable=True) + + # Who placed it. The id goes null if the user is deleted, but the username + # snapshot survives: a break-glass record that forgets who used it when the + # account is removed is worthless at review time. + createdbyuserid = db.Column( + db.Integer, + db.ForeignKey('users.userid', ondelete='SET NULL'), + nullable=True, + ) + createdbyusername = db.Column(db.String(100), nullable=True) + + reason = db.Column(db.Text, nullable=False) + + __table_args__ = ( + db.Index('idx_policyactionhold_asset', 'assetid'), + db.Index('idx_policyactionhold_expiry', 'expiresat'), + ) + + def __repr__(self): + until = self.expiresat or 'indefinite' + return (f"") + diff --git a/plugins/computers/services/policyresolver.py b/plugins/computers/services/policyresolver.py index ba1a7f14..cdcc5440 100644 --- a/plugins/computers/services/policyresolver.py +++ b/plugins/computers/services/policyresolver.py @@ -57,17 +57,39 @@ class EffectivePolicy: actions maps action name -> EffectiveAction (first-match-wins already applied). Convenience views expose the gate allowlist and desired-state set that enforcement and the endpoints seam consume. + + heldactions are action names under an unexpired break-glass hold. They are + withheld from desired_actions (reconciliation stops fighting the admin) but + deliberately LEFT IN allowed_actions: a hold suspends drift correction, it + does not revoke permission to run the action by hand. """ - def __init__(self, actions): + def __init__(self, actions, heldactions=None): self.actions = actions + self.heldactions = set(heldactions or ()) def allowed_actions(self): """Set of action names permitted on this endpoint (gate + desired).""" return set(self.actions.keys()) def desired_actions(self): - """action name -> EffectiveAction for mode == 'desired' only.""" + """action name -> EffectiveAction for mode == 'desired' only. + + Held actions are excluded, so the reconciler does not re-enqueue the + very change an admin just broke glass to make. + """ + return {name: eff for name, eff in self.actions.items() + if eff.mode == 'desired' and name not in self.heldactions} + + def holdable_actions(self): + """Desired-mode action names, INCLUDING ones already held. + + Deliberately not desired_actions(): that view hides held actions, so + building a "what can I hold" picker from it would drop an action the + moment it was held and make the current state unreadable. Only + desired-mode actions appear, because a hold suspends reconciliation and + gate-mode actions have none to suspend. + """ return {name: eff for name, eff in self.actions.items() if eff.mode == 'desired'} @@ -165,7 +187,35 @@ def effective_policy_for(assetid): return EffectivePolicy({}) business_policies = BusinessPolicy.query.filter_by(isactive=True).all() - return build_effective_policy(computer, business_policies) + effective = build_effective_policy(computer, business_policies) + effective.heldactions = _held_actions_for(assetid) + return effective + + +def _held_actions_for(assetid): + """Action names under an unexpired break-glass hold for this endpoint. + + Expiry is compared here, at resolve time, rather than swept by a background + job. A sweeper that stalled would silently keep every hold it failed to + clear alive, which is reconciliation disabled by accident - so the row's own + expiresat is the single authority on whether it still applies. + + A null expiresat is an INDEFINITE hold and is always active. It must be + matched explicitly: a bare `expiresat > now` drops null rows, which would + resume reconciliation of exactly the actions an admin pinned open-endedly - + the reconciler restarting a service someone deliberately stopped. + """ + from datetime import datetime + from sqlalchemy import or_ + from ..models import PolicyActionHold + + rows = (PolicyActionHold.query + .filter(PolicyActionHold.assetid == assetid, + PolicyActionHold.isactive.is_(True), + or_(PolicyActionHold.expiresat.is_(None), + PolicyActionHold.expiresat > datetime.utcnow())) + .all()) + return {row.action for row in rows} def _monitor_action_ids(effective): diff --git a/tests/test_plugins/test_computers_policy_authoring_gate.py b/tests/test_plugins/test_computers_policy_authoring_gate.py new file mode 100644 index 00000000..2fdc9047 --- /dev/null +++ b/tests/test_plugins/test_computers_policy_authoring_gate.py @@ -0,0 +1,181 @@ +"""Authoring a policy requires the permission to run what the policy carries. + +A policy is a deferred instruction to run something. Gating only the enqueue +route (plugins/endpoints/api/routes.py) left authoring wide open: computers.edit +alone could author a policy carrying run_script and let the agent execute it on +every machine in scope, which undoes the whole point of splitting the big four. + +These tests pin the four ways a policy's EFFECT can change - authoring actions, +widening scope, reactivating, and composing - so a future edit cannot quietly +reopen any one of them. +""" + +from werkzeug.security import generate_password_hash + +from shopdb.core.models import Setting + + +CONFIG_URL = '/api/computers/policies/configuration' +BUSINESS_URL = '/api/computers/policies/business' + + +def _enable_policies(db): + if not Setting.query.filter_by(key='computers_configpolicies_enabled').first(): + db.session.add(Setting(key='computers_configpolicies_enabled', + value='true')) + db.session.commit() + + +def _user_with_perms(client, db, username, permnames): + """A non-admin holding exactly permnames. Returns login headers.""" + from shopdb.core.models import User, Role, Permission + + Permission.seed() + Permission.seed_entries([ + ('endpoints.runscript', 'Push scripts', 'endpoints'), + ('endpoints.restart', 'Remote restart', 'endpoints'), + ]) + db.session.flush() + role = Role(rolename=username + 'role') + db.session.add(role) + db.session.flush() + role.permissions = Permission.query.filter( + Permission.name.in_(permnames)).all() + user = User(username=username, email=username + '@test.local', + passwordhash=generate_password_hash('testpass')) + user.roles.append(role) + db.session.add(user) + db.session.commit() + login = client.post('/api/auth/login', + json={'username': username, 'password': 'testpass'}) + token = login.get_json()['data']['access_token'] + return {'Authorization': f'Bearer {token}'} + + +def _policy_editor(client, db, username='policyeditor', extra=()): + return _user_with_perms( + client, db, username, + ['computers.view', 'computers.create', 'computers.edit', + 'computers.delete', *extra]) + + +def _make_policy(db, name, action='run_script', scope=None, isactive=True): + from plugins.computers.models import (ConfigurationPolicy, + ConfigurationPolicyAction) + policy = ConfigurationPolicy(name=name, scopejson=scope, isactive=isactive) + db.session.add(policy) + db.session.flush() + db.session.add(ConfigurationPolicyAction( + configurationpolicyid=policy.configurationpolicyid, action=action)) + db.session.commit() + return policy.configurationpolicyid + + +def test_authoring_run_script_needs_the_runscript_permission(client, db): + """The core escalation: computers.create alone must not mint a policy that + runs arbitrary scripts.""" + _enable_policies(db) + headers = _policy_editor(client, db) + + response = client.post(CONFIG_URL, headers=headers, json={ + 'name': 'Sneaky', 'actions': [{'action': 'run_script'}], + }) + assert response.status_code == 403, response.get_json() + message = response.get_json()['data']['error']['message'] + assert 'endpoints.runscript' in message + + +def test_authoring_run_script_is_allowed_with_the_permission(client, db): + """Holding the grant makes it ordinary work, not a special case.""" + _enable_policies(db) + headers = _policy_editor(client, db, extra=['endpoints.runscript']) + + response = client.post(CONFIG_URL, headers=headers, json={ + 'name': 'Legitimate', 'actions': [{'action': 'run_script'}], + }) + assert response.status_code == 201, response.get_json() + + +def test_widening_scope_on_an_existing_policy_is_gated(client, db): + """Scope widening changes what an already-authored action DOES without + touching the action list. Gating the payload alone would miss it.""" + _enable_policies(db) + policyid = _make_policy(db, 'Narrow', scope={'type': 'pctype', 'id': 1}) + headers = _policy_editor(client, db) + + response = client.put(f'{CONFIG_URL}/{policyid}', headers=headers, + json={'scopejson': None}) + assert response.status_code == 403, response.get_json() + assert 'endpoints.runscript' in response.get_json()['data']['error']['message'] + + +def test_renaming_a_policy_is_not_gated(client, db): + """A rename changes nothing that runs. Gating it would be security theatre + that stops people tidying up.""" + _enable_policies(db) + policyid = _make_policy(db, 'Old Name') + headers = _policy_editor(client, db) + + response = client.put(f'{CONFIG_URL}/{policyid}', headers=headers, + json={'name': 'New Name', + 'description': 'clearer wording'}) + assert response.status_code == 200, response.get_json() + + +def test_reactivating_a_policy_is_gated_but_deactivating_is_not(client, db): + """Reactivation re-arms the action. Deactivation cannot widen anything, and + refusing it would strand a live policy nobody present can switch off.""" + _enable_policies(db) + dormant = _make_policy(db, 'Dormant', isactive=False) + live = _make_policy(db, 'Live', isactive=True) + headers = _policy_editor(client, db) + + blocked = client.put(f'{CONFIG_URL}/{dormant}', headers=headers, + json={'isactive': True}) + assert blocked.status_code == 403, blocked.get_json() + + allowed = client.put(f'{CONFIG_URL}/{live}', headers=headers, + json={'isactive': False}) + assert allowed.status_code == 200, allowed.get_json() + + +def test_composing_a_privileged_policy_into_a_business_policy_is_gated( + client, db): + """Composition is the quiet escalation: attaching a configuration policy + runs its actions without the composer ever naming them.""" + _enable_policies(db) + policyid = _make_policy(db, 'Privileged') + headers = _policy_editor(client, db) + + response = client.post(BUSINESS_URL, headers=headers, json={ + 'name': 'Wrapper', + 'items': [{'configurationpolicyid': policyid}], + }) + assert response.status_code == 403, response.get_json() + assert 'endpoints.runscript' in response.get_json()['data']['error']['message'] + + +def test_deleting_a_policy_carrying_an_elevated_action_is_gated(client, db): + """Deleting a gate policy REMOVES a restriction, so it changes what + endpoints will run just as surely as authoring one does.""" + _enable_policies(db) + policyid = _make_policy(db, 'Restriction') + headers = _policy_editor(client, db) + + response = client.delete(f'{CONFIG_URL}/{policyid}', headers=headers) + assert response.status_code == 403, response.get_json() + + +def test_the_action_catalog_marks_what_the_caller_may_author(client, db): + """Marked, not hidden. Hiding would make the editor look like the capability + does not exist; marking says it exists and is not yours.""" + _enable_policies(db) + headers = _policy_editor(client, db, extra=['endpoints.restart']) + + response = client.get('/api/computers/policies/actions', headers=headers) + assert response.status_code == 200, response.get_json() + catalog = {row['action']: row for row in response.get_json()['data']} + + assert catalog['system_restart']['mayauthor'] is True + assert catalog['run_script']['mayauthor'] is False + assert catalog['run_script']['requiredpermission'] == 'endpoints.runscript' diff --git a/tests/test_plugins/test_endpoints_policy_override.py b/tests/test_plugins/test_endpoints_policy_override.py new file mode 100644 index 00000000..5c600378 --- /dev/null +++ b/tests/test_plugins/test_endpoints_policy_override.py @@ -0,0 +1,885 @@ +"""Admin break-glass over policy gating, and the holds that make it stick. + +Policy gating is right almost always and wrong at 3am. These tests pin the +override's boundaries: it must be explicit, admin-only, reasoned, audited, and +it must NOT become a way to acquire an elevated permission the caller lacks. + +The hold half exists because an override on a desired-mode action is a fight +with the reconciler - without suppression the next check-in re-enqueues the +opposite command and the override looks broken. +""" + +from datetime import datetime, timedelta + +from werkzeug.security import generate_password_hash + +from shopdb.core.models import Setting + + +def _enable_policies(db): + if not Setting.query.filter_by(key='computers_configpolicies_enabled').first(): + db.session.add(Setting(key='computers_configpolicies_enabled', + value='true')) + db.session.commit() + + +def _endpoint(db, hostname='holdbox'): + """An Asset + Computer pair, the minimum a policy can resolve against.""" + from shopdb.api import Asset, AssetType + from plugins.computers.models import Computer + + assettype = AssetType.query.filter_by(assettype='computer').first() + if assettype is None: + assettype = AssetType(assettype='computer', pluginname='computers', + tablename='computers') + db.session.add(assettype) + db.session.flush() + + asset = Asset(assetnumber=hostname, name=hostname, + assettypeid=assettype.assettypeid, isactive=True) + db.session.add(asset) + db.session.flush() + db.session.add(Computer(assetid=asset.assetid, hostname=hostname)) + # The enqueue route 404s without a reported state, so the endpoint has to + # look like it has checked in at least once. + from plugins.endpoints.models import EndpointState + db.session.add(EndpointState(assetid=asset.assetid, hostname=hostname)) + db.session.commit() + return asset.assetid + + +def _login(client, username, password='testpass'): + response = client.post('/api/auth/login', + json={'username': username, 'password': password}) + token = response.get_json()['data']['access_token'] + return {'Authorization': f'Bearer {token}'} + + +def _user(db, client, username, permnames, admin=False): + from shopdb.core.models import User, Role, Permission + + Permission.seed() + Permission.seed_entries([ + ('endpoints.restart', 'Remote restart', 'endpoints'), + ]) + db.session.flush() + role = Role(rolename='admin' if admin else username + 'role') + existing = Role.query.filter_by(rolename=role.rolename).first() + if existing is not None: + role = existing + else: + db.session.add(role) + db.session.flush() + role.permissions = Permission.query.filter( + Permission.name.in_(permnames)).all() + user = User(username=username, email=username + '@test.local', + passwordhash=generate_password_hash('testpass')) + user.roles.append(role) + db.session.add(user) + db.session.commit() + return _login(client, username) + + +def _deny_everything(db, assetid): + """A policy that matches this endpoint but permits only an unrelated action, + so the action under test is denied by gating.""" + from plugins.computers.models import (ConfigurationPolicy, + ConfigurationPolicyAction, + BusinessPolicy, BusinessPolicyItem) + policy = ConfigurationPolicy(name='Locked down', scopejson=None) + db.session.add(policy) + db.session.flush() + db.session.add(ConfigurationPolicyAction( + configurationpolicyid=policy.configurationpolicyid, + action='mtconnect_status')) + business = BusinessPolicy(name='Fleet') + db.session.add(business) + db.session.flush() + db.session.add(BusinessPolicyItem( + businesspolicyid=business.businesspolicyid, + configurationpolicyid=policy.configurationpolicyid)) + db.session.commit() + + +def _url(assetid): + return f'/api/endpoints/{assetid}/commands' + + +def test_policy_denial_still_applies_without_an_override(client, db): + """The default path must be unchanged: no flag, no bypass.""" + _enable_policies(db) + assetid = _endpoint(db) + _deny_everything(db, assetid) + headers = _user(db, client, 'plainadmin', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler'}) + assert response.status_code == 400, response.get_json() + assert 'not permitted by policy' in \ + response.get_json()['data']['error']['message'] + + +def test_a_non_admin_cannot_override_a_policy_denial(client, db): + """endpoints.command is enough to queue work, never enough to break glass.""" + _enable_policies(db) + assetid = _endpoint(db) + _deny_everything(db, assetid) + headers = _user(db, client, 'operator', + ['endpoints.view', 'endpoints.command']) + + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler', + 'policyoverride': True, + 'overridereason': 'trying it on'}) + assert response.status_code == 403, response.get_json() + assert 'admin' in response.get_json()['data']['error']['message'] + + +def test_an_override_without_a_reason_is_refused(client, db): + """An unexplained break-glass is an unreviewable one.""" + _enable_policies(db) + assetid = _endpoint(db) + _deny_everything(db, assetid) + headers = _user(db, client, 'reasonless', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler', + 'policyoverride': True}) + assert response.status_code == 400, response.get_json() + assert 'overridereason' in response.get_json()['data']['error']['message'] + + +def test_an_admin_override_is_accepted_and_audited(client, db): + """The whole point, plus the audit trail that justifies allowing it.""" + from shopdb.api import AuditLog + + _enable_policies(db) + assetid = _endpoint(db) + _deny_everything(db, assetid) + headers = _user(db, client, 'breakglass', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler', + 'policyoverride': True, + 'overridereason': 'line down, need status'}) + assert response.status_code in (200, 201), response.get_json() + + entry = AuditLog.query.filter_by(action='policyoverridden').first() + assert entry is not None, 'break-glass must leave an audit record' + + +def test_override_is_checked_after_the_elevated_permission_gate(client, db): + """The override widens WHERE an action may run, never WHICH actions the + caller may use. + + A human admin holds every permission via the admin bypass, so this is moot + for people and cannot be shown with an admin fixture. It matters for TOKENS: + authz suspends the admin bypass for a scoped personal token, so a token + scoped to endpoints.command is refused at the elevated gate and never + reaches the override branch at all - even though its owner is an admin, and + even though it asks for the override explicitly. + """ + from shopdb.core.models import Permission + + _enable_policies(db) + assetid = _endpoint(db, 'tokenbox') + _deny_everything(db, assetid) + Permission.seed_entries([ + ('apitokens.create', 'Mint tokens', 'core'), + ]) + db.session.commit() + ownerheaders = _user(db, client, 'tokenadmin', + ['endpoints.view', 'endpoints.command', + 'apitokens.create'], admin=True) + + minted = client.post('/api/apitokens', headers=ownerheaders, + json={'name': 'agent', + 'scopes': ['endpoints.command']}) + secret = minted.get_json()['data']['secret'] + headers = {'Authorization': f'Bearer {secret}'} + + response = client.post(_url(assetid), headers=headers, + json={'action': 'system_restart', + 'policyoverride': True, + 'overridereason': 'leaked token tries it on'}) + assert response.status_code == 403, response.get_json() + + +def test_a_hold_suppresses_desired_state_reconciliation(client, db): + """Without this the reconciler undoes the override on the next check-in.""" + from plugins.computers.models import PolicyActionHold + from plugins.computers.services.policyresolver import effective_policy_for + + _enable_policies(db) + assetid = _endpoint(db, 'desiredbox') + + from plugins.computers.models import (ConfigurationPolicy, + ConfigurationPolicyAction, + BusinessPolicy, BusinessPolicyItem) + policy = ConfigurationPolicy(name='Keep it running', scopejson=None) + db.session.add(policy) + db.session.flush() + db.session.add(ConfigurationPolicyAction( + configurationpolicyid=policy.configurationpolicyid, + action='service_start', mode='desired', + desiredstate={'running': True})) + business = BusinessPolicy(name='Desired fleet') + db.session.add(business) + db.session.flush() + db.session.add(BusinessPolicyItem( + businesspolicyid=business.businesspolicyid, + configurationpolicyid=policy.configurationpolicyid)) + db.session.commit() + + assert 'service_start' in effective_policy_for(assetid).desired_actions() + + db.session.add(PolicyActionHold( + assetid=assetid, action='service_start', + expiresat=datetime.utcnow() + timedelta(minutes=60), + reason='maintenance window')) + db.session.commit() + + effective = effective_policy_for(assetid) + assert 'service_start' not in effective.desired_actions() + # A hold suspends drift correction; it does not revoke the permission to + # run the action by hand. + assert 'service_start' in effective.allowed_actions() + + +def test_an_expired_hold_stops_suppressing(client, db): + """Policy resumes on its own. Nobody has to remember to clear a hold, and a + stalled sweeper cannot silently disable reconciliation forever.""" + from plugins.computers.models import PolicyActionHold + from plugins.computers.services.policyresolver import effective_policy_for + + _enable_policies(db) + assetid = _endpoint(db, 'expiredbox') + + from plugins.computers.models import (ConfigurationPolicy, + ConfigurationPolicyAction, + BusinessPolicy, BusinessPolicyItem) + policy = ConfigurationPolicy(name='Still running', scopejson=None) + db.session.add(policy) + db.session.flush() + db.session.add(ConfigurationPolicyAction( + configurationpolicyid=policy.configurationpolicyid, + action='service_start', mode='desired', + desiredstate={'running': True})) + business = BusinessPolicy(name='Expiry fleet') + db.session.add(business) + db.session.flush() + db.session.add(BusinessPolicyItem( + businesspolicyid=business.businesspolicyid, + configurationpolicyid=policy.configurationpolicyid)) + db.session.add(PolicyActionHold( + assetid=assetid, action='service_start', + expiresat=datetime.utcnow() - timedelta(minutes=1), + reason='window that already closed')) + db.session.commit() + + assert 'service_start' in effective_policy_for(assetid).desired_actions() + + +def test_active_holds_are_visible_to_a_non_admin_viewer(client, db): + """A hold explains drift, so it must not be admin-only reading. + + An operator who can see the machine but not the hold is left debugging + suppressed reconciliation as if it were a fault. + """ + from plugins.computers.models import PolicyActionHold + + assetid = _endpoint(db) + db.session.add(PolicyActionHold( + assetid=assetid, action='service_stop', + expiresat=datetime.utcnow() + timedelta(minutes=60), + createdbyusername='nightadmin', + reason='vendor on site')) + db.session.commit() + + headers = _user(db, client, 'viewer', ['endpoints.view']) + response = client.get(f'/api/endpoints/{assetid}/policyholds', + headers=headers) + assert response.status_code == 200, response.get_json() + holds = response.get_json()['data'] + assert len(holds) == 1 + assert holds[0]['action'] == 'service_stop' + assert holds[0]['createdbyusername'] == 'nightadmin' + assert holds[0]['reason'] == 'vendor on site' + + +def test_expired_holds_are_not_reported_as_active(client, db): + """Expiry is evaluated on read, matching the resolver. + + If the route reported expired rows the banner would claim enforcement is + paused long after it resumed - worse than showing nothing. + """ + from plugins.computers.models import PolicyActionHold + + assetid = _endpoint(db) + db.session.add(PolicyActionHold( + assetid=assetid, action='service_stop', + expiresat=datetime.utcnow() - timedelta(minutes=1), + createdbyusername='nightadmin', + reason='window that already closed')) + db.session.commit() + + headers = _user(db, client, 'viewer2', ['endpoints.view']) + response = client.get(f'/api/endpoints/{assetid}/policyholds', + headers=headers) + assert response.status_code == 200, response.get_json() + assert response.get_json()['data'] == [] + + +def _desired_policy(db, assetid, action='service_start'): + """A policy putting one action under desired-state enforcement.""" + from plugins.computers.models import (ConfigurationPolicy, + ConfigurationPolicyAction, + BusinessPolicy, BusinessPolicyItem) + policy = ConfigurationPolicy(name=f'Desired {action}', scopejson=None) + db.session.add(policy) + db.session.flush() + db.session.add(ConfigurationPolicyAction( + configurationpolicyid=policy.configurationpolicyid, + action=action, mode='desired', desiredstate={'running': True})) + business = BusinessPolicy(name=f'Fleet {action}') + db.session.add(business) + db.session.flush() + db.session.add(BusinessPolicyItem( + businesspolicyid=business.businesspolicyid, + configurationpolicyid=policy.configurationpolicyid)) + db.session.commit() + + +def test_an_admin_can_place_a_hold_directly(client, db): + """The hold must be reachable deliberately, not only by tripping a denial.""" + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'holdadmin', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(f'/api/endpoints/{assetid}/policyholds', + headers=headers, + json={'action': 'service_start', + 'reason': 'vendor on site', + 'minutes': 60}) + assert response.status_code == 200, response.get_json() + holds = response.get_json()['data'] + assert [h['action'] for h in holds] == ['service_start'] + + # The point of a hold: reconciliation stops asking for this action. + from plugins.computers.services.policyresolver import effective_policy_for + assert 'service_start' not in effective_policy_for(assetid).desired_actions() + + +def test_a_non_admin_cannot_place_a_hold(client, db): + """endpoints.command queues work; it does not switch off enforcement.""" + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'holdoperator', + ['endpoints.view', 'endpoints.command']) + + response = client.post(f'/api/endpoints/{assetid}/policyholds', + headers=headers, + json={'action': 'service_start', + 'reason': 'trying it on'}) + assert response.status_code == 403, response.get_json() + + +def test_a_hold_requires_a_reason(client, db): + """An unexplained hold is an unattributable outage waiting to happen.""" + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'holdadmin2', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(f'/api/endpoints/{assetid}/policyholds', + headers=headers, + json={'action': 'service_start', 'reason': ' '}) + assert response.status_code == 400, response.get_json() + + +def test_holding_a_non_desired_action_is_refused(client, db): + """A hold on a gate-mode action would suppress nothing. + + Accepting it would report a hold that has no effect, which is worse than + refusing outright. + """ + _enable_policies(db) + assetid = _endpoint(db) + _deny_everything(db, assetid) + headers = _user(db, client, 'holdadmin3', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(f'/api/endpoints/{assetid}/policyholds', + headers=headers, + json={'action': 'mtconnect_status', + 'reason': 'not under enforcement'}) + assert response.status_code == 400, response.get_json() + assert 'nothing to hold' in \ + response.get_json()['data']['error']['message'] + + +def test_an_admin_can_release_a_hold_early(client, db): + """Release resumes reconciliation without waiting for expiry.""" + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'holdadmin4', + ['endpoints.view', 'endpoints.command'], admin=True) + + client.post(f'/api/endpoints/{assetid}/policyholds', headers=headers, + json={'action': 'service_start', 'reason': 'swapping a disk'}) + + from plugins.computers.services.policyresolver import effective_policy_for + assert 'service_start' not in effective_policy_for(assetid).desired_actions() + + response = client.delete( + f'/api/endpoints/{assetid}/policyholds/service_start', headers=headers) + assert response.status_code == 200, response.get_json() + assert response.get_json()['data'] == [] + assert 'service_start' in effective_policy_for(assetid).desired_actions() + + +def test_releasing_a_hold_keeps_the_audit_row(client, db): + """Release soft-deletes: the hold IS the record of what was suppressed.""" + from plugins.computers.models import PolicyActionHold + + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'holdadmin5', + ['endpoints.view', 'endpoints.command'], admin=True) + + client.post(f'/api/endpoints/{assetid}/policyholds', headers=headers, + json={'action': 'service_start', 'reason': 'keep this record'}) + client.delete(f'/api/endpoints/{assetid}/policyholds/service_start', + headers=headers) + + row = PolicyActionHold.query.filter_by(assetid=assetid, + action='service_start').first() + assert row is not None + assert row.isactive is False + assert row.reason == 'keep this record' + + +def test_holdable_actions_lists_desired_actions_with_hold_state(client, db): + """The picker must keep showing an action after it is held. + + Built from desired_actions() it would vanish on being held, making the + current state unreadable. + """ + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'holdadmin6', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.get(f'/api/endpoints/{assetid}/holdableactions', + headers=headers) + assert response.status_code == 200, response.get_json() + assert response.get_json()['data'] == [ + {'action': 'service_start', 'held': False}] + + client.post(f'/api/endpoints/{assetid}/policyholds', headers=headers, + json={'action': 'service_start', 'reason': 'still listed'}) + + response = client.get(f'/api/endpoints/{assetid}/holdableactions', + headers=headers) + assert response.get_json()['data'] == [ + {'action': 'service_start', 'held': True}] + + +def _lock_url(assetid): + return f'/api/endpoints/{assetid}/lock' + + +def test_a_lock_refuses_a_non_admin_command(client, db): + """The whole point: a locked endpoint stops other people acting on it. + + No policy is enabled here, so nothing but the lock can be doing the + refusing - otherwise this test would pass for the wrong reason. + """ + assetid = _endpoint(db) + adminheaders = _user(db, client, 'lockadmin', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(_lock_url(assetid), headers=adminheaders, + json={'reason': 'mid repair'}) + + headers = _user(db, client, 'lockoperator', + ['endpoints.view', 'endpoints.command']) + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler'}) + assert response.status_code == 403, response.get_json() + + +def test_a_lock_refusal_names_the_owner_and_reason(client, db): + """A bare "forbidden" sends an operator hunting through policy for a cause + that is not there. The refusal has to say who to ask and why.""" + assetid = _endpoint(db) + adminheaders = _user(db, client, 'lockadmin2', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(_lock_url(assetid), headers=adminheaders, + json={'reason': 'validated part running'}) + + headers = _user(db, client, 'lockoperator2', + ['endpoints.view', 'endpoints.command']) + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler'}) + message = response.get_json()['data']['error']['message'] + assert 'lockadmin2' in message + assert 'validated part running' in message + + +def test_an_admin_acts_through_a_lock_without_an_override(client, db): + """The lock gates WHO may act, not WHICH actions exist, so there is no + policy decision for an admin to break past. Requiring break-glass here would + mean an admin overriding their own quarantine to do the repair it was set + for.""" + assetid = _endpoint(db) + headers = _user(db, client, 'lockadmin3', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(_lock_url(assetid), headers=headers, + json={'reason': 'mine to work on'}) + + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler'}) + assert response.status_code == 201, response.get_json() + + +def test_a_lock_cannot_be_broken_by_a_policy_override(client, db): + """The two gates are independent and the lock is the outer one. + + policyoverride exists to push past a POLICY denial. If it also dissolved a + lock, any non-admin who learned the flag could act on a quarantined machine, + and the lock would be advisory. + """ + _enable_policies(db) + assetid = _endpoint(db) + _deny_everything(db, assetid) + adminheaders = _user(db, client, 'lockadmin4', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(_lock_url(assetid), headers=adminheaders, + json={'reason': 'do not touch'}) + + headers = _user(db, client, 'lockoperator4', + ['endpoints.view', 'endpoints.command']) + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler', + 'policyoverride': True, + 'overridereason': 'trying to slip past'}) + assert response.status_code == 403, response.get_json() + assert 'locked' in response.get_json()['data']['error']['message'].lower() + + +def test_a_non_admin_cannot_place_a_lock(client, db): + assetid = _endpoint(db) + headers = _user(db, client, 'lockoperator5', + ['endpoints.view', 'endpoints.command']) + + response = client.post(_lock_url(assetid), headers=headers, + json={'reason': 'not my call'}) + assert response.status_code == 403, response.get_json() + + +def test_a_lock_requires_a_reason(client, db): + """The reason is the message a blocked operator reads, not paperwork.""" + assetid = _endpoint(db) + headers = _user(db, client, 'lockadmin6', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(_lock_url(assetid), headers=headers, + json={'reason': ' '}) + assert response.status_code == 400, response.get_json() + + +def test_locking_twice_does_not_stack_a_second_lock(client, db): + """Two active locks on one endpoint is not a state the table should hold, + and the caller who locked a locked machine got what they asked for.""" + assetid = _endpoint(db) + headers = _user(db, client, 'lockadmin7', + ['endpoints.view', 'endpoints.command'], admin=True) + + first = client.post(_lock_url(assetid), headers=headers, + json={'reason': 'first'}) + second = client.post(_lock_url(assetid), headers=headers, + json={'reason': 'second'}) + assert second.status_code == 200, second.get_json() + # The original stands - the second call did not silently replace the reason + # the first admin recorded. + assert second.get_json()['data']['endpointlockid'] == \ + first.get_json()['data']['endpointlockid'] + assert second.get_json()['data']['reason'] == 'first' + + +def test_releasing_a_lock_restores_non_admin_commands(client, db): + assetid = _endpoint(db) + adminheaders = _user(db, client, 'lockadmin8', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(_lock_url(assetid), headers=adminheaders, + json={'reason': 'briefly'}) + client.delete(_lock_url(assetid), headers=adminheaders) + + headers = _user(db, client, 'lockoperator8', + ['endpoints.view', 'endpoints.command']) + response = client.post(_url(assetid), headers=headers, + json={'action': 'service_status', + 'target': 'spooler'}) + assert response.status_code == 201, response.get_json() + + +def test_releasing_a_lock_keeps_the_row_as_history(client, db): + """Released by stamping releasedat, not by deleting. The record of who + quarantined a machine is most wanted after the lock is gone and no longer + there to explain itself.""" + from plugins.endpoints.models import EndpointLock + + assetid = _endpoint(db) + headers = _user(db, client, 'lockadmin9', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(_lock_url(assetid), headers=headers, + json={'reason': 'keep this record'}) + client.delete(_lock_url(assetid), headers=headers) + + row = EndpointLock.query.filter_by(assetid=assetid).first() + assert row is not None + assert row.reason == 'keep this record' + assert row.lockedbyusername == 'lockadmin9' + assert row.releasedat is not None + assert row.releasedbyusername == 'lockadmin9' + + +def test_releasing_an_unlocked_endpoint_is_a_404(client, db): + """So a caller can tell "unlocked it" from "there was nothing to unlock".""" + assetid = _endpoint(db) + headers = _user(db, client, 'lockadmin10', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.delete(_lock_url(assetid), headers=headers) + assert response.status_code == 404, response.get_json() + + +def test_a_non_admin_can_read_the_lock(client, db): + """Read-gated, not admin-gated. An operator who cannot see the lock is left + with a bare refusal and files a ticket about a machine that is working.""" + assetid = _endpoint(db) + adminheaders = _user(db, client, 'lockadmin11', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(_lock_url(assetid), headers=adminheaders, + json={'reason': 'visible to all'}) + + headers = _user(db, client, 'lockoperator11', ['endpoints.view']) + response = client.get(_lock_url(assetid), headers=headers) + assert response.status_code == 200, response.get_json() + assert response.get_json()['data']['reason'] == 'visible to all' + + +def test_an_unlocked_endpoint_reports_no_lock(client, db): + assetid = _endpoint(db) + headers = _user(db, client, 'lockoperator12', ['endpoints.view']) + + response = client.get(_lock_url(assetid), headers=headers) + assert response.status_code == 200, response.get_json() + # success_response(None) omits data entirely rather than emitting null, so + # "no lock" is an absent key. The frontend reads it as response.data?.data + # || null, which lands on the same answer either way. + assert response.get_json().get('data') is None + + +def test_a_lock_does_not_suppress_policy_reconciliation(client, db): + """The two axes stay separate. A lock gates callers; it must not reach into + the resolver and start suppressing desired-state actions, which is the hold's + job and carries an expiry a lock does not have.""" + from plugins.computers.services.policyresolver import effective_policy_for + + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'lockadmin13', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(_lock_url(assetid), headers=headers, + json={'reason': 'locked but still reconciled'}) + + assert 'service_start' in effective_policy_for(assetid).desired_actions() + + +def test_an_admin_can_hold_several_actions_at_once(client, db): + """One dialog submission covers the whole job, not one action per trip. + + An admin about to work on a machine usually needs several actions pinned, + and making them repeat the dialog invites stopping halfway. + """ + from plugins.computers.services.policyresolver import effective_policy_for + + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid, action='service_start') + _desired_policy(db, assetid, action='service_stop') + headers = _user(db, client, 'multiadmin1', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(f'/api/endpoints/{assetid}/policyholds', + headers=headers, + json={'actions': ['service_start', 'service_stop'], + 'reason': 'stripping the enclosure', + 'minutes': 60}) + assert response.status_code == 200, response.get_json() + held = {h['action'] for h in response.get_json()['data']} + assert held == {'service_start', 'service_stop'} + + desired = effective_policy_for(assetid).desired_actions() + assert 'service_start' not in desired + assert 'service_stop' not in desired + + +def test_a_multi_action_hold_is_all_or_nothing(client, db): + """One bad name rejects the request whole. + + Partial success is the wrong failure mode on a break-glass control: the + admin walks away believing the machine is pinned, and the reconciler undoes + the work covered by the holds that never landed. + """ + from plugins.computers.models import PolicyActionHold + + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid, action='service_start') + headers = _user(db, client, 'multiadmin2', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post( + f'/api/endpoints/{assetid}/policyholds', + headers=headers, + # service_start is holdable; mtconnect_status is not under enforcement. + json={'actions': ['service_start', 'mtconnect_status'], + 'reason': 'half of this is invalid'}) + assert response.status_code == 400, response.get_json() + + # The valid half must NOT have been written on the way to the refusal. + assert PolicyActionHold.query.filter_by(assetid=assetid).count() == 0 + + +def test_a_zero_minute_hold_never_expires(client, db): + """0 means indefinite: it outlives any window a dated hold would have.""" + from plugins.computers.models import PolicyActionHold + from plugins.computers.services.policyresolver import effective_policy_for + + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'indefadmin1', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(f'/api/endpoints/{assetid}/policyholds', + headers=headers, + json={'actions': ['service_start'], + 'reason': 'awaiting a replacement part', + 'minutes': 0}) + assert response.status_code == 200, response.get_json() + + row = PolicyActionHold.query.filter_by(assetid=assetid, + action='service_start').first() + assert row.expiresat is None + # Reported active and honoured by the resolver despite having no expiry. + assert response.get_json()['data'][0]['expiresat'] is None + assert 'service_start' not in effective_policy_for(assetid).desired_actions() + + +def test_an_indefinite_hold_can_still_be_released(client, db): + """The one hold that never self-clears must be clearable by hand. + + Without a null branch in the release query this would be the only kind of + hold the release button could not touch - permanently stuck. + """ + from plugins.computers.services.policyresolver import effective_policy_for + + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'indefadmin2', + ['endpoints.view', 'endpoints.command'], admin=True) + client.post(f'/api/endpoints/{assetid}/policyholds', headers=headers, + json={'actions': ['service_start'], 'reason': 'indefinite', + 'minutes': 0}) + + response = client.delete( + f'/api/endpoints/{assetid}/policyholds/service_start', headers=headers) + assert response.status_code == 200, response.get_json() + assert 'service_start' in effective_policy_for(assetid).desired_actions() + + +def test_a_negative_hold_window_is_refused(client, db): + """A negative window places an already-expired hold. + + That reads as success in the UI while suppressing nothing - the silent + failure the holdable-action check exists to prevent. + """ + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'indefadmin3', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(f'/api/endpoints/{assetid}/policyholds', + headers=headers, + json={'actions': ['service_start'], + 'reason': 'backwards', 'minutes': -30}) + assert response.status_code == 400, response.get_json() + + +def test_re_holding_an_active_action_does_not_stack_rows(client, db): + """Idempotent, matching the endpoint lock. + + A duplicate row would have to be released once per hidden copy before + reconciliation actually resumed. + """ + from plugins.computers.models import PolicyActionHold + from plugins.computers.services.policyresolver import effective_policy_for + + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'dupeadmin1', + ['endpoints.view', 'endpoints.command'], admin=True) + + for _ in range(2): + client.post(f'/api/endpoints/{assetid}/policyholds', headers=headers, + json={'actions': ['service_start'], 'reason': 'twice'}) + + assert PolicyActionHold.query.filter_by(assetid=assetid, + action='service_start').count() == 1 + + # One release is therefore enough to resume reconciliation. + client.delete(f'/api/endpoints/{assetid}/policyholds/service_start', + headers=headers) + assert 'service_start' in effective_policy_for(assetid).desired_actions() + + +def test_a_single_action_hold_payload_still_works(client, db): + """The pre-list payload shape stays accepted so older callers keep working.""" + _enable_policies(db) + assetid = _endpoint(db) + _desired_policy(db, assetid) + headers = _user(db, client, 'legacyadmin1', + ['endpoints.view', 'endpoints.command'], admin=True) + + response = client.post(f'/api/endpoints/{assetid}/policyholds', + headers=headers, + json={'action': 'service_start', + 'reason': 'legacy payload'}) + assert response.status_code == 200, response.get_json() + assert [h['action'] for h in response.get_json()['data']] == \ + ['service_start']