ge-aerospace/shopdb-flask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Validate what a form accepts, and keep a diff to one asset

Browse Source 
The validation tail from the 2026-09-03 run. Each is small; together they are
the difference between a field that checks its input and one that looks like it
does.

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

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

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

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

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

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

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

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

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

Scope: product
This commit is contained in:
cproudlock
2026-09-03 16:13:59 -04:00
parent 1a8f426e93
commit 8fee09e748
13 changed files with 531 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
shopdb/core/api/applications.py
View File

@@ -2,6 +2,7 @@
import glob
import hashlib
import re
import os
from flask import Blueprint, request, current_app
@@ -20,6 +21,7 @@ from shopdb.utils.responses import (
)
from shopdb.utils.pagination import get_pagination_params, paginate_query
from shopdb.utils.import_mode import apply_import_timestamps
from shopdb.utils.datevalues import DateValueError, parse_date
# Uploads live in the instance dir, like model images and part photos, so a
@@ -326,6 +328,24 @@ def create_version(app_id: int):
if not data or not data.get('version'):
return error_response(ErrorCodes.VALIDATION_ERROR, 'version is required')
# A digest is a claim the delivery path CHECKS against, so a wrong one is
# worse than none: an over-long value was silently truncated to the column
# width and came back as 64 plausible hex characters, indistinguishable
# from a real SHA-256. Either it is a SHA-256 or it is refused.
digest = (data.get('sha256') or '').strip().lower()
if digest and not re.fullmatch(r'[0-9a-f]{64}', digest):
return error_response(
ErrorCodes.VALIDATION_ERROR,
'sha256 must be 64 hexadecimal characters')
# Refused rather than stored raw: a malformed date used to reach MySQL,
# which (before STRICT_TRANS_TABLES) wrote 0000-00-00 and then broke every
# read of this application.
try:
releasedate = parse_date(data.get('releasedate'), 'releasedate')
except DateValueError as bad:
return error_response(ErrorCodes.VALIDATION_ERROR, str(bad))
if AppVersion.query.filter_by(appid=app_id, version=data['version']).first():
return error_response(
ErrorCodes.CONFLICT,
@@ -336,9 +356,9 @@ def create_version(app_id: int):
version = AppVersion(
appid=app_id,
version=data['version'],
releasedate=data.get('releasedate'),
releasedate=releasedate,
notes=data.get('notes'),
sha256=data.get('sha256'),
sha256=digest or None,
silentargs=data.get('silentargs')
)

12
shopdb/core/api/maplevels.py
View File

@@ -177,9 +177,17 @@ def create_level():
data = request.get_json() or {}
name = (data.get('levelname') or '').strip()
buildingid = data.get('buildingid')
# Names the fields this route actually reads. It said 'buildingname',
# which this handler never looks at, so somebody who omitted the level name
# was told to supply something else entirely.
if not name or not buildingid:
return error_response(ErrorCodes.VALIDATION_ERROR,
'buildingname and buildingid are required')
missing = [field for field, value in
(('levelname', name), ('buildingid', buildingid))
if not value]
return error_response(
ErrorCodes.VALIDATION_ERROR,
'%s %s required' % (' and '.join(missing),
'is' if len(missing) == 1 else 'are'))
if not db.session.get(Building, buildingid):
return error_response(ErrorCodes.NOT_FOUND, 'No such building',
http_code=404)

28
shopdb/core/api/settings.py
View File

@@ -811,6 +811,34 @@ def update_setting(key: str):
if _is_secret(key) and value == SECRET_MASK:
return success_response(_serialize_setting(setting), message='Setting unchanged')
# The declared valuetype is a promise about what every reader will get, and
# nothing checked it: "A6-not-a-number" was stored verbatim on an integer
# key and every consumer then read 0, because Setting.get_typed_value
# swallows the ValueError. The setting looked set, the behaviour was the
# default, and nothing said so.
valuetype = (setting.valuetype or 'string').lower()
if value is not None and not isinstance(value, bool):
text = str(value).strip()
if valuetype == 'integer' and text:
try:
int(text)
except ValueError:
return error_response(
ErrorCodes.VALIDATION_ERROR,
'%s is a whole-number setting (got %r)' % (key, value))
elif valuetype == 'float' and text:
try:
float(text)
except ValueError:
return error_response(
ErrorCodes.VALIDATION_ERROR,
'%s is a number setting (got %r)' % (key, value))
elif valuetype == 'boolean' and text:
if text.lower() not in ('true', 'false', '1', '0', 'yes', 'no'):
return error_response(
ErrorCodes.VALIDATION_ERROR,
'%s is a true/false setting (got %r)' % (key, value))
# Convert value to string for storage
if isinstance(value, bool):
setting.value = 'true' if value else 'false'