75ee3b7c1cc4680039f23b4a364f63f33d79fbbb
838 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
cproudlock
|
75ee3b7c1c |
Keep the agent's build output out of the publication tree
A local 'dotnet build' leaves agent/*/obj and agent/*/bin full of absolute host paths. They are gitignored, but the export rsyncs the WORKING TREE, so .gitignore does not apply and the scrub gate stopped the whole export - which is what it is for, but it made publication depend on whoever built the agent last having cleaned up. Same shape as the stray .env.bak the excludes already document. Note the trap that follows from it: an --exclude also PROTECTS a path from --delete, so the copies the first failed run had already placed in the publication tree had to be removed by hand before the next run could pass. Scope: product |
||
|
cproudlock
|
a46058a4dd |
Close the last three restore-gate gaps, and make the gate test the real gate
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 |
||
|
cproudlock
|
e2705d732a |
Gate a restore on every schema chain, not just the core one
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 |
||
|
cproudlock
|
fa6b99bf91 |
Finish a restore without a browser, from the set that was asked for
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 |
||
|
cproudlock
|
83190422c9 |
Act on the review of the wilm import
Six findings from a four-agent review of the imported work, plus the contract bump the import owed. Nothing here changes what the imported features do. The contract version was not bumped for a change that widens what a plugin may declare. The asset-panel vocabulary gained two tab groups (serviceprocess, reports), a datetime column format, a map.body key and a tabs section that renders as a table. 0.29.0 bumped for exactly this class of change when the optional group was added, so this is 0.31.0 with a ledger row. All four are optional and resolve server-side when absent, so a plugin declaring none of them is unaffected. Site Backups now answers honestly where it has nothing behind it. The overlay is a compose profile: a separate container writes sets onto a shared volume and the api container only reads them. A Windows/IIS install runs no such container and the default root resolves to a drive-letter path nothing creates, so Back Up Now either failed on PermissionError or reported a run queued that would never happen, under a banner reading "No verified backup yet" - which describes a backup that failed, not a feature that was never installed. The three write routes refuse with the same 404 shape the configuration-policy routes use for a disabled feature, the list and status routes report whether a store exists, and the page says so instead of offering buttons. Two listAll callers still read a response that is not there. listAll resolves to a plain array through fetchAllPages, so response.data.data throws: PC Types settings rendered empty, and in Configuration Policies the throw happened inside the shared try, so PC types, applications, the action catalog and monitors were all left unassigned by one bad read. The agent package upload cannot succeed on IIS as shipped. MAX_CONTENT_LENGTH went to 256 MB to admit the ~69 MB tray binary, but IIS request filtering caps a body at 30,000,000 bytes and answers 413 before waitress sees the request, so web.config needs the matching requestLimits. The backup image could not be built by anyone outside a TLS-intercepting network. Dockerfile.backup did a plain COPY of a gitignored corporate root CA, and a COPY of a missing file fails the build. The trailing glob makes it a no-op when the cert is absent. endpointstates.domain was the one new string column written without a clamp. Under STRICT_TRANS_TABLES an over-long value raises Data too long and aborts the whole check-in, so a single malformed report would stop a machine reporting telemetry and events entirely. Not fixed here, and left with upstream: the restore path finalizes its schema upgrade only from a browser poll loop, so closing the tab mid-restore leaves the data behind the code; it restores instance files from the newest snapshot rather than the set that was asked for; and it takes no safety dump before the destructive load. All three are in the Docker-only overlay, which this site does not run, and none can be validated here. Scope: product |
||
|
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
|
||
|
cproudlock
|
9947a477b8 |
Seed DeviceId in the registry from the identity a bay already carries
Phase 3 of the device-identity proposal moves device identity to
HKLM:\SOFTWARE\GE\ShopDB\DeviceId, which survives the profile cleanup and disk
hygiene that eats C:\Enrollment and needs no new key: the collector already
opens that path for BaseUrl and ApiToken. This is the backfill that gets a
value in there without re-imaging anything.
The source depends on the pc-type, and that is the whole point. DeviceId names
the DEVICE, while a machine number answers "which bay is this" - a different
fact that coincides for exactly one family:
CMM the machine number IS the instrument id (CMM4), which is why
cmmid.txt has always doubled as identity. Copied.
part marker the machine number is the OPERATION, and several markers serve
one operation. Copying it would name the operation as the
device, which the collector refuses as the wrong kind of asset
because a machine filed under a device label reads as a marker
everywhere downstream. Never copied.
Keyence, no per-bay id has ever existed; asset-id.txt is the only
Genspect, identity these have.
wax-trace
anything else not a device bay, so nothing is written. DeviceId is not
consumed for those pc-types and a value left there is a
landmine for the day one gains a device spec.
Resolution per family mirrors Report-AssetToShopDB.ps1, including the 9999
placeholder skip and reading WOW6432Node before the native eDNC key.
Refuses to overwrite a DeviceId that disagrees, unless -Force. asset-id.txt is
written once at imaging and nothing updates it on a device swap, so a stale
file clobbering a corrected registry value would reintroduce the exact defect
this move exists to fix. GE-Enforce already reached the same conclusion for
machine-number.txt.
Verified in the win11 VM across ten cases: not-a-device bay, part marker,
CMM precedence, idempotent re-run, machine-number exclusion, eDNC fallback,
9999 skip, stale-file refusal (exit 2), -Force, and -WhatIf. The part-marker
guard was then planted - flipping it to treat the machine number as identity
writes operation 0613 into DeviceId, so the passing test is not vacuous.
Scope: product
|
||
|
cproudlock
|
a6775c1da1 |
docs: ADR-018 supersedes ADR-005, metrology instruments are machines
ADR-005 split machines from measuring tools on the grounds that they are two asset classes. It named the metrology population as CMMs, vision systems, wax trace and Genspect; the plugin was then built against a docstring naming calipers, micrometers and thread gages. Those are not the same population, and the difference is the whole decision: bay-mounted capital equipment with a controlling PC on one side, hand tools in a drawer on the other. Sixteen months on, measuringtools holds zero rows against 204 machines. The metrology inventory waiting to be entered is 134 instruments - 35 CMM, 32 form tracer, 23 vision, 18 wax, 14 measuring, 8 Genspect, 2 eddy, 1 spline - every one bay-mounted with a controlling PC, and not one hand gage among them. CMM is already a machinetype, and Part Marker is already a machine while Form Tracer is a measuring tool, so the line is already drawn somewhere no operator can predict. Retires the plugin and the measuring_tool asset type. Calibration is not lost: it moves onto machines as four nullable columns with status still derived at read, which is a gain rather than a port because a machine could not record calibration at all before. ADR-005's one concrete objection - that "show me all measuring tools" would need an enumeration of type names - is answered with machinetypes.ismetrology, so it stays a single-column filter. Taken now because measuringtools is empty. After the 134 rows are entered they carry relationships, audit history, map positions and collector links, and the migration stops being a drop. Scope: product |
||
|
cproudlock
|
349e8c89bd |
docs: EventSaver has no compiled-in folder fallback any more
The adoption guide told an adopting site that a missing EventSaver.ini makes the binary fall back to "the reference site's file server". That was true of EventSaver.cs until the ADR-015 fix set Folder to empty, and EVENTSAVER.md was updated then while this page was not, so the two documents disagreed about what a missing ini does. Current behaviour is that it shows nothing, which is the point: failing visibly beats displaying another site's slides. Says that, and adds the case the old text was accidentally describing - a .scr compiled before the fix still carries the path, so a binary somebody was handed is not the same as one they built. Scope: product |
||
|
cproudlock
|
bfc30f4c7c |
docs: branch model documents one branch per instance
CONTRIBUTING.md still said "main is the product every site runs and the publication mirror exports" and framed wj/wilm as site branches feeding it. That was true when main was the only thing anyone deployed. It is not now: WJ production runs wj, the /ops instance runs dev, and main deploys nowhere. Nothing in the repository mentioned dev at all, so the branch a deploy script refuses to run without was undocumented. Replaces that section with the branch/instance table and the dev -> wj -> main promotion order, and says why promoting before validating is the wrong way round. Keeps the wilm note but moves it up with the reason it cannot be merged locally: it sits on the publication lineage and shares no ancestor. CLAUDE.md gains the branch-to-server pointer and records that export-github.sh now builds a dist only for wj and dev. docs/DEPLOY.md is deliberately left alone: `git pull origin main` is correct there. That document is the generic connected-Docker deploy another site follows, and main is exactly what another site adopts. Scope: product |
||
|
cproudlock
|
b5737f47c1 |
Serve gunicorn with threaded workers so a retry storm cannot starve the UI
Taken from the other site, who hit it in production today (their commit
|
||
|
cproudlock
|
b9f4aea4b8 |
Make the deploy scripts enforce which branch reaches which server
The rule for this site, stated plainly: only `wj` reaches /shopdb (WJ production), only `dev` reaches /ops, and `main` is the curated release other sites adopt and deploys nowhere. The tooling did not know that, and in two places actively assumed the opposite. export-github.sh refused --dist on any branch but main, because production was assumed to install main. It does not - it installs wj. That guard fired today: a wj export produced its bundle and then died at the dist step, so the dists on the share came from the main export that happened to run next. The code going to production and the frontend going to production would have come from different branches, silently, and nothing downstream could tell. Now one branch builds one dist: wj builds the /shopdb dist, dev builds the /ops dist, and any other branch is refused because a dist it built could only ever be installed by mistake. Each dist carries a .dist-source stamp naming the branch, commit, base path and build time. The two deploy scripts copy from the clone on WHATEVER branch it is sitting on and only ever checked the destination (the web.config mount path), so a clone left on the wrong branch deployed the wrong code to a correctly-identified server without a word. Both now refuse unless the clone is on their branch, and both read the dist's .dist-source and refuse a frontend built from anywhere else. A directory of hashed filenames says nothing about where it came from, and robocopy /MIR will mirror whichever one it is pointed at. pull-shopdb-bundle.cmd and update-shopdb-github.cmd silently defaulted to main. That was safe when main was the only thing anyone deployed; with three branches it is a way to carry the wrong one to a server without ever typing its name. The argument is required now, and the error lists what each branch is for. tools/mirror-wilm.sh is the missing half of fetch-wilm-branch.cmd: that script only puts a bundle on the share, and wilm does not appear on gitea by itself. The new one fetches the carried bundle and pushes wilm to gitea, printing the bundle's age first - a mirror that looks authoritative and is a week old is worse than no mirror - and noting that wilm sits on the publication lineage, so it shares no ancestor with main/wj/dev and cannot be merged here. Scope: product |
||
|
cproudlock
|
c1a4c9eba6 |
Let a PC be assigned a business unit, and a retired asset be found
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 |
||
|
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 |
||
|
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 |
||
|
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
|
||
|
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 |
||
|
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 |
||
|
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 |
||
|
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
|
||
|
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
|
||
|
cproudlock
|
fb63ef9d30 |
Make a retired asset findable and restorable from the product
The retire dialog says "History and relationships are kept, and it can be
restored." The API always kept that promise - ?active=false lists retired rows,
PUT {"isactive": true} restores one - and no screen in the product could reach
either. A retired asset simply vanished: the lists filtered it out, global
search missed it, and its detail page still rendered Edit and Retire with
nothing anywhere saying it was retired.
Five list pages gained a "Show retired" toggle that passes active=false, and a
retired row now carries a badge. Five detail pages gained a banner saying the
record is retired and what that means, a Restore button, and the suppression of
Edit and Retire while it is retired - offering Retire on an already-retired
record was the part that made the state invisible.
Verified end to end in a browser: retire a machine, the detail page says so and
offers Restore, the list hides it until the toggle is ticked and badges it when
shown, and Restore puts it back (isactive true again on the API).
One thing this does NOT do: global search still does not reach a retired asset.
That is a separate surface with its own filter, and worth deciding on
deliberately rather than sliding in here - a search that returns retired
records by default would change every result page.
Scope: product
|
||
|
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
|
||
|
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 |
||
|
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 |
||
|
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 |
||
|
cproudlock
|
45acaf0b8f |
Resolve an asset type by name, and send a map position with its level
Wave 5, the asset forms. Both defects are the same mistake in different clothes: a form deciding something it should have been told. Four forms carried a hardcoded asset-type id - const COMPUTER_ASSETTYPEID = 2, and the same for machines, printers and network devices - to tell CustomFieldsInputs which type's fields to render. Those ids come from seed order. On the instance they were written against, computer was 2; on a freshly seeded database computer is 1 and 2 is network_device. So the PC form asked for another type's custom fields, rendered them, and dropped everything typed into them behind a success toast, because the values were keyed to a type the record does not have. Verified in a browser against a correctly built dist: the form now requests assettypeid=1 on an instance where computer is 1. The name is the stable thing - assettype is a fixed vocabulary and the API already publishes the mapping - so composables/assetTypeIds.js resolves it once per page load. A failed load leaves it undefined, which renders no custom-field panel: a missing panel is honest, a guessed id is what this replaces. Three of the five forms that emit a map position sent mapx and mapy without levelid. ADR-017 is explicit that a position without its level cannot be drawn on the right floor plan, so a picked pin saved a null level and rendered nowhere - while the form displayed "Position: x, y" back, because it kept the value it had never sent. The ADR-017 gate should have caught that and did not: it globbed '*.py' only, so it policed what the server emits and never what the browser sends. It now covers .vue and .js too, and accepts both quoting styles. Verified by planting: drop levelid from PCForm again and the gate names that exact line. Five composable tests, including a fixture with a DIFFERENT seed order - the case the constants got wrong. Still open from this area, and deliberately not in this commit: the PC form writes an INHERITED location back as the record's own. Fixing it needs the API to distinguish an owned locationid from a derived one, which is a contract change rather than a form fix. Scope: product |
||
|
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.
|
||
|
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
|
||
|
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 |
||
|
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
|
||
|
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 |
||
|
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 |
||
|
cproudlock
|
377cc69264 |
Make a partly-undone import retryable, and refuse a plan it cannot deliver
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 |
||
|
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 |
||
|
cproudlock
|
28269e429a |
Import the WINC batches CLI and its README
One commit from the sister site, purely additive: a read-only `batches` command
so an operator working from the shell can find a batchid to undo, and a README
covering the four WINC commands. Two new files under scripts/site_imports/winc/,
nothing existing touched.
Verified rather than assumed: it changes none of the files fixed in
|
||
|
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 |
||
|
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 |
||
|
cproudlock
|
e4db112296 |
Use a valuetype the settings column actually documents
printedparts_kiosk_token declared valuetype 'secret'. The column documents string, boolean, integer and json, and that was the only 'secret' in the codebase - it worked only because Setting.get falls through to the raw string when it recognises nothing. Masking never depended on it: settings.py::_is_secret keys on the NAME containing 'token', so the value is still hidden in API reads and in the audit log. Changed to string, which is what it is. Scope: product |
||
|
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
|
||
|
cproudlock
|
852aec5b7f |
Verify the launch-path hardening on Windows, and fix what that found
The previous commit said the link-resolving change had never met a real filesystem. Run in the win11 VM against real fixtures, it turned out to reject EVERY legitimate launch path. ResolveDirectoryChain walked up looking for a junction and eventually asked whether "C:\" was a link. That throws DirectoryNotFoundException, and the caller treats a resolve failure as a rejection, so the first legitimate case came back "cannot resolve the real path: Could not find a part of the path 'C:\'". The agent would have failed closed on everything - an outage wearing a security fix's clothes. It compiled with zero warnings and read correctly. Stop at the volume root, and never let "cannot tell whether this is a link" turn into a rejection: TryResolveLink returns null on any failure, and the assembled path is re-checked against the roots regardless, so an unresolvable component still cannot widen what is accepted. One expectation in the probe was mine being wrong rather than the code. 8.3 short names used to be rejected because the lexical prefix check could not match C:\PROGRA~1 against C:\Program Files - fail closed, but by accident of not understanding the path rather than by intent. Resolution now expands it to the real directory, which is inside the approved root, so accepting it is correct. Verified on Windows, all ten cases: accept a real exe under the approved root accept 8.3 short name resolving into the approved root reject exe reached through a junction pointing out of the root reject alternate data stream on a file in the root reject traversal back out of the root reject UNC path reject sibling directory sharing the root's prefix reject wrong extension reject file that does not exist reject empty path The probe is committed as agent/ShopDbEndpointAgent.PolicyProbe so this is repeatable. It is NOT in CI and its README says why: junctions, alternate data streams and 8.3 names do not exist on Linux, and a suite that skipped every case it exists for would report green and mean nothing. This repo has been bitten by that shape more than once already. Scope: product |
||
|
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 |
||
|
cproudlock
|
88a683dad3 |
Act on the five-agent review of the wilm import
Findings the reviewers verified, fixed here. The larger cluster they found is NOT fixed and is listed at the end. Frontend. reportImageUrl is called from the template, so it ran on every render with no in-flight guard: a report whose fetch was pending was re-requested, and because each resolution reassigns the map the whole grid re-rendered and re-fired the rest, costing up to N-squared requests for N thumbnails. Two blobs resolving for one key orphaned the first object URL where revoke could not reach it, and a failure was never recorded so a broken image retried forever. Guarded, deduplicated, failures remembered. Three CSS variables did not exist, so their hardcoded fallbacks always won: --bg-alt (the "No screenshot" tile, near-black in light mode), and --warning-text/-bg/-border (dark khaki on a dark modal). Replaced with real tokens. The DEV badge's literal #c62828 is var(--danger). Installer and launcher. The generated site installer ran docker compose with no -p, so the project name defaulted to the package folder - which carries the version. Installing the next version created a new project with new empty volumes and the site came up looking wiped, old data stranded under the old project. Pinned to shopdb-<channel>. Start-ShopDBDev deleted any existing -TargetDir that lacked a .git, recursively and without asking. For a script handed round as "save it anywhere and run it", a mistyped -TargetDir pointed at Documents would have destroyed it. It now only reclaims a directory that is empty or carries this launcher's own marker, and refuses otherwise. Dev secrets came from Get-Random, which is System.Random: a 32-bit seed, and all four values off one stream, so recovering one recovers the rest. They sign JWTs for a stack a pilot may point at real imported data. CSPRNG now, and the API port binds to loopback like the DB rather than every interface. Two of my own from earlier in this session. The operator-report size check sat below request.form, and the first access to that parses and spools the whole multipart - so it ran after the thing it prevents. Moved above. And the channel remap missed build-channel-installer.ps1 entirely, where it set BOTH channels to main: a "stable" package built from the trunk, the exact mislabel the remap was for. stable now resolves the newest v* tag. Also removed a zero-byte docs/ShopDB-Planner-Tasks-v2.csv that rode in. NOT FIXED - needs a decision, and it is the biggest thing the review found. Migrations 0006/0007 freeze the originating site's fleet convention into product seed: a shipped policy literally named "RPCA Shopfloor App Monitors", a hardcoded 'Shopfloor PC' type deciding who gets the shipped watch-list, and shipped rows the API refuses to edit or deactivate - including their launch paths. A site whose eDNC is on another drive cannot correct it and gets permanent false drift. Those are stamped migrations, so this gets harder to undo at every install. See the review notes before anyone upgrades a second site through 0006-0007. Suite 2314 passing, frontend 316, all three PowerShell files parse. Scope: product |
||
|
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 |
||
|
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 |
||
|
cproudlock
|
d45a0d15b4 |
Make the full suite runnable in parallel, and close the dev/prod pin drift
The suite had grown to ~2300 tests and 5m30s. Profiling says there is no hot
spot to fix: the 25 slowest tests sum to about 60s of 331s, and the rest is
per-test fixture cost. The db fixture drops and recreates all 94 tables for
every test, measured at 0.058s a cycle, which is roughly 40% of the wall clock
on its own.
That shape parallelises almost linearly, so pytest-xdist buys more than any
rewrite would:
pytest -q 2309 passed in 331s
pytest -q -n auto 2309 passed in 103s (8 cores)
Deliberately NOT in pytest.ini addopts. Worker startup is dead weight on the
targeted single-file runs that make up most of a working day; the whole-suite
run before a push is where it pays. Opt in with -n auto.
A run distributed across 8 workers also has to be order-independent to pass at
all, so this is incidental evidence that the suite has no inter-test coupling.
Recompiling requirements-dev.txt to add xdist also moved idna, packaging and
pyjwt. That is the -c requirements.txt constraint doing its job rather than a
drift: all three ARE shipped, and the dev file had fallen BEHIND production
(idna 3.13 in dev against 3.19 in prod). CI was testing against older versions
than sites install, which is the exact failure the header comment on that file
was written about. Now matched.
Scope: product
|
||
|
cproudlock
|
28bf7ea1bf |
Regenerate the project map for the imported plugins and tests
Scope: product |
||
|
cproudlock
|
2e42972f44 |
Map the image channels to how this repo ships, and disarm auto-publish
The channel work assumed the branch model it was written against: upstream, main is a release branch and dev-be is the trunk. Neither holds here. main IS our trunk - 212 commits past v0.11.3 - and a release is a tag (ADR-007). dev-be does not exist at all, so the edge half would never have fired and the launchers would have cloned a missing branch. Taken literally it would have published our unreleased trunk to every site as :stable, which is the opposite of what the word promises to someone who has no way to check. Remapped throughout: tag v* -> :stable the thing that was actually released main -> :edge the trunk, DEV badge lit publish-images.yml keys the channel on github.ref_type = tag rather than on a branch name, so the mapping cannot silently drift back. Start-ShopDBDev.ps1, dev-bootstrap.ps1 and build-channel-installer.ps1 resolve stable to the newest v* tag via git ls-remote --sort=-v:refname, warning and falling back to main if a repo has no tags yet. DEV-BUILD-TEST.md follows. The push: trigger is commented out. Automatic publishing waits on someone confirming the GHCR package exists under this owner and deciding who may pull it; workflow_dispatch works meanwhile. Turning it on is one uncomment. All three scripts parse clean under pwsh, and the tag-selection expression was run against this repo rather than eyeballed. Per the standing rule, the runtime paths still want a pass in the win11 VM before anyone leans on them. Scope: product |
||
|
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 |
||
|
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
|