security: remove bot token from webhook URL, fix no-op framework patching (v0.4.0) - #9
Closed
AhooraZen wants to merge 4 commits into
Closed
Conversation
…hing
Audit of every source file surfaced three classes of problem in a tool whose
whole purpose is working from a network-filtered region.
Security:
- `andro-cfw serverless` registered `/webhook?token=<BOT_TOKEN>` with Telegram.
That URL is stored in the bot's webhook config and replayed on every update.
The token is now an encrypted Worker secret and the webhook URL is bare.
- The webhook is authenticated with `setWebhook(secret_token=...)`, verified in
constant time against `X-Telegram-Bot-Api-Secret-Token`. Previously anyone who
learned the worker URL could inject fabricated updates.
- The worker no longer trusts a caller-supplied `?token=` query parameter.
- CORS `*` on both origin and headers made every deployment a general-purpose
CORS bypass for the Bot API. Now opt-in via ALLOWED_ORIGINS.
- Node.js install no longer pipes a downloaded script into `sudo bash` by
default (ANDRO_CFW_ALLOW_NODESOURCE opts in; ANDRO_CFW_NO_AUTO_INSTALL opts
out of auto-install entirely).
- Token prompt uses getpass; $TELEGRAM_BOT_TOKEN preferred over argv.
- Key dir and per-account wrangler OAuth dirs are 0700; account labels are
validated so they cannot escape ~/.andro_cfw/accounts; URL schemes pinned.
Correctness:
- `andro_cfw.patch()` was a no-op for aiogram, python-telegram-bot, pyrogram and
hydrogram. A bot called it, saw no error, and talked straight to
api.telegram.org. aiogram and PTB are now patched for real; the MTProto
clients are removed with an explanatory warning, since an HTTP Bot API proxy
cannot route them.
- setWebhook success was inferred from HTTP 200; the Bot API returns
{"ok": false} with status 200 for most rejections.
- Session writes are atomic, so a load-balancer thread can no longer leave an
undecryptable cfw.session behind.
- The all-workers-down 503 is framed; unframed HTTP/1.1 hung the client.
- Hop-by-hop headers are not relayed, request bodies are capped, stop() closes
the listening socket, and persistence moved out of the balancer lock.
- The shipped worker template no longer contains a hardcoded demo bot.
- Version is single-sourced from package metadata (the v0.3.2 tag shipped code
reporting 0.3.0).
Testing: 76 -> 121 tests. The four patch() tests asserted against MagicMock,
which auto-creates any attribute touched, so they passed regardless of behavior.
CI now gates on ruff and mypy and runs on macOS and Windows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI sets ANDRO_CFW_NO_AUTO_INSTALL=1 so the suite can never trigger a system package install. That short-circuits check_node_toolchain() before the auto-install branch, so test_check_node_toolchain failed on every CI runner while passing locally, where the variable is unset. Clear it in an autouse fixture so these tests exercise the path they name. The test that covers the opt-out sets the variable explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Windows has no POSIX permission bits -- os.chmod only toggles the read-only flag, so S_IMODE reports 0o777 / 0o666 regardless. The production code already treats the chmod as best-effort; only the assertions were platform-specific. Surfaced by the newly added Windows CI job, which this repo never ran before. On Windows, access to these paths is governed by the ACL %USERPROFILE% carries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the two findings the previous plan set deferred, and drops plans/. PERF-04 -- the load balancer buffered whole request and response bodies in memory. A 50 MB upload plus a few concurrent transfers pinned that much RAM per request thread. Responses are now relayed in 64 KB chunks; only the first 512 bytes are held, which is all the Cloudflare quota-page sniffing needs. Request bodies spool: in memory up to 1 MB, on disk above it. Spooling rather than streaming off the client socket is deliberate -- failover replays the same body against the next worker, so it has to remain seekable. Two framing cases fell out of streaming and are handled explicitly: urllib decodes chunked upstream responses, leaving no Content-Length, so those are delimited with `Connection: close`; and HEAD sends headers with no body. `_forward` became `_open_upstream`, returning the response unread so the caller can stream it and owning-close it. Its tests were rewritten accordingly, plus new coverage for body replay on failover, spill-to-disk, chunked relay, HEAD, and a client that disconnects mid-download. DIR-04 -- the worker proxied to a hardcoded api.telegram.org. UPSTREAM_API_ORIGIN now points it at a self-hosted telegram-bot-api instance, which lifts Telegram's 50 MB upload cap. Only the origin is used, and a malformed or non-http(s) value falls back to Telegram rather than sending traffic somewhere unintended. plans/ is removed: every plan in it is now implemented. 133 tests pass; ruff and mypy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.