Addresses all critical and high-severity findings from Fable security review.
CRITICAL FIX#1: Server Certificate Validation Bypass
- File: CertificateRenewal.cs
- Issue: Agent accepted ANY server certificate when ServerCertThumbprint not configured
- Fix: Now validates against Windows trusted roots when pinning not configured
- Impact: Prevents MITM attacks during renewal requests
CRITICAL FIX#2: Certificate Revocation Check Incomplete
- Files: asset.py, client_cert.py, migration 0022
- Issue: Revocation check used hasattr() and silently skipped when columns missing
- Fix: Applied migration 0022, uncommented revocation columns, removed hasattr()
- Impact: Revoked certificates can no longer authenticate or renew
CRITICAL FIX#3: Expired Certificates Cannot Renew
- Files: routes.py (renewal endpoint), CertificateRenewal.cs
- Issue: Expired certs fail at TLS handshake before reaching renewal endpoint
- Fix: Added API key + hostname fallback for renewal when cert expired
- Agent now sends hostname in request body for API key authentication
- Impact: Agents can renew expired certificates, 30-day window becomes effective
HIGH FIX#4: Hostname Injection Vulnerability
- File: routes.py
- Issue: No validation before passing hostname to certificate generation
- Fix: Added regex validation - 1-64 chars, alphanumeric plus dot/hyphen/underscore
- Impact: Prevents injection attacks via malicious hostnames
HIGH FIX#5: Rate Limit Bypass via Clock Manipulation
- File: routes.py
- Issue: Naive UTC datetime arithmetic vulnerable to timezone changes
- Fix: Use timezone-aware datetime with explicit UTC conversion
- Impact: Rate limit cannot be bypassed via clock manipulation
HIGH FIX#6: Hard Process Termination Loses In-Flight Work
- File: Program.cs
- Issue: Environment.Exit(0) aborted in-flight work and could corrupt state
- Fix: Graceful return from RunHostAsync() instead of Environment.Exit()
- Impact: Service exits cleanly, no lost telemetry or corrupted local state
HIGH FIX#7: Missing Database Transaction Isolation
- File: routes.py
- Issue: Race condition in concurrent renewal requests
- Fix: Added with_for_update() row-level locking for both auth paths
- Impact: Prevents concurrent renewals from bypassing rate limit
MEDIUM FIX#8: Missing Audit Logging for Failures
- File: routes.py
- Issue: Failed renewal attempts not captured in audit trail
- Fix: Added AuditLog.log() for cert generation and database update failures
- Impact: Security monitoring can detect repeated renewal failures
SECURITY HARDENING:
- Server cert validation: Never accepts certificates blindly
- Revocation enforcement: Fail closed if columns missing
- Expired cert handling: Dual-auth allows renewal while maintaining security
- Input validation: Hostname must match safe character set
- Transaction safety: Row locks prevent race conditions
- Audit trail: All failures logged for security monitoring
OPERATIONAL IMPROVEMENTS:
- Agents can now renew expired certificates (no more manual intervention)
- Graceful shutdown prevents data loss during renewal
- Rate limiting immune to timezone issues
- Comprehensive audit trail for troubleshooting
ADR COMPLIANCE:
- Maintains ADR-001 (Asset as platform contract)
- Maintains ADR-006 (Collector contract - backward compatible)
- Database naming (snake_case columns)
- No unicode/emojis
- Security best practices: ALL CRITICAL/HIGH issues resolved
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
AGENT SELF-RENEWAL:
- Agent checks certificate expiry on startup
- If < 30 days remaining, requests renewal from server
- Server generates new certificate and updates database
- Agent installs new cert, updates config, and restarts
SERVER-SIDE (/api/endpoints/certificate/renew):
- Requires authentication via existing (expiring) client certificate
- Rate limited: 1 renewal per asset per 24 hours
- Generates 2-year self-signed certificate with cryptography library
- Computes SHA1 fingerprint (matches nginx $ssl_client_fingerprint)
- Updates assets table with new fingerprint and expiration
- Returns PFX/P12 certificate with random password to agent
- Audit logs all renewal operations
AGENT-SIDE (CertificateRenewal.cs):
- CheckAndRenewIfNeeded() called on service startup before host starts
- Finds current certificate by thumbprint in LocalMachine\My store
- Checks expiry: if < 30 days, POST to /api/endpoints/certificate/renew
- Authenticates renewal request with expiring certificate (mTLS)
- Receives PFX + password in JSON response
- Imports new certificate to LocalMachine\My store
- Grants BUILTIN\Users read access to private key (for tray app)
- Updates appsettings.json with new thumbprint
- Exits with code 0 so service manager restarts agent with new cert
CERTIFICATE GENERATION (certificate_renewal.py):
- Uses cryptography library to generate RSA 2048-bit key pairs
- Self-signed certificates with CN=hostname, 2-year validity
- Extended key usage: CLIENT_AUTH
- Subject Alternative Name: hostname as DNS name
- Exports as password-protected PFX/P12 for Windows import
- SHA1 fingerprint computed from DER encoding (nginx compatible)
STARTUP FLOW:
1. Agent service starts (Program.cs RunHostAsync)
2. CertificateRenewal.CheckAndRenewIfNeeded() runs BEFORE host starts
3. If cert expires in < 30 days, requests renewal from server
4. If renewal succeeds, agent exits (Environment.Exit(0))
5. Windows Service Manager automatically restarts the service
6. New process picks up updated thumbprint from appsettings.json
7. Agent continues with renewed certificate
SECURITY:
- Renewal requires valid (not expired) existing certificate
- Rate limit prevents renewal loop abuse
- Server validates cert not revoked before renewal
- Audit logging tracks all renewal operations
- PFX password is random, used once during import, never stored
PERFORMANCE:
- Renewal check adds ~100-200ms to service startup
- Only triggers when expiry < 30 days (rare operation)
- No impact on steady-state agent performance
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
CRITICAL FIXES:
- Enforce cert authentication on collector telemetry endpoint
- Add authorization check in report_command_result (prefer cert over hostname)
- Implement server certificate pinning in C# agents (optional)
- Add constant-time fingerprint comparison to prevent timing attacks
- Validate fingerprint format (40 or 64 hex chars only)
AUTHENTICATION:
- Created shopdb/core/auth/client_cert.py with authenticate_by_client_cert()
- Extracts fingerprint from nginx header (SHA1), IIS environ, or ARR header
- Validates against assets.client_cert_fingerprint in database
- Checks certificate expiration (cert_expires_at column)
- Falls back to API key when cert not present (dual-auth during rollout)
AUTHORIZATION:
- shopdb/core/api/collector.py: Added cert auth to generic_collect() before
API key check. Sets g.cert_authenticated_asset_id when cert validates.
- plugins/endpoints/plugin.py: apply_collector_payload() compares
g.cert_authenticated_asset_id with payload's resolved assetid. Raises
PermissionError if mismatch (prevents agent A from submitting data for
asset B by changing hostname field).
- plugins/endpoints/api/routes.py: report_command_result() prefers
g.cert_authenticated_asset_id over hostname parameter. Requires either
cert or hostname for authorization.
AGENT CHANGES:
- AgentConfig.cs: Added ServerCertThumbprint config for certificate pinning
- CollectorClient.cs: Validates server cert thumbprint when configured
- CommandClient.cs: Validates server cert on WebSocket TLS
- FleetAlertClient.cs: Validates server cert on WebSocket TLS
- When ServerCertThumbprint is empty, accepts all certs (rollout compat)
REVOCATION SUPPORT:
- Added cert_revoked_at and cert_revocation_reason columns to assets table
- Migration 0022_client_cert_revocation.py adds columns
- Revocation check uses hasattr() for backward compatibility
- Revocation requires manual database UPDATE (instant kill switch)
SECURITY HARDENING:
- Constant-time fingerprint comparison via hmac.compare_digest()
- Format validation: must be 40 (SHA1) or 64 (SHA256) hex chars
- SHA1 documented as nginx limitation, acceptable for air-gapped self-signed
- Audit logging for failed authentication attempts (AuditLog.log)
DEPLOYMENT:
- Dual-auth during rollout: cert OR API key both work
- Agents without certs continue using API key (backward compatible)
- No impact on existing fleet until certs provisioned
- Rollout: server first, pilot certs, fleet rollout, retire API key
Addresses Fable security review findings: 1 critical + 5 high vulnerabilities
related to authentication bypass, authorization flaws, and timing attacks.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: Users experiencing frequent forced re-logins after backend restarts,
especially with 8-hour token expiration in production.
Root cause analysis:
1. JWT tokens stored in localStorage on browser
2. Backend restarts with potentially different JWT secret key
3. Frontend doesn't validate stored token on app startup
4. User navigates normally, then first API request returns 401
5. User surprised with sudden logout
This fix adds token validation on app startup. If a stored token exists,
immediately verify it with /api/auth/me before mounting the app. If the
token is invalid (expired or stale secret), user is logged out at startup
rather than mid-session.
Additional issue found (not committed - .env files in .gitignore):
- .env and .env.docker had DIFFERENT JWT_SECRET_KEY values
- Fixed locally: standardized to same key across all .env files
- Action required: Verify production JWT_SECRET_KEY is consistent and persisted
Changes:
- frontend/src/main.js: Call authStore.fetchUser() on app load if token exists
- Logout happens immediately at page load, not on first interaction
- Better UX: "please log in" vs "surprise logout while working"
Future improvement (not in this commit):
- Implement automatic token refresh before expiration
- Add proactive refresh timer (e.g., every 7 hours for 8-hour tokens)
- Would eliminate re-login entirely for active sessions
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Feature: Support Code 39 symbology alongside existing QR and CODE128
- Added CODE39 option to code type dropdown (front and back sides)
- Updated renderOne() to handle code39 type with format parameter
- Code 39 shares CSS styling with CODE128 (linear barcode layout)
- Updated error message to be format-agnostic
Use case: Legacy systems and equipment that require Code 39 format
for compatibility (common in manufacturing and inventory systems)
Technical notes:
- JsBarcode library already supports CODE39 format
- Passed format parameter to barcodeSvgDataUri() function
- Both CODE128 and CODE39 use .type-barcode CSS class
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fix 1: Remove duplicate Setting import violating plugin contract
- plugins/endpoints/api/routes.py:3551 imported shopdb.core.models.Setting
- Setting already imported from shopdb.api at line 21
- Removed local import to comply with plugin contract (ADR-001)
- Test: test_plugins_only_import_contract_surface
Fix 2: Align installer defaults with measuringtools manifest
- measuringtools manifest declares default_enabled: true
- Installer (ShopDBFlask.iss) excluded it from PluginDefault()
- Removed measuringtools from exclusion list (now 4 excluded, not 5)
- Exclusions now match manifests: usb, employees, geenforce, printedparts
- Test: test_wizard_defaults_match_the_manifests
Historical note: measuringtools was promoted from default_enabled false to
true but the installer config was never updated to reflect this change.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Bugfix: Fix AttributeError in /api/plugins/settings-routes endpoint
- Changed get_enabled_plugin_instances() to get_all_plugins()
- Added explicit enabled check with pm.registry.is_enabled()
- Test failure: test_no_route_answers_a_nonsense_filter_with_a_server_error
Optimization: Enable parallel test execution (Phase 1 of pytest speedup)
- Added pytest-xdist configuration to pytest.ini
- Enabled -n auto (auto-detect CPU cores, typically 8 workers)
- Added slow marker for future test categorization
- Expected speedup: 4-8x (20 min -> 2.5-5 min)
Root cause: 2857 tests recreate entire schema per test (0.4-0.6s overhead each)
- Phase 2 (savepoint isolation) will achieve 80x total speedup
- See docs/PYTEST-OPTIMIZATION-PLAN.md for full analysis
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
New endpoint must answer unauthenticated pages (kiosks, TV displays)
so they can load plugin routes before login. Gate test now passes.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Document new endpoint that exposes plugin-contributed settings routes
for dynamic frontend UI extension.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Plugins can now register their settings pages dynamically via get_settings_routes()
hook, eliminating hardcoded imports that break lean builds.
Backend:
- Add get_settings_routes() hook to BasePlugin (shopdb/plugins/base.py)
- Implement in ComputersPlugin for Operating Systems management
- Add /api/plugins/settings-routes endpoint to expose plugin routes
Frontend:
- ModelCatalogHub fetches plugin routes and dynamically adds them via router.addRoute()
- Tabs render dynamically from pluginTabs ref
- Components lazy-load when clicked
Benefits:
- Lean builds work without manual route exclusions
- Plugins self-contain their UI extensions
- No core code changes needed when adding plugin settings pages
- Follows plugin architecture best practices
Fixes TODO from commit a03fd3de4f
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Use git ls-files instead of grep -r for ~5x speedup
- Only scans tracked files, respects .gitignore automatically
- Add USE_GIT_LS_FILES=1 flag (default enabled)
- Filter out *.standalone.js and *.min.js from checks
- Fallback to grep -r for non-git directories
Before: ~5-10 seconds scanning all files on disk
After: ~1-2 seconds scanning only tracked files
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Replace all unicode (emojis, checkmarks, arrows, em-dashes) with ASCII in .claude/ docs
- Comment out OperatingSystemsList route in core.js - breaks lean builds without computers plugin
- Comment out Operating Systems tab in ModelCatalogHub
- TODO: Plugins should dynamically register their settings routes
Fixes#2 CI check failures on wj branch
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Security fix: .claude/mcp-servers.json contained plain-text passwords:
- MySQL dev password: shopdb-dev-password
- VM SSH password: vos313 (SupportUser and shopfloor accounts)
These credentials are now in GitHub history and should be rotated.
Changes:
- Add .claude/settings*.json and mcp-servers.json to .gitignore
- Remove these files from git tracking (files remain locally)
- Keep shared team resources: skills/, plans/, setup/, maintenance/
These are personal/machine-specific configuration:
- settings.json: Personal hooks (stop-dev.cmd), preferences
- mcp-servers.json: Local paths, database credentials, SSH passwords
Each developer should configure their own .claude/ settings locally.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Brings docker-compose.prebuilt.yml in line with the migrate service
refactor from wilm (commit 58a3670529). Migrations, plugin bootstrap,
and seeds now run in a dedicated service with restart: "no" so failures
block deployment instead of crash-looping.
Changes:
- Add dedicated migrate service (runs db upgrade, bootstrap, seeds)
- API now depends on migrate: service_completed_successfully
- Database bound to 127.0.0.1:3306 (not 0.0.0.0) for security
- UTF-8 charset enforced server-wide
- Healthcheck uses TCP not socket (prevents false-healthy during init)
Testing:
- Verified migrations are idempotent (safe to re-run)
- Verified failed migration blocks API startup (fail-closed)
- Verified startup sequence: db -> migrate -> api (44s)
- Database at schema version 7d51_endpoint_logcollections (head)
Aligns dev environment with pull/airgap deployment pattern.
Endpoints plugin now respects the site_timezone setting from /settings.
All timestamps in endpoint details, script runs, and log collections are
now converted from UTC to the configured site timezone for display.
Changes:
- Added _get_site_timezone() helper to read site_timezone setting
- Added _localize_timestamp() to convert UTC datetimes to site timezone
- Applied localization to GET /api/endpoints/<id> (last_seen, created_at)
- Applied localization to GET /api/endpoints/<id>/scriptruns (execution times)
- Applied localization to GET /api/endpoints/<id>/logcollections (collection times)
This aligns endpoints plugin with notifications and backups plugins which
already use site_timezone for their time-sensitive operations.
Changed FleetAlertStore to save acknowledgements in:
ProgramData/ShopDB/FleetAlerts/acknowledged-alerts.json
Instead of:
ProgramData/ShopDbAgent/acknowledged-alerts.json
Now matches the standard folder structure used by other agent components.
The dev stack ran schema work from the api entrypoint while the pull and
airgap stacks ran it from a dedicated migrate service. Same repo, two
different init paths, and the dev one was missing a step.
That gap was real, not theoretical: the first run of this service on a
stack that had been up for days created 13 default settings. The entrypoint
only ran `db upgrade` and `plugin upgrade-all`, never the seeds, so any
patch adding a permission or a setting reached pull/airgap sites and
silently skipped dev. endpoints.agentupdate was exactly that shape of
change.
Running init from the api entrypoint has two other problems. Under
`restart: unless-stopped` a failing migration crash-loops, re-attempting
partially-applied schema work and burying the original error under retry
noise; the migrate service uses `restart: "no"` so a failure exits once and
stays readable. And `plugin upgrade-all` was tolerated as a warning, so the
app would start with plugin tables missing - the shape of the
endpointlogcollections bug. Chained with && it now stops the rollout
instead.
No fallback path is kept in the entrypoint. Compose honours
service_completed_successfully even when a single service is named, so
`up -d api` still runs migrate to completion first - verified, api stays in
Created until migrate exits 0. A skip-flag would have meant the safe
behaviour depended on remembering to set a variable, and deployment here
goes through an install script that invokes compose, so there is no second
caller to protect. The entrypoint keeps only the database wait, which is
about this container's readiness to serve rather than schema state.
The seeds are all create-if-missing, so running them on every start is a
no-op once current: the second run reported 0 permissions, 0 settings and
scopes already up to date. Ordering in the chain is load-bearing - plugin
tables cannot be created before the plugins are installed, and seedscopes
only links permissions that already exist as rows, so seeds run last.
migrate carries the full app environment including CORS_ORIGINS even though
it never serves a request, because create_app() validates config before any
CLI command dispatches and the production config refuses to build without an
explicit allowlist. It mounts instance_data because bootstrap-plugins.sh
writes plugins.json there; it and api never run concurrently, so the shared
mount is not contended.
The Dockerfile's standalone `docker run` note now states that the image does
not migrate itself and lists the sequence to run first.
An operator could see that an endpoint was behind the published build but
had no way to act on it. update_app resolves against Application rows, and
the agent lives in the version store, so there was nothing to push.
Bridge the two: the staged build is packaged and registered as a reserved
Application row, and a managed PowerShell script is seeded to install it.
The endpoint downloads that package over the existing collector-authenticated
app-package route, so no parallel delivery path is introduced.
The new route prepares the delivery and then DELEGATES to
enqueue_endpoint_command as a digest-pinned run_script. It deliberately does
not implement its own enqueue: a second path would duplicate, and eventually
drift from, the endpoint lock, the Configuration Policy gate, stale-command
reaping and the at-most-one-in-flight rule. A fleet-wide agent swap is the
last action that should skip a quarantine check. Tests pin that delegation
rather than only the happy path.
Because it lands as run_script, the caller needs endpoints.runscript on top
of endpoints.agentupdate. That is intended: this delivers arbitrary SYSTEM
PowerShell by the same mechanism as any other script push. The audit event
is written only on an actual 201, so a refused push does not record an
upgrade that never happened.
Replacing the agent is self-modifying the control plane - the process
receiving the command is the one being swapped. The script therefore
re-launches itself detached and exits immediately, because the agent runs
run_script as a direct child and holds its stdout/stderr pipes while
waiting on it. Expect the command to report queued rather than success:
the process that would report completion is stopped mid-run. Judge the
result by the version the endpoint reports afterward.
Tray relaunch does not use the installer's `explorer.exe <path>` trick.
That works in install-shopdbagent.ps1 only because the installer runs as
the user - note its own `-ne "SYSTEM"` guard. This script always runs as
SYSTEM in session 0, where the hand-off reaches no shell and exits 0, so
Start-Process reported success for a relaunch that never happened. It also
cannot descend into the session with a token-duplicating launch:
UserContextSpool documents that WTSQueryUserToken is blocked by endpoint
protection on these hosts. Instead a one-shot scheduled task targets the
pinned ShopFloor account, matching how the user-context spool already
resolves that user, and the tray must run as that account or the
mapped-drive probe reports the wrong (empty) drive table. The relaunch is
verified by polling for the process rather than assumed, and stays
best-effort: the Run key still starts the tray at next logon, and a late
tray is not worth rolling back a good agent upgrade.
panel_status now carries agentbehind/agentversion/agentcurrent so the UI
gates the action on a flag rather than parsing "(behind X)" out of a label.
The button is confirmed by typing UPDATE, naming both versions and the
service interruption, matching how run_script and system_restart are gated.
Verified on live endpoints: upgraded 1.13.0.0 and 1.15.0.0 machines to
1.16.0.0. run_script delivery, digest fetch and system context all predate
1.13.0.0, so no minimum agent version is declared for this path.
Gate collect_log on a minimum agent version and bridge the agent version
store to the applications table so a staged build can actually be pushed.
update_app resolves an install source from the applications table alone; it
cannot see agentstore. A build could therefore be staged and current while no
application row existed at all, which is what the fleet was in: the agent was
unpushable. Staging is now the single source of truth - it packages the
current build and upserts the row in the same request, so the row cannot
describe a build that is no longer staged.
PARTIAL BUILD ADVISORY
This is steps 1-4 of a longer rollout and is deliberately incomplete:
- There is no push trigger yet. The registered package is resolvable and
verifiable, but nothing enqueues an upgrade; the "behind current build"
indicator is still display-only.
- The upgrade template script, its script row, and the enqueue route
(endpoints.agentupdate) are not written.
- Blocking constraint for the next slice: run_script executes
powershell.exe -File <script> with NO argument channel, so a delivered
upgrade script cannot be passed BaseUrl/CollectorKey and must either
source them itself or run_script must be extended first.
Nothing here touches a live endpoint. Staging only changes what a future
push would contain.
Details:
- Package members carry a fixed timestamp. zipfile stamps the current mtime
by default, so identical input produced a different archive digest on
every call. update_app pins a download by SHA-256, so a drifting hash
would rewrite installersha256 out from under an endpoint mid-download and
fail its verification.
- All three routes that can change 'current' re-register (upload,
adopt-legacy, rollback). Covering only upload would leave the row pinned
to the build an operator just rolled away from, and the push would ship
it anyway - silent drift.
- Registration cannot fail an upload. The binary is staged and audited by
the time it runs, so a packaging problem is reported in the response
payload rather than raised; returning an error would tell the operator a
successful upload was rejected.
- The package is written to a temp name and os.replace'd, so an interrupted
write cannot leave a truncated archive at the path the row advertises.
- The row is isinstallable but hidden: machine-managed, and it must not
appear in the software catalogue beside real licensed applications.
- Added application_package_location() to the shopdb.api contract surface
(lazy, matching collector_authorized) returning the package directory and
URL prefix together. agentregistry reached into shopdb.core internals,
violating the plugin import contract; sourcing both from one place also
stops the write location and installpath from drifting apart.
collect_log is refused at enqueue below its minimum agent version, with
numeric comparison - string ordering misplaces 1.9.0.0 against 1.16.0. The
refusal is mirrored in the UI so a command slot is not spent on a call that
is guaranteed to fail.
Test isolation: create_app('testing') resolves instance_path to the real repo
instance/ directory, so agent upload tests write into the developer's actual
store. The new tests redirect both the store and package output into tmp_path
and include a guard test that fails if that isolation regresses. NOTE: the
pre-existing test_endpoints_agentupload.py still has this leak and deposits
1.5.0.0 and 1.6.3 into instance/siteagent/ on a full run - not fixed here.
Suite: 2834 passed, 11 skipped. The two failures (test_docs_publishable, and
a symlink scan needing Windows privileges) were confirmed pre-existing by
stashing this work and reproducing them.
The new table was declared as a model and created by migration 7d51, but never added to the explicit ownership map. test_owned_tables_match_declared_models caught the drift.
Not cosmetic bookkeeping: the per-plugin migration uses this map to decide which tables it owns, so an unlisted table is invisible to plugin-scoped baseline and prune. The prune path would have treated endpointlogcollections as unowned - collected logs surviving or vanishing on a schema operation that should have been deterministic about them.
Log Collector: a new collect_log action reads a plaintext log file from a
known path and returns its tail for viewing and download. Rides the existing
endpoints.updateapp grant rather than inventing one, since collecting a log is
the same trust level as pushing an application. Collected logs land in their
own table with per-asset retention of 20 instead of overloading the command
audit row, and the agent refuses UNC, traversal, wildcards, directories and
non-text content independently of the server check.
collect_log is POLICY_EXEMPT. Every existing Configuration Policy predates the
action and so cannot name it, and the intersection in ALLOWED_ACTIONS_FOR drops
anything unnamed - so every policied endpoint refused log collection outright,
including for admins, who saw a permission-shaped error for a grant they held.
Exemption from policy is not exemption from permission: the elevated grant
still applies, and a regression test asserts both halves so a future edit
cannot quietly loosen the grant while "fixing" the policy gate.
Script output capture: successful runs can return output, gated per script by
captureoutput. Failures always returned it; success never did, which left a
green badge as the only evidence a run produced.
Timestamps: Activity and Operator Reports rendered date-only because the shared
formatter's 'date' branch drops the clock time. Run Scripts additionally
labels its timestamp, since a bare time meant "completed" on a terminal row but
"queued" on an in-flight one - indistinguishable, and the wrong reading.
Agent 1.16.0. Minor: collect_log is a new action name, so an older agent asked
for it fails closed on the unknown-action default.
alertscope was stored, serialised and editable in the authoring form but
never read when alerts were handed to an endpoint, so every fleet alert
went to every machine regardless of what was entered. The control implied
a capability nothing enforced.
get_fleet_alerts now resolves the polling endpoint's facts and filters
each alert through its scope. Supported keys: hostname, assettype,
computertype, location, businessunit. A key may hold one value or a list
(any match), matching is case-insensitive since these are operator-typed
names, and multiple keys narrow together.
Unrecognised keys are ignored rather than treated as match-nothing. A
typo like {"hostnam": "BAY-01"} would otherwise mute the alert fleet-wide
and look identical to a delivery failure; sending wider than intended is
the safe direction for something whose job is to warn people. A garbled
non-dict scope falls back to the whole fleet for the same reason. The
real fix for typos belongs at authoring time, not delivery.
BusinessUnit is not exported from shopdb.api, so its name is resolved by
table rather than importing the model and breaking the plugin import
contract.
Ten tests cover the narrowing cases and the fail-open behaviour.
Brings in wilm commits:
- Render script surfaces correctly in dark mode
- Stop scripts reporting success after doing nothing
- Correct two gate commands that pass without running
- Gate the script library on the grant that pushes scripts
- Let the policy list edit the rows the API accepts
- Cap plugin revision ids at the 32 chars MySQL stores
- Split shipped monitor bundle into per-monitor policies
Resolved conflict in agent ShopDbEndpointAgent.csproj by merging:
- wilm 1.14.0 script verdict and output capture features
- wj 1.13.1-1.13.5 tray coordination releases
- Bumped to 1.14.1 for coordinated agent+tray release
The wj merge brought in a half-built fleet alert feature: server-side UI
behaviour still being stood up over a delivery proof of concept. It left
five red tests. This clears them and fixes a real hole underneath.
The cross-machine guard never ran. acknowledge_fleet_alert read .assetid
off _resolve_asset_for_hostname, which returns the id itself, so the
check raised AttributeError on every call and answered 500. That guard is
what stops one endpoint acknowledging another's alert, and the collector
key is shared across the fleet, so until now any bay could clear an alert
on another machine's behalf. Compare the id directly.
Gate both alert routes the way the rest of the agent channel is gated.
They are collector-key authenticated, not JWT+role, because a shop floor
bay has no user session - the same shape as claim_next_command and
upload_operator_report. Declared in the authz allowlists with the reason,
rather than inventing a permission no agent could hold.
Settle the delivery contract ahead of the durable agent work. The poll
response is authoritative for a host: every alert that should be live on
it right now. An alert absent from the feed is the agent's instruction to
retire the pending toast, which is how a server-side close or cancel
reaches a machine already displaying one. Acknowledgement is terminal per
product decision, so a close after an ack is a no-op and the agent can
retry a queued ack safely. A toast that merely timed out is not an
acknowledgement.
Stop reaching past the plugin contract surface for the mailer. One import
was a plain redirect; the other pulled a private helper to pre-check
recipients, duplicating what send_alert already does. It also returned
early when no email recipients existed, which silently suppressed the
independent webhook leg - misconfigured SMTP was dropping Teams alerts.
Register NotificationAcknowledgement in get_models() and give the
notifications plugin ownership of notificationacknowledgements, then move
the expected head to notifications0006fleetalerts.
Add the coverage whose absence let the .assetid bug ship. Thirteen tests
over gating, authoritative-feed withdrawal, terminal and idempotent
acknowledgement, per-asset scoping, and the cross-machine guard. Five of
them fail against the pre-fix handler.
Remaining suite failures are pre-existing and local: the symlink scan
needs elevation, and the docs check wants files absent from this checkout.
Brings in the fleet alert notification concept, the operator report email
alerts, and the tray work that goes with them (tray 1.6.5).
Two files needed resolution:
- ShopDbEndpointAgent.csproj: wj carried 1.13.1 through 1.13.4, all
tray-coordination bumps with no agent code behind them. Ours carried
1.14.0, which is real code. Kept every changelog entry and took 1.14.0
as the version, since it already supersedes the 1.13.x line.
- plugins/endpoints/api/routes.py auto-merged; verified the errormode
descriptor survived intact.
KNOWN RED, INHERITED NOT CAUSED: five backend tests fail on this merge -
authz on endpoints.acknowledge_fleet_alert, the public GET surface scan,
two plugin-contract import checks, and the notifications plugin
migration/model agreement. All five were confirmed failing on a clean
worktree of origin/wj before merging, so they arrive with the branch
rather than from the merge. They reflect the fleet alert work being
half-landed: the push path was a delivery proof of concept and the
server-side surface was still being stood up.
These are not fixed here on purpose. The next change replaces that
delivery path with the endpoint agent, so the notification persists on
the endpoint until the operator acknowledges it or the server closes it,
and the affected surfaces get their gating and migration settled as part
of that work rather than being patched twice.
The explainer panel used var(--surface, #fff) and PCDetail's Run Scripts
and Corporate Tools styles used --text-primary, --text-secondary and
--bg-secondary. None of those four tokens exist in this app's theme, so
var() fell through: the explainer rendered a solid white block over the
dark background and the surrounding copy lost its intended contrast.
The real tokens are --bg-card, --text and --text-light. A fallback
colour is what made this hard to catch - it looks correct in light mode
and only misrenders on the theme the shop floor actually runs, so it
would not have shown up in authoring or in review.
Both files now carry a test asserting their styles reference only
defined tokens and contain no var(--x, #colour) fallbacks. That test
immediately caught a further dead --warning fallback in the banner.
The two long per-field explanations in the Add Script form are now
collapsed behind "How this works". The modal already scrolled, and
three paragraphs of standing prose between the inputs pushed Save off
screen, so the copy was being skimmed past rather than read.
A run_script command was judged solely on PowerShell's exit code, and
PowerShell treats most errors as non-terminating: it writes the error,
runs the next line, and still exits zero. The account-provisioning
script reported succeeded having provisioned nothing, and because no
output was captured there was no evidence to contradict the verdict.
Scripts now carry an errormode. Strict prepends $ErrorActionPreference
= 'Stop' so the first error ends the run and the exit code means what
it says. It is opt-in per script rather than global because bodies
authored against the permissive semantics rely on them - tolerating a
failing cleanup, probing with a cmdlet that errors when a thing is
absent, looping past a bad item - and would start aborting midway,
leaving endpoints half-configured. The default is unchanged, so every
existing script keeps running exactly as before.
The prologue is applied after digest verification, so the SHA-256 still
validates the bytes the server delivered.
stdout/stderr is captured on both paths: redirected streams for SYSTEM,
a transcript file under the ShopFloor profile for user-session runs,
since CreateProcessAsUser cannot inherit the service's pipes across the
session boundary. Output is always logged locally and returned to the
server only on failure, so routine output stays on the machine that
produced it instead of filling command history.
Also adds the optional per-script working directory, which the same
surfaces needed, and a collapsible explainer on the scripts page - the
dropdown is not self-explanatory, and an operator who does not know
about non-terminating errors has no reason to ever leave the default.
Agent 1.14.0. An older agent ignoring errormode keeps today's behaviour.
- Get pc_access_domain from settings to build full FQDN (hostname.device.geaerospace.net)
- Display FQDN in email subject and body instead of short hostname
- Fix machine_url to use actual request host instead of localhost fallback
- Remove unnecessary try-except around machine URL construction
- Device: shows only hostname (not name)
- Removed Asset Number line
- Machine#: shows only machine number as clickable link (if relationship exists)
- Keep Reason, Reported, Details, and Screenshot
- Query AssetRelationship to find if PC has a machine relationship
- Subject line now shows: "Operator Issue Report: HOSTNAME (Machine #6906)"
- Email body includes machine name and number as clickable link
- Machine info only appears if relationship exists
- Change device URL from API endpoint (/api/assets/{id}) to frontend page (/shopdb/pcs/{id})
- Add asset number to email subject line: "Operator Issue Report: {name} ({number})"
- Bump agent to 1.13.5 and tray to 1.6.6 for coordinated release
- Log when function is called with report details
- Log SMTP config and recipients being used
- Log before and after email send attempt
- Change warning to error with full traceback for failures
SECURITY FIXES:
- Escape all user-controlled values (hostname, reason, details) to prevent XSS
- Fix endpoint name from 'assets.detail' to 'assets.get_asset'
- Fix fallback URL to be absolute instead of relative path
- Remove render_email() to avoid subject duplication in email body
- Check recipients exist before expensive HTML generation
- Build email template directly with proper escaping throughout
- Use render_email() to wrap HTML in proper email template
- Add error handling and logging for URL generation
- Add log statement before sending to aid debugging
- Import render_email at module level
- Send alert to default alert recipients when operator submits issue report
- Email includes device link, hostname, asset tag, reason, details, and timestamp
- Screenshot link included when available
- Uses existing send_alert() mailer infrastructure
npx vitest run bypasses the pretest hook that stages plugin frontends, so
plugin specs were never collected - it reported 28 spec files where npm test
reports 30. A plugin frontend spec could pass CI unread. Bare bash resolves to
WSL on Windows dev boxes and fails to find /bin/bash, which redirected to a log
reads as a clean style run with no findings.
Both are the same failure shape: a command that reports success without having
done the work.
Also records that a multi-line here-string passed to git commit -m silently
no-ops, reporting success without creating the commit.
The script library was gated on applications.*, so anyone holding the
software-delivery grant could rewrite the bytes of a script that a separately
trusted operator later pushed to a bay. The push was gated on
endpoints.runscript; the CONTENT of what got pushed was not. An installer is a
vendor artefact, but a script here is arbitrary PowerShell this site wrote that
the agent runs as SYSTEM, so the write and the push must require the same
trust.
All six script routes now require endpoints.runscript, including the body
download - reading the bytes is not a lesser act than listing them. Core names
the permission as a string rather than importing it from the endpoints plugin
so the module stays importable with the plugin absent.
This tightens access and there is deliberately no backfill: the operator scope
reaches applications.create/edit, so operator-scope holders lose the library.
Granting them endpoints.runscript on upgrade would undo the change and hand
every operator remote SYSTEM code execution, since the same grant gates the
push. A test pins that no prebuilt scope confers it.
Adds the System Scripts settings page and card. The route requires the
permission rather than admin, because the grant is meant for non-admin
operators and the sibling admin gate would lock them out of the library the
grant exists for. The settings icon map gains terminal and key; the agent-key
card had been asking for key since it shipped and silently drawing a puzzle
piece.
The library had no test coverage before this.
- Dispose GraphicsPath after creating Region to prevent GDI handle leak
- Track and dispose all Font objects to prevent GDI+ resource leak
- Join worker thread before disposal to ensure client not used after disposal
- Fix Screen.PrimaryScreen null dereference with fallback rectangle
- Propagate cancellation token to FetchAlertsAsync to stop orphaned requests
- Bump tray to 1.6.5, agent to 1.13.4
