diff --git a/docker-compose.yml b/docker-compose.yml index 5407a3c4..71afcbc6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -90,7 +90,8 @@ services: flask plugin upgrade-all && flask seed permissions && flask seed settings && - flask seed reference-data + flask seed reference-data && + flask seed shipped-scripts volumes: - ./plugins:/app/plugins:ro - ./scripts:/app/scripts:ro diff --git a/plugins/endpoints/api/reportipscript.py b/plugins/endpoints/api/reportipscript.py new file mode 100644 index 00000000..6628a7e7 --- /dev/null +++ b/plugins/endpoints/api/reportipscript.py @@ -0,0 +1,159 @@ +"""Seed the Report IP desktop-shortcut script into the ShopDB script library. + +run_script only delivers bodies that live in the script library, pinned by +SHA-256. The shortcut-equivalent tool therefore has to exist as a real Script +row with a stored body before Corporate Tools can push it. + +The template is source-controlled at +plugins/endpoints/api/templates/report-ip-shortcut.ps1 and seeded from there +rather than typed into the library by hand, so the reviewed script and the one +endpoints actually run are the same artifact. Editing the row's body in the UI +is pointless - the next seed overwrites it. + +That last point is load-bearing here in a way it is not for an ordinary script. +REPORT_IP_SCRIPT_NAME is named in CORPTOOLS_PUSHABLE_SCRIPTS, so holders of +endpoints.corptools may push this one script WITHOUT holding +endpoints.runscript. Granting on a name is only safe because the name maps to a +body that comes from source control and is digest-pinned on delivery: the name +cannot be made to mean something else. If this row were hand-authorable, the +allowance would become "corptools may run arbitrary code", which is exactly the +bundling that endpoints.runscript was split out to prevent. + +It lives under the plugin, not under agent/, because the Docker image copies +shopdb/, plugins/, migrations/ and scripts/ but NOT agent/. A template read +from agent/ resolves fine on a developer checkout and then fails in the +deployed container, which is the worst place to discover it. + +Unlike the agent self-upgrade seeder there is no placeholder to render: the +script resolves its own target on the endpoint (shortcut first, installed path +as fallback), so the body is shipped verbatim. +""" + +import hashlib +import os + +from shopdb.api import Script, db, script_body_location + +# Reserved script name. Unique in the library, which makes seeding idempotent +# and keeps a hand-made script from colliding with the managed one. This exact +# string is what the enqueue route matches when deciding whether +# endpoints.corptools is sufficient, so it must stay in lockstep with +# CORPTOOLS_PUSHABLE_SCRIPTS in plugins/endpoints/commands.py - the test +# test_reserved_report_ip_name_matches_the_commands_constant pins them together. +REPORT_IP_SCRIPT_NAME = 'Report IP - Desktop Shortcut' + +_REPORT_IP_DESCRIPTION = ( + 'Managed: reproduces double-clicking the Report IP icon on the Public ' + 'Desktop, re-reporting this computer\'s IP address. Runs in the signed-in ' + 'user\'s session and fails closed if nobody is logged on. ' + 'Seeded from source control - edits here are overwritten.' +) + +_TEMPLATE_FILENAME = 'report-ip-shortcut.ps1' + + +def template_path(): + return os.path.join(os.path.dirname(os.path.abspath(__file__)), + 'templates', _TEMPLATE_FILENAME) + + +def _script_dir(): + """Where script bodies live. Resolved through the contract surface helper + so this cannot drift from the directory the download route serves from.""" + directory, _prefix = script_body_location() + return directory + + +def render_template(): + """Return the script body verbatim. + + No substitution: the endpoint resolves its own target at run time. Kept as + a function so the seeder reads the file through one place and a future + placeholder has an obvious home. + """ + with open(template_path(), 'r', encoding='utf-8') as handle: + return handle.read() + + +def seed_report_ip_script(): + """Create or refresh the managed Report IP script row and its body. + + Returns a dict describing the seeded script. Idempotent: unchanged input + rewrites nothing and leaves the digest alone. + """ + body = render_template().encode('utf-8') + digest = hashlib.sha256(body).hexdigest() + + script = Script.query.filter_by(scriptname=REPORT_IP_SCRIPT_NAME).first() + created = script is None + if created: + script = Script( + scriptname=REPORT_IP_SCRIPT_NAME, + scriptdescription=_REPORT_IP_DESCRIPTION, + # USER context, and this is not a preference - SYSTEM cannot do + # this job at all. Measured on a live endpoint: launched as SYSTEM + # the tool immediately re-entered the interactive session, leaving + # the work in a different process, session and account + # (pid 7240, session 1, owner ...\ShopFloor) while the SYSTEM-side + # launcher exited 0 straight away. The command then reported + # "completed (system context)" having observed nothing - a success + # that means "handed off", not "worked". + # + # This is the same session-0 boundary EndpointCollector already + # documents for mapped drives: session 0 has no access to a user's + # drive table, HKCU or profile, so a SYSTEM process cannot see - + # let alone report on - what happens in the user's session. The + # agent's user path crosses that boundary properly via + # CreateProcessAsUser and returns the child's REAL exit code. + # + # The tradeoff accepted here: user context fails closed when nobody + # is signed in. That is the correct failure - an honest "no one is + # logged on" beats a green result that observed nothing. + runcontext='user', + # Capture successful output too. The whole point of this tool is + # the response line it prints, and an operator who pushed it needs + # to read that line to know the IP was actually reported - a bare + # "succeeded" answers a different question. + captureoutput=True, + ) + db.session.add(script) + db.session.flush() + + # Body filename follows the library convention (script-.ps1) so the + # collector download route serves it like any other script. + filename = 'script-{}.ps1'.format(script.scriptid) + path = os.path.join(_script_dir(), filename) + + unchanged = (script.filename == filename and script.sha256 == digest + and os.path.isfile(path)) + if not unchanged: + os.makedirs(_script_dir(), exist_ok=True) + # Temp-then-replace so an interrupted write cannot leave a truncated + # body at the path the row's digest already claims to describe. + temppath = path + '.tmp' + with open(temppath, 'wb') as handle: + handle.write(body) + os.replace(temppath, path) + script.filename = filename + script.sha256 = digest + + # Re-assert the managed fields on every seed. A row edited in the UI drifts + # back to the shipped definition here, which is what "edits are + # overwritten" has to mean for the corptools allowance to stay honest: an + # operator must not be able to flip this to runcontext=system, or + # deactivate it, and leave the name still pushable. The context is part of + # what makes the result trustworthy, so it is re-asserted like the rest. + script.scriptdescription = _REPORT_IP_DESCRIPTION + script.runcontext = 'user' + script.captureoutput = True + script.isactive = True + db.session.commit() + + return { + 'scriptid': script.scriptid, + 'scriptname': script.scriptname, + 'created': created, + 'updated': not unchanged, + 'sha256': digest, + 'bytes': len(body), + } diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index 1cb5ebcd..38eee429 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -31,7 +31,7 @@ from ..models import ( EndpointState, EndpointEvent, EndpointCommand, EndpointApplication, EndpointHealthState, OperatorReport, EndpointLogCollection, ) -from ..commands import COMMAND_ALLOWLIST, is_allowed_action, ALLOWED_ACTIONS_FOR, row_actions_for, requires_target, action_targetkind, elevated_permission_for, minimum_agent_version_for, action_minimum_versions, parse_agent_version, agent_meets_minimum, PLACE_POLICY_HOLD, ACTIVE_POLICY_HOLDS, HOLDABLE_ACTIONS_FOR, RELEASE_POLICY_HOLD, ACTIVE_LOCK_FOR, PLACE_ENDPOINT_LOCK, RELEASE_ENDPOINT_LOCK +from ..commands import COMMAND_ALLOWLIST, is_allowed_action, ALLOWED_ACTIONS_FOR, row_actions_for, requires_target, action_targetkind, elevated_permission_for, corptools_may_push_script, minimum_agent_version_for, action_minimum_versions, parse_agent_version, agent_meets_minimum, PLACE_POLICY_HOLD, ACTIVE_POLICY_HOLDS, HOLDABLE_ACTIONS_FOR, RELEASE_POLICY_HOLD, ACTIVE_LOCK_FOR, PLACE_ENDPOINT_LOCK, RELEASE_ENDPOINT_LOCK from . import agentstore from . import agentauth from ..eventcatalog import CRITICAL_EVENT_IDS, event_severity, event_title @@ -2992,10 +2992,28 @@ def enqueue_endpoint_command(asset_id: int): # require_permission exists to prevent, reintroduced by checking by hand. elevated = elevated_permission_for(action) if elevated and not caller_holds(elevated): - return error_response( - ErrorCodes.FORBIDDEN, - f'Action {action!r} requires the {elevated} permission', - http_code=403) + # One narrow exception: a SHIPPED script whose reviewed body is + # operationally equivalent to something the person could already do + # unaided at the machine (Report IP is an icon on every Public Desktop). + # Those are pushable with endpoints.corptools alone. + # + # The allowance is keyed on the script's STORED name, read off a row we + # resolve here - never on the caller-supplied target. Matching what the + # requester typed would let anyone name their way past the grant, and + # relaxing the run_script ACTION instead would hand every corptools + # holder the whole library (the script id rides in the request body), + # rebuilding the endpoints.deploy bundle the permission split removed. + allowed_by_shipped_script = False + if action == 'run_script' and caller_holds('endpoints.corptools'): + resolved, _resolve_error = _resolve_run_script(target) + if resolved is not None: + allowed_by_shipped_script = corptools_may_push_script( + resolved.get('scriptname')) + if not allowed_by_shipped_script: + return error_response( + ErrorCodes.FORBIDDEN, + f'Action {action!r} requires the {elevated} permission', + http_code=403) # Break-glass: an admin may push past a Configuration Policy denial, but # only by asking for it explicitly and saying why. The override is scoped to diff --git a/plugins/endpoints/api/templates/report-ip-shortcut.ps1 b/plugins/endpoints/api/templates/report-ip-shortcut.ps1 new file mode 100644 index 00000000..fdcd4376 --- /dev/null +++ b/plugins/endpoints/api/templates/report-ip-shortcut.ps1 @@ -0,0 +1,74 @@ +# Report IP - reproduces double-clicking the "Report IP" desktop icon. +# +# Seeded from source control by plugins/endpoints/api/reportipscript.py. Edits +# made to the library row are overwritten on the next seed: the corptools +# allowance grants this script BY NAME, so the name has to keep meaning this +# reviewed body. +# +# RUNS IN THE USER SESSION, and that is a hard requirement rather than a +# preference. Measured on a live endpoint, launching this tool as SYSTEM has it +# immediately re-enter the interactive session - the work ended up in a separate +# process under the signed-in account (session 1) while the SYSTEM-side launcher +# exited 0 within a second, having observed nothing. Every attempt to capture +# output or a verdict from the SYSTEM side therefore reported success for work +# it never saw. +# +# This is the same session-0 boundary EndpointCollector documents for mapped +# drives: a SYSTEM process has no view of a user's drive table, HKCU or profile. +# The agent's user-context path crosses it properly (CreateProcessAsUser into +# the active console session) and returns the child's real exit code, so the +# verdict below is trustworthy in a way the SYSTEM path could not be. +# +# Consequently this script stays deliberately thin: the agent owns launching, +# waiting and output capture. Re-implementing any of that here is what produced +# the false results. + +$ErrorActionPreference = 'Stop' + +# Resolved from the shortcut when present, so the arguments stay whatever the +# icon on the desktop actually uses. The fallback matches the installed tool. +$ShortcutPath = 'C:\Users\Public\Desktop\Report IP.lnk' +$Exe = 'C:\ProgramData\ReportIP\GE_ReportIP_3_v1.EXE' +$ExeArgs = @('/ForceUpdate=True', '/S') + +# Working directory comes from the vendor's own scheduled task definition +# (\ReportIP), which runs the tool from its install folder. +$WorkDir = 'C:\ProgramData\ReportIP' + +if (Test-Path -LiteralPath $ShortcutPath) { + try { + $link = (New-Object -ComObject WScript.Shell).CreateShortcut($ShortcutPath) + if ($link.TargetPath) { + $Exe = $link.TargetPath + if ($link.Arguments) { $ExeArgs = @($link.Arguments) } + if ($link.WorkingDirectory) { $WorkDir = $link.WorkingDirectory } + } + } catch { + Write-Output "could not read the shortcut, using the installed path: $($_.Exception.Message)" + } +} + +if (-not (Test-Path -LiteralPath $Exe)) { + Write-Output "executable not found: $Exe" + exit 2 +} + +if (-not (Test-Path -LiteralPath $WorkDir)) { $WorkDir = Split-Path -Parent $Exe } + +# Identity is logged because it is the thing most likely to be wrong: if this +# ever reports SYSTEM, the run is meaningless for the reasons in the header. +Write-Output "running as: $([Security.Principal.WindowsIdentity]::GetCurrent().Name)" +Write-Output "session: $((Get-Process -Id $PID).SessionId)" +Write-Output "launching: $Exe $($ExeArgs -join ' ')" + +# -Wait, and the tool's own exit code is the verdict. No success-string scan: +# the tool reports its outcome through its own channel, and inferring success +# from console text was what produced both false failures and false successes +# in earlier revisions. +$process = Start-Process -FilePath $Exe ` + -ArgumentList $ExeArgs ` + -WorkingDirectory $WorkDir ` + -PassThru -Wait + +Write-Output "exit code: $($process.ExitCode)" +exit $process.ExitCode diff --git a/plugins/endpoints/commands.py b/plugins/endpoints/commands.py index eb3a81e1..a0793ae5 100644 --- a/plugins/endpoints/commands.py +++ b/plugins/endpoints/commands.py @@ -238,6 +238,44 @@ def elevated_permission_for(action): return ELEVATED_ACTION_PERMISSIONS.get(action) +# Shipped scripts that endpoints.corptools may push without holding +# endpoints.runscript. +# +# This is an allowance on ONE NAMED SCRIPT, never on the run_script action. The +# distinction is the whole point: relaxing the action would let any corptools +# holder push ANY script in the library - including the agent self-upgrade, and +# anything an operator uploads later - because the script id rides in the +# caller-controlled request body. That would silently rebuild the endpoints.deploy +# bundle the permission split took apart, which is the one outcome this must not +# produce. +# +# Granting on a name is only defensible because these names are MANAGED: each is +# seeded from a source-controlled template, pinned by SHA-256 on delivery, and +# re-asserted on every seed (see reportipscript.py). The name therefore cannot be +# made to mean something else by editing a row. An entry here is a statement that +# the reviewed body is operationally equivalent to something the user could +# already do unaided - Report IP is an icon on every Public Desktop in the +# company, so pushing it grants no capability the person did not already have. +# +# Do NOT add a name here because a denial is inconvenient. The test for +# membership is that a non-privileged employee can already trigger the same work +# by hand on the machine in front of them. Anything else needs endpoints.runscript. +CORPTOOLS_PUSHABLE_SCRIPTS = frozenset({ + 'Report IP - Desktop Shortcut', +}) + + +def corptools_may_push_script(scriptname): + """True when endpoints.corptools alone is enough to push this script. + + Matched on the exact managed name. Callers must resolve the target to a real + Script row FIRST and pass the stored scriptname, never a caller-supplied + string: matching what the requester typed would let anyone name their way + past the grant. + """ + return scriptname in CORPTOOLS_PUSHABLE_SCRIPTS + + def minimum_agent_version_for(action): """The lowest agent version that implements an action, or None if any will do.""" return COMMAND_ALLOWLIST.get(action, {}).get('minagentversion') @@ -449,6 +487,19 @@ POLICY_EXEMPT_ACTIONS = frozenset({ # still requires endpoints.updateapp via ELEVATED_ACTION_PERMISSIONS, and # the agent still refuses UNC, traversal, directories and non-text files. 'collect_log', + # Script push is gated by WHO the caller is, not by what a PC type exposes. + # Configuration Policy describes machine shape; it has no view on whether a + # given person may execute code, so a policy that never mentions run_script + # must not be read as withdrawing it. Leaving it policy-gated forced sites + # to keep an all-PCs "allow run_script" policy row alive purely to avoid + # losing the action everywhere - a rule that encoded no real intent and + # whose deletion silently disarmed Script Push on every policied endpoint. + # Authority lives on the caller axis instead: run_script maps to + # endpoints.runscript in ELEVATED_ACTION_PERMISSIONS, checked with + # caller_holds at the enqueue route BEFORE policy is consulted, so a scoped + # personal token cannot inherit it from an admin owner. This exemption + # widens WHERE the action may run, never WHO may run it. + 'run_script', }) diff --git a/shopdb/cli/__init__.py b/shopdb/cli/__init__.py index 47d749d3..b3058406 100644 --- a/shopdb/cli/__init__.py +++ b/shopdb/cli/__init__.py @@ -859,6 +859,38 @@ def seed_settings(): click.echo(click.style(f"{created} default settings created.", fg='green')) +@seed_cli.command('shipped-scripts') +@with_appcontext +def seed_shipped_scripts(): + """Seed managed scripts that ship with the build into the script library. + + Idempotent: an unchanged template rewrites nothing and leaves the stored + digest alone. Safe to run on every deploy, which is the point - a managed + script row edited in the UI drifts back to the shipped definition here. + + Guarded: a lean build without the endpoints plugin has no shipped scripts + to seed, so the command reports that and exits rather than crashing the + seed chain it runs in. + """ + try: + from plugins.endpoints.api import reportipscript + except ImportError: + click.echo('Endpoints plugin is not installed - no shipped scripts to seed.') + return + + seeded = reportipscript.seed_report_ip_script() + if seeded['created']: + state = 'created' + elif seeded['updated']: + state = 'updated' + else: + state = 'already current' + click.echo(click.style( + f"{seeded['scriptname']}: {state} " + f"(scriptid={seeded['scriptid']}, {seeded['bytes']} bytes)", + fg='green')) + + # Demo assets carry this assetnumber prefix so a re-run skips what it made and # an operator can bulk-delete them without touching imported/real rows. DEMO_PREFIX = 'DEMO-' diff --git a/tests/test_plugins/test_endpoints_plugin.py b/tests/test_plugins/test_endpoints_plugin.py index 775d6bdf..e3b80e2e 100644 --- a/tests/test_plugins/test_endpoints_plugin.py +++ b/tests/test_plugins/test_endpoints_plugin.py @@ -12,7 +12,8 @@ from plugins.endpoints.eventcatalog import EVENT_CATALOG, CRITICAL_EVENT_IDS from plugins.endpoints.commands import ( COMMAND_ALLOWLIST, is_allowed_action, elevated_permission_for, requires_target, minimum_agent_version_for, agent_meets_minimum, - parse_agent_version, + parse_agent_version, corptools_may_push_script, + CORPTOOLS_PUSHABLE_SCRIPTS, ) @@ -168,6 +169,55 @@ def test_endpoints_declares_the_restart_permission(): assert 'endpoints.command' in permissions +def test_run_script_still_requires_the_runscript_permission(): + """The corptools allowance is an exception for one named script, not a + relaxation of run_script. The action itself must keep demanding + endpoints.runscript, otherwise a corptools holder could push anything in + the library.""" + assert elevated_permission_for('run_script') == 'endpoints.runscript' + + +def test_corptools_may_push_only_the_shipped_report_ip_script(): + """Corporate Tools can push the managed Report IP script by name, and + nothing else. A site-authored script that happens to be selected from the + same tab gets no allowance.""" + assert corptools_may_push_script('Report IP - Desktop Shortcut') is True + assert corptools_may_push_script('Uninstall Everything') is False + assert corptools_may_push_script(None) is False + assert corptools_may_push_script('') is False + + +def test_corptools_pushable_scripts_stays_a_single_managed_entry(): + """Guards the blast radius of the exception. Growing this set widens what + endpoints.corptools can deliver without endpoints.runscript, so adding an + entry should be a deliberate change that fails this test first.""" + assert CORPTOOLS_PUSHABLE_SCRIPTS == {'Report IP - Desktop Shortcut'} + + +def test_corptools_allowance_name_matches_the_seeder(): + """The allowance matches on the stored script name, so the seeder and the + allowlist must agree exactly. If either string is edited alone the + Corporate Tools button silently 403s.""" + from plugins.endpoints.api.reportipscript import REPORT_IP_SCRIPT_NAME + assert REPORT_IP_SCRIPT_NAME in CORPTOOLS_PUSHABLE_SCRIPTS + + +def test_report_ip_script_body_does_not_relaunch_itself(): + """Pins the session-0 lesson. Launched as SYSTEM this tool re-enters the + interactive session, so the launcher observes nothing and exits 0 - a + success that means 'handed off', not 'worked'. The body must therefore + stay thin and let the agent's user-context path own launching and the exit + code; re-introducing a cmd.exe wrapper or a success-string scan is what + produced the false results.""" + from plugins.endpoints.api.reportipscript import template_path + with open(template_path(), 'r', encoding='utf-8') as handle: + body = handle.read() + assert 'ComSpec' not in body + assert 'completed successfully' not in body + assert '-Wait' in body + assert 'exit $process.ExitCode' in body + + def test_collect_now_is_allowlisted_targetless_and_routine(): """collect_now forces an off-cadence telemetry report. It is allowlisted so the enqueue route accepts it, carries no wire target (fixed action), and @@ -217,6 +267,62 @@ def test_collect_log_survives_a_policy_that_never_mentions_it(): assert elevated_permission_for('collect_log') == 'endpoints.updateapp' +def test_run_script_survives_a_policy_that_never_mentions_it(monkeypatch): + """Script Push is gated by caller permission, not by machine shape. + + Before this exemption the action stayed reachable only because a site kept + an all-PCs "allow run_script" policy row alive. That row expressed no real + configuration intent, and deleting it silently removed Script Push from + every policied endpoint at once - a failure that reads as authorization + ("not permitted by policy") while the caller's grant was never in doubt. + Authority belongs on the caller axis, so a policy that is silent about + run_script must not be read as withdrawing it. + """ + import plugins.endpoints.commands as commands + + class _Effective: + def allowed_actions(self): + # A real, unrelated policy: it shapes services and says nothing + # whatsoever about running scripts. + return {'service_start', 'service_stop'} + + monkeypatch.setattr( + 'plugins.computers.services.policyresolver.effective_policy_for', + lambda assetid: _Effective(), + ) + + allowed = commands.ALLOWED_ACTIONS_FOR(assetid=1) + + assert 'run_script' in allowed + # Exemption from POLICY is not exemption from PERMISSION: the grant must + # survive the exemption rather than be loosened by it. The enqueue route + # checks this with caller_holds BEFORE consulting policy, so a scoped + # personal token cannot inherit it from an admin owner. + assert elevated_permission_for('run_script') == 'endpoints.runscript' + + +def test_policy_exemption_never_widens_past_the_code_allowlist(monkeypatch): + """Exempt actions are re-added, but the base allowlist still bounds them. + + The exemption set is unioned in after the policy intersection, so a bug + there could hand back an action the command layer does not implement. + """ + import plugins.endpoints.commands as commands + + class _Effective: + def allowed_actions(self): + return {'service_start'} + + monkeypatch.setattr( + 'plugins.computers.services.policyresolver.effective_policy_for', + lambda assetid: _Effective(), + ) + + allowed = commands.ALLOWED_ACTIONS_FOR(assetid=1) + + assert allowed <= set(commands.COMMAND_ALLOWLIST.keys()) + + def test_collect_log_declares_the_agent_version_that_implements_it(): """collect_log names its minimum agent version.