Skip to content

Code review follow-ups: medium and low findings #18

@mwtcmi

Description

@mwtcmi

Tracking issue for the smaller findings from the recent full-codebase review. Filing individually as the work begins.

Medium

  • ListCallForwards — use BMO Core->getAllUsers() instead of direct SELECT
  • ListExtensionRange — validate from/to are numeric and from <= to
  • ListBackupRuns — full filesystem paths in response at read-tier
  • GetVoicemailcount(glob(...)) is a TypeError in PHP 8.1+ if glob returns false
  • DialplanFileexec() fallback at lines 169-171 outside runFwconsole wrapper
  • GetExternalIp — no exit code check, returns empty on failure
  • DeleteSavedQuery — missing dry_run key in success response
  • GetTimeGroup — fragile positional array access $group[1]
  • GetAllRingGroups — returns raw grplist string; inconsistent with GetRinggroup
  • RunSavedQuery — no cURL timeout
  • QueuePauseAgent — string "false" is truthy
  • MuteCalldirection parameter unvalidated
  • SetRecording — reflects $_SERVER['HTTP_HOST'] into response URL
  • TraceCallFlow — dead branch at lines 168-186 misclassifies ext-local voicemail
  • ListBackupJobs / ListBackupRuns — duplicated helper methods
  • GetAdvancedSetting — typo CLOBERFREEPBXCONF should be CLOBBERFREEPBXCONF
  • Pm2Manage — dynamic method dispatch instead of explicit match
  • ExportCsv, GetFailedCalls, GetPeakHours, GetBusiestExtensions, GetCdrStats$limit concatenated into SQL via (int) cast (not exploitable but inconsistent)
  • ExportCsv — no cleanup mechanism for generated CSVs; PII accumulates on disk

Low / housekeeping

  • ~20 tools with single-line execute() bodies (diff/review friction)
  • Tools returning raw AMI/fwconsole strings instead of structured data (ListChannels, ListSipPeers, ListSounds)
  • Duplicate tools to consolidate: GetBackup/GetBackupDetails, GetVoicemail/GetMailbox, GetQueue/GetQueueDetails
  • Many tools missing explicit permissionLevel() (see also Multi-PBX fleet management #3)
  • DisableExtension description says "Delete/disable" but hard-deletes
  • LintTypeahead.php — dead extractKeyword() method
  • AuditSearch.php — dead $db variable
  • Legacy requiredPermission() returns non-null strings that aren't enforced

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions