Review safe-polish: docs accuracy, dead imports, no-emoji, geenforce robustness
Some checks failed
CI / backend (push) Has been cancelled
CI / naming (push) Has been cancelled
CI / frontend (push) Has been cancelled

From the full multi-agent review (0 high, 7 medium, 17 low findings). Applies
the mechanical, low-risk items; design/policy findings left for a decision.

Docs accuracy: CLAUDE.md contract 0.10.0 -> 0.11.0 and both stale Alembic head
citations -> 7d24_customfield_searchable / 31 migrations; Dockerfile bundled-
plugin comment fixed (drop nonexistent "equipment", add machines +
measuringtools, count eleven).

Style/naming (LOCKED rules): remove a CSS-escaped pushpin emoji before location
search results (no-emoji policy); rename ManifestEditor shareRoot -> shareroot
(variable mirrors the API field verbatim).

Dead code: remove confirmed-unused imports across ~20 modules (require_role/
require_permission scaffold residue, stray db/Vendor/Model/current_user/Optional/
error_response); drop unused build_scope import + a stale GEENFORCE_API_KEY
docstring clause in geenforce. Migration files left untouched.

Correctness: geenforce ingest robustness - record_enforcement_report now 400s
on a non-dict counts / non-list results instead of 500; _apply_app_link ignores
a non-numeric appid per its docstring instead of 500. Regression tests added.

Backend query.get sweep finished: auth.py refresh -> db.session.get (last one).

910 backend tests pass; pyflakes clean; naming green; frontend build green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
cproudlock
2026-07-13 08:02:43 -04:00
parent 85a6ab8645
commit cd353b6432
31 changed files with 263 additions and 234 deletions

View File

@@ -43,9 +43,9 @@ Refactor phases 0-5 landed; phase 6 (multi-site distribution readiness) largely
### Active state ### Active state
- 808 tests passing, naming/style check green, Gitea Actions CI (backend + naming + frontend build) - 808 tests passing, naming/style check green, Gitea Actions CI (backend + naming + frontend build)
- `__contract_version__` at 0.10.0 (product `__version__` 0.7.0, tags v0.5.0/v0.6.0/v0.7.0 - distinct series, ADR-007) - `__contract_version__` at 0.11.0 (product `__version__` 0.7.0, tags v0.5.0/v0.6.0/v0.7.0 - distinct series, ADR-007)
- 11 bundled plugins all satisfy contract: computers, employees, knowledgebase, machines, measuringtools, network, notifications, printers, slides, usb, warranty - 11 bundled plugins all satisfy contract: computers, employees, knowledgebase, machines, measuringtools, network, notifications, printers, slides, usb, warranty
- Core Alembic chain: baseline `68b3947ae14f` -> head `7d23_user_mustchangepassword` (30 core migrations). Each plugin owns its own chain (ADR-008); deploy runs `flask db upgrade` then `flask plugin upgrade-all`. Reproducible + idempotent from empty. - Core Alembic chain: baseline `68b3947ae14f` -> head `7d24_customfield_searchable` (31 core migrations). Each plugin owns its own chain (ADR-008); deploy runs `flask db upgrade` then `flask plugin upgrade-all`. Reproducible + idempotent from empty.
- API is migration-complete: an admin PAT + docs/IMPORT-API.md let a script/LLM import the whole legacy DB (X-Import-Mode preserves timestamps). - API is migration-complete: an admin PAT + docs/IMPORT-API.md let a script/LLM import the whole legacy DB (X-Import-Mode preserves timestamps).
- Pre-1.0 framework; sister sites should pin tight `core_version` ranges until contract reaches 1.0 - Pre-1.0 framework; sister sites should pin tight `core_version` ranges until contract reaches 1.0
@@ -134,4 +134,4 @@ Each plugin must have:
- `migrations/FIX_LOCATIONONLY_EQUIPMENT_TYPES.md` - LocationOnly equipment type fix - `migrations/FIX_LOCATIONONLY_EQUIPMENT_TYPES.md` - LocationOnly equipment type fix
- `migrations/PRODUCTION_MIGRATION_GUIDE.md` - production import methods - `migrations/PRODUCTION_MIGRATION_GUIDE.md` - production import methods
- `migrations/rename_underscore_columns.sql` - one-time rename of snake_case columns to lowercase concatenated (per CONTRIBUTING.md) - `migrations/rename_underscore_columns.sql` - one-time rename of snake_case columns to lowercase concatenated (per CONTRIBUTING.md)
- `migrations/versions/` - the core Alembic chain (baseline `68b3947ae14f` -> head `7d16_directoryemployees`). Run `flask db upgrade` to apply. - `migrations/versions/` - the core Alembic chain (baseline `68b3947ae14f` -> head `7d24_customfield_searchable`). Run `flask db upgrade` to apply.

View File

@@ -2,8 +2,9 @@
# #
# One image, one site. Per ADR-004, each adopting facility runs its own # One image, one site. Per ADR-004, each adopting facility runs its own
# stack with its own DB, secrets, and enabled-plugin list. This image # stack with its own DB, secrets, and enabled-plugin list. This image
# bundles all ten core plugins (computers, employees, equipment, # bundles all eleven core plugins (computers, employees, knowledgebase,
# knowledgebase, network, notifications, printers, slides, usb, warranty); # machines, measuringtools, network, notifications, printers, slides, usb,
# warranty);
# install them at runtime with `flask plugin install <name>`. # install them at runtime with `flask plugin install <name>`.
# #
# The frontend is built in a first stage and its dist output is copied into # The frontend is built in a first stage and its dist output is copied into

View File

@@ -464,10 +464,6 @@ watch(results, () => {
color: var(--text-light); color: var(--text-light);
} }
.result-location::before {
content: '\1F4CD ';
}
.result-ticket { .result-ticket {
font-family: monospace; font-family: monospace;
font-size: 0.75rem; font-size: 0.75rem;

View File

@@ -36,7 +36,7 @@
<div class="setting-row full-width"> <div class="setting-row full-width">
<label> <label>
<span>On-share export root</span> <span>On-share export root</span>
<input v-model="shareRoot" placeholder="\\server\share\dt\shopfloor or /path" /> <input v-model="shareroot" placeholder="\\server\share\dt\shopfloor or /path" />
<small class="input-hint"> <small class="input-hint">
Used by Export to Share. During Milestone 1 the engine still reads Used by Export to Share. During Milestone 1 the engine still reads
these files; export is your push to the fleet. these files; export is your push to the fleet.
@@ -587,7 +587,7 @@ const selectedId = ref(null)
const detail = ref(null) const detail = ref(null)
const error = ref('') const error = ref('')
const notice = ref('') const notice = ref('')
const shareRoot = ref('') const shareroot = ref('')
// Phase-aware editing: the PREINSTALL runner implements only MSI/EXE types and // Phase-aware editing: the PREINSTALL runner implements only MSI/EXE types and
// Registry/File detection (other types/detections are silently skipped there), // Registry/File detection (other types/detections are silently skipped there),
@@ -637,11 +637,11 @@ async function loadScopes() {
} catch (e) { error.value = 'Failed to load PC types' } } catch (e) { error.value = 'Failed to load PC types' }
} }
async function loadConfig() { async function loadConfig() {
try { shareRoot.value = payload(await api.get('/geenforce/config')).shareroot } catch (e) { /* optional */ } try { shareroot.value = payload(await api.get('/geenforce/config')).shareroot } catch (e) { /* optional */ }
} }
async function saveConfig() { async function saveConfig() {
try { try {
await api.put('/geenforce/config', { shareroot: shareRoot.value }) await api.put('/geenforce/config', { shareroot: shareroot.value })
flash('Share root saved.') flash('Share root saved.')
} catch (e) { error.value = 'Failed to save share root' } } catch (e) { error.value = 'Failed to save share root' }
} }

View File

@@ -3,11 +3,11 @@
from flask import Blueprint, request from flask import Blueprint, request
from flask_jwt_extended import jwt_required from flask_jwt_extended import jwt_required
from shopdb.api import db, Asset, AssetType, OperatingSystem, Application, AppVersion, AuditLog, Communication, CommunicationType, Setting, success_response, error_response, paginated_response, ErrorCodes, get_pagination_params, paginate_query from shopdb.api import db, Asset, AssetType, OperatingSystem, Application, AuditLog, Communication, CommunicationType, Setting, success_response, error_response, paginated_response, ErrorCodes, get_pagination_params, paginate_query
from ..models import Computer, ComputerType, ComputerInstalledApp, AccessProtocol, ComputerAccess from ..models import Computer, ComputerType, ComputerInstalledApp, AccessProtocol, ComputerAccess
from shopdb.api import require_permission, require_role, apply_import_timestamps from shopdb.api import require_permission, apply_import_timestamps
computers_bp = Blueprint('computers', __name__) computers_bp = Blueprint('computers', __name__)

View File

@@ -5,8 +5,7 @@ Two audiences:
Full CRUD + publish lands in P2; this is the P1/first-slice read surface. Full CRUD + publish lands in P2; this is the P1/first-slice read surface.
- Client (service token, geenforce.fetch scope): GET /manifest serves the - Client (service token, geenforce.fetch scope): GET /manifest serves the
CURRENT PUBLISHED snapshot for a scope, never the live draft. Auth mirrors the CURRENT PUBLISHED snapshot for a scope, never the live draft. Auth mirrors the
collector's managed-token pattern (X-API-Key or Bearer PAT), plus an optional collector's managed-token pattern (X-API-Key or Bearer PAT).
GEENFORCE_API_KEY env bootstrap.
""" """
from functools import wraps from functools import wraps
@@ -219,8 +218,14 @@ def _apply_app_link(entry, payload):
appid = payload.get('appid') appid = payload.get('appid')
if appid in (None, '', 0): if appid in (None, '', 0):
entry.appid = None entry.appid = None
elif db.session.get(Application, int(appid)): return
entry.appid = int(appid) # Ignore a non-numeric id rather than 500 (matches the docstring contract).
try:
appid = int(appid)
except (ValueError, TypeError):
return
if db.session.get(Application, appid):
entry.appid = appid
@geenforce_bp.route('/scopes', methods=['POST']) @geenforce_bp.route('/scopes', methods=['POST'])

View File

@@ -147,7 +147,7 @@ class GeEnforcePlugin(BasePlugin):
def import_share_cmd(shareroot, preinstall, scope): def import_share_cmd(shareroot, preinstall, scope):
"""Import on-share manifests into draft rows (idempotent rebuild).""" """Import on-share manifests into draft rows (idempotent rebuild)."""
from flask import current_app from flask import current_app
from .importer import discover_share, load_manifest_file, build_scope from .importer import discover_share, load_manifest_file
from .service import replace_scope_draft from .service import replace_scope_draft
with current_app.app_context(): with current_app.app_context():

View File

@@ -127,6 +127,12 @@ def record_enforcement_report(payload):
phase = (payload.get('phase') or 'runtime').strip() phase = (payload.get('phase') or 'runtime').strip()
counts = payload.get('counts') or {} counts = payload.get('counts') or {}
results = payload.get('results') or [] results = payload.get('results') or []
# Guard wrong JSON shapes: a bad type must be a clean 400, not a 500 from a
# later .get()/iteration (the route maps ValueError to 400).
if not isinstance(counts, dict):
raise ValueError('counts must be an object')
if not isinstance(results, list):
raise ValueError('results must be a list')
failed = int(counts.get('failed', 0)) failed = int(counts.get('failed', 0))
installed = int(counts.get('installed', 0)) installed = int(counts.get('installed', 0))

View File

@@ -16,7 +16,7 @@ from shopdb.api import (
from ..models import KnowledgeBase from ..models import KnowledgeBase
from shopdb.api import require_permission, require_role, apply_import_timestamps from shopdb.api import require_permission, apply_import_timestamps
knowledgebase_bp = Blueprint('knowledgebase', __name__) knowledgebase_bp = Blueprint('knowledgebase', __name__)

View File

@@ -3,11 +3,11 @@
from flask import Blueprint, request from flask import Blueprint, request
from flask_jwt_extended import jwt_required from flask_jwt_extended import jwt_required
from shopdb.api import db, Asset, AssetType, Vendor, Model, AuditLog, success_response, error_response, paginated_response, ErrorCodes, get_pagination_params, paginate_query, resolve_dualpath_pairs, dualpath_single_machine_enabled from shopdb.api import db, Asset, AssetType, AuditLog, success_response, error_response, paginated_response, ErrorCodes, get_pagination_params, paginate_query, resolve_dualpath_pairs, dualpath_single_machine_enabled
from ..models import Machine, MachineType from ..models import Machine, MachineType
from shopdb.api import require_permission, require_role, apply_import_timestamps from shopdb.api import require_permission, apply_import_timestamps
machines_bp = Blueprint('machines', __name__) machines_bp = Blueprint('machines', __name__)

View File

@@ -7,7 +7,7 @@ from shopdb.api import db, Asset, AssetType, Vendor, AuditLog, success_response,
from ..models import NetworkDevice, NetworkDeviceType, Subnet, VLAN from ..models import NetworkDevice, NetworkDeviceType, Subnet, VLAN
from shopdb.api import require_permission, require_role, apply_import_timestamps from shopdb.api import require_permission, apply_import_timestamps
network_bp = Blueprint('network', __name__) network_bp = Blueprint('network', __name__)

View File

@@ -12,7 +12,7 @@ from shopdb.api import db, success_response, error_response, paginated_response,
from ..models import Notification, NotificationType from ..models import Notification, NotificationType
from shopdb.api import require_permission, require_role from shopdb.api import require_permission
notifications_bp = Blueprint('notifications', __name__) notifications_bp = Blueprint('notifications', __name__)

View File

@@ -19,7 +19,7 @@ from ..services import (
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
from shopdb.api import require_permission, require_role, apply_import_timestamps from shopdb.api import require_permission, apply_import_timestamps
printers_asset_bp = Blueprint('printers_asset', __name__) printers_asset_bp = Blueprint('printers_asset', __name__)

View File

@@ -8,7 +8,6 @@ from typing import List, Dict, Optional, Type
from flask import Flask, Blueprint from flask import Flask, Blueprint
from shopdb.plugins.base import BasePlugin, PluginMeta from shopdb.plugins.base import BasePlugin, PluginMeta
from shopdb.api import db
from .models import USBDevice, USBDeviceType, USBCheckout from .models import USBDevice, USBDeviceType, USBCheckout
from .api import usb_bp from .api import usb_bp

View File

@@ -16,7 +16,6 @@ import time
import requests import requests
from flask import current_app from flask import current_app
from shopdb.api import db
from shopdb.api import Setting from shopdb.api import Setting
# Two-level Dell token cache. Dell rate-limits the token endpoint, so a fresh # Two-level Dell token cache. Dell rate-limits the token endpoint, so a fresh

View File

@@ -5,7 +5,7 @@ import logging
from flask import Flask, send_from_directory from flask import Flask, send_from_directory
from .config import config from .config import config
from .extensions import db, migrate, jwt, cors, ma, init_extensions from .extensions import db, jwt, init_extensions
from .plugins import plugin_manager from .plugins import plugin_manager
# Platform contract version. See ADR-001 for the contract surface and # Platform contract version. See ADR-001 for the contract surface and

View File

@@ -54,7 +54,7 @@ def _require_computer_models():
return models, None return models, None
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_permission
applications_bp = Blueprint('applications', __name__) applications_bp = Blueprint('applications', __name__)

View File

@@ -14,7 +14,7 @@ from shopdb.utils.responses import (
) )
from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.pagination import get_pagination_params, paginate_query
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_permission
from shopdb.utils.import_mode import apply_import_timestamps from shopdb.utils.import_mode import apply_import_timestamps
assets_bp = Blueprint('assets', __name__) assets_bp = Blueprint('assets', __name__)

View File

@@ -4,7 +4,7 @@ from flask import Blueprint, request
from flask_jwt_extended import jwt_required from flask_jwt_extended import jwt_required
from shopdb.core.models import AuditLog from shopdb.core.models import AuditLog
from shopdb.utils.responses import success_response, error_response, ErrorCodes from shopdb.utils.responses import success_response
auditlogs_bp = Blueprint('auditlogs', __name__) auditlogs_bp = Blueprint('auditlogs', __name__)

View File

@@ -162,7 +162,7 @@ def login():
def refresh(): def refresh():
"""Refresh access token using refresh token.""" """Refresh access token using refresh token."""
user_id = get_jwt_identity() user_id = get_jwt_identity()
user = User.query.get(int(user_id)) user = db.session.get(User, int(user_id))
if not user or not user.isactive: if not user or not user.isactive:
return error_response( return error_response(

View File

@@ -13,7 +13,7 @@ from shopdb.utils.responses import (
) )
from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.pagination import get_pagination_params, paginate_query
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_role
from shopdb.utils.import_mode import apply_import_timestamps from shopdb.utils.import_mode import apply_import_timestamps
businessunits_bp = Blueprint('businessunits', __name__) businessunits_bp = Blueprint('businessunits', __name__)

View File

@@ -13,7 +13,7 @@ from shopdb.utils.responses import (
) )
from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.pagination import get_pagination_params, paginate_query
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_role
from shopdb.utils.import_mode import apply_import_timestamps from shopdb.utils.import_mode import apply_import_timestamps
locations_bp = Blueprint('locations', __name__) locations_bp = Blueprint('locations', __name__)

View File

@@ -17,7 +17,7 @@ from shopdb.utils.responses import (
) )
from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.pagination import get_pagination_params, paginate_query
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_role
from shopdb.utils.import_mode import apply_import_timestamps from shopdb.utils.import_mode import apply_import_timestamps
models_bp = Blueprint('models', __name__) models_bp = Blueprint('models', __name__)

View File

@@ -5,7 +5,7 @@ machinetypes; the machines plugin now owns the "machinetypes" name.
""" """
from flask import Blueprint, request from flask import Blueprint, request
from flask_jwt_extended import jwt_required, current_user from flask_jwt_extended import jwt_required
from shopdb.extensions import db from shopdb.extensions import db
from shopdb.core.models import ModelType from shopdb.core.models import ModelType
@@ -17,7 +17,7 @@ from shopdb.utils.responses import (
) )
from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.pagination import get_pagination_params, paginate_query
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_role
from shopdb.utils.import_mode import apply_import_timestamps from shopdb.utils.import_mode import apply_import_timestamps
modeltypes_bp = Blueprint('modeltypes', __name__) modeltypes_bp = Blueprint('modeltypes', __name__)

View File

@@ -13,7 +13,7 @@ from shopdb.utils.responses import (
) )
from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.pagination import get_pagination_params, paginate_query
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_role
from shopdb.utils.import_mode import apply_import_timestamps from shopdb.utils.import_mode import apply_import_timestamps
operatingsystems_bp = Blueprint('operatingsystems', __name__) operatingsystems_bp = Blueprint('operatingsystems', __name__)

View File

@@ -5,7 +5,7 @@ from flask_jwt_extended import jwt_required
from shopdb.utils.responses import success_response, error_response, ErrorCodes from shopdb.utils.responses import success_response, error_response, ErrorCodes
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_role
plugins_bp = Blueprint('plugins', __name__) plugins_bp = Blueprint('plugins', __name__)

View File

@@ -13,7 +13,7 @@ from shopdb.utils.responses import (
) )
from shopdb.utils.pagination import get_pagination_params, paginate_query from shopdb.utils.pagination import get_pagination_params, paginate_query
from shopdb.utils.authz import require_permission, require_role from shopdb.utils.authz import require_role
from shopdb.utils.import_mode import apply_import_timestamps from shopdb.utils.import_mode import apply_import_timestamps
vendors_bp = Blueprint('vendors', __name__) vendors_bp = Blueprint('vendors', __name__)

View File

@@ -18,7 +18,6 @@ where BOTH assets are active are resolved.
import re import re
from collections import namedtuple from collections import namedtuple
from shopdb.extensions import db
from shopdb.core.models import Asset, AssetRelationship, RelationshipType from shopdb.core.models import Asset, AssetRelationship, RelationshipType
# secondaryassetids: set of the non-primary bay asset ids (hide when collapsing) # secondaryassetids: set of the non-primary bay asset ids (hide when collapsing)

View File

@@ -1,7 +1,7 @@
"""Standardized API response helpers.""" """Standardized API response helpers."""
from flask import jsonify, make_response from flask import jsonify, make_response
from typing import Any, Dict, List, Optional from typing import Any, Dict, List
from datetime import datetime, timezone from datetime import datetime, timezone
import uuid import uuid

View File

@@ -144,6 +144,17 @@ def test_entry_curated_app_link(client, db, auth_headers):
assert any(a['appname'] == 'eDNC (tracked)' for a in picker) assert any(a['appname'] == 'eDNC (tracked)' for a in picker)
def test_entry_nonnumeric_appid_ignored(client, db, auth_headers):
"""A non-numeric appid must be ignored (link stays null), not 500. Matches
the _apply_app_link docstring contract."""
scopeid = _create_scope(client, auth_headers)
created = client.post(f'/api/geenforce/scopes/{scopeid}/entries',
json={'Name': 'x', 'Type': 'MSI', 'appid': 'notanumber'},
headers=auth_headers)
assert created.status_code == 201, created.get_json()
assert created.get_json()['data']['appid'] is None
def test_invalid_entry_type_rejected(client, db, auth_headers): def test_invalid_entry_type_rejected(client, db, auth_headers):
scopeid = _create_scope(client, auth_headers) scopeid = _create_scope(client, auth_headers)
resp = client.post(f'/api/geenforce/scopes/{scopeid}/entries', resp = client.post(f'/api/geenforce/scopes/{scopeid}/entries',

View File

@@ -36,6 +36,19 @@ def _token(client, auth_headers, scopes):
return resp.get_json()['data']['secret'] return resp.get_json()['data']['secret']
def test_report_malformed_counts_is_400(client, db, app, auth_headers):
"""A wrong JSON shape (counts as a string, results as a string) must be a
clean 400, not a 500 from a later .get()/iteration."""
_seed_and_publish(app)
secret = _token(client, auth_headers, ['geenforce.report'])
for bad in ({'counts': 'nope'}, {'results': 'nope'}):
body = {'hostname': 'WJCMM01', 'scopename': 'gea-shopfloor-cmm'}
body.update(bad)
resp = client.post('/api/geenforce/report', json=body,
headers={'X-API-Key': secret})
assert resp.status_code == 400, resp.get_json()
def test_report_recorded_and_listed(client, db, app, auth_headers): def test_report_recorded_and_listed(client, db, app, auth_headers):
_seed_and_publish(app) _seed_and_publish(app)
secret = _token(client, auth_headers, ['geenforce.report']) secret = _token(client, auth_headers, ['geenforce.report'])