Repository navigation
security(api): route docstrings are published to the OpenAPI schema and into generated api.ts — a security rationale written there is a disclosure #16827
Description
Activity
- added a commit that references this issue
on Sep 16, 2026 Surveyed the tree, and found the mechanism that bounds the exposure
The concern raised on this issue was fair: both known instances were found by accident, so "2 found"
was not "2 exist" and the tree was unsurveyed. It is surveyed now, and the answer is better than
expected — for a reason worth building the guard around.Method
AST sweep over
autobot-backend/**/*.py, selecting functions carrying an HTTP-verb route decorator,
matching defect-narrative language in the docstring (not bare issue references — a first pass keyed
on#\d{4,5}matched 1,655 of 2,424 handlers and discriminated nothing).The detector was validated against both known positives before it was allowed to report, so a clean
result means "looked and found nothing" rather than "did not look":detector validated against 2/2 known positives examined=2392 python files, 2424 route handlers, matched_lines=16Result: 16 matches, and effectively none are live disclosures
- 12 are security-scanning endpoints legitimately describing their own subject —
analytics_dfa.py
get_vulnerabilities,code_intelligence.pysecurity_analyze,list_vulnerability_types,
security_assessment.pyadd_vulnerability. A vulnerability scanner's docstring says
"vulnerability". Not findings. - 3 are intentional design notes stating an access property rather than a defect:
heartbeat.py:324("can be resumed by any authenticated user"),settings.py:857
("available to any authenticated user (not just admins)"),llc/api/companies.py:210
("open to any authenticated user"). These are what a consumer should be told. - 1 is a real defect narrative:
api/user_management/users.py:102search_users_for_sharing—
"security(users): /user-management/users/search answers anonymous callers with names from every organisation #16279: this route used to need no login. It searched through a hand-built platform-admin context
with no org, so an anonymous caller could list names from every organisation."
The one real narrative is not published, and that is the useful finding
FastAPI uses a route's docstring as the OpenAPI
descriptiononly when the decorator does not supply
one.search_users_for_sharingpasses an explicitdescription=, so its docstring never reaches the
schema. Verified empirically against the committed artifact rather than inferred from the framework's
documented behaviour:grep -c "anonymous caller could list names" api.ts -> 0 (docstring, shielded) grep -c "for sharing dialogs" api.ts -> 1 (the decorator's description=) grep -c "User-paused agents can be resumed…" api.ts -> 1 (plain docstring, published)Three greps, three different predictions, all confirmed.
What this changes about the fix
The exposure set is not "every route docstring". It is "route docstrings on handlers that do not
pass an explicitdescription=". That is a smaller, precisely computable set, and it makes the guard
cheaper and more accurate than the keyword check originally suggested here:- A guard should flag defect language only on handlers lacking an explicit
description=, which
removes theusers.pyclass of false positive entirely. - It also hands authors a second legitimate remedy alongside "move it to a comment": supply an explicit
description=with the contract, and the docstring stays free for whatever the next maintainer needs.
That is strictly better than deleting real reasoning to satisfy a linter.
users.py:102should still probably get an explicit comment rather than relying on the shield, since
the shield is invisible and one removeddescription=republishes it — but it is not a live disclosure
today and should not be counted as one.Standing count
Live published disclosures found on
main: 0. The two known instances were both on branches and
both are fixed (695373b61on #16811;f711dd3c9on #16442). The sweep script is in this session's
scratchpad and is ~40 lines; worth landing as the guard's starting point rather than rewritten.- 12 are security-scanning endpoints legitimately describing their own subject —
Third instance in one day — and this one publishes a live gap, not a fixed one
Recording a third occurrence, because the pattern across the three changes what this issue is asking for.
# PR What reached the artifact Defect state 1 #16811 the #16580 presence authorisation narrative already fixed 2 #16442 "any authenticated user could delete any host" already fixed 3 #16854 "no code path currently checks paused-task state before continuing work" still live The first two described defects that were closed by the same PR carrying the docstring. The third
describes a control that does not work today — emergency stop pauses a task set nothing downstream
consults (#16843, currentlyneeds-decision). So a precise description of how to defeat a safety
control is in a generated frontend artifact while the control is still broken.And it was not hypothetical exposure: the auto-fix-generated-types bot had already committed the
narrative verbatim intoapi.tsby the time a reviewer saw it.Why the earlier survey result does not settle this
My sweep on this issue measured zero live published disclosures on
mainacross 2,424 route
handlers, which was accurate — and is the wrong measurement to act on. It counted the stock. The
problem is the flow: three new instances entered in a single day, from three different authors,
each caught by a reviewer or by the bot happening to commit where someone was already looking. A stock
of zero maintained entirely by luck and reviewer attention is not a controlled state.Detection rate is the number that matters here, and it is unmeasurable by construction — we know about
the three that were caught and nothing about any that were not.What changes about the ask
The guard suggested on this issue should be built, not left filed. The mechanism established
earlier makes it cheap and accurate: FastAPI uses a docstring as the OpenAPIdescriptiononly when
the decorator does not pass one, verified empirically against the committed artifact. So the check only
needs to fire on route handlers without an explicitdescription=, which removes the false-positive
class entirely (users.py:102carries a defect narrative and is correctly shielded).Two further points the three instances establish:
- Security PRs are the high-risk case, because a reviewer reasonably wants the reasoning near the
code. All three instances were security or safety work. The guard should not ask authors to delete
reasoning — 42's resolution on security(secrets): admin-gate infrastructure hosts, fix vault-backed template names (#16426, #16427) #16442 is the model: keep the contract (Requires: admin permission.)
in the docstring, move the abuse narrative to a#comment. - The generated artifact needs fixing in the same commit. On security(secrets): admin-gate infrastructure hosts, fix vault-backed template names (#16426, #16427) #16442 and fix(control): emergency-stop reports what it actually paused instead of a fixed success string #16854 the prose was
already inapi.ts, so it could not be a revert of a bot commit — it had to be hand-corrected to
match the new docstrings.
Refs #16843 (the live gap being described), #16811, #16442, #16854.
- Security PRs are the high-risk case, because a reviewer reasonably wants the reasoning near the
- added a commit that references this issue
on Sep 17, 2026
A route docstring is a published artifact, and nothing tells the author that
FastAPI publishes a route handler's docstring as the OpenAPI operation
description. That schema isthen consumed by
openapi-typescriptand committed toautobot-frontend/src/types/generated/api.tsby the generated-types bot, and served by anything that reads the schema.
So prose written for a reviewer — rationale, defect history, the reasoning behind an access-control
decision — leaves the backend the moment it is written in the wrong place, and the author gets no
signal at all.
The instance
While reviewing #16580, the explanation of the presence authorisation defect was written into the route
docstring: "any signed-in user could list the online users of any session id", with a sibling-by-sibling
breakdown of which routes checked what. The generated-types bot then committed that prose into
api.ts, which is the only reason it was noticed. Fixed in695373b61— the rationale moved to a codecomment beside the handler, the docstring kept what the route does and requires, and the bot's
api.tshunk was reverted in the same commit so the generated file matches the corrected schema.
That is wrong twice over: it is not what a consumer of the route needs, and it publishes the shape and
history of an access-control defect into an artifact whose audience the backend does not control.
The class, measured
This is not one file. On
main:Sampled content includes operational prose such as
This operation is irreversible. Deleted logs cannot be recovered.— legitimate, and exactly the shape that an incautious security rationale would take.The pipeline is broad and works as designed; what is missing is any guard on what enters it.
Why it is not self-correcting
review pass. The feedback loop is long and indirect — here it only closed because the bot happened to
commit where the author was already looking.
rationale, because a reviewer reasonably wants that reasoning near the code.
Not established
Whether the OpenAPI schema is served to unauthenticated callers (e.g. a reachable
/openapi.jsonor/docs), and what the bundled frontend exposes ofapi.tsto a browser. That determines whether thisis internal-only leakage or public disclosure, and it should be answered before severity is set rather
than assumed either way.
Suggested guard
A pre-commit or CI check over route handlers that flags a docstring containing defect-disclosure
markers — issue references, "vulnerability", "any signed-in user could", "was not checked", CVE-style
phrasing — and directs the author to a code comment instead. Narrow and keyword-based is fine; the goal
is to interrupt the author at write time, which is the only point where the cost is zero.
Pairing that with a short rule in the review docs — rationale goes in a comment, contract goes in the
docstring — makes the distinction teachable rather than tribal.
Found while reviewing #16580 / #16811. Refs #16811.