ge-aerospace/shopdb-flask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Close the review findings on the policy routes, installer and import

Browse Source 
Applied on top of af13337 so nothing already on wilm is rewritten.

AUTHORIZATION. The six new policy GETs were @jwt_required(optional=True) with
no permission at all, so an anonymous caller could list configuration and
business policies - gated only by a feature flag that 404s while off. They now
require computers.view. The six policy write routes stacked @jwt_required()
above @require_permission, the same pattern removed from nine endpoints routes
in this very push: the outer decorator answers an anonymous request 401 before
require_permission can consult the guest role, so a grant to guests silently
does nothing. Removed, and the now-unused import with them.

tests/test_core/test_public_read_surface.py failed at af13337 with seven ungated
GETs. It passes now. The seventh, endpoints.claim_next_command, is genuinely
agent-facing - authenticated in the handler by the endpoints collector key,
because a bay has no user session - so it joins PUBLIC_ENDPOINTS with its reason
written down, exactly as report_command_result already does in test_authz.py.
Note which half is public: claiming a queued command is the agent's business,
while ENQUEUEING one is the real authority and stays gated on endpoints.command.

INSTALLER. -BaseUrl and -CollectorKey are now mandatory with no defaults, and
https is enforced unless -AllowInsecureHttp is passed for a local test box. The
defaults were a real hostname (ADR-015) and a live collector key: a key in
source is a key in git history, and that key is also the command channel's kill
switch. Removing it here does not undo the disclosure - it must be rotated on
the server, and purged from af13337 - but it stops the next clone carrying it.

IMPORT. A committed import wrote thousands of rows and left no audit trail; it
now writes one AuditLog row inside the same transaction, so it cannot survive a
rollback or go missing from a commit. The computer-type dropdown asked for
per_page: 200, which the API ignores in favour of perpage and clamps to 100
anyway - it uses a new computersApi.types.listAll(), the pattern already applied
to the other selectors. And the comment claiming the template example rows are
"emitted as commented rows the engine skips" was the opposite of the truth: they
import as live data, the emitted instructions say so, and a test depends on it.

Not fixed here, deliberately: rotating the leaked key, purging it from history,
and the endpoint agent's own hardening (per-host credentials, a protected
process denylist so stop cannot kill security tooling, signing before the
PowerShell runner is enabled, registry ACLs, and the 25-second long poll that
will pin a worker thread per bay).
This commit is contained in:
cproudlock
2026-08-28 15:52:15 -04:00
parent af133372b4
commit 75627d4398
7 changed files with 67 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
agent/installer/install-shopdbagent.ps1
View File

@@ -13,13 +13,25 @@
[CmdletBinding()]
param(
# ShopDB server root URL for this instance. Defaulted to the test instance;
# pass -BaseUrl "http://<host>:<port>" to override.
[string] $BaseUrl = "http://H1HWNL84.device.geaerospace.net:5001",
# ShopDB server root URL for this instance. REQUIRED - there is no default,
# because a default here means one site's hostname shipped to every site
# (ADR-015) and, worse, a working install pointed at somebody else's server.
#
# https is required. The collector key travels in the X-API-Key header and
# every telemetry payload follows it, so plain http puts both on the wire in
# clear. Pass -AllowInsecureHttp only for a local test instance.
[Parameter(Mandatory = $true)]
[string] $BaseUrl,
# Endpoints-only collector key: this instance's .env COLLECTOR_API_KEY_ENDPOINTS.
# REQUIRED and never defaulted - a key in source is a key in git history, and
# this one is also the command channel's kill switch.
[Parameter(Mandatory = $true)]
[string] $CollectorKey,
# Permit an http:// BaseUrl. For a local test instance only.
[switch] $AllowInsecureHttp,
# Endpoints-only collector key (this instance's .env
# COLLECTOR_API_KEY_ENDPOINTS).
[string] $CollectorKey = "l755ddRw1iSeOcSTh9N9wXM4gGT7N6WIVN9crOY6uZU",
[string] $InstallDir = "C:\Program Files\ShopDB\EndpointAgent",
[string] $ServiceName = "ShopDbEndpointAgent",
@@ -39,6 +51,12 @@ param(
[switch] $NoDefenderExclusion
)
# Refuse cleartext unless it was asked for explicitly. The key is a bearer
# credential for the whole fleet's telemetry and command channel.
if ($BaseUrl -notmatch '^https://' -and -not $AllowInsecureHttp) {
throw "BaseUrl must be https:// (got '$BaseUrl'). Pass -AllowInsecureHttp for a local test instance."
}
$ErrorActionPreference = "Stop"
$sourceDir = Split-Path -Parent $MyInvocation.MyCommand.Path

5
frontend/src/api/index.js
View File

@@ -208,6 +208,11 @@ export const computersApi = {
list(params = {}) {
return api.get('/computers/types', { params })
},
// Every type, for a dropdown. perpage is capped at 100 server-side, so
// asking for 200 returns 100 and silently truncates the list.
listAll(params = {}) {
return fetchAllPages('/computers/types', params)
},
get(id) {
return api.get(`/computers/types/${id}`)
},

4
frontend/src/views/settings/ImportSiteAssets.vue
View File

@@ -672,8 +672,8 @@ async function addComputerType() {
async function loadComputerTypes() {
try {
const response = await computersApi.types.list({ per_page: 200 })
computerTypes.value = response.data?.data || response.data || []
// listAll returns the rows themselves, already paged through.
computerTypes.value = await computersApi.types.listAll()
} catch {
computerTypes.value = []
}

19
plugins/computers/api/policy_routes.py
View File

@@ -12,7 +12,6 @@ allowlist, so a bad tree or an unknown action never reaches the database.
"""
from flask import request
from flask_jwt_extended import jwt_required
from shopdb.api import (db, AuditLog, success_response, error_response,
ErrorCodes, require_permission)
@@ -63,7 +62,7 @@ def _businesspolicy_dict(policy):
# =============================================================================
@computers_bp.route('/policies/configuration', methods=['GET'])
@jwt_required(optional=True)
@require_permission('computers.view')
def list_configuration_policies():
gate = _feature_gate()
if gate:
@@ -73,7 +72,7 @@ def list_configuration_policies():
@computers_bp.route('/policies/configuration/<int:policy_id>', methods=['GET'])
@jwt_required(optional=True)
@require_permission('computers.view')
def get_configuration_policy(policy_id: int):
gate = _feature_gate()
if gate:
@@ -113,7 +112,6 @@ def _apply_actions(policy, actions):
@computers_bp.route('/policies/configuration', methods=['POST'])
@jwt_required()
@require_permission('computers.create')
def create_configuration_policy():
gate = _feature_gate()
@@ -155,7 +153,6 @@ def create_configuration_policy():
@computers_bp.route('/policies/configuration/<int:policy_id>', methods=['PUT'])
@jwt_required()
@require_permission('computers.edit')
def update_configuration_policy(policy_id: int):
gate = _feature_gate()
@@ -204,7 +201,6 @@ def update_configuration_policy(policy_id: int):
@computers_bp.route('/policies/configuration/<int:policy_id>', methods=['DELETE'])
@jwt_required()
@require_permission('computers.delete')
def delete_configuration_policy(policy_id: int):
gate = _feature_gate()
@@ -236,7 +232,7 @@ def delete_configuration_policy(policy_id: int):
# =============================================================================
@computers_bp.route('/policies/business', methods=['GET'])
@jwt_required(optional=True)
@require_permission('computers.view')
def list_business_policies():
gate = _feature_gate()
if gate:
@@ -248,7 +244,7 @@ def list_business_policies():
@computers_bp.route('/policies/business/<int:policy_id>', methods=['GET'])
@jwt_required(optional=True)
@require_permission('computers.view')
def get_business_policy(policy_id: int):
gate = _feature_gate()
if gate:
@@ -284,7 +280,6 @@ def _apply_items(policy, items):
@computers_bp.route('/policies/business', methods=['POST'])
@jwt_required()
@require_permission('computers.create')
def create_business_policy():
gate = _feature_gate()
@@ -315,7 +310,6 @@ def create_business_policy():
@computers_bp.route('/policies/business/<int:policy_id>', methods=['PUT'])
@jwt_required()
@require_permission('computers.edit')
def update_business_policy(policy_id: int):
gate = _feature_gate()
@@ -355,7 +349,6 @@ def update_business_policy(policy_id: int):
@computers_bp.route('/policies/business/<int:policy_id>', methods=['DELETE'])
@jwt_required()
@require_permission('computers.delete')
def delete_business_policy(policy_id: int):
gate = _feature_gate()
@@ -380,7 +373,7 @@ def delete_business_policy(policy_id: int):
# =============================================================================
@computers_bp.route('/policies/actions', methods=['GET'])
@jwt_required(optional=True)
@require_permission('computers.view')
def list_policy_actions():
"""The allowlisted action names + labels a policy may reference."""
gate = _feature_gate()
@@ -395,7 +388,7 @@ def list_policy_actions():
@computers_bp.route('/policies/effective/<int:asset_id>', methods=['GET'])
@jwt_required(optional=True)
@require_permission('computers.view')
def get_effective_policy(asset_id: int):
"""Preview the resolved effective policy for one endpoint (debug/UI)."""
gate = _feature_gate()

14
shopdb/core/api/settings.py
View File

@@ -512,6 +512,20 @@ def import_asset_csvs():
# even when commit was asked for.
db.session.rollback()
elif commit:
# One audit row for the whole run, written INSIDE the same
# transaction so it cannot survive a rollback or go missing from a
# commit. Every other write in this app is auditable; a bulk import
# that creates thousands of rows was the one path where nobody could
# answer "who loaded this, and when".
AuditLog.log(
'imported', 'SiteAssets',
entityname=', '.join(f['table'] for f in files_report) or 'none',
changes={
'files': [f['table'] for f in files_report],
'created': sum(f['created'] for f in files_report),
'updated': sum(f['updated'] for f in files_report),
'reconciled': reconciled_count,
})
db.session.commit()
applied = True
else:

9
shopdb/core/services/csvimport.py
View File

@@ -78,9 +78,12 @@ IMPORTABLE = OrderedDict([
# site can download the set, see the pattern, and keep it as a starter workbook:
# every name referenced by a later file (Bay 3, Dell, Computer, ...) is defined
# in an earlier one, so the whole set imports cleanly as a demo before a single
# real row is typed. Emitted as commented rows the engine skips, so a freshly
# downloaded template never writes example data by accident. Values avoid commas
# on purpose - the generator writes them without CSV quoting.
# real row is typed. They are emitted as LIVE ROWS, not comments: importing a
# freshly downloaded template writes PC-001, CNC-014, Dell and Bay 3, and
# updates any existing In Use / Retired status row it matches on. The emitted
# instructions say so, and test_template_example_set_imports_cleanly depends on
# it. Delete the example rows before importing real data. Values avoid commas on
# purpose - the generator writes them without CSV quoting.
EXAMPLE_ROWS = OrderedDict([
('assetstatuses', [
{'status': 'In Use', 'description': 'Actively deployed', 'color': '#22c55e', 'isactive': '1'},

10
tests/test_core/test_public_read_surface.py
View File

@@ -73,6 +73,16 @@ PUBLIC_ENDPOINTS = {
# The unguessable token IS the authorisation, and the recipient of a secure
# send has no account here.
'tools.peek_secret',
# The endpoint agent long-polls for its next command. Authenticated inside
# the handler by the endpoints collector API key - the same shape as the
# collector blueprint - because a bay has no user session. The credential is
# also the channel's kill switch. Note which half is public: CLAIMING a
# queued command is agent-facing, while ENQUEUEING one is the actual
# authority and stays gated on endpoints.command, audited with the
# requesting user. Same reasoning as endpoints.report_command_result in
# tests/test_core/test_authz.py.
'endpoints.claim_next_command',
}
# Reads that need a LOGIN but no named permission, because the handler decides