chore: clear the audit backlog by k0te1ch · Pull Request #42 · k0te1ch/PodBoxBot

k0te1ch · 2026-09-10T15:19:21Z

Closes out the tech-debt list from the 2026-08-15 audit, plus the findings that
turned up while working through it.

aislop scan goes from 12/100 to 70/100: 9 errors to 0, 32 warnings to 5. The
five that remain are false positives or deliberate decisions, listed at the
bottom. Suites: bot 155, FTP 12, WordPress 20, Boosty 27.

Security and supply chain

Every high-severity advisory that reaches runtime: aiohttp to 3.14.3,
cryptography to 50.0.1 (bot and all three publishers), asyncssh to 2.24.0,
pyasn1 to 0.6.4, pytest to 9. All the constraints already allowed the
patched releases, so these are re-locks rather than range changes.

Two more high advisories (poetry 1.8.5, dulwich 0.21.7) came in through
poetry-plugin-up in the bot's dev group. A poetry plugin belongs in poetry's
own environment, and as a project dependency it was also the documented cause
of CI having to install --only main,testing. It is installed with
poetry self add from the Makefile now.

The Boosty and WordPress images ran poetry lock during the build and never
copied a lock in, so the versions in those two production containers could not
be read off the repository — and Boosty had no lock at all. Both install from
the committed lock now, like the bot and FTP images already did. Poetry is
pinned to 2.3.4 everywhere: an unpinned pip install --upgrade poetry is what
produced the flaky "Cannot install dialog-engine" / "Cannot install pyaes"
failures, where poetry downgraded virtualenv mid-install and broke its own
files.

pre-commit was in the bot's runtime dependencies and shipping into the
production image; it moves to the dev group, where the Makefile always assumed
it was.

The WordPress publisher's cookie jar was written with pickle and read back
with pickle.load. It is its own file under secrets/, so this was never a
live exploit, but nothing about storing name/value/domain/path needs that risk.
It is JSON now. A file left by the old version is detected and ignored, never
deserialized — the publisher logs in again and rewrites it, so no manual step
on the host; the one visible effect is an extra form login after deploy.

Bugs

The bot hardcoded FTP.cwd("postshow") while the FTP publisher uploads to
the configurable FTP_POSTSHOW_DIR. Changing that key pointed the publisher
at a new folder while the bot kept counting episodes in the old one. fix(ftp): upload postshow episodes into FTP_POSTSHOW_DIR #32
fixed the upload side; this is the read side.
split_into_messages promised in its docstring to break up oversized
content, but an item longer than the limit passed through whole — Telegram
answers "message is too long" to that.
The pinned-message handler deleted the "X закрепил сообщение" notice for
every pin, including ones made by people. It exists to tidy up after the
bot's own announcement pins, so it compares from_user against bot.id.
shutdown_bot() and restart_bot() are plain def, but the admin callbacks
awaited them. It only ever worked because neither returns. restart_bot also
called the bare exit(), which site installs for the interactive
interpreter.
makemigrations caught CommandError, printed it and returned normally, so
a failed revision left exit code 0.
COMMANDS was an empty list, so every start called set_my_commands([]) and
wiped the bot's command menu.

Dead code and drift

The whole app/bot/services/telethon_methods/ package is unreachable —
nothing imports it, and the only dynamic import in the bot resolves keyboard
modules by name. Removing it also settles the eight excepts that logged
without the caught error and the two except: pass blocks in html_parser.py
that were being kept because the parser came from Telethon upstream; neither
argument survives the code being unreachable.

Also gone: lxml/feedparser/fake-useragent from the bot (and feedparser
from the WordPress publisher, equally unused there), the four WP_* settings
no bot module has read since publishing moved out, seven comments that only
restated the line below them, two unused CHUNK_SIZE constants, and four
stale TODOs. app/ has no TODO left.

README documented a Patreon publisher in the service tree, the topic diagram,
the compose build command, the venv setup list and as a CI job. There is no
app/publishers/Patreon, no service, no schema and no job.

Refactors

Both files the audit flagged as too large are split along seams that were
already marked:

bot_methods.py  521 -> bot_methods.py    61  process control, log archive
                       messaging.py     236  send, broadcast, split, pin
                       release_notes.py 246  changelog parsing, version

wordpress.py    489 -> wordpress.py     297  form login, session, upload_post
                       wp_http.py       137  cookies, bot-protection, retries
                       podlove.py        94  Podlove REST

Pure moves. The WordPress split uses mixins rather than collaborator objects
so the methods still read the same self._session / self._rest_session and
no attribute plumbing changes.

Also

A .github/dependabot.yml, which the repository did not have — some open
alerts point at a root poetry.lock that was removed long ago and nothing
told Dependabot to stop looking. Minor and patch bumps are grouped, because
four services with nearly identical dependency lists otherwise produce four
PRs per aiohttp release.

The Makefile is rewritten against paths that exist: run, migrate and
makemigrations pointed at app/main.py, which never has. Every recipe was
run before being written down.

Left alone on purpose

Five aislop findings are false positives; no rule was disabled for them.
router.py:25 uses logger.exception, so the traceback is there.
FTP_methods.py:68 catches asyncio.wait_for's TimeoutError, which carries
no message — {e!r} would add TimeoutError() instead of information.
migrate_topics.py:30 logs with {e} and re-raises. main.py:103 is a
side-effect import for handler registration, with a # noqa: F401 and a
comment. boosty_client.py:50 is the fixed host of Boosty's reversed internal
API, the pair of the API_URL the library already hardcodes — now documented
in place rather than moved to config.

Two TODOs stayed because they describe real unfinished work, both now tracked:
audio_handler's hardcoded notification flag became FORWARD_PIN_SILENT, and
confluent-kafka's deprecated AvroConsumer is its own migration.

Worth knowing before merge

The first rebuild of the Boosty and WordPress images will install from the
committed locks instead of resolving fresh, so their versions may shift
relative to what is running now. #41 overlaps with the bumps here and has
fallen behind main; #30 will rebuild itself against these commits.

The bot stopped needing all three when the legacy app/bot/utils/wordpress.py went away; nothing under app/bot imports them. feedparser was equally dead in the WordPress publisher — it only ever pulled in sgmllib3k. lxml and fake-useragent stay there, wordpress.py uses both. Locks rebuilt: removals only, no version churn.

WP_URL/WP_LOGIN/WP_PASSWORD/WP_APP_PASSWORD and the strip_wp_url validator were left over from the legacy in-bot publisher. Publishing moved to app/publishers/WordPress, which reads its own credentials from shared config, and no module under app/bot has imported these since. The .env keys stay — the publisher still needs them. The test env no longer has to fake them either: they were only there to satisfy required fields on the bot's Settings.

Nothing imports the package: not services/__init__.py, not the handlers, not the tests, and the only dynamic import in the bot (services/keyboards.py) resolves keyboard modules by name. Last touched by a repo-wide formatting pass. Removing it settles two audit items at once — the eight excepts in telethon_methods.py that logged without the caught error, and the two 'except ... : pass' blocks in html_parser.py that were being kept only because the parser was ported from Telethon upstream. Neither argument survives the code being unreachable. telethon itself stays: services/__init__.py and cli.py still build a TelegramClient for the anon session.

The command caught CommandError, printed it and returned normally, so a failed revision left exit code 0 — anything scripting the CLI read the failure as success. Now it raises ClickException, which prints to stderr and exits 1, with the 'run migrate first' hint folded into the message. The two informational print() calls become click.echo(), which is what the rest of a click entry point should use.

…rsion Seven comments that only repeated the line under them ('# Define the path for the ZIP archive' above log_zip = ...). The banner comments in main.py went the same way. get_version() walked pyproject.toml with .get('tool', {}).get('poetry', {}), so a malformed file and a version-less one both returned None with nothing in the log. Indexing plus an explicit KeyError says which happened.

shutdown_bot() and restart_bot() are plain def, but the admin callbacks awaited them. It only ever worked because neither returns — one raises KeyboardInterrupt through os.kill(SIGINT), the other SystemExit — so the await was never reached. The first helper that returned normally would have raised 'object NoneType can't be awaited' at the admin panel. restart_bot() also called the bare exit(), which site installs for the interactive interpreter and is absent under python -S. sys.exit() is the same SystemExit without the dependency. Drops the '# TODO: Рестарт бота' note above them — restart_bot has existed for a while.

Closes the '# TODO: ПРОВЕРКА НА ТО, ЧТОБЫ ЗАКРЕПИЛ БОТ!' left on the handler. It deleted every 'X закрепил сообщение' service message in the chat, so a pin made by a person had its notice wiped by the bot too. The handler exists to tidy up after the bot's own announcement pins (utils.bot_methods.pin_message), so it now compares from_user against bot.id and leaves everything else. Also drops the commented-out DELETE_PINNED_SERVICE_MESSAGE import — the config key it speculates about has never existed.

split_into_messages promised in its docstring to break oversized content up, but a single item longer than max_length was passed through whole — Telegram answers 'message is too long' to that. Closes the '# TODO: if item_length > max_length' sitting on the function. The new helper cuts on character boundaries, so a multibyte item doesn't come back as mojibake. It does not understand markup: an HTML tag can land on a seam. That is acceptable here because the live path (send_release_note -> broadcast_message_to_users) hands over sections already cut to size and never reaches this branch — it is a guard, not the normal route. Tests also pin the existing quirk that a non-empty header gets no separator before the first item.

…sher Every high-severity advisory that actually reaches runtime: aiohttp 3.13.5 / 3.14.1 -> 3.14.3 (bot, all three publishers) cryptography 48.0.1 -> 50.0.1 (bot, all three publishers) asyncssh 2.23.0 -> 2.24.0 (FTP publisher) All four constraints already allowed the patched releases, so this is a re-lock rather than a version-range change. The Boosty publisher had no poetry.lock at all — its versions were whatever resolved at image build time, and Dependabot could only report against its pyproject.toml. It has a lock now, so its dependencies are pinned and tracked like the others. Test suites after the bumps: bot 153, FTP 12, WordPress 19, Boosty 27.

The Boosty and WordPress images ran 'poetry lock' during the build and never copied a lock in, so those two production images resolved their dependencies fresh on every build — two services whose deployed versions nobody could read off the repository. Both now copy the lock and install from it, like the bot and FTP images already did. CI loses the same two re-lock steps. Poetry is pinned to 2.3.4 — the version that produced the committed locks — in all four Dockerfiles and all four CI jobs. Unpinned 'pip install --upgrade poetry' is what produced the flaky 'Cannot install dialog-engine' / 'Cannot install pyaes' failures: poetry downgraded virtualenv mid-install and broke its own files. Dropped poetry-plugin-up from the bot's dev group. It pinned poetry 1.8.5, which dragged in dulwich 0.21.7 — both high-severity advisories, and the documented cause of CI having to install --only main,testing. A poetry plugin belongs in poetry's own environment, so the Makefile installs it with 'poetry self add'. pyasn1, the one runtime advisory left, goes to 0.6.4. pre-commit moves from the bot's runtime dependencies to the dev group, where the Makefile always assumed it was; it was being installed into the production image. Also drops 'readme = "README.md"' from the bot and FTP pyprojects — neither file exists, so 'poetry check' failed and a plain 'poetry install' errored out. All four services now pass 'poetry check --lock'.

The audit flagged UPLOAD_URL as a hardcoded environment URL. It isn't one: it's the fixed host of Boosty's reversed internal API, the pair of API_URL that the library already hardcodes. There is no deployment that would point it elsewhere, so a comment records the decision instead of moving it to config. Same for the imports at the bottom of tests/conftest.py — they sit below the sys.path and env bootstrap on purpose, which now says so in a comment.

README listed Patreon as a shipped publisher — in the service tree, in the Kafka topic diagram, in the compose build command, in the local-venv setup list and as a CI job. None of it exists: there is no app/publishers/Patreon, no publisher_patreon service, no Avro schema and no test-publisher-patreon job. Anyone following the README would have hit four dead ends. What is real is that BasePublisher has the paywall hooks a Patreon publisher would use, so that stays, described as groundwork rather than a service.

get_last_post_ID did FTP.cwd("postshow") to find the latest postshow episode and derive the next episode number for the template prompt. The FTP publisher uploads those same files to FTP_POSTSHOW_DIR, which is configurable — so changing that key pointed the publisher at a new folder while the bot kept counting in the old one and proposed a wrong episode number. PR #32 made the upload side honour the setting; this is the read side of it. Both now resolve the same .env key with the same default, so the default deployment behaves exactly as before. Closes the '# TODO: вынести в настройки' on that line.

Triage of what was left, rather than a blanket delete: main.py '# TODO CHECK THIS' — no content to act on; the branch it marks works and is covered by tests. scheduler.py '# TODO: Аннотации' — the annotations are there. scheduler.py '# TODO: Обработка ошибок' — init_scheduler_jobs registers no jobs, so there is nothing to guard. Replaced by a note saying the module is scaffolding gated behind ENABLE_APSCHEDULER. progress_callbacks.py '# TODO: Рестарт бота' — copy-pasted from bot_methods; restarting has nothing to do with progress bars. Its CHUNK_SIZE came along in the same paste and is unused — the class takes aiogram's DEFAULT_CHUNK_SIZE. bot_methods' copy is unused too. Two stay because they describe real unfinished work: cli.py's note about COMMANDS (still an empty list, so set_my_commands currently clears the menu) and audio_handler's hardcoded disable_notification. .env.example now documents FTP_POSTSHOW_DIR, which both the bot and the FTP publisher read.

bot_methods.py held 521 lines across three unrelated subjects: controlling the process, sending Telegram messages, and parsing CHANGELOG.md into a release note. Split along the '# region' boundaries that were already marking them: utils/bot_methods.py 62 lines shutdown/restart, log archive utils/messaging.py 237 lines send, broadcast, split to limit, pin utils/release_notes.py 246 lines changelog parsing, version, notify admins Pure moves — no behaviour change. The dependency runs one way: release_notes imports broadcast_message_to_users from messaging. Call sites updated rather than re-exported from bot_methods, so there is no compatibility shim to clean up later. Test files moved to directories matching their new modules.

_load_cookies called pickle.load on a file from disk. It is the publisher's own file under secrets/, so this was never a live exploit — but unpickling executes whatever the payload says, and nothing about the format requires that risk. Cookies are name/value/domain/path; JSON holds them fine and can be read by a human debugging a login problem. domain and path are written explicitly rather than flattened to a dict: the bot-protection cookie is set on its own domain, and losing that would break the replay logic in _request. A file left behind by the pickle version is detected and ignored, never deserialized — the publisher logs it and logs in again, which rewrites the file as JSON. So the upgrade needs no manual step on the host; the one visible effect is one extra form login after deploy. The default path follows the format: /app/data/cookie.pkl -> cookie.json. Tests now use a real RequestsCookieJar instead of asserting a pickled dict, cover the round trip including domain/path, and pin that a legacy pickle file is not unpickled.

wordpress.py was 489 lines doing four jobs at once. Two of them lift out cleanly, because they were already separate concerns: wordpress.py 297 lines form login, session lifecycle, upload_post wp_http.py 137 lines cookie jar, bot-protection replay, GET retries podlove.py 94 lines Podlove REST: episode metadata and chapters Mixins rather than collaborator objects, so this stays a pure move: the methods still read self._session / self._cookie_path / self._rest_session exactly as before, and no attribute plumbing changes. podlove.py does not need HTTP_TIMEOUT — _rest_request goes through the mixin's _request, which applies the default itself. One test patch target moved with the code it patches: _load_cookies now lives in wp_http, so the os.path.exists patch points there.

…e configurable COMMANDS was an empty list with /menu and /feedback commented out above it, so every start called set_my_commands([]) and wiped the bot's command menu. Neither commented-out command has a handler. /start does, and it is the one command any user needs, so that is what the menu now shows. /admin stays out of the list deliberately: its router is behind IsPrivate+IsAdmin, and there is no reason to advertise it to everyone. disable_notification for the announcement pin was a local variable set to False with a note to move it to settings — it is FORWARD_PIN_SILENT now, defaulting to the same False, so behaviour is unchanged until someone sets it. Both closed TODOs; app/ no longer has any.

It had drifted badly. run/migrate/makemigrations pointed at app/main.py, which has never existed — the entry point is app/bot/main.py and only works with app/bot as the working directory, because it imports its own modules bare. 'poetry install --with=dev,test' named a group called test; the group is testing. docker-compose was still the v1 command. The header said NOT WORKING IN *NIX without saying what about it wasn't. Every recipe here was run before being written down. Two things the targets have to work around, both now in a comment at the top: * 'poetry -C <dir> run' executes with cwd=<dir>, so relative tests/ paths break — hence cd plus $(CURDIR); * ruff only finds its config (line-length 119) from app/bot, since there is no root pyproject.toml. Run from the repo root it applies the default 88 and formats differently from CI. Adds test-publishers, mirroring CI's split: publisher tests need their own service deps, which the bot env does not have. poetry-plugin-up is installed with 'poetry self add' in update, matching its removal from the dev group.

Dependabot was running off repository settings with no config in the tree, which is why some open alerts point at a root poetry.lock that does not exist — it was removed long ago and nothing told Dependabot to stop looking there. The config lists the four poetry projects explicitly, plus the six Dockerfiles and the actions, and leaves the root out. Minor and patch bumps are grouped into one PR. Ungrouped, four services with nearly the same dependency list produce four separate PRs for every aiohttp release — which is roughly how #35 and #41 came to look the way they do. .gitattributes is deliberately narrow, with no blanket `* text=auto`, so it cannot trigger a repo-wide line-ending renormalization. The Makefile is the reason it exists at all: its recipes need real tabs and break if a CR ends up in them.

The last dependency advisory that was not already covered by the earlier re-locks. pytest 8 stays vulnerable; 9.0.3 is the fix. pytest-cov 5 caps pytest below 9, so it goes to 7 along with it. pytest-asyncio 1.x owns the event loop and refuses a custom event_loop fixture — tests/conftest.py had one, and pytest-asyncio 0.24 was already warning that it "will lead to errors in the future". Nothing requested the fixture by name, so removing it is all the migration needed; all 73 async tests already carry @pytest.mark.asyncio, and asyncio_default_fixture_loop_scope stays valid. The publisher CI jobs install their test tooling with pip rather than poetry, so their pins move too. Suites after the bump: bot 155, FTP 12, WordPress 20, Boosty 27. The bot suite went from 72 warnings to none — they were all this deprecation. The one warning left is confluent-kafka's AvroConsumer, which is a separate job.

k0te1ch added 21 commits September 1, 2026 23:14

Navigation Menu

chore: clear the audit backlog - #42

chore: clear the audit backlog#42
k0te1ch wants to merge 21 commits into
mainfrom
chore/tech-debt-backlog

k0te1ch commented Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

k0te1ch commented Sep 10, 2026

Security and supply chain

Bugs

Dead code and drift

Refactors

Also

Left alone on purpose

Worth knowing before merge

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant