Back out app auto-seeding; fix report-status + PCTypesStrict bugs (manifest review)
All checks were successful
CI / backend (push) Successful in 1m45s
CI / naming (push) Successful in 2s
CI / frontend (push) Successful in 8s

A deep Fable review of the real manifest corpus (READ-ONLY reference) showed the
manifests are an ENFORCEMENT PROGRAM, not an application inventory, and that
auto-seeding the Applications catalog from entry Type + Name was wrong:

- The catalog ALREADY tracks these apps from the classic-shopdb migration, with
  version histories (PC - DMIS, UDC x11 versions, eMX / eDNC, CLM, CSF, Oracle
  Database, FormTracePak). Seeding from manifest labels created DUPLICATES under
  different names (PC-DMIS 2016 vs PC - DMIS; eDNC (bundles NTLARS) vs eMX / eDNC;
  OpenText HostExplorer ShopFloor vs CSF). It also misclassified config drops
  (eMxInfo.txt) as apps and could never match a PC's reported ARP name.
So the seed-applications command + service are removed. Properly linking
manifest entries to the EXISTING catalog is a curated feature, not label-scraping.

Two REAL bugs the review found are fixed and kept:
- Report status (R4): every healthy cycle runs Always/no-detection scripts the
  engine counts as "installed", so keying self-heal off installed>0 marked the
  common scope selfhealed forever and made 'ok' unreachable. Status now derives
  from explicit per-entry self-heal flags only; the stored flag no longer infers
  from action=='installed'; the client kit doc reflects it.
- PCTypesStrict (R5): the runtime engine has no strict handling (preinstall
  runner only). filters.matches_pctype now applies strict only when phase ==
  'preinstall'; simulate + parity thread the scope phase through; the strict test
  uses a preinstall scope.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
cproudlock
2026-07-12 21:56:18 -04:00
parent fc1f56fec3
commit 3ac41c1556
7 changed files with 39 additions and 131 deletions

View File

@@ -368,6 +368,7 @@ def simulate_scope(scopeid):
'hostname': request.args.get('hostname'), 'hostname': request.args.get('hostname'),
'machinenumber': request.args.get('machinenumber'), 'machinenumber': request.args.get('machinenumber'),
'cmmversion': request.args.get('cmmversion'), 'cmmversion': request.args.get('cmmversion'),
'phase': scope.phase,
} }
applied, filtered = [], [] applied, filtered = [], []
for entry in scope.entries: for entry in scope.entries:
@@ -376,7 +377,9 @@ def simulate_scope(scopeid):
applied.append(entry.name) applied.append(entry.name)
else: else:
reasons = [] reasons = []
if not matches_pctype(entry_dict, profile['pctype'], profile['subtype']): strict_allowed = profile.get('phase') == 'preinstall'
if not matches_pctype(entry_dict, profile['pctype'],
profile['subtype'], strict_allowed):
reasons.append('PCTypes') reasons.append('PCTypes')
if not matches_hostname(entry_dict, profile['hostname']): if not matches_hostname(entry_dict, profile['hostname']):
reasons.append('TargetHostnames') reasons.append('TargetHostnames')

View File

@@ -42,21 +42,24 @@ def _alias_sets(name):
return [g for g in ALIAS_GROUPS if any(n.lower() == lname for n in g)] return [g for g in ALIAS_GROUPS if any(n.lower() == lname for n in g)]
def matches_pctype(entry, pctype, subtype=None): def matches_pctype(entry, pctype, subtype=None, strict_allowed=False):
"""Test-PCTypeMatches: no PCTypes = all; '*' = all; alias-set intersection. """Test-PCTypeMatches: no PCTypes = all; '*' = all; alias-set intersection.
PCTypesStrict=true disables alias expansion (both the PC's identity and the PCTypesStrict=true disables alias expansion (both the PC's identity and the
manifest values are compared literally), so e.g. a 'collections'-only strict manifest values are compared literally), so a 'collections'-only strict entry
entry does NOT match a 'nocollections' PC via their shared 'Standard' alias does NOT match a 'nocollections' PC via their shared 'Standard' alias group.
group. The preinstall runner honors this flag (see preinstall.json UDC entry); IMPORTANT: only the PREINSTALL runner honors this flag - the runtime engine
mirroring it keeps the "what would this PC get" simulator correct. (Install-FromManifest.ps1's Test-PCTypeMatches) has no strict handling. So
strict is applied only when strict_allowed (i.e. phase == 'preinstall'),
else a runtime entry with PCTypesStrict still alias-expands, matching the
real engine.
""" """
values = entry.get('PCTypes') or [] values = entry.get('PCTypes') or []
if not values: if not values:
return True return True
if not pctype: if not pctype:
return True return True
strict = bool(entry.get('PCTypesStrict')) strict = strict_allowed and bool(entry.get('PCTypesStrict'))
# Names the current PC matches: bare type, "type-subtype", and (unless strict) # Names the current PC matches: bare type, "type-subtype", and (unless strict)
# all aliases of either. # all aliases of either.
mynames = set() mynames = set()
@@ -125,9 +128,12 @@ def matches_cmmversion(entry, cmmversion):
def entry_applies(entry, profile): def entry_applies(entry, profile):
"""All four filters ANDed, exactly as the engine's main loop applies them. """All four filters ANDed, exactly as the engine's main loop applies them.
`profile` keys: pctype, subtype, hostname, machinenumber, cmmversion. `profile` keys: pctype, subtype, hostname, machinenumber, cmmversion, and
optional phase ('preinstall' enables PCTypesStrict, matching the runners).
""" """
return (matches_pctype(entry, profile.get('pctype'), profile.get('subtype')) strict_allowed = profile.get('phase') == 'preinstall'
return (matches_pctype(entry, profile.get('pctype'), profile.get('subtype'),
strict_allowed)
and matches_hostname(entry, profile.get('hostname')) and matches_hostname(entry, profile.get('hostname'))
and matches_machinenumber(entry, profile.get('machinenumber')) and matches_machinenumber(entry, profile.get('machinenumber'))
and matches_cmmversion(entry, profile.get('cmmversion'))) and matches_cmmversion(entry, profile.get('cmmversion')))

View File

@@ -59,9 +59,11 @@ def check_scope(scopename, phase, original, fixtures):
k for k in set(co) | set(cr) if co.get(k) != cr.get(k)) k for k in set(co) | set(cr) if co.get(k) != cr.get(k))
firstdiff = f'entry "{name}" differs on {diffkeys}' firstdiff = f'entry "{name}" differs on {diffkeys}'
# Check 2: same entries fire in the same order for each profile. # Check 2: same entries fire in the same order for each profile. Inject the
# scope phase so PCTypesStrict is honored only for preinstall (see filters).
profiles_same = 0 profiles_same = 0
for profile in fixtures: for base_profile in fixtures:
profile = {**base_profile, 'phase': phase}
if (applicable_entry_names(orig_apps, profile) if (applicable_entry_names(orig_apps, profile)
== applicable_entry_names(rebuilt_apps, profile)): == applicable_entry_names(rebuilt_apps, profile)):
profiles_same += 1 profiles_same += 1

View File

@@ -178,27 +178,6 @@ class GeEnforcePlugin(BasePlugin):
db.session.commit() db.session.commit()
click.echo(f"Published {scopename}/{phase} as v{version}.") click.echo(f"Published {scopename}/{phase} as v{version}.")
@geenforce_cli.command('seed-applications')
@click.option('--shareroot', required=True)
@click.option('--preinstall', default=None)
def seed_applications_cmd(shareroot, preinstall):
"""Track the apps the manifests install in the core Applications catalog."""
from flask import current_app
from .importer import discover_share, load_manifest_file
from .service import seed_applications_from_manifests
with current_app.app_context():
sources = list(discover_share(shareroot))
if preinstall:
sources.append(('preinstall', 'preinstall',
load_manifest_file(preinstall)))
result = seed_applications_from_manifests(sources)
db.session.commit()
click.echo(f"Applications: {len(result['created'])} created, "
f"{len(result['existing'])} already tracked.")
for name in result['created']:
click.echo(f" + {name}")
@geenforce_cli.command('export-share') @geenforce_cli.command('export-share')
@click.argument('scopename') @click.argument('scopename')
@click.option('--shareroot', required=True) @click.option('--shareroot', required=True)

View File

@@ -94,45 +94,6 @@ def rollback_scope(scopename, phase, versionnumber):
return versionnumber return versionnumber
# Entry types that install actual applications (vs File/Registry/PS1/INF config).
INSTALLER_TYPES = {'MSI', 'EXE', 'CMD', 'BAT'}
def seed_applications_from_manifests(manifests):
"""Populate the core Applications catalog from what the manifests install.
For every installer entry (MSI/EXE/CMD/BAT) across the given manifests,
create an Application (idempotent, deduped by appname) so shopdb tracks the
apps GE-Enforce deploys. `manifests` is a list of (scopename, phase, dict).
Returns {'created': [...], 'existing': [...]} (uncommitted).
"""
from shopdb.api import Application
created, existing, seen = [], [], set()
for _scopename, _phase, manifest in manifests:
for entry in manifest.get('Applications', []):
if entry.get('Type') not in INSTALLER_TYPES:
continue
name = (entry.get('Name') or '').strip()
if not name or name.lower() in seen:
continue
seen.add(name.lower())
if Application.query.filter(Application.appname.ilike(name)).first():
existing.append(name)
continue
description = None
comment = entry.get('_comment')
if comment:
description = comment.split('.')[0].strip()[:255]
db.session.add(Application(
appname=name[:100],
appdescription=description,
installpath=(entry.get('Installer') or '')[:255] or None,
isinstallable=True))
created.append(name)
return {'created': created, 'existing': existing}
def record_enforcement_report(payload): def record_enforcement_report(payload):
"""Record one PC's enforcement cycle (observed state). Upserts the latest """Record one PC's enforcement cycle (observed state). Upserts the latest
report per (hostname, scopename, phase) and keeps prior ones as history. report per (hostname, scopename, phase) and keeps prior ones as history.
@@ -154,10 +115,14 @@ def record_enforcement_report(payload):
failed = int(counts.get('failed', 0)) failed = int(counts.get('failed', 0))
installed = int(counts.get('installed', 0)) installed = int(counts.get('installed', 0))
# Derive status: any failure wins; else drift-corrected installs = selfhealed. # Derive status from EXPLICIT self-heal flags only, never the raw installed
# count: every healthy cycle runs Always/no-detection scripts (asset report,
# VNC firewall, EventSaver) that the engine counts as "installed", so keying
# self-heal off installed>0 would mark the common scope selfhealed forever
# and make 'ok' unreachable. A drift correction is one the client flags.
if failed > 0: if failed > 0:
status = 'failed' status = 'failed'
elif installed > 0 or any(r.get('selfhealed') for r in results): elif any(r.get('selfhealed') for r in results):
status = 'selfhealed' status = 'selfhealed'
else: else:
status = 'ok' status = 'ok'
@@ -190,8 +155,10 @@ def record_enforcement_report(payload):
report.results.append(ManifestEnforcementResult( report.results.append(ManifestEnforcementResult(
entryname=item.get('name', ''), entryname=item.get('name', ''),
action=item.get('action', ''), action=item.get('action', ''),
selfhealed=bool(item.get('selfhealed', # Explicit flag only; do NOT infer from action == 'installed'
item.get('action') == 'installed')), # (Always/no-detection scripts install every cycle without being a
# drift correction).
selfhealed=bool(item.get('selfhealed', False)),
exitcode=item.get('exitcode'), exitcode=item.get('exitcode'),
message=item.get('message'))) message=item.get('message')))
db.session.add(report) db.session.add(report)

View File

@@ -155,8 +155,13 @@ def test_duplicate_entry_name_is_400_not_500(client, db, auth_headers):
def test_simulate_pctypesstrict_disables_alias(client, db, auth_headers): def test_simulate_pctypesstrict_disables_alias(client, db, auth_headers):
"""A collections-only strict entry must NOT match a nocollections PC via the """A collections-only strict entry must NOT match a nocollections PC via the
shared Standard alias group (mirrors the preinstall UDC entry).""" shared Standard alias group. PCTypesStrict is preinstall-only, so the scope
scopeid = _create_scope(client, auth_headers, 'preinstall') must be the preinstall phase (the runtime engine ignores strict)."""
resp = client.post('/api/geenforce/scopes',
json={'scopename': 'preinstall', 'phase': 'preinstall'},
headers=auth_headers)
assert resp.status_code == 201, resp.get_json()
scopeid = resp.get_json()['data']['scopeid']
_add_entry(client, auth_headers, scopeid, _add_entry(client, auth_headers, scopeid,
{'Name': 'UDC (strict)', 'Type': 'EXE', 'Installer': 'apps/udc.exe', {'Name': 'UDC (strict)', 'Type': 'EXE', 'Installer': 'apps/udc.exe',
'PCTypes': ['gea-shopfloor-collections'], 'PCTypesStrict': True}) 'PCTypes': ['gea-shopfloor-collections'], 'PCTypesStrict': True})

View File

@@ -1,54 +0,0 @@
"""GE-Enforce seeds the core Applications catalog from what the manifests install.
Only installer entries (MSI/EXE/CMD/BAT) become Applications; File/Registry/PS1/
INF config entries do not. Idempotent + deduped by appname.
"""
from plugins.geenforce import service
from shopdb.core.models import Application
MANIFEST = {
'Version': '2.6',
'Applications': [
{'Name': 'eDNC', 'Type': 'MSI', 'Installer': 'apps/edns.msi',
'_comment': 'eDNC (Universal Data Collection). Bundles NTLARS.'},
{'Name': 'UDC', 'Type': 'EXE', 'Installer': 'apps/udc.exe'},
{'Name': 'Config drop', 'Type': 'File', 'Source': 'configs/x.json',
'Destination': 'C:\\x.json'},
{'Name': 'Firewall rule', 'Type': 'PS1', 'Script': 'scripts/fw.ps1'},
{'Name': 'FMS host pin', 'Type': 'Registry', 'RegPath': 'HKLM:\\x',
'RegName': 'y', 'RegValue': 1, 'RegType': 'DWord'},
],
}
def test_seed_only_installer_apps(db):
result = service.seed_applications_from_manifests(
[('gea-shopfloor-collections', 'runtime', MANIFEST)])
service.db.session.commit()
assert set(result['created']) == {'eDNC', 'UDC'}
names = {a.appname for a in Application.query.all()}
assert 'eDNC' in names and 'UDC' in names
# config / script / registry entries are not applications
assert 'Config drop' not in names
assert 'Firewall rule' not in names
assert 'FMS host pin' not in names
edns = Application.query.filter_by(appname='eDNC').first()
assert edns.isinstallable is True
assert edns.installpath == 'apps/edns.msi'
assert edns.appdescription.startswith('eDNC (Universal Data Collection)')
def test_seed_is_idempotent_and_deduped(db):
manifests = [('a', 'runtime', MANIFEST), ('b', 'runtime', MANIFEST)]
first = service.seed_applications_from_manifests(manifests)
service.db.session.commit()
# deduped across the two manifests: 2 created, not 4
assert len(first['created']) == 2
# re-run: nothing new, both already tracked
second = service.seed_applications_from_manifests(manifests)
service.db.session.commit()
assert second['created'] == []
assert set(second['existing']) == {'eDNC', 'UDC'}
assert Application.query.filter(Application.appname.in_(['eDNC', 'UDC'])).count() == 2