d9eb7a74dbe490d7b74c5a397aef390d95ff4e6f
256 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
cproudlock
|
4d0baaff77 |
Gate each search domain on its own permission
Global search is gated on assets.view, and that was the whole check. Behind it the route queried twelve domains: applications, the knowledge base, guides, the staff directory, assets, measuring tools, USB devices, printed parts, custom fields, notifications, hostnames, and vendor and model reference data, plus addresses and subnets for an IP-shaped query. None of them consulted their own permission. So anyone who could look up an asset number could also read the staff directory out of the HR database, the USB checkout history and who holds which drive, and the network inventory. The SEARCH_DOMAINS settings look like they bound this but they are site feature toggles, not authorization. Each domain now answers only to a caller who holds its permission. Assets, custom fields and vendor/model stay on assets.view because all three emit asset rows routed to asset pages. caller_holds rather than require_permission: refusing the whole request because one domain is off-limits would make search useless to anyone holding less than everything. A domain the caller cannot see contributes nothing and the rest still answers. This is what makes assets.view safe to grant the guest role again. It left the grantable set in |
||
|
cproudlock
|
dd52666d6b |
Order the supplies card by how empty the cartridge is
Rows sorted on iscritical then on printer NAME. Critical is everything
at or below 5%, so a cartridge at 5% and one at 0% tied on the first key
and the alphabet decided which came first.
Wrong order on its own, and worse than it looks: the dashboard queue
keeps only the first few rows per source, so a printer that was
genuinely empty was truncated off the card while a fuller one held its
place. Reported from the floor as "starts at 5% and hides one at 0%",
which is exactly the two bugs compounding.
remaining is already normalised for waste cartridges - a full waste bin
reads as low remaining - so one ascending sort is worst-first for every
supply type, and iscritical falls out of it rather than needing its own
key. Name only breaks ties.
worstremaining is carried on the row because the supplies list is
rendered to text ("Black 0%") before it reaches the client, so the
number the sort needs is gone by then.
The toner forecast report was already worst-first (it sorts on daysleft)
and is unchanged.
Scope: product
(cherry picked from commit
|
||
|
cproudlock
|
dbc43162c5 |
Pin the approved content digest on an endpoint command
Delivery descriptors for run_script and update_app are resolved when the
agent claims a command, not when an operator queues it, and the command
row recorded no digest: the enqueue path resolved the descriptor purely
to validate it and threw the result away.
So what an operator approved and what an endpoint runs were two separate
lookups against mutable content, minutes or hours apart. Anyone able to
replace a script body or an installer package could do so in the gap.
The claim then re-resolves, the descriptor faithfully describes the new
bytes, the agent verifies correctly, and it runs as LocalSystem. The
audit trail names the operator who queued the original.
The gate on the verb and the gate on the payload were also different
grants: pushing needs endpoints.deploy, but replacing what gets pushed
needs only applications.edit.
pinnedsha256 records the digest as it stood at enqueue, and the claim
refuses a descriptor that has moved, failing the push with an audit row
instead of shipping it. Null for actions that deliver no content, and
for rows queued before this migration, which claim as before. A
descriptor that vanished entirely is not treated as tampering: that path
already sends the command through undescribed so the agent fails closed.
Separately, a result may now only finalize a command that is actually
running. That needs no agent change and closes two things: reporting on
a command that was never claimed, and overwriting a recorded failure
with a success.
The agent now sends its hostname on the result callback so the server
can check it owns the command it is finalizing. The shared collector key
does not say who is calling, so without it any agent can post a result
for any command id. The server treats the parameter as optional for
agents that predate this and enforces it when present, so there is no
flag day. This is a floor, not a fix: only a per-endpoint credential
makes the caller's identity provable, and that is tracked separately.
The .NET side is unbuilt here and needs a build plus a win11 VM run.
Scope: product
(cherry picked from commit
|
||
|
cproudlock
|
ccd8221adf |
Key the client address on the connection, not a header the caller sends
client_ip() read the first X-Forwarded-For hop unconditionally. The
login rate limit and the account lockout are keyed on that value and the
audit log records it, so rotating the header per request bought an
unbounded budget and an untraceable trail.
This walked around TRUSTED_PROXY_COUNT, which exists to decide exactly
this. The app leaves ProxyFix off by default and says why, ten lines of
comment away: ProxyFix trusts X-Forwarded-*, a client can send those
itself, and the login limiter is keyed on the address. Then client_ip
read the header anyway, so the stated protection never applied.
Resolution is now from remote_addr, which ProxyFix has already filled
from the header wherever a proxy is genuinely configured. Both cases end
at the same trustworthy source, and the port still comes off because ARR
forwards clientip:port and ProxyFix passes it through verbatim.
geenforce and tools had each hand-rolled the same mistake for their own
limiters. Rather than duplicate security-relevant logic a third time,
client_ip and strip_port join the shopdb.api surface (contract 0.28.0).
geenforce's _trusted_client_ip, which already refused the raw header for
its IP allowlist, is untouched and was the model for this.
The stability table was also missing rows for 0.26.0 and 0.27.0.
INSTALL-WINDOWS-IIS.md never mentioned TRUSTED_PROXY_COUNT, so a site
installed from it runs behind IIS with the setting at 0: every request
looks like it came from IIS, the whole site shares one rate-limit
bucket, and the audit log records the proxy for every action. It now
says so, and says not to set it when nothing is in front.
Three test modules simulated separate callers by setting
X-Forwarded-For, which is the header a real attacker rotates to escape
the limit. One of them, test_client_ip_prefers_forwarded_first_hop_
stripped, asserted the defect outright. The suite could not have caught
this. They now drive REMOTE_ADDR, with a regression guard that forging
the header buys nothing.
Scope: product
(cherry picked from commit
|
||
|
cproudlock
|
d1a7907a2c |
End a user's sessions when the account is deactivated
There is no token denylist in this product, so the JWT user loader is
the only place a deactivated account can be stopped, and it did a bare
lookup by id. Somebody deactivated at 09:00 kept a working access token
until it expired, and a refresh token that minted fresh ones for thirty
days.
/api/auth/refresh and the API-token path already checked isactive. This
makes the loader consistent with them rather than inventing policy.
Scope: product
(cherry picked from commit
|
||
|
cproudlock
|
85e8a7b877 |
Decide guest grants explicitly instead of by permission name
guest_grantable_permissions() opened any permission whose name ended
'.view' or '.read' to the guest role, on the reasoning that reading is
the safe default. That is true of a permission gating a list of asset
numbers and false of one gating bytes, and a suffix cannot tell them
apart. Twenty-four of eighty-five qualified and none had been decided:
applications.view reached the SYSTEM-context PowerShell bodies and
the installer binaries pushed to bays
backups.view returned the full payload that the separate
backups.download permission exists to protect
assets.view gated a global search that queries the staff
directory, USB history and network inventory with
no per-domain check of its own
audit.view usernames, addresses, the before/after of every
settings.view change, and every setting row - both admin
category, opened by naming alone
users.view gates nothing at all; list_users is
require_role('admin')
Every entry is now declared, by CORE_GUEST_GRANTABLE or by the plugin
that owns the permission, having read what its routes actually return.
Adding one is a code change, which is the point: it puts the decision
in a diff rather than in a suffix.
The two routes serving script bodies and installer packages move to a
new applications.download permission, which is not grantable, so a site
can still open the catalogue metadata to its floor without the bytes
going with it.
The declared set preserves the grants this site had already chosen:
applications.view, computers.view, endpoints.view, reports.view,
tools.secrets, warranty.view.
Two known gaps are recorded on the hooks that expose them rather than
silently carried: the computer serializer publishes every column plus
resolved rdp:// and vnc:// links, and the per-asset endpoint panels are
keyed on a small integer so walking ids reconstructs much of the roster
that endpoints.fleet exists to protect. Neither is widened here.
A shipped test was granting guests audit.view and passing.
Scope: product
(cherry picked from commit
|
||
|
cproudlock
|
5f27f81e88 |
Clear the review blockers on the merged wilm work
A multi-agent review of the merged branch produced 29 findings that survived
adversarial verification. This clears the blockers and the four worth fixing
before it reaches main. The design held up well: path-traversal defence on all
four serving routes is clean, the agent's SHA-256 check fails closed, enqueue
authority is proportionate and audited, and no site literals or credential leaks
in the new C#.
THE TESTS NEVER RAN
pytest.ini sets testpaths = tests, so plugins/endpoints/tests/ was collected ZERO
times. Fifty-five lines of delivery assertions were dead code, and run directly
five of them failed. plugins/printedparts/tests/ had been invisible for longer.
That is the worst shape a gap can take: the suite is green, the coverage is
imaginary, and the collector auth on a download route could be deleted without
anything going red.
Both files moved under tests/. tests/test_no_hidden_tests.py now fails if a test
file appears anywhere pytest will not look; it caught a staged copy inside the
installer bundle on its first run, and was verified by planting a violation.
The five failures were stale expectations, and they split by cause. His: the
applications panel made six panels where his test said five, and 'applications'
belongs in KNOWN_PAYLOAD_KEYS now that it is persisted - his own test contradicted
his own feature. Ours: we renamed the events card to activity, moved the
fleet-wide cards to endpoints.fleet, and made the list route a bare route('')
which Flask rejects unless the blueprint is mounted under a prefix.
FOUR FIXES
The snapshot table's key was a 32-bit autoincrement on a table rewritten
wholesale every check-in, so it climbs at (endpoints x apps) per report and
exhausts on MySQL. Widened while the table is unreleased. Widening it broke every
insert on SQLite, which only autoincrements a column typed exactly INTEGER
PRIMARY KEY, so it carries a dialect variant - and nothing caught either fault,
because no test inserted into that table at all. One does now.
Ingest truncated displayname but not displayversion or publisher. On strict MySQL
an over-long registry value fails the whole check-in transaction, so one machine
with a malformed Uninstall entry goes silent entirely. SQLite does not enforce
lengths, so the suite would never have shown it.
update_app's version: branch pinned AppVersion.sha256, but that route resolves the
version to its APPLICATION and serves application.installpath - so the digest
described an artifact the route never sends, and where it was null the agent had
nothing to verify and failed open. It now pins the file actually served.
The same branch ended the delivery URL at the id, so an MSI arrived with no
extension and could not be routed through msiexec. It carries the filename now.
Agent packaging moved off require_role('admin') to a new endpoints.agentpackage
permission. The generated ZIP carries a live collector key, so this is a heavier
grant than deploying to one endpoint - and a role cannot be given to one person
for one job the way a permission can.
Full suite 2231 passed (2198 before, plus the 22 that never ran and 11 new),
frontend 314.
|
||
benjamin.baker
|
7ac1772881 |
Add installer/script delivery metadata to the core software model
AppVersion gains sha256 and silentargs so the endpoint update_app push can pin exact installer bytes and carry per-version silent switches; Application gains installersha256 for the app-level install source. New ShopDB-hosted script library (Script model plus scripts API) backs run_script delivery, with the body fetched by digest on push rather than pre-staged or sent over the command wire. Two idempotent migrations with real downgrades. Script added to the shopdb.api contract surface for plugin use. |
||
|
benjamin.baker
|
5f8061c55d |
Self-heal endpoint command queue wedged by a dead in-flight command
The command queue is at-most-one-in-flight, but nothing ever expired a stuck command. When an agent claimed a command (RUNNING) then died before reporting a result - killed, uninstalled, or offline mid-flight - the row stayed in-flight forever and every operator action for that endpoint returned 409 'already in flight'. Reap in-flight rows past a staleness ceiling (15m for RUNNING, 60m for PENDING) to a new STATUS_EXPIRED at enqueue time, inside the same transaction, so the queue self-heals lazily the next time someone queues work. No background job and no schema migration - status is free-text. |
||
|
benjamin.baker
|
1fa005eb8a |
Green up
Tracking docs/openapi.json (so the app can serve the spec at runtime) makes docs/ exist on the published GitHub mirror, which defeated the skipif(not DOCS.is_dir()) guard in the docs publish/version tests: they then ran against a prose-less checkout and failed on CI. Key the skip off docs/*.md prose instead, matching the canonical published-checkout definition in scripts/check-gates-ran.sh. Also: the OpenAPI generator emitted a trailing-slash path (/api/docs/), which a strict proxy like IIS treats as a redirecting resource a subpath-mounted client cannot follow; normalise trailing slashes in _norm_path and correct the committed spec. And allow docx (python-docx) as an undeclared import: it is used only by scripts/docs/generate_docx.py, a developer tool that never runs on a deployed site. |
||
|
benjamin.baker
|
902339baf9 |
Persist and surface endpoint installed-software (SBOM)
The endpoint agent already reports an installed-software list (SBOM), but ingress dropped it: the collector schema had no entry for it and there was nowhere to store it, so operators could not see what is installed on a PC. Persist it as a per-asset snapshot in a new endpointapplications table, replaced on every report (current inventory is what operators ask for, not install history). Add applications to the collector schema and KNOWN_PAYLOAD_KEYS so ingress stops discarding it, a read route for the raw collection, and a collapsed-by-default Installed software panel to surface it. appid is nullable groundwork for a future Managed Applications link; no matcher is wired yet. |
||
|
cproudlock
|
b5fc5c01d1 |
Stay quiet when a site never asked for supply monitoring
Splitting "cannot see the cartridges" from "nothing is low" was right, but it treated two unlike cases as one. isconfigured means enabled AND a URL AND a token, so its absence means the site did not ask for supply monitoring at all - and a permanent "Supply monitoring is not configured" line is a card reporting nothing wrong every day, which is how a board teaches people to stop reading it. It matters for sites adopting ShopDB without Zabbix, who would otherwise inherit a warning about a feature they declined. Not configured now returns a plain empty and the card hides. Only configured and unanswerable reports itself, because that is something meant to work that is not working. Also withdraws an unfounded inference. A dev box has no route to Zabbix by design, so "not reachable" there is the expected state and says nothing about what production can see. The value of this change is not that it revealed a production fault; it is that when production's Zabbix does go down, the board will say so instead of displaying "Low Supplies 0". Four tests pin the three cases apart: unreachable reports, unconfigured stays quiet, and a real zero from a reachable Zabbix is not flagged as blindness. The fourth holds the envelope's rows, since carrying a reason changed the response shape. Verified they are not vacuous by restoring the original branch in place - the unreachable test fails against it. Full suite 2182 passed. |
||
|
cproudlock
|
96a4a38c8c |
Make the legacy import safe to resume, and aimable without editing it
Three defects broke the loader's own documented resume guarantee. All three were silent: nothing raised, the run finished cheerfully, and fewer rows arrived than anyone counted. THE PAGE SIZE WAS CLAMPED, SO THE 409-RESOLVE SEARCHED THE FIRST 100 ROWS _upsert asked for ?per_page=10000 and read one response. The server clamps any page size to MAX_PAGE_SIZE, which is 100 - not an error, not a warning, just 100 rows. Anything past that resolved to None, so the id never reached the crosswalk and the run quietly dropped a model's link on every machine that used it, or an application's installs. The target already holds more than 100 models and more than 100 applications, so this fired on any re-run or resume - exactly the case the loader documents as safe. _resolve_existing now tries the exact natural-key filter the API has exposed all along (?modelnumber= on models, ?appname= on applications, ?vendor= on vendors) and then pages properly, bounded. An endpoint without such a filter ignores the unknown parameter and the paged walk finds the row. stage_metrology repeated the same mistake in its own way: it skipped 409s entirely, so a resumed run left the tool out of the crosswalk and the Controls edge unmade - metrology PCs driving nothing. stage_printers already had the resolve branch; that stage was written later and missed it. THE CROSSWALK WAS NOT BOUND TO THE DATABASE IT DESCRIBED idmap.json maps this site's legacy ids to assetids in ONE database, and those assetids mean nothing anywhere else. Keeping it across a drop-and-rebuild left warranties and printer assignments attaching to whatever asset now holds that id: wrong data, no error. The runbook's two "drop and rebuild" steps never mentioned the file. The loader now writes a uuid marker into the TARGET's own settings table, so it dies with the data, and stamps it into the crosswalk. A crosswalk built against a different database is refused before a row is written. The database name alone would not do - a rebuild keeps the name. A file written before this guard is adopted rather than refused, so no existing crosswalk is stranded. Also adds --idmap, so one checkout can drive more than one target. THE SOURCE DATABASES ARE NAMED, NOT HARDCODED 'shopdb_src' appeared at 29 call sites, so aiming the loader at the prodscratch restore meant editing run.py. Those names are now LOGICAL and resolved in Source.db() from SOURCE_DB_MAIN / SOURCE_DB_USB / SOURCE_DB_EMPLOYEES, with host, port, user and password overridable too. The call sites are untouched - resolving centrally avoids missing one, and the literals now read as documentation of which source a stage draws from. The run prints every source it redirected, so "it imported nothing" can be told apart from "it read the wrong database". This is what stood between the loader and the prodscratch run. TWO SMALLER ONES _source_password() raised IndexError rather than its intended message whenever the DATABASE_URL user is not root, which it will not be on a real server. main() returned 1 on failure into a caller that discarded it. A run that logged 200 endpoint errors exited 0 and read as a success in a deploy log; it now exits non-zero. TESTS AND DOCS The loader had no tests at all. Sixteen now cover the resolve, the crosswalk binding and the source aliasing - the logic that can run without a MySQL source, which is the part that was wrong. Verified they are not vacuous by running the old algorithm against the same fixture: it returns None where the new one returns the row. The README claimed 15 stages "complete" with counts from 2026-07-13 that two later routing changes invalidated, and the runbook told operators to verify a fresh import against them. Both corrected, with the three gaps NOT fixed here recorded plainly: warranties, notifications and knowledgebase POST without a duplicate check so resuming across them doubles rows; there is no --dry-run or target guard; and the verify stage counts target assets against source machines, which is apples to oranges. Full suite 2178 passed. |
||
|
cproudlock
|
c22c81afa5 |
Show the controls link from both ends, and fix the dead chip links
THE RECIPROCAL LINKS BUG A PC's page showed "Serves machine 7803" at the top. The machine's page showed nothing equivalent, though the same edge was right there in the relationships card at the bottom. That asymmetry read as missing data and was reported as such. It was neither missing data nor an API fault. The endpoint returns both directions and always did; AssetTriageBand already walked outgoing AND incoming and deduped, with a comment explaining that production holds reciprocal rows. It then discarded every peer whose asset type was not 'machine', and hardcoded the label "Serves machine". So the component could only ever work from the PC side, and it was mounted only on the PC page. The counterpart type and label are now props, defaulting to the old values so the PC page is unchanged. The machine page mounts the band with counterparttype="computer" and the label "Controlled by". Eliminated with evidence rather than assumed, because this bug had already been misdiagnosed twice: the endpoint runs two queries and returns both arrays; the generic card renders both; that route has no pagination to clamp; and a single permission gate cannot admit one direction and not the other. THE RELATIONSHIP TYPE WAS ALSO HARDCODED Not every asset pair is linked by 'controls'. A machine reaches a printer through usesprinter and defaultprinter, so the band was blank on the printer page - the one page where the link is not obvious from the asset itself. The types are now a prop, and the printer page passes its own. THE CHIPS WERE DEAD LINKS Found while adopting the band more widely. It linked to /assets/<assetid>. There is no /assets/:id route and no catch-all, so every chip in the band was dead - and this change would have propagated that to four more pages. AssetRelationships.vue already resolved this correctly and privately. That function is now frontend/src/utils/assetRoute.js and both components use it, so a third caller cannot get it wrong again. Two things it gets right that are easy to miss: the detail routes key on pluginid, not assetid, so passing assetid opens the wrong record; and the lookup is now case-insensitive, because csvimport.py seeds 'Computer' and 'Machine' capitalised while this site's database holds them lowercase - a case-sensitive match produced dead links at whichever site seeded the other way. The band now carries pluginid and assettypename on each chip, which the resolver needs. TESTS assetRoute had none and neither did the band. Seven cover the resolver, including the assetid-vs-pluginid confusion and the seeding-case difference. On the backend, nothing asserted that the TARGET of a directional edge receives it in `incoming` - only that rows carry isdirectional. That missing assertion is this bug's exact shape, so it is now pinned: the endpoint was right all along, and the next such report should not cost anyone a re-diagnosis of the API. Verified on screen: the machine page shows "Controlled by G6K76CW3ESF" in the hero, the PC page is unchanged, and a page with no such link renders no empty band. Full suite 2162 passed, frontend 280 passed. |
||
|
cproudlock
|
1cc2a6b782 |
Recommend the smallest upgrade that clears an advisory, not the highest
The rollup took the highest fix version across a component's advisories. That is wrong whenever an advisory is patched on more than one release branch. nanoid is the case that exposed it. Both advisories against it are fixed on two branches - 3.3.16 and 5.1.16, 3.3.18 and 5.1.6 - and the rollup recommended 5.1.16 with a MAJOR warning. But nanoid is transitive, pulled in by postcss at `^3.3.11`, a range no 5.x release can satisfy. The recommendation was not merely a bigger job than necessary, it was unreachable: the actual target is 3.3.18, a patch inside the existing range. Per advisory the target is now the lowest published fix above the version in use; per component it is the highest of those, since one upgrade has to satisfy every advisory against it. Fixes on older branches are skipped rather than offered as a downgrade - axios advisories list 0.30.x fixes that would have been a downgrade from 1.13.2. This also gives the MAJOR label meaning. It now fires only when no fix exists on the current branch, rather than whenever some branch happened to be higher. |
||
|
cproudlock
|
794bb4668f |
Add vulnerability scanning on top of the SBOM
The SBOM was generated and installed onto every server, and nothing read it. "Are we exposed to this advisory" was still a person going and looking, which meant it happened when somebody remembered to. scripts/scan_sbom.py queries OSV.dev by purl - no account, no API key, no agent, no vulnerability database to maintain on a box that cannot reach the internet. The SBOM is already the input OSV wants, so this is a lookup against what shipped rather than a second scan of the tree that could disagree with it. Three decisions worth recording: Shipped and build-only findings are both reported, but only shipped ones can fail a build. Two thirds of the components here are build tooling the SBOM already marks scope 'excluded', and the highest-severity match in the whole tree today is a CRITICAL against the test runner. A gate that fails a release over that trains everybody to bypass it, and then it catches nothing. Severity is taken from the advisory, never derived. Where there is only a CVSS vector the report says UNKNOWN, which is shown and never gates. A guessed score is worse than an honest gap because somebody would act on it. The allowlist is a record rather than a mute button: each entry carries the reason, the accepter, the date, and what would invalidate the reasoning. The first entry is a worked example - a PyJWT algorithm-confusion advisory that needs both mixed algorithm families and a raw JSON JWK as the key, and this application configures neither. The report ends with a per-component upgrade rollup, since that is the part somebody acts on. A target on a different major version is labelled rather than listed beside patch bumps as though the two cost the same. Current state: 8 shipped components have fixes available, none of them exploitable through a path this application opens today. |
||
|
cproudlock
|
60c35728e8 |
Import USB records off an external cmmc_usb database, so a site can retire it
A site running external mode keeps its devices and check-in/out history in a separate cmmc_usb database. There was no way to bring that in, so the choice was to keep the second database forever or lose the history. This adds 'flask usb import-external' - dry run by default, --commit to write - which reads through the same connection settings the routes already use and never writes to the source. Two things it has to reconcile, and they are why it is a command rather than a few INSERTs. The schemas disagree about SHAPE. External is an event log, one row per action; self-hosted is a paired row carrying both times. Pairing walks each device in time order, and real logs after years of use are untidy. The rule is that nothing is invented and nothing vanishes: a drive still out becomes an open checkout and is NOT flagged (that is just a drive somebody has), a return with no matching issue is skipped and named, and two issues in a row leave the first open rather than inventing a return time nobody recorded. Identity is the other. usbdevices.serialnumber is unique and NOT NULL and the external devices table has no serial, so device_id becomes it. Blank or duplicated ids stop that device rather than inventing a key. external device_owner has no column here, so it is appended to the device notes rather than dropped. Also adds usbcheckouts.scannedviruses, the one field with nowhere to land. waswiped already means what external calls sanitized - selfhosted.py was mapping it - but scanned_viruses was HARDCODED None in the log output: the API promised the field and had nothing behind it, so an imported history would have rendered blank. It now reads and writes properly. Nullable, never defaulted to false. An unrecorded scan is not a failed scan, and for removable-media records those are very different claims. Full suite 2131 passed; seven new tests pin the pairing rules. |
||
|
cproudlock
|
af42ba3589 |
Register the endpoints list without a trailing slash, and stop hiding its errors
An endpoint reported telemetry correctly for hours while the Fleet page showed
"No endpoints found". The PC detail page displayed its panels the whole time, so
ingest was plainly working; only the list was empty.
The list route was the one route in 331 paths registered with a trailing slash:
@endpoints_bp.route('/', methods=['GET']) -> /api/endpoints/
The browser client calls it without one, so Flask answered a 308 redirect to an
ABSOLUTE url. On dev that resolves and nobody notices. Behind the IIS subpath it
cannot: with no ProxyFix, Werkzeug builds that Location from the scheme and host
it believes it has, so it emitted http:// and dropped the /shopdb prefix. A
browser will not follow an https page to http, the request failed, and the
page's catch block rendered the empty state.
Every other route in the plugin worked throughout, because none of them carries
a trailing slash - the detail panels call /endpoints/asset/<id>/status and were
never redirected. Detail works, list empty is the signature of this bug.
Registered as '' now, so the rule is /api/endpoints. That is what
CONTRIBUTING.md's API section spells and what every other list route in the
product already did. The inventory and the generated spec are updated to match.
Also here:
- tests/test_core/test_no_redirecting_routes.py fails the build if any /api
rule is registered with a trailing slash and strict slashes. Verified by
planting the violation: it named the offending route, then passed once
reverted. It takes the shared app fixture rather than building its own
application - doing that rebinds SQLAlchemy and leaves the NEXT module
without tables, which showed up as three unrelated "no such table: settings"
failures visible only in a full run.
- The Fleet page now renders "Could not load endpoints (HTTP nnn). This is a
failed request, not an empty fleet." A list that reports a 403 and an empty
fleet identically is what made this take as long as it did.
- TRUSTED_PROXY_COUNT (default 0) applies ProxyFix when set. Off by default on
purpose: it makes X-Forwarded-* trusted, and a client can send those itself,
while the login rate limit and account lockout are keyed on the client
address. Set it to 1 on an instance behind IIS.
Full suite 2124 passed, vitest 260 passed, naming and docs gates green.
|
||
|
cproudlock
|
784964c907 |
Bring the wilm branch into main: endpoints plugin, agent, policies, CSV import
Ports the corporate-GitHub wilm branch onto main, keeping the two branches
separate. Thirteen upstream commits arrive with their authorship intact; the
review fixes and the portability work below are on top.
What arrives:
endpoints plugin - a ShopDB agent reporting endpoint state, telemetry,
services and interfaces, with a command channel the agent
long-polls. Ingest stays on the generic collector route
(ADR-006); the agent key is a UI-managed single-scope
token whose revocation is an instant kill switch.
computers policies - Configuration and Business Policies (RMM-style), scope
trees validated at write time, actions checked against
the endpoints command allowlist. Feature-gated: every
route 404s while the setting is off, so enabling it can
never half-expose the surface.
asset CSV import - templates, a collector script carrying the site path, and
a validate-then-commit import. A plugin can own a virtual
column plus a post-write row hook, which is how a PC's
machine number becomes a controls link without a
hand-built relationships sheet.
agent/ - the Windows agent and its installer.
Contract 0.21.0 -> 0.24.0, additive throughout: register_import_hook (0.22),
mint/list/revoke_service_token (0.23), collector_authorized (0.24). The stability
table had no rows for any of them and now does.
MySQL 5.6 portability. The branch stored three policy columns as sa.JSON, the
only JSON columns in the product, and 5.6 has no JSON type: the migration failed
with a bare syntax error near the word JSON. They are TEXT now, through a
JSONText type decorator, so all seventeen call sites still read and write dicts.
The convention elsewhere is already TEXT (guides.blocksjson,
geenforce.manifestjson).
Review fixes carried on top:
- six policy GETs were jwt_required(optional=True), which authenticates a
caller without checking anything; they require computers.view now, and six
redundant jwt_required decorators are gone
- the agent installer took a default BaseUrl and CollectorKey, so a mistyped
run silently pointed an endpoint at the wrong instance; both are mandatory
and a non-https BaseUrl throws unless -AllowInsecureHttp is passed
- a committing CSV import wrote no audit row
Documentation: the 34 new routes are described in docs/api-inventory.json, which
is hand-maintained on purpose, and the generated spec now covers 331 paths and
477 operations across 22 surfaces. DEVELOPMENT-SETUP's 1071 troubleshooting row
said "use MySQL 8 for dev" and now gives the actual fix, including the trap that
5.6 downgrades ROW_FORMAT=DYNAMIC to Compact with a warning instead of an error.
Full suite 2123 passed, vitest 260 passed, all seventeen plugin chains migrate
clean on MySQL 5.6 and the naming, docs, parity and read-surface gates are green.
|
||
|
cproudlock
|
8953ce8210 |
Ticking a section off crosses out the section
It struck the heading and left everything underneath looking untouched, which is the opposite of what somebody ticking "this section is done" means. The strike now covers every block under that heading, down to the next heading of the same or higher rank - so a top-level section takes its sub-sections with it. That needed the rank, and the HTML importer was throwing it away: h2, h3 and h4 all arrived as a flat 'heading', so a parent heading had no children to cross out. It was the visible half of a real loss - a converted article kept its words and lost its shape. Headings now carry their level and render at it, and a guide written before this reads as flat, exactly as it did. Code blocks, tables and images inside a finished section fade but are not struck. A line through a command makes it unreadable, and re-reading the command is usually the reason to look back at a step already done. The five imported guides were re-imported so they carry their structure. |
||
|
cproudlock
|
afce24379c |
Reading anything needs a permission
A sweep of the URL map found 165 GET routes that answered anybody, with no token at all: the whole asset inventory, every machine, PC, printer and network device, the USB checkout history and who is holding which drive, the staff directory with its photographs, every core report. Guest access had been argued about one feature-flag at a time while the front door stood open. Each of those now carries the read permission it belongs to. That also makes the guest role mean something: a site can open exactly the reads it chooses, and nothing follows by accident. Thirty-three endpoints stay public, and each is named with its reason rather than left to be inferred: the app shell and its bootstrap, the settings allowlist (the handler already filters an anonymous caller down to branding and a few keys), images fetched by <img src> which carries no Authorization header, what a display kiosk renders before anybody signs in, the parts and USB kiosks, the client scripts that run as SYSTEM before a user session exists, and a secure-send link whose unguessable token IS the authorisation. tests/test_core/test_public_read_surface.py is the standing gate, the companion to the mutating-route sweep that has guarded writes for months. A new public read fails CI until somebody adds it to that list in front of a reviewer. Four further reads need a login but no permission, because the handler decides from who you are - your own user record, your own API tokens, the role catalogue the grid renders - and the same test proves each still refuses an anonymous caller. Verified not vacuous by ungating one route and watching the sweep name it. The employees plugin gained employees.view; the directory is names, SSOs, photographs and reporting lines, and it had no permission of any kind. Around ninety tests asserted payload shapes against endpoints that answered anybody. They are about what the answer says, not about who may ask, so they are signed in now rather than the gate being loosened. The ones that assert an anonymous caller is refused were left exactly as they were. |
||
|
cproudlock
|
2331c03664 |
Tick a guide's sections off as you work through it
Only checklist items could be ticked, which answers "what have I done" but not the question a long procedure actually raises: where was I. Somebody looks away to do the thing the guide asked for and looks back at a wall of text. Every step and every heading now carries a checkbox while reading. Ticking one strikes it through and fades it rather than hiding it, because somebody scanning back up the page has to still be able to read what they did. The number is struck through with it, but the step's own "Step 3" label is not - that is the address of the thing, not part of what was done. Headings count, not just steps. A guide converted from an HTML article often has fifty headings and no steps at all, and those are exactly the guides long enough to lose your place in. That also means the card's "N steps to tick" stops reading as a broken feature on a guide that is entirely tickable. Reading only, as the checklists already were: in the editor a tick would be an edit, and an author clicking around their own draft would quietly change what the next reader sees as done. Progress stays per browser and per version - shopfloor PCs share a login, so one technician's ticks must not greet the next person on that bay, and step 4 of version 3 is not step 4 of version 2. |
||
|
cproudlock
|
151d7c4f44 |
Anyone not signed in holds the guest role
Whether somebody with no account may see something was being answered one feature at a time: a securesend_allow_guests setting, an enable flag per kiosk, and very nearly a third for guides - each with its own settings page, its own vocabulary and its own hand-written branch inside a route. Every one of them answered a permissions question, and this app already administers permissions per role. So a caller with no token is not nobody: they hold the seeded `guest` role. require_permission consults it when there is no identity, which is the entire mechanism. The role is seeded holding NOTHING, so an upgrade opens no door; a site opens one by ticking a box in the same grid it uses for every other role, and closes it the same way. Revocation is read on every request rather than cached, because taking guest access away is what somebody does the moment they realise what they opened, and it has to bite immediately. The redundant @jwt_required() above require_permission is gone from 184 routes. It was belt-and-braces before; now it is actively wrong, because it refuses a request with no token before the permission check can have an opinion, so a grant would appear to do nothing. What a guest may hold is bounded. Any .view or .read permission is grantable. A WRITE has to be blessed by whoever owns it, through the new get_guest_grantable_permissions hook (contract 0.21.0), and Secure Send is the only case that exists: sending a password to a vendor is the one write this product means to offer somebody who never signs in. Everything else is refused by the API and greyed out in the grid rather than warned about - a checkbox that quietly makes deleting an asset anonymous is not a warning anybody reads twice. A guest write records no creator: the trail says when and from where, never who, and that is the reason for the bar. GE-Enforce and Slides gained read permissions so they can be opened to guests without handing out the manage right. Their read routes accept either through require_any_permission, so nothing an existing manager could reach has moved. The navigation endpoint now returns only what the caller can actually reach. Without that a signed-out visitor gets a sidebar of the whole app where every link bounces off the login page, which reads as a broken site rather than as a closed door. A nav item may name several permissions; holding any one is enough, which is what GE-Enforce and Slides need now that a viewer and a manager hold different strings. Two things this found on the way: current_user is a LocalProxy, so `current_user is None` is False for a request with nobody behind it - harmless under a required JWT, an AttributeError the moment verification became optional. And an image pasted into a guide before its first save was never attached to it, so it was visible to editors and invisible to every reader; saving now attaches it, and `flask guides fix-images` repairs what is stored. The guides guest setting written earlier today is gone. Granting guides.view to the guest role replaces it, which is the point. |
||
|
cproudlock
|
8531559f22 |
A guide can be carried to another site
Guides are written on one site and needed on all of them, and the sites that matter are air-gapped from wherever the writing happens. Until now a procedure was stranded in the database it was typed into: the HTML importer could rebuild one from its original article, but anything authored in the editor had nowhere to go. flask guides export writes one .guide.json per guide, with every image the blocks reference carried inside it as base64. One file on purpose - a bundle that needs its images copied alongside it is a bundle that will eventually arrive without them. flask guides import reads them back, a directory at a time, and refuses anything that is not a bundle before it writes a row rather than half-importing a stray .json sitting in the same folder. What travels is the content: title, summary, tags, blocks, images. What does not is ids, revision history, authorship and publication state, because those belong to whichever database issued them - an imported guide starts as a draft at revision 1, owned by whoever ran the import. The SLUG does travel: it is the address somebody pastes into a work order, and the same guide should answer to the same name on every site. --replace updates the guide already carrying that slug instead of creating a second one, which is how a corrected guide is re-shipped. It drops the stored images first: the incoming bundle is the whole truth for that guide, and a guide re-shipped monthly would otherwise accumulate orphans nothing points at. Bundles refuse a version they are too old to read rather than importing the parts they recognise. A newer site's guide is not a guide this one can be trusted to render. Proven by round-tripping a five-guide export back into the same site, including one carrying two PNG screenshots, and reading it in a browser. |
||
|
cproudlock
|
c8efadefba |
An HTML article can become a guide
A site adopting this app arrives holding procedures already written: styled HTML pages that get mailed around, opened off a share, and edited by whoever still has the file. Those pages are exactly what a guide is for, and retyping one into the editor is how a good procedure stays on a share forever. flask guides import-html reads the page and produces blocks. It recognises the vocabulary those pages actually use - a step box with its own number, an alert in one of three colours, a code block with a copy button, a checkbox list, a table with a header row - and maps each to the block that already renders it, lifting embedded images into guideimages so a document stops carrying its screenshots as base64. Page furniture goes: the header, the footer, and the table of contents a guide builds for itself from its own headings. The conversion is lossy on purpose. Colour, fonts, width and nested layout are dropped, because storing blocks rather than markup is what makes dark mode, printing and tickable steps work at all. So an import is always a DRAFT and the default is a dry run that reports what it would produce and writes nothing. Blocks go through the same validator the API uses: an import must not be able to store something the editor could not have produced. The parser is tolerant, because real articles are not well-formed. An unclosed <p> is normal HTML and the browser closes it at the next block element; without the same rule one unclosed tag swallows the rest of the article into a single paragraph, which is what it did before the test caught it. Proven end to end against four real articles, including one carrying two PNG screenshots, read back in a browser. |
||
|
cproudlock
|
f59aca5a1b |
USB check-in / check-out audit, as a report rather than a paged list
CMMC removable-media tracking is an audit obligation, and answering "who had this drive, and when" meant paging through the check-in/out list by hand. GET /api/usb/reports/audit returns the events - a take and a return are separate rows, because a count cannot answer the question an audit actually asks - with summary figures riding along for the people who only want the shape of it, and a CSV for the people who want it in a spreadsheet. The window is named rather than a day count: today, last month, six months, a year, all time. Nobody should have to work out that a year is 365. Both directory modes answer in one voice. The external path reads the cmmc_usb checkinoutlog; self-hosted expands one usbcheckouts row into its two ends, so a returned drive is distinguishable from one still out, and the window filter runs in SQL against either end of the loan - a drive taken before the window and returned inside it is a check-in the window has to show. Rows come back from the mode-specific half; the CSV, the summary and the response shape are built once in the route. Ordering is total in both modes. Timestamp alone ties for two events in the same second, and the same query then returns them in a different order each run, which reads as the log being unstable. |
||
|
cproudlock
|
7c47ae1f7c |
USB kiosk: take and return a drive without a ShopDB login
The people who actually take USB drives are standing at the cabinet, and most of them have no account here. Check-out and check-in were gated behind a login that was not doing much anyway: both modes already attribute the action to a scanned BADGE rather than to the signed-in user, so the login was gating the screen, not the record. /usb-kiosk is a full-screen touch page modelled on the parts kiosk, off unless a site sets usb_kiosk_enabled - every route 404s until then, rather than 403, so a disabled kiosk does not advertise that there is a door here. It works against the self-hosted tables or an existing cmmc_usb database. THE LIST IS THE TRADE. These drives carry no barcode, so unlike the parts kiosk there is nothing to present and the screen has to offer a list - which exposes more than a scan-first design, where you had to already hold a drive to learn anything. So the list is not anonymous: an SSO comes first and must resolve to a real person before any drive is named, which is the same bar taking a drive already had to clear. It is a POST so the badge stays out of proxy access logs. The main screen shows the caller's own drives, what is free, and a COUNT of everything else; holders of other people's drives appear only behind a second deliberate tap, so nobody reads the whole cabinet off a wall panel in passing. A badge that resolves to no employee is REFUSED, unlike the signed-in route which creates the user row. A checkout nobody can trace to a person defeats the tracking, and at a kiosk there is no operator standing there to vouch for it. Returning somebody else's drive asks first and then records the badge that was SCANNED, so the log says who actually handed it back. That is the honest record and it needs no change to a schema this app does not own. Refusing a mismatch outright is a setting but not the default: a drive left behind by somebody on leave would otherwise be unreturnable at the kiosk, which just moves the problem to a person with a login. The authz sweep caught these writes, twice - once for the two mutations and again for the two list endpoints. Both are exempted with the reasoning written out beside the parts-kiosk entry rather than silenced. The list endpoints are POSTs that change nothing. Verified by driving the real page: SSO on the keypad, tap Take, database moves. |
||
|
cproudlock
|
a9289ee670 |
printedparts 0003 was an unguarded ALTER, and no test could see it
Unlike its twenty siblings it ran a bare add_column plus create_index, so `flask plugin upgrade-all` failed with "duplicate column name: gagelabtag" on any database whose tables were built by create_all and stamped nowhere - and a chain stops at its first error, so 0004 never applied either. The index is guarded by COLUMN SET rather than by name: create_all names indexes by SQLAlchemy's convention and the migration by this repo's, so a name-keyed guard would have added a second index over the same column instead of skipping. The test that should have caught it was restricted to plugins whose whole chain is a single baseline, on the reasoning that a longer chain meets a second problem - an ALTER whose column is already present - that guarding a create_table does not fix. True, but the conclusion was wrong: the answer is to guard the ALTERs too, not to stop testing the chains that have them. It now covers every post-cutover chain, and was confirmed to fail against the unguarded migration before the guard went in. |
||
|
cproudlock
|
dffa2082be |
Backups: stop storing credentials, and fix a dedup chain that never matched
The NTLARS/DNC registry carries an FTP password in cleartext and it was stored verbatim. That put it in the backuprevisions table, on the revision panel which renders values, and in the download route which hands back a ready-to-import .reg - so anyone holding backups.view could read it off a web page. Redaction happens at PARSE, so the secret never reaches the database at all. Redacting on display would have left it at rest and one query away. What is deliberately NOT done is dropping the value silently. This codec already refuses to discard an unparseable value, precisely because a backup that looks complete and restores a machine with a setting missing is the worst outcome. A redacted value keeps its name, is listed in the projection, and renders as a COMMENT naming what must be set by hand. Emitting a placeholder as the password would produce a machine that looks restored and cannot connect, and the technician would go debugging the network. scripts/redact_backup_secrets.py scrubs revisions recorded before this, dry run by default. It leaves contenthash alone: the hash identifies the configuration a bay reported, so rewriting it would make every bay's next post look like a change and insert a duplicate revision across the fleet. Separately, sourcehostname is now normalized at ingest. samesource() maps '' to IS NULL for the query while the insert stored the raw '', so rows written by a client posting an empty hostname could never match their own next post: every post inserted, the table grew without bound, and the history filled with identical entries nobody could read. Whitespace did the same thing more quietly. |
||
|
cproudlock
|
e60d6b74d8 |
Search: a domain that FAILED no longer looks like one that found nothing
Every domain catches broadly and returns no rows so one broken table cannot take the whole search box down. That part is right. On its own, though, it makes a DOWN domain indistinguishable from an EMPTY one: the technician sees no results, is offered a spelling correction, and concludes the record does not exist. It does - the search just could not look. Failed domains are now named in the response and stated above the results. The suggestion is deliberately still offered on a degraded run. Suppressing it was tried first and is worse: a chronically unreachable domain, an external HR directory say, would silently disable spelling help for every search for ever with nothing saying why. Naming the failure is the honest fix; withholding a second useful signal is not. Guides search also moves to the published revision. It correctly limited itself to published GUIDES, but matched and displayed title/summary/tags/blocksjson from the Guide row - which is the WORKING copy. A published guide with unpublished edits matched on wording that only exists in the draft, and the reader opened it to find the word absent. |
||
|
cproudlock
|
9d96006eae |
Audit log retention: a way to purge, off by default
The audit log is append-only and nothing ever removed a row, so it grows for as long as a site runs. This site reached tens of thousands of pages, which is past the point of being useful: nobody pages through that to find anything. Retention is OPT-IN and the default keeps everything. An upgrade that silently started deleting a site's audit history would be indefensible, so audit_retentiondays defaults to 0 and nothing is removed until somebody sets a number. The Site & Facility card shows the log's size and exactly how many entries a purge would delete BEFORE anything is pressed. Deleting happens in committed batches. That is not an optimisation: one DELETE over a million rows builds a single enormous transaction and holds locks across the table while it runs, which on this server means blocking the collectors posting inventory. The API caps its batch count so the request cannot outlive a proxy timeout, and a run that stops early reports how much is left rather than claiming it finished - a purge reporting success over 900k remaining rows would be the same silent half-failure this codebase keeps getting bitten by. `flask audit stats` and `flask audit purge` for the server, the latter reporting by default and deleting only with --apply, with no batch cap because it is not behind a proxy and a purge that stops halfway leaves the operator guessing. audit.purge is a separate permission from audit.view. Reading the log and destroying it are different acts, and plenty of people need only the first. Chunking selects a page of ids and deletes those rather than using DELETE ... LIMIT, which is MySQL-only while the suite runs on SQLite. |
||
|
cproudlock
|
3c68ac2850 |
ADR-015: the site-literal check was matching nothing, and had been all along
The patterns are assembled from fragments so the published script does not
contain the strings it greps for. They used the apostrophe-escape form
('a'\''b'), which EMBEDS a literal apostrophe, instead of the concatenation form
('a''b'), which joins them. Every pattern therefore carried a stray quote in the
middle and could never match a line of real code, so the check passed everything
from the day it was written.
Verified the only way that means anything: plant a violation, watch it fail,
revert. Fixed, it immediately found 25 real hits.
Among them, in seven places, the default ServiceNow URLs - one organisation's QA
tenant, shipped with the integration ENABLED. Any site that had not configured
ServiceNow still rendered ticket links, pointing its technicians at a tenant
they cannot reach; an adopting site would have been pointed into somebody else's
instance entirely. There is no sensible cross-site default for "which ServiceNow
is yours", so the URLs ship blank and the integration ships off, matching the
Zabbix block that got this right.
A named SaaS tenant is as site-specific as a hostname, so it joins the patterns.
Colocated .spec.js files are excluded the same way tests/ already is: sample data
naming a facility is not a shipped literal.
|
||
|
cproudlock
|
cebb3e9e44 |
Guides: make images render, and stop drafts leaking to readers
Images were a broken icon for every user. The stored URL was bound straight to <img src>, and a plain image request carries no Authorization header while this app keeps its JWT in one, so every fetch was a 401. They are now fetched through the API client, which also fixes the stored '/api/...' path under a subpath mount where the API is not at /api at all. A failed image says so in words rather than rendering as a broken icon, and the editor's own thumbnails go through the same path - an author needs to see the screenshot they pasted. Revision and image routes gated on their parent guide. Both were gated on guides.view alone, and revision ids are sequential, so being able to read one guide was enough to walk the ids and read every unpublished draft on the site - the opposite of what docs/GUIDES.md promises. An image in a PUBLISHED guide still serves to any reader; one that only appears in a draft does not. The guide list, the tag chips and search now match the PUBLISHED text rather than the working copy. Matching the working copy handed readers a hit on wording that is not in the page when they open it. The slug no longer follows the title. It was regenerated on every save that carried a title and the editor autosaves every three seconds, so fixing a typo in a heading silently moved the URL and broke exactly the pasted links the stored slug exists to protect. publish() now aborts when the pre-publish save failed. It ran regardless, so the author saw "Published version N" while readers got the previous revision. Link rendering: text is escaped before formatting, so a captured URL arrives carrying entities and was escaped a second time - every multi-parameter ServiceNow and SharePoint link opened with &amp; in it. Emphasis could also reach inside an href, and '//host' passed as an in-app link when the browser reads it as off-site. Tests are now HTTP-level. The draft-invisibility rule lived entirely in routes that no test executed, which is how three of them shipped serving drafts. |
||
|
cproudlock
|
2c016de59e |
Guides answer the search box, and proofing catches what spellcheck cannot
A guide was invisible to global search. Core search has a hardcoded block per domain rather than a hook, so a new plugin stays unreachable until someone adds one - which is the same shape as the gap this test file already documents for USB devices and printed parts: records with a list page, labels in the world, and no way to find them from the search box. Guides are searched by title, summary, tags AND body. The body matters more here than elsewhere: people look for a procedure by something written halfway down it, not by a title they never read. PUBLISHED ONLY - a draft is deliberately invisible to readers, and search is exactly where that rule leaks if nobody writes it down. The test proves it with a draft sharing a tag and a body word with a published guide, so a missing filter would show up as a ride-along. PROOFING catches what a spellchecker cannot, because none of these are spelling: a smart quote in a command so it will not run, a checklist item with no text so a step is silently missing, a heading left with nothing under it after a reorganise, an image with no alt text, a TODO published by accident. Warnings, never blocks - somebody publishing at 14:40 with one empty bullet should be told, not stopped, and a procedure that exists imperfectly beats one nobody wrote. The smart-character issue reports once per block with a one-click fix, rather than once per offending character: an opening and a closing quote are one problem, and listing both makes the panel look like it is nagging. The character table itself is written as escapes. The style gate forbids non-ASCII in source, and a file whose job is detecting smart quotes cannot contain any - which the gate caught, correctly. |
||
|
cproudlock
|
38d1e367fc |
Guides: write a procedure in ShopDB, not beside it
The knowledgebase plugin is a link registry - a description, a URL and some keywords pointing at a document held somewhere else. That leaves the documents themselves as HTML files on a share: no search, no history, no way to know whether the copy pinned to a noticeboard is current, and no way for a technician to tick off where they are. A guide is stored as an ordered list of typed blocks rather than a blob of HTML, because three of the four things this feature exists for depend on it. A tickable step needs a stable id to be remembered; dark mode needs semantic markup rather than pasted colours; and print needs to know where a step begins so it does not split one across a page. Free HTML gives none of that and lets pasted styling fight the theme. Inline text is a restricted Markdown subset - bold, italic, inline code, links, line breaks - escaped BEFORE formatting, so nothing an author pasted can reach the DOM as markup. No fonts, sizes or colours: emphasis is semantic and the theme decides how it looks. That is what makes a guide written next year match one written today, and dark mode work without an author thinking about it. DRAFTS AND VERSIONS. The guide row is the working copy; every save that changes something writes an immutable revision; and a separate pointer says which revision READERS get. Without that pointer, editing a live procedure changes it under a technician who is halfway through following it. Publishing promotes an existing revision rather than snapshotting, so a publish with unsaved edits cannot ship a version nobody reviewed. Restoring writes a new revision with old content, so the record of what people were told in between survives. Saves dedup on a content hash covering title, summary, tags and blocks, so an autosave that changed nothing writes no version - otherwise the history fills with identical entries and hides the three edits that mattered. Progress lives in the browser, not the database: shopfloor PCs share a login, so per-user ticks would show one technician's progress to the next person on that bay, which reads as "already done". Keyed by guide and version, because step 4 of version 3 is not step 4 of version 2. Five permissions, and the split between edit and publish is the point: a technician can write a procedure up while somebody else decides it is fit for the floor. create-without-edit works too - an author may edit their own guide until it is published, after which other people are following it. |
||
|
cproudlock
|
28af92aaec |
An unmeasurable backup must not hide a working one
CI caught this: judging a host by its stalest kind treated a never-confirmed revision (lastseenat NULL) as the worst case, so it won. The verdict for a NULL confirmation is tri-state None, which renders as NO BADGE - so a host with a healthy ntlars backup and one backfilled row showed nothing at all. Hiding a working backup is the one thing that badge must never do. A NULL confirmation is not stale, it is unmeasurable. The stalest kind is now chosen among kinds that can actually be judged, which is the rule staleness.py already applies: a chain becomes measurable the first time its PC posts and says nothing until then. If every kind is unmeasurable the host still reports which kind it has, with a None verdict and no badge. The test that caught it pinned the old newest-wins behaviour, which part markers made wrong; it now pins stalest-measurable-wins and keeps its NULL clause, which was right all along and is what found the bug. Also worth recording how it was missed: the change was verified against test_geenforce_reporting.py and test_backups.py, and this assertion lives in test_geenforce_publish_gate.py, which was never run. |
||
|
cproudlock
|
52b9fa1ec1 |
Keep the site hostname out of the repair script test
The publication scrub gate rejects internal references, and a real site hostname in a test is still one in the published repo. The rewrite does not care which server it is. |
||
|
cproudlock
|
315d4f46ce |
Repair backup revisions that recorded a mapped drive
Snapshots written before the client was fixed stored "W:\backup\partmarker\..." as their location. That mapping exists only inside the SYSTEM logon session, only during an enforcement cycle, and only on the bay that wrote it, so the path recorded against a marker resolved to nothing for the person most likely to open it. The script rewrites backuprevisions.sharepath and nothing else - no revision is created, deleted, re-filed or re-hashed, and no file on the share moves, since the bytes are already where the UNC will point. Dry run by default. It matches on the drive letter alone and leaves everything else untouched, including paths that are already UNC and paths on another drive. This repairs a specific known mistake rather than normalising whatever it can reach, which is the version of this script that would eventually rewrite something it should not have. rewrite() is kept pure so the decision of WHICH paths to touch is testable without an app context; the tests cover the already-UNC, other-drive, empty, case-difference and trailing-separator cases. |
||
|
cproudlock
|
500f7d6ad6 |
Give the backups plugin a settings page, and show where a snapshot is
The plugin shipped five settings and no page for any of them. The core settings catalog is a hardcoded list a plugin cannot extend, so backups_shareroot and the collection interval were reachable only through the API or a flask shell on the server - which is not a configurable setting in any useful sense, and it is how today went: the setting simply was not there to set. The page is contributed the ADR-010 way rather than by editing core - a card from get_settings_cards plus a route in the plugin's own routes.js, both of which disappear when the plugin is disabled. Filed under Machines, next to Machine Types, because that is where the assets these back up live. It also states the two things the settings do not say for themselves: that health is judged on when a backup was last CONFIRMED rather than when it last changed, and that retention prunes database revisions only - files on the share stay, so enabling it on a share-backed kind leaves orphaned snapshot directories. Snapshot paths are now shown as full UNC text in both places: a Snapshot row on the Marker Contents card, and the existing path on the history page, which gains a copy button. Deliberately NOT a link - browsers refuse to navigate from an https page to file://, so a clickable path looks actionable and silently does nothing, which is worse than no link. Copy, then paste into Explorer. The path comes off the revision, never composed from the configured share root: a snapshot written before the layout changed still lives where it was put. |
||
|
cproudlock
|
ee208808f4 |
Judge a PC's backup by its stalest kind, not its newest
Until part markers, a PC reported exactly one backup kind and "newest revision wins" was the same thing as the truth. A marker bay reports two: ntlars for the machine's DNC settings, partmarker for the controller's patterns. With newest-wins, the daily ntlars post keeps the badge green while the marker backup has been failing for a month. The failure shows only on that marker's asset page, which is not where anyone looks to find out something stopped - and "is this still being checked" is the one question this badge exists to answer. So a host is green only when everything it backs up is current, and the badge names the kind that is not. Freshest revision per kind first, because lastseenat only ever moves on the newest revision of a chain; judging every row would report a healthy, frequently changing config as permanently stale. Known consequence, and the right direction to err: a device retired from a PC leaves its last revision behind, so that host stays stale until the revision is pruned. It complains about a backup that genuinely is not running rather than staying quiet about one. The dashboard card needed no change - it already lists every stale chain independently, so a stale partmarker chain was never hidden there. |
||
|
cproudlock
|
512cc9be76 |
Let a share-backed backup say what is in it
A 'share' kind keeps its bytes on the file share and gives ShopDB only a hash and a UNC path. That is right for an opaque vendor file, but it means the app cannot answer the question a tech actually arrives with - what does this part marker hold, and which pattern disappeared last month. Reading the share from the app server is not an option: the download routes refuse to mount it on purpose, because a permissions slip there becomes an unexplained empty panel. So the collector now accepts an optional manifest alongside the hash: the index, not the content. Names, sizes, per-file hashes, and the marking system each pattern was authored for. Advisory throughout - a client that sends none behaves exactly as before, and dedup still keys on contenthash. The marker asset gains a Marker Contents card built from it: counts by type, the controller family, and a diff against the previous snapshot in the same source chain. Removed files are named rather than counted, because "1 removed" only tells someone to go and look. Files edited in place are named too - the file set can be identical while the content hash moved, and reporting no change there would contradict the reason the revision exists. The manifest is deliberately NOT covered by contenthash, which attests the bytes on the share. A client that starts describing itself therefore posts an unchanged hash, so a no-op backfills a manifest the row does not yet have - otherwise the card would appear only on markers that had recently changed, and stay hidden on every healthy one. The same no-op refreshes sharepath, so a snapshot that moved on the share stops being pointed at where it used to be. Manifests are validated at ingest rather than at render: a malformed one would otherwise become the latest revision and fail every view of the asset page until the content next changed, outliving the client bug that caused it. sharedir() for the marker kind is overridden to the layout the site actually uses, so what ShopDB displays and validates matches what the collector writes. |
||
|
cproudlock
|
d51127841d |
Merge catalog rows that are the same name typed two ways
The legacy data carried space-stripped twins: 'DellInc.' beside 'Dell Inc.', 'Windows10' beside 'Windows 10', 'OptiPlex7080' beside 'OptiPlex 7080'. They rode into the shipped seed catalog, so every site that ran `seed catalog` inherited them, and they stay invisible until somebody opens a vendor picker and finds the same company listed twice. Not a code fault. The collector strips only leading and trailing whitespace, matches with ilike, and treats the OS list as lookup-only, so it can neither have created these nor be creating more. They arrived with the classic-ASP import, which holds the same three pairs today. `flask catalog dedupe` merges only whitespace-and-punctuation twins, because those are one name typed two ways. 'Dell' and 'Dell Inc.' normalise apart and are left alone: that is a naming choice and only a person can make it. The surviving row is the most referenced, tie-broken toward the spaced spelling, which is the one somebody pressed the space bar for. `flask catalog merge` makes that judgment call explicitly, and refuses to invent a target that does not exist, so a typo cannot move rows into a new vendor. `flask catalog rename` handles the case neither can reach: a stripped name with NO twin, where there is nothing to merge into. All three report and change nothing unless --apply is passed, and dedupe is idempotent so it is safe in a deploy script. References are discovered from the LIVE schema rather than listed in code. There are 15 foreign keys into these three tables across 7 tables, and machines.controllervendorid and printerdrivers.modelnumberid are not ones anybody would enumerate by hand. Every reference is repointed before the row is deleted, so nothing is orphaned, and a plugin adding a table is covered without editing this command. |
||
|
cproudlock
|
7d7caba56b |
Let a maintenance script find the database the site actually uses
Running `python scripts/export_catalog.py` on a deployed site died with 1045 "Access denied for user 'root'@'localhost'", naming a user that site does not have. Nothing loads .env for a plain script run. wsgi.py calls load_dotenv() for the served app and the flask CLI loads .env for itself, so both reach the real database; a bare `python scripts/...` gets neither. DATABASE_URL then stays unset, FLASK_ENV defaults to development, and config.py falls back to a hardcoded root@localhost URL. The read-only failure above is the harmless version. Six of these seven scripts WRITE - backfill, reclassify, retype, collapse, refile, import - and on any box where those fallback root credentials happen to resolve, they would have silently rewritten the wrong database with no error at all. That is also why this survived: the dev .env points at root, so the fallback matched what the developer expected and nothing ever looked wrong here. All seven now load .env from the repo root, resolved from __file__ rather than the working directory, since these are run by hand during a deployment and rarely from the repo root. tests/test_maintenance_scripts.py pins it for any future script: every script that calls create_app must call load_dotenv, before create_app rather than after it, and with an explicit path rather than a cwd-relative search. |
||
|
cproudlock
|
7b79a444b8 |
Scaffold a plugin's frontend into the plugin, so it can be pruned
`flask plugin new` wrote views to frontend/src/views/<name>/ and a route file to frontend/src/router/routes/<name>.js. Every one of the 16 bundled plugins keeps its frontend in plugins/<name>/frontend/ instead, staged into .plugins-staged by stage-frontend.mjs and aggregated into routes.gen.js. It was not visibly broken, which is why it lasted: the router still globs ./routes/*.js during the Phase 4 dual-location transition, so a scaffolded plugin's pages loaded. What a new plugin author silently lost was ADR-013 lean builds. Pruning works off the staged tree, so a frontend in the core tree cannot be excluded from a site build, and deleting the plugin strands its views behind. The templates for the correct layout already existed; only the output paths were wrong. Also fixed the imports they carry: core imports now use the @ alias (@/api, @/components/...), which resolves both before and after staging, where the previous ../../api was correct only in the old location. Sibling views stay relative and travel with the route file. frontend_dir is gone from scaffold_plugin. There is no second output tree to point at, and a plugin developed in its own repo now gets the same frontend as a bundled one rather than being skipped. Proven end to end: scaffolded a throwaway plugin, confirmed it stages, builds real chunks, and that SITE_PLUGINS=network prunes it, which is what the old layout could not do. |
||
|
cproudlock
|
eecb0595dd |
Open the record a search result found, not the list it sits in
A NOTICE result linked to /notifications, so clicking the row search had already identified dropped the reader on the list page to find it again by hand. The id was in the payload the whole time; only the URL ignored it. `/notifications/<id>` is the existing notification-detail route. The subnet searcher had the same defect two functions away: an IP search resolves which subnet the address falls inside, then linked to /network, the hub. It now links to /networks/<id>, the subnet detail route. Every other search result type already carried its id. These were the only two that did not. |
||
|
cproudlock
|
adc7ef43f9 |
Stop the collector being able to set a printer assignment
The printer feature rests on one rule: a collector report is OBSERVED state and never ASSIGNED state. If a drifted bay's own state can become what it is told to install, enforcement stops meaning anything, because a bay that installed the wrong printer makes itself right simply by reporting it. _sync_printer_links broke that rule. It ran on every collector PC push and, when the payload carried defaultprinter or printers, wrote defaultprinter and connectedto rows from the report - defaultprinter being one of the two types _reconcile_edges is meant to own alone, from authenticated routes a human calls. It was recorded as dormant. It was not. The collector schema ADVERTISED both keys and the example client in COLLECTOR-INTEGRATION.md set $payload['defaultprinter'], so the documentation was recruiting callers for it - while the same document, under "Observed is not assigned", promised that nothing on this path writes a usesprinter or defaultprinter row. The document contradicted itself and the code followed the wrong half. Nothing in the fleet armed it, because the first-party reporter never sent those fields, but any third-party client written from the docs would have. Retired the function, its _sync_one helper and the now-unused PRINTER_LINK_ORIGIN label, and removed both keys from the schema. IGNORED, NOT REJECTED. Unknown payload keys are not validated, so a client written against the old docs keeps pushing successfully and only its printer keys stop doing anything. The response warns, naming the keys and pointing at POST /api/collector/printers for reporting queues and seed-from-observed for adopting them. Dropping them in silence would leave an integrator believing assignments were still being written, which is a worse failure than the one being fixed. The docs were the larger half of this. The schema table rows are gone, the "PC -> printer relationship sync" section is replaced by a note saying what happens now to anyone still sending the keys, and the example client no longer enumerates Win32_Printer at all - it points at Report-PrintersToShopDB.ps1, which posts to the observed endpoint and posts NOTHING on a failed enumeration, since an empty list would erase the host's rows. test_collector_printers.py pinned the old behaviour, so it now pins the opposite: the keys create no relationships, the push still succeeds and says why, a payload without them stays quiet, a human-made default survives a report naming a different printer, and the schema no longer offers the keys. |
||
|
cproudlock
|
b96637e1a1 |
Let a baseline meet a table that is already there
Reported: `flask plugin upgrade-all` failed the tools plugin with "table securesendsecrets already exists". Every post-cutover baseline emitted a plain op.create_table. That is correct only while a table's existence and its plugin's Alembic stamp can never disagree, and they can. Anything that builds tables from the models - db.create_all(), which `flask db-utils create-all` calls - creates them and stamps nothing. A run that creates a table and then fails before its stamp commits leaves the same split, and re-running is the obvious thing to try. In both cases upgrade-all meets existing tables with an unstamped chain and the create is fatal. Nothing caught it because the migration test DROPS the post-cutover tables db.create_all() built before running upgrade-all. That arranges the one state where an unguarded create works, and never the state the report came from. The new test is the same sequence with the drop removed for the plugin under test. create_table_if_missing and create_index_if_missing are in alembic_template.py next to create_plugin_tables, which had the idempotency rule right and turns out to be called by nobody - its docstring claimed every 0001 used it. Fixed the docstring rather than rewiring the baselines onto it, because the reason they hand-write their DDL still stands: the helper's per-plugin MetaData cannot resolve a foreign key to a core table at CreateTable-compile time. INDEXES ARE MATCHED ON COLUMNS, NOT NAME, and that distinction matters. The two creators disagree about names - db.create_all() takes SQLAlchemy's default convention (ix_securesendsecrets_token) and a baseline spells it the way this repo's naming rules require (ixsecuresendsecretstoken). A name-keyed guard would have called the index missing and added a second one over the same column, which MySQL permits with a warning, so every affected site would have quietly grown duplicates. The test asserts no table ends up with two indexes over the same columns. Applied to all nine table-creating plugin migrations, not just the tools baseline that was reported: the four later migrations that create tables have the same exposure, and geenforce, measuringtools, printedparts and backups are the same defect waiting for a different site. WHAT THIS DOES NOT FIX. It makes a baseline idempotent, not a whole chain. A longer chain can still meet an ALTER whose column is already present - printedparts adding gagelabtag to a table create_all built at head is the case that showed up while testing. Guarding every op one at a time is the wrong answer to that; a database built from the models is already AT head and its chains want stamping rather than replaying, and there is no `flask plugin stamp` today. Left alone deliberately rather than half-built, and the new test is scoped to single-baseline plugins so it does not pretend otherwise. |
||
|
cproudlock
|
8750ba2e74 |
Let a site open secure send to guests, and choose how many times a link opens
Two asks, and the first is a real security decision so it is a SETTING that defaults to the safe answer rather than a code change somebody inherits. securesend_allow_guests, OFF by default. With it off the model is unchanged: a login plus tools.secrets. With it on, somebody with no account - at a shared bay terminal, a contractor - can create a link too. The cost is named in the setting's own description and pinned by a test: a guest secret has NO CREATOR, so the audit trail can say when and from where but never who. That is the whole reason it is a switch a person turns on deliberately. IT FAILS CLOSED. A missing row, a half-migrated site, an unhappy database - the answer is no. An error must never be the thing that opens an unauthenticated write endpoint. A SIGNED-IN USER STILL NEEDS THE PERMISSION even when guests are allowed, which is surprising enough that a test pins it: opening the door to guests is a decision about the door, not about revoking what a role holds. Guest creation is rate limited per address, same shape as the login limiter. Not a security boundary - a determined person has more addresses - but an open endpoint should not be an easy place to park data or fill a disk. VIEWS ARE NOW A NUMBER, 1 to 100, instead of a four-item dropdown. How many people need a password is the sender's business: a shift handover is not one colleague on a call. The floor is what matters, not the ceiling - unlimited views would just be a password on a web page. Also folded in, from the retention question: nothing removed a secret that EXPIRED WITHOUT BEING OPENED. The recipient goes on holiday, the link is never clicked, and the ciphertext sat there for good - not readable, but it made "nothing lingers" untrue and grew the table in exactly the case where the feature failed. Any create now sweeps expired rows and a late click tidies its own, so it stays bounded without a scheduler an air-gapped site would never add. Two gates caught mistakes on the way: current_user is a LocalProxy and is never None, so `current_user is None` silently took the signed-in branch for a guest; and cache was imported straight from shopdb.extensions when it is already on the shopdb.api contract surface. 1836 tests green. |
||
|
cproudlock
|
d4b49b91b4 |
Move secure send into Tech Tools, where somebody would look for it
|
||
|
cproudlock
|
98458672eb |
Hand somebody a password without leaving it in a chat window
The habit this replaces is pasting a credential into Teams, where it stays until someone deletes the conversation. Tabled on 2026-08-21 pending a decision about scope; the decision was text only, so this is the half that survives it. BACKEND ONLY. There is no page yet, so nothing here is reachable by a person. TEXT, NOT DOCUMENTS, and that shapes everything. Dropping files removed the export-control question entirely - a password is not technical data - along with blob storage, size and MIME handling, and the need to authenticate the recipient so a document fetch could be pinned to a name. TWO DOORS. Creating needs a login AND securesend.create: anonymous creation would be an untraceable way to move data out of a system that otherwise records who did what, and the creator is the only person these rows can ever name. Reading needs neither, because the recipient usually has no account here and the link is handed over on a call - the unguessable token IS the authorisation, which is why it is 128 bits of randomness and why the row is spent on first read. REDEEMING IS A POST, and that is the detail that would have broken this in practice. Teams, Outlook and Safe Links prefetch GET urls to build previews and scan for malware, so a GET that redeemed would let a preview window consume a secret before its recipient ever saw it. The GET route that remains reports only whether a link is still live, so the read page can say "opening this will destroy it" without spending the only view to find out. THE SIZE LIMIT IS THE WHOLE ENFORCEMENT. The key lives in the URL fragment, which browsers never transmit, so the server holds ciphertext it cannot read and cannot tell a password from a page of a drawing. MAX_CIPHERTEXT is therefore not a tuning knob: raising it quietly turns this into the document transfer it was scoped not to be. The refusal says what the limit is FOR, because somebody hitting it is usually trying to send a file and the answer is a different tool. Line counts, file signatures and refusing a pasted file belong in the browser, guide an honest person, and are bypassable by anyone willing to POST directly. NOT ENUMERABLE. Missing, expired and spent all answer identically - telling them apart would confirm to anyone holding a guessed token whether it ever named anything. A spent or expired row is deleted on the way past rather than waiting for a sweeper, because it has no reason to keep its ciphertext. Its own plugin rather than a Tech Tool: that plugin is browser-only by design, and this needs storage, expiry and a view counter. Not an asset extension either, so no assetid and no asset type - the scaffold assumes one and it was removed. FOUND BY ITS OWN TESTS: expiryhours=0 returned 201. `int(x or DEFAULT)` treats an explicit zero as absent, so "expire immediately" silently became "expire in a day". A caller who sends a number is now told when it is not allowed rather than quietly given a different one. BEFORE ANYONE RELIES ON THIS, test that your link rewriter preserves URL fragments: create a secret, send the link to yourself in Teams, click it from the message, and check the key is still after the #. If Safe Links strips it the secret is unrecoverable and the server cannot help, which is the point of the design. The fallback is passphrase mode, worth building only if that test says so. Three product gates caught what a new plugin has to declare, and each was right to: the route-parity gate wanted the endpoints described in api-inventory.json, the lean-build guard wanted the plugin listed in the universal profile the released installer builds from, and the authz sweep wanted the public redeem route named in EXEMPT_ENDPOINTS with a reason. That last one matters most - a mutating route that answers an unauthenticated caller is a decision somebody has to write down, not a default, and reading a secret is a mutation because it spends it. |