feat: Telegram channel adapter, test button, and setup guide

[rjonesbsink]

Summary

Adds the Telegram channel adapter — the backend behind a Settings panel that has been shipping a complete configuration UI, including a "Send Test" button, with nothing behind it.

Scope reduced per @ejosterberg's review. The proc_open conversion, the wmic → Get-CimInstance fallback, the MySQL 8.0 TEXT DEFAULT fix and the telegram_bot_token blanking are all in the dev tree already (8a9ec2a and others), so this branch is now only the part with nothing behind it, rebased onto current main. Eleven files became five.

What was broken

• No inc/channels/telegram.php, so broker_send('telegram', …) was rejected as an unregistered channel — silently, for routed incident/PAR/system alerts.
• Nothing bound to #btnTestTelegram, so the shipped "Send Test" button did nothing at all.

Changes

• inc/channels/telegram.php — Bot API sendMessage over cURL, registered via broker_register() and picked up by inc/broker.php's glob.
• assets/js/config.js — the missing #btnTestTelegram handler, mirroring #btnTestSlack. Deliberately does not touch loadTelegramConfig()'s save path: the data-secret blanking is fixed generally in the dev tree and changing it here would collide.
• docs/TELEGRAM-SETUP-GUIDE.md — modelled on ZELLO-SETUP-GUIDE.md.
• Channel-table rows in MESSAGE-ROUTING-GUIDE.md and ROUTING-ENGINE-REFERENCE.md.

The four review items

1 — Destination pinned to configuration. The $message['telegram_chat_id'] override is gone; the chat id is read from config only.

Verified rather than asserted: with an invalid configured chat id and a valid override supplied, the send fails closed on validation and transmits nothing. If the override were still honoured it would have proceeded.

with invalid config + valid override:
  {"success":false,"error":"Telegram chat ID is malformed (…)"}
  => PINNED — override ignored, failed closed, nothing sent

The reasoning is recorded in a comment at the call site, including why it was unreachable (no provider currently permits an arbitrary top-level key) and why that is not a guarantee this codebase owns.

2 — cURL options stated explicitly. VERIFYPEER, VERIFYHOST 2, no redirects, HTTPS-only for both protocols and redirects, CONNECTTIMEOUT 5. Matches inc/webhooks.php and api/dmr-lookup.php. Existing TIMEOUT 10 kept, with the synchronous-inside-broker_send() consequence noted in a comment.

3 — Format validation, failing closed. Token against /^\d+:[A-Za-z0-9_-]{20,}$/, chat id against /^-?\d{1,20}$/, both with actionable messages instead of an opaque Telegram 404. _telegram_status() applies the same checks, so malformed credentials report not_configured rather than a "configured" state that cannot send.

4 — Setup guide. Records both non-guessable facts: the chat id must be the group's negative id (a positive DM id fails with Forbidden: bot can't initiate conversation with a user, which reads as a permissions problem rather than a wrong id), and the bot must already be a member of the group before getUpdates shows the chat.

Testing

Verified against a live bot and group on Windows/IIS, PHP 8.4.22, MySQL 8.0:

• Adapter registers through the broker glob; _telegram_status() reports configured.
• Send Test delivers to the group.
• Malformed token and malformed chat id are both rejected before any request is made.
• The pinning test above.

[rjonesbsink]

Added a second commit here per @ejosterberg's note on #7: telegram_bot_token had the identical silent-blanking bug (matches the _token$ suffix in is_secret_setting_key(), so the server always masked it — loadTelegramConfig() just never accounted for that).

Rather than bolt on a one-off fix, brought Telegram's two fields onto the same data-key/data-secret + applySettingsToForm()/collectSettingsFromForm() pattern the rest of Settings uses, plus the manual "stored — leave blank to keep" placeholder handling feed_api_key needed (since applySettingsToForm() doesn't render that from the _set sentinel on its own — the gap you mentioned filing separately).

Verified against tests/test_settings_secret_fields.php pulled from main: telegram_bot_token no longer appears in the failure list. The three failures the test still reports on this branch (the other eight fields + two boolean toggles) are already fixed independently by 07b9d1d and aren't part of this PR's scope — they'll merge in on their own.

[ejosterberg]

Thank you for this — and for the four issue reports that came with it. Root-causing @exec() being fatal under disable_functions (rather than just "status page is broken") is the kind of report that saves a maintainer days, and you did it four times in about forty-eight hours. It's appreciated.

I reviewed this with a security focus and test-merged it against main. Summary: the security posture is clearly better with this change, and I want to take it. There's one correctness issue I need resolved first, because it lands on exactly the environment this PR targets.

What I checked, and what's good

• Every proc_open argv is a static literal — no user-controlled value reaches any command. Moving from a shell string to an argv array eliminates the metacharacter surface, so the absence of escapeshellarg is correct rather than an omission.
• function_exists('proc_open') is a valid guard — disable_functions does remove entries from the function table.
• api/health.php keeps its auth.php + is_admin() gate; chat.php's test_channel keeps admin + CSRF and builds the broker message from fixed keys, so a caller can't inject telegram_chat_id.
• No secrets added, no new dependency, no json_error_safe() regression.
• Merges with zero conflicts; test_settings_secret_fields.php passes 14/14 on the merged tree; the API-contract audit shows no new findings; all eight files lint clean.
• The wmic → PowerShell fallback genuinely works — the existing parser loops each line against /^(\d{14})/, so the single-line PowerShell output matches.
• You were right to drop that stale "array form isn't universally available" comment; composer.json requires PHP >= 8.0.

Also worth noting because it isn't in the PR body: the settings.php submit-handler change fixes a latent bug where an always-empty telegram_bot_token was posted and wiped the stored value on every save — the same class as #7. Good catch, whether or not it was deliberate.

The one blocker: pipe deadlock

In sql/run_migrations.php (~314-321), tools/install_fresh.php (~410-417) and tools/check-schema.php (~346-354), the code drains stdout to EOF and then reads stderr. If the child fills the stderr pipe buffer first, both sides block and neither can proceed — and there's no timeout.

Measured on Windows: 4 KB of stderr passes, 8 KB hangs indefinitely.

This is likely precisely where the PR is aimed. A hardened host running php.ini-production has log_errors on, so PHP 8.4 deprecation notices from migration scripts go to stderr — and a migration run emits plenty. The old exec(… 2>&1) merged the streams and could not deadlock, so this is a regression rather than a pre-existing risk.

The smallest fix, verified clean up to 1 MB, also matches what the surrounding code already does with $stdout . $stderr:

2 => ['redirect', 1]   // instead of ['pipe', 'w'] — then drop the stderr read

Worth applying the same treatment in api/health.php, inc/tts/engine.php and proxy/ZelloProxyApp.php, which currently fclose() stderr unread. Low risk there given the tiny outputs, but it's the same shape.

Two minor things, not blockers

• inc/channels/telegram.php:186 — on an API failure with no description, the raw response body is returned to the admin UI. Worth trimming to a fixed message.
• inc/channels/telegram.php:199-204 — the silent catch { }. It's a faithful clone of slack.php, so I won't hold the PR for it, but our conventions ask for at least an error_log().

Happy for you to push the pipe fix to this branch, or say the word and I'll apply it on merge and credit you. Either way this is going in.

One question: was the Telegram adapter something you needed, or added for completeness? Asking because it's a new user-facing channel and I'd want a line in the setup docs before it ships.

[rjonesbsink]

Thanks for the thorough review — especially for measuring the deadlock threshold rather than just flagging the shape. You're right on every point, and I've pushed all of it.

The blocker: fixed, and I reproduced your numbers exactly

Your diagnosis was correct and it was a regression I introduced. On this box (Windows/IIS, PHP 8.4.22, disable_functions = shell_exec, exec, system, passthru, popen):

stderr	before	after
4 KB	passes	passes
8 KB	hangs indefinitely	passes
64 KB	—	passes
1 MB	—	passes

I also ran it end-to-end through the real runner rather than only a synthetic harness: dropped in a temporary migration that writes 74 KB to stderr, then ran php sql/run_migrations.php both ways. Pre-fix it timed out. Post-fix it applied in 122 ms with the stderr content still present in the log tail — worth confirming, since the point of the merge is not losing diagnostics.

['redirect', 1] applied to the three deadlock sites. As you noted, those all did $stdout . $stderr downstream anyway, so the merge is what the code already wanted.

For the three lower-risk ones I went with the null device rather than a redirect — api/health.php, inc/tts/engine.php, proxy/ZelloProxyApp.php all replaced commands that ended in 2>/dev/null, so discarding stderr preserves the original intent instead of mixing it into a value the caller parses (tts_bin_on_path and binOnPath test whether stdout is non-empty, so stderr landing there could produce a false positive). Verified after: checkOs() returns real uptime, tts_bin_on_path('php') is true, and a nonexistent binary is false.

While in there: the three remaining 2 => ['pipe','w'] sites are already safe

You mentioned the same shape being worth checking. I looked at the pre-existing call sites this PR doesn't touch, and all three are protected — by different means, so it's not luck:

• install_fresh.php's Unix mariadb branch — the shell command already ends in 2>&1, so that descriptor never receives data.
• tts_run_pipe() — non-blocking reads plus a 30 s timeout with proc_terminate.
• ZelloProxyApp::runPipe() — non-blocking plus a deadline and proc_terminate($proc, 9).

Bounded rather than unbounded in each case. Mine was the only unguarded one. Left them alone.

Both minor items done

• telegram.php no longer returns the raw response body when Telegram omits description — fixed message to the UI, body to error_log() (truncated to 500 chars) so it stays diagnosable.
• The silent catch in _telegram_get_config() now logs. That one was worth more than tidiness: a settings-table failure was previously indistinguishable from a blank field, so a real DB problem surfaced as "Telegram not configured".

Your question: needed, not for completeness

It's in real use here. I have a bot posting to an actual group — token and chat ID configured, test message confirmed landing. Two things I hit that might be worth a line in whatever docs you add, since neither is guessable:

1. The chat ID must be the group's, and it's negative. getUpdates will happily hand you the private chat ID from your own DM with the bot (positive), and using it fails with Forbidden: bot can't initiate conversation with a user — which reads like a permissions problem, not a wrong-ID problem. Cost me a while.
2. The bot has to be a member of the group before getUpdates shows that chat at all.

I added the missing telegram row to the channel tables in docs/MESSAGE-ROUTING-GUIDE.md and docs/ROUTING-ENGINE-REFERENCE.md alongside the other production channels. Happy to write a fuller setup section modelled on ZELLO-SETUP-GUIDE.md if you'd rather it live there — say the word and I'll add it to this PR.

One unrelated observation

tests/test_health_check.php reports 3 failures on this machine — inc/health-check.php lints, tools/check-health.php lints, and CLI runs and exits 0 or 1 (got 255). Not from this PR: neither file is touched by it, and both lint clean under php -l directly. The cause is that the test harness itself calls exec() at lines 46, 228 and 248, so it hits the exact bug this PR fixes — 255 being the fatal-error exit. Pre-existing and environment-specific, so out of scope here, but it does mean the suite can't fully self-check on a hardened host. Happy to file it separately.

Also: #11 is a one-line feof()-after-fclose() fatal in migrations.php that crashes the script-preview rows — found while chasing a resurfaced instance of this same disable_functions issue.

[rjonesbsink]

Filed the test-suite observation as #13.

One correction while tracing it for the writeup: I said above that the got 255 was the fatal-error exit code. It isn't — $cliRc is pre-initialised to 255 before the try, and the catch (Throwable $e) swallows the Error: Call to undefined function exec(), so nothing ever ran and the sentinel survives. Same conclusion (pre-existing, exec()-related, not from this PR), but the catch blocks are the active ingredient rather than a crashing child — which is the part that matters for fixing it, so worth stating correctly.

[ejosterberg]

Review outcome: nothing dangerous, four changes wanted before merge. The merge decision is Eric's.

Full adversarial pre-merge security review done against head 84d7b4a, looking specifically for command injection in the proc_open conversion and for anything the Telegram adapter could be made to do that it should not. Summary first, then the four items, then something about the proc_open half you will want to know before doing more work on it.

The escapeshellarg() removals are correct and should stay removed. Worth saying plainly because they look alarming in a diff. escapeshellarg() quotes for a shell, and after this change there is no shell — array-form proc_open() goes straight to execvp/CreateProcess, so a ;, |, $(…) or a backtick inside an argument is inert data rather than escaped syntax. Re-adding it would be an active bug: the child would receive literal quote characters. Every argv element at all eight converted sites traced to a literal, a PHP constant, or a filesystem-derived path. The only non-literal is $bin in tts_bin_on_path() / binOnPath(), which is admin-configured, is rejected if it contains a path separator, and is passed as a single argv element to which/where. It cannot become a command.

Your second round of fixes landed properly. The pipe-deadlock fix is right, including the judgement inside it: ['redirect', 1] for the three sites whose callers merged the streams anyway, and the null device for the three that discard stderr. You were right that a pipe there would have let stderr contaminate tts_bin_on_path()'s non-empty-stdout test and produce a false positive. The error_log redaction is right too — the response body is logged, and the token lives in the URL and is never logged.

No SSRF in the Telegram adapter. The host is a hard-coded literal and the only interpolated component is the token, which lands in the path, after the authority — a @, .., ? or # in it cannot re-point the request at another host, and redirects are not followed. Correct settings store, too: _telegram_get_config() reads the settings table, which is what get_variable() reads and what the Settings UI writes. The config table / get_setting() trap does not apply here — I checked, because that one is easy to get wrong and silent when you do.

---

The four changes

1. Pin telegram_chat_id to configuration — the one that matters

inc/channels/telegram.php:21:

$chatId = $message['telegram_chat_id'] ?? $config['telegram_chat_id'] ?? '';

should be

$chatId = $config['telegram_chat_id'] ?? '';

No path can reach this today. I traced every one and none of them can — every broker_send() call site builds its message from a fixed, hard-coded key list. But there is exactly one path that forwards a message array wholesale: the routing engine's forward in inc/router.php, where _router_transform() rewrites only body, priority and type, so every other key survives verbatim into the destination adapter. And two receive handlers return raw third-party JSON into that path — _slack_receive() returns $data['messages'], _sms_receive() returns $data['threads']. Neither Slack's conversations.history object nor Pushbullet's thread object lets a message author add an arbitrary top-level key, so the override is unreachable.

That safety is a property of a third party's response schema, not of anything this project controls or gets told about when it changes. If a top-level telegram_chat_id ever became settable — a provider schema change, a new ingest endpoint that decodes a request body into a message array, another adapter returning raw JSON the way those two do — every routed message goes to a chat of the attacker's choosing: incident type, dispatch address, patient counts, responder identity and last-known location. Silent, and the routing log records forwarded and success.

Flagging a latent issue as required-before-merge because this project's own history is the argument. assigns.rec_facility_id was a column "nothing writes", which turned out to be a lost mass-casualty capability. un_status.extra_data_target was an ENUM widened for a value nothing ever set. Both were in exactly this state, and both cost rounds of "still not working" before anyone looked.

The Slack adapter you cloned this from has now been pinned the same way (d8e24ae in the dev tree), with tests/test_channel_destination_pinning.php stating the rule once for all ten adapters — and it checks inc/channels/telegram.php conditionally, so it is already waiting for your file rather than needing to be remembered. Fixing only Telegram would have left the pattern for whoever writes the next adapter; that one is on us, not on you.

A per-message destination may well be a good feature later — routing weather to one channel and dispatch to another is a fair thing to want. But it needs an admin-configured allowlist plus the router's _is_routed_forward trust marker, not an unchecked key. The chat id is bound to the bot credential; it is not a per-message recipient like to.

2. State the cURL security options explicitly

The defaults are safe — VERIFYPEER true, VERIFYHOST 2, no redirect following — so this is hardening, not a bug. But every other outbound caller in the codebase states them (inc/webhooks.php:231, api/dmr-lookup.php:149-150, tools/aprs-poller.php:157), which means a reader cannot tell "safe by default" from "nobody checked", and a host with an unusual curl.* ini changes the answer.

curl_setopt_array($ch, [
    CURLOPT_SSL_VERIFYPEER  => true,
    CURLOPT_SSL_VERIFYHOST  => 2,
    CURLOPT_FOLLOWLOCATION  => false,
    CURLOPT_PROTOCOLS       => CURLPROTO_HTTPS,
    CURLOPT_REDIR_PROTOCOLS => CURLPROTO_HTTPS,
    CURLOPT_CONNECTTIMEOUT  => 5,
]);

CURLOPT_TIMEOUT = 10 is already there and is adequate. Note that sends are synchronous inside broker_send(), so a route fanning out to Telegram adds up to 10 seconds to whatever request triggered it. Acceptable, worth knowing.

3. Validate the token and chat-id formats, and fail closed

Not exploitable, per the SSRF note above. But a typo or a pasted-with-whitespace token produces an opaque Telegram 404 rather than "that isn't a bot token" — and validation makes item 1 harder to abuse by construction.

!preg_match('/^\d+:[A-Za-z0-9_-]{20,}$/', $token)
!preg_match('/^-?\d{1,20}$/', (string) $chatId)

4. A setup section in the docs, before the channel is announced

You supplied the two non-guessable facts and they should not stay buried in a PR thread: the chat id must be the group's (negative, -100…) — a positive DM id fails with Forbidden: bot can't initiate conversation with a user, which reads like a permissions problem — and the bot must already be a member of the group before getUpdates shows the chat at all. ZELLO-SETUP-GUIDE.md is the model.

Not blockers: no 4096-character cap on text (Telegram returns a clear error, which is surfaced), and no rate limiting (the only interactive trigger is the admin-gated test_channel, and routed sends inherit the routing engine's own limits).

---

Read this before you rebase: most of the proc_open half is already in the dev tree

This is the awkward part, and you should hear it straight rather than discover it in a merge conflict.

Eric asked you not to rebase onto the two commits he had already pushed, saying he would resolve overlap on merge. Since then the exec/shell_exec conversion was implemented independently in the dev tree as 8a9ec2a, covering the same six files and the same eight call sites, with the same argv-array approach and the same reasoning about escapeshellarg. So of what is in this PR today:

Part	State in the dev tree
proc_open conversion, all six files	already there (8a9ec2a), independently
wmic → Get-CimInstance fallback for Windows 11 24H2	already there
MySQL 8.0 TEXT DEFAULT	already fixed before that
telegram_bot_token silent-blanking (550980c)	fixed generally — applySettingsToForm() now handles every data-secret field, plus a server-side backstop in api/config-admin.php
inc/channels/telegram.php + the #btnTestTelegram handler	not in the tree. Still genuinely wanted.

There is also now a suite gate, tests/test_no_shell_command_execution.php, which fails if a string-form proc_open() appears, if any argv-literal element contains a superglobal or a variable concatenation, or if exec/shell_exec/system/passthru/popen/backticks reappear in those six files. Your conversion is what that exists to protect.

I would rather tell you now than let you spend an evening reconciling eleven files. The Telegram adapter is the part with nothing behind it, and it is the part worth your time. A scope reduction to inc/channels/telegram.php + the config.js handler + the setup doc, with the four changes above, would be a clean thing to land. Entirely your call, and Eric's on the merge either way.

One consequence to be aware of, since it is invisible from this side: this repository is a one-way, full-tree-replace snapshot of a private dev tree. A change merged only here is overwritten by the dev tree's version of that file at the next release — or deleted outright, if the dev tree has no such file. Git raises no objection and no diff shows it. There is now a guard that refuses to publish when that would happen (tools/release-divergence-check.php, which compares the staged snapshot against this repo's main and against the tree the last release published, and fails closed if it cannot reach either). But the flow it enforces is that an accepted contribution gets applied in the dev tree; merging here is the acknowledgement, not the shipping. That is why "already in the dev tree" above means shipped — and why your branch showing as behind is not anyone rejecting it.

For the record, reviewing this turned up two pre-existing defects in the code paths it touches — three settings panels silently wiping stored credentials on save, and CLI-only scripts being reachable over HTTP. Both are fixed (1923ba5, 50fcc74). Neither was yours; they were found because reviewing your work meant reading that code carefully. Details are in my comment on #9 and in #13.

[rjonesbsink]

All four changes made, and the scope reduction taken. Force-pushed — the branch is now five files instead of eleven, rebased onto current main.

Thank you for the heads-up on the dev tree. Reconciling eleven files against work that was already done would have been a genuinely wasted evening, and it is not something I could have discovered from this side.

The four

1 — chat id pinned to config. Done, and I verified it rather than just making the edit: with an invalid configured chat id and a valid override supplied, the send fails closed on validation and transmits nothing. If the override were still honoured it would have gone through.

with invalid config + valid override:
  {"success":false,"error":"Telegram chat ID is malformed (…)"}
  => PINNED — override ignored, failed closed, nothing sent

Your reasoning is recorded in a comment at the call site — the router forwarding the array wholesale, the two receive handlers returning raw third-party JSON, and specifically the point that its unreachability is a property of someone else's response schema rather than of this codebase. That last part is the bit a future reader needs and would not otherwise reconstruct.

2 — cURL options explicit. All six, matching inc/webhooks.php. Kept TIMEOUT 10 and noted the synchronous-inside-broker_send() consequence in a comment, since that is invisible at the call site.

3 — validation. Both patterns as suggested, failing closed with a message that names what is wrong. I also applied the same checks in _telegram_status() — a malformed token otherwise reports configured for a channel that cannot send, which seemed worse than an honest not_configured.

4 — setup guide. docs/TELEGRAM-SETUP-GUIDE.md, modelled on the Zello guide, with both gotchas given their own headings and troubleshooting entries keyed by the literal error text — Forbidden: bot can't initiate conversation with a user is the one that costs an hour, precisely because it sounds like a permissions problem.

One deliberate omission

I left loadTelegramConfig()'s save path alone and added only the #btnTestTelegram handler. My earlier commit had refactored it onto applySettingsToForm()/collectSettingsFromForm(), which needed data-key/data-secret attributes on the fields — but you have fixed the blanking generally in the dev tree, so changing it here would collide with your version. Flagging it because the omission is intentional rather than an oversight.

On the snapshot model

Understood, and useful to know — I had assumed a merge here was the shipping mechanism. The one practical consequence I would flag: the proc_open conversion is not in this repo's main yet, so anyone installing from a release before the next snapshot still hits the disable_functions failure. Not asking for anything, and I appreciate that is exactly what the divergence guard exists to manage — just noting it in case the ordering matters for the next release.

Happy to fold in anything else. And no objection at all if the answer to the per-message destination idea is "not without the allowlist and the trust marker" — that was your point and I think it is the right one.

[ejosterberg]

Merged. Thank you — this is a genuinely useful contribution, and the way you handled the review made it easy to land.

What landed

• inc/channels/telegram.php — the adapter, registered through broker_register() and picked up by inc/broker.php's glob.
• The missing #btnTestTelegram handler in assets/js/config.js. You were right that the Settings panel had shipped a complete Telegram UI — including a Send Test button — with nothing behind it: no adapter, so a routed send was rejected as an unregistered channel, and the button did nothing at all.
• docs/TELEGRAM-SETUP-GUIDE.md, plus the telegram rows in MESSAGE-ROUTING-GUIDE.md and ROUTING-ENGINE-REFERENCE.md.

Reducing the scope to just the Telegram work once the rest had landed in the dev tree was the right call and made this reviewable on its own.

The four security changes, and why each was needed

Three of these you applied on the branch after the review; the fourth was already in the dev tree. Recording them here so the reasoning is on the record with the code.

1. The destination chat id is pinned to configuration — inc/channels/telegram.php:53

The original line was $chatId = $message['telegram_chat_id'] ?? $config['telegram_chat_id'] ?? '';.

Nothing could reach it. Every broker_send() call site in the tree builds its message array from a fixed key list, and api/chat.php's test_channel is admin-gated, CSRF-checked, and passes five literal keys. So this was not a live vulnerability, and I want to be clear about that.

It was a live hazard. inc/router.php forwards a matched message array wholesale to the destination adapter — _router_transform() rewrites body, priority and type and leaves every other key intact — and two receive handlers already return raw third-party JSON into that path (_slack_receive() returns $data['messages'], _sms_receive() returns $data['threads']). Neither Slack nor Pushbullet currently lets a message author set an arbitrary top-level key, so the override was unreachable — but that is a property of someone else's response schema, not of this codebase, and nothing tells us when it changes. If it ever did, every routed message — incident type, dispatch address, patient count, responder callsign and coordinates — would go to an attacker-chosen chat, and the routing log would say forwarded.

The reason a latent issue was treated as blocking is that this project has been here twice already: assigns.rec_facility_id was "a column nothing writes" right up until it turned out to be a lost mass-casualty capability, and un_status.extra_data_target was an ENUM widened for a value nothing ever set. "Unreachable today" is exactly what those looked like.

telegram_chat_id is not a per-message recipient like to — it is the destination bound to the bot credential. Config is where it belongs. (inc/channels/slack.php still has the same shape for slack_channel; that is pre-existing and not yours, and it is on the list.)

2. cURL security options stated explicitly — inc/channels/telegram.php:85-94

VERIFYPEER => true, VERIFYHOST => 2, FOLLOWLOCATION => false, PROTOCOLS and REDIR_PROTOCOLS both CURLPROTO_HTTPS, CONNECTTIMEOUT => 5 alongside the existing TIMEOUT => 10.

The defaults were already safe, so this changed no behaviour on a normal host. It matters for two reasons: a host with unusual curl.* ini settings changes the answer, and a reader cannot otherwise tell "verified safe" from "nobody checked". Every other outbound caller in the codebase states them (inc/webhooks.php, api/dmr-lookup.php, tools/aprs-poller.php), so this is now consistent with them. The connect timeout is the one that does real work: broker_send() is synchronous, so a route fanning out to Telegram delays whatever dispatch action triggered it, and an unresponsive host is the case that hangs longest.

3. Token and chat-id format validation, failing closed — inc/channels/telegram.php:28-29, 58-61, 152-153

Not a security fix so much as a diagnosability one — there was no SSRF here, since the host is a hard-coded literal and the token lands in the path, after the authority, where it cannot re-point the request. But a token pasted with trailing whitespace produced an opaque 404 from Telegram, which reads like a permissions problem. Failing closed with a message naming the actual fault is better. Applying the same checks in _telegram_status() was a nice touch: a status of "configured" that cannot send is worse than an honest one.

4. A durable gate on the argv-array property — tests/test_no_shell_command_execution.php

The review asked for this to land in the same merge as your proc_open conversion. It went in with 8a9ec2a and is already in the tree, so there was nothing to do here. Worth saying plainly: dropping escapeshellarg() in that conversion was correct, not an omission. Array-form proc_open goes to execvp/CreateProcess, so there is no shell for a metacharacter to be syntax in — re-adding the escaping would have been an active bug. The gate exists so that stays true after both of us have forgotten why.

Your adapter is in the dev tree, so it survives releases

This is worth knowing if you contribute again. openises/TicketsCAD is published from a private development tree as a one-way, full-tree-replace snapshot. A change merged only here is overwritten by the next release — or deleted outright if the dev tree has no such file — and git raises no objection, because from this side it just looks like the maintainer committed a tree in which your change is absent.

So merging alone would not have kept your work. Your commit is now in the dev tree with your authorship preserved (74feacb), which is what actually makes it permanent. It is accompanied there by tests/test_telegram_channel_security.php, which proves the chat-id pin behaviourally rather than by reading the source: it drives the real _telegram_send() with a well-formed token, a malformed configured chat id, a well-formed override on the message, and an empty body. The guard order means the pin holding returns "chat ID is malformed" and the pin removed returns "Message body required" — so the outcome names which value was resolved, and neither path reaches the network. It then runs the same harness against a mutant built from the real source with the pin taken back out, to show the assertion actually fails when the property is gone.

Reintroducing the vulnerable line into the adapter takes that file to 25 passed / 5 failed. So if anyone ever "simplifies" the pin away, they will hear about it.

Thanks again — for the adapter, for the setup guide (the two non-guessable facts about the negative group id and bot membership are exactly the things that cost people an afternoon), and for turning the review around as quickly as you did.