From 31dc9d49e1518c33da7b0399e2b08e48e88235e2 Mon Sep 17 00:00:00 2001
From: "benjamin.baker" <benjamin.baker@geaerospace.com>
Date: Mon, 7 Sep 2026 13:03:07 -0400
Subject: [PATCH] Add an All PCs policy scope that excludes servers

The policy builder only offered "Matches everything" (the `true` scope node),
which also targets servers. Authoring an estate-wide policy almost always means
"every PC an operator sits at", and a reboot or script push against a server is
a materially different event.

Adds an `allpcs` scope node that matches every endpoint except those whose
computer type is server-designated. Server designation reads the computer type's
ICON slug, which is nominally a UI hint and is now load-bearing; it was chosen
over matching the type NAME (a site can rename "Server") and over a new isserver
column (needs a migration). It is isolated behind SERVER_ICON so promoting it to
a real flag later is a one-place change.

The `true` node is retained and still matches servers, so existing policies keep
their exact meaning rather than silently re-scoping. It stays visible in the
dropdown only when editing a policy that already uses it; new policies default
to `allpcs`.

Covered by tests/test_plugins/test_computers_policy_allpcs.py, verified against
a planted canary so the server-exclusion assertions are known to bite.
---
 .../views/ConfigurationPoliciesList.vue       |  4 +-
 .../frontend/views/ScopeNodeEditor.vue        | 11 +--
 plugins/computers/services/policyresolver.py  | 10 ++-
 plugins/computers/services/policyscope.py     | 29 ++++++-
 .../test_computers_policy_allpcs.py           | 87 +++++++++++++++++++
 5 files changed, 129 insertions(+), 12 deletions(-)
 create mode 100644 tests/test_plugins/test_computers_policy_allpcs.py

diff --git a/plugins/computers/frontend/views/ConfigurationPoliciesList.vue b/plugins/computers/frontend/views/ConfigurationPoliciesList.vue
index 2dfcd5b3..45eb98ae 100644
--- a/plugins/computers/frontend/views/ConfigurationPoliciesList.vue
+++ b/plugins/computers/frontend/views/ConfigurationPoliciesList.vue
@@ -140,7 +140,7 @@ const error = ref('')
 const form = ref(emptyForm())
 
 function emptyForm() {
-  return { name: '', description: '', scopejson: { type: 'true' }, actions: [] }
+  return { name: '', description: '', scopejson: { type: 'allpcs' }, actions: [] }
 }
 
 onMounted(() => loadData())
@@ -221,7 +221,7 @@ function openModal(item = null) {
     form.value = {
       name: item.name || '',
       description: item.description || '',
-      scopejson: item.scopejson ? JSON.parse(JSON.stringify(item.scopejson)) : { type: 'true' },
+      scopejson: item.scopejson ? JSON.parse(JSON.stringify(item.scopejson)) : { type: 'allpcs' },
       actions: (item.actions || []).map(a => ({
         action: a.action, mode: a.mode || 'gate',
         desiredstate: a.desiredstate, params: a.params, intervalminutes: a.intervalminutes
diff --git a/plugins/computers/frontend/views/ScopeNodeEditor.vue b/plugins/computers/frontend/views/ScopeNodeEditor.vue
index 439a43ed..f93d8e65 100644
--- a/plugins/computers/frontend/views/ScopeNodeEditor.vue
+++ b/plugins/computers/frontend/views/ScopeNodeEditor.vue
@@ -2,7 +2,8 @@
   <div class="scope-node">
     <div class="scope-node-row">
       <select v-model="node.type" class="form-control scope-type" @change="onTypeChange">
-        <option value="true">Matches everything</option>
+        <option value="allpcs">All PCs (excludes servers)</option>
+        <option v-if="node.type === 'true'" value="true">Matches everything (incl. servers)</option>
         <option value="pctype">PC type is</option>
         <option value="app">App is installed</option>
         <option value="and">ALL of (AND)</option>
@@ -71,20 +72,20 @@ function onTypeChange() {
   delete node.nodes
   delete node.node
   if (node.type === 'and' || node.type === 'or') {
-    node.nodes = [{ type: 'true' }]
+    node.nodes = [{ type: 'allpcs' }]
   } else if (node.type === 'not') {
-    node.node = { type: 'true' }
+    node.node = { type: 'allpcs' }
   }
 }
 
 function addChild() {
-  props.node.nodes.push({ type: 'true' })
+  props.node.nodes.push({ type: 'allpcs' })
 }
 
 function removeChild(index) {
   props.node.nodes.splice(index, 1)
   if (props.node.nodes.length === 0) {
-    props.node.nodes.push({ type: 'true' })
+    props.node.nodes.push({ type: 'allpcs' })
   }
 }
 </script>
diff --git a/plugins/computers/services/policyresolver.py b/plugins/computers/services/policyresolver.py
index cdcc5440..955e5304 100644
--- a/plugins/computers/services/policyresolver.py
+++ b/plugins/computers/services/policyresolver.py
@@ -20,7 +20,7 @@ The result is an EffectivePolicy: a flat, resolved map of action name -> decisio
 and the allowlist need.
 """
 
-from .policyscope import evaluate_scope
+from .policyscope import SERVER_ICON, evaluate_scope
 
 
 class EffectiveAction:
@@ -100,15 +100,21 @@ class EffectivePolicy:
 def build_endpoint_context(computer):
     """Resolve a Computer row into the context the scope evaluator needs.
 
-    Returns {'computertypeid': int|None, 'appids': set[int]}.
+    Returns {'computertypeid': int|None, 'appids': set[int], 'isserver': bool}.
     """
     appids = set()
     for link in getattr(computer, 'installedapps', None) or ():
         if getattr(link, 'isactive', True) and link.appid is not None:
             appids.add(link.appid)
+    # Server designation rides on the computer type's icon slug; policyscope
+    # owns that rule, we just resolve it for the endpoint. An unset type is not
+    # a server.
+    computertype = getattr(computer, 'computertype', None)
+    isserver = getattr(computertype, 'icon', None) == SERVER_ICON
     return {
         'computertypeid': getattr(computer, 'computertypeid', None),
         'appids': appids,
+        'isserver': isserver,
     }
 
 
diff --git a/plugins/computers/services/policyscope.py b/plugins/computers/services/policyscope.py
index 049c272d..9a660055 100644
--- a/plugins/computers/services/policyscope.py
+++ b/plugins/computers/services/policyscope.py
@@ -11,7 +11,21 @@ Node grammar (all dict nodes carry a "type"):
     {"type": "not",    "node": <node>}
     {"type": "pctype", "id": <computertypeid:int>}
     {"type": "app",    "id": <appid:int>}
-    {"type": "true"}   # matches everything (an unscoped/global policy)
+    {"type": "allpcs"} # every PC EXCEPT those whose type is designated a server
+    {"type": "true"}   # matches everything, servers included
+
+"allpcs" vs "true": authoring an estate-wide policy almost always means "every
+PC an operator sits at", not "every computer we have a row for". Those differ by
+the servers, where a reboot or a script push is a materially different event.
+"true" is retained because a stored scope of None is equivalent to it and older
+policies were written with it, but the builder offers "All PCs" instead.
+
+Server designation is read from the computer type's ICON slug ('server'). That
+field is nominally a UI hint, which is a real wart - it is load-bearing here. It
+was chosen over matching the type NAME because a site can rename "Server" but a
+renamed type keeps its icon, and over a new isserver column because that needs a
+migration this did not warrant. If server designation ever becomes a first-class
+flag, this is the single place that reads it.
 
 Kept pure and dependency-free (no db, no Flask) so it is trivially unit-tested
 and so a future GE-Enforce assembly engine can reuse the exact same evaluator on
@@ -22,8 +36,12 @@ the exact same AST when it composes policies.
 # recursion. Ten levels is far past any policy a human would author.
 MAX_SCOPE_DEPTH = 10
 
+# The computer-type icon slug that designates a type as a server. See the module
+# docstring for why the icon carries this.
+SERVER_ICON = 'server'
+
 _LOGICAL = {'and', 'or', 'not'}
-_LEAF = {'pctype', 'app', 'true'}
+_LEAF = {'pctype', 'app', 'true', 'allpcs'}
 
 
 class ScopeError(ValueError):
@@ -60,7 +78,7 @@ def validate_scope(node, _depth=0):
         ident = node.get('id')
         if not isinstance(ident, int) or isinstance(ident, bool):
             raise ScopeError(f'{ntype} requires an integer "id"')
-    # 'true' carries no payload.
+    # 'true' and 'allpcs' carry no payload.
 
 
 def evaluate_scope(node, context):
@@ -69,6 +87,7 @@ def evaluate_scope(node, context):
     context is expected to expose:
         context['computertypeid'] -> int or None
         context['appids']         -> set/collection of installed appids
+        context['isserver']       -> bool, whether the type is server-designated
 
     Returns True when the endpoint falls in scope. An empty/None scope means the
     policy is unscoped and matches everything (same as {"type": "true"}), which
@@ -80,6 +99,10 @@ def evaluate_scope(node, context):
     ntype = node['type']
     if ntype == 'true':
         return True
+    if ntype == 'allpcs':
+        # Absent designation means "not a server": an endpoint with no type set
+        # is an ordinary PC until something says otherwise.
+        return not context.get('isserver', False)
     if ntype == 'and':
         return all(evaluate_scope(child, context) for child in node['nodes'])
     if ntype == 'or':
diff --git a/tests/test_plugins/test_computers_policy_allpcs.py b/tests/test_plugins/test_computers_policy_allpcs.py
new file mode 100644
index 00000000..fe39a32e
--- /dev/null
+++ b/tests/test_plugins/test_computers_policy_allpcs.py
@@ -0,0 +1,87 @@
+"""Scope evaluation for the All PCs policy target.
+
+These are pure unit tests: policyscope has no db or Flask dependency, so the
+whole grammar can be exercised with plain dicts. build_endpoint_context needs
+only duck-typed stand-ins for a Computer row.
+"""
+import pytest
+
+from plugins.computers.services.policyresolver import build_endpoint_context
+from plugins.computers.services.policyscope import (
+    ScopeError,
+    evaluate_scope,
+    validate_scope,
+)
+
+
+class FakeComputerType:
+    def __init__(self, icon):
+        self.icon = icon
+
+
+class FakeComputer:
+    """Minimal stand-in for a Computer row, no db required."""
+
+    def __init__(self, computertypeid=None, icon=None, installedapps=()):
+        self.computertypeid = computertypeid
+        self.computertype = FakeComputerType(icon) if icon is not None else None
+        self.installedapps = installedapps
+
+
+def test_allpcs_is_a_valid_scope_node():
+    validate_scope({'type': 'allpcs'})
+
+
+def test_allpcs_matches_an_ordinary_pc():
+    context = {'computertypeid': 1, 'appids': set(), 'isserver': False}
+    assert evaluate_scope({'type': 'allpcs'}, context) is True
+
+
+def test_allpcs_does_not_match_a_server():
+    """The whole point of the option: servers are excluded."""
+    context = {'computertypeid': 4, 'appids': set(), 'isserver': True}
+    assert evaluate_scope({'type': 'allpcs'}, context) is False
+
+
+def test_allpcs_matches_an_endpoint_with_no_type_designation():
+    """An untyped endpoint is an ordinary PC until something says otherwise."""
+    context = {'computertypeid': None, 'appids': set()}
+    assert evaluate_scope({'type': 'allpcs'}, context) is True
+
+
+def test_true_still_matches_a_server():
+    """'true' keeps its old meaning so existing policies do not silently shift."""
+    context = {'computertypeid': 4, 'appids': set(), 'isserver': True}
+    assert evaluate_scope({'type': 'true'}, context) is True
+
+
+def test_allpcs_composes_with_other_nodes():
+    """AND of allpcs and an app still excludes a server running that app."""
+    scope = {
+        'type': 'and',
+        'nodes': [{'type': 'allpcs'}, {'type': 'app', 'id': 7}],
+    }
+    pc = {'computertypeid': 1, 'appids': {7}, 'isserver': False}
+    server = {'computertypeid': 4, 'appids': {7}, 'isserver': True}
+    assert evaluate_scope(scope, pc) is True
+    assert evaluate_scope(scope, server) is False
+
+
+def test_context_marks_a_server_icon_type_as_a_server():
+    computer = FakeComputer(computertypeid=4, icon='server')
+    assert build_endpoint_context(computer)['isserver'] is True
+
+
+def test_context_does_not_mark_a_desktop_type_as_a_server():
+    computer = FakeComputer(computertypeid=1, icon='desktop')
+    assert build_endpoint_context(computer)['isserver'] is False
+
+
+def test_context_handles_a_computer_with_no_type():
+    computer = FakeComputer()
+    assert build_endpoint_context(computer)['isserver'] is False
+
+
+def test_an_unknown_node_type_is_still_rejected():
+    with pytest.raises(ScopeError):
+        validate_scope({'type': 'allpc'})