diff --git a/agent/ShopDbEndpointAgent.Core/LaunchPathPolicy.cs b/agent/ShopDbEndpointAgent.Core/LaunchPathPolicy.cs index 4976dc38..bd4f6c11 100644 --- a/agent/ShopDbEndpointAgent.Core/LaunchPathPolicy.cs +++ b/agent/ShopDbEndpointAgent.Core/LaunchPathPolicy.cs @@ -63,6 +63,19 @@ public sealed class LaunchPathPolicy return false; } + // Reject an alternate data stream before anything else looks at the + // path. On .NET Framework an interior colon made GetFullPath throw; on + // net8.0 it does not, so "readme.txt:payload.exe" survives, reports an + // .exe extension, passes the root check, and File.Exists returns true + // for the stream. CreateProcess will run it. Write access to any + // existing file under an approved root would otherwise be enough. + var lastSegment = Path.GetFileName(resolved); + if (lastSegment.Contains(':')) + { + reason = "path names an alternate data stream"; + return false; + } + var extension = Path.GetExtension(resolved); if (!ApprovedExtensions.Contains(extension, StringComparer.OrdinalIgnoreCase)) { @@ -82,10 +95,81 @@ public sealed class LaunchPathPolicy return false; } - fullPath = resolved; + // Everything above is LEXICAL: GetFullPath is pure string normalization + // and never touches the disk, so the root check so far constrains the + // name, not the file. File.Exists follows reparse points, which means a + // junction anywhere under an approved root pointed the check at one + // place and the execution at another. Creating a junction needs no + // privilege, only write access to the parent - and vendor install trees + // under Program Files are not reliably locked down. + // + // So resolve what the path actually refers to and re-check THAT. A + // resolve failure rejects: this is the last gate before the agent runs + // the file. + string finalPath; + try + { + finalPath = ResolveFinalPath(resolved); + } + catch (Exception ex) + { + reason = $"cannot resolve the real path: {ex.Message}"; + return false; + } + + if (!IsUnderApprovedRoot(finalPath)) + { + reason = "path resolves outside every approved launch root"; + return false; + } + + fullPath = finalPath; return true; } + // The path with every symlink and junction on it followed, so the caller + // checks the file that will actually execute. The leaf is resolved first, + // then the directory chain above it - a junction is usually placed on a + // DIRECTORY, so resolving only the leaf would miss the case this exists to + // catch. + private static string ResolveFinalPath(string resolved) + { + var name = System.IO.File.ResolveLinkTarget(resolved, returnFinalTarget: true)?.FullName; + if (!string.IsNullOrWhiteSpace(name)) + { + // ResolveLinkTarget resolves the file itself; run it back through + // GetFullPath so the comparison sees the same shape as the roots. + return Path.GetFullPath(name); + } + + // Not a link. Its directory chain still might be, so resolve that. + var directory = Path.GetDirectoryName(resolved); + if (string.IsNullOrEmpty(directory)) + return resolved; + var realDirectory = ResolveDirectoryChain(directory); + return Path.GetFullPath(Path.Combine(realDirectory, Path.GetFileName(resolved))); + } + + // Walk up the directory chain resolving any junction or symlink on it. A + // junction is usually placed on a DIRECTORY, not on the executable, so + // resolving only the leaf would miss the case this exists to catch. + private static string ResolveDirectoryChain(string directory) + { + var resolvedDirectory = System.IO.Directory + .ResolveLinkTarget(directory, returnFinalTarget: true)?.FullName; + if (!string.IsNullOrWhiteSpace(resolvedDirectory)) + return resolvedDirectory; + + var parent = Path.GetDirectoryName(directory); + if (string.IsNullOrEmpty(parent) || parent == directory) + return directory; + + var realParent = ResolveDirectoryChain(parent); + return realParent == parent + ? directory + : Path.Combine(realParent, Path.GetFileName(directory)); + } + private bool IsUnderApprovedRoot(string resolved) { foreach (var root in approvedRoots) diff --git a/docs/PROJECT-MAP.md b/docs/PROJECT-MAP.md index d92c6cd6..8dc6d688 100644 --- a/docs/PROJECT-MAP.md +++ b/docs/PROJECT-MAP.md @@ -27,7 +27,7 @@ with `flask plugin upgrade-all`. Both are needed on a deploy. | backups | `backups0003clearlastseen` | | computers | `computers0004policies` | | employees | `employees0002photo` | -| endpoints | `endpoints0009opreports` | +| endpoints | `endpoints0010renamepolicy` | | geenforce | `geenforce0004minlib` | | guides | `guides0001baseline` | | knowledgebase | `knowledgebase0001anchor` | @@ -90,6 +90,6 @@ Manifest-less directories under `plugins/` are core frontend surface and always ## Size -- test functions defined: **1619** (parametrised cases collect higher) +- test functions defined: **1624** (parametrised cases collect higher) - documented API paths: **353** (`docs/openapi.json`, regenerate with `scripts/gen_openapi.py`) diff --git a/plugins/endpoints/api/monitors.py b/plugins/endpoints/api/monitors.py index 4e8f7e0e..db73d185 100644 --- a/plugins/endpoints/api/monitors.py +++ b/plugins/endpoints/api/monitors.py @@ -21,16 +21,30 @@ from flask_jwt_extended import current_user from shopdb.api import ( db, success_response, error_response, ErrorCodes, require_permission, - require_role, + require_role, Setting, ) from ..models import EndpointMonitor from .routes import endpoints_bp -# Shipped monitors default to this PC type until they are authored as policies. +# Which PC type the shipped monitors attach to, as a SETTING rather than a +# literal (ADR-015). PC-type vocabulary is per-site seed data: a plant whose +# type is called "Shop Floor Workstation" got no shipped monitors at all when +# this was hardcoded, and a plant that happens to use the name for something +# else got all of them. The default matches the seeded type so nothing changes +# for a site that uses it; a blank setting attaches the shipped bundle to +# nothing, which is the right neutral behaviour for a site that does not want +# it rather than a silent mismatch. +SHIPPED_PCTYPE_SETTING = 'plugin.endpoints.shippedmonitorpctype' SHIPPED_DEFAULT_PCTYPE = 'Shopfloor PC' + +def shipped_pctype(): + """The PC type the shipped monitor bundle attaches to at this site.""" + configured = Setting.get(SHIPPED_PCTYPE_SETTING, default=SHIPPED_DEFAULT_PCTYPE) + return (configured or '').strip() + _VALID_KINDS = ('process', 'service') @@ -93,9 +107,6 @@ def update_monitor(monitor_id): if monitor is None: return error_response(ErrorCodes.NOT_FOUND, 'Monitor not found', http_code=404) - if monitor.source != 'local': - return error_response(ErrorCodes.FORBIDDEN, - 'Shipped monitors cannot be edited', http_code=403) body = request.get_json(silent=True) or {} kind = (body.get('kind') or monitor.kind or '').strip().lower() @@ -110,8 +121,27 @@ def update_monitor(monitor_id): if problem: return error_response(ErrorCodes.VALIDATION_ERROR, problem, http_code=400) - monitor.kind = kind - monitor.displayname = (body.get('displayname', monitor.displayname) or '').strip() + # A shipped row is product-owned IDENTITY, not a product-owned opinion about + # one site's disk layout. It used to refuse every edit, which meant a plant + # whose eDNC lives on D:, or that runs no UDC at all, could neither correct + # the launch path nor switch the monitor off - and then got permanent false + # drift warnings for it. ADR-015 allows seed data precisely because a site + # can edit it afterwards. + # + # So: kind and displayname are the identity 0007's policy bundle matches on + # and stay fixed on a shipped row; the paths and the active flag, which are + # what actually differ between plants, are editable. + if monitor.source != 'local': + for field in ('kind', 'displayname'): + if field in body and (body.get(field) or '').strip() != ( + getattr(monitor, field) or ''): + return error_response( + ErrorCodes.FORBIDDEN, + 'A shipped monitor keeps its kind and display name; its ' + 'paths and active flag can be changed', http_code=403) + else: + monitor.kind = kind + monitor.displayname = (body.get('displayname', monitor.displayname) or '').strip() monitor.processname = (body.get('processname', monitor.processname) or '').strip() or None monitor.servicename = (body.get('servicename', monitor.servicename) or '').strip() or None monitor.launchpath = (body.get('launchpath', monitor.launchpath) or '').strip() or None @@ -142,13 +172,14 @@ def delete_monitor(monitor_id): def _resolve_pctype_monitors(pctype): """Monitor definitions a PC type should watch: shipped defaults + policy. - Shipped monitors are attached to SHIPPED_DEFAULT_PCTYPE as a code rule; + Shipped monitors attach to the PC type named by the shipped-pctype setting; policy-attached monitors come from the computers policy resolver by PC type. Returns a list of EndpointMonitor rows (active only), de-duped by id. """ selected = {} - if pctype == SHIPPED_DEFAULT_PCTYPE: + shipped_for = shipped_pctype() + if shipped_for and pctype == shipped_for: for monitor in EndpointMonitor.query.filter_by( source='shipped', isactive=True).all(): selected[monitor.endpointmonitorid] = monitor diff --git a/plugins/endpoints/migrations/versions/0007_shipped_monitor_policy.py b/plugins/endpoints/migrations/versions/0007_shipped_monitor_policy.py index eb43a6cc..e7672199 100644 --- a/plugins/endpoints/migrations/versions/0007_shipped_monitor_policy.py +++ b/plugins/endpoints/migrations/versions/0007_shipped_monitor_policy.py @@ -20,17 +20,27 @@ CRUD API refuses to edit or delete them): trying to START the process when a launch path is mapped - never something the product ships switched on for every endpoint. - BusinessPolicy "RPCA Shopfloor App Monitors" + BusinessPolicy "Shopfloor App Monitors" the site-level wrapper, carrying the Configuration Policy as its one item. + It was named for the originating plant when this arrived. A shipped row is + product-owned and every adopting site sees it, so the name cannot be one + site's designator (ADR-015). Migration 0010 renames it on installs that + already stamped this one. Monitors are looked up by identity (kind, displayname), never by a hardcoded id, so the bundle survives id drift across installs. Lives in the endpoints chain, after 0006, because the monitor rows it references -are seeded there. The computers policy tables are created by the core baseline -before per-plugin chains run; this migration still guards on their presence and -no-ops cleanly if they are absent, and is idempotent on the policy/business-policy -names so re-running never duplicates the bundle. +are seeded there. The policy tables belong to the COMPUTERS plugin and are +created by its own post-cutover chain, not by the core baseline; endpoints +declares a hard dependency on computers, which is what orders the two. + +It guards on those tables being present and no-ops if they are not, and it is +idempotent on the policy names so re-running never duplicates the bundle. Be +clear about what the guard costs, though: Alembic stamps the revision either +way, so if it ever no-ops the bundle is simply never seeded and nothing comes +back to finish the job. That is acceptable only because the dependency makes +the absent case unreachable in a supported install. """ from alembic import op @@ -47,7 +57,7 @@ depends_on = None CONFIG_POLICY_NAME = 'Shopfloor App Monitors' -BUSINESS_POLICY_NAME = 'RPCA Shopfloor App Monitors' +BUSINESS_POLICY_NAME = 'Shopfloor App Monitors' # The shipped monitors this bundle attaches, by identity (kind, displayname). _SHIPPED_MONITORS = ( diff --git a/plugins/endpoints/migrations/versions/0010_rename_shipped_policy.py b/plugins/endpoints/migrations/versions/0010_rename_shipped_policy.py new file mode 100644 index 00000000..1636f502 --- /dev/null +++ b/plugins/endpoints/migrations/versions/0010_rename_shipped_policy.py @@ -0,0 +1,60 @@ +"""Rename the shipped business policy off one site's designator. + +0007 seeded it as "RPCA Shopfloor App Monitors". RPCA is the originating +plant's facility designator, and the row is product-owned: source='shipped', +seeded into every install, and the CRUD API refuses to rename it. So every +adopting site would carry another plant's label at the top of its policy list +with no way to change it, which is exactly what ADR-015 exists to prevent. + +0007 now seeds the neutral name, which covers a fresh install. This migration +covers the installs that already stamped 0007 with the old one - the +originating site, and anyone who upgraded before this landed. + +Renames only when the old name is present AND the new one is not, so it is +idempotent and cannot collide with a site that already has a policy of its own +under the new name. Downgrade is deliberately one-way: putting another site's +designator back into a shipped row is not a state worth being able to reach. + +Revision ID: endpoints0010renamepolicy +Revises: endpoints0009opreports +""" +from alembic import op + +import sqlalchemy as sa + + +revision = 'endpoints0010renamepolicy' +down_revision = 'endpoints0009opreports' +branch_labels = None +depends_on = None + + +OLD_NAME = 'RPCA Shopfloor App Monitors' +NEW_NAME = 'Shopfloor App Monitors' + + +def _has_table(bind, name): + return sa.inspect(bind).has_table(name) + + +def upgrade(): + bind = op.get_bind() + # The policy tables belong to the computers plugin. Same guard as 0007: if + # they are not there, there is nothing seeded to rename. + if not _has_table(bind, 'businesspolicies'): + return + + exists = bind.execute( + sa.text('SELECT COUNT(*) FROM businesspolicies WHERE name = :name'), + {'name': NEW_NAME}).scalar() + if exists: + return + + bind.execute( + sa.text('UPDATE businesspolicies SET name = :new WHERE name = :old'), + {'new': NEW_NAME, 'old': OLD_NAME}) + + +def downgrade(): + # One-way on purpose. See the module docstring. + pass diff --git a/tests/test_plugin_migrations.py b/tests/test_plugin_migrations.py index 30ccb6f9..e1be5a7a 100644 --- a/tests/test_plugin_migrations.py +++ b/tests/test_plugin_migrations.py @@ -97,7 +97,9 @@ EXPECTED_HEAD_REVISION['warranty'] = 'warranty0002proof' # seeds the reusable monitor catalog, 0007 ships the shopfloor monitor policy # bundle, 0008 adds the warn-only reconciler drift cache column, and 0009 adds # the operatorreports table for tray issue reports. -EXPECTED_HEAD_REVISION['endpoints'] = 'endpoints0009opreports' +# 0010 renames the shipped business policy off the originating site's +# designator on installs that already stamped 0007 (ADR-015). +EXPECTED_HEAD_REVISION['endpoints'] = 'endpoints0010renamepolicy' # Plugins built after the cutover: their 0001 baseline really creates tables the # core chain never owned. diff --git a/tests/test_plugins/test_endpoints_policy_monitors.py b/tests/test_plugins/test_endpoints_policy_monitors.py index e088e8c4..af4c38ed 100644 --- a/tests/test_plugins/test_endpoints_policy_monitors.py +++ b/tests/test_plugins/test_endpoints_policy_monitors.py @@ -197,3 +197,86 @@ def test_resolved_monitors_unknown_hostname_is_404(client, db, app): response = client.get(RESOLVED_URL, query_string={'hostname': 'NOSUCHBOX'}, headers=_collector_headers(app)) assert response.status_code == 404 + + +# --- shipped rows are product-owned IDENTITY, not a fixed opinion about disks -- +# +# The shipped bundle used to be uneditable in every field and attached to a +# hardcoded PC-type name. A plant whose eDNC sits on another drive, or whose +# type is called something else, could neither correct it nor turn it off, and +# then carried permanent false drift for it. ADR-015 allows seed data because a +# site can edit it afterwards. + +def _shipped(db, displayname='eDNC', **kwargs): + from plugins.endpoints.models import EndpointMonitor + + monitor = EndpointMonitor( + kind=kwargs.pop('kind', 'process'), + displayname=displayname, + processname=kwargs.pop('processname', 'DncMain'), + launchpath=kwargs.pop('launchpath', r'C:\Program Files\Dnc\bin\DncMain.exe'), + source='shipped', isactive=True, **kwargs) + db.session.add(monitor) + db.session.flush() + return monitor + + +def test_a_site_can_correct_a_shipped_monitors_launch_path( + client, db, auth_headers): + monitor = _shipped(db) + response = client.put( + '/api/endpoints/monitors/%d' % monitor.endpointmonitorid, + json={'launchpath': r'D:\Dnc\bin\DncMain.exe'}, headers=auth_headers) + assert response.status_code == 200 + assert response.get_json()['data']['launchpath'] == r'D:\Dnc\bin\DncMain.exe' + + +def test_a_site_can_switch_off_a_shipped_monitor_it_does_not_run( + client, db, auth_headers): + monitor = _shipped(db, displayname='Universal Data Collector', + processname='UDC') + response = client.put( + '/api/endpoints/monitors/%d' % monitor.endpointmonitorid, + json={'isactive': False}, headers=auth_headers) + assert response.status_code == 200 + assert response.get_json()['data']['isactive'] is False + + +def test_a_shipped_monitor_keeps_the_identity_the_policy_matches_on( + client, db, auth_headers): + """0007 attaches its bundle by (kind, displayname), so those stay put.""" + monitor = _shipped(db) + response = client.put( + '/api/endpoints/monitors/%d' % monitor.endpointmonitorid, + json={'displayname': 'Renamed'}, headers=auth_headers) + assert response.status_code == 403 + assert monitor.displayname == 'eDNC' + + +# --- which PC type the shipped bundle attaches to is a setting --------------- + +def test_shipped_monitors_follow_the_configured_pctype(client, db, app): + from plugins.endpoints.api.monitors import ( + SHIPPED_PCTYPE_SETTING, _resolve_pctype_monitors) + + _shipped(db) + Setting.set(SHIPPED_PCTYPE_SETTING, 'Shop Floor Workstation') + db.session.commit() + + # The plant's own name for the type resolves the bundle... + assert [m.displayname for m in + _resolve_pctype_monitors('Shop Floor Workstation')] == ['eDNC'] + # ...and the name this shipped with no longer does. + assert _resolve_pctype_monitors('Shopfloor PC') == [] + + +def test_a_blank_shipped_pctype_attaches_the_bundle_to_nothing(client, db, app): + """The neutral behaviour for a site that does not want the bundle: off, + rather than silently attached to whatever happens to match.""" + from plugins.endpoints.api.monitors import ( + SHIPPED_PCTYPE_SETTING, _resolve_pctype_monitors) + + _shipped(db) + Setting.set(SHIPPED_PCTYPE_SETTING, '') + db.session.commit() + assert _resolve_pctype_monitors('Shopfloor PC') == [] diff --git a/tests/test_plugins/test_endpoints_shipped_monitor_policy.py b/tests/test_plugins/test_endpoints_shipped_monitor_policy.py index bbdccfeb..901d586d 100644 --- a/tests/test_plugins/test_endpoints_shipped_monitor_policy.py +++ b/tests/test_plugins/test_endpoints_shipped_monitor_policy.py @@ -4,7 +4,7 @@ Migration endpoints0007shippedpolicy offloads the eDNC/UDC/MTConnect monitors from the agent's hardcoded appsettings.json into a product-owned policy bundle: a shipped ConfigurationPolicy "Shopfloor App Monitors" (global scope, one desired-state monitor action per shipped monitor) wrapped in a shipped -BusinessPolicy "RPCA Shopfloor App Monitors". +BusinessPolicy "Shopfloor App Monitors". Run the endpoints chain against a fresh engine, alongside the computers policy tables the bundle writes into, and assert the bundle exists, is source='shipped' @@ -25,7 +25,7 @@ _ENDPOINTS_VERSIONS = _ROOT / 'plugins' / 'endpoints' / 'migrations' / 'versions _COMPUTERS_VERSIONS = _ROOT / 'plugins' / 'computers' / 'migrations' / 'versions' CONFIG_POLICY_NAME = 'Shopfloor App Monitors' -BUSINESS_POLICY_NAME = 'RPCA Shopfloor App Monitors' +BUSINESS_POLICY_NAME = 'Shopfloor App Monitors' def _load(path, prefix):