Repository navigation
tech-debt(transcriber): allowed-audio-extension set duplicated across three modules, one of them a security boundary #13512
Description
Discovered during the code-graph extension audit for #13505. Different scope — filed
separately rather than dropped (CLAUDE.md Rule 6).
Summary
The allowed-audio-extension set is declared three times, byte-identical, with no shared source:
$ grep -rn "ALLOWED_EXTENSIONS = {\".wav\"" --include=*.py autobot-backend/
autobot-backend/transcriber/upload_security.py:20:ALLOWED_EXTENSIONS = {".wav", ".mp3", ".mp4", ".m4a", ".ogg", ".flac", ".webm"}
autobot-backend/transcriber/routes/recordings.py:39:_ALLOWED_EXTENSIONS = {".wav", ".mp3", ".mp4", ".m4a", ".ogg", ".flac", ".webm"}
autobot-backend/media/audio/ffmpeg_service.py:24:ALLOWED_EXTENSIONS = {".wav", ".mp3", ".mp4", ".m4a", ".ogg", ".flac", ".webm"}
Why it matters
One of these is an upload security boundary (upload_security.py). The other two are a
route guard and a processing guard. Three copies of a security-relevant allowlist drift
independently: adding a format to the route guard without the security module admits a file the
validator was never updated to accept, and removing one from the security module without the
others leaves two paths still advertising it.
Nothing today keeps them in sync — no shared constant, no test asserting equality.
Proposed fix
Promote one canonical set (autobot_shared/ or the existing utils/file_categorization.py
registry, which already holds every other extension set in the platform) and have all three
sites import it. Nothing is deleted; the two duplicates become imports.
Whether the three should be identical is worth one moment's thought during the fix — a
processing pipeline may legitimately accept a format the upload boundary rejects. If they
should differ, they should differ explicitly, as named subsets of one canonical set, not by
accident of three literals that happen to match today.
Acceptance criteria
- One canonical audio-extension set; all three sites import it
- Any intentional difference between the three is expressed as a named subset, not a
separate literal - A test fails if a fourth literal copy is introduced
Activity
fix(media): one canonical audio-extension allowlist for upload, route…
github-actions commentedon Aug 5, 2026
PR #13616 (merged to
Dev_new_gui) references this issue with a close keyword.If this issue is fully resolved, close it manually. If work remains, no action is needed.
mrveiss commentedon Aug 5, 2026
Landed as
92be57a44(PR #13616). Three copies of the audio-extension allowlist — including the upload security boundary — now bind oneSecurityConstants.ALLOWED_AUDIO_EXTENSIONS, with a guard test proving it catches a fourth (planted one; it named the file).The count in this issue was low: there are five sets, and two have already drifted — filed as #13615.
api/knowledge.pyaccepts.mkvthe upload boundary rejects;audio_connector.pyomits.mp4/.webmeverything else accepts. Left alone deliberately: reconciling either widens a security boundary or changes ingestion scope, which are decisions, not refactors. The guard is scoped to sets equal to the canonical one so it does not force that call by failing.mrveiss commentedon Aug 10, 2026
Referenced as the worked example on umbrella #13916 (canonical-source duplication).
This issue is the shape the other four children should take, for three reasons beyond the consolidation itself:
repo_tests/audio_extension_allowlist_test.py) that fails when a fourth literal copy appears — not just a migration. refactor(types): consolidate 15+ Severity/RiskLevel/Level enums onto constants/status_enums.py:Severity #6689 consolidated 15+ severity enums without one and 15 exist again today.api/knowledge.pyaccepting.mkv,audio_connector.pyomitting.mp4/.webm) with the reasoning that reconciling them changes what the upload boundary admits — a decision, not a refactor.No action needed here; recorded so the pattern is findable from the example rather than only from the umbrella.
mrveiss commentedon Oct 2, 2026
Two more instances of this issue's shape, found during a duplicate-definition sweep. Posting here rather than filing, because this issue is the worked example of exactly this pattern — a copy written in good faith, nothing comparing it to the original, and the divergence surfacing later as a defect in an unrelated place.
1.
is_safe_file— the copy lacks a security fix the original received.autobot-backend/api/files.py:292rejects/and\, and checksPath(filename).name != filename(security(files): upload endpoint validates the directory but joins the client filename unchecked — write can leave the sandbox #13394, ci: Python unit-test suite is not a required check — red tests merged to Dev_new_gui twice #13162).autobot-backend/api/conversation_files.py:261is its own copy, with its ownALLOWED_EXTENSIONSand_DANGEROUS_FILENAME_CHARS, and without that path check.Practical impact is limited rather than absent: the downstream
conversation_file_manager.py:629appliesPath(...).nameitself, so a traversal attempt is flattened one layer later. The defect is that the boundary check is absent at the boundary, and the second copy's allowlist can drift from the first independently.2.
RBACMiddleware— byte-identical in both backends, and the code says so.autobot-backend/user_management/middleware/rbac_middleware.pyandautobot-slm-backend/user_management/middleware/rbac_middleware.pyare the same 225-line class plus eight helpers, differing only in a comment block (#14750 vs #14654). The comment atautobot-backend/.../rbac_middleware.py:436admits it is byte-identical to the SLM's.The notable part is what is already shared: the models,
OrganizationService,TeamServiceandBaseServicewere all moved intoautobot_shared/user_managementby the #12647 de-fork. The middleware was left behind, so an authorization component is the one piece of that subsystem with two copies and no equality check. Nothing enforces that they stay identical, and the only reason they currently are is that nobody has edited one.Both are a lower severity than this issue's own audio-extension case — neither pair is known to differ in behaviour today beyond the missing path check. They are recorded here so the class has its instances listed in one place rather than being rediscovered.