Backup folders on network shares often refuse SQLite's file locks (CIFS
without nobrl, NFS without lockd). SQLite reports a refused lock as
SQLITE_BUSY, so every backup to such a folder retried until the 60 second
give-up and blamed a locked database, while the live database was not
locked at all (#1246).
The target is a scratch .pending file that nothing else opens, so open it
with nolock=1. The give-up message now names the live database, since that
is the only file a backup can still wait on.
can_write_to_data_dir() touched and then deleted data/test.txt, which
silently wiped any file of the user's that happened to have that name.
It now uses tempfile.NamedTemporaryFile in the data dir, which picks an
unused name and removes only the file it created.
The metrics refresh only ever set label sets, never removed them, so a
spool that was archived or deleted (or a deleted filament, or a renamed
filament's old info labels) kept being exported with its last values
until the server restarted.
Each refresh now drops the label sets the database no longer has. The
current values are set before the stale ones are removed rather than
clearing the gauges first, because /metrics is served from a worker
thread and a scrape landing mid-refresh would otherwise see live series
briefly disappear.
sqlite3's Connection.backup() retries forever while the source is busy,
calling progress with 0 of 0 pages each time. The backup ran synchronously
on the event loop, so a lock that did not clear froze every request and
spammed "Copied 0 of 0 pages." into the log (#1191).
Run backups in a worker thread, give up after the source has been locked
for 60 seconds, and serialize backups now that they can overlap. Per-page
progress drops to debug.
measure() treated a spool_weight of 0 the same as a missing one and fell
back to the filament's tare, and since #1164 the vendor's. A refill wound
on no spool has a real tare of 0, so its scale reading was short by the
filament's spool weight. Only a missing tare now falls back.
The server checks the latest GitHub release in the background at startup
and every 12h, logs a warning when a newer version exists, and exposes
latest_version/update_available on /info. The client_v2 footer shows a
link to the new release. Disable with SPOOLMAN_UPDATE_CHECK=FALSE.
The filament pickers only ever showed the first 8 SpoolmanDB matches, so entries further down a broad search could not be reached. They now load 20 at a time and fetch the next page when the result list is scrolled to the bottom.
GET /external/filament/search takes an optional offset and returns the total match count in x-total-count, like the other list endpoints. Requests without offset behave as before. The lowercased search text is built once per catalog load instead of on every search, since paging means one scan per page.
Fixes#1136
A tag can now be linked to a filament type, not just to one spool of it. The
tag table already carried filament_id and target_type for exactly this, so
there is no migration.
API, all additive:
POST /filament/{id}/tag link; 409 if anything else holds the UID
DELETE /filament/{id}/tag/{uid} unlink
GET /filament?tag=<uid> exact lookup on the normalized UID
Filament.tags also inside every spool's embedded filament
TagScan.matched_filament_id always present, like matched_spool_id
TagConflictMessage.filament_id names a filament holder on a 409
One UID still identifies one thing across both kinds: a spool's tag cannot be
linked to a filament or the reverse, and neither ?tag= lookup answers with the
other kind. The scan debounce key now includes the kind, since spool 7 and
filament 7 are different answers.
SpoolTag and SpoolTagParameters are renamed Tag and TagLinkParameters while
they are unreleased, so the schema does not describe filament tags as spool
tags forever.
client_v2: the filament inspector gets the Tags section, the link dialog names
and moves a tag held by either kind, and scanning a filament's tag opens that
filament when auto-navigate is on.
Tag having two delete-orphan parents was checked on SQLAlchemy 2.0.51: new tags
flush, unlinking deletes the row, and deleting either parent cascades.
Proven on sqlite, postgres, mariadb and cockroachdb: 558 integration tests
each. Unit tests, svelte-check and the Playwright tag specs pass.
The Add tag dialog waits for a fresh tap. That is fine when the tag is a
sticker in your hand, and awkward when the reader is a scale: the spool is
sitting on the pad to be weighed, and filling in the field means lifting it
and putting it back. A reader also cannot help here, because it has no reason
to report a tag that has not moved.
The registry already tracks each reader by id, name and last_seen, all in
memory. Keeping the last UID beside them is enough for a client to offer what
is on the reader right now, with no back channel to the device, no protocol
change, and nothing to update on the agent side - every existing ESPHome
reader gets this for free.
One UID, not a history: this answers "what is on the reader", not "what has
been". It lives and dies with the registry entry, so it is gone on restart
like the rest of it, and no scan is written anywhere.
The relay debounced on (uid, reader_id) and time alone, so a scan whose match
had changed looked like a duplicate of one that resolved to nothing. An agent
that taps an unlinked tag, links it in its own UI, then re-reports the scan had
its correction swallowed for the whole 3 s window, leaving every subscriber that
trusts the broadcast still showing an unknown tag.
The key now includes matched_spool_id, so a repeat is only a repeat while the
scan still resolves to the same thing. Repeats of a still-unlinked tag keep
collapsing on (uid, reader, None), which is the reason the debounce exists.
should_broadcast takes the match as a required argument rather than an optional
one so a future call site cannot quietly drop it again.
Fixes#1115
A tag maps a UID to "the thing to bring up", and only some of those things are
spools -- or rows at all. Locations are the case that settles the shape: a
location in Spoolman is a String(64) on Spool, not a table, so a tag meaning
"show me Shelf A" can never be a foreign key.
Getting this wrong is asymmetrically expensive. Renaming the table, dropping
spool_id's NOT NULL and adding a discriminator all mean altering a populated
table on four databases; adding an endpoint or a target kind later is additive
and free. The migration has shipped to nobody -- it is not on master and
v0.26.1 predates it -- so the schema is settled now and the unshipped revision
is rewritten in place rather than chased with a second one.
spool_tag becomes tag:
uid unchanged, still globally unique -- one physical tag, one thing
target_type which kind of target is in force, rather than leaving readers
to infer it from whichever column is non-null
spool_id nullable now; targets that ARE rows keep real foreign keys, so
filament_id the database still cascades deletes and refuses orphans
target_value targets that are not rows; a new kind of this sort costs no
migration at all, only a new target_type string
Exactly-one-target is enforced in database/tag.py rather than by a CHECK: no
migration in this tree uses one, and MySQL below 8.0.16 parses and silently
ignores them, so a constraint here would be real on three databases out of
four. Single write path beats a constraint that is only sometimes a constraint
-- the same argument, in the same module, as normalize_uid.
filament_id gets a column and a foreign key but no ORM relationship: nothing
writes filament tags, and a selectin collection on Filament would add a query
to every filament listing for rows that cannot exist. ON DELETE CASCADE holds
integrity until the feature that needs the relationship arrives.
No API surface changes. POST /spool/{id}/tag, ?tag=, the spool response's tags
and matched_spool_id are all exactly as they were; the reads are just explicit
now about wanting a spool rather than assuming the tag they found is one.
TagConflictError's spool_id becomes optional for the same reason, which the
client already handled -- it only offers "move it here" when there is an id.
Proven on sqlite, postgres, mariadb and cockroachdb: 526 integration tests each.
register() kept datetime.utcnow() while should_broadcast() and readers() moved
to datetime.now(tz=utc), so the naive last_seen it stored was compared against
an aware cutoff during pruning: GET /tag/reader raised "can't compare
offset-naive and offset-aware datetimes" as soon as any reader had scanned.
The existing unit tests could not catch it because they inject `now` into every
call. Added one that exercises each method's own clock instead.
Also adds design/nfc-phase-1-frontend-handoff.md: the API shapes the client
will consume, the behaviours it has to get right, two stale paths in the phase
1 doc's frontend plan, and the open UI decisions.
Implements the backend half of design/nfc-phase-1-tag-identity.md. Format
agnostic: it keys only on a tag's hardware UID, so it works with blank NTAG
stickers, Prusa NFC-V tags, and the Bambu/Creality/Qidi MIFARE tags whose
contents Spoolman can never decrypt.
Answers the question the whole external ecosystem has been working around:
which spool is this physical tag? Every project so far invented its own way to
smuggle a Spoolman id into the tag contents, which fails the moment the tag is
a vendor tag you cannot write to.
Data model: new spool_tag table, additive only, with a unique index on uid.
Many tags per spool, since copying a vendor tag onto a blank sticker leaves one
spool with two UIDs. UIDs are normalized to uppercase hex with separators
stripped, in the database layer rather than a Pydantic validator, so the unique
constraint holds for callers that never arrive over HTTP.
API, all additive:
GET /spool?tag=<uid> exact match on the normalized UID
POST /spool/{id}/tag 201, 409 with the conflicting spool id
DELETE /spool/{id}/tag/{uid} 204
POST /tag/scan broadcast and return the match
GET /tag/reader readers seen recently
WS /tag/scan[/{reader_id}] the relay
Link and unlink emit the existing spool event, so clients need no new wiring.
Spool responses gain a tags array.
The relay decouples where the reader is from where the user is looking: an
agent POSTs a scan, a paired browser reacts. Scans are ephemeral, never
persisted, and go through a SECOND WebsocketManager instance rather than the
shared one. The shared tree propagates to subscribers at every level, and the
root /api/v1/ pool means "any change", so putting scans there would inject a
novel resource into every existing consumer's stream. An integration test pins
that they never reach the entity websockets.
Repeated identical (uid, reader_id) scans are debounced before the broadcast
only, since readers re-read a tag that is sitting still. The response always
carries the match, so a de-duplicated scan never looks to a device like a
failed lookup.
Migration creates the table and its indexes in one revision, following the
extra-fields tables rather than the doc's suggested split. The DDL/DML split
cockroachdb needs does not apply when there is no data migration, and the 4-DB
suite confirms it.
Verified: 506 integration tests on sqlite, postgres, mariadb and cockroachdb;
263 unit tests; ruff clean.
Frontend, docs, and the #783 action links are not included.
SubscriptionTree.send let a per-socket failure escape the broadcast loop, so
every subscriber after the failing one silently received nothing. A socket can
start closing between the CONNECTED state check and the send, and starlette
then raises "connection is closing"; on the shared tree that means one browser
tab closing could stop Moonraker hearing about a spool.
Each subscriber is now isolated: a write failure drops that socket and the
broadcast continues. Iterate a snapshot too, since dropping mutates the set.
Dropping also had to stop going through remove(path, ...). By the time send()
is walking a node, path is the part of the route still to be traversed, not the
pool the socket subscribed with, so remove() descended into a child and looked
in the wrong set: the dead socket stayed put to fail again on the next event.
Observed in the integration suite as a subscriber missing an event it was
subscribed to, alongside "Failed to send websocket message" on an unrelated
filament broadcast.
The validator fix only guards the write path. Instances that already hit #780
keep 500ing on GET /filament because the stored value exceeds the response
model's max_length, and the UI can't repair what it can't list.
Filament.from_db now strips the # and returns None when the stored value still
isn't a valid 6/8 character code, so a single bad row no longer takes down the
whole list. A migration chained from 415a8f855e14 rewrites existing rows one by
one through op.get_bind() rather than with SQL string functions, so it behaves
the same on sqlite, postgres, mysql and cockroachdb. downgrade() is a no-op.
The color_hex and multi_color_hexes validators normalized a local copy of
the value (uppercasing it and removing a leading "#") in order to validate
it, but then returned the caller's original string. A "#"-prefixed 8-digit
color such as "#FF000000" was therefore stored with 9 characters, which no
longer fits the String(8) color_hex column, so the write failed and every
subsequent read of that filament returned a 500.
Both validators now return the value with the "#" removed, matching the
un-prefixed form the rest of the codebase and the frontend already assume.
Adds tests/test_color_hex_validation.py covering both validators for the
prefixed and un-prefixed, 6- and 8-digit forms.
The unit tests are supposed to run with no database, no server and no
built client, but tests/test_configjs.py imported spoolman.main, which
builds the SPA handler at module level and so needs client_v2/build to
exist. That passed locally, where the bundle is lying around, and failed
CI's style job, which never builds the client.
Moves the rendering into client.render_config_js, which the endpoint now
calls, so the test exercises the real code path without the app.
Task 10: config.js JSON-encodes the base path, so a backslash cannot
escape the string literal the way the old " check allowed; search.py
escapes LIKE wildcards at both its call sites, using a helper moved into
database/utils.py so it and extra_field_query share one implementation.
Task 9 was a pair of decisions rather than fixes. /info keeps its paths:
both clients render them and removing response fields breaks API v1, while
the disclosure buys nothing against an API that is unauthenticated anyway.
/metrics keeps its shape but now says what it publishes when switched on.
Also pins the unit tests to a throwaway data directory -- importing
spoolman.main creates one as a side effect, and it was the real one.
Defaulting it on was the wrong trade. The only hostname it can refuse is a
registrable public domain, so it broke exactly the reverse-proxy-on-a-real-
domain deployments that configure nothing today -- a 400 on every request
until they found a new variable -- while the deployments rebinding can
actually reach are addressed by IPs, single-label names and .local, which
it never refuses anyway. The group that breaks and the group that gains
barely overlap.
SPOOLMAN_ALLOWED_HOSTS is now the on switch as well as the allowlist, so
there is no state where an operator has to think about this without having
asked for it. SPOOLMAN_CORS_ORIGIN deliberately does not enable it, or
setting CORS for a Fluidd instance would start refusing your own hostname.
Closes the DNS rebinding hole, which the origin guard structurally cannot:
under rebinding the attacker's page keeps its own name, so Origin and Host
agree with each other and the origin guard trusts them both, while the
address behind that name is the victim's Spoolman instance.
The default allows everything a stranger cannot point at someone else's
machine -- any IP literal, any single-label name, and the non-registrable
suffixes -- because what the attack needs is a registrable public domain.
That leaves a reverse proxy on a real domain, which cannot be told apart
from the attack and so must declare itself via SPOOLMAN_ALLOWED_HOSTS.
Each list endpoint parsed `sort` inline with a bare split(":"), so `?sort=name`
raised "not enough values to unpack" and `?sort=name:sideways` a KeyError from
SortOrder[...]. Both surfaced as a 500 (todo.md task 8).
parse_nested_field also gated on hasattr, so any attribute that merely existed
passed through to order_by() -- `?sort=metadata:asc` got as far as calling
.asc() on the MetaData object. It now checks the SQLAlchemy mapper's columns,
so only real columns are accepted; relationship traversal through filament. and
vendor. still works.
Sort parsing moves into a shared parse_sort() used by all four call sites, which
tolerates surrounding whitespace and rejects a missing colon, an empty field
name or an unknown direction with a message saying what was expected.
database/vendor.py turned out not to use parse_nested_field at all -- it did a
bare getattr(models.Vendor, fieldstr) -- so /vendor kept returning 500 even
after the helper was fixed. It now uses the shared helper like filament.py.
All three endpoints verified end-to-end: 400 for each malformed form, 200 for
valid single and multi-field sorts, and no unhandled exceptions in the log.
Two lower-severity findings from the audit (todo.md tasks 6 and 7).
A spreadsheet executes a cell that begins with =, +, -, @, tab or CR, so a
vendor named `=cmd|' /C calc'!A0` runs when someone opens the CSV export
(CWE-1236). Such cells are now prefixed with a single quote, which spreadsheets
read as "this is literal text" and do not display. Only strings are escaped --
numbers reach the writer as numeric types, so a negative number cannot be
mistaken for a formula and must not grow a stray quote. Header names need no
escaping: they are fixed attribute names or extra-field keys, which are already
constrained to ^[a-z0-9_]+$.
Exports also gained Content-Disposition, so they download under a sensible name
rather than rendering in the browser.
Extra-field values had no size limit and the columns are Text(), so a single
request could store megabytes: unbounded database growth, and the resulting
websocket event overflowed the 1 MiB default frame limit, which killed live
updates for every connected client. Values are now capped at the same 65535
characters as settings. The number of extra fields per entity is capped too --
the per-entity key count was already bounded, since unknown keys are rejected,
but the registry itself could be grown without limit, and it is cached in memory
and embedded in every entity response.
Finally, a setting larger than the column limit returned 500; the ValueError is
now caught and returned as a 400.
Verified end-to-end: a 2 MB extra value is refused on both create and update, an
oversized setting returns 400 and leaves the setting unset, normal values still
work, and a vendor named `=cmd|' /C calc'!A0` exports as `'=cmd|' /C calc'!A0`
while the JSON export keeps the raw name.
allow_origins=["*"] with allow_credentials=True makes Starlette echo the
caller's origin back and set Access-Control-Allow-Credentials, which tells the
browser it may send cookies to whichever site asked. Spoolman has no cookies of
its own, so this looked harmless, but plenty of instances sit behind a reverse
proxy that does the authentication with one -- and there it let any website the
user visited make authenticated requests as them. Now credentials are refused
whenever the resolved origin list contains "*" (todo.md task 5).
Adds a README Security section. The README has no configuration reference at
all -- that lives in the Wiki -- so rather than starting one, this documents the
things a user has to know before deciding where to run Spoolman: that there is
no authentication by design, that it belongs behind an authenticating reverse
proxy if it is reachable beyond the LAN, what SPOOLMAN_CORS_ORIGIN is for, why
"*" is a bad idea on a shared network, and that a reverse proxy needs to forward
the original Host.
Verified with SPOOLMAN_CORS_ORIGIN=* against a real server: the response now
carries `access-control-allow-origin: *` and no credentials header, where the
audit saw the caller's origin echoed with credentials allowed.
Rotating deletes the oldest restore point and shifts the rest down, so enough
calls in quick succession leave nothing but snapshots of whatever state the
database is in now. POST /backup takes no parameters, so a bodyless cross-origin
form post reached it; the audit destroyed a full six-deep history that way
(todo.md task 2).
The origin guard from the previous commit already refuses that request, but the
rotation itself should not be this easy to abuse, so two further changes:
Rotation is skipped when the database has not actually changed. Not by comparing
the live database file, which the audit suggested: under WAL the live file does
not yet contain recent commits, so comparing it would skip backups that were
genuinely needed. Instead the snapshot is written to a pending file first and
compared against the newest backup -- sqlite's backup API reads through the
connection, so it sees WAL content. Identical means the pending file is
discarded and the history is left alone. This is fail-safe: if the comparison
ever came out unequal for an unchanged database, the result is the old behaviour
of rotating, never a missed backup.
Rotation is also rate-limited to one per five minutes, returning the newest
existing backup instead. Never applied when there is no backup to fall back on.
BackupResponse gained a `created` field so a client can tell the difference.
Additive, so v1-compatible, but load-bearing: telling a user they have a fresh
restore point when they do not is exactly the wrong failure mode here.
Verified against a real server -- six rapid calls now leave exactly one backup
and never rotate.
Closes the two confirmed CSRF findings and the cross-site websocket hijack
from the backend security audit (todo.md tasks 1 and 3).
POST /setting/{key} took a bare `str` body, and FastAPI only JSON-parses
application/*json -- anything else arrived as raw bytes that lax-mode Pydantic
coerced. A text/plain body was therefore accepted, which is exactly what
<form enctype="text/plain"> sends, so any website the user visited could
rewrite base_url, label_designs, locations or extra_fields_* through their
browser. Now requires a JSON content type.
The audit suggested Body(media_type="application/json") for this, but that only
annotates the OpenAPI schema and is not enforced at runtime -- a text/plain post
to a pinned endpoint still returns 200. Hence an explicit dependency. Both
clients already send application/json, and a charset parameter is accepted, so
this is wire-compatible.
TrustedOriginMiddleware refuses state-changing HTTP methods and every websocket
handshake from an untrusted origin, using the helper added in the previous
commit. A middleware rather than per-endpoint dependencies so that none of the
nine websocket endpoints can be missed, and one added later cannot forget to
opt in. Reads are left alone -- the same-origin policy already covers them --
but every websocket is guarded, since websockets are exempt from CORS and
otherwise leak the whole inventory to any origin that opens one.
It is ordered inside the CORS middleware so a legitimate cross-origin client
can read the 403 body instead of an opaque CORS error.
Writing a malformed extra_fields_* array through the settings endpoint bypassed
validate_extra_field and left GET /field/{entity} permanently returning 500;
that array is now validated, and the registry cache is invalidated on write so
the /setting and /field paths no longer disagree until restart.
Verified against a real uvicorn server by replaying the audit's exploit: the
text/plain write is refused, a bodyless form post that would have reset a
setting is refused, a websocket from a foreign origin gets HTTP 403 on the
handshake, and clients that send no Origin at all -- Moonraker, OctoPrint,
Home Assistant, curl -- are unaffected on both HTTP and websockets.
Tasks 1-3 of the backend security audit all need to answer "is this browser
origin someone we trust". Build it once, driven entirely by the existing
SPOOLMAN_CORS_ORIGIN rather than a new env var.
- Unset means same-origin only, which is right for nearly every deployment.
Non-browser clients (Moonraker, OctoPrint) send no Origin and are unaffected.
- A list means those origins plus our own.
- "*" and debug mode mean "I do not want origin checks" and are honoured as
such, with a loud startup warning. An operator who writes "*" is opting out;
reinterpreting it as something narrower would leave them no way to say so.
Also accept X-Forwarded-Host, so a reverse proxy that rewrites Host (nginx
without proxy_set_header Host $host, Apache without ProxyPreserveHost On) does
not see the guard reject its own web UI. A malicious page cannot set that
header on any of the vectors the guard exists to stop: forms cannot set request
headers, nor can the browser WebSocket API, and a fetch that adds one needs a
CORS preflight it will not get.
get_cors_origin() now normalizes its entries -- it was a bare split(","), so
"https://a, https://b" yielded " https://b", which no Origin header can match.
CORS itself benefits from the same fix. The raw value is kept in the startup
log so a typo stays visible.
Adds tests/ for backend unit tests, runnable with `poe test` and wired into the
lefthook ci group. Nothing enforces this helper yet; that is tasks 1-3.
Donkie