Files
shopdb-flask/docs/wiki-drafts/Security-Audit-2026-09-11.md
cproudlock f9d6b57f0f Move the documentation to the line that ships, and correct it against the code
The docs lived on one lineage and the fleet code on another. That is why six
pages described a shared fleet key as how an agent authenticates, why
COLLECTOR-AUTH had to warn that the per-PC credential code was not in the
repository it sat in, and why ADR-019 recorded a decision whose implementation
was somewhere else. Documentation that cannot see the code it documents goes
stale by construction, and no amount of editing fixes that.

So the corpus moves here: 84 files, 56 pages, 20 ADRs, the proposals, INDEX.md,
llms.txt and both gates. Filenames are unchanged, because a wiki page name is
derived from the filename and a rename breaks every link anybody saved.

THE POINT OF THE MOVE IS THE CORRECTION IT ENABLES. Every caveat that said "not
in this tree" was false the moment the files landed beside the code, so each was
rewritten from what is actually here:

- COLLECTOR-AUTH gained the real enroll request body, the two bounds on the
  unauthenticated route (per-IP rate limit, pending cap with stale rows dropped
  before a real PC is refused), and the rejected pending reason that binds to
  the endpoint identity rather than the nonce.
- ADR-019 now describes an implementation that is present, including its tests,
  and says plainly that what remains is the FLEET migration, not the code.
- proposals/device-identity claimed phase 1 was unbuilt. plugins/computers
  declares deviceid on the collector schema and prefers it over the older
  fallbacks. Corrected in the page, its summary and the index.

WINDOWS/IIS IS NOW DOCUMENTED AS THE LEGACY PATH, per the product decision, and
carefully: it is what production runs today. Every Windows page opens by saying
it is supported, not for a new site, and where a new site should go instead. No
procedure was cut - an operator on an IIS box at 2am needs these pages to work
exactly as well tomorrow. MIGRATE-TOPOLOGY stops being a footnote and becomes
the off-ramp, which is why it nearly doubled: what was there did not match how
the Docker stack really works here.

The Docker material that already lived on this line was kept and given
ownership rather than buried under the ported Windows-centric prose: DEPLOY now
describes the four services, the migrate service that runs the upgrade chain
before api starts, TLS terminated by nginx, and the plugin count corrected from
thirteen to seventeen - a number both lineages had wrong.

ROADMAP is rewritten from evidence rather than memory: what has shipped, read
out of the changelog, the ADRs and the manifests, and ahead of it exactly two
items - finalize a distributed release and polish the setup wizard.

PROJECT-MAP is generated and was stale; the GENERATOR was what was wrong.
--check passes now.

API-REFERENCE is retired to a pointer. /api/docs serves 573 operations with
derived auth, query parameters and harvested examples, and a hand-written page
competing with that is a liability. The file stays so its wiki page and its
inbound links survive.

AND THE MOVE TURNED ON A SCRUB THAT HAD NEVER RUN HERE. test_docs_publishable
keys on docs/*.md, which this line did not have, so it skipped - its own comment
predicted what would surface. Porting the corpus activated it and it found 48
lines that must not reach a published wiki: branch names abbreviating the site,
assistant-tooling paths, the site name in two deployment pages, and an employee
ID in a Windows path that the scrub does not even look for.
2026-09-19 09:10:15 -04:00

50 KiB

Security Audit - September 11, 2026, refreshed September 18, 2026

Read with the date on it. This is a point-in-time audit, kept as the record of what was found and what it turned out to be, not as current guidance. One thing it says repeatedly is no longer true: it was written when this line carried no ADRs, and the full ADR set now lives beside it in docs/adr. Where this page says a decision record is missing or lives elsewhere, look there first.

Scope: Endpoints plugin, C# agent, system tray application, and (new on 2026-09-18) the USB and printed-parts kiosks
Branch reviewed: the deployment branch at 35d4fcd41 (2026-09-16). Range re-read for this refresh: 2ac2e6d33..HEAD, 38 commits, 2026-09-11 to 2026-09-16
Severity, 2026-09-11 set: 4 critical, 6 high, 8 medium
Severity, added 2026-09-18: 1 high, 4 medium, 9 low
Status: see the remediation summary. Every 2026-09-11 finding below now carries a verdict read off the code on this branch, with a file and line.

This page covers two dates. The findings numbered 1 to 18 were raised on 2026-09-11 and their status lines were rewritten on 2026-09-18. The findings numbered 19 and up were raised on 2026-09-18 and did not exist on the earlier page.

What changed since 2026-09-11

Two programmes landed in this range and both move the security boundary.

Per-PC agent credentials. The agent no longer has to authenticate with one fleet-wide shared key. Each PC can hold its own secret: plugins/endpoints/models/credential.py:36 stores only a sha256 hash and a 12-character display prefix, plugins/endpoints/api/agentauth.py makes X-Endpoint-Credential the agent auth header, and plugins/endpoints/api/enrollment.py:289 adds an unauthenticated /enroll that a PC uses to ask for one. On the endpoint the secret is a DPAPI LocalMachine blob written through a SYSTEM and Administrators only DACL (agent/ShopDbEndpointAgent.Core/AgentCredentials.cs:203, agent/ShopDbEndpointAgent.Core/ProtectedFileSystem.cs:219). Once a PC holds an active credential the server refuses the shared key for it (plugins/endpoints/api/agentauth.py:129). The operator sequence for the cutover is plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md.

Kiosk network admission. Commit da91a2915 added a second way for an unauthenticated kiosk caller to be admitted: a source address inside a configured CIDR list, alongside the existing per-device token. Both the USB kiosk (plugins/usb/api/routes.py:798) and the printed-parts kiosk (plugins/printedparts/api/routes.py:551) call request_ip_allowlisted, backed by the new shopdb/utils/ipallowlist.py. Both lists default to empty, which matches nothing (shopdb/utils/ipallowlist.py:44), and neither can override the kiosk's own enabled flag. This widens an unauthenticated write surface from a handful of provisioned screens to every PC on the named networks, so the correctness of the client IP the server sees now matters more than it did.

Also in range: the agent's server certificate pinning was completed, the tray stopped holding secrets, a one-line bootstrap install was added, and the 2026-09-12 audit reports were deliberately removed from the product tree (8eee6f19d) because they described weaknesses in detail on the branch that deploys to production. This page keeps that spirit: findings are located precisely enough to fix, and no exploit recipe is written out.

Executive Summary

The headline of the 2026-09-11 page, unauthenticated inventory writes, is closed on the ingest routes that actually exist on this branch: POST /api/collector/<pluginname> requires either a per-PC credential or a collector key, and neither branch of shopdb/core/api/collector.py:251 reaches the plugin without one.

The 2026-09-11 page's second headline, the collector key sitting unprotected in the registry, was still open when this review ran and was fixed the same day: the generated installer now restricts HKLM\SOFTWARE\GE\ShopDB to SYSTEM and Administrators in both views (finding 19). The key itself stays on purpose until every consumer is on a per-PC credential, and bays installed before this change keep the old DACL until they are reinstalled or upgraded. The per-PC credential work in this range is the exit from the shared key entirely, but only for PCs that have enrolled.

The third headline, no certificate validation in the agent, is fixed and was fixed properly, not worked around.

Critical Findings (P0)

1. Unauthenticated Inventory Updates [CLOSED, ON THE ROUTES THAT EXIST]

2026-09-18 verdict: closed, but not by the mechanism the 2026-09-11 page described, and the evidence it published cannot be reproduced.

None of the four paths named in the original fix (/api/endpoints/inventory, /api/endpoints/heartbeat, /api/endpoints/{id}/apps, /api/endpoints/{id}/snapshots) exists on this branch. Endpoint ingest is the ADR-006 generic collector: @collector_bp.route('/<pluginname>', methods=['POST']) at shopdb/core/api/collector.py:229. That route admits a caller in exactly two ways and neither is anonymous:

  • a per-PC secret in X-Endpoint-Credential, resolved to an asset id or rejected 401 CREDENTIAL_INVALID (shopdb/core/api/collector.py:255-264)
  • otherwise _check_collector_auth, which takes a per-plugin or shared collector key, or a collector.ingest scoped managed token, from the header only (shopdb/core/api/collector.py:154)

A credential caller also cannot write for another PC: the three schema-declaring plugins raise PermissionError on an asset mismatch and the dispatcher maps it to 403 ASSET_MISMATCH (shopdb/core/api/collector.py:294, plugins/computers/plugin.py:729, plugins/printers/plugin.py:556, plugins/endpoints/plugin.py:1422).

What is still open: four legacy collector routes take the shared key alone and are not covered by the new refusal. See finding 20.

Residual risk, restated: the original text said device tokens sit in agent config files protected by Windows ACLs. On this branch the per-PC credential is not in a config file at all; it is a DPAPI LocalMachine blob in %ProgramData%\ShopDB\EndpointAgent\credential written through a protected DACL (agent/ShopDbEndpointAgent.Core/AgentCredentials.cs:207-215). The fleet shared key is the one that is still readable, and that is finding 19.

2. Deployment Credential Exposure in Registry [STILL OPEN]

2026-09-18 verdict: still open, and the described fix did not ship. Promoted to the highest open severity and restated in full as finding 19.

The 2026-09-11 page said the deployment script would store the token in an ACL'd config file, remove the registry key, and have the agent read the file rather than the registry. On this branch:

  • the generated installer still writes the key to the registry with no ACL: plugins/endpoints/api/siteagent.py:328, through a helper that does only CreateSubKey plus SetValue in both views (plugins/endpoints/api/siteagent.py:394-411). There is no SetAccessControl or RegistrySecurity anywhere in that file.
  • the collection script still reads the key back out of the registry: plugins/computers/client/Report-AssetToShopDB.ps1:367-375, both the native and the WOW6432Node view.
  • the registry path in the original write-up (HKLM\SOFTWARE\ShopDB\Agent, value CollectorApiKey) never matched this product. The real location is HKLM\SOFTWARE\GE\ShopDB, value CollectorKey.

What did change, and it matters: there is now a keyless build. When the package is built without the shared key the installer emits no -CollectorKey parameter and no key write at all (plugins/endpoints/api/siteagent.py:331-334), and the PC enrolls for its own credential instead. The rollout runbook plans to clear CollectorKey from both registry views at cutover (plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md:351-361). So the exposure ends when the fleet finishes enrolling, not before.

Note on the registry generally: the same key is also where the agent's server trust pins live, by design (plugins/endpoints/client/Set-ShopDBServerPins.ps1:4-20, precedence stated at plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md:106-111). Pins are trust anchors, not secrets. Do not read "get the key out of the registry" as "the registry must hold nothing".

3. No Certificate Validation in Agent [FIXED]

2026-09-18 verdict: fixed, and fixed by pinning rather than by re-enabling default validation and hoping.

agent/ShopDbEndpointAgent.Core/AgentHttpFactory.cs is the only place the agent constructs an HTTP client, and every channel goes through pins.Validate(cert, errors, channel) (AgentHttpFactory.cs:52). Validate (AgentHttpFactory.cs:291) has no path that returns true for an unmatched pin: unpinned falls back to SslPolicyErrors.None (AgentHttpFactory.cs:302), pinned requires an SPKI sha256 or legacy thumbprint match (AgentHttpFactory.cs:314). A failed registry re-read returns null and keeps the existing pins rather than silently unpinning.

The three SYSTEM PowerShell reporters refuse to send at all when pins are configured but the check cannot be enabled, including under pwsh (plugins/computers/client/Report-AssetToShopDB.ps1:228-247, and the same block in plugins/printers/client/Report-PrintersToShopDB.ps1:125 and plugins/endpoints/api/templates/upgrade-shopdbagent.ps1:110).

The blanket accept-any-certificate callback the original finding quoted survives in exactly one place, a developer connectivity probe that is not shipped to endpoints: test-vm-connectivity.ps1:44.

4. Hardcoded Admin Credentials [PARTLY FIXED]

2026-09-18 verdict: partly fixed. The production path is blocked; the credential is still in the source tree.

flask seed test-user still creates admin / admin123 (shopdb/cli/__init__.py:636), and the string appears four times in that file. What changed is that the command now refuses to run outside DEBUG or TESTING and says why (shopdb/cli/__init__.py:619-623), pointing the operator at flask seed admin, which generates a password.

The file the original finding quoted, tests/fixtures/users.py, does not exist on this branch, so the described fix to it cannot be confirmed.

Residual risk: unchanged. A dev-only well-known credential still ships in the tree, and git history still holds the older forms.

High Priority Findings (P1)

5. Collector Key Exposure in Committed Files [FIXED, ONE CLAIM CORRECTED]

2026-09-18 verdict: the redaction holds. git ls-files | xargs grep -l "shopdb_pat_" returns nothing on this branch, and the assistant settings file, the assistant local-settings file and the assistant tool-config file are ignored (.gitignore:174-176).

Correction: the original page's prevention line is not true of this branch. The committed pre-commit hook runs the naming and style gate only (.githooks/pre-commit:5), and scripts/check-naming-and-style.sh has no credential pattern check at all. Its eleven checks are non-ASCII, banned shorthand, three snake_case rules, frontend API params, plugin frontend imports, the shared code-library module, ADR-017 map levels and ADR-015 site literals (scripts/check-naming-and-style.sh:151-395). Nothing blocks a commit that reintroduces a key.

6. Password Exposure in MCP Config [FIXED AS FAR AS THIS BRANCH GOES]

2026-09-18 verdict: the assistant tool-config file is ignored (.gitignore:176) and is not in git ls-files. The accepted risk recorded in 2026-09-11, that the exposed dev passwords were not rotated, is unchanged and still accepted.

7. Missing Rate Limiting on Login [FIXED]

2026-09-18 verdict: fixed, in two layers.

  • Per-IP fixed-window limiter: _login_ratelimited at shopdb/core/api/auth.py:35, applied at shopdb/core/api/auth.py:80, defaults 30 hits per 300 seconds.
  • Per-account lockout: after MAX_FAILED_LOGINS consecutive bad passwords the account is locked (shopdb/core/api/auth.py:113-118), and a locked account is rejected before the password is checked so a valid password cannot reset the clock mid-lockout (shopdb/core/api/auth.py:100).

Known limit, stated in the code: under the default SimpleCache the limiter's counter is per-process, so with N gunicorn workers the effective budget is N times the configured maximum (shopdb/core/api/auth.py:39-42). The per-account lockout is the control that does not have that weakness. Both are keyed on the client address, which is why the client-IP plumbing in finding 23 matters.

8. SQL Injection in Asset Search [STILL A FALSE POSITIVE]

2026-09-18 verdict: re-checked, still no injection.

The raw SQL that exists is parameterised. The USB external-database path builds a WHERE clause out of fixed fragments and passes every value as a parameter (plugins/usb/api/routes.py:277-299); the only interpolated text is the fragment list itself. plugins/employees/api/routes.py:113 binds through %s.

One related note that is not injection: plugins/computers/plugin.py:2080 runs a raw db.text query joining the core assets, assetrelationships and assettypes tables inside the flask computers retype CLI. No user input reaches it, so it is not a security finding. It is hidden coupling to the core schema, which is an architecture item, not one for this page.

9. Session Token Storage in LocalStorage [UNCHANGED]

2026-09-18 verdict: unchanged. The JWT and the refresh token are still written to and read from localStorage (frontend/src/stores/auth.js:6-7, frontend/src/stores/auth.js:43-44). No HttpOnly cookie path was added in this range.

Partial compensating control added since: the app now sets X-Content-Type-Options, X-Frame-Options, Referrer-Policy and a frame-ancestors 'self' CSP on every response (shopdb/__init__.py:334-341). That is not a script-src policy and the code says so deliberately.

10. Audit Log Unbounded Growth [MECHANISM BUILT, OFF BY DEFAULT]

2026-09-18 verdict: the tooling now exists; it does nothing unless an operator turns it on and runs it.

  • Service: shopdb/core/services/auditretention.py
  • Admin API: GET /api/auditlogs/retention (shopdb/core/api/auditlogs.py:195) and the purge route at shopdb/core/api/auditlogs.py:211
  • CLI: shopdb/cli/__init__.py:1894 and shopdb/cli/__init__.py:1918

configureddays() returns 0 when audit_retentiondays is unset, and 0 means keep forever (shopdb/core/services/auditretention.py:123-133). There is no scheduler in that module, so nothing purges on its own. The row and size figures quoted in 2026-09-11 come from the reference site database and cannot be confirmed from the repository.

Medium Priority Findings (P2)

11. Endpoint Event Accumulation [STILL OPEN]

2026-09-18 verdict: still open. EndpointEvent is endpointevents (plugins/endpoints/models/endpoint.py:333) and there is no retention, purge or prune code anywhere under plugins/endpoints/. The audit-log retention service added in this range covers auditlog only. The row and size figures quoted in 2026-09-11 are site measurements and are not verifiable here.

12. Missing HTTPS Enforcement in Dev [CHANGED; THE STATED MITIGATION IS GONE]

2026-09-18 verdict: the specific mitigation the page described no longer exists, and that is an improvement. NODE_TLS_REJECT_UNAUTHORIZED appears nowhere in the tree. The Vite dev proxy target is now an environment variable defaulting to http://localhost:5001, and TLS verification for an https target is off unless VITE_DEV_SECURE=true (frontend/vite.config.js:5-18). Dev only; production still terminates TLS in front of the app.

13. Docker Log Rotation Not Configured [STILL OPEN]

2026-09-18 verdict: still open. There is no logging: block anywhere in docker-compose.yml. Both hops log to stdout (nginx.conf:36, docker-compose.yml:213-214), so the host's Docker log driver defaults apply unchanged. This now has a second consequence: see finding 25.

14-18. Additional Medium Findings [PARTLY ADDRESSED, SOURCE NOW MISSING]

2026-09-18 verdict: the file this section pointed at is gone. That assistant memory directory does not exist on this branch; commit 8eee6f19d moved the 2026-09-12 audit reports and scan output out of the product tree on purpose. The detail behind findings 14 to 18 is therefore not readable from here.

Of the five items summarised in the original bullet list, two can be answered from the code:

  • No content security policy headers: partly addressed. shopdb/__init__.py:334-341 sets a security header block including Content-Security-Policy: frame-ancestors 'self', and uploads carry a much stricter default-src 'none'; sandbox (shopdb/utils/uploads.py:13, applied at shopdb/utils/uploads.py:35). A real script-src policy is still not set, deliberately.
  • Directory listing enabled in nginx: no autoindex directive appears in nginx.conf, and nginx defaults to off.

The remaining three, input validation gaps, world-readable backup files and old TLS versions, cannot be confirmed or refuted without the removed source document. Treat them as unverified rather than as fixed.

New findings, 2026-09-18

High

19. The fleet shared key is written to the registry with no ACL (FIXED 2026-09-18; the key itself is transitional by design)

Risk: for a shared-key build, the generated install.ps1 writes the collector key to HKLM\SOFTWARE\GE\ShopDB\CollectorKey (plugins/endpoints/api/siteagent.py:328) through a helper that does only CreateSubKey plus SetValue in both registry views (plugins/endpoints/api/siteagent.py:394-411). No RegistrySecurity, no SetAccessControl. The key inherits read for BUILTIN\Users. The repository states this outcome itself, in a file added in this same range: .github/GE-ENFORCE-COLLECTOR-FIX.html:377-380.

Why it is still authority: authenticate_agent accepts X-API-Key whenever shared_key_allowed() (plugins/endpoints/api/agentauth.py:184-192, default true at plugins/endpoints/api/agentauth.py:67), and shared_key_refused_for refuses it only for a hostname whose asset holds an active credential (plugins/endpoints/api/agentauth.py:129-134). So for any not-yet-enrolled, revoked or re-enroll-blocked PC the shared key still claims queued commands (plugins/endpoints/api/routes.py:3352) and downloads script bodies and installer packages (plugins/endpoints/api/routes.py:3511, :3550, :3589).

Impact: any operator logged on to a bay can read the fleet key and act as any un-enrolled endpoint: claim and answer its queued commands, write false compliance records, and fetch hosted installers and script bodies. It also means revoke plus re-enroll-block leaves a compromised PC holding a working credential of the older kind.

Contrast: the per-PC credential got the opposite treatment in the same range. DPAPI LocalMachine inside a SYSTEM and Administrators only DACL (agent/ShopDbEndpointAgent.Core/AgentCredentials.cs:203-217, agent/ShopDbEndpointAgent.Core/ProtectedFileSystem.cs:219-232), and appsettings.json is re-ACLed at runtime (agent/ShopDbEndpointAgent/AgentPipeServer.cs:494-522). The registry key alone was left open.

Status, 2026-09-18: fixed. The generated installer now calls Protect-HklmShopdbKey after provisioning, which breaks inheritance, drops the inherited entries rather than copying them, and states SYSTEM (S-1-5-18) and Administrators (S-1-5-32-544) by SID in both registry views (plugins/endpoints/api/siteagent.py). It runs for the key-less package too, because ServerPins live in the same key and the registry is authoritative for pinning. A bay that cannot take the ACL still installs, and says plainly that the key stays readable there. Four tests cover it in tests/test_plugins/test_endpoints_siteagent_build.py, including the write-then-lock ordering.

The key's continued existence is a decision, not an oversight. The shared key stays until upgrade-shopdbagent.ps1, Report-AssetToShopDB.ps1 and Report-PrintersToShopDB.ps1 are off it, because rollback dies without it; the site is mid-migration to per-bay credentials and the key-less package (includesharedkey=False) is already the path for new installs. What this finding was about was the DACL, and that is now closed. The remaining work is the migration itself, and finally clearing CollectorKey from both views as the runbook plans (plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md:351-361).

Not closed by this: bays that already carry the key keep their inherited DACL until they are reinstalled or upgraded through a package built after this change.

Medium

20. Turning the shared key off does not close the four legacy collector routes

generic_collect enforces the new rule: 401 SHARED_KEY_REFUSED when agent_shared_key_refused(identityvalue) is true (shopdb/core/api/collector.py:279-284), which plugins/endpoints/api/agentauth.py:129-134 makes true fleet-wide once plugin.endpoints.sharedkeyallowed is off.

Four legacy routes never consult it. /pc (shopdb/core/api/collector.py:349), /apps (:387), /heartbeat (:459) and /bulk (:494) are decorated @require_api_key, which checks only COLLECTOR_API_KEY or a collector.ingest scoped token (shopdb/core/api/collector.py:154-164). /pc then writes lastreporteddate, loggedinuser, lastboottime and the asset serial number for any hostname it can resolve (shopdb/core/api/collector.py:367-376); /apps and /bulk rewrite installed-app inventory.

Two details that make this worse than the runbook implies. _env_fallback_disabled(pluginname) returns False when pluginname is None (shopdb/core/api/collector.py:60-74), so these routes keep honouring the .env COLLECTOR_API_KEY even after plugin.endpoints.envkeyfallback is switched off. And "Regenerate key" mints a new collector.ingest.endpoints token and clears the env fallback but never changes COLLECTOR_API_KEY (plugins/endpoints/api/routes.py:1864-1884), so runbook Phase 6 step 4 does not close this either.

Bounded by: the UI-managed agent key is scoped collector.ingest.endpoints (plugins/endpoints/api/routes.py:1834) and _collector_scopes_for(None) allows only the generic collector.ingest (shopdb/core/api/collector.py:46-57), so a PC holding only that token is already refused on all four routes. The exposure needs a copy of the env shared key, and the reachable damage is inventory-field integrity. No command issuance and no credential minting.

ADR-006 says the legacy /pc endpoint is removed before v1.0. At __version__ 0.12.0 keeping it is not yet a violation, but it is a hole in a boundary this branch advertises.

21. Rejecting an enrollment request is not durable

enroll_endpoint is unauthenticated by design. When a row for (hostname, serialnumber) already exists, it returns rejected only when the presented nonce matches the stored one (plugins/endpoints/api/enrollment.py:383). A REJECTED row presented with a different nonce falls into the non-pending branch at plugins/endpoints/api/enrollment.py:390 and is rewritten into a fresh cycle: new nonce, new requestedat, decidedat and decidedby cleared (:393-396). Execution then falls through to the match block, and with the fleet shared key present it is auto-issued a credential outright (plugins/endpoints/api/enrollment.py:426-427). Both gates default on (plugins/endpoints/api/agentauth.py:67, :71). Without the key the request simply lands back in the admin approval queue, indistinguishable from a first-time request.

There is no second lever: reject_enroll_request sets no durable flag (plugins/endpoints/api/enrollment.py:753-770), and reenrollblocked lives on EndpointCredential (plugins/endpoints/models/credential.py:56), which revoke_endpoint_credential 404s on when no credential row exists (plugins/endpoints/api/enrollment.py:801-805). So a PC that never enrolled cannot be blocked.

Bounded by: auto-issue additionally requires a Computer record whose hostname matches and whose Asset serial matches case-insensitively (plugins/endpoints/api/enrollment.py:405-421), so this does not grant a credential for an arbitrary asset. The existing test covers only the same-nonce case (tests/test_plugins/test_endpoints_credentials.py:250).

22. The one-line bootstrap stages its script in a world-writable path and runs it elevated

COMMAND_SCRIPT_PATH is a literal C:\Windows\Temp\shopdb-bootstrap.ps1 (plugins/endpoints/api/bootstrap.py:68), and the emitted command downloads to that path and then executes it, with a Remove-Item -Force against the same path in the finally block (plugins/endpoints/api/bootstrap.py:164-169). The module docstring and the script header both say the line is pasted into an elevated PowerShell (plugins/endpoints/api/bootstrap.py:3-5, :209-213). The same literal is published in plugins/endpoints/AGENT-CONTRACT.md:794 and plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md:296.

C:\Windows\Temp is not admin-only: its default DACL grants BUILTIN\Users create-file plus CREATOR OWNER full control on what they create, and overwriting a file's contents does not reset its owner or DACL. Nothing verifies the script after download; the sha256 checks in the script body cover the downloaded ZIP's binaries, not the script itself.

Bounded by: this needs an attacker who can already log on interactively to that specific bay and a tech who later pastes the one-liner on that same PC. The bootstrap path is documented as the one-off route (plugins/endpoints/api/bootstrap.py:7-11), not how managed bays get the agent.

Fix direction: stage under a path only Administrators and SYSTEM can write, or verify the script against a server-stamped hash before executing it.

23. Kiosk network admission rests on a header the client can send, and proxy mode verifies nothing

request_ip_allowlisted reads client_ip(), which is strip_port(request.remote_addr) (shopdb/utils/clientip.py:53). On the Windows deployment remote_addr is not the TCP peer: deploy/windows/web.config:31 launches waitress with --trusted-proxy=127.0.0.1 --trusted-proxy-headers=x-forwarded-for --trusted-proxy-count=1, and HttpPlatformHandler always connects from loopback, so waitress always parses X-Forwarded-For and promotes the selected hop into REMOTE_ADDR.

The control that makes that hop honest is the IIS rewrite rule that overwrites the header with {REMOTE_ADDR}, and it ships commented out (deploy/windows/web.config:90-102). The installer enables and then verifies it only under -ClientIpSource direct: the whole "CLIENT-IP RULE IS NOT IN EFFECT" check sits inside if ($ClientIpSource -eq 'direct') at deploy/windows/installer/shopdb-install.ps1:2509-2523. In proxy mode the rule is deliberately left off (:2041-2046) and nothing checks that a proxy is actually in front. The module docstring says the same thing about the failure mode: it does not fail closed (shopdb/utils/ipallowlist.py:17-26).

Bounded by: -ClientIpSource defaults to direct (deploy/windows/installer/shopdb-install.ps1:129), which installs URL Rewrite, uncomments the block and verifies it live, so a default install is covered. With a real proxy in proxy mode the attack also fails, because waitress takes the last hop and ARR appends the true peer after any client-supplied value. Both kiosk allowlists default to empty and both kiosks are additionally gated on their enabled flag (plugins/usb/api/routes.py:798-801, plugins/printedparts/api/routes.py:551-557).

The real gap: proxy mode performs no verification at all. A server set to proxy mode with no proxy really there, or a hand-maintained web.config where the rewrite block was never uncommented, admits an attacker-chosen address. The same header also satisfies the GE-Enforce IP path (plugins/geenforce/api/routes.py:82-108), which is pre-existing and not introduced by da91a2915, and it re-keys the login rate limit and account lockout on a value the caller chose.

Low

24. Revoke does not stop a live agent; it falls back to the shared key

plugins/endpoints/models/credential.py:7-10 says reenrollblocked keeps a revoked PC from enrolling again, listing "a compromised PC" among the uses. The containment does not exist. On a 401 the agent deletes its credential and clears it (agent/ShopDbEndpointAgent.Core/AgentCredentials.cs:107-135), then every later request takes the shared-key branch (agent/ShopDbEndpointAgent.Core/AgentHttpFactory.cs:119-120). Server side, shared_key_refused_for refuses the shared key only when an active credential exists (plugins/endpoints/api/agentauth.py:129-134), and revoke has just cleared that (plugins/endpoints/api/enrollment.py:773-825).

This is documented intended behaviour, not an undetected gap: plugins/endpoints/api/agentauth.py:11-14 states it, and plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md:379-380 says outright that the server admits the shared key again. What needs fixing is the model comment, which implies a containment the design never provides. There is no per-PC deny until the shared key is off fleet-wide.

25. The bootstrap token travels in the query string and is written to both access logs

_resolve_token takes the whole authorization from the URL (plugins/endpoints/api/bootstrap.py:182) for both token routes (plugins/endpoints/api/bootstrap.py:60-61). Both hops log the full request line: nginx.conf:36 is access_log /dev/stdout combined, and the gunicorn access format uses %(r)s (docker-compose.yml:213-214). The IIS deployment logs cs-uri-query in its W3C log by default too.

The same range deliberately stopped logging the setup token for exactly this reason (shopdb/__init__.py, setup-token block), and the audit rows this module writes carry only tokenprefix (plugins/endpoints/api/bootstrap.py:192-195).

Bounded by: what a replayed token buys is bounded and carries no secret. download_bootstrap_package is explicitly keyless (plugins/endpoints/api/bootstrap.py:532-535) and the script it serves contains only the base URL and pins. The reader also needs container or IIS log access, which is already privileged.

26. The unauthenticated enroll queue can be filled to its cap

enroll_endpoint takes no auth and writes a row for every unseen (hostname, serialnumber) pair (plugins/endpoints/api/enrollment.py:350-358). The bounds are a per-IP fixed window of 300 per 60 seconds (plugins/endpoints/api/enrollment.py:65-66) and a pending cap of 500 (plugins/endpoints/api/enrollment.py:255). Junk rows count toward the cap: an unknown hostname returns _pending(row, REASON_UNKNOWN_HOSTNAME) at plugins/endpoints/api/enrollment.py:411, which leaves the row PENDING. There is no bulk-reject, prune or stale-row expiry in that module. The limiter's counter is per-process (plugins/endpoints/api/ratelimit.py:10-11), so the real budget is the configured maximum times the worker count.

Bounded by: while sharedkeyallowed is on (the default), a PC that cannot enroll is not offline; it keeps reporting on the fleet key. This only becomes an outage for new bays once the shared key is turned off at the end of the rollout.

27. The kiosk confirms an employee name for any badge it is handed

kiosk_session normalises the posted badge and returns the resolved employee name in the response body (plugins/usb/api/selfhosted.py:346-371, resolver at :34-57); the external mode does the same (plugins/usb/api/routes.py:904-960). The route carries no auth decorator (plugins/usb/api/routes.py:905-906) and there is no rate limiter anywhere in either file. kiosk_others additionally returns the holder name for every checked-out drive to the same caller, gated only on usb_kiosk_allow_return_for_others, which defaults true (plugins/usb/plugin.py:169-172).

Before da91a2915 this needed the per-device token, so it was confined to screens an administrator had provisioned. It is now reachable by anything inside usb_kiosk_allowed_cidrs.

Bounded by: the kiosk must be enabled and the caller must be inside a deliberately configured CIDR. The data is badge-to-name, which is printed on the badges themselves and visible to any signed-in user, and the surface is read-only. An unrecognised badge answers 422 rather than silently.

28. The kiosk allowlist settings accept a world-open range with no server-side validation

usb_kiosk_allowed_cidrs (plugins/usb/plugin.py:154-168) and printedparts_kiosk_allowed_cidrs (plugins/printedparts/plugin.py:176-188) are plain string settings with no validator, and the settings PUT path applies none. ip_in_cidrs parses each entry with ipaddress.ip_network(part, strict=False) (shopdb/utils/ipallowlist.py:55), so 0.0.0.0/0 and ::/0 parse and match everything. The only shape check is client-side and cosmetic (frontend/src/components/NetworkAllowlistPicker.vue:84-87), and 0.0.0.0/0 satisfies its regex.

The realistic route in is an administrator who cannot get the allowlist to match, most likely because the client-IP rule is missing and every caller reads as loopback, and who widens the range to make it work.

kiosk_provisioning_link returns /usb-kiosk?token=<token> (plugins/usb/api/routes.py:849) and the printed-parts twin returns /parts-kiosk?token=<token> (plugins/printedparts/api/routes.py:578). Both are gated on settings.edit and both write an audit row first (plugins/usb/api/routes.py:846-848, plugins/printedparts/api/routes.py:574-576). The token then lands in the IIS or nginx access log, browser history and any clipboard history on the admin's PC, and nothing rotates it after a reveal.

The same file elsewhere states the contrasting reasoning: kiosk_session is a POST specifically so the badge does not end up in a proxy access log the way a query string would (plugins/usb/api/routes.py:914-916).

Qualification: the query-string model is not introduced here. It is the mechanism the token setting itself prescribes (plugins/usb/plugin.py:146-151), so the new route only surfaces a URL the product already documented. This is hardening, not a regression.

30. Every deployment warning about the client-IP rule still names only GE-Enforce

Three operator-facing places enumerate what depends on the X-Forwarded-For rewrite rule, and none was updated when the kiosk allowlists were added: deploy/windows/installer/shopdb-install.ps1:2520-2522, deploy/windows/installer/shopdb-preflight.ps1:322-325 and deploy/windows/web.config:51-58. None names usb_kiosk_allowed_cidrs or printedparts_kiosk_allowed_cidrs, which are now the only allowlists in the product gating unauthenticated writes. The dependency is stated only in the settings descriptions (plugins/usb/plugin.py:161-168, plugins/printedparts/plugin.py:183-187), which the person building the server does not read.

One claim not to carry forward: the remediation line offered at deploy/windows/installer/shopdb-install.ps1:2522, "or use token authentication", is correct for the kiosks. The per-device token is a genuine alternative to the allowlist on both.

31. This page, and three other documents, attributed a credential policy to ADR-016 that ADR-016 does not contain

The References section of the 2026-09-11 version of this page described ADR-016 as "Credential Delivery (device token authentication)". That is wrong, and the same misattribution is repeated at docs/wiki-drafts/README.md:316-323, the assistant-instructions file, line 119 and the assistant quick-reference file line 135. Grepping the ADR set for device tokens returns nothing in any ADR.

The worst form is in the SECURITY.md page that docs/wiki-drafts/README.md:429 queues for publication. Under "ADR-016: Credential Delivery" it lists device tokens, personal access tokens and environment variables as the delivery mechanisms and then bans, at docs/wiki-drafts/README.md:323, "Hardcoded in source, registry keys, git history". ADR-016 decides none of that. Its Decision is a fleet credential store with Fernet at rest, a targeting and precedence resolver reusing plugins/geenforce/filters.py, an explain endpoint and GET /api/collector/credentials behind a dedicated credentials.fetch scope. Its own alternatives section recommends registry-only provisioning as a worthwhile first step.

The blanket ban on registry keys also contradicts shipped code on this branch: plugins/computers/client/Report-AssetToShopDB.ps1:367-374 reads the collector key from HKLM:\SOFTWARE\GE\ShopDB, and plugins/endpoints/client/Set-ShopDBServerPins.ps1:4-20 writes the server trust pins there by design.

Status: the References section of this page is corrected below. The other three are not, and the SECURITY.md draft should not be published as written. Note that the deployment branch carries no docs/adr directory at all, so there is no ADR text on this branch to correct a reader who follows the wiki.

Related, and unverifiable from this branch: ADR-016's own mechanism is still unbuilt. Grepping shopdb/ and plugins/ for credentials.fetch, collector/credentials and Fernet returns nothing, and plugins/endpoints/plugin.py:163-253 declares only endpoints.* permissions. The per-PC credential programme in this range is a different subject under the same word: the agent's own identity secret, not a store that delivers third-party credentials to a PC. The design is sound and consistent with ADR-016's intent, but nothing on this branch records it as a decision. That record belongs on the lineage that holds docs/adr.

32. Kiosk check-in records no wipe or scan state, so the new CMMC audit columns stay blank

35d4fcd41 added sanitized and scanned_viruses to the USB audit report on the stated grounds that custody without the state of the media on return is half the story. Both values are written only on the authenticated return path (plugins/usb/api/selfhosted.py:275-280, plugins/usb/api/routes.py:562-571). The kiosk returns write neither: kiosk_checkin updates only check-in time, SSO and name (plugins/usb/api/selfhosted.py:527-531), and the external kiosk insert lists only badge, device, action, timestamp and locker (plugins/usb/api/routes.py:1157-1162).

This is an honest blank, not a false record: auditflag renders the NULL as an empty cell rather than "no" (plugins/usb/api/selfhosted.py:547-559), and the tri-state is deliberate. But da91a2915 has just widened the kiosk from a few provisioned screens to every PC on the allowed networks, so it is about to become the path most returns take.

Correction to note: the signed-in path does not answer the scan question either. USBDetail.vue:226-230 and USBList.vue:267-271 send only sanitized, never scanned_viruses, so plugins/usb/api/routes.py:564 always receives None and that column is blank for every return through any first-party UI.

Verified as sound

Read on 2026-09-18 and found correct. Recorded so the next review does not re-litigate them.

  • Credential storage on the server. Only a sha256 hash is persisted, plus a 12-character display prefix. The secret exists once, in the 201 enroll response (plugins/endpoints/api/enrollment.py:212-222). No admin route re-reads or re-displays it; issue_credential has exactly two callers, both inside enroll_endpoint.
  • Cross-asset binding. A credential caller cannot act for another PC. bind_hostname returns ASSET_MISMATCH for a hostname resolving elsewhere (plugins/endpoints/api/agentauth.py:206-231), and every agent-facing route goes through it.
  • Admin surface authorization. Approve, reject, revoke, allow-reenroll, the settings PUT, the migration wave and all three bootstrap-token routes are @require_role('admin') (plugins/endpoints/api/enrollment.py:642, :755, :773, :827, :852, :1134; plugins/endpoints/api/bootstrap.py:420, :475, :486).
  • Tray holds no secret. The tray reaches the SYSTEM agent over a named pipe with a SYSTEM and Administrators full, INTERACTIVE read-write ACL and FirstPipeInstance anti-squat (agent/ShopDbEndpointAgent/AgentPipeServer.cs:126-146); no pipe operation returns the credential (:246-256).
  • Bootstrap token scope and lifetime. Hashed at rest, expiry mandatory and bounded to 1 to 168 hours, uses bounded to 1 to 500, revocable, and unknown, malformed, expired, revoked and exhausted all answer an identical 404 so the URL confirms nothing (plugins/endpoints/models/bootstrap.py:197-212, plugins/endpoints/api/bootstrap.py:109-111, :176-189). The package it serves never carries the collector key.
  • GE-Enforce payload scoping. /payload/<sha256> enforces the token's resource scope before the ETag shortcut, so a bound token cannot even confirm another scope's hash exists (plugins/geenforce/api/routes.py:252-283).
  • GET /api/plugins/settings-routes is no longer anonymous. It moved to @require_permission('settings.view') (shopdb/core/api/plugins.py:47) and was removed from the public-read allowlist with the reasoning recorded (tests/test_core/test_public_read_surface.py:29-34).
  • The kiosk admission gate fails closed in every direction it claims to. Empty setting matches nothing, an unparseable caller address matches nothing, a malformed CIDR entry is skipped rather than widening the match (shopdb/utils/ipallowlist.py:44-57), and the allowlist cannot override the enabled flag. Device-token comparison uses hmac.compare_digest in both kiosks (plugins/usb/api/routes.py:762, plugins/printedparts/api/routes.py:536). An unadmitted caller gets 404, not 403.
  • No secrets committed in the range. Every sdbe_ and sdbb_ hit in the diff is a prefix constant, a regex, a doc example or a test fixture. certs/ remains ignored except its README, openssl.cnf and the two regeneration scripts.

Performance Findings

Agent Polling Inefficiency [LARGELY ADDRESSED]

2026-09-18 verdict: the behaviour the 2026-09-11 page measured no longer matches the code.

  • The cadence is not 30 seconds. agent/ShopDbEndpointAgent/appsettings.json:5 ships cadenceseconds: 900 with 60 seconds of jitter and 30 seconds of startup jitter, so roughly 96 check-ins per day per endpoint rather than 2880.
  • Application inventory is a delta, not a full rewrite. plugins/endpoints/plugin.py:1187 onward builds an incoming fingerprint map, validates each fingerprint as 64 lowercase hex, and applies only the difference; a genuine unique-key collision is caught inside a savepoint and reconciled row by row (plugins/endpoints/plugin.py:1301-1336, :1359-1410) rather than sinking the check-in.
  • Two hotfixes in this range removed row-level locks and hot-path logging from the collector check-in path (affe455c5, 1ed0c4d21).

The traffic and write-volume figures in the 2026-09-11 text were site measurements against the old cadence and should not be carried forward.

Remediation Summary

Finding Severity Status as of 2026-09-18 Evidence
1. Unauthenticated inventory updates Critical CLOSED on the routes that exist shopdb/core/api/collector.py:251
2. Collector key unprotected in registry Critical STILL OPEN, see finding 19 plugins/endpoints/api/siteagent.py:328
3. No certificate validation in agent Critical FIXED by SPKI pinning agent/ShopDbEndpointAgent.Core/AgentHttpFactory.cs:291
4. Hardcoded admin credentials Critical PARTLY FIXED, dev-only guard added shopdb/cli/__init__.py:619
5. Collector key in git High FIXED; no pre-commit secret scan exists .gitignore:174, .githooks/pre-commit:5
6. Passwords in mcp-servers.json High FIXED on this branch; keys not rotated .gitignore:176
7. Missing login rate limiting High FIXED, limiter plus account lockout shopdb/core/api/auth.py:35, :113
8. SQL injection in asset search High FALSE POSITIVE, re-confirmed plugins/usb/api/routes.py:277
9. Session token in localStorage High UNCHANGED frontend/src/stores/auth.js:6
10. Audit log retention High BUILT, off by default, no scheduler shopdb/core/services/auditretention.py:123
11. Endpoint events retention Medium STILL OPEN, no purge code exists plugins/endpoints/models/endpoint.py:333
12. HTTPS enforcement in dev Medium CHANGED, TLS bypass removed frontend/vite.config.js:5
13. Docker log rotation Medium STILL OPEN docker-compose.yml (no logging block)
14-18. Additional medium findings Medium PARTLY ANSWERED, source document removed shopdb/__init__.py:334, 8eee6f19d
19. Shared key in registry with no ACL High FIXED 2026-09-18 (installed bays until reinstall) plugins/endpoints/api/siteagent.py, Protect-HklmShopdbKey
20. Legacy collector routes ignore the refusal Medium OPEN shopdb/core/api/collector.py:349
21. Enroll rejection is not durable Medium OPEN plugins/endpoints/api/enrollment.py:390
22. Bootstrap stages elevated script in Windows\Temp Medium OPEN plugins/endpoints/api/bootstrap.py:68
23. Kiosk admission spoofable in proxy mode Medium OPEN deploy/windows/installer/shopdb-install.ps1:2509
24. Revoke falls back to the shared key Low BY DESIGN, model comment wrong plugins/endpoints/models/credential.py:8
25. Bootstrap token in query string Low OPEN plugins/endpoints/api/bootstrap.py:182
26. Enroll queue can be filled to cap Low OPEN plugins/endpoints/api/enrollment.py:350
27. Badge to name oracle on the kiosk Low OPEN plugins/usb/api/selfhosted.py:346
28. Allowlist accepts 0.0.0.0/0 Low OPEN plugins/usb/plugin.py:154
29. Provisioning link carries the token in a URL Low OPEN, pre-existing mechanism plugins/usb/api/routes.py:849
30. Client-IP warnings name only GE-Enforce Low OPEN deploy/windows/installer/shopdb-install.ps1:2520
31. ADR-016 misattributed in four documents Low ONE FIXED HERE, three open docs/wiki-drafts/README.md:316
32. Kiosk check-in records no wipe or scan Low OPEN plugins/usb/api/selfhosted.py:463
Agent polling inefficiency Performance LARGELY ADDRESSED agent/ShopDbEndpointAgent/appsettings.json:5

Testing Performed

This refresh was a read of the code at 35d4fcd41, not a live exercise against a server. What was run:

The naming and style gate:

bash scripts/check-naming-and-style.sh
# exit 0, all checks pass

Its ADR-015 site-literal grep is not vacuous: a control pattern over the same six directories returns 1995 lines.

Kiosk network allowlist suite:

pytest tests/test_plugins/test_kiosk_network_allowlist.py
# 20 passed

Tracked-secret scan:

git ls-files | xargs grep -l "shopdb_pat_" | wc -l
# 0

Gate coverage gaps found while testing

  • The ADR-015 site-literal scan does not look at the C# agent. The grep is scoped to plugins/ shopdb/ scripts/ deploy/ frontend/src/ tools/ with --include on .py, .vue, .js, .ps1, .psm1, .sh, .iss, .json and .html (scripts/check-naming-and-style.sh:434-440). agent/ is absent and .cs is not an included extension. Roughly 2500 lines of C# were added under agent/ in this range, all of which ships to a sister site. Re-running the assembled site patterns over agent/ by hand returns one hit, prose in agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj:108, which ADR-015 explicitly permits. The tree is clean; the gate simply would not notice if it stopped being clean. ADR-015's own 2026-08-14 amendment widened the scan on exactly this reasoning, because the literals that reached a second site were in the client scripts and the installer. The C# agent is now that client channel.
  • The gate has no credential pattern check. See finding 5. The 2026-09-11 page's claim that it includes one is not true of this branch.

Recommendations

Do first:

  1. ACL the collector key registry value in the generated installer, or finish enrollment and clear it (finding 19).
  2. Make the shared-key refusal cover /pc, /apps, /heartbeat and /bulk, or retire them (finding 20).
  3. Make an admin rejection durable so a new nonce cannot restart the cycle (finding 21).
  4. Stage the bootstrap script somewhere a standard user cannot pre-create, or verify it against a server-stamped hash (finding 22).
  5. Verify in proxy mode that a proxy is really in front, the way direct mode already verifies the rewrite rule (finding 23).

Then:

  1. Do not publish the SECURITY.md wiki page as drafted; it attributes a policy to ADR-016 that ADR-016 does not contain and bans the registry storage the fleet depends on (finding 31).
  2. Name the two kiosk allowlists in the installer, preflight and web.config warnings (finding 30).
  3. Add server-side validation that rejects a world-open range on both kiosk allowlist settings (finding 28).
  4. Move the bootstrap token out of the query string, or accept it and document the log exposure (finding 25).
  5. Bound or expire pending enrollment rows so the queue cannot be held at its cap (finding 26).
  6. Correct the reenrollblocked comment so it does not claim containment of a compromised PC (finding 24).

Carried over from 2026-09-11 and still open:

  1. Retention for endpointevents (finding 11).
  2. Docker log rotation (finding 13).
  3. A retention value and a schedule for the audit log, now that the tooling exists (finding 10).
  4. HttpOnly cookie sessions for non-kiosk users (finding 9).
  5. Recover or re-derive the detail behind findings 14 to 18, which left the tree with 8eee6f19d.

Extend the gates:

  1. Add agent/ and .cs to the ADR-015 site-literal scan.
  2. Add a credential pattern check to the gate so the pre-commit hook actually blocks a committed key.

References

  • Per-PC credential rollout runbook: plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md
  • Agent contract: plugins/endpoints/AGENT-CONTRACT.md
  • ADR-006: Collector API, the single dynamic dispatch route and its response contract
  • ADR-015: Site-specific configuration, and the 2026-08-14 amendment that set the scan's scope
  • ADR-016: Credential Delivery. A fleet credential store with Fernet at rest and a credentials.fetch scope. It is not about device tokens, and it does not ban registry storage. Still unbuilt on this branch. See finding 31.
  • The ADR set is not in this repository. It lives on the product lineage; the deployment branch has no docs/adr directory.
  • The 2026-09-12 detailed audit reports were removed from the product tree by 8eee6f19d and are kept with the site's working records. That directory does not exist here, so the 2026-09-11 page's pointers into it are dead.