Merge origin/main (West Jeff) into wilm

Bring in the natural-key import fix, Metrics->Monitors rename, PC detail tabbing and panel groups, guest search grants, and the 2026-08-30 security fixes. Reconcile endpoint packaging/installer, migrations, contract version, and tests; keep the delivery uninstaller and 6-step installer alongside main's permission-based RBAC and pin-drift protection.

tests/test_core/test_auth_ratelimit.py

@@ -32,11 +32,18 @@ def ratelimit_on(app):
 
 
 def _bad_login(client, ip):
-    """Attempt a login for a non-existent user from a given source IP."""
+    """Attempt a login for a non-existent user from a given source IP.
+
+    The caller is set through REMOTE_ADDR, the peer that actually connected,
+    because that is what the limiter keys on. It used to be sent as
+    X-Forwarded-For, which meant these tests changed identity using the very
+    header a real attacker would rotate to escape the limit - so the suite
+    could not have told the difference.
+    """
     return client.post(
         '/api/auth/login',
         json={'username': 'ghost', 'password': 'wrong'},
-        headers={'X-Forwarded-For': ip},
+        environ_base={'REMOTE_ADDR': ip},
     )
 
 

tests/test_core/test_clientip.py

@@ -1,4 +1,12 @@
-"""strip_port / client_ip normalization (ARR forwards clientip:port)."""
+"""client_ip: which address the app treats as the caller, and port normalization.
+
+The answer is remote_addr. X-Forwarded-For is written by the caller, and the
+login rate limit, the account lockout and the audit log all key on this value,
+so honoring the header let a caller rotate it for an unbounded budget and an
+untraceable trail. Where a proxy really is in front, TRUSTED_PROXY_COUNT turns
+on ProxyFix and the header arrives already resolved into remote_addr - ports
+included, because ARR forwards clientip:port.
+"""
 
 from shopdb.utils.clientip import strip_port, client_ip
 
@@ -34,12 +42,94 @@ class _FakeRequest:
         self.remote_addr = remote_addr
 
 
-def test_client_ip_prefers_forwarded_first_hop_stripped():
-    request_obj = _FakeRequest(headers={'X-Forwarded-For': '10.0.0.5:51000, 10.0.0.1'},
-                       remote_addr='127.0.0.1')
+def test_client_ip_ignores_the_forwarded_header():
+    """The caller sends X-Forwarded-For, so it cannot decide who the caller is.
+
+    This test used to assert the opposite. The login rate limit, the account
+    lockout and the audit log all key on client_ip, so honoring the header let
+    a caller rotate it for an unbounded budget and an untraceable trail. Where
+    a proxy really is in front, TRUSTED_PROXY_COUNT turns on ProxyFix and the
+    header reaches us already resolved into remote_addr.
+    """
+    request_obj = _FakeRequest(
+        headers={'X-Forwarded-For': '10.0.0.5:51000, 10.0.0.1'},
+        remote_addr='127.0.0.1')
+    assert client_ip(request_obj) == '127.0.0.1'
+
+
+def test_client_ip_strips_a_port_proxyfix_passed_through():
+    """ARR writes clientip:port into the header and ProxyFix copies it verbatim
+    into remote_addr, so the port still has to come off."""
+    request_obj = _FakeRequest(headers={}, remote_addr='10.0.0.5:51000')
     assert client_ip(request_obj) == '10.0.0.5'
 
 
 def test_client_ip_falls_back_to_remote_addr_stripped():
     request_obj = _FakeRequest(headers={}, remote_addr='192.168.1.9:60000')
     assert client_ip(request_obj) == '192.168.1.9'
+
+
+# ---------------------------------------------------------------------------
+# Through a real request, and through the login limiter that keys on this.
+# ---------------------------------------------------------------------------
+def test_the_forwarded_header_does_not_decide_the_client_ip(app):
+    from shopdb.utils.clientip import client_ip
+
+    with app.test_request_context(
+            '/', headers={'X-Forwarded-For': '10.0.0.99'},
+            environ_base={'REMOTE_ADDR': '203.0.113.7'}):
+        from flask import request
+        assert client_ip(request) == '203.0.113.7'
+
+
+def test_the_peer_address_decides_the_client_ip(app):
+    from shopdb.utils.clientip import client_ip
+
+    with app.test_request_context(
+            '/', environ_base={'REMOTE_ADDR': '203.0.113.8'}):
+        from flask import request
+        assert client_ip(request) == '203.0.113.8'
+
+
+def test_a_forwarded_port_is_still_stripped(app):
+    """IIS ARR writes "clientip:port" and ProxyFix passes it through verbatim,
+    so the port has to come off remote_addr or every request looks like a new
+    client."""
+    from shopdb.utils.clientip import client_ip
+
+    with app.test_request_context(
+            '/', environ_base={'REMOTE_ADDR': '203.0.113.9:54321'}):
+        from flask import request
+        assert client_ip(request) == '203.0.113.9'
+
+
+def test_rotating_the_forwarded_header_does_not_reset_the_login_limiter(app,
+                                                                        client,
+                                                                        db):
+    """The budget follows the connection, not a header the caller writes.
+
+    Sprayed across many usernames on purpose: the per-account lockout bounds
+    guessing at ONE account, so the IP limiter is the only thing standing
+    between a caller and unlimited attempts spread across the directory. That
+    is what rotating the header used to defeat.
+    """
+    caller = {'REMOTE_ADDR': '203.0.113.50'}
+    previous = dict(app.config)
+    app.config['AUTH_RATELIMIT_ENABLED'] = True
+    app.config['AUTH_RATELIMIT_MAX'] = 3
+    try:
+        seen = set()
+        for hop in range(10):
+            response = client.post(
+                '/api/auth/login',
+                headers={'X-Forwarded-For': '198.51.100.{}'.format(hop)},
+                environ_base=caller,
+                json={'username': 'nobody{}'.format(hop), 'password': 'wrong'})
+            seen.add(response.status_code)
+            if response.status_code == 429:
+                break
+    finally:
+        app.config['AUTH_RATELIMIT_ENABLED'] = previous.get(
+            'AUTH_RATELIMIT_ENABLED', False)
+        app.config['AUTH_RATELIMIT_MAX'] = previous.get('AUTH_RATELIMIT_MAX', 30)
+    assert 429 in seen, 'rotating X-Forwarded-For bought an unbounded budget'

tests/test_core/test_deactivated_user.py

@@ -0,0 +1,61 @@
+"""Deactivating an account ends its sessions.
+
+There is no token denylist in this product, so the JWT user loader is the only
+place a deactivated account can be stopped. It did a bare lookup by id, which
+meant deactivating somebody left their access token working until it expired and
+their refresh token minting new ones for thirty days. /auth/refresh and the API
+token path already checked isactive; the loader did not.
+"""
+from werkzeug.security import generate_password_hash
+
+
+def _login(client, username='deactivateme', password='testpass'):
+    response = client.post('/api/auth/login',
+                           json={'username': username, 'password': password})
+    assert response.status_code == 200, response.get_json()
+    return response.get_json()['data']
+
+
+def _make_user(db, username='deactivateme'):
+    from shopdb.core.models import User
+
+    user = User(
+        username=username,
+        email=f'{username}@test.local',
+        passwordhash=generate_password_hash('testpass'),
+    )
+    db.session.add(user)
+    db.session.commit()
+    return user
+
+
+def test_a_live_token_stops_working_once_the_account_is_deactivated(client, db):
+    user = _make_user(db)
+    tokens = _login(client)
+    headers = {'Authorization': 'Bearer {}'.format(tokens['access_token'])}
+
+    assert client.get('/api/auth/me', headers=headers).status_code == 200
+
+    user.isactive = False
+    db.session.commit()
+
+    assert client.get('/api/auth/me', headers=headers).status_code == 401
+
+
+def test_a_deactivated_account_cannot_refresh(client, db):
+    user = _make_user(db, 'refreshme')
+    tokens = _login(client, 'refreshme')
+    headers = {'Authorization': 'Bearer {}'.format(tokens['refresh_token'])}
+
+    user.isactive = False
+    db.session.commit()
+
+    assert client.post('/api/auth/refresh', headers=headers).status_code == 401
+
+
+def test_an_active_account_is_unaffected(client, db):
+    """The loader must not reject everybody; isactive defaults true."""
+    _make_user(db, 'stillhere')
+    tokens = _login(client, 'stillhere')
+    headers = {'Authorization': 'Bearer {}'.format(tokens['access_token'])}
+    assert client.get('/api/auth/me', headers=headers).status_code == 200

tests/test_core/test_guest_role.py

@@ -98,12 +98,61 @@ def test_a_site_with_no_guest_role_behaves_as_it_always_did(client, db):
 # The guardrail
 # ---------------------------------------------------------------------------
 
-def test_a_view_permission_is_grantable(app):
+def test_a_declared_permission_is_grantable(app):
     from shopdb.core.models import guest_grantable_permissions
     with app.app_context():
         grantable = guest_grantable_permissions()
-    assert 'assets.view' in grantable
     assert 'reports.view' in grantable
+    assert 'applications.view' in grantable
+
+
+def test_a_view_permission_is_not_grantable_merely_for_being_a_view(app):
+    """The suffix stopped deciding. These are all '.view' and none is declared.
+
+    settings.view gates every setting row; audit.view the before/after of every
+    change; users.view nothing at all. Each was anonymous-grantable purely
+    because of how it was spelled.
+
+    assets.view is deliberately NOT in this list. It was, while global search
+    answered twelve domains behind it; now that search gates each domain on its
+    own permission the grant reaches assets alone, and the floor needs it to
+    look an asset number up without signing in.
+    """
+    from shopdb.core.models import guest_grantable_permissions
+    with app.app_context():
+        grantable = guest_grantable_permissions()
+    for name in ('settings.view', 'audit.view', 'users.view'):
+        assert name not in grantable, name
+
+
+def test_the_guest_features_that_already_shipped_stay_grantable(app):
+    """Both were granted on a live site before a grant had to be declared.
+
+    Inverting the rule to explicit declaration dropped every permission that
+    had qualified by its .view suffix, including two the product ships guest
+    flows for: guides, whose nav item and list route both reason about a
+    signed-out reader, and the tools page, without which a guest holding
+    tools.secrets cannot reach Secure Send at all.
+
+    A site that granted them under the old rule was then unable to save the
+    guest role AT ALL - the API refuses the whole request when any submitted
+    permission is undeclared, and the grid resubmits what the role already
+    holds. That deadlock is what this guards against, so anyone narrowing the
+    grantable set again has to look at what is already granted in the field.
+    """
+    from shopdb.core.models import guest_grantable_permissions
+    with app.app_context():
+        grantable = guest_grantable_permissions()
+    assert 'guides.view' in grantable
+    assert 'tools.view' in grantable
+
+
+def test_the_bytes_behind_applications_are_not_grantable(app):
+    """applications.view is grantable only because the bytes moved off it."""
+    from shopdb.core.models import guest_grantable_permissions
+    with app.app_context():
+        grantable = guest_grantable_permissions()
+    assert 'applications.download' not in grantable
 
 
 def test_a_write_permission_is_not_grantable(app):
@@ -127,13 +176,29 @@ def test_the_api_refuses_to_grant_a_guest_a_write(client, db, auth_headers,
     assert guestrole.permissions == []
 
 
-def test_the_api_allows_granting_a_guest_a_view(client, db, auth_headers,
-                                                guestrole):
+def test_the_api_allows_granting_a_guest_a_declared_read(client, db,
+                                                         auth_headers,
+                                                         guestrole):
+    response = client.put('/api/users/roles/{}'.format(guestrole.roleid),
+                          headers=auth_headers,
+                          json={'permissions': ['reports.view']})
+    assert response.status_code == 200
+    assert [p.name for p in guestrole.permissions] == ['reports.view']
+
+
+def test_the_api_refuses_an_undeclared_read(client, db, auth_headers,
+                                            guestrole):
+    """This test used to grant guests audit.view and pass, which is the bug.
+
+    audit.view ends in '.view' and gates usernames, addresses and the
+    before/after of every change in the database.
+    """
     response = client.put('/api/users/roles/{}'.format(guestrole.roleid),
                           headers=auth_headers,
                           json={'permissions': ['audit.view']})
-    assert response.status_code == 200
-    assert [p.name for p in guestrole.permissions] == ['audit.view']
+    assert response.status_code == 400
+    assert 'audit.view' in response.get_json()['data']['error']['message']
+    assert guestrole.permissions == []
 
 
 def test_the_guest_role_cannot_be_deleted(client, auth_headers, guestrole):
@@ -155,8 +220,9 @@ def test_the_role_list_says_what_a_guest_may_hold(client, auth_headers,
     rows = client.get('/api/users/roles', headers=auth_headers).get_json()['data']
     guest = next(r for r in rows if r['rolename'] == 'guest')
     assert guest['isguest'] is True
-    assert 'assets.view' in guest['grantable']
+    assert 'reports.view' in guest['grantable']
     assert 'assets.delete' not in guest['grantable']
+    assert 'audit.view' not in guest['grantable']
 
     other = next((r for r in rows if r['rolename'] != 'guest'), None)
     if other is not None:
@@ -188,3 +254,39 @@ def test_a_bad_token_is_still_refused(client, db, guestrole):
     response = client.get('/api/auditlogs',
                           headers={'Authorization': 'Bearer not.a.token'})
     assert response.status_code == 422
+
+
+# ---------------------------------------------------------------------------
+# What the chrome is allowed to draw
+# ---------------------------------------------------------------------------
+
+def test_a_guest_is_not_offered_search_before_the_grant(client, guestrole):
+    """The box is hidden until somebody opens the door, not by default."""
+    body = client.get('/api/dashboard/capabilities').get_json()['data']
+    assert body['cansearch'] is False
+
+
+def test_granting_a_guest_assets_view_offers_them_search(client, db, guestrole):
+    """The point of the whole exercise: a signed-out visitor standing at a
+    machine gets the box, so they can look up the number in front of them."""
+    grant(db, guestrole, 'assets.view')
+    body = client.get('/api/dashboard/capabilities').get_json()['data']
+    assert body['cansearch'] is True
+
+
+def test_a_member_without_assets_view_is_not_offered_search(client, db,
+                                                            member_headers):
+    """It used to be drawn for anyone signed in, and answered 401 to the ones
+    who could not use it."""
+    body = client.get('/api/dashboard/capabilities',
+                      headers=member_headers).get_json()['data']
+    assert body['cansearch'] is False
+
+
+def test_capabilities_does_not_report_what_the_guest_role_holds(client, db,
+                                                                guestrole):
+    """It answers about the CALLER only. A signed-out reader learns whether
+    their own box is drawn, not the shape of the site's guest policy."""
+    grant(db, guestrole, 'assets.view')
+    body = client.get('/api/dashboard/capabilities').get_json()['data']
+    assert set(body) == {'cansearch'}

tests/test_core/test_pluginui.py

@@ -142,3 +142,86 @@ def test_asset_presentation_skip_disabled_plugin(app, client, auth_headers, monk
 
     entries = _data(client, '/api/pluginui/asset-presentation', auth_headers)
     assert 'measuring_tool' not in {e.get('assettype') for e in entries}
+
+
+# =============================================================================
+# asset-panel groups (which detail-page tab a panel lands in)
+# =============================================================================
+
+def test_every_panel_comes_back_with_a_group(app, client, db, auth_headers,
+                                             monkeypatch):
+    """The client filters by group, so a panel without one must not vanish.
+
+    Resolution happens on the server precisely so a plugin written before tabs
+    existed still lands somewhere a person can see.
+    """
+    from shopdb.core.api.pluginui import ASSET_PANEL_GROUPS
+    pm = app.extensions['plugin_manager']
+    monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True)
+    assetid = _make_asset(db)
+
+    panels = _data(client, '/api/pluginui/asset-panels?assetid=%d' % assetid,
+                   auth_headers)
+    assert panels, 'no panels matched, so this test proves nothing'
+    for panel in panels:
+        assert panel.get('group') in ASSET_PANEL_GROUPS, panel.get('id')
+
+
+def test_an_undeclared_group_defaults_to_details(app, client, db, auth_headers,
+                                                 monkeypatch):
+    """Omitting group is the documented answer for most panels."""
+    pm = app.extensions['plugin_manager']
+    monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True)
+    plugin = next(iter(pm.get_all_plugins().values()))
+    monkeypatch.setattr(plugin, 'get_asset_panels', lambda: [{
+        'id': 'nogroup', 'title': 'No group', 'assettypes': ['*'],
+        'endpoint': '/api/whatever', 'render': 'keyvalue', 'position': 1,
+    }])
+    assetid = _make_asset(db)
+
+    panels = _data(client, '/api/pluginui/asset-panels?assetid=%d' % assetid,
+                   auth_headers)
+    panel = next(p for p in panels if p['id'] == 'nogroup')
+    assert panel['group'] == 'details'
+
+
+def test_a_group_nobody_defined_falls_back_rather_than_disappearing(
+        app, client, db, auth_headers, monkeypatch):
+    """A typo must not drop the panel off every tab.
+
+    The page renders known groups only, so an unrecognised value that survived
+    to the client would be a panel that exists and is never shown anywhere.
+    """
+    pm = app.extensions['plugin_manager']
+    monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True)
+    plugin = next(iter(pm.get_all_plugins().values()))
+    monkeypatch.setattr(plugin, 'get_asset_panels', lambda: [{
+        'id': 'typo', 'title': 'Typo', 'assettypes': ['*'],
+        'endpoint': '/api/whatever', 'render': 'keyvalue', 'position': 1,
+        'group': 'montiors',
+    }])
+    assetid = _make_asset(db)
+
+    panels = _data(client, '/api/pluginui/asset-panels?assetid=%d' % assetid,
+                   auth_headers)
+    panel = next(p for p in panels if p['id'] == 'typo')
+    assert panel['group'] == 'details'
+
+
+def test_group_matching_is_not_case_or_space_sensitive(app, client, db,
+                                                       auth_headers, monkeypatch):
+    """'Monitors ' is obviously meant to be monitors; do not send it to details."""
+    pm = app.extensions['plugin_manager']
+    monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True)
+    plugin = next(iter(pm.get_all_plugins().values()))
+    monkeypatch.setattr(plugin, 'get_asset_panels', lambda: [{
+        'id': 'shouty', 'title': 'Shouty', 'assettypes': ['*'],
+        'endpoint': '/api/whatever', 'render': 'keyvalue', 'position': 1,
+        'group': '  Monitors ',
+    }])
+    assetid = _make_asset(db)
+
+    panels = _data(client, '/api/pluginui/asset-panels?assetid=%d' % assetid,
+                   auth_headers)
+    panel = next(p for p in panels if p['id'] == 'shouty')
+    assert panel['group'] == 'monitors'

tests/test_core/test_public_read_surface.py

@@ -28,6 +28,10 @@ PUBLIC_ENDPOINTS = {
     'plugins.list_enabled_plugins',
     'dashboard.get_navigation',      # filtered per caller; a guest sees only
                                      # what the guest role can reach
+    'dashboard.get_capabilities',    # booleans about the CALLER, used to decide
+                                     # whether the search box is drawn at all;
+                                     # it must answer a signed-out visitor or
+                                     # the guest grant cannot reach the chrome
     'dashboard.health_check',        # monitoring has no account
 
     # Settings: the handler serves an unauthenticated caller only the public

tests/test_core/test_search_domain_permissions.py

@@ -0,0 +1,94 @@
+"""Global search answers each domain only to a caller who holds ITS permission.
+
+The route is gated on assets.view, and that used to be the entire check. One
+permission then queried twelve domains, so anyone who could look up an asset
+number also read out of the staff directory, the USB checkout history, the
+notification log and the network inventory.
+
+That is why assets.view could not be handed to guests, which cost the floor its
+search. Gating each domain separately is what makes the grant safe again: it
+reaches assets and nothing else.
+"""
+from unittest.mock import patch
+
+import pytest
+
+from shopdb.core.models import Permission, Role
+
+
+def _grant_guest(db, *names):
+    guest = Role.guest()
+    guest.permissions = [p for p in Permission.query.filter(
+        Permission.name.in_(names)).all()]
+    db.session.commit()
+
+
+@pytest.fixture
+def guestrole(db):
+    """The seeded guest role, holding nothing."""
+    Permission.seed()
+    Role.seedguest()
+    db.session.commit()
+    return Role.guest()
+
+
+def _search(client, term='a'):
+    response = client.get('/api/search?q={}'.format(term))
+    assert response.status_code == 200, response.get_json()
+    return response.get_json()['data']
+
+
+FAKE_EMPLOYEE = [{
+    'First_Name': 'Ada', 'Last_Name': 'Smith', 'SSO': 508123,
+    'Team': 'Machining', 'Role': 'Operator',
+}]
+
+
+def _with_employee_directory():
+    """The HR directory is an external database and is unreachable in tests, so
+    the domain silently returned nothing and a leak test over it proved nothing.
+    Patched here so there is something real to leak."""
+    return patch('shopdb.core.api.search._employee_rows',
+                 return_value=FAKE_EMPLOYEE)
+
+
+def test_assets_view_alone_does_not_reach_the_staff_directory(client, db,
+                                                              guestrole):
+    """The finding this exists for: ?q=smith returned employee records to a
+    signed-out visitor holding only assets.view."""
+    _grant_guest(db, 'assets.view')
+    with _with_employee_directory():
+        data = _search(client, 'smith')
+    assert 'employee' not in {row['type'] for row in data['results']}
+
+
+def test_employees_view_does_reach_it(client, db, guestrole):
+    """The other direction, so the gate is not passing by refusing everything."""
+    _grant_guest(db, 'assets.view', 'employees.view')
+    with _with_employee_directory():
+        data = _search(client, 'smith')
+    employees = [row for row in data['results'] if row['type'] == 'employee']
+    assert len(employees) == 1
+    assert employees[0]['title'] == 'Ada Smith'
+
+
+def test_search_still_answers_a_guest_with_assets_view(client, db, guestrole):
+    """The regression that prompted this: guests lost search entirely.
+
+    assets.view left the grantable set because of the leak above, which took
+    the floor's asset lookup with it.
+    """
+    _grant_guest(db, 'assets.view')
+    response = client.get('/api/search?q=press')
+    assert response.status_code == 200
+
+
+def test_a_guest_with_nothing_granted_is_refused(client, db, guestrole):
+    _grant_guest(db)
+    assert client.get('/api/search?q=press').status_code in (401, 403)
+
+
+def test_assets_view_is_grantable_again(app):
+    from shopdb.core.models import guest_grantable_permissions
+    with app.app_context():
+        assert 'assets.view' in guest_grantable_permissions()

tests/test_csv_import.py

@@ -335,3 +335,29 @@ def test_reconcile_still_binds_normalized_exact(db):
     _db.session.rollback()
 
 
+
+
+def test_a_numeric_assetnumber_is_read_as_a_name_not_an_id(db):
+    """This site numbers operations 0NNN, so numeric natural keys are real.
+
+    An all-digit string used to be taken at face value as an id. `0600` became
+    id 600 - a different asset, or none - which is how an Azure device import
+    died on a foreign key partway through, and how a PC could instead have been
+    bound silently to a printer's record. The label wins when one matches.
+    """
+    resolver = Resolver()
+    seed_reference(resolver)
+    import_csv('assets', 'assetnumber,assettypeid\n0600,Machine\n',
+               resolver=resolver, commit=True)
+    realid = _db.session.execute(_db.text(
+        "select assetid from assets where assetnumber = '0600'")).scalar()
+
+    # Force the ambiguity: an id that is NOT the one named 0600.
+    assert resolver.resolve('assets', 'assetid', '0600') == realid
+
+
+def test_a_number_naming_nothing_is_still_read_as_an_id(db):
+    """Round-tripped exports and sites that know their ids keep working."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    assert resolver.resolve('assets', 'assetid', '999999') == 999999

tests/test_no_hidden_tests.py

@@ -0,0 +1,50 @@
+"""No test file may sit where pytest will not collect it.
+
+pytest.ini sets `testpaths = tests`, so a test file anywhere else is never run
+by `pytest` and never run in CI. It still passes review, still gets maintained,
+and still reports nothing.
+
+That is not hypothetical. plugins/endpoints/tests/test_plugin.py arrived with a
+merge carrying 55 lines of delivery assertions and was collected ZERO times; run
+directly, five of its tests failed. plugins/printedparts/tests/test_plugin.py had
+been sitting uncollected for longer. Both are now under tests/.
+
+The failure mode is the worst kind: the suite is green, the coverage is imaginary,
+and the authorisation on a route can be deleted without anything going red.
+"""
+from pathlib import Path
+
+REPO = Path(__file__).resolve().parents[1]
+
+# Where pytest actually looks (pytest.ini: testpaths = tests).
+COLLECTED = REPO / 'tests'
+
+# Trees that legitimately hold test-shaped files pytest must not collect:
+# vendored dependencies, build output, and the frontend's own vitest specs.
+IGNORED = ('node_modules', 'venv', '.git', 'dist', '__pycache__',
+           'site-packages', '.plugins-staged', 'frontend')
+
+# The Windows installer stages a whole copy of the app under here. It is build
+# output, not source, and the export excludes it - caught by this test on its
+# first run, which is the sort of thing the ignore list exists for.
+IGNORED_TREES = ('deploy/windows/installer/bundle',)
+
+
+def test_every_test_file_lives_where_pytest_looks():
+    stray = []
+    for path in REPO.rglob('test_*.py'):
+        if any(part in IGNORED for part in path.parts):
+            continue
+        relative = path.relative_to(REPO).as_posix()
+        if any(relative.startswith(tree) for tree in IGNORED_TREES):
+            continue
+        if COLLECTED in path.parents:
+            continue
+        stray.append(path.relative_to(REPO))
+
+    assert not stray, (
+        'These test files are never collected, because pytest.ini limits\n'
+        'collection to tests/. They pass review and report nothing:\n  '
+        + '\n  '.join(str(p) for p in sorted(stray))
+        + '\n\nMove them under tests/ (tests/test_plugins/ for plugin tests).'
+    )

tests/test_plugin_migrations.py

@@ -94,7 +94,7 @@ EXPECTED_HEAD_REVISION['warranty'] = 'warranty0002proof'
 # adds the endpointcommands queue plus the MTConnect state columns, 0003
 # adds the generic command target/targetkind columns, and 0004 adds the
 # endpointapplications SBOM snapshot table.
-EXPECTED_HEAD_REVISION['endpoints'] = 'endpoints0004apps'
+EXPECTED_HEAD_REVISION['endpoints'] = 'endpoints0005cmdpin'
 
 # Plugins built after the cutover: their 0001 baseline really creates tables the
 # core chain never owned.

tests/test_plugins/test_endpoint_applications_model.py

@@ -0,0 +1,87 @@
+"""The installed-software snapshot table: it must actually accept inserts.
+
+This table is rewritten wholesale on every endpoint check-in - delete the
+asset's rows, insert the current inventory - so its surrogate key climbs at
+(endpoints x installed apps) per report and never reuses a value. On MySQL a
+32-bit key exhausts and inserts stop working permanently.
+
+Widening it to BigInteger broke every insert on SQLite, because SQLite only
+autoincrements a column typed exactly INTEGER PRIMARY KEY. Neither fault was
+caught by the suite, for the same reason: nothing inserted into this table at
+all. This does.
+"""
+from shopdb.api import db
+from plugins.endpoints.models.endpoint import EndpointApplication
+
+
+def test_rows_insert_and_the_key_autoincrements(app):
+    with app.app_context():
+        db.create_all()
+        first = EndpointApplication(assetid=1, displayname='Probe', displayversion='1.0')
+        second = EndpointApplication(assetid=1, displayname='Probe 2', displayversion='2.0')
+        db.session.add_all([first, second])
+        db.session.commit()
+
+        assert first.endpointapplicationid is not None
+        assert second.endpointapplicationid is not None
+        assert second.endpointapplicationid != first.endpointapplicationid
+
+
+def test_the_key_is_wide_enough_for_a_table_that_is_rewritten_every_cycle(app):
+    """BIGINT on MySQL, INTEGER on SQLite - both halves matter.
+
+    Dropping the variant breaks SQLite inserts; dropping the BigInteger breaks
+    MySQL years later, which is worse because it is silent until it is not.
+    """
+    from sqlalchemy.dialects import mysql, sqlite
+
+    with app.app_context():
+        column = EndpointApplication.__table__.c.endpointapplicationid
+        assert 'BIGINT' in str(column.type.compile(dialect=mysql.dialect()))
+        assert 'INTEGER' in str(column.type.compile(dialect=sqlite.dialect()))
+
+
+def test_over_long_registry_values_are_clamped_not_fatal(app):
+    """One malformed Uninstall entry must not cost the whole check-in.
+
+    On MySQL in strict mode an over-long value raises Data too long and fails the
+    transaction, taking telemetry and events down with it - so that endpoint goes
+    silent until somebody uninstalls the offending program. SQLite does not
+    enforce declared lengths, so the suite would never have shown it.
+    """
+    from plugins.endpoints.plugin import EndpointsPlugin
+
+    with app.app_context():
+        db.create_all()
+        warnings = []
+        stored = EndpointsPlugin()._replace_applications(
+            assetid=4242,
+            applications=[{
+                'displayname': 'A' * 400,
+                'displayversion': 'V' * 400,
+                'publisher': 'P' * 400,
+            }],
+            warnings=warnings,
+        )
+        db.session.commit()
+
+        assert stored == 1
+        row = EndpointApplication.query.filter_by(assetid=4242).one()
+        assert len(row.displayname) == 255
+        assert len(row.displayversion) == 128
+        assert len(row.publisher) == 255
+
+
+def test_a_blank_value_becomes_null_rather_than_an_empty_string(app):
+    from plugins.endpoints.plugin import EndpointsPlugin
+
+    with app.app_context():
+        db.create_all()
+        EndpointsPlugin()._replace_applications(
+            assetid=4243,
+            applications=[{'displayname': 'Thing', 'publisher': '   '}],
+            warnings=[],
+        )
+        db.session.commit()
+        row = EndpointApplication.query.filter_by(assetid=4243).one()
+        assert row.publisher is None

tests/test_plugins/test_endpoint_delivery_descriptor.py

@@ -0,0 +1,151 @@
+"""An update_app delivery must pin the bytes it actually delivers.
+
+The agent verifies the downloaded file against the descriptor's sha256 and fails
+closed on a mismatch - which is worthless if the descriptor carries no digest,
+and misleading if it carries a digest for a different file.
+
+Both were true of the version: branch. It pinned AppVersion.sha256, but
+/api/endpoints/apppackage/<appversionid> resolves the version to its application
+and serves application.installpath, so the digest described an artifact that
+route never sends; where the version had no sha256 the descriptor carried none
+at all and the agent had nothing to check.
+
+It also ended the URL at the id, so an MSI arrived with no extension and the
+agent's Path.GetExtension could not route it through msiexec.
+"""
+import pytest
+
+from shopdb.api import db
+from shopdb.core.models import Application, AppVersion
+
+
+@pytest.fixture
+def hosted_app(app, request):
+    # Unique per test: appname is unique-constrained and the database outlives
+    # a single test here, so a fixed name collides on the second use.
+    with app.app_context():
+        db.create_all()
+        application = Application(
+            appname='Widget-%s' % request.node.name[-24:],
+            installpath='/api/applications/package/widget-setup.msi',
+            installersha256='a' * 64,
+        )
+        db.session.add(application)
+        db.session.commit()
+        version = AppVersion(appid=application.appid, version='2.1', sha256=None)
+        db.session.add(version)
+        db.session.commit()
+        yield application.appid, version.appversionid
+
+
+def test_a_version_delivery_carries_a_digest_even_when_the_version_has_none(app, hosted_app):
+    from plugins.endpoints.api.routes import _resolve_update_app
+
+    appid, appversionid = hosted_app
+    with app.app_context():
+        descriptor, error = _resolve_update_app(f'version:{appversionid}')
+
+    assert error is None, error
+    # Not None: this is the whole point. A blank pin makes the agent fail open.
+    assert descriptor['sha256'] == 'a' * 64
+
+
+def test_a_version_delivery_keeps_the_installer_extension(app, hosted_app):
+    """Without the filename an MSI is not routed through msiexec."""
+    from plugins.endpoints.api.routes import _resolve_update_app
+
+    appid, appversionid = hosted_app
+    with app.app_context():
+        descriptor, _ = _resolve_update_app(f'version:{appversionid}')
+
+    assert descriptor['installpath'].endswith('.msi')
+
+
+def test_a_package_with_no_recorded_digest_is_refused_not_delivered_unverified(app):
+    """Refusing to enqueue beats delivering something nothing can verify."""
+    from plugins.endpoints.api.routes import _resolve_update_app
+
+    with app.app_context():
+        db.create_all()
+        application = Application(appname='Unhashed-%s' % id(app),
+                                  installpath='/api/applications/package/x.exe')
+        db.session.add(application)
+        db.session.commit()
+        version = AppVersion(appid=application.appid, version='1.0', sha256=None)
+        db.session.add(version)
+        db.session.commit()
+
+        descriptor, error = _resolve_update_app(f'version:{version.appversionid}')
+
+    assert descriptor is None
+    assert 'SHA-256' in error
+
+
+# ---------------------------------------------------------------------------
+# The digest is pinned when an operator approves the push, not when the agent
+# claims it. Between those two moments the bytes behind a descriptor can be
+# replaced by anyone who can edit the catalog, and the re-resolved descriptor
+# would faithfully describe the NEW bytes.
+# ---------------------------------------------------------------------------
+
+def test_an_unpinned_command_still_claims(app):
+    """Actions that deliver no content, and rows queued before the pin existed."""
+    from plugins.endpoints.api.routes import _pin_holds
+    from plugins.endpoints.models import EndpointCommand
+
+    with app.app_context():
+        command = EndpointCommand(assetid=1, action='mtconnect_start',
+                                  pinnedsha256=None)
+        assert _pin_holds(command, None) is True
+        assert _pin_holds(command, {'sha256': 'b' * 64}) is True
+
+
+def test_a_matching_digest_claims(app):
+    from plugins.endpoints.api.routes import _pin_holds
+    from plugins.endpoints.models import EndpointCommand
+
+    with app.app_context():
+        command = EndpointCommand(assetid=1, action='run_script',
+                                  pinnedsha256='c' * 64)
+        assert _pin_holds(command, {'sha256': 'c' * 64}) is True
+
+
+def test_a_swapped_digest_does_not_claim(app):
+    """The finding this exists for: approve one script, ship another.
+
+    An operator queues run_script against a reviewed body. Before the agent's
+    long poll returns, somebody with applications.edit replaces the body and
+    the digest is restamped. Without the pin the claim re-resolves, hands the
+    agent the attacker's digest, and the agent verifies it correctly.
+    """
+    from plugins.endpoints.api.routes import _pin_holds
+    from plugins.endpoints.models import EndpointCommand
+
+    with app.app_context():
+        command = EndpointCommand(assetid=1, action='run_script',
+                                  pinnedsha256='c' * 64)
+        assert _pin_holds(command, {'sha256': 'd' * 64}) is False
+
+
+def test_a_vanished_descriptor_is_not_treated_as_a_swap(app):
+    """A deleted script already sends the command through with no descriptor so
+    the agent fails closed; that path must not be reported as tampering."""
+    from plugins.endpoints.api.routes import _pin_holds
+    from plugins.endpoints.models import EndpointCommand
+
+    with app.app_context():
+        command = EndpointCommand(assetid=1, action='run_script',
+                                  pinnedsha256='c' * 64)
+        assert _pin_holds(command, None) is True
+
+
+def test_enqueue_records_the_digest_it_approved(app, hosted_app):
+    """The resolved descriptor used to be discarded at enqueue (`_descriptor`)."""
+    from plugins.endpoints.api.routes import _resolve_update_app
+
+    appid, appversionid = hosted_app
+    with app.app_context():
+        descriptor, error = _resolve_update_app(str(appversionid))
+        assert error is None
+        # This is the value the enqueue path now writes to pinnedsha256.
+        assert descriptor['sha256'] == 'a' * 64

tests/test_plugins/test_endpoints_plugin.py

@@ -0,0 +1,273 @@
+"""Endpoints plugin smoke + collector-contract tests.
+
+Asserts the plugin loads cleanly, satisfies the framework contract, and that
+its ADR-006 collector schema is well formed. Extend with DB-backed ingest tests
+as the plugin grows.
+"""
+
+from flask import Blueprint
+
+from plugins.endpoints.plugin import EndpointsPlugin, CURATED_EVENT_IDS
+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,
+)
+
+
+def test_endpoints_plugin_meta_is_valid():
+    """meta returns a PluginMeta with the expected name and prefix."""
+    plugin = EndpointsPlugin()
+    assert plugin.meta.name == 'endpoints'
+    assert plugin.meta.api_prefix == '/api/endpoints'
+    assert 'computers' in plugin.meta.dependencies
+
+
+def test_endpoints_plugin_get_blueprint_returns_blueprint():
+    """get_blueprint returns a Flask Blueprint, not None."""
+    plugin = EndpointsPlugin()
+    assert isinstance(plugin.get_blueprint(), Blueprint)
+
+
+def test_endpoints_plugin_get_models_returns_models():
+    """get_models returns the two telemetry tables."""
+    plugin = EndpointsPlugin()
+    models = plugin.get_models()
+    assert len(models) >= 2
+    for model in models:
+        assert hasattr(model, '__tablename__')
+
+
+def test_endpoints_collector_schema_identity_is_hostname():
+    """ADR-006 schema declares hostname as the identity field."""
+    plugin = EndpointsPlugin()
+    schema = plugin.get_collector_schema()
+    assert schema is not None
+    assert schema['identityfield'] == 'hostname'
+    assert 'hostname' in schema['fields']['required']
+
+
+def test_endpoints_curated_event_ids_include_core_security_ids():
+    """The curated catalog covers the agreed logon/stability IDs."""
+    for eventid in (4624, 4625, 4634, 4672, 4697, 4698, 41, 4618, 55, 7011,
+                    10016, 1001):
+        assert eventid in CURATED_EVENT_IDS
+
+
+def test_curated_ids_match_the_event_catalog():
+    """Ingest filter and display catalog cannot drift apart."""
+    assert CURATED_EVENT_IDS == frozenset(EVENT_CATALOG)
+
+
+def test_critical_event_ids_are_a_subset_of_the_catalog():
+    """Dashboard stability card only queries known critical IDs."""
+    assert set(CRITICAL_EVENT_IDS)
+    assert set(CRITICAL_EVENT_IDS).issubset(set(EVENT_CATALOG))
+
+
+def test_endpoints_asset_panels_are_computer_scoped():
+    """Every asset panel targets computer assets only, with an endpoint."""
+    plugin = EndpointsPlugin()
+    panels = plugin.get_asset_panels()
+    assert len(panels) == 6
+    for panel in panels:
+        assert panel['assettypes'] == ['computer']
+        assert panel['endpoint'].startswith('/api/endpoints/asset/{assetid}')
+        assert panel['render'] in ('keyvalue', 'list', 'table')
+
+
+def test_services_and_processes_are_split_into_two_filtered_cards():
+    """The single services card is now two cards filtered by row kind, both
+    reading the same services snapshot so a future top-20 collector is just
+    another filtered panel over the same feed."""
+    plugin = EndpointsPlugin()
+    panels = {p['id']: p for p in plugin.get_asset_panels()}
+    services = panels['endpoints-tracked-services']
+    processes = panels['endpoints-tracked-processes']
+    assert services['title'] == 'Tracked Services'
+    assert processes['title'] == 'Tracked Processes'
+    assert services['filter'] == {'key': 'kind', 'equals': 'service'}
+    assert processes['filter'] == {'key': 'kind', 'equals': 'process'}
+    # Both slice one endpoint feed.
+    assert services['endpoint'] == processes['endpoint']
+    assert services['endpoint'].endswith('/services')
+    # Command wiring carries over to both cards.
+    assert services['permission'] == 'endpoints.command'
+    assert processes['permission'] == 'endpoints.command'
+
+
+def test_activity_card_starts_collapsed_and_sits_below_live_state():
+    """The activity feed is history, so it must not push live state down.
+
+    Renamed from the events card when events and commands merged into one feed.
+    It is no longer the LAST panel - installed software sits below it - so the
+    assertion is that it ranks below every live-state panel rather than being
+    the maximum, which would break again the next time a reference panel is
+    added underneath.
+    """
+    plugin = EndpointsPlugin()
+    panels = plugin.get_asset_panels()
+    activity = next(p for p in panels if p['id'] == 'endpoints-activity')
+    # NOT collapsed by default, unlike the events card it replaced. The feed
+    # surfaces critical events, and a card that hides the one thing worth seeing
+    # is the reason the health dot and the visible rows disagreed. Collapsible,
+    # so a reader can put it away.
+    assert activity.get('collapsible') is True
+    assert activity.get('collapsedDefault') is not True
+    live = [p['position'] for p in panels
+            if p['id'] in ('endpoints-status', 'endpoints-tracked-services',
+                           'endpoints-tracked-processes', 'endpoints-interfaces')]
+    assert activity['position'] > max(live)
+
+
+def test_endpoints_dashboard_widgets_declared():
+    """Quiet + stability exception cards are declared and hide when empty."""
+    plugin = EndpointsPlugin()
+    widgets = plugin.get_dashboard_widgets()
+    ids = {w['id'] for w in widgets}
+    assert {'endpoints-quiet', 'endpoints-stability'} <= ids
+    for widget in widgets:
+        assert widget['render'] == 'exceptions'
+        assert widget['empty'] == 'hide'
+        # The fleet-wide cards read the whole roster, so they are gated on
+        # endpoints.fleet, not the per-endpoint endpoints.view a guest may hold.
+        assert widget['permission'] == 'endpoints.fleet'
+
+
+def test_endpoints_navigation_item_points_at_fleet_view():
+    """The fleet list view is exposed as a single /endpoints nav item."""
+    plugin = EndpointsPlugin()
+    items = plugin.get_navigation_items()
+    routes = {item['route'] for item in items}
+    assert '/endpoints' in routes
+
+
+def test_system_restart_is_allowlisted_and_policy_selectable():
+    """Restart is a real allowlisted action, so the policy builder (which
+    reads COMMAND_ALLOWLIST) surfaces it as a selectable action."""
+    assert is_allowed_action('system_restart')
+    assert 'system_restart' in COMMAND_ALLOWLIST
+    assert COMMAND_ALLOWLIST['system_restart']['label'] == 'Restart computer'
+
+
+def test_system_restart_requires_the_elevated_restart_permission():
+    """Restart maps to endpoints.restart so the enqueue route can demand a
+    heavier grant than the routine endpoints.command toggle. Routine actions
+    carry no elevated requirement."""
+    assert elevated_permission_for('system_restart') == 'endpoints.restart'
+    assert elevated_permission_for('mtconnect_start') is None
+
+
+def test_endpoints_declares_the_restart_permission():
+    """The dedicated restart permission is declared for role-grid seeding."""
+    plugin = EndpointsPlugin()
+    permissions = {name for name, *_ in plugin.get_permissions()}
+    assert 'endpoints.restart' in permissions
+    assert 'endpoints.command' in permissions
+
+
+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
+    needs no elevated grant beyond endpoints.command."""
+    assert is_allowed_action('collect_now')
+    assert 'collect_now' in COMMAND_ALLOWLIST
+    assert not requires_target('collect_now')
+    assert elevated_permission_for('collect_now') is None
+
+
+def test_run_script_and_update_app_are_allowlisted_delivery_actions():
+    """The two software-delivery actions are allowlisted with a targetkind so
+    the enqueue route accepts them and carries their subject on the wire:
+    run_script carries a script name, update_app carries an appversionid."""
+    for action in ('run_script', 'update_app'):
+        assert is_allowed_action(action)
+        assert action in COMMAND_ALLOWLIST
+        assert requires_target(action)
+    assert COMMAND_ALLOWLIST['run_script']['targetkind'] == 'script'
+    assert COMMAND_ALLOWLIST['update_app']['targetkind'] == 'app'
+
+
+def test_delivery_actions_require_the_elevated_deploy_permission():
+    """run_script and update_app both demand endpoints.deploy ON TOP OF
+    endpoints.command, so pushing software is a strictly heavier grant than a
+    routine service/process toggle."""
+    assert elevated_permission_for('run_script') == 'endpoints.deploy'
+    assert elevated_permission_for('update_app') == 'endpoints.deploy'
+
+
+def test_endpoints_declares_the_deploy_permission():
+    """The dedicated software-delivery permission is declared for role seeding."""
+    plugin = EndpointsPlugin()
+    permissions = {name for name, *_ in plugin.get_permissions()}
+    assert 'endpoints.deploy' in permissions
+
+
+def test_known_payload_keys_cover_persisted_and_relayed_fields():
+    """The ingest allow-set must list every key the handler consumes today, so
+    those keys are never mislogged as 'unpersisted'. Anything the agent sends
+    outside this set is logged for observability before columns exist."""
+    from plugins.endpoints.plugin import KNOWN_PAYLOAD_KEYS
+
+    persisted_or_relayed = {
+        'hostname', 'adopt', 'loggedinuser', 'serialnumber', 'pctype',
+        'machinenumber', 'vendorname', 'modelnumber', 'osname', 'ipaddress',
+        'agentversion', 'lastboottime', 'uptimeseconds', 'idleseconds',
+        'cpupercent', 'memorypercent', 'services', 'ports', 'interfaces',
+        'scheduledtasks', 'events',
+    }
+    assert persisted_or_relayed <= KNOWN_PAYLOAD_KEYS
+
+
+def test_new_hardware_telemetry_keys_are_logged_not_silently_known():
+    """Disk/bios/memory inventory is collected at the agent before the server
+    persists it. Those keys must be OUTSIDE the known set so ingest logs them
+    rather than dropping them silently."""
+    from plugins.endpoints.plugin import KNOWN_PAYLOAD_KEYS
+
+    for key in ('disks', 'bios', 'memory', 'processor', 'osinfo', 'domain'):
+        assert key not in KNOWN_PAYLOAD_KEYS
+
+    # 'applications' USED to be in this list. It is persisted now - that is what
+    # the installed-software work added - so it belongs in the known set, and
+    # asserting its absence would fail the moment the feature it tests works.
+    assert 'applications' in KNOWN_PAYLOAD_KEYS
+
+
+def test_telemetrymeta_route_is_registered_for_freshness_gating():
+    """The WebUI gates panel refreshes on lastreportedat advancing, so the
+    lightweight freshness probe must be exposed on the plugin blueprint."""
+    from flask import Flask
+
+    application = Flask(__name__)
+    # A url_prefix is REQUIRED here: the fleet list registers as route(''), which
+    # Flask rejects as a bare rule and accepts under a prefix. That is deliberate
+    # - CONTRIBUTING says API routes carry no trailing slash - so the test has to
+    # mount the blueprint the way the app does.
+    application.register_blueprint(EndpointsPlugin().get_blueprint(),
+                                   url_prefix='/api/endpoints')
+    rules = {rule.rule for rule in application.url_map.iter_rules()}
+    assert any(rule.endswith('/asset/<int:asset_id>/telemetrymeta') for rule in rules)
+
+
+def test_fleet_list_route_resolves_without_trailing_slash():
+    """The Fleet list route must match the slash-less URL the SPA calls.
+
+    The list is the only route on the bare blueprint prefix. If it demands a
+    trailing slash, Flask 308-redirects /api/endpoints to /api/endpoints/, and
+    behind the /shopdb reverse proxy that redirect drops the path prefix and
+    the list call fails while the detail routes (which carry a path suffix)
+    keep working. strict_slashes=False makes both forms resolve directly.
+    """
+    from flask import Flask
+
+    application = Flask(__name__)
+    application.register_blueprint(
+        EndpointsPlugin().get_blueprint(), url_prefix='/api/endpoints')
+    adapter = application.url_map.bind('localhost')
+
+    endpoint, _ = adapter.match('/api/endpoints', method='GET')
+    assert endpoint.endswith('list_endpoints')
+    endpoint_slash, _ = adapter.match('/api/endpoints/', method='GET')
+    assert endpoint_slash.endswith('list_endpoints')
+

tests/test_plugins/test_geenforce_payload_hardening.py

@@ -107,21 +107,47 @@ def test_payload_download_is_rate_limited(client, app, auth_headers):
 
 
 def test_rate_limit_is_per_ip(client, app, auth_headers):
-    """A second caller (different X-Forwarded-For) has its own budget: one IP
-    being throttled must not throttle everyone."""
+    """A second caller has its own budget: one IP being throttled must not
+    throttle everyone.
+
+    Callers are distinguished by REMOTE_ADDR, the peer that actually connected,
+    which is what ProxyFix fills from X-Forwarded-For when a proxy is really in
+    front of this instance.
+    """
     sha = _store(app, b'per-ip blob')
     key = _fetch_key(client, auth_headers)
-    hot = {**key, 'X-Forwarded-For': '203.0.113.202'}
-    cool = {**key, 'X-Forwarded-For': '203.0.113.203'}
+    hot = {'REMOTE_ADDR': '203.0.113.202'}
+    cool = {'REMOTE_ADDR': '203.0.113.203'}
     with _config(app, GEENFORCE_PAYLOAD_RATELIMIT_MAX=1,
                  GEENFORCE_PAYLOAD_RATELIMIT_WINDOW_SECONDS=300):
         assert client.get(f'/api/geenforce/payload/{sha}',
-                          headers=hot).status_code == 200
+                          headers=key, environ_base=hot).status_code == 200
         assert client.get(f'/api/geenforce/payload/{sha}',
-                          headers=hot).status_code == 429
+                          headers=key, environ_base=hot).status_code == 429
         # Fresh IP still gets its first request through.
         assert client.get(f'/api/geenforce/payload/{sha}',
-                          headers=cool).status_code == 200
+                          headers=key, environ_base=cool).status_code == 200
+
+
+def test_rate_limit_cannot_be_reset_by_forging_a_forwarded_header(
+        client, app, auth_headers):
+    """The budget follows the connection, not a header the caller writes.
+
+    The limiter used to key on the first X-Forwarded-For hop, so rotating that
+    header per request bought an unbounded budget and the limit stopped being
+    a limit.
+    """
+    sha = _store(app, b'forged header blob')
+    key = _fetch_key(client, auth_headers)
+    caller = {'REMOTE_ADDR': '203.0.113.210'}
+    with _config(app, GEENFORCE_PAYLOAD_RATELIMIT_MAX=1,
+                 GEENFORCE_PAYLOAD_RATELIMIT_WINDOW_SECONDS=300):
+        assert client.get(f'/api/geenforce/payload/{sha}', headers=key,
+                          environ_base=caller).status_code == 200
+        for hop in ('198.51.100.1', '198.51.100.2', '198.51.100.3'):
+            spoofed = {**key, 'X-Forwarded-For': hop}
+            assert client.get(f'/api/geenforce/payload/{sha}', headers=spoofed,
+                              environ_base=caller).status_code == 429
 
 
 def test_rate_limit_can_be_disabled(client, app, auth_headers):

tests/test_plugins/test_printedparts_plugin.py

@@ -0,0 +1,30 @@
+"""Printedparts plugin smoke tests.
+
+Asserts the plugin loads cleanly and satisfies the framework contract.
+Replace and extend with domain tests as you build the plugin out.
+"""
+
+from plugins.printedparts.plugin import PrintedpartsPlugin
+
+
+def test_printedparts_plugin_meta_is_valid():
+    """PrintedpartsPlugin.meta returns a PluginMeta with the expected name."""
+    plugin = PrintedpartsPlugin()
+    assert plugin.meta.name == 'printedparts'
+    assert plugin.meta.api_prefix == '/api/printedparts'
+
+
+def test_printedparts_plugin_get_blueprint_returns_blueprint():
+    """get_blueprint returns a Flask Blueprint, not None."""
+    from flask import Blueprint
+    plugin = PrintedpartsPlugin()
+    assert isinstance(plugin.get_blueprint(), Blueprint)
+
+
+def test_printedparts_plugin_get_models_returns_a_model():
+    """get_models returns a list with at least one SQLAlchemy model."""
+    plugin = PrintedpartsPlugin()
+    models = plugin.get_models()
+    assert len(models) >= 1
+    for model in models:
+        assert hasattr(model, '__tablename__')

tests/test_plugins/test_supplies_card_ordering.py

@@ -0,0 +1,101 @@
+"""The supplies card must put the emptiest cartridge first.
+
+It sorted on iscritical, then on PRINTER NAME. Critical is everything at or
+below 5%, so a cartridge at 5% and one at 0% tied on the first key and the
+alphabet decided the order.
+
+That is bad ordering on its own, and worse than it looks: the dashboard queue
+takes only the first few rows per source, so a printer that is genuinely empty
+was truncated out of the card while a fuller one held its place. The card
+existed to answer "what do I walk out and change today" and was answering it
+with the alphabet.
+"""
+from unittest.mock import patch
+
+import pytest
+
+
+def _call(client, auth_headers):
+    response = client.get('/api/printers/dashboard/supplies',
+                          headers=auth_headers)
+    assert response.status_code == 200
+    return response.get_json()['data']
+
+
+@pytest.fixture(autouse=True)
+def _clear_cache(app):
+    from shopdb.extensions import cache
+    cache.delete('printers_low_supplies')
+    yield
+    cache.delete('printers_low_supplies')
+
+
+def _printer(printerid, name, remaining, status='critical'):
+    return {
+        'printerid': printerid,
+        'printername': name,
+        'assetnumber': 'A{}'.format(printerid),
+        'location': 'Shop floor',
+        'mapx': None, 'mapy': None, 'levelid': None,
+        'supplies': [{
+            'name': 'Black Toner', 'level': remaining, 'remaining': remaining,
+            'status': status, 'iswaste': False, 'isdrum': False,
+            'partnumbers': [], 'lastseen': None, 'color': 'black',
+            'supplytype': 'toner',
+        }],
+    }
+
+
+def _patched(rows):
+    return patch('plugins.printers.api.asset_routes._get_low_supplies_data',
+                 return_value={'printers': rows,
+                               'summary': {'total_checked': len(rows),
+                                           'low': 0, 'critical': len(rows)}})
+
+
+def test_the_empty_cartridge_comes_before_the_nearly_empty_one(client,
+                                                               auth_headers):
+    """'Alpha' at 5% used to precede 'Zulu' at 0% purely on the name."""
+    with _patched([_printer(1, 'Alpha', 5), _printer(2, 'Zulu', 0)]):
+        data = _call(client, auth_headers)
+
+    order = [row['printername'] for row in data['rows']]
+    assert order == ['Zulu', 'Alpha']
+
+
+def test_rows_are_ordered_by_how_empty_they_are(client, auth_headers):
+    with _patched([_printer(1, 'Alpha', 4),
+                   _printer(2, 'Bravo', 0),
+                   _printer(3, 'Charlie', 2),
+                   _printer(4, 'Delta', 5)]):
+        data = _call(client, auth_headers)
+
+    remaining = [row['worstremaining'] for row in data['rows']]
+    assert remaining == sorted(remaining)
+    assert remaining == [0, 2, 4, 5]
+
+
+def test_a_zero_percent_printer_survives_the_queue_truncation(client,
+                                                              auth_headers):
+    """The dashboard queue keeps only the first few rows per source.
+
+    With five fuller printers whose names sort earlier, the empty one used to
+    fall off the end of the card entirely.
+    """
+    rows = [_printer(index, name, 5) for index, name in enumerate(
+        ['Alpha', 'Bravo', 'Charlie', 'Delta', 'Echo'], start=1)]
+    rows.append(_printer(6, 'Zulu', 0))
+
+    with _patched(rows):
+        data = _call(client, auth_headers)
+
+    # QUEUEPERSOURCE is 5 on the frontend; first place is what matters here.
+    assert data['rows'][0]['printername'] == 'Zulu'
+    assert [r['printername'] for r in data['rows'][:5]].count('Zulu') == 1
+
+
+def test_the_name_still_breaks_a_tie(client, auth_headers):
+    with _patched([_printer(1, 'Bravo', 3), _printer(2, 'Alpha', 3)]):
+        data = _call(client, auth_headers)
+
+    assert [row['printername'] for row in data['rows']] == ['Alpha', 'Bravo']