Commit Graph

296 Commits

Author SHA1 Message Date
cproudlock
a46058a4dd Close the last three restore-gate gaps, and make the gate test the real gate
Some checks failed
CI / naming (push) Successful in 3s
CI / frontend (push) Successful in 12s
CI / migrations-mysql (push) Successful in 42s
CI / backend (push) Failing after 5m13s
Three defects that all shared a shape: the restore path looked checked and was
not.

An unreadable data revision was treated as a legacy set. backup.sh discards the
mysql error when reading alembic_version, so a set could be written with an
empty datarev; the compatibility check then fell through to the code-to-code
head comparison - the very check datarev was introduced to replace - and badged
the set restorable on no evidence at all. A manifest that CARRIES the key and
left it empty is now refused, while one with no key at all is still the genuine
legacy case and keeps the fallback. backup.sh records the failure in run history
rather than passing it over in a log line, since the set it just wrote cannot be
restored.

Dumps were bound to the schema they came from. mysqldump --databases bakes
CREATE DATABASE and USE <name> into the file, so restoring onto a site whose
MYSQL_DATABASE differs loaded every row into the dump's schema, left the live
database untouched, and reported success. New sets are dumped without it and
load into the schema named at restore time. Existing sets are still bound, so
the gate reads which schema they name and refuses a mismatch instead of
restoring beside the live data. Also dropped --no-tablespaces into the main
dump: it needs the PROCESS privilege the app user does not have on MySQL 8.

The gate's own test carried a copy of the gate. test-restore-datarev.sh
re-implemented the predicate in its own shell functions, so it would have stayed
green through any drift in restore.sh - including the plugin-chain hole fixed in
the previous commit, which it could never have caught. restore.sh now takes
RESTORE_CHECK_ONLY, which stops it immediately after the gates having touched
nothing, plus overridable paths for the migration and plugin trees. The test
drives the shipped script and asserts on its exit code, so the verdict under
test IS the verdict that ships. Confirmed non-vacuous by planting a violation:
disabling the plugin revision check turns the suite red.

Nine cases now, up from five, covering both plugin-chain directions, a plugin
the build does not have, and the schema binding.

Moving the schema check earlier was prompted by that test: it had been placed
after the safety copy, so it needed a live database to reach a verdict it can
reach for free. It sits with the other gates now and stops before anything is
written.

Round-tripped against a real stack after the dump format change: backup, mutate,
restore, data back to backup-time state, and the new dumps confirmed to carry no
schema name.

Suite 2607 passed / 9 skipped.

Scope: product
2026-09-06 18:31:37 -04:00
cproudlock
e2705d732a Gate a restore on every schema chain, not just the core one
Some checks failed
CI / naming (push) Successful in 4s
CI / frontend (push) Successful in 13s
CI / migrations-mysql (push) Failing after 15s
CI / backend (push) Failing after 4m56s
The compatibility gate read alembic_version and nothing else, but plugin schemas
move independently of core. Roll the code back to an older plugin revision and
the core head can be identical, so a dump carrying plugin tables the code has
never heard of passed the gate, loaded - destroying what was there - and only
then failed in 'flask plugin upgrade-all' with "Can't locate revision". A failed
restore that has already replaced what it was restoring over is the worst
outcome this feature can produce, and it was reachable from an ordinary rollback.

Backups now stamp the applied revision of every alembic_version_<plugin> table
alongside the core one, read from the dumped database the same way. restore.sh
checks each against that plugin's own versions directory and refuses before
touching anything, naming the plugin and the revision to deploy past. A plugin
in the dump that this build does not have at all is refused for the same reason.
The list endpoint's compatibility badge asks the same question, so the UI cannot
offer a set the restore will reject.

Sets taken before the stamp existed carry no dataplugins and keep their old
core-only verdict. Their plugin revisions are unknowable, and making every
existing set unrestorable to close this would be a worse trade than the gap.

Proven against a real stack, reproducing the exact case: core head identical,
endpoints data stamped at a revision the build does not have. The restore
refused, the live row was still there afterwards, and it stopped early enough
that the pre-restore safety copy was never needed. The same set with its true
revision restored normally.

Suite 2605 passed / 9 skipped, with the gate covered both ways plus the legacy
fallback.

Scope: product
2026-09-06 18:02:39 -04:00
cproudlock
fa6b99bf91 Finish a restore without a browser, from the set that was asked for
Some checks failed
CI / naming (push) Successful in 4s
CI / frontend (push) Successful in 14s
CI / migrations-mysql (push) Failing after 14s
CI / backend (push) Failing after 4m57s
Closes the three defects in the imported restore path, at Ben's request. Each
was proven against a real stack before and after: two backup sets with different
data and different instance files, then a restore of the OLDER one, which is the
case that was broken.

A restore no longer depends on a tab staying open. The backup sidecar has no
Flask, so it loads the dump and leaves a sentinel for the api container to run
the migrations. The only thing that ever acted on that sentinel was the poll
loop of the modal that queued the restore, so closing the tab - or letting a
token expire - during the minutes a load takes left the data at the old revision
under new code, every query on a newer column failing, and nothing retrying. A
watcher in the api container now claims and runs it. The endpoint no longer runs
the upgrade at all; it only makes sure something is pending, which is also how a
failed upgrade is retried. That takes Alembic out of the request path, where it
could outrun the gunicorn worker timeout and be killed mid-chain, leaving a
half-applied revision the next attempt trips over.

The claim is a rename, because rename(2) is atomic: several workers and a
browser poll can race the same sentinel and exactly one wins. A failed upgrade
parks itself as .upgrade-failed rather than re-arming, since putting it back
would rerun the same broken chain on every poll forever.

Instance files come from the set being restored. The restic call asked for
'latest' regardless of which set was named, so restoring anything but the newest
paired that set's DATABASE with the NEWEST snapshot's uploads and plugins.json -
a mismatch nothing downstream could detect. Now scoped by the set's own tag,
which backup.sh has always written. The tree is replaced rather than restored
over, so files created after the snapshot are gone afterwards: a restore has to
put the site back to the snapshot, not merge into it. Done as a staged swap
rather than 'restic restore --delete --target /', because that aims a delete
pass at the container root and the cost of getting its filter wrong is the whole
filesystem.

Nothing destructive happens before a copy is taken. The dump carries DROP TABLE
per table with no transaction around it, and the instance tree is replaced
wholesale, so a wrong set or a failure part-way through left nothing to go back
to. restore.sh now writes a pre-restore-<stamp>/ set - dump, instance tarball
and a README saying what it is and how to load it - and refuses to touch
anything if that copy cannot be written.

Two more found while proving the above, both of which stopped the overlay
working at all:

* Every script under scripts/backup/ was committed non-executable, while every
  other .sh in the repo is 755. The compose entrypoints are
  /opt/backup/watch.sh and backup.sh from a read-only bind mount, so the backup
  and watcher containers could not start from a clean checkout on any machine,
  and nothing could fix it at runtime.

* Dockerfile.backup could not build without the corporate CA staged. A plain
  COPY of the gitignored cert fails outright; the trailing-glob form does too,
  because BuildKit fails a COPY whose pattern matches nothing - which is exactly
  the case being handled. Copying the directory and testing for the file inside
  it is the form that works both ways. Verified by building the image with no
  cert present.

Suite 2600 passed / 9 skipped. The finalizer has unit tests covering the atomic
claim, the parked failure and the endpoint no longer running the upgrade.

Scope: product
2026-09-06 17:38:10 -04:00
cproudlock
6e36df841e Import the wilm endpoint telemetry, PC Detail rework and backup overlay
Ports 37 of Benjamin Baker's commits from the wilm branch, mirrored from the
corporate GitHub at 7905917c. wilm rides the publication lineage and shares no
ancestor with wj, so this is a filtered tree patch rather than a merge:
94 files, and every path the publication export strips (docs/, tools/, mcp/,
CLAUDE.md, start-*.sh, .gitea/, the three excluded tests) is held back, because
a raw diff would delete each of them from this tree.

What comes across:

* Endpoint telemetry, phase 2. Physical disk health from two sources, NIC link
  state, boot type, Defender health, processor/memory/chassis inventory, the
  listening TCP set, mapped network drives and Windows Update history. Five
  additive migrations, 0012 through 0016, off the 0011 head.
* Mapped drives are collected by a probe running in the interactive user's
  session, because they do not exist in the agent's session 0. Two Windows
  sources can disagree about one drive and the worse status wins, so a
  half-broken share is never rendered as healthy.
* Windows events are reported once each, keyed on the channel's own record id,
  instead of being re-sent whenever a report overlapped the last one.
* PC Detail rework: Details cards flow, the hand-written Network card gives way
  to a full address history, Storage and disk health merge into one card with
  two sections, Operator Reports and Corporate Tools get their own tabs.
* An opt-in restic backup overlay with a third container, restore gated on the
  dump's own data revision rather than the code head, and backup/restore history.
* The agent's version is read from the binary's PE version rather than trusted
  from the csproj, and the Fleet view gains an Agent Version column.
* The enqueue route now asks caller_holds instead of the owner's permissions,
  which closes the scoped-token elevation on run_script, update_app and
  system_restart.

Held back deliberately:

* agent/installer/install-shopdbagent.ps1 stays deleted, as upstream deleted it.
  The installer that reaches a bay is generated by _install_script() in
  plugins/endpoints/api/siteagent.py; the standalone script had stopped being
  shipped and was collecting fixes no endpoint ever ran.
* The .gitignore hunk adding docs/*.md. Correct on the publication lineage,
  where docs/ becomes the wiki and its absence is how the gates detect a
  published checkout. Here it would silently ignore every new doc.
* Their deletions of docs/DEV-BUILD-TEST.md and docs/PLUGIN-HOOKS.md. Upstream
  those were stubs; here PLUGIN-HOOKS.md is the 31KB document CLAUDE.md points
  readers at.
* Five zero-byte files committed by an accidental `git add -A`, a run log from
  the author's own machine, and docs/notes/, which is working scratch that says
  so in its own header and belongs in devnotes/ on this side.

Carried with the import so it lands green: the 17 new routes are described in
docs/api-inventory.json with the spec and project map regenerated, and two new
frontend specs read fixtures through fileURLToPath(URL) which node rejects under
jsdom's own URL global.

Suite 2591 passed / 9 skipped; frontend 391 passed; naming, style and doc gates
clean.

Scope: product
2026-09-06 16:47:24 -04:00
cproudlock
c1a4c9eba6 Let a PC be assigned a business unit, and a retired asset be found
Some checks failed
CI / naming (push) Successful in 3s
CI / frontend (push) Successful in 13s
CI / migrations-mysql (push) Successful in 42s
CI / backend (push) Failing after 4m47s
The last two items from the run.

The PC detail page has always rendered a Business Unit row, and the API has
always accepted the field. Only the form had no control, so it could be read and
never set from the UI - a value that appeared the moment somebody set it through
the API and could not be reached otherwise. The form has the picker now, loaded
with listAll() rather than a capped page.

Global search excluded inactive assets, so a retired record existed, was
reachable by URL, and could not be found by anybody who did not already know
where it was - including whoever wanted to restore it, which the retire dialog
promises they can. Retired assets are searchable now.

That is only safe with the ordering, so it is part of the change rather than a
follow-up: somebody searching a machine number almost always wants the machine
still on the floor, so a retired match drops 60 points of relevance, sorts below
every live one, and carries a 'retired' flag the results page renders as a
badge. Indistinguishable retired results would be worse than none.

Four tests, verified by planting the isactive filter back: three fail.

Scope: product
2026-09-03 17:08:03 -04:00
cproudlock
a1802a2bf5 Match the number on the bin, and let an unset kiosk say so
The last two kiosk findings.

Keypad entry collected EVERY digit in an item's code, and the shipped prefix
contains one: '3DP0004' collapsed to 30004, so an operator typing the 4 printed
on the label was told no part matches that barcode and had to work out that the
leading 3 belonged to the prefix. The number on a label is the number at the
end, so the trailing digit run is what is compared now. An ambiguous number is
still refused rather than guessed - taking from the wrong bin is worse than
being asked to scan the label.

A parts kiosk that had never been provisioned drew its normal "Scan the barcode
on the bin" screen and answered "Not found" to every scan, which reads as "we
do not stock that part" rather than "this screen was never set up". The server
cannot say which: the routes answer a bare 404 on purpose, so a caller without
the device token does not learn the door exists, and that stays. But the BROWSER
knows whether it holds a token, and saying so discloses nothing - an
unprovisioned screen now says it is not set up and how to fix it.

For the case the client genuinely cannot distinguish - kiosk switched off versus
a part that is not stocked, both a 404 by design - three misses in a row add a
line suggesting the kiosk may have been switched off. Truthful without the
server having to leak anything.

Three tests, verified by planting the all-digits match back.

Scope: product
2026-09-03 17:00:49 -04:00
cproudlock
f4d3827214 Record how a PC is reached, and stop pages implying they can do things
Three findings, all about a screen or a log saying something that is not so.

Editing a PC wrote an audit entry for its named fields and nothing for the two
changes that matter most when somebody investigates later: the primary IP
address, which is how the machine is found, and the remote-access protocols and
ports, which is how somebody connects to it. Both are on the same form, in the
same request, and both were invisible afterwards - a PC that moved address had
no history of moving, and a PC that gained a VNC listener had no record of
gaining one. Both are recorded now, and re-sending the same value still records
nothing, because an audit log full of non-events is one nobody reads.

While proving that, a silent discard turned up in the same lines: both the
create and the update path did `if ip_comtype:` around the IP write and did
nothing at all when the 'IP' communication type row was missing. Somebody typed
an address, saved, got a 200, and the address was gone. A missing reference row
is a seeding gap, not a reason to drop what was typed, so it is created on
demand.

The PCs list rendered a 403 as "No computers found", which tells a person
without the permission that the fleet is empty and sends them looking for the
data rather than for the permission. It now says which permission is missing.

The Site Policies and Configuration Policies pages rendered their empty table
and an Add button while the policy feature flag was off - every route answers
404 "Policy feature is not enabled", so the button could never work. Both pages
now say the feature is off and that the empty list is a consequence of that,
not of nothing being configured.

Nine tests. The audit entries verified by planting: remove either and two fail.

Scope: product
2026-09-03 16:56:22 -04:00
cproudlock
30d0d4f6d5 Stop asking for more rows than the server will ever return
You are right that this is the one we keep hitting. perpage is capped at 100
server-side, and the cap is applied SILENTLY: a request for 500 returns the
first 100 rows with a 200, so a picker built that way shows the first 100
options and drops the rest with nothing anywhere reporting a problem. The
missing vendor simply is not in the list.

Twelve call sites were still doing it - vendor and business-unit pickers on the
machine and measuring-tool forms, and the type lists behind PCs, machines,
printers, network devices, measuring tools and configuration policies. Each now
either pages through with listAll()/fetchAllPages() or asks for what it can
actually have. vendorsApi and businessunitsApi gained listAll(), which they
lacked, which is why those two sites reached for 500 in the first place.

The gate is the point. tests/test_core/test_perpage_cap.py reads every .vue and
.js source under frontend/src and plugins/*/frontend and fails on any perpage
over 100. It has to read the SOURCE rather than the running app, because the
response is a valid 200 either way - no runtime assertion can see this one.
Verified by planting a perpage: 500 back into MachineForm: the gate names that
exact line.

Separately, /api/auditlogs built its own meta with the pagination fields flat
and a 'pages' key, while every other list returns
meta.pagination.{page,perpage,total,totalpages,hasnext,hasprev}. A client
written against the documented contract found nothing where it looked, on the
one list whose whole purpose is being read by somebody checking what happened.
It uses paginated_response now.

Also carried here, from the previous batch's working tree: Asset.to_dict emits
ownlocationid/ownmapx/ownmapy/ownlevelid alongside the resolved values, and the
PC form binds to those. The merge of an inherited location is right for a reader
and wrong for a form, which could not tell an owned value from a derived one and
wrote the inherited one back as the record's own.

Scope: product
2026-09-03 16:49:55 -04:00
cproudlock
81ad221e6a Let a typed employee name find the employee
Reported from the floor: somebody types the employee's whole name into the
notification or recognition picker and presses Enter instead of clicking the
suggestion, and the entry does not pull from the employee database.

Two causes, one symptom.

The search matched the WHOLE query against first name or last name, so "Dana
Reyes" found nobody - no single column holds that string. Typing a full name is
what a person does before pressing Enter, and it was the one thing guaranteed to
return an empty list. A multi-word query is now treated as a full name: every
word must appear somewhere in the name, in either column, so "Reyes Dana" works
too, and "Dana Reyes" does not also return Sam Reyes. A single word still
searches broadly, and an SSO still resolves. Both directory backends changed,
the self-hosted tables and the external HR database.

With no suggestion to click, the form then turned the typed text into
`NAME:<text>` - a label linked to nothing. On screen that chip is identical to a
real selection; the difference only shows up later as no photo, no directory
link, and a recognition the person it is about cannot be found by. Both pickers
(the notifications form and the shared EmployeeSearch component) now look a
typed name up before deciding it is free text, through one helper.

An exact full-name match wins over a longer partial one, because somebody typing
"Dana Reyes" means that person and not "Dana Reyes-Smith". A genuinely ambiguous
name is NOT guessed at - the candidates are shown and the operator picks, which
is exactly when picking wrong matters most. A directory that cannot be reached
reports failure rather than silently producing a label.

Eleven tests. The search rules verified by planting the single-term query back.

One note for whoever reads the spec: the failure-branch test uses a thenable
that rejects rather than a mock returning a rejected promise. The runner
collects such a promise as an unhandled error even when the code under test
catches it, and fails the very test that proves the catch works.

Scope: product
2026-09-03 16:45:15 -04:00
cproudlock
cbdfcbabb7 One badge vocabulary, and images a browser can actually load
Two operator-visible defects and three doc corrections, closing the tail of the
2026-09-03 run.

THE BADGE. The two kiosks read the same physical badges and disagreed. The
parts kiosk understood both shapes a scanner emits - the bare SSO and the PayNo
wrapper 0<digits>BZ - while the USB kiosk compared the raw string to an SSO, so
the wrapper resolved to nobody. An operator whose keyboard-wedge scanner emits
it could take a 3D-printed part and could not check out a drive, with the same
badge, at two cabinets a few metres apart.

The shape now lives in shopdb/utils/badges.py on the contract surface, because
a plugin may not import another plugin and because a badge is a property of the
site rather than of a cabinet. Both kiosks use it, and both store the DIGITS
rather than the string that arrived: one row per spelling makes a custody
history that cannot be joined to itself.

That closed a subtler hole in the same place. str.isdigit() is true for
Arabic-Indic and other decimal forms, and int() parses them, so a badge written
in those characters was accepted, matched a REAL employee, and was then stored
as an SSO string nothing else matches. ASCII digits only. An empty scan and an
unreadable one now get different answers, because they are different problems
for the person at the cabinet.

THE IMAGES. /api/applications/image/<filename> and /api/models/image/<filename>
required a permission, and a browser fetching <img src="..."> sends no
Authorization header - so the gate did not protect the file, it guaranteed the
picture never appeared. Both are ungated now, like the four sibling image routes
(branding logo, map blueprint, employee photo, part picture), and registered in
PUBLIC_ENDPOINTS with that reasoning.

/api/printers/lookup?fqdn= never matched anything: both arguments were compared
against Communication.ipaddress, so a hostname lookup 404d on printers whose
hostname was on the record. An FQDN now matches the hostname, with a fallback to
the short name.

The slide manager gained the per-slide hold time. The PATCH route and the feed
have always carried `seconds`; nothing called it, so the one documented
per-slide control was curl-only.

Docs: API-REFERENCE listed the employee lookups and the whole notifications
surface as unauthenticated when they answer 401, and called both kiosks
"deliberately open" when they are off by default and refuse anything without the
site's device token. The installer download says applications.download, which is
what the code requires. api-inventory.json said printers/lookup was
jwt-optional.

Fourteen tests. The badge rule verified by planting the old per-kiosk behaviour:
three fail, one in each kiosk.

Scope: product
2026-09-03 16:25:08 -04:00
cproudlock
8fee09e748 Validate what a form accepts, and keep a diff to one asset
The validation tail from the 2026-09-03 run. Each is small; together they are
the difference between a field that checks its input and one that looks like it
does.

Subnets accepted any string containing a slash, so 'notanip/xx' was stored -
and _devices_in_subnet then swallowed the ValueError from ip_network and
returned an empty device list, so the subnet looked real and permanently empty.
It is parsed with the same library that has to read it later, a prefix length is
required (ipaddress reads a bare 10.1.1.0 as a /32, which is legal and never
what somebody typing a subnet meant), and the stored form is normalised so
10.1.1.5/24 and 10.1.1.0/24 cannot become two rows nobody can tell apart.

VLAN numbers had no range check: 99999 and -5 were stored. Worse, a non-numeric
value reached a duplicate probe that compares against an integer column, which
MySQL coerces to 0 - so 'xyz' matched an existing VLAN 0 and the 409 named a row
that had nothing to do with it. 1-4094 per 802.1Q, checked before the probe.
Deleting a VLAN that subnets still reference now answers 409 like every sibling
in-use guard, rather than 400, which told the caller their request was malformed
when the request was fine and the state was the problem.

An appversion sha256 was stored raw, so an over-long value was truncated to the
column width and came back as 64 plausible hex characters - indistinguishable
from a real digest, and wrong. A digest is a claim the delivery path checks
against, so it is 64 hex characters or it is refused. The releasedate goes
through the strict parser for the same reason it did everywhere else.

PUT /api/settings/<key> ignored the declared valuetype. "A6-not-a-number" went
into an integer key verbatim and every consumer then read 0, because
get_typed_value swallows the ValueError - the setting looked set and the
behaviour was the default.

/api/maplevels told a caller who omitted the level name that 'buildingname and
buildingid are required'; buildingname is a field that handler never reads. It
now names the fields it actually wants.

Restoring a guide revision that is already current writes nothing, because
save() dedups on the content hash - and the response still said "Restored
version N as a new draft version", describing a revision that does not exist
while the unchanged revision list disagreed.

GET /api/backups/revisions/<id>/diff?against=<id> accepted any revision id, so
pointing it at another asset's revision rendered that asset's configuration as
the "before" side of this asset's history: a diff that reads as a change on this
machine and describes a different one. Same asset and same kind, or refused.
The default branch was always asset-scoped; only the explicit one was not.

Two frontend fixes: the per-page selector rendered blank on any list defaulting
to a size not in [10,20,50,100] (network devices uses 25), so the operator could
not see the current page size; and an emptied PC alias was dropped from the
payload instead of sent, so it could not be cleared - the form came back showing
the alias that had just been deleted.

Twenty-two tests. The backups guard verified by planting.

Scope: product
2026-09-03 16:13:59 -04:00
cproudlock
1a8f426e93 Make revoking final, keep password rules in one place, drop a grant nobody read
Three credential and authz findings, all the same shape: a second path to a
decision, with weaker rules than the path built for it.

PUT /api/apitokens/<id> {"isactive": true} un-revoked a token, and the ORIGINAL
secret worked again - so "revoked" meant "paused until somebody flips it back",
for a credential whose secret is assumed to be out by the act of revoking it.
Revocation is one way now. Deactivating through the same route still works;
bringing one back does not, and the refusal says to mint a new token.

PUT /api/users/<self> accepted a `password` field with no current-password
proof and no length rule, while /api/auth/change-password requires both. When
two routes reach the same state the weaker one is the real rule, so the stronger
one was decoration. Changing your own password through the users route is
refused and points at the right endpoint. An ADMIN may still set one - that is
how a temporary password is issued - with the same 8-character floor, and
mustchangepassword is now set, because a password somebody else chose is
temporary by definition.

tools.secrets was declared guest-grantable, so the role grid offered it and
stored it, and the secure-send route never consulted the guest role at all: it
decides on securesend_allow_guests, which fails closed and carries the rate
limit. Ticking the box changed nothing in either direction, so the screen and
the behaviour disagreed. The declaration is gone; tools.view stays, because it
really does gate the nav entry.

Removing it needed care: update_role refuses the WHOLE request when a submitted
permission is not grantable, and the grid resubmits what the role already holds,
so a site whose guest role held tools.secrets could no longer save that role at
all - the deadlock fixed in 723a4ff, re-created from the other end. Migration
7d41 revokes the grant from the guest role, and nothing is lost because it
granted nothing.

Eight tests. The first two verified by planting: remove either guard and three
fail.

Scope: product
2026-09-03 16:03:47 -04:00
cproudlock
f6e5f6ca47 Store and serve a maintenance date as a date
The machine maintenance panel showed every date one calendar day early on this
site, and would show it a day late east of UTC.

lastmaintenancedate and nextmaintenancedate were DATETIME columns holding
midnight, and BaseModel.to_dict serialises a datetime as isoformat + 'Z', so
2026-12-15 00:00:00 went out as "2026-12-15T00:00:00Z" and the browser
converted it into local time. There is no time in a maintenance date: the form
offers a date picker, the CSV importer takes '2026-06-01', and the value is only
ever compared against other dates. Every sibling already models it correctly -
measuringtools.lastcalibrationdate, warranties.startdate and enddate are all
db.Date - so machines were the odd one out.

Migration 7d40 converts both columns; MySQL truncates a midnight time so no
data moves, and SQLite neither enforces nor needs it.

That exposed two more things. The route was assigning the raw request string to
the column, which a DATE column rejects outright - it now goes through the
strict parser from wave 2, so a mistyped maintenance date is refused with a 400
rather than stored or nulled. And BaseModel.to_dict had no branch for a plain
date, so one reached Flask's encoder and came out as an HTTP-date
("Thu, 15 Jan 2026 00:00:00 GMT") - which a browser parses back into a moment
and can still render as the day before. Dates now serialise as "2026-01-15",
with datetime checked first because it is a subclass of date.

Verified against MySQL on a live instance: the column is DATE, the API serves
2026-01-15, a mistyped date is refused with 400, and a rename on a machine that
has maintenance dates saves - the 500 from the audit-log fix in 5d81f50c, now
confirmed end to end rather than only in tests.

Scope: product
2026-09-03 15:51:22 -04:00
cproudlock
61a1c0b4fb Answer API errors in the envelope, say when supplies are unread, badge each chain
Three more findings from the 2026-09-03 run.

Werkzeug answered 405, 415 and a malformed JSON body itself, in HTML. A client
written against the documented {status,data,meta} contract got a page of markup
with no error code, which is indistinguishable from the server having fallen
over. The 405 had a second edge: on a route the caller cannot otherwise see, a
method-not-allowed CONFIRMS the path exists, so a gated kiosk route meant to
read as absent announced itself by refusing the wrong verb. All three now
answer in the envelope, and only under /api/ - everything else is the SPA and
keeps the framework's behaviour, deep links included.

The Toner Report rendered "0 Printers Checked / 0 Low / 0 Critical" and "No
printers match the selected filter" while the API it had just called said
supply monitoring was unreachable. Zeros read as a clean bill of health for a
fleet nobody has looked at. The page now carries the reason the API gave and
suppresses the counts, because a number nobody measured is not a result. Same
defect shape as the supplies card fixed on 2026-08-29; `unavailable` was
already in the payload and the page ignored it.

Backup revisions badged "current" per (asset, kind). NTLARS settings describe a
MACHINE but are read off the controlling PC, so a machine with two reporting
PCs has two independent chains of the same kind - and only the higher revision
id was badged, so the second PC's newest revision showed as history and lost
its last-verified stamp. The machine looked like one source re-reporting rather
than two. sourcehostname is the chain; a revision without one forms its own,
which keeps the old behaviour for everything that has only ever had one source.

Nine tests. The envelope ones verified by planting: remove the three handlers
and three fail.

Scope: product
2026-09-03 15:21:58 -04:00
cproudlock
2fb924f18e Let a printer driver carry the name Windows actually matches on
The driver form and the API both dropped `drivername` and `vendorid`. POST
returned 201 with both null, PUT accepted them and stored nothing, and no
screen offered either field - so there was no interface in the product that
could set them.

That is not a cosmetic gap. `drivername` is the exact string the INF declares,
and Add-PrinterDriver matches on it; `name` is ours to choose and means nothing
to Windows. With it null the shipped converger has nothing to match and
installs nothing, and the drift check that compares an observed driver name
against the expected one could never fire, because the expected value was never
stored. `vendorid` is how a universal driver (HP UPD, Xerox GPD) attaches to a
whole make instead of needing a near-duplicate row per model.

Both are now read on create and update, and the driver form gained the two
fields with hints saying what they are for - the INF name box says plainly that
leaving it blank means the converger cannot install that driver. A whitespace
value clears rather than stores: an empty box means "not known", and a blank
string would look set and match nothing.

Three tests. Verified by planting: drop the two fields from the create path
again and the round-trip test fails.

Scope: product
2026-09-03 15:16:33 -04:00
cproudlock
b1e5dd734d Draw a dual-bay secondary where it actually is, and make two counts add up
Three map and aggregation defects. The first is a behaviour change you asked
for; the other two are the 2026-09-03 run's findings.

A dualpath secondary bay was excluded from /api/assets/map unconditionally
while the collapse setting is on, so positioning it was a silent no-op: the
form took the pin, stored mapx/mapy, and the map never drew it. One marker per
pair is right when the pair sits in one place, and that stays the default - but
a dual-bay machine whose second bay is physically elsewhere is exactly what
positioning it separately means. Now: a secondary with its own position draws
its own marker; one without is represented by the primary's, which carries the
combined '2007 / 2008' label. Nothing extra is needed for the fallback, because
the feed already requires mapx/mapy - an unpositioned bay never reaches the
loop.

The combined label is now suppressed when the partner draws its own marker.
Labelling two separate pins '2007 / 2008' would say one machine is in two
places, which is worse than either bay being unlabelled.

Verified against a live instance: unpositioned secondary gives one marker
labelled with both bays; positioned gives two markers each with its own name;
clearing the position puts it back to one. That third case matters - a
one-way door would be its own bug.

machines/dashboard/summary applied the dualpath exclusion to total and bytype
but not to bystatus, so the status bars summed to more than the headline above
them, by exactly the number of secondary bays. A breakdown that does not add up
to its own total is an afternoon of reconciling.

The measuring-tool calibrationstatus filter ran AFTER pagination. It is a
derived value so it cannot be a SQL filter, but applying it to the current
page meant asking for 'overdue' with one overdue tool in sixteen returned an
empty page 1, a total of 16, eight pages, and the row itself on page 3. When
that filter is in play the matching set is now built and filtered before
paging - the same tradeoff the old comment named, spent on correct answers
instead of a cheaper query.

Seven tests. The pagination one verified against the committed version, which
fails it.

Scope: product
2026-09-03 15:07:49 -04:00
cproudlock
1d2bbb3630 One key for audit retention, and stop the SAML page claiming a feature exists
Wave 6: two settings pages that told an administrator the site was configured
when it was not. That is worse than a missing feature, because nobody goes
looking for it.

Audit retention had TWO keys. shopdb/core/services/auditretention.py reads
`audit_retentiondays` (category site, default 0). A second `audit_retention_days`
was seeded under category audit with a default of 90, and the Audit & Logging
page wrote to that one - so an administrator set a period, reloaded, saw it
persisted, and nothing ever consulted it. Retention stayed off. The page now
writes the key that is read, the duplicate is out of the seed list, and
migration 7d39 carries a value an operator actually set on the dead key over to
the live one before deleting it: their intent was "keep 30 days", and the fix
should not silently discard it. The page's copy also claimed logs "will be
automatically purged", which nothing does - purging is manual, from the site
settings page or `flask audit purge --apply`.

The Authentication page offered a full SAML configuration - metadata URL, entity
id, ACS URL, auto-create, admin group - and reported "Configured - SAML login
available" once two fields held text. There is no SAML anywhere in the codebase:
no login route, no ACS, no assertion handling. SAML IS planned, so the settings
stay and the page keeps them; what changes is that it now says plainly that
nothing consumes them yet, and the status line never claims availability.

The dangerous control there was "Allow Local Login". Turning it off did nothing
at all, so an administrator could believe they had closed local password login
while it stayed wide open. It is locked on until there is another way in, and
says why. CONFIG.md marks the whole block reserved rather than describing it as
working configuration.

Scope: product
2026-09-03 14:53:57 -04:00
cproudlock
ffced2f822 Keep a share path inside the share, a UTC timestamp in UTC, and a public map public
Three confirmed findings from the 2026-09-03 run, each independent.

GE-Enforce scope names are directory names on the imaging share, and create_scope
checked only that the name was non-empty. "../escape" wrote a manifest one level
above the share root, "../../escape2" two, and "/tmp/anywhere" wherever it liked
- proven by writing the files and then deleting them. The exposure is the
permission split rather than the traversal itself: geenforce.manage creates the
scope and geenforce.publish writes the file, so a manage-only author could aim a
publisher's write at any path that process can reach. Names are now a single
safe path segment, checked at creation AND again at the write (rows predating the
check exist, and the write is the line that acts), with the resolved target
confirmed to sit under the share root.

Agent timestamps were converted with astimezone(tz=None), which means the
SERVER's local zone, not UTC. An agent reporting 17:26:16Z was stored as
13:26:16 on this EDT box and re-served labelled 'Z', while the same row's
server-written lastreportedat stayed true UTC - one row carrying two time bases
four hours apart. The dashboard's 48-hour stability window compares a real UTC
cutoff against these values, so events left it four hours early. Invisible on a
UTC server, which is how it survived, and nothing in the Dockerfile, the compose
files or the Windows deploy pins TZ. AGENT-CONTRACT.md mandates UTC on the wire,
so a naive value is now taken as UTC rather than guessed at.

The three /api/maplevels reads went back to optional-auth. ADR-017 rule 6, the
API inventory (auth "none"), the module's own docstring and the comment in
PrinterInstallerMap.vue all say they are public, because the printer installer
map runs before anyone logs in and needs a blueprint to draw on. afce2437 swept
165 genuinely-open GET routes behind require_permission and caught these in the
net, so the anonymous installer map has drawn no floor plan since. Restored
rather than amending the ADR: blueprint paths and level names are not secrets,
and the asset positions that go on top already render on public kiosk pages.
The writes stay gated, and a test proves it.

Registered in PUBLIC_ENDPOINTS with the reason. That gate checks a public route
is DECLARED public; the new test checks it actually answers, which is the half
that was broken.

Ten tests. All three verified by planting: restore the non-empty scope check and
eight fail, restore the local-zone conversion and two fail, put the permission
back on the level list and one fails.

Scope: product
2026-09-03 14:34:12 -04:00
cproudlock
5d81f50c33 Stop the audit log from being what fails the write
Wave 4, second half. PUT /api/machines/<id> returned 500 and rolled the whole
edit back for any machine with a maintenance date recorded - including a rename
that never touched the date. The Machine edit form could not save such a
machine at all, which is most of them once a maintenance schedule exists.

The route builds {'old': getattr(row, field)} straight off the model, so the
changes dict carries whatever the column holds: a date, a datetime, a Decimal.
AuditLog.changes serialised it with a bare json.dumps, which raises on all
three, and nothing caught it. The audit entry and the edit share a transaction,
so the failure took the edit with it.

json.dumps now has a fallback encoder: dates and datetimes become ISO strings,
Decimals become floats, sets and bytes get something readable, and anything
else becomes its repr rather than an exception. An audit entry that reads a
little awkwardly is worth more than a failed write - the alternative, and what
happened, is losing the change AND the record of it.

Fixed at the audit log rather than in the machines route: every audited route
builds its changes dict the same way, so the next one to audit a date field
would have hit the same wall.

Four tests, including the exact shape the machines route produces. Verified by
planting the bare dumps back: two fail.

Scope: product
2026-09-03 14:25:55 -04:00
cproudlock
13e992d863 Ask what the caller may do, not what its owner may do
Wave 4, first half. A personal API token scoped to endpoints.command alone
could queue system_restart, run_script and update_app against a shop-floor PC.

require_permission has always suspended the admin bypass for a scoped token,
and tests pin that. What nothing covered was a route checking a SECOND
permission by hand: the endpoints command route asked
current_user.haspermission(elevated), which reads the token's OWNER. An admin
minting a narrow token for a script therefore handed out remote restart, script
execution and app deployment - the exact bypass require_permission exists to
prevent, reintroduced one hand-rolled check at a time.

caller_holds is now scope-aware, so the answer is about the caller rather than
the person behind it: the owner must hold the permission AND, when the request
carries a scoped token, the scope list must name it. An unscoped token is the
person and is unchanged; a session is unchanged; guests still answer from the
guest role.

Both hand-rolled checks in the tree now use it - the endpoints elevated gate and
the secure-send tools.secrets check, which had the same shape and the same
consequence at a smaller scale.

Two tests, one proving the refusal and one proving an unscoped token still
passes (a fix that quietly broke every service token would be worse than the
bug). Verified by planting the owner-based check back: the first fails.

Reported by the fleet-agent lane of the 2026-09-03 test run and confirmed by an
independent verifier that minted its own token and queued its own commands.

Scope: product
2026-09-03 14:23:10 -04:00
cproudlock
debef39a18 Answer a nonsense filter with a 400, and sweep every list route for the rest
Wave 3. Six findings, one cause: `int(request.args.get('typeid'))` with nothing
catching the ValueError. The caller sent a bad argument and got
500 INTERNAL_ERROR plus a traceback in the log, which says the server is broken
when the truth is that 'abc' is not a number.

shopdb/utils/queryargs.intarg reads it and raises QueryArgError, which an
app-level handler answers as a 400 naming the argument. App-level on purpose: a
route should not have to remember, and forgetting was the entire defect. The
policy differs from get_pagination_params, which falls back to a default for a
bad page number - a nonsense page is harmless and the caller still wants their
list, but silently ignoring a nonsense FILTER returns rows they believe they
excluded.

The gate matters more than the six fixes. tests/test_core/test_bad_query_args.py
sweeps all 131 registered no-parameter GET routes under /api/ with twelve
argument shapes and fails on any 5xx. It found sites the test run had not
reported - statusid and locationid on /api/assets, the second asset-list route,
limit on the employee lookup and the printer supply history - because the
pattern is how a list filter gets written rather than a property of any one
route. Verified non-vacuous by planting the old `int(...)` back into
/api/machines: four tests fail.

Four usb routes are skipped by exact path, with the reason: they reach the
EXTERNAL cmmc_usb database, so their 500 in a test environment is a missing
database rather than an argument fault. That is the complete set - every usb
GET route calling cmmc_usb_connection - not a convenient subset.

One correction worth recording: the first pass rewrote every
`if x := request.args.get(...)` in the touched files, including text arguments,
so a search for 'ZZNOTHINGMATCHES' briefly became a 400. Caught by
test_list_search_identifiers. Only id-shaped arguments are integers.

Scope: product
2026-09-03 14:18:49 -04:00
cproudlock
12392a361e Refuse a value the database cannot store, instead of quietly changing it
Wave 2 of the remediation plan. Eight findings from the feature test run were
the same defect twice: a write that reports success while storing something
other than what was sent.

The first half is a date parser that swallowed its own error. Two plugins
carried an identical private _parse_date returning None on ValueError, so an
unparseable date and a deliberately cleared one produced the same write.
PUT /api/warranty/<id> with enddate '30-06-2027' answered 200 "Warranty
updated" and stored NULL, dropping the record out of every expiry surface on
the strength of a typo; the measuring-tool calibration date did the same and
flipped the tool to 'unknown', out of the calibration report and the map
overlay. shopdb/utils/datevalues.py now holds one strict parser on the contract
surface: an explicitly empty value clears the field, anything that is not a
date is refused with a 400 naming the field, and those are different intents
with different answers. The vendor-lookup paths keep a lenient wrapper on
purpose - a provider changing its date shape must not fail the refresh of an
otherwise good record, and no person typed that value.

The second half is why the truncation findings were possible at all. MySQL was
running non-strict: sql_mode on the 5.6 server is NO_ENGINE_SUBSTITUTION with
no STRICT_TRANS_TABLES, so it accepted writes it could not store and adjusted
them. A 257-character username became its first 100 characters (and the next
distinct one then collided with it); an over-long asset number was cut at 50; a
malformed releasedate became '0000-00-00' and permanently broke every read of
that application. All with a 201. The mode is now set on the CONNECTION rather
than left to the server, so a site whose my.cnf disagrees still gets it.

That trades silent corruption for a DataError, which uncaught is a 500 - and a
500 says the server is broken when the truth is that the value does not fit. An
error handler maps it to a 400 naming the column where MySQL names one.
Verified by hand against MySQL 5.6: POST /api/users with a 257-character
username now answers 400 "That value is too long for username." and stores
nothing, where it previously stored a truncated row and answered 201.

Twelve tests. The date guard is verified by planting: restore the swallowing
parser and four of them fail. The DataError tests raise the error directly
because the suite runs on SQLite, which does not enforce VARCHAR length at all -
there is no value that would trigger it there, which is precisely why this went
unnoticed.

Scope: product
2026-09-03 13:58:41 -04:00
cproudlock
aa8028f96e Let the row refuse the write both kiosks were deciding in Python
Wave 1 of the remediation plan for the 2026-09-03 feature test run. Two kiosks,
one shape of defect, one critical consequence.

Both kiosks read the state, decided in Python, and then wrote. Nothing stops two
requests interleaving between the read and the write, so both see the same
pre-state and both proceed. Reproduced on MySQL 5.6 over HTTP, five concurrent
takes of 5 from a bin holding 5: all five were accepted and quantityonhand
landed on -20, because each request wrote its own arithmetic. The ledger had
recorded every one of them, so the cached quantity and the ledger disagreed
permanently, which is the number the shop floor actually reads. The USB kiosk
had the same shape: concurrent scans of one drive all succeeded and inserted
several open custody rows, so one drive was recorded out to two people and the
documented 409 never fired.

The comparison now lives in the WHERE clause, where the database settles it at
write time. printedparts._move_quantity issues
UPDATE ... WHERE quantityonhand >= :n and treats rowcount 0 as the refusal,
raising InsufficientStock, which take and adjust turn back into the message they
already gave - carrying the quantity the database holds now, not the stale one
the request read. The low-stock alert derives its before-value from what the
write produced for the same reason. Both USB paths claim the device with
ischeckedout (self-hosted) or status (external cmmc_usb) in the WHERE clause;
the external one claims BEFORE writing the log line, so a refused claim leaves
nothing to undo. Check-in is guarded the same way.

After the fix, the same five concurrent takes: one accepted, four refused,
quantityonhand 0 and the ledger sum 0.

Check-in also closes EVERY open row for the device rather than the newest.
One device can only be out once, so a second open row is corruption, and
closing only the newest left the older ones open forever - the device then read
as returned while its custody history said otherwise.

Separately, usbcheckouts now records who handed a drive back
(checkinsso/checkinname, migration usb0004checkinidentity). Returning for a
colleague is a supported flow that the kiosk asks to confirm, but the row it
wrote attributed the check-in to the HOLDER, so the custody history and the
audit report both named somebody who was not there. Nullable and deliberately
not backfilled: copying the holder's badge into old rows would manufacture
evidence for the exact claim this stops making.

Four tests. Both guards verified by planting: strip the condition out of either
UPDATE and its test fails.

Scope: product
2026-09-03 13:58:26 -04:00
cproudlock
377cc69264 Make a partly-undone import retryable, and refuse a plan it cannot deliver
Some checks failed
CI / naming (push) Successful in 3s
CI / frontend (push) Successful in 12s
CI / migrations-mysql (push) Successful in 41s
CI / backend (push) Failing after 4m44s
Five findings from the review of the WINC import, plus the rendering each fix
needed to be usable.

An undo that hit a conflict wrote status 'undone' anyway, and undo_batch returns
immediately for an undone batch. So the one entry that needed a retry was the
only one that could never get it: the human resolves the conflict, clicks Undo,
and gets a no-op. A conflicted undo is now 'partially-undone', undoneat is
stamped only when the batch is genuinely finished, and the batches table offers
"Retry undo" for that status. The column was String(16) and the new value is
exactly 16 characters, so migration 7d38 widens it to 32 rather than rely on an
exact fit.

_UNDO_REFERENCES listed only assets, so a business unit this batch created was
hard-deleted even when something else still pointed at it. Two of the three
missing referrers are real FKs (dashboarddefaults.businessunitid,
businessunits.parentid): MySQL raises and the whole undo rolls back, so the
batch cannot be undone at all. The third, notifications.businessunitid, has no
FK, so the delete succeeded and left a notification pointing at an id that no
longer exists - silently, on both engines. All four are listed and the guard is
tested through the route.

The reference guard also queried tables that need not exist. A lean site drops
the tables of plugins it does not install (ADR-014), and notifications is
plugin-owned, so on such a site the query raised 1146, rolled the undo back and
left the batch permanently un-undoable. Absent tables are skipped; a table that
does not exist cannot hold a reference.

_PACKET_GROUPS reports on location, but apply_plan only ever creates business
units - it sets locationid only when the location already resolved. A plan
needing a new location asked the admin to acknowledge it, got the tick, wrote
nothing, and reported nothing. build_packet now takes the groups the writer can
actually create; anything else needed is 'uncreatable' and blocks at plan time,
where the admin can act on it, rather than dying at commit on a message about
acknowledging something the screen had no control for. The writer refuses the
same case independently. Both the console packet and the import page name the
blocking groups: setting blocking without printing the reason would leave the
banner pointing at nothing.

The web undo route caught ValueError only, so refusing a batch belonging to
another importer surfaced as a 500. That refusal is new too: the journal is a
general primitive and undo_batch does not filter on kind, so any other
importer's batches were reversible through this endpoint, carrying WINC's
references map - which lists none of that importer's tables, making every
reference check pass and every delete unconditional.

Nine tests. The two guards were verified by planting violations: dropping
dashboarddefaults from the references map, and passing creatable=None, each fail
the test written for them.

Scope: product
2026-09-03 09:09:55 -04:00
cproudlock
405a2ef919 Cap request bodies, close a hole in the gate, fix to_dict and two settings pages
Six fixes that were ours regardless of what the sister site does. Four came out
of our own reviews; two you reported from the settings pages.

MAX_CONTENT_LENGTH was set nowhere. Every upload route caps its own payload, but
each of those checks runs after the body has arrived: the first touch of
request.form or request.files makes Werkzeug parse the whole multipart and spool
the file to a temp file, so a route can reject the content and still have paid
for the transfer and the disk. Three separate reviews found that shape in three
different routes, which makes it a property of the app rather than of any
handler. 128 MB, clearing the 64 MB agent binary with room for the envelope.

The naming gate excluded devnotes with --exclude-dir, which matches a directory
of that NAME at any depth - so a force-added plugins/<plugin>/devnotes/ was
invisible to every check in the file, ADR-015 included. Replaced with a path
anchored post-filter. Verified by planting the same site literal in root
devnotes/ AND in plugins/endpoints/devnotes/: the gate now reports exactly one,
the nested one. Before, it reported none.

BaseModel.to_dict walked __table__.columns and published every one, so a model
without an override published whatever it carried - including columns added
later by someone not thinking about this method. User has no override, so
User.to_dict() returned its bcrypt hash. Nothing calls it in a response path
today, which is luck rather than design. A blanket allowlist would empty the
output of all 34 models relying on the default, so this pins a floor instead:
named secret-bearing columns are never emitted, and a model may extend the set.
Eight tests, including a sweep over every mapped model.

ActionPicker's Esc handlers had .prevent but not .stop, so the event bubbled to
the wrapper and closed the whole picker instead of returning to the monitor list.

The branding page reported "error loading image" on the site and badge logos.
Two bugs stacked. The load failure was real: the preview used the stored value
as a raw src, and that value is root-relative, so under /shopdb or /ops it
resolved against the SERVER root and 404d - siteSettings.js wraps every logo in
withBase() and this page was the one place that did not. With that fixed two of
three were still invisible, because the shipped GE mark is fill="white" for the
dark sidebar and was rendering white-on-white. The preview tile now sits on a
mid-tone checker so artwork of any colour reads against it.

The floor-map levels table ran off the right with its cells crushed - nine
columns of inputs, no scroll wrapper, and .data-table is styled nowhere. My
first attempt reached for the global .table-container and made it WORSE: that
class also sets white-space: nowrap, and a full sentence of hint text lives in
one of the cells, so nowrap forced the table wider still. It now scrolls in a
local wrapper, the hint wraps, and the name input has a width instead of
collapsing to one character.

Suite 2377, frontend 324, gate green.

Scope: product
2026-09-03 08:38:47 -04:00
cproudlock
13be0ef90f Import the wilm agent store and WINC adoption work, with four fixes
Eleven commits from the sister site: a reversible Maximo/WINC machine-adoption
import, a versioned agent binary store with upload and rollback, remote-BuildKit
CI, tray version reporting, and UI fixes. Applied as a patch again - the
published mirror rewrites hashes, so the branches have unrelated histories and
git refuses to merge them.

Reviewed by three agents before merging. Four things fixed here.

THE PUBLISH WORKFLOW WAS DEAD. The new remote-BuildKit step in
publish-images.yml sits at column 0 instead of indented under steps:, so the
whole file is invalid YAML and GitHub would refuse it. Their entire GHCR
distribution channel - which Start-ShopDBDev.ps1 and the compose files consume -
was broken at the source and clearly never run. Indented; all five workflows now
parse.

THE IMPORT'S APPROVAL CHECK COULD NOT FAIL. commit() built the plan and the
approval packet from the same upload and then asked the writer whether they
matched, so packet_matches_plan compared a hash against itself. The documented
contract - "a human approves a specific, frozen plan, and the writer can prove
nothing drifted" - did not exist in the web path: an admin could preview file A
and commit file B, gated only by the needs-create group NAMES they ticked. The
preview's planhash is now required at commit and checked against a freshly
rebuilt plan. Their tests committed without previewing, which is what let this
through; they go through the real flow now, and three new tests pin it - all
confirmed to fail with the check stubbed out.

endpoints.agentpackage QUIETLY GREW. It used to mean "wrap the staged exe in a
ZIP"; with the upload routes it also means "choose the executable every future
package installs and runs as SYSTEM". Before this, staging needed filesystem
access to the server. The permission description still said only "generate and
download a package", so an admin granting it could not see what they were
granting. Reworded.

NOTHING RECORDED WHAT WAS STAGED, OR BY WHOM. The manifest held version, size
and timestamp; re-uploading a version replaces the bytes in place under the same
label. A sha256 of the stored file is now recorded, and the upload is
audit-logged with it - after an incident there was previously no answer to who
put that binary on the fleet.

Also documented the nine new routes in docs/api-inventory.json (that gate cannot
run on the mirror, so it falls to us at promotion time again), regenerated the
spec and the project map, and kept our own docs where they conflicted: docs/ is
scrubbed from the mirror, so their DEV-BUILD-TEST.md still had the dev-be
channel mapping we corrected, and their PLUGIN-HOOKS.md was a new 45-line file
rather than our 737-line canonical one. Their journal section is appended to
ours.

Contract version comes across as 0.30.0 for a genuinely additive surface.

Suite 2369, frontend 324, gate green.

Scope: product
2026-09-03 07:54:14 -04:00
cproudlock
7f6aaf2e64 Gate the USB kiosk the same way, and make an unreachable setting a test failure
The USB kiosk had a feature flag and nothing else. A flag is a decision, not a
credential: with the kiosk on, anyone who could reach the app could move a drive
between checked-out and checked-in and sign it with a badge they guessed. Same
reasoning that closed the parts kiosk, smaller blast radius - which is why
Darkwing did not raise it and why it was worth asking about anyway.

Both kiosks now share one mechanism. The device token moved out of the
printedparts api wrapper into a request interceptor that attaches X-Kiosk-Token
by URL prefix, with one localStorage key per kiosk - they are separate screens
in separate rooms, and a shared token would mean provisioning one grants the
other. claimKioskToken() is shared too, so both views take the token from a
?token= URL once and strip it from the address bar identically.

Then the part that matters more than either fix. I told you to set the new
settings in the UI, and that was impossible: every plugin settings page carries
a hardcoded key list, and nothing had added them. printedparts_kiosk_enabled,
printedparts_kiosk_token and usb_kiosk_token were declared, seeded and read
correctly, and rendered by no page - changeable only by writing SQL.

tests/test_plugin_settings_reachable.py now fails when any declared setting
appears nowhere in its plugin's frontend. It found seven MORE that were already
in that state, all pre-existing:

  collector_corporate_ranges          computers_configpolicies_enabled
  computers_machinelink_alerts        computers_quietreporthours
  geenforce_reportstaleminutes        printers_dashboardpercent
  warranty_expiringdays

All seven now have fields. I first parked them in a KNOWN_GAPS ratchet, then
fixed them and deleted the ratchet - a backlog list in a new check is a way of
agreeing the check does not apply yet. There is no backlog; the check is
absolute.

Two of my own mistakes on the way, both caught by tests rather than by reading:
the blanket rename of the USB guard also rewrote the call inside _kiosk_open, so
it called itself until the stack ran out; and the first version of the reachable
check skipped all 17 plugins because it looked for get_settings() when the
method is get_settings_defaults() - it passed by testing nothing, which is the
exact shape it exists to catch.

Suite 2336, frontend 316, build clean.

Scope: product
2026-09-02 18:08:34 -04:00
cproudlock
744bc4341a Shut the parts kiosk behind a device token, and stop the health probe leaking
Both from the second Darkwing scan. Every claim in it was verified against the
code before acting.

The kiosk was the product's only unauthenticated write, justified by a decision
record saying it was "physically rate-limited: it serves a touch screen on the
shop floor". Nothing in code enforced the physical half, and an HTTP route is
reachable from wherever the app is. What that left open:

  - _kiosk_find_item falls back to matching bare digits against the numeric
    tail of every active item, so 1, 2, 3 walk the catalogue without knowing a
    barcode;
  - a badge is a short numeric SSO, and the route answered 422 for an unknown
    one and 200 for a real one - an oracle for sorting guesses into employees;
  - the write then decremented stock and stamped the ledger, which IS the audit
    trail, with whatever SSO the caller supplied;
  - quantity was bounded only by what was on hand, so one request emptied a bin.

A badge is attribution, never authorization. Both kiosk routes now need the
site's kiosk flag AND a per-device token, and 404 rather than 403 without them
so a caller does not learn the door is there. The READ is gated too: gating only
the write would have left the enumeration oracle running against a kiosk a site
had switched off.

The token is per-DEVICE, not per-page, because a kiosk is a browser on a touch
screen and a token served inside the page is readable by anyone who can load it.
An admin opens the screen once as /parts-kiosk?token=..., the page moves it into
that browser's localStorage and strips it from the address bar.

Also: quantity is capped per take, JSON true no longer passes as 1
(isinstance(True, int) is True), and every badge failure on the anonymous route
returns one message. The gated restock and adjust handlers keep the specific
message - there the person is known and the detail is help, not an oracle. I
changed the wrong one of those three handlers first; caught by the test.

/api/dashboard/health formatted str(e) straight into the body of a route
PUBLIC_ENDPOINTS declares open because "monitoring has no account". A pymysql
OperationalError names the database account, the app host's address, the driver
and the failing statement. It is logged server-side now and the body carries a
coarse status. The version goes with it - it told an anonymous caller which
release to look up. Status stays 200 either way: a monitor keyed on the body's
status field is the contract here, and turning a degraded database into a
transport failure is a separate decision from not leaking the exception.

Nine tests. The three gate tests were confirmed to fail with the gate stubbed
out, rather than assumed to cover it. Suite 2325, frontend 316, build clean.

Scope: product
2026-09-02 17:42:54 -04:00
cproudlock
e98e15259e Unpick one site's conventions from the shipped bundle, and bind the launch-path
check to the file it actually runs

Two clusters the five-agent review found. Both are about a product that another
plant has to be able to install.

ADR-015: the shipped monitor bundle was the originating plant's fleet
convention frozen into product seed.

  The business policy was seeded as "RPCA Shopfloor App Monitors". RPCA is that
  plant's facility designator, the row is source='shipped', and the API refused
  to rename it - so every adopting site would carry another plant's label at the
  top of its policy list forever. 0007 now seeds the neutral name, and 0010
  renames it on installs that already stamped 0007, guarded so it cannot collide
  with a policy a site made itself. Downgrade is one-way on purpose: putting
  another site's designator back is not a state worth reaching.

  Which PC type the bundle attaches to was the literal 'Shopfloor PC'. A plant
  whose type is "Shop Floor Workstation" silently got no shipped monitors at
  all, and one using that name for something else got all of them. It is a
  setting now, defaulting to the seeded name so nothing changes here, and a
  blank value attaches the bundle to nothing - the right neutral answer for a
  site that does not want it.

  Shipped rows refused every edit, including isactive and launchpath. A plant
  whose eDNC is on D:, or that runs no UDC, could not correct the path or switch
  the monitor off, and then carried permanent false drift for it. ADR-015 allows
  seed data BECAUSE a site can edit it afterwards. Paths and the active flag are
  now editable; kind and displayname stay fixed, because that pair is the
  identity 0007's bundle matches on.

  Also corrected two false claims in 0007's docstring: the policy tables belong
  to the computers plugin, not the core baseline, and a guarded no-op does NOT
  reconcile later - Alembic stamps the revision either way, so the bundle would
  simply never seed.

LaunchPathPolicy was purely lexical. GetFullPath is string normalization that
never touches the disk, so the root check constrained the NAME while File.Exists
and CreateProcess followed reparse points to somewhere else. A junction under a
writable directory in an approved root - creating one needs no privilege, only
write access to the parent, and vendor trees under Program Files are not
reliably locked down - turned WebUI monitor-edit rights into code execution as
the console operator. It now resolves the real target, leaf and directory chain
both, and re-checks THAT against the roots, rejecting if resolution fails.

Alternate data streams passed the whole policy on net8.0: an interior colon no
longer throws in GetFullPath, so "readme.txt:payload.exe" reported an .exe
extension, passed the root check, and existed. Rejected explicitly now.

Six tests added for the shipped-row and pctype behaviour. The Core project
compiles clean. The launch-path change has NOT been exercised against real
junctions, ADS or 8.3 names - that needs the win11 VM, and per the reviewer it
wants a planted-violation suite there rather than my say-so.

Still open from the review: monitor revocation does not take effect until agent
restart, ActionPicker is keyboard-inoperable, the lightbox has no focus trap,
one attached monitor grants generic any-target process control, and screenshots
are retained unencrypted in the user profile.

Scope: product
2026-09-02 15:44:17 -04:00
cproudlock
b5540e631c Merge the wilm endpoint-monitor and channel-distribution work into main
Six commits from wilm-import: the squashed import, route documentation, the
channel remap onto our tag-and-trunk model, the project map, parallel test
support, and the four smaller review findings.

Merged rather than cherry-picked: the published mirror rewrites hashes, so the
commits do not apply one at a time against our history even though the squashed
diff applies clean.

Scope: product

# Conflicts:
#	docs/PROJECT-MAP.md
2026-09-02 15:19:55 -04:00
cproudlock
720f4ddbb2 Close the four smaller findings from the wilm import review
None blocked the merge; all four were mechanisms that did not mechanise.

_RUNNING_STATES existed twice, in api/routes.py and services/reconciler.py, with
a comment asking a human to keep them in lockstep. The panel and the reconciler
read the same agent snapshot, so a drift between the copies would paint a
service green and flag it as drift on the same page. One definition now, owned
by the reconciler and imported by routes - api -> services is the direction the
plugin layout already runs in, and the reconciler's import of routes is deferred,
so there is no cycle.

The operator-report upload checked its 10 MB cap AFTER upload.read() had already
pulled the whole body into memory, so the check could not prevent the thing it
was there to prevent. content_length is now an early out and the read itself is
bounded to cap+1, which is enough to know the cap was exceeded without taking a
body that lied about its length. The multipart envelope carries the other form
fields too, so the early out allows 64 KB of slack rather than rejecting an
image that is just under.

The same handler forced the extension to .png and the mimetype to image/png
whatever the bytes were, then served the file back under that type. The upload
headers (CSP sandbox + nosniff) make the document case inert, so this was never
the hole it looks like, but nothing is gained by storing a file we already know
is not what it claims. The PNG signature is now checked.

The lightbox bound Escape and the arrow keys to a div and never focused it, so
every one of those bindings was dead until the user happened to click the
backdrop. It takes focus on open and hands it back to the thumbnail on close.

On the two upload tests, honestly: the PNG one was confirmed to fail with the
check removed. The oversize one passes either way, because the old code did
reject an oversized image - it just did so after reading all of it. That test
pins the response, not the fix. Whether the read is bounded is not observable
through the response, so nothing here proves it; the change stands on reading
the handler.

Suite 2311 passing, frontend 316 passing, build clean.

Scope: product
2026-09-02 15:07:50 -04:00
cproudlock
30b7a5a8bf Correct the claim that the two kiosks are gated to the same bar
Darkwing's reconnaissance flagged POST /api/printedparts/kiosk/take as an
unauthenticated write reaching a ledger commit and an outbound mail/webhook
trigger. Analysis dropped it, correctly: it is a declared exemption with a
decision record. Nothing to fix in the code.

The comment beside it was wrong, though. It described the USB kiosk writes as
open at the "same bar as the parts kiosk" while listing a gate the parts kiosk
does not have: usb_kiosk_enabled, off by default, every route a 404 until a site
sets it. grep -rn kiosk_enabled plugins/printedparts/ returns nothing.

So a site that adopts the product and has no parts kiosk still serves a live
unauthenticated inventory-decrement endpoint with no switch to turn it off, and
the registry that is supposed to be the record of these decisions said the
opposite. Whether printedparts should gain the same flag is a behaviour change
for a kiosk that is in daily use here, so it is left as a decision rather than
made silently; this only stops the comment misleading the next reader.

Scope: product
2026-09-02 13:54:06 -04:00
cproudlock
f3a291a28d Stop a request-supplied host reaching the generated installer script
Found by the Darkwing scan of main (run 33654922078). Its own review dismissed
the finding as not remotely exploitable and it is right about that: nothing can
make a victim's browser attach a custom header, and deploy/windows/web.config
does no output caching. It still recommended the hardening, and the reasoning
for the dismissal is a property of OUR IIS config, not of the product - every
site runs its own instance behind its own proxy under ADR-004.

_batch_base_url() interpolated a caller-supplied host into the single-quoted
PowerShell -Uri literal of the .bat this route hands out. site_base_url ships
blank, so that fallback is the default path on a fresh install, not an edge
case, and GET /api/printers/install-batch is public.

Three rules now:
  - the host is validated as a hostname before it is interpolated, so the quote
    that would end the literal never reaches it,
  - X-Forwarded-Host is only believed when TRUSTED_PROXY_COUNT declares a proxy
    -- the rule shopdb/utils/clientip.py already applies to X-Forwarded-For.
    That was centralised for this exact reason and did not cover the Host
    variant,
  - a request supplying neither usable header gets a relative base, which still
    downloads correctly from the browser that fetched the script.

Three tests pin it, and all three were confirmed to fail with the old body
restored rather than assumed to cover it.

Scope: product
2026-09-02 13:52:06 -04:00
cproudlock
27b2817fd7 Import the wilm endpoint-monitor and channel-distribution work
Everything on origin/wilm that is not on the published main, applied as one
change because the published history has rewritten hashes: per-commit
cherry-pick conflicts against our main even though the squashed diff applies
clean.

Contents: endpoint monitor catalog + policy attachment + warn-only drift
reconciler, operator issue reports from the tray, agent launch-path allowlist,
and the GHCR channel-image distribution path for dev workstations.

NOT READY FOR main. Open items:
  - 10 new routes are absent from docs/api-inventory.json, so
    test_api_inventory_parity fails. That gate SKIPS on the published mirror
    (the test ships, its data file is scrubbed), which is why wilm never saw it.
  - dev-bootstrap.ps1 and Start-ShopDBDev.ps1 hardcode the ge-aero GitHub org.
  - plugins/endpoints/plugin.py carries a whitespace-only re-indent.

Scope: product
2026-09-02 10:05:44 -04:00
cproudlock
c8e0de4dc1 Let the server decide which NIC is a PC's address, and keep the rest
All checks were successful
CI / naming (push) Successful in 4s
CI / frontend (push) Successful in 12s
CI / backend (push) Successful in 4m15s
CI / migrations-mysql (push) Successful in 41s
A CMM reported 100.0.0.1 as its address and never reported 10.48.255.210 at
all. Both halves of that were the same mistake.

THE BAY WAS CHOOSING. The collector schema took a single `ipaddress`, so the
script picked one NIC and dropped the others - preferring an address inside a
configured range, else whichever interface carried a default route. A bay
cannot make that judgement: it does not know the site's addressing. On this
CMM the Marvell AQtion 10Gbit link to its controller is a real hardware NIC,
up, and carries a default route of its own, so it satisfied every test the
script had. It won, and the corporate address was discarded before the request
was even built - which is why it is not in ShopDB even as a second row.

So the bay now reports EVERY connected hardware NIC with a routable IPv4, and
takes no view. The server picks the primary from collector_corporate_ranges, a
new per-site setting, and writes a Communication row for each of the others.
Naming the ranges is what turns the answer from a guess into a decision; with
none set it still falls back to the default-route flag and then to the first
reported, both guesses, both better than before only because nothing is thrown
away now.

Rows are keyed on the MAC where the bay sends one, because an address moves
between adapters and a MAC does not. A NIC that is down at check-in is left
alone rather than deleted - being down is not evidence the record is wrong,
and hand-entered gateway, DNS and serial settings live on those same rows.
Exactly one row ends up flagged primary, and only a row this report touched
can claim it, so a check-in cannot silently demote an address set by hand.

-AllowedRanges and the CollectorRanges registry value are now accepted and
ignored, logging a line that says where the setting moved. The parameter stays
because a scheduled task still passing it would otherwise fail on an unknown
argument.

A bay on an older collector sends one address and no interfaces; that keeps
working and is pinned by a test.

Scope: product
2026-08-31 17:40:21 -04:00
cproudlock
259fe0de8f Read a numeric natural key as a name before reading it as an id
All checks were successful
CI / naming (push) Successful in 4s
CI / frontend (push) Successful in 13s
CI / migrations-mysql (push) Successful in 41s
CI / backend (push) Successful in 4m18s
An Azure device import died part-way with a foreign-key error, and the crash
was the lucky outcome.

The Azure adapter routes a device that already exists to its real asset by
writing that asset's assetnumber into the assetid column of the CSV it
generates, expecting the engine to look it up. The resolver instead takes any
all-digit string at face value as a literal id. This site numbers operations
0NNN, so numeric assetnumbers are ordinary here: `0704` was read as id 704,
which does not exist, hence the error. `0600` would have been read as id 600 -
a printer - and bound a PC to it silently, reporting success.

Two changes, both needed.

The resolver now tries the label first for an all-digit string and only falls
back to reading it as an id when nothing is named that. The face-value
behaviour exists for a real reason - exports round-trip and carry real ids, as
its original comment says - so it stays as the fallback rather than being
removed. Somebody writing `0600` in a column keyed on assetnumber means the
asset called 0600, and binding the row to an unrelated record is far worse than
failing.

_existing_hostname_map now hands over the assetid rather than the assetnumber.
Giving the resolver the id it is going to treat the value as makes the
passthrough correct by construction instead of correct by luck.

Verified against a restored copy of the production database: 234 devices, 221
matching an existing computer and updating in place, 13 new, no problems. The
same run previously failed on the foreign key.

Neither half was wrong alone. The passthrough dates from the original CSV
import and the hostname map from the wilm merge months later; they only meet
at a site whose natural keys are numeric.

Scope: product
2026-08-31 15:55:21 -04:00
cproudlock
2feb569686 Rename Metrics to Monitors and move it second
All checks were successful
CI / naming (push) Successful in 4s
CI / frontend (push) Successful in 12s
CI / migrations-mysql (push) Successful in 43s
CI / backend (push) Successful in 4m17s
"Metrics" promised numbers. The tab holds tracked services, tracked processes
and network interfaces - things being WATCHED and their current state, only
one of which is a measurement. Monitors is what the panels actually are, and
it matches the word the agent's own config already uses for them.

Moved to second, ahead of Software. What is being watched on this box is the
question asked right after "what is it", and well before its application
inventory - which is reference data somebody goes looking for deliberately.

Tab order is now Details, Monitors, Software, Activity.

Contract 0.29.0 keeps its number. It is unreleased, so the group vocabulary it
introduces is still being settled and nothing can yet depend on the old name.
Both the server and the client fall back to 'details' for a group they do not
recognise, so a site that deploys these halves out of step gets a misplaced
card rather than a missing one.

Scope: product
2026-08-31 15:21:46 -04:00
cproudlock
2122c3b45c Tab the PC detail page, and let a panel say which tab it belongs to
All checks were successful
CI / naming (push) Successful in 3s
CI / frontend (push) Successful in 13s
CI / migrations-mysql (push) Successful in 42s
CI / backend (push) Successful in 4m24s
The page had grown to roughly ten cards in one flow, and three more surfaces
were wanted on it: installed software, live metrics, and the service controls.
Stacking those into the same column would have buried what the page is for.

Tabs were mocked and rejected on 2026-08-28, on the grounds that the strip
duplicated the sidebar and cost hero space. That objection is answered by
where the strip sits: the hero and the triage band stay above it and never
move, so "is this machine OK" is still answered without a click, and the strip
partitions the detail underneath rather than navigating anywhere.

WHY A PANEL DECLARES ITS OWN GROUP

Most cards on a detail page are not the page's. They arrive through the
ADR-010 get_asset_panels hook, so a page that sorted panels into tabs by
knowing their ids would defeat the hook it is built on. Panels now carry an
optional group instead, and the page only knows the five group names.

Resolution happens server-side, in the asset-panels route, so that an absent
or misspelled group becomes 'details' before the client ever sees it. The
client renders known groups only; an unrecognised value that survived to it
would be a panel that exists and is displayed nowhere. Contract 0.28.0 ->
0.29.0, additive - every existing plugin keeps working untouched, and the
endpoints status panel is deliberately left ungrouped so the default path is
exercised by a shipped plugin and not only by tests.

A tab is offered only when something fills it. An empty tab reads as broken
rather than as "this site does not collect that", so the strip is built from
the groups that actually returned data, and a tab that disappears on refetch
falls back to Details rather than leaving the page blank.

AssetDetailTabs is core, not a PCDetail component: machines, printers,
network devices and measuring tools all have the same shape, and the shared
asset-detail shell they need starts here.

Scope: product
2026-08-31 14:24:16 -04:00
cproudlock
df083776b3 Offer the search box to whoever can actually search
All checks were successful
CI / naming (push) Successful in 3s
CI / frontend (push) Successful in 12s
CI / migrations-mysql (push) Successful in 42s
CI / backend (push) Successful in 4m24s
The sidebar box was drawn for anyone signed in, on the reasoning that
offering it to a signed-out reader gives them a control that can only
answer 401. That stopped being the right test when the guest role became
able to hold assets.view: the box was hidden from exactly the visitor the
grant was reopened for, and a guest could reach search only by typing
/search?q= by hand.

The client cannot decide this. Its hasPermission getter reads the signed-in
user's permission list and a guest has no user at all, so asking it about a
guest answers no for everyone. get_navigation already had the problem and
already solved it by letting the server decide, so capabilities does the
same thing and reports what the CALLER holds.

It reports booleans about the caller and never what the guest role holds in
general, so it tells an anonymous reader nothing the rendered page does not.
Public because it is the pre-login call that decides whether the box is
drawn at all.

Members who lack assets.view now lose a box that only ever 401'd for them.
On a failed call the box stays hidden rather than falling back to the old
signed-in test, which would put it back in front of those same members.

Scope: product
2026-08-31 09:56:38 -04:00
cproudlock
723a4ff19b Declare the guest grants that already shipped
Inverting the grantable rule from the .view suffix to explicit declaration
dropped every permission that had qualified by how it was spelled. Two of
them were shipped guest features with grants already in the field.

guides.view: the nav item and list_guides both reason in comments about a
signed-out reader holding it, and the route is written around one - drafts
are filtered out, search matches the published revision so draft wording
cannot leak through a hit, and the author branches cannot open for a caller
with no user id.

tools.view: without it a guest holding tools.secrets has permission to use
Secure Send and no way to reach the page, because tools.view is what puts
the entry in the nav. The tools themselves run in the browser and touch no
data.

The failure was worse than a missing checkbox. update_role refuses the
WHOLE request when any submitted permission is undeclared, and the role
grid resubmits what the role already holds, so a site that granted either
of these under the old rule could no longer save the guest role at all -
including to tick assets.view, which is the grant the shop floor needs to
look up an asset number without signing in.

The test asserts both stay grantable, so narrowing the set again means
looking at what is already granted in the field rather than only at what
the catalogue declares.

Scope: product
2026-08-31 09:56:19 -04:00
cproudlock
4d0baaff77 Gate each search domain on its own permission
Global search is gated on assets.view, and that was the whole check.
Behind it the route queried twelve domains: applications, the knowledge
base, guides, the staff directory, assets, measuring tools, USB devices,
printed parts, custom fields, notifications, hostnames, and vendor and
model reference data, plus addresses and subnets for an IP-shaped query.
None of them consulted their own permission.

So anyone who could look up an asset number could also read the staff
directory out of the HR database, the USB checkout history and who holds
which drive, and the network inventory. The SEARCH_DOMAINS settings look
like they bound this but they are site feature toggles, not
authorization.

Each domain now answers only to a caller who holds its permission.
Assets, custom fields and vendor/model stay on assets.view because all
three emit asset rows routed to asset pages.

caller_holds rather than require_permission: refusing the whole request
because one domain is off-limits would make search useless to anyone
holding less than everything. A domain the caller cannot see contributes
nothing and the rest still answers.

This is what makes assets.view safe to grant the guest role again. It
left the grantable set in 3474028 because of the leak above, and that
took the shop floor's asset lookup with it - a signed-out visitor at a
machine could no longer search for the asset number in front of them.
Now the grant reaches assets and nothing else.

The first version of the leak test passed with the gates removed. The
staff directory is an external HR database and is unreachable under
test, so the domain silently returned nothing and the assertion proved
nothing. It patches _employee_rows now, and fails when the gate goes.

Scope: product
(cherry picked from commit 0a01ddcf25)
2026-08-31 09:06:25 -04:00
cproudlock
dd52666d6b Order the supplies card by how empty the cartridge is
Rows sorted on iscritical then on printer NAME. Critical is everything
at or below 5%, so a cartridge at 5% and one at 0% tied on the first key
and the alphabet decided which came first.

Wrong order on its own, and worse than it looks: the dashboard queue
keeps only the first few rows per source, so a printer that was
genuinely empty was truncated off the card while a fuller one held its
place. Reported from the floor as "starts at 5% and hides one at 0%",
which is exactly the two bugs compounding.

remaining is already normalised for waste cartridges - a full waste bin
reads as low remaining - so one ascending sort is worst-first for every
supply type, and iscritical falls out of it rather than needing its own
key. Name only breaks ties.

worstremaining is carried on the row because the supplies list is
rendered to text ("Black 0%") before it reaches the client, so the
number the sort needs is gone by then.

The toner forecast report was already worst-first (it sorts on daysleft)
and is unchanged.

Scope: product
(cherry picked from commit 6644a07ae1)
2026-08-31 09:06:25 -04:00
cproudlock
dbc43162c5 Pin the approved content digest on an endpoint command
Delivery descriptors for run_script and update_app are resolved when the
agent claims a command, not when an operator queues it, and the command
row recorded no digest: the enqueue path resolved the descriptor purely
to validate it and threw the result away.

So what an operator approved and what an endpoint runs were two separate
lookups against mutable content, minutes or hours apart. Anyone able to
replace a script body or an installer package could do so in the gap.
The claim then re-resolves, the descriptor faithfully describes the new
bytes, the agent verifies correctly, and it runs as LocalSystem. The
audit trail names the operator who queued the original.

The gate on the verb and the gate on the payload were also different
grants: pushing needs endpoints.deploy, but replacing what gets pushed
needs only applications.edit.

pinnedsha256 records the digest as it stood at enqueue, and the claim
refuses a descriptor that has moved, failing the push with an audit row
instead of shipping it. Null for actions that deliver no content, and
for rows queued before this migration, which claim as before. A
descriptor that vanished entirely is not treated as tampering: that path
already sends the command through undescribed so the agent fails closed.

Separately, a result may now only finalize a command that is actually
running. That needs no agent change and closes two things: reporting on
a command that was never claimed, and overwriting a recorded failure
with a success.

The agent now sends its hostname on the result callback so the server
can check it owns the command it is finalizing. The shared collector key
does not say who is calling, so without it any agent can post a result
for any command id. The server treats the parameter as optional for
agents that predate this and enforces it when present, so there is no
flag day. This is a floor, not a fix: only a per-endpoint credential
makes the caller's identity provable, and that is tracked separately.

The .NET side is unbuilt here and needs a build plus a win11 VM run.

Scope: product
(cherry picked from commit d64b7122ab)
2026-08-31 09:06:25 -04:00
cproudlock
ccd8221adf Key the client address on the connection, not a header the caller sends
client_ip() read the first X-Forwarded-For hop unconditionally. The
login rate limit and the account lockout are keyed on that value and the
audit log records it, so rotating the header per request bought an
unbounded budget and an untraceable trail.

This walked around TRUSTED_PROXY_COUNT, which exists to decide exactly
this. The app leaves ProxyFix off by default and says why, ten lines of
comment away: ProxyFix trusts X-Forwarded-*, a client can send those
itself, and the login limiter is keyed on the address. Then client_ip
read the header anyway, so the stated protection never applied.

Resolution is now from remote_addr, which ProxyFix has already filled
from the header wherever a proxy is genuinely configured. Both cases end
at the same trustworthy source, and the port still comes off because ARR
forwards clientip:port and ProxyFix passes it through verbatim.

geenforce and tools had each hand-rolled the same mistake for their own
limiters. Rather than duplicate security-relevant logic a third time,
client_ip and strip_port join the shopdb.api surface (contract 0.28.0).
geenforce's _trusted_client_ip, which already refused the raw header for
its IP allowlist, is untouched and was the model for this.

The stability table was also missing rows for 0.26.0 and 0.27.0.

INSTALL-WINDOWS-IIS.md never mentioned TRUSTED_PROXY_COUNT, so a site
installed from it runs behind IIS with the setting at 0: every request
looks like it came from IIS, the whole site shares one rate-limit
bucket, and the audit log records the proxy for every action. It now
says so, and says not to set it when nothing is in front.

Three test modules simulated separate callers by setting
X-Forwarded-For, which is the header a real attacker rotates to escape
the limit. One of them, test_client_ip_prefers_forwarded_first_hop_
stripped, asserted the defect outright. The suite could not have caught
this. They now drive REMOTE_ADDR, with a regression guard that forging
the header buys nothing.

Scope: product
(cherry picked from commit 7260244b7e)
2026-08-31 09:06:25 -04:00
cproudlock
d1a7907a2c End a user's sessions when the account is deactivated
There is no token denylist in this product, so the JWT user loader is
the only place a deactivated account can be stopped, and it did a bare
lookup by id. Somebody deactivated at 09:00 kept a working access token
until it expired, and a refresh token that minted fresh ones for thirty
days.

/api/auth/refresh and the API-token path already checked isactive. This
makes the loader consistent with them rather than inventing policy.

Scope: product
(cherry picked from commit 9c1848fc61)
2026-08-31 09:06:25 -04:00
cproudlock
85e8a7b877 Decide guest grants explicitly instead of by permission name
guest_grantable_permissions() opened any permission whose name ended
'.view' or '.read' to the guest role, on the reasoning that reading is
the safe default. That is true of a permission gating a list of asset
numbers and false of one gating bytes, and a suffix cannot tell them
apart. Twenty-four of eighty-five qualified and none had been decided:

  applications.view  reached the SYSTEM-context PowerShell bodies and
                     the installer binaries pushed to bays
  backups.view       returned the full payload that the separate
                     backups.download permission exists to protect
  assets.view        gated a global search that queries the staff
                     directory, USB history and network inventory with
                     no per-domain check of its own
  audit.view         usernames, addresses, the before/after of every
  settings.view      change, and every setting row - both admin
                     category, opened by naming alone
  users.view         gates nothing at all; list_users is
                     require_role('admin')

Every entry is now declared, by CORE_GUEST_GRANTABLE or by the plugin
that owns the permission, having read what its routes actually return.
Adding one is a code change, which is the point: it puts the decision
in a diff rather than in a suffix.

The two routes serving script bodies and installer packages move to a
new applications.download permission, which is not grantable, so a site
can still open the catalogue metadata to its floor without the bytes
going with it.

The declared set preserves the grants this site had already chosen:
applications.view, computers.view, endpoints.view, reports.view,
tools.secrets, warranty.view.

Two known gaps are recorded on the hooks that expose them rather than
silently carried: the computer serializer publishes every column plus
resolved rdp:// and vnc:// links, and the per-asset endpoint panels are
keyed on a small integer so walking ids reconstructs much of the roster
that endpoints.fleet exists to protect. Neither is widened here.

A shipped test was granting guests audit.view and passing.

Scope: product
(cherry picked from commit 347402896c)
2026-08-31 09:06:25 -04:00
cproudlock
5f27f81e88 Clear the review blockers on the merged wilm work
All checks were successful
CI / naming (push) Successful in 3s
CI / frontend (push) Successful in 13s
CI / migrations-mysql (push) Successful in 42s
CI / backend (push) Successful in 4m18s
A multi-agent review of the merged branch produced 29 findings that survived
adversarial verification. This clears the blockers and the four worth fixing
before it reaches main. The design held up well: path-traversal defence on all
four serving routes is clean, the agent's SHA-256 check fails closed, enqueue
authority is proportionate and audited, and no site literals or credential leaks
in the new C#.

THE TESTS NEVER RAN

pytest.ini sets testpaths = tests, so plugins/endpoints/tests/ was collected ZERO
times. Fifty-five lines of delivery assertions were dead code, and run directly
five of them failed. plugins/printedparts/tests/ had been invisible for longer.

That is the worst shape a gap can take: the suite is green, the coverage is
imaginary, and the collector auth on a download route could be deleted without
anything going red.

Both files moved under tests/. tests/test_no_hidden_tests.py now fails if a test
file appears anywhere pytest will not look; it caught a staged copy inside the
installer bundle on its first run, and was verified by planting a violation.

The five failures were stale expectations, and they split by cause. His: the
applications panel made six panels where his test said five, and 'applications'
belongs in KNOWN_PAYLOAD_KEYS now that it is persisted - his own test contradicted
his own feature. Ours: we renamed the events card to activity, moved the
fleet-wide cards to endpoints.fleet, and made the list route a bare route('')
which Flask rejects unless the blueprint is mounted under a prefix.

FOUR FIXES

The snapshot table's key was a 32-bit autoincrement on a table rewritten
wholesale every check-in, so it climbs at (endpoints x apps) per report and
exhausts on MySQL. Widened while the table is unreleased. Widening it broke every
insert on SQLite, which only autoincrements a column typed exactly INTEGER
PRIMARY KEY, so it carries a dialect variant - and nothing caught either fault,
because no test inserted into that table at all. One does now.

Ingest truncated displayname but not displayversion or publisher. On strict MySQL
an over-long registry value fails the whole check-in transaction, so one machine
with a malformed Uninstall entry goes silent entirely. SQLite does not enforce
lengths, so the suite would never have shown it.

update_app's version: branch pinned AppVersion.sha256, but that route resolves the
version to its APPLICATION and serves application.installpath - so the digest
described an artifact the route never sends, and where it was null the agent had
nothing to verify and failed open. It now pins the file actually served.

The same branch ended the delivery URL at the id, so an MSI arrived with no
extension and could not be routed through msiexec. It carries the filename now.

Agent packaging moved off require_role('admin') to a new endpoints.agentpackage
permission. The generated ZIP carries a live collector key, so this is a heavier
grant than deploying to one endpoint - and a role cannot be given to one person
for one job the way a permission can.

Full suite 2231 passed (2198 before, plus the 22 that never ran and 11 new),
frontend 314.
2026-08-30 17:17:13 -04:00
benjamin.baker
7ac1772881 Add installer/script delivery metadata to the core software model
AppVersion gains sha256 and silentargs so the endpoint update_app push can pin exact installer bytes and carry per-version silent switches; Application gains installersha256 for the app-level install source. New ShopDB-hosted script library (Script model plus scripts API) backs run_script delivery, with the body fetched by digest on push rather than pre-staged or sent over the command wire. Two idempotent migrations with real downgrades. Script added to the shopdb.api contract surface for plugin use.
2026-08-30 15:19:35 -04:00
benjamin.baker
5f8061c55d Self-heal endpoint command queue wedged by a dead in-flight command
The command queue is at-most-one-in-flight, but nothing ever expired a stuck command. When an agent claimed a command (RUNNING) then died before reporting a result - killed, uninstalled, or offline mid-flight - the row stayed in-flight forever and every operator action for that endpoint returned 409 'already in flight'. Reap in-flight rows past a staleness ceiling (15m for RUNNING, 60m for PENDING) to a new STATUS_EXPIRED at enqueue time, inside the same transaction, so the queue self-heals lazily the next time someone queues work. No background job and no schema migration - status is free-text.
2026-08-30 15:19:35 -04:00
benjamin.baker
1fa005eb8a Green up
Tracking docs/openapi.json (so the app can serve the spec at runtime) makes docs/ exist on the published GitHub mirror, which defeated the skipif(not DOCS.is_dir()) guard in the docs publish/version tests: they then ran against a prose-less checkout and failed on CI. Key the skip off docs/*.md prose instead, matching the canonical published-checkout definition in scripts/check-gates-ran.sh.

Also: the OpenAPI generator emitted a trailing-slash path (/api/docs/), which a strict proxy like IIS treats as a redirecting resource a subpath-mounted client cannot follow; normalise trailing slashes in _norm_path and correct the committed spec. And allow docx (python-docx) as an undeclared import: it is used only by scripts/docs/generate_docx.py, a developer tool that never runs on a deployed site.
2026-08-30 15:19:35 -04:00