The spec described 573 operations with a schema each and almost nothing a
reader could copy: 12 operations carried an example, and no operation described
a query parameter at all, because a Flask handler's query string is invisible to
url_map. A route whose real interface is ?code= documented nothing.
Both are now derived from evidence rather than written by hand.
QUERY PARAMETERS come from each view's AST - request.args.get, the subscript
form, getlist, and this repo's own helpers - and a read that does not resolve to
a literal name is not emitted. 0 to 87 operations, 350 parameter objects. The
enumerations some handlers validate (sort=assetnumber|hostname) are deliberately
NOT published: deriving the accepted set reliably needs more than a literal
read, and a wrong enum is exactly the invented contract this work avoids.
EXAMPLES are harvested from the test suite, which already sends real bodies and
asserts real responses, so they are transcripts rather than inventions. 12 to
about 324. The harvest is re-runnable and optional; generation works without it.
THE HARVEST PUBLISHED TWO REAL CREDENTIALS, and that is the important part of
this commit. A bootstrap token and a kiosk setup code reached docs/openapi.json
inside URL values - a file served at /api/docs on production and copied to a
wiki. Neither scrubber layer could see them: the keys are 'scripturl' and 'url',
which name no secret, and the opaque-value rule excludes ':' and '/' on purpose
so that example URLs survive. Nothing was committed.
Four changes came out of it:
- a URL-credential rule that scrubs the parameter VALUE and keeps the URL, so
?t= is redacted and ?page=2&perpage=50 still reads as an example
- redaction runs on the way OUT as well as on the way in. Harvested examples
live in the inventory and a refresh preserves them, so a scrubber fixed after
a harvest protects only future harvests while the stored copy keeps
publishing. The published file can no longer carry a secret the current rules
would catch, whatever the inventory holds
- the leak gate now holds its own patterns instead of importing the generator's.
Checked with the scrubber's own rules it could only ever assert that redact()
ran, and it was green while both tokens sat in the file. It now knows the
product's token prefixes, a URL-credential shape, and a crude entropy
backstop. Replanting the exact credential that shipped fails it
- a DELETE only declares a body when the HANDLER reads one, derived from its
AST. A transcript proves the client sent something: the suite posts {} to
routes that ignore it, and that published "this route reads a JSON body" on
DELETE /api/endpoints/agentkey, whose handler never calls get_json.
249 operations still carry no example, because no test exercises them with a
JSON body. The honest fix is a test, not a hand-written example, and the next
harvest picks each one up for free.
246 lines
10 KiB
Python
246 lines
10 KiB
Python
"""Every route the app serves is described in docs/api-inventory.json.
|
|
|
|
The inventory has two halves. Its STRUCTURE - surface, method, path and the auth
|
|
each route actually enforces - is derived from the live url_map and the
|
|
decorators on each view by `gen_openapi.py --refresh`. Its PROSE - what an
|
|
endpoint is FOR, what it accepts, a worked example - is human, and a refresh
|
|
never overwrites words that are already there.
|
|
|
|
This module is the coverage half of that, and it is the half a generator cannot
|
|
do for you: a refresh describes whatever is served at the moment it runs, so
|
|
without a gate the file rots the moment the next route lands.
|
|
|
|
It had. On this branch the inventory was not tracked at all, so the skipif below
|
|
fired, this module reported green while checking nothing, and 24 routes went to
|
|
production documented nowhere - including the whole endpoint-agent delivery
|
|
surface and both kiosks. scripts/check-gates-ran.sh now asserts this module RAN,
|
|
in every checkout rather than only in one with wiki prose in it.
|
|
|
|
So: add a route, run `gen_openapi.py --refresh`, and replace the seeded purpose
|
|
with the reason the route exists - which is while the author still knows it.
|
|
"""
|
|
|
|
import importlib.util
|
|
import json
|
|
import re
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from shopdb import create_app
|
|
|
|
REPO = Path(__file__).resolve().parents[1]
|
|
INVENTORY = REPO / 'docs' / 'api-inventory.json'
|
|
|
|
pytestmark = pytest.mark.skipif(
|
|
not INVENTORY.is_file(),
|
|
reason='docs/ is excluded from publication; nothing to check here')
|
|
|
|
# Flask's own machinery, not this product's API.
|
|
EXEMPT_ENDPOINTS = {'static'}
|
|
|
|
# Documented as a family rather than per-file: these serve the frontend bundle
|
|
# and its assets, not JSON.
|
|
EXEMPT_RULE_PREFIXES = ('/static', '/assets')
|
|
|
|
|
|
def normalise(rule):
|
|
"""Flask `<int:id>` and inventory `<id>` describe the same path.
|
|
|
|
Converters are an implementation detail of the route, and the inventory was
|
|
written without them. Comparing on the converter would report a difference
|
|
that is not one.
|
|
"""
|
|
return re.sub(r'<(?:[^:>]+:)?([^>]+)>', r'<\1>', rule)
|
|
|
|
|
|
def served_operations():
|
|
app = create_app('testing')
|
|
served = set()
|
|
for rule in app.url_map.iter_rules():
|
|
if rule.endpoint in EXEMPT_ENDPOINTS:
|
|
continue
|
|
if any(rule.rule.startswith(prefix) for prefix in EXEMPT_RULE_PREFIXES):
|
|
continue
|
|
if not rule.rule.startswith('/api'):
|
|
continue
|
|
for method in sorted((rule.methods or set()) - {'HEAD', 'OPTIONS'}):
|
|
served.add((method, normalise(rule.rule)))
|
|
return served
|
|
|
|
|
|
def documented_operations():
|
|
documented = set()
|
|
for surface in json.loads(INVENTORY.read_text()):
|
|
for entry in surface.get('endpoints', []):
|
|
path = normalise(entry.get('path') or '')
|
|
# The inventory writes multi-verb routes as 'PUT|PATCH'. Splitting
|
|
# on comma and slash alone left those undocumented in this check
|
|
# AND dropped from the generated spec entirely.
|
|
for method in re.split(r'[/,|]', entry.get('method') or 'GET'):
|
|
method = method.strip().upper()
|
|
if method:
|
|
documented.add((method, path))
|
|
return documented
|
|
|
|
|
|
def test_every_served_route_is_documented():
|
|
missing = sorted(served_operations() - documented_operations())
|
|
assert not missing, (
|
|
'%d route(s) serve traffic with no entry in docs/api-inventory.json. '
|
|
'Describe them there (purpose, auth, params, example) and regenerate '
|
|
'the spec with scripts/gen_openapi.py:\n %s'
|
|
% (len(missing), '\n '.join('%s %s' % row for row in missing)))
|
|
|
|
|
|
def test_nothing_documented_has_been_removed():
|
|
"""A documented route that no longer exists sends a reader to a 404."""
|
|
stale = sorted(documented_operations() - served_operations())
|
|
assert not stale, (
|
|
'%d documented route(s) are no longer served:\n %s'
|
|
% (len(stale), '\n '.join('%s %s' % row for row in stale)))
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# The other half: the auth string, which nothing gated until now
|
|
# ---------------------------------------------------------------------------
|
|
#
|
|
# The two tests above check that a route EXISTS in both directions. Nothing
|
|
# checked what the entry SAYS about it, so a wrong or stale auth string passed
|
|
# every gate in the repo - and auth is the field a site reads to decide what its
|
|
# guest role and its firewall may reach, and the field the published spec turns
|
|
# into a machine-readable security requirement.
|
|
#
|
|
# It had rotted exactly that way: nine operations documented as needing no
|
|
# credential while the handler required one, and a kiosk route documented as
|
|
# ungated when its gate is a single-use setup code. Both survived a refresh,
|
|
# because a refresh only rewrites; nothing compared.
|
|
#
|
|
# So this re-derives auth from the live app with the generator's own classifier
|
|
# and fails on any disagreement with the committed file. The derivation is
|
|
# authoritative: hand-editing an auth string in the inventory does not survive
|
|
# the next --refresh, so the fix for a wrong one is always to teach the sweep in
|
|
# scripts/gen_openapi.py, or to gate the view.
|
|
|
|
|
|
def _generator():
|
|
"""scripts/gen_openapi.py, loaded by path.
|
|
|
|
It is a script, not a package module, so there is nothing to import. Loading
|
|
it runs its imports and nothing else - it builds no app until it is asked
|
|
to.
|
|
"""
|
|
path = REPO / 'scripts' / 'gen_openapi.py'
|
|
spec = importlib.util.spec_from_file_location('gen_openapi', path)
|
|
module = importlib.util.module_from_spec(spec)
|
|
spec.loader.exec_module(module)
|
|
return module
|
|
|
|
|
|
def derived_auth():
|
|
"""{(METHOD, path): auth} as the classifier reads the code right now."""
|
|
generator = _generator()
|
|
app = create_app('testing')
|
|
reviewed, blueprints = generator._reviewed_exceptions()
|
|
derived = {}
|
|
for rule, methods, endpoint, view in generator._served_routes(app):
|
|
auth = generator._auth_for(view, endpoint, reviewed, blueprints)
|
|
for method in methods:
|
|
derived[(method, normalise(rule))] = auth
|
|
return derived
|
|
|
|
|
|
def documented_auth():
|
|
"""{(METHOD, path): auth} as docs/api-inventory.json has it committed."""
|
|
documented = {}
|
|
for surface in json.loads(INVENTORY.read_text()):
|
|
for entry in surface.get('endpoints', []):
|
|
path = normalise(entry.get('path') or '')
|
|
for method in re.split(r'[/,|]', entry.get('method') or 'GET'):
|
|
method = method.strip().upper()
|
|
if method:
|
|
documented[(method, path)] = entry.get('auth') or ''
|
|
return documented
|
|
|
|
|
|
def test_documented_auth_is_what_the_code_enforces():
|
|
"""A stale auth string is a lie about who may call a route.
|
|
|
|
Only routes present on both sides are compared. A route missing from either
|
|
is a coverage fault, and the two tests above already name it; reporting it
|
|
twice here would bury this one.
|
|
"""
|
|
derived = derived_auth()
|
|
documented = documented_auth()
|
|
disagreements = []
|
|
for key in sorted(set(derived) & set(documented)):
|
|
if derived[key] != documented[key]:
|
|
disagreements.append(
|
|
'%s %s\n documented: %s\n enforced: %s'
|
|
% (key[0], key[1], documented[key] or '(blank)', derived[key]))
|
|
assert not disagreements, (
|
|
'%d route(s) document an auth that is not what the code enforces. The '
|
|
'inventory is derived, so fix the code or teach the classifier in '
|
|
'scripts/gen_openapi.py, then re-run it with --refresh:\n %s'
|
|
% (len(disagreements), '\n '.join(disagreements[:15])))
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# The third derived field: query parameters
|
|
# ---------------------------------------------------------------------------
|
|
#
|
|
# A Flask rule carries its path arguments and nothing else, so a route whose
|
|
# real interface is '?code=' or '?search=' documented no parameters at all -
|
|
# /api/usb/kiosk/setup among them, where '?code=' is the ONLY way to call it
|
|
# successfully. gen_openapi.py now reads them out of each view's AST and writes
|
|
# them into the inventory as `queryparams`.
|
|
#
|
|
# That field is DERIVED, like auth, and gated the same way and for the same
|
|
# reason: a refresh only rewrites, so without a comparison a hand-edit or a
|
|
# stale file publishes a query contract the handler does not implement. A
|
|
# parameter that is not really there is the worst kind of documentation defect,
|
|
# because a reader believes it, sends it, and is silently ignored.
|
|
|
|
|
|
def derived_queryparams():
|
|
"""{(METHOD, path): [parameter, ...]} as the sweep reads the code now."""
|
|
generator = _generator()
|
|
app = create_app('testing')
|
|
derived = {}
|
|
for rule, methods, endpoint, view in generator._served_routes(app):
|
|
parameters = generator._query_parameters(view)
|
|
for method in methods:
|
|
derived[(method, normalise(rule))] = parameters
|
|
return derived
|
|
|
|
|
|
def documented_queryparams():
|
|
"""{(METHOD, path): [parameter, ...]} as the inventory has it committed."""
|
|
documented = {}
|
|
for surface in json.loads(INVENTORY.read_text()):
|
|
for entry in surface.get('endpoints', []):
|
|
path = normalise(entry.get('path') or '')
|
|
for method in re.split(r'[/,|]', entry.get('method') or 'GET'):
|
|
method = method.strip().upper()
|
|
if method:
|
|
documented[(method, path)] = entry.get('queryparams') or []
|
|
return documented
|
|
|
|
|
|
def test_documented_query_parameters_are_what_the_handler_reads():
|
|
derived = derived_queryparams()
|
|
documented = documented_queryparams()
|
|
disagreements = []
|
|
for key in sorted(set(derived) & set(documented)):
|
|
if derived[key] != documented[key]:
|
|
disagreements.append(
|
|
'%s %s\n documented: %s\n read: %s'
|
|
% (key[0], key[1],
|
|
[item['name'] for item in documented[key]],
|
|
[item['name'] for item in derived[key]]))
|
|
assert not disagreements, (
|
|
'%d route(s) document query parameters that are not what the handler '
|
|
'reads. The field is derived, so regenerate with '
|
|
'scripts/gen_openapi.py --refresh:\n %s'
|
|
% (len(disagreements), '\n '.join(disagreements[:15])))
|