Repository navigation
fix(analytics): _package_router_files maps nested subpackages to the parent prefix, inventing endpoints #12956
Description
Activity
- added a commit that references this issue
on Jul 29, 2026 Closed by PR #12970, squash-merged as
a18596379. Both findings addressed; thanks for filing rather than letting them drop.1. Nested subpackages —
_package_router_filesnow walks one level and recurses instead ofrglob, so a nested router subpackage resolves under its own prefix. Confirmed this was latent, not live:llc/apinests no router subpackages today, so nothing was actually mis-mapped on the current tree — which is exactly why it needed the regression test, since there was no live signal to catch it.2. Docstring vs check — tightened the code rather than softening the claim, since the claim was the correct behaviour. A submodule is now included only when its router is imported under an alias and that alias is passed to
include_router.Measured on indexed source
c5939a51-…:#12950 #12970 router files outside api/28 36 endpoints found 2157 2220 real routes matched 1916 1960 (+44) scanned but not real 38 38 missing findings 290 244 Tightening gained 44 real routes rather than losing any — binding alias→module finds submodules the old router-declaration filter missed — and the spurious count did not move, so nothing was invented.
One thing worth recording from the fix: three of my #12950 fixtures failed under the tightened check because they mounted
costs_routerwithout importing it — a shape no real registry produces (llc/api/__init__.pycarries 30from .x import router as x_routerlines). I corrected the fixtures rather than loosening the check. A test that only passes against impossible input was giving false assurance about precisely the behaviour under review.Also acted on the process gap this exposed: I swept all 16 other PRs merged today for unaddressed non-bot comments — #12950 was the only one; everything else was bot output. My pre-merge sweep now checks for non-bot comments alongside check status, which would have caught this before it reached
Dev_new_gui.The
..-traversal note is recorded as not-a-finding, as you framed it.
Context
Found while dogfooding the new
secreviewskill (#12951) against PR #12950.Both findings were reported on that PR
(#12950 (comment)) but it
merged as
d48146e11without them addressed, so they are now live onDev_new_gui. Filing so they are not dropped.Both are in
_package_router_filesinautobot-backend/api/codebase_analytics/api_endpoint_scanner.py, and both arethe same defect class #12945 set out to fix: a submodule mapped to a prefix it
is not actually served under, which produces phantom endpoints that later
resurface as false "orphaned" findings.
1.
rglobdescends into nested subpackages, but only the top-level package prefix is appliedrglobwalks the entire tree while the__-prefix filter removes the nested__init__.pythat carries that subpackage's own prefix. So a nested routersubpackage's modules are mapped to the parent package's
served_prefix, andthe only file that could correct them is excluded:
sub/b.pyis emitted with the parent's prefix. If a registry-mounted packageever nests a router subpackage, this invents endpoints under the wrong path.
Fix: use
glob("*.py")for the flat case, or recurse through_package_router_filesper nested__init__.pyso every level contributes itsown prefix.
2. Docstring overclaims the
include_routercheckThe docstring states:
The code only checks that the package mounts something:
and then includes every submodule declaring an
APIRouter(prefix=...),mounted or not. A module that declares a router the package never mounts still
contributes routes — the same phantom-endpoint problem, narrower in scope.
Fix: match the mounted router names, or soften the docstring to what the
code actually guarantees. Code and comment disagreeing is its own hazard here,
since the docstring is the stated rationale for the design.
Not a finding
joinpath(*module_path.split("."))would let a..segment escapebackend_dir, but the router registry is code-controlled rather than userinput. Recorded only so it is not re-raised as a security issue later.
Acceptance criteria
include_routerbehaviour agree — either the check is tightened or the claim is narrowed