> For the complete documentation index, see [llms.txt](https://docs.mikopbx.com/mikopbx-development/llms.txt). Markdown versions of documentation pages are available by appending `.md` to page URLs; this page is available as [Markdown](https://docs.mikopbx.com/mikopbx-development/module-developement/best-practices.md).

# Best practices

## Best practices

This page is the rulebook for writing MikoPBX modules that are clean, secure, and indistinguishable from Core code. It targets **MikoPBX 2025.1.1+, PHP 8.4, Phalcon 5.9.3**. Everything here is verified against the Core source and the example modules shipped under `Extensions/EXAMPLES/`.

The running example throughout this documentation is a fictional module **`ModuleBlackList`** (config class `BlackListConf`, main class `BlackListMain`, model `BlackListNumbers`, table `m_BlackListNumbers`, front-end `module-black-list.js`). Every rule below is also anchored to a **real** example module by repo-relative path so you can read working code.

{% hint style="info" %}
**How to read this page.** The conventions come first (naming, file layout, PHP idioms). The anti-patterns come second, each with a *Detection* signal, the *Problem*, and a copy-paste *Fix*. Security anti-patterns (`S1`–`S6`) are priority fixes — treat them before any code-quality cleanup.
{% endhint %}

### File headers

Every PHP file in a module starts the same way and never emits a closing tag:

```php
<?php

declare(strict_types=1);

namespace Modules\ModuleBlackList\Lib;
```

* `declare(strict_types=1);` is mandatory on **every** file (anti-pattern #20 below). It makes scalar type hints reject silent coercion — a `string` passed where `int` is declared throws `TypeError` instead of becoming `0`.
* No closing `?>`. A trailing tag risks emitting stray whitespace that corrupts headers or generated config files.

See a real header in `Extensions/EXAMPLES/AMI/ModuleExampleAmi/Lib/ExampleAmiConf.php`.

### The DI import rule

This single import trips up almost every new module. Phalcon 5 moved the container class into its own sub-namespace:

```php
// CORRECT — Phalcon 5.9.3
use Phalcon\Di\Di;

$di = Di::getDefault();
$db = $di->get('db');
```

```php
// WRONG — Phalcon 4 era, will not resolve in 5.9.3
use Phalcon\Di;
```

Confirmed in real module code at `Extensions/EXAMPLES/AMI/ModuleExampleAmi/Lib/WorkerExampleAmiAMI.php` (`$di = Di::getDefault();`) and `Extensions/EXAMPLES/REST-API/ModuleExampleRestAPIv2/Lib/RestAPI/Backend/Actions/GetUsersAction.php`.

### Naming conventions

Consistent names are not cosmetic — the loader, the table-name resolver, the asset pipeline, and the REST router all derive paths from these patterns. Deviating breaks autodiscovery silently.

| Element                | Convention                                   | `ModuleBlackList` example                            |
| ---------------------- | -------------------------------------------- | ---------------------------------------------------- |
| Module ID / directory  | `Module{Feature}` (PascalCase)               | `ModuleBlackList`, dir `Extensions/ModuleBlackList/` |
| `moduleUniqueID`       | identical to Module ID                       | `ModuleBlackList`                                    |
| Config class           | `{Feature}Conf`                              | `BlackListConf`                                      |
| Main logic class       | `{Feature}Main`                              | `BlackListMain`                                      |
| Setup class            | always `PbxExtensionSetup`                   | `Setup/PbxExtensionSetup.php`                        |
| Model                  | `{Entity}`                                   | `BlackListNumbers`                                   |
| DB table               | `m_{Entity}` (auto from model)               | `m_BlackListNumbers`                                 |
| Web controller         | `Module{Feature}Controller`                  | `ModuleBlackListController`                          |
| Form                   | `Module{Feature}Form`                        | `ModuleBlackListForm`                                |
| Worker                 | `Worker{Feature}{Type}`                      | `WorkerBlackListAMI`                                 |
| REST resource classes  | `Controller` / `Processor` / `DataStructure` | `Lib/RestAPI/Numbers/Controller.php`                 |
| REST action            | `{Verb}{Entity}Action`                       | `GetListAction`, `SaveRecordAction`                  |
| JS source / compiled   | `module-{kebab}.js`                          | `public/assets/js/src/module-black-list.js`          |
| CSS                    | `module-{kebab}.css`                         | `public/assets/css/module-black-list.css`            |
| Translation key prefix | `module_{feature}_`                          | `module_black_list_NumberColumn`                     |
| Dialplan context       | `[module-{kebab}-{purpose}]`                 | `[module-black-list-check]`                          |

#### Namespaces

The PHP namespace mirrors the directory layout exactly (PSR-4):

```
Modules\ModuleBlackList\Setup            → Setup/PbxExtensionSetup.php
Modules\ModuleBlackList\Lib              → Lib/BlackListConf.php, Lib/BlackListMain.php
Modules\ModuleBlackList\Models           → Models/BlackListNumbers.php
Modules\ModuleBlackList\App\Controllers  → App/Controllers/ModuleBlackListController.php
Modules\ModuleBlackList\App\Forms        → App/Forms/ModuleBlackListForm.php
```

#### REST API path scheme

```
/pbxcore/api/v3/module-{kebab}/{resource}
/pbxcore/api/v3/module-{kebab}/{resource}/{id}
/pbxcore/api/v3/module-{kebab}/{resource}/{id}:{action}
```

```
GET    /pbxcore/api/v3/module-black-list/numbers
POST   /pbxcore/api/v3/module-black-list/numbers
GET    /pbxcore/api/v3/module-black-list/numbers/123
DELETE /pbxcore/api/v3/module-black-list/numbers/123
```

See the live v3 implementation in `Extensions/EXAMPLES/REST-API/ModuleExampleRestAPIv3/Lib/RestAPI/Tasks/`.

### PHP 8.4 idioms

Write modern PHP everywhere **except** Phalcon model column properties (see the [model exception](#the-phalcon-model-exception) immediately after).

#### Typed properties on non-model classes

`Conf`, `Main`, `Worker`, controllers, and service classes get full property types:

```php
class BlackListMain extends PbxExtensionBase
{
    public string $logPath = '';
    public readonly string $moduleUniqueId;
}
```

#### Constructor property promotion

```php
final class BlackListNotifier
{
    public function __construct(
        private readonly string $apiUrl,
        private readonly int $timeout = 5,
    ) {}
}
```

#### `match` over `switch` and over dynamic dispatch

Use `match` for request routing inside Conf hooks. It is exhaustive, returns a value, and uses strict comparison. This is the canonical REST callback pattern.

{% code title="Lib/BlackListConf.php" %}

```php
public function moduleRestAPICallback(array $request): PBXApiResult
{
    $action = $request['action'] ?? '';
    $data   = $request['data'] ?? [];

    return match ($action) {
        'check'  => $this->checkAction($data),
        'reload' => $this->reloadAction(),
        default  => $this->createErrorResult("Unknown action: {$action}"),
    };
}
```

{% endcode %}

This is exactly the shape used in `Extensions/EXAMPLES/AMI/ModuleExampleAmi/Lib/ExampleAmiConf.php` (`moduleRestAPICallback`). `match` is also the prescribed fix for the dynamic-dispatch security hole — see [S6](#s6-dynamic-dispatch-from-user-input).

#### Named arguments and backed enums

```php
enum CallDirection: string
{
    case Incoming = 'incoming';
    case Outgoing = 'outgoing';
}

$direction = CallDirection::from($channelVar);   // throws on unknown value
```

Named arguments make multi-flag calls self-documenting:

```php
Processes::processPHPWorker(
    className: WorkerBlackListAMI::class,
);
```

#### Full type declarations on methods

Every method declares parameter types and a return type — including `void` and `: never`. Anti-pattern #15 flags untyped methods.

```php
public function isBlocked(string $number): bool { /* ... */ }
private function refreshCache(): void { /* ... */ }
```

### The Phalcon model exception

{% hint style="warning" %}
**Model column properties are the one place you do NOT apply the PHP 8.4 typed-property rules.** This is intentional and matches the Core models — it is NOT an anti-pattern, and code reviewers/linters must not flag it.
{% endhint %}

SQLite stores everything as text, and Phalcon hydrates model columns from those text values. The Core models therefore follow this exact pattern:

{% code title="Models/BlackListNumbers.php" %}

```php
class BlackListNumbers extends ModelsBase
{
    // Primary key — ALWAYS untyped. Phalcon assigns it after insert.
    public $id;

    // String columns — nullable, with a string default.
    public ?string $number = '';

    // Booleans / small integers stored as string ('0' / '1').
    public ?string $enabled = '1';

    // Integer foreign keys — nullable int, default null.
    public ?int $userid = null;
}
```

{% endcode %}

Why each rule holds:

* **`public $id;` untyped** — the row has no `id` until after `save()`; a typed property would have to be uninitialized then suddenly populated, fighting the ORM.
* **`?string` columns with `''` default** — values arrive as strings from SQLite; nullable absorbs `NULL` columns.
* **Integers-as-strings (`'0'`/`'1'`)** — flag columns round-trip as text; declaring them `?string` avoids lossy coercion.
* **Nullable int FKs (`?int … = null`)** — an unset foreign key is genuinely `NULL`.

This is confirmed verbatim in the Core models `src/Common/Models/Sip.php` (`public $id;`, `public ?string $disabled = '0';`), `src/Common/Models/Extensions.php` (`public ?int $userid = null;`), and `src/Common/Models/CallQueues.php`. Use those as your reference, not example modules — see also [Data model](/mikopbx-development/module-developement/data-model.md).

***

## Code-quality anti-patterns

Each entry lists how to spot it, why it hurts, and the fix. Numbers match the Core anti-pattern checklist (`reference/anti-patterns.md` in the `mikopbx-module` skill).

### #1 \[CRITICAL] `MikoPBXVersion.php` in new modules

**Detection.** A `MikoPBXVersion.php` file or `MikoPBXVersion::getDefaultDi()` calls in a module whose `module.json` declares `min_pbx_version` ≥ 2025.1.1.

**Problem.** `MikoPBXVersion` was a legacy compatibility shim. On a 2025.1.1+ baseline it is dead weight, and it hides the real DI container behind an indirection.

**Fix.** Delete the file and call the container directly:

```php
// BEFORE (legacy)
$di = MikoPBXVersion::getDefaultDi();

// AFTER (modern)
use Phalcon\Di\Di;

$di = Di::getDefault();
```

`Di::getDefault()` is the pattern used throughout the examples, e.g. `Extensions/EXAMPLES/AMI/ModuleExampleAmi/Lib/WorkerExampleAmiAMI.php`.

### #2 \[CRITICAL] `shell_exec` for reloads instead of framework methods

**Detection.** `shell_exec(...)` or `exec(...)` in `Lib/*Conf.php` invoking `asterisk -rx`, `safe_asterisk`, or service scripts.

**Problem.** Shelling out to reload Asterisk bypasses MikoPBX process management, skips config regeneration, races with other reloads, and runs unescaped (see [S3](#s3-command-injection)).

**Fix.** Use the framework's reload entry points. The two you will actually call from a module are:

```php
use MikoPBX\Core\System\System;
use MikoPBX\Core\System\Processes;

// Reload Asterisk Manager (AMI) — applies a manager.conf user, etc.
System::invokeActions(['manager' => 0]);

// Reload cron jobs after touching scheduled tasks.
System::invokeActions(['cron' => 0]);

// (Re)start this module's own workers — see #14 below.
foreach ($this->getModuleWorkers() as $worker) {
    Processes::processPHPWorker($worker['worker']);
}
```

This is the real pattern in `Extensions/EXAMPLES/AMI/ModuleExampleAmi/Lib/ExampleAmiConf.php` (`onAfterModuleEnable()` calls `System::invokeActions(['manager' => 0])`) driving `Extensions/EXAMPLES/AMI/ModuleExampleAmi/Lib/ExampleAmiMain.php` (`startAllServices()` loops over `getModuleWorkers()` and calls `Processes::processPHPWorker()`).

{% hint style="warning" %}
`System::invokeActions()` recognizes only the `manager` and `cron` keys — other keys are silently ignored. It is a thin wrapper that dispatches to `WorkerModelsEvents::invokeAction(ReloadManagerAction::class)` / `ReloadCrondAction::class`, and is itself marked **`@deprecated`** in favour of calling `MikoPBX\Core\Workers\WorkerModelsEvents::invokeAction(...)` directly. The shipped example modules still use the `System::invokeActions(['manager' => 0])` wrapper, so it remains the most readable choice for module code today; prefer the `WorkerModelsEvents::invokeAction()` form if you want the non-deprecated call. For dialplan and SIP regeneration, the modern Core APIs are `MikoPBX\Core\Asterisk\Configs\ExtensionsConf::reload()` and `MikoPBX\Core\Asterisk\Configs\SIPConf::reload()`. The old facades `PBX::dialplanReload()` / `PBX::sipReload()` (and the whole `MikoPBX\Core\System\PBX` class) are **`@deprecated`** — do not use them in new code. The one non-deprecated reload method on `PbxConf` is `MikoPBX\Core\System\Configs\PbxConf::coreReload()`.
{% endhint %}

### #3 \[HIGH] Monolithic `Conf` class

**Detection.** `Lib/{Feature}Conf.php` over \~500 lines, or business logic (HTTP calls, parsing, ORM loops) living inside hook methods.

**Problem.** The `Conf` class exists to implement [hooks](/mikopbx-development/module-developement/module-class.md) and delegate. Stuffing logic into it makes it untestable and couples lifecycle events to behavior.

**Fix.** Extract behavior into `{Feature}Main`. Hooks stay thin:

```php
// Lib/BlackListConf.php
public function onAfterModuleEnable(): void
{
    System::invokeActions(['manager' => 0]);
    (new BlackListMain())->startAllServices();
}
```

`Extensions/EXAMPLES/AMI/ModuleExampleAmi/` is the canonical split: `ExampleAmiConf.php` (hooks) delegates to `ExampleAmiMain.php` (logic).

### #7 \[HIGH] Phantom model fields

**Detection.** Code reads `$record->someField` where `someField` is not a declared column property on the model.

**Problem.** Phalcon returns `null` for an undeclared property access instead of erroring, so the bug is silent — branches misfire, data goes unsaved.

**Fix.** Only access properties that are declared with column annotations on the model. If you need a new field, add it to the model **and** to the installer's migration (see [Data model](/mikopbx-development/module-developement/data-model.md)). Reference real models in `src/Common/Models/`.

### #8 \[MEDIUM] `die()` / `exit()` in library and worker classes

**Detection.** `die(` or `exit(` in `Lib/*.php` or `bin/*.php` (worker code).

**Problem.** Hard exits skip destructors and `finally` blocks, prevent graceful worker shutdown, and make the code untestable.

**Fix.** In a worker, log and flag for restart; in a library, throw.

```php
// In a worker (bin/WorkerBlackListAMI.php)
$this->logger->writeError('Settings not set');
$this->needRestart = true;
return;

// In a library (Lib/BlackListMain.php)
throw new \RuntimeException('Settings not set');
```

### #9 \[MEDIUM] `1*$variable` casting idiom

**Detection.** `1*$var` or `1*shell_exec(...)`.

**Problem.** A PHP 4-era trick to coerce to a number. Obscure and bypasses strict typing intent.

**Fix.** Cast explicitly:

```php
$count = (int) $var;
```

### #10 \[MEDIUM] `md5(print_r(...))` for change detection

**Detection.** `md5(print_r($data, true))`.

**Problem.** `print_r()` output is ambiguous — distinct structures can render identically — so the hash misses real changes.

**Fix.** Hash a canonical, unambiguous encoding:

```php
$hash = md5(json_encode($data, JSON_THROW_ON_ERROR));
```

### #13 \[MEDIUM] File-based IPC instead of Redis

**Detection.** `file_put_contents(... json_encode ...)` paired with `file_get_contents(... json_decode ...)` used to pass state between processes.

**Problem.** Files have no atomicity or TTL — workers race, and stale state accumulates.

**Fix.** Use the shared Redis service from the DI container, with an expiry:

```php
$redis = Di::getDefault()->get('redis');
$redis->setex("blacklist_state:{$id}", 600, json_encode($data, JSON_THROW_ON_ERROR));
```

### #14 \[MEDIUM] Manual worker killing

**Detection.** `Processes::killByName(...)` inside `Lib/*Conf.php` to restart workers.

**Problem.** Manually killing PIDs fights the safe-scripts watchdog, which will restart workers on its own schedule, producing flapping.

**Fix.** Let the framework manage worker lifecycle. Enumerate your workers through `getModuleWorkers()` (declared in `MikoPBX\Modules\Config\ConfigClass`) and hand each to `Processes::processPHPWorker()`, which restarts-or-starts as needed:

```php
foreach ($this->getModuleWorkers() as $worker) {
    Processes::processPHPWorker($worker['worker']);
}
```

This is `startAllServices()` in `Extensions/EXAMPLES/AMI/ModuleExampleAmi/Lib/ExampleAmiMain.php`.

### #20 / #21 \[LOW] Missing `strict_types` and wrong DI import

Covered above under [File headers](#file-headers) and [The DI import rule](#the-di-import-rule). Detection: `grep -L "declare(strict_types" *.php` finds files missing the declaration; `use Phalcon\Di;` (without `\Di`) is the wrong import.

***

## Security anti-patterns

{% hint style="danger" %}
Modules run inside the PBX with full filesystem and DB access; AGI scripts run as **root**. A vulnerable module compromises the whole appliance. Fix every `S`-level finding **before** any code-quality work. For the authorization model and how endpoints are protected, see [Rights and authentication](/mikopbx-development/cookbook/rights-and-auth.md).
{% endhint %}

### S1 \[CRITICAL] Unauthenticated endpoints

**Detection.** A REST route registered with `noAuth = true` — the 6th element (`$additionalRoute[5]`) of a route array returned by `getPBXCoreRESTAdditionalRoutes()`. When that flag is `true`, `Request::thisIsModuleNoAuthRequest()` matches the URI and lets the request through **without** authentication.

**Problem.** An unauthenticated route that mutates state, originates calls, or returns sensitive data is an open door.

**Fix.** Never set `noAuth = true` for endpoints that modify settings, originate calls, expose credentials/tokens, touch CDR or recordings, or serve files by user path. Keep them behind the default auth, or verify an HMAC/token inside the handler.

### S2 \[CRITICAL] SQL injection in `findFirst` / `find`

**Detection.** String interpolation inside a condition: `Model::findFirst("col='{$x}'")`.

**Problem.** User input concatenated into the WHERE clause is classic SQL injection.

**Fix.** Always use parameterized binds:

```php
// BEFORE (vulnerable)
BlackListNumbers::findFirst("number='{$userInput}'");

// AFTER (safe)
BlackListNumbers::findFirst([
    'conditions' => 'number = :num:',
    'bind'       => ['num' => $userInput],
]);
```

### S3 \[CRITICAL] Command injection

**Detection.** A variable inside `shell_exec` / `Processes::mwExec` / `exec` without `escapeshellarg()`. Especially dangerous in `agi-bin/` (root) and workers handling external data (CDR filenames, SIP headers, API payloads).

**Problem.** Unescaped input becomes shell syntax — arbitrary command execution.

**Fix.** Wrap **every** variable:

```php
// BEFORE (vulnerable)
shell_exec("$sox $inputFile $outputFile");

// AFTER (safe)
shell_exec(sprintf(
    '%s %s %s',
    $sox,
    escapeshellarg($inputFile),
    escapeshellarg($outputFile),
));
```

### S4 \[CRITICAL] Path traversal / arbitrary file read

**Detection.** `fopen` / `file_get_contents` / `readfile` / `fpassthru` on a user-supplied path with no validation against an allowed base directory.

**Problem.** `../../etc/...` escapes the intended directory and reads arbitrary files.

**Fix.** Resolve the real path and confirm it is inside the allowlisted base:

```php
$resolved    = realpath($userInput);
$allowedBase = realpath('/storage/usbdisk1/mikopbx/astspool/monitor/');

if ($resolved === false || !str_starts_with($resolved, $allowedBase)) {
    $this->sendError(403);
    return;
}
```

### S5 \[CRITICAL] Reflected XSS

**Detection.** `$_REQUEST` / `$_GET` / `$_POST` echoed into HTML or inline JS without escaping.

**Problem.** Attacker-controlled markup executes in the admin's browser.

**Fix.** Escape on output:

```php
<?php echo htmlspecialchars($input ?? '', ENT_QUOTES, 'UTF-8'); ?>
```

### S6 \[HIGH] Dynamic dispatch from user input

**Detection.** `$this->$action(...)` / `self::$method(...)` where the name comes from the request.

**Problem.** A user-controlled method name exposes **every** public method of the class as a callable endpoint.

**Fix.** Replace variable dispatch with an explicit `match` allowlist (the same idiom used for [REST routing](#match-over-switch-and-over-dynamic-dispatch)):

```php
$result = match ($request['action'] ?? '') {
    'check'  => $this->checkAction($data),
    'reload' => $this->reloadAction(),
    default  => throw new \InvalidArgumentException('Unknown action'),
};
```

***

### Quick checklist before you ship

* [ ] Every `.php` file: `<?php` + `declare(strict_types=1);`, no closing tag.
* [ ] `use Phalcon\Di\Di;` — never `use Phalcon\Di;`.
* [ ] Non-model classes: typed properties, promoted constructors, full method types.
* [ ] Models: untyped `public $id;`, nullable `?string`/`?int` columns (the [exception](#the-phalcon-model-exception)).
* [ ] No `MikoPBXVersion`, no `PBX::*Reload()` facade — use `System::invokeActions()`, `Processes::processPHPWorker()`, or the modern `*Conf::reload()` methods.
* [ ] No `shell_exec`/`exec` for reloads; no `die()`/`exit()` in libs or workers.
* [ ] `Conf` delegates to `Main`; no monolith.
* [ ] All shell arguments wrapped in `escapeshellarg()`.
* [ ] All ORM lookups parameterized; all output escaped; no unauthenticated mutating routes.

Related reading: [The module configuration class](/mikopbx-development/module-developement/module-class.md) · [Data model](/mikopbx-development/module-developement/data-model.md) · [Rights and authentication](/mikopbx-development/cookbook/rights-and-auth.md).


---

# Agent Instructions
This documentation is published with GitBook. GitBook is the documentation platform designed so that both humans and AI agents can read, navigate, and reason over technical content effectively. Learn more at gitbook.com.

## Querying This Documentation
If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter, and the optional `goal` query parameter:

```
GET https://docs.mikopbx.com/mikopbx-development/module-developement/best-practices.md?ask=<question>&goal=<endgoal>
```

`ask` is the immediate question: it should be specific, self-contained, and written in natural language.
`goal` is optional and describes the broader end goal you are ultimately trying to accomplish on behalf of the user. GitBook uses it to tailor the answer towards what is most useful for that goal.

The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.