Gate the script library on the grant that pushes scripts
The script library was gated on applications.*, so anyone holding the software-delivery grant could rewrite the bytes of a script that a separately trusted operator later pushed to a bay. The push was gated on endpoints.runscript; the CONTENT of what got pushed was not. An installer is a vendor artefact, but a script here is arbitrary PowerShell this site wrote that the agent runs as SYSTEM, so the write and the push must require the same trust. All six script routes now require endpoints.runscript, including the body download - reading the bytes is not a lesser act than listing them. Core names the permission as a string rather than importing it from the endpoints plugin so the module stays importable with the plugin absent. This tightens access and there is deliberately no backfill: the operator scope reaches applications.create/edit, so operator-scope holders lose the library. Granting them endpoints.runscript on upgrade would undo the change and hand every operator remote SYSTEM code execution, since the same grant gates the push. A test pins that no prebuilt scope confers it. Adds the System Scripts settings page and card. The route requires the permission rather than admin, because the grant is meant for non-admin operators and the sibling admin gate would lock them out of the library the grant exists for. The settings icon map gains terminal and key; the agent-key card had been asking for key since it shipped and silently drawing a puzzle piece. The library had no test coverage before this.
This commit is contained in:
160
tests/test_core/test_script_library_gate.py
Normal file
160
tests/test_core/test_script_library_gate.py
Normal file
@@ -0,0 +1,160 @@
|
||||
"""The script library is gated on its own grant, not on applications.*.
|
||||
|
||||
A script here is not an application. An installer is a vendor artefact; a script
|
||||
is arbitrary PowerShell this site wrote that the agent runs as SYSTEM. While the
|
||||
library was gated on applications.*, anyone holding the software-delivery grant
|
||||
could rewrite the bytes of a script a trusted operator later pushed - the push
|
||||
was gated on endpoints.runscript, but the CONTENT of what got pushed was not.
|
||||
That is privilege escalation wearing a content-management hat.
|
||||
|
||||
These tests pin the gate itself. The library had NO coverage before this.
|
||||
"""
|
||||
import io
|
||||
|
||||
import pytest
|
||||
from werkzeug.security import generate_password_hash
|
||||
|
||||
from shopdb.core.models import Permission
|
||||
|
||||
|
||||
APPLICATIONS_GRANTS = [
|
||||
('applications.view', 'View applications', 'applications'),
|
||||
('applications.create', 'Create applications', 'applications'),
|
||||
('applications.edit', 'Edit applications', 'applications'),
|
||||
('applications.delete', 'Delete applications', 'applications'),
|
||||
('applications.download', 'Download installers', 'applications'),
|
||||
]
|
||||
SCRIPT_GRANT = ('endpoints.runscript', 'Run staged scripts', 'endpoints')
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def seeded(db):
|
||||
Permission.seed_entries(APPLICATIONS_GRANTS + [SCRIPT_GRANT])
|
||||
db.session.commit()
|
||||
|
||||
|
||||
def _user_with(client, db, username, grants):
|
||||
"""Create a user holding exactly the named permissions, return its headers.
|
||||
|
||||
Direct permissions, no role: a role named 'admin' would short-circuit the
|
||||
gate and prove nothing about the permission under test.
|
||||
"""
|
||||
from shopdb.core.models import Permission, User
|
||||
|
||||
user = User(
|
||||
username=username,
|
||||
email=f'{username}@test.local',
|
||||
passwordhash=generate_password_hash('testpass'),
|
||||
)
|
||||
for name in grants:
|
||||
permission = Permission.query.filter_by(name=name).one()
|
||||
user.directpermissions.append(permission)
|
||||
db.session.add(user)
|
||||
db.session.commit()
|
||||
|
||||
login = client.post('/api/auth/login',
|
||||
json={'username': username, 'password': 'testpass'})
|
||||
assert login.status_code == 200, login.get_json()
|
||||
token = login.get_json()['data']['access_token']
|
||||
return {'Authorization': f'Bearer {token}'}
|
||||
|
||||
|
||||
def test_the_applications_grant_does_not_open_the_script_library(
|
||||
client, db, seeded):
|
||||
"""The whole point: every applications.* grant, and still no access."""
|
||||
headers = _user_with(client, db, 'appsonly',
|
||||
[name for name, _d, _c in APPLICATIONS_GRANTS])
|
||||
|
||||
assert client.get('/api/scripts', headers=headers).status_code == 403
|
||||
assert client.post('/api/scripts', headers=headers,
|
||||
json={'scriptname': 'x'}).status_code == 403
|
||||
|
||||
|
||||
def test_the_runscript_grant_opens_the_script_library(client, db, seeded):
|
||||
headers = _user_with(client, db, 'pusher', ['endpoints.runscript'])
|
||||
|
||||
assert client.get('/api/scripts', headers=headers).status_code == 200
|
||||
created = client.post('/api/scripts', headers=headers,
|
||||
json={'scriptname': 'map-drives'})
|
||||
assert created.status_code == 201, created.get_json()
|
||||
|
||||
|
||||
def test_writing_a_script_body_needs_the_same_grant(client, db, seeded):
|
||||
"""Uploading the bytes is the dangerous half; it must not be cheaper to
|
||||
reach than the push that runs them."""
|
||||
author = _user_with(client, db, 'author', ['endpoints.runscript'])
|
||||
created = client.post('/api/scripts', headers=author,
|
||||
json={'scriptname': 'payload'})
|
||||
scriptid = created.get_json()['data']['scriptid']
|
||||
|
||||
appsuser = _user_with(client, db, 'appsuser2',
|
||||
[name for name, _d, _c in APPLICATIONS_GRANTS])
|
||||
body = (io.BytesIO(b"Write-Output 'hi'\r\n"), 'payload.ps1')
|
||||
denied = client.post(f'/api/scripts/{scriptid}/body', headers=appsuser,
|
||||
data={'file': body},
|
||||
content_type='multipart/form-data')
|
||||
assert denied.status_code == 403
|
||||
|
||||
|
||||
def test_reading_a_script_body_needs_the_same_grant(client, db, seeded):
|
||||
"""Reading is not a lesser act: the body is the SYSTEM-context PowerShell
|
||||
itself, so it must not be readable by a weaker grant than writing it."""
|
||||
appsuser = _user_with(client, db, 'appsuser3',
|
||||
[name for name, _d, _c in APPLICATIONS_GRANTS])
|
||||
response = client.get('/api/scripts/body/script-1.ps1', headers=appsuser)
|
||||
assert response.status_code == 403
|
||||
|
||||
|
||||
def test_upload_pins_the_digest_of_the_stored_bytes(client, db, seeded):
|
||||
"""The agent refuses a body whose sha256 does not match, so a wrong digest
|
||||
here is a script that can never run. Untested until now."""
|
||||
import hashlib
|
||||
|
||||
headers = _user_with(client, db, 'digest', ['endpoints.runscript'])
|
||||
created = client.post('/api/scripts', headers=headers,
|
||||
json={'scriptname': 'digest-check'})
|
||||
scriptid = created.get_json()['data']['scriptid']
|
||||
|
||||
payload = b"Write-Output 'pinned'\r\n"
|
||||
uploaded = client.post(
|
||||
f'/api/scripts/{scriptid}/body', headers=headers,
|
||||
data={'file': (io.BytesIO(payload), 'digest-check.ps1')},
|
||||
content_type='multipart/form-data')
|
||||
assert uploaded.status_code == 200, uploaded.get_json()
|
||||
|
||||
data = uploaded.get_json()['data']
|
||||
assert data['sha256'] == hashlib.sha256(payload).hexdigest()
|
||||
assert data['hasbody'] is True
|
||||
|
||||
|
||||
def test_a_duplicate_script_name_is_refused(client, db, seeded):
|
||||
"""The accidental-overwrite guard the library was specified to have."""
|
||||
headers = _user_with(client, db, 'dupes', ['endpoints.runscript'])
|
||||
first = client.post('/api/scripts', headers=headers,
|
||||
json={'scriptname': 'same-name'})
|
||||
assert first.status_code == 201
|
||||
second = client.post('/api/scripts', headers=headers,
|
||||
json={'scriptname': 'same-name'})
|
||||
assert second.status_code == 400
|
||||
assert 'already exists' in second.get_json()['data']['error']['message']
|
||||
|
||||
|
||||
def test_no_prebuilt_scope_confers_the_script_permission():
|
||||
"""Pins the decision NOT to backfill the tightened gate.
|
||||
|
||||
Moving the library onto endpoints.runscript takes it away from operator
|
||||
scope holders, who reach applications.create/edit. The tempting upgrade fix
|
||||
is to grant endpoints.runscript to whoever holds applications.edit - that
|
||||
would undo the whole change AND hand every operator remote SYSTEM code
|
||||
execution, because the same grant gates the push. If this test fails,
|
||||
someone has widened a scope; do not relax it.
|
||||
"""
|
||||
from shopdb.core.models.user import (
|
||||
SCOPE_NEVER_GRANT,
|
||||
scope_permission_names,
|
||||
)
|
||||
from shopdb.core.api.scripts import SCRIPT_PERMISSION
|
||||
|
||||
assert SCRIPT_PERMISSION in SCOPE_NEVER_GRANT
|
||||
for scope in ('reporter', 'technician', 'operator'):
|
||||
assert SCRIPT_PERMISSION not in scope_permission_names(scope)
|
||||
Reference in New Issue
Block a user