Describe the API this branch actually serves
docs/openapi.json is served live at /api/docs on the production box and documented none of the routes added since 2026-09-11. It is generated from docs/api-inventory.json, which is not tracked on this branch, so the generator had no input - and tests/test_api_inventory_parity.py skipped itself for the same reason. No route here had ever been checked against the published contract. The inventory is now generated from this branch's own url_map rather than carried over from elsewhere, with each route's auth derived from the decorators and gates on its view rather than guessed. A route whose gate cannot be classified is marked for review, not quietly called public. gen_openapi.py can refresh it from the live app, so this does not rot again, and human-authored prose survives a refresh. The parity test now RUNS instead of skipping. The thing that mattered more than the missing pages: nine operations were published with "security: []" - the machine-readable claim that no credentials are needed - while their handlers require one. Two of them serve the bootstrap installer and the agent package. A generated client, a firewall plan or anything else reading the spec would have been told those are anonymous. Required auth is now expressed as required, and a token-or-network gate is expressed as optional auth with the network half named in the description, which is what it actually is. A test re-derives auth from the live app and fails when the committed inventory disagrees, because the parity check only ever compared route lists: a wrong auth string passed everything. test_openapi_spec.py asserted that optional auth was the majority of the surface, a premise the guest-role and permission model has made false. It now holds the property that matters - an optional-auth route is expressed as optional - rather than a count that was true once. gen_openapi.py also loads .env before building the app, like every other script that builds one, or a plain python run falls back to the hardcoded localhost URL.
This commit is contained in:
@@ -1,20 +1,26 @@
|
||||
"""Every route the app serves is described in docs/api-inventory.json.
|
||||
|
||||
The inventory is hand-maintained, and deliberately so: its value is the prose -
|
||||
what each endpoint is FOR, what it accepts, a worked example - and none of that
|
||||
can be derived from the code. An audit of all 372 documented operations found
|
||||
zero phantom routes and zero wrong parameter names, so the maintenance itself is
|
||||
in good order.
|
||||
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.
|
||||
|
||||
What it cannot do is notice a route somebody added. Twenty-two were serving
|
||||
traffic undocumented, including an entire plugin's surface and every one of the
|
||||
dashboard card endpoints - each of them added after its surface was written.
|
||||
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.
|
||||
|
||||
So the prose stays hand-written and the COVERAGE is enforced here. Adding an
|
||||
endpoint now means describing it, in the same commit, which is when the author
|
||||
still knows what it is for.
|
||||
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
|
||||
@@ -93,3 +99,87 @@ def test_nothing_documented_has_been_removed():
|
||||
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])))
|
||||
|
||||
@@ -72,15 +72,83 @@ def test_write_verbs_can_carry_a_body(spec):
|
||||
'from this spec cannot send one:\n ' + '\n '.join(offenders[:15]))
|
||||
|
||||
|
||||
def documented_auth(operation):
|
||||
"""The auth string the generator wrote into this operation's description.
|
||||
|
||||
The spec is the only thing this module reads, and the description is where
|
||||
the human-readable auth lands ('**Auth:** jwt-optional (...)'). Comparing
|
||||
the published `security` against it is how a lie gets caught: the two come
|
||||
from the same inventory entry and must agree.
|
||||
"""
|
||||
match = re.search(r'\*\*Auth:\*\*\s*(.+)', operation.get('description') or '')
|
||||
return match.group(1).strip().lower() if match else ''
|
||||
|
||||
|
||||
def test_optional_auth_is_expressed_as_optional(spec):
|
||||
"""An endpoint that works logged out must not be published as requiring a
|
||||
token - that is a lie to every reader, and it is the majority case here."""
|
||||
optional = [operation for _, _, operation in operations(spec)
|
||||
if any(item == {} for item in operation.get('security') or [])]
|
||||
assert len(optional) > 50, (
|
||||
'The inventory documents over a hundred jwt-optional endpoints; the '
|
||||
'spec should express them with an empty security requirement '
|
||||
'alongside the scheme, not as bearer-required.')
|
||||
"""A route whose auth is optional is published as optional, either way round.
|
||||
|
||||
WHY THE COUNT IS GONE. This asserted `len(optional) > 50`, on the premise
|
||||
that optional auth was the majority case here - it said so in as many
|
||||
words. It is not any more. The guest role and the permission model moved
|
||||
the read surface to `permission:*`, which publishes as bearer-required and
|
||||
is correct, and the optional count fell to 14. The assertion went red
|
||||
without a single route being wrong.
|
||||
|
||||
A count was never the property anyone cares about. It goes red when the
|
||||
codebase legitimately changes shape, and it stays green while an individual
|
||||
optional route is published as bearer-required, which is the actual defect.
|
||||
Lowering the number to 14 would only buy time until the next legitimate
|
||||
move. So the premise is dropped and the per-route property is checked
|
||||
instead, whatever the total: an operation whose auth says optional carries
|
||||
BOTH the empty requirement and a scheme, and an operation that publishes
|
||||
the empty requirement says optional in its auth.
|
||||
"""
|
||||
offenders = []
|
||||
for path, verb, operation in operations(spec):
|
||||
auth = documented_auth(operation)
|
||||
security = operation.get('security') or []
|
||||
openly = any(item == {} for item in security)
|
||||
scheme = any(item for item in security)
|
||||
if 'optional' in auth:
|
||||
if not openly:
|
||||
offenders.append(
|
||||
'%s %s: auth says optional, published as required (%r)'
|
||||
% (verb.upper(), path, security))
|
||||
elif not scheme:
|
||||
offenders.append(
|
||||
'%s %s: auth says optional, published with no scheme at '
|
||||
'all, so a caller is never told what the credential is'
|
||||
% (verb.upper(), path))
|
||||
elif openly:
|
||||
offenders.append(
|
||||
'%s %s: published as optional, but its auth says %r'
|
||||
% (verb.upper(), path, auth))
|
||||
assert not offenders, (
|
||||
'Optional auth means "works without a credential, does more with one", '
|
||||
'and OpenAPI writes it as the empty requirement ALONGSIDE the '
|
||||
'scheme:\n ' + '\n '.join(offenders[:15]))
|
||||
|
||||
|
||||
def test_an_operation_needing_no_credential_says_so_in_its_auth(spec):
|
||||
"""`security: []` is a machine-readable claim that no credential is needed.
|
||||
|
||||
Nine operations published it while their handler required one, including
|
||||
the two that hand a bootstrapping PC its agent package - `security: []` and
|
||||
a 404 for anyone who does not present the single-use token. The generator
|
||||
reached that by substring: the auth string started with the word 'none', in
|
||||
a sentence that went on to name the token. This is the gate on that class
|
||||
of defect from the spec side, where a reader or a generated client sees it.
|
||||
"""
|
||||
offenders = [
|
||||
'%s %s: %r' % (verb.upper(), path, documented_auth(operation))
|
||||
for path, verb, operation in operations(spec)
|
||||
if not operation.get('security')
|
||||
and 'none' not in documented_auth(operation)
|
||||
and 'public' not in documented_auth(operation)
|
||||
]
|
||||
assert not offenders, (
|
||||
'These operations publish "no credentials needed" while their auth '
|
||||
'names a credential:\n ' + '\n '.join(offenders[:15]))
|
||||
|
||||
|
||||
def test_an_endpoint_that_needs_no_token_cannot_fail_with_401(spec):
|
||||
|
||||
Reference in New Issue
Block a user