Nice. I’ve been building out some API proxies for FPBX endpoints for use in other projects and I concur with your approach/workflow for integrating AI into the development process.
I’ve been using Claude (Opus 4.6, max effort) pretty much exclusively and it’s not uncommon for initial/design prompts to take 20-30 minutes to produce before giving to the agent. I find being thoroughly detailed and as explicit as possible returns the best results.
Regarding your conversation points:
- (See above) Otherwise, I haven’t actually ran into any issues integrating AI into the dev process, at least not the way I utilize AI in my workflow. For me it’s just one of my tools (as opposed to vibe coding I guess?)
- Other use cases:
- Monitoring - Any potential for an agent to “check in” on things? I find it’s a bit nuanced to know when real degradation is occurring on FPBX. There’s obvious things to watch for like no extensions are registering or watching error logs, but it’s harder to say something like “at what threshold (number of extensions failing registration) do we now have a critical situation”. So leveraging the higher intuition of an LLM may suit itself well here. Also, context-aware escalation might be possible where minor degradation only triggers an email notification, but major degradation results in a phone call, etc.
- Summaries as well as troubleshooting specific call logs/stacks, CDRs - Provide general summaries/comments based on what the agent is observing in pjsip debug log, in real-time. Insights could be basic such as pointing out errors, but could be more subtle like noticing longer delays with certain modules or where a call might be getting stuck for a longer period of time. Also, troubleshooting/summarization for specific calls that may have had trouble. This might lend itself well to the chat: give it a call ID, number, or timestamp and it runs down the call to see if there were any obvious issues or peculiarities.
- Active Spam Filtering/Denylist - Similar to Fail2Ban, but specifically for valid, inbound calls. Discover common spam behavior by profiling call patterns and then using those behaviors to flag numbers as potential spam. Going further, when flagged, inbound calls could be dynamically routed first through a “Antispam Bot Check” IVR that presents the caller with a math question and asks them to press the number for the answer, e.g. “What’s 2+3?” and the caller has to press 5. Not sure how many would actually use this in prod, but just an idea as our only solution to spam callers so far has just been to have any IVR (even if it just says something generic like “Press 1 to reach the front office.”)
- See dump below.
Thanks for taking the time to make this write-up and describing your process.
============================
Frogman Tools — Full Code Review (222 files)
Overall Assessment: NEEDS WORK
The architecture is sound — BMO-first routing is followed, the confirm-gate
pattern is consistently applied on write tools, and SQL is parameterized almost
everywhere. The issues below are real but not systemic design flaws. Most fall
into a few recurring patterns that can be addressed systematically.
CRITICAL (7 findings)
C1. API tokens stored in plaintext — CreateApiToken.php:35
Raw bin2hex(random_bytes(32)) written directly to oc_api_tokens. If the
table is ever leaked (backup, SQLi elsewhere), every token is immediately
usable. Store as hash(‘sha256’, $token) and compare on auth. Return the raw
token only once on creation.
C2. AMI command injection — multiple tools
User-controlled or DB-sourced values interpolated into $astman->Command()
strings. AMI is line-framed text — a newline in the value injects additional
AMI actions.
- ConfbridgeListLive.php:15 — $params[‘room’] with no format validation
- DiagnoseTrunk.php:45,59,64 — $trunkName from DB, unsanitized
- DiagnoseExtension.php:35,59 — $ext is numeric-validated so low risk, but
same pattern
- GetTrunkStatus.php:52, GetExtensionHealth.php:45 — same pattern
Fix: validate all AMI command arguments against /+$/ before
interpolation.
C3. Path traversal in GetVoicemail — GetVoicemail.php:18-20
$ext is used directly in a filesystem path
(/var/spool/asterisk/voicemail/default/$ext) and glob(). The validate() only
checks !empty() — no numeric pattern. A value like ../../etc probes arbitrary
directories. Add preg_match(‘/^\d+$/’, $params[‘ext’]).
C4. Dialplan injection via templates — Dialplan/Templates.php + DialplanApply.php
Template parameters (url, extension, code, file, etc.) are interpolated into
raw Asterisk dialplan text and written to extensions_custom.conf with no
escaping. A caller can inject arbitrary dialplan directives via newlines or
unbalanced parentheses. The confirm:true gate is the only control — there is
no content-level validation.
C5. Plaintext password in API response — ResetPassword.php:52
Auto-generated password returned in the tool response. The audit log rule
(“every tool execution gets outcome records”) means this password is
permanently stored in oc_audit_log and readable by anyone who can call
fm_audit_search. Same concern applies to AddExtension.php:172 which returns
secret in the response.
C6. ExportCsv.php writes to a web-accessible path — ExportCsv.php:63-74
CDR exports with PII (caller IDs, timestamps) are written under the module’s
assets/exports/ directory with a predictable timestamp-based filename. No
cleanup mechanism, no per-user scoping on the download endpoint.
C7. SipTrace.php uses raw exec() outside the hardened wrapper — SipTrace.php:72,98
Direct exec() calls for nohup tcpdump and pkill bypass runFwconsole(). The
pgid file at a fixed path (/tmp/frogman_sip_trace.log.pgid) is also a
shared-state collision risk between concurrent callers.
HIGH (19 findings)
H1. Missing input validation — widespread pattern
Many tools check only !empty() without format/type validation. Contrast with
DiagnoseExtension and PjsipEndpointDetails which correctly use preg_match.
Affected tools:
- DisableVoicemail, EnableVoicemail — ext should be numeric
- DisableTrunk, EnableTrunk — id should be numeric
- DeleteRinggroup — id should be numeric
- PjsipQualify — ext should be alphanumeric
- ToggleDaynight — id should be numeric
- ToggleDnd — ext should be numeric
- ModuleDisable/Enable/Install/Uninstall/Upgrade — name should be /+$/i
- BackupCreate — id should be UUID or numeric
- MonitorCall, StopMonitorCall, TransferCall — channel should match channel-name pattern
- OriginateCall — ext/dest should be numeric/dialable
H2. Permission level too low on sensitive tools
Several tools default to PERM_READ via inheritance but expose sensitive data:
- ListManagers — exposes AMI manager secrets — should be PERM_ADMIN
- ListPinsets — exposes outbound PIN codes — should be PERM_ADMIN
- AuditSearch — exposes all tool params, session IDs, user actions — should be PERM_ADMIN
- ShowDialplanContext — exposes full Asterisk dialplan for any context — should be PERM_ADMIN
- GetMcpConfig — exposes root SSH connection instructions — should be PERM_ADMIN
- RunSavedQuery — executes arbitrary saved GraphQL (including mutations) — should be PERM_WRITE min
- BackupStatus — exposes filesystem paths, transaction UUIDs — should be PERM_ADMIN
- WhosCalling — exposes CDR call history with caller IDs — should be PERM_WRITE min
H3. UpdateExtension.php silently zeroes voicemail password and email — lines 87-89
Hard-coded $merged[‘vm’] = ‘no’; $merged[‘vmpwd’] = ‘’; $merged[‘email’] = ‘’;
applied unconditionally. Every update to name/secret/CID also clears voicemail
config. The dry-run preview doesn’t show this side effect. This is data loss
on confirmed writes.
H4. ToggleTimecondition.php — no range check on state — lines 20-21
User-supplied state is cast to (int) but not range-checked against valid
values (0, 1, 2). state=99 produces an empty label in the response and calls
setState(99) on the BMO.
H5. UpdateSipNat.php — no IP/CIDR validation — lines 8-11
external_ip and local_network accept arbitrary strings and pass directly to
Sipsettings->setConfig(). A malformed IP breaks SIP registration for all
endpoints.
H6. SetCallForward.php — type not validated against allowlist — lines 8-11
Valid values are CF, CFB, CFU but any string is accepted and forwarded to BMO.
H7. SearchPbx.php — wrong column for queues query — line 32
Queries queues_config.descr as a top-level column, but FreePBX stores queue
descriptions as key-value rows (keyword=‘descr’). This silently returns zero
queue results for every search.
H8. ChatParser conference commands use wrong parameter name — lines 1501-1523
ChatParser builds [‘id’ => $m[1]] for conference tools, but all four
Confbridge tools validate $params[‘room’]. Conference commands via chat
always fail with “Parameter ‘room’ is required.”
H9. FwconsoleCmd allowlist gap — line 16
context is an unanchored bare word matching any subcommand starting with
“context”. The read-only detection is a separate hardcoded list that can
drift from the allowed list.
H10. Reload.php overcounts active calls — lines 18-21
Does not use isAsteriskInternalChannel() to filter Message/* and AsyncGoto/*
pseudo-channels. Inflated call count may block operators from reloading.
H11. str_shuffle() weakens password entropy — CreateAdmin.php:107, ResetPassword.php:36
Passwords are generated with random_int() (good) then shuffled with
str_shuffle() which uses mt_rand (not cryptographically secure). Replace
with Fisher-Yates using random_int().
H12. RepairUsermanLinks.php — early exit returns dry_run: false without confirm — lines 73-79
When nothing needs repair, the response says dry_run: false even when called
without confirm:true. Same pattern in RinggroupAddMember.php:50-59.
H13. DiagnoseTrunk exposes raw AMI endpoint output — line 61
endpoint_raw dumps the full AMI response which may include SIP auth details.
No redaction.
MEDIUM (20 findings)
M1. ListCallForwards.php:10 — direct SELECT from users table instead of BMO
Core->getAllUsers()
M2. ListExtensionRange.php:8-9 — no check that from <= to, no numeric
validation
M3. ListBackupRuns.php:146-147 — exposes full filesystem paths in response at
read level
M4. GetVoicemail.php:19-20 — glob() returns false on error; count(false) is a
TypeError in PHP 8.1+
M5. DialplanApply.php:25-34 — template parameters pass through with no
per-template validation
M6. DialplanFile.php:169-171 — exec(‘asterisk -rx …’) fallback outside
runFwconsole() wrapper
M7. GetExternalIp.php:10-11 — no exit code check; returns empty string on
failure with no error
M8. DeleteSavedQuery.php:51-55 — missing dry_run key in success response,
breaking consistent pattern
M9. GetTimeGroup.php:24 — fragile positional array access $group[1] on BMO
return value
M10. GetAllRingGroups.php:10 — returns raw grplist string; GetRinggroup splits
it into an array
M11. RunSavedQuery.php:86-95 — no cURL timeout; hangs indefinitely on slow
systems
M12. QueuePauseAgent.php:16 — “false” string is truthy; caller sending
paused: “false” gets paused
M13. MuteCall.php:15-16 — direction parameter unvalidated; should be in|out|all
M14. SetRecording.php:105-107 — $_SERVER[‘HTTP_HOST’] reflected into response
URL (host header injection)
M15. SipTrace.php:26,39-40 — single global trace file; concurrent callers
collide
M16. TraceCallFlow.php:168-186 — dead unreachable branch; ext-local voicemail
destinations misclassified
M17. ListBackupJobs.php / ListBackupRuns.php — duplicated buildLocationMap() and
boolish() methods
M18. GetAdvancedSetting.php:16 — typo: CLOBERFREEPBXCONF should be
CLOBBERFREEPBXCONF
M19. Pm2Manage.php:20 — dynamic method dispatch $freepbx->Pm2->$action()
instead of explicit match
M20. $limit concatenated into SQL via (int) cast instead of bound parameter —
ExportCsv.php:50, GetFailedCalls.php:10, GetPeakHours.php:10,
GetBusiestExtensions.php:10, GetCdrStats.php:10
LOW (8 cross-cutting patterns)
L1. ~20+ tools cram entire execute() bodies onto a single line, making review
and diffing nearly impossible
L2. Several tools return raw AMI/fwconsole output strings instead of structured
data (ListChannels, ListSipPeers, ListSounds)
L3. Duplicate/near-identical tools: GetBackup/GetBackupDetails,
GetVoicemail/GetMailbox, GetQueue/GetQueueDetails
L4. Many tools don’t explicitly declare permissionLevel(), relying on inherited
default — makes the security surface implicit
L5. DisableExtension.php:13 description says “Delete/disable” but the tool
hard-deletes (irreversible)
L6. LintTypeahead.php:125-128 — dead extractKeyword() method (singular) never
called
L7. AuditSearch.php:64 — dead $db variable assigned but never used
L8. Legacy requiredPermission() returns non-null strings in some tools but the
value isn’t enforced anywhere
What’s Working Well
-
SQL injection protection — parameterized queries used consistently across all
222 tools. No string-interpolated user input into SQL found.
-
Confirm gate pattern — all write tools implement confirm: true / dry-run
correctly with strict === true comparison preventing truthy string bypass.
-
BMO-first routing — tools correctly prefer BMO over direct DB, AMI over exec,
matching the documented hierarchy.
-
oc_* table discipline — no tool writes to another module’s DB tables. All
direct writes target Frogman’s own tables.
-
runFwconsole() wrapper — well-designed centralized command runner with ANSI
stripping, escapeshellarg, background mode, and TTY emulation.
-
Standout implementations — ListBackupRuns (inferred missed-tick logic),
SetExtensionEmail (dual-layer update with verify read-back),
RepairUsermanLinks (idempotent repair with thorough pre-flight).
Recommended Fix Priority
-
Immediate — C1 (token hashing), C2 (AMI injection), C3 (path traversal),
C5 (password in response)
-
This week — C4 (dialplan injection), C6 (export file exposure),
H2 (permission levels), H3 (UpdateExtension data loss),
H7 (SearchPbx broken query)
-
Next sprint — H1 (input validation sweep), H8 (ChatParser param name),
H11 (password entropy), remaining medium items
-
Backlog — Low items, formatting cleanup, duplicate tool consolidation
Up front: this is experimental. Don’t run it on production yet - test box only please, you’ve been warned.
handles complaints.
and coincidentally one of my favorite presentation slides – been using modified versions of this gem at several conferences for the past year or so!
