Skip to content

Commit 88c0574

Browse files
committed
fix(cdr,asterisk): resume MixMonitor after failed attended transfer
Two regressions made the call recording stop at the moment of an attended-transfer attempt and never resume, even when the original A↔B bridge survives — losing ~50% of the conversation audio. Lua dialplan: skip_special_exten() guard was added around transfer_dial_hangup in ec17302 ("fix(upload,cdr,security): chunk size, CEL guard, rate limit whitelist"). That guard short-circuits EXTEN='h', which is exactly the path Asterisk uses for the hangup handler — so event_transfer_dial_hangup() never fired, and the AMI UserEvent never reached WorkerCallEvents. Removed the guard (hangup_chan and hangup_chan_meetme already correctly run guard-free). PHP recovery action: ActionTransferDialHangup::fillNotAnsweredCdr was silently broken in c0a5527 ("Translate comments into English rewrite phpdoc blocks #522") — that commit replaced OR with AND in the CDR filter and substituted agi_channel for the empty dst_chan, so the transfer-attempt row was never closed; it also added an int-vs-string comparison count() !== '1' that always returned early. Filter now matches on transfer="1" AND src_chan=TRANSFERERNAME, and the count check is count() !== 1. The success path (fillLocalChannelCdr) is untouched. CheckSSHConfig: /etc/shadow integrity check now skips when SSHDisablePasswordLogins=true. PAM bookkeeping (lastchg, passwd -l, etc.) mutates shadow even when password-based SSH login is disabled, producing spurious "SSH password changed externally" alerts. tests/Calls infrastructure also gets several fixes used to surface and reproduce the bug: * start.sh::cleanup() no longer runs 'monit stop all' (that locks out the host via dropbear). It stops only asterisk + php-fpm, waits for DB handles to release, removes stale -wal/-shm before restoring the production DB, then brings services back. Without the -wal/-shm cleanup the cp leaves the DB in "malformed" state. * TestCallsBase::getOffPeers() excludes peers with forwarding configured (test 09 was getting ANSWERED CDR via mobile redirect instead of the expected NO_ANSWER) and sorts results for determinism. * Test 09 sample CDR updated for Asterisk 22 behaviour: 2 CDR rows (not 4), and the transfer-attempt duration is ~6s (PJSIP retry window on an unreachable endpoint), not 1s. * db/updateDb.php seed step clears forwarding for extension 504 so the test DB stays valid after regenerations; binary db/mikopbx.db already reflects this state.
1 parent 6897fff commit 88c0574

8 files changed

Lines changed: 100 additions & 10 deletions

File tree

src/Core/Asterisk/Configs/lua/extensions.lua

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1947,7 +1947,13 @@ extensions.lua_dial_create_chan["_.!"] = function() if not skip_special_ext
19471947
extensions.dial_answer["_.!"] = function() if not skip_special_exten() then event_dial_answer() end end
19481948
extensions.lua_transfer_dial_create_chan["_.!"] = function() if not skip_special_exten() then event_transfer_dial_create_chan() end end
19491949
extensions.transfer_dial_answer["_.!"] = function() if not skip_special_exten() then event_transfer_dial_answer() end end
1950-
extensions.transfer_dial_hangup["_.!"] = function() if not skip_special_exten() then event_transfer_dial_hangup() end end
1950+
-- transfer_dial_hangup runs from two paths:
1951+
-- 1) `exten => h,1,Goto(transfer_dial_hangup,h,1)` (hangup handler, EXTEN='h')
1952+
-- 2) `Gosub(transfer_dial_hangup,${EXTEN},1)` (post-Dial, EXTEN=dialed number)
1953+
-- event_transfer_dial_hangup() branches on EXTEN internally and is safe in both cases —
1954+
-- never gate it with skip_special_exten(): that suppressed the 'h' path entirely and
1955+
-- broke MixMonitor resume after failed attended transfers.
1956+
extensions.transfer_dial_hangup["_.!"] = function() event_transfer_dial_hangup() end
19511957
extensions.hangup_chan["_.!"] = function() event_hangup_chan() end
19521958
extensions.queue_start["_.!"] = function() if not skip_special_exten() then event_queue_start() end end
19531959
extensions.queue_answer["_.!"] = function() if not skip_special_exten() then event_queue_answer() end end

src/Core/Workers/Libs/WorkerCallEvents/ActionTransferDialHangup.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,14 @@ private static function fillLocalChannelCdr(WorkerCallEvents $worker, array $dat
126126
*/
127127
private static function fillNotAnsweredCdr(WorkerCallEvents $worker, array $data): void
128128
{
129+
// Close the transfer-attempt CDR. When the transfer target never picked up
130+
// (e.g. CHANUNAVAIL / NO_CONTACTS), the row has transfer=1 and an empty dst_chan —
131+
// matching on src_chan=TRANSFERERNAME is enough to identify it.
129132
$filter = [
130-
'linkedid=:linkedid: AND endtime = "" AND (src_chan=:src_chan: AND dst_chan=:dst_chan:)',
133+
'linkedid=:linkedid: AND endtime = "" AND transfer = "1" AND src_chan = :src_chan:',
131134
'bind' => [
132135
'linkedid' => $data['linkedid'],
133136
'src_chan' => $data['TRANSFERERNAME'],
134-
'dst_chan' => empty($data['dst_chan'])?$data['agi_channel']:$data['dst_chan'],
135137
],
136138
];
137139
/** @var CallDetailRecordsTmp $m_data */
@@ -152,7 +154,8 @@ private static function fillNotAnsweredCdr(WorkerCallEvents $worker, array $data
152154
],
153155
];
154156
$m_data = CallDetailRecordsTmp::find($filter);
155-
if ($m_data->count() !== '1') {
157+
// Phalcon Resultset::count() returns int — compare as int, not against string '1'.
158+
if ($m_data->count() !== 1) {
156159
// The transfer is not completed or channels no longer exist.
157160
return;
158161
}

src/Core/Workers/Libs/WorkerPrepareAdvice/CheckSSHConfig.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,25 @@ class CheckSSHConfig extends Injectable
4242
* The SSH password is now stored as SHA-512 hash in SSH_PASSWORD,
4343
* so we only check if /etc/shadow was modified outside of MikoPBX.
4444
*
45+
* When password-based SSH logins are disabled (SSHDisablePasswordLogins=true),
46+
* /etc/shadow is not security-relevant for SSH access — dropbear accepts
47+
* public-key auth only. PAM bookkeeping (lastchg, locking, system maintenance)
48+
* still mutates /etc/shadow and would otherwise raise spurious alerts.
49+
*
4550
* @return array<string, array<int, array<string, mixed>>> An array containing warning messages.
4651
*/
4752
public function process(): array
4853
{
4954
$messages = [];
55+
56+
$passwordLoginsDisabled = PbxSettings::getValueByKey(
57+
PbxSettings::SSH_DISABLE_SSH_PASSWORD,
58+
false
59+
);
60+
if (filter_var($passwordLoginsDisabled, FILTER_VALIDATE_BOOLEAN)) {
61+
return $messages;
62+
}
63+
5064
$hashFile = PbxSettings::getValueByKey(PbxSettings::SSH_PASSWORD_HASH_FILE, false);
5165

5266
// Check if /etc/shadow was modified outside of MikoPBX

tests/Calls/Scripts/09-call-A-to-B-attended-B-to-offNum/start.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
require_once __DIR__.'/../TestCallsBase.php';
2121

2222
$sampleCDR = [];
23+
// Main A↔B conversation continues after the failed attended-transfer attempt;
24+
// the call recording resumes within the same CDR (single recording file).
2325
$sampleCDR[] = ['src_num'=>'aNum', 'dst_num'=>'bNum', 'duration'=>'14', 'billsec'=>'12', 'fileDuration' => '7'];
24-
$sampleCDR[] = ['src_num'=>'bNum', 'dst_num'=>'offNum','duration'=>'1', 'billsec'=>'0', 'fileDuration' => '0'];
25-
$sampleCDR[] = ['src_num'=>'bNum', 'duration'=>'3', 'billsec'=>'2', 'fileDuration' => '2'];
26-
$sampleCDR[] = ['src_num'=>'aNum', 'duration'=>'5', 'billsec'=>'5', 'fileDuration' => '5'];
26+
// Attended-transfer attempt to an unreachable offNum — separate CDR, NO_ANSWER, no recording.
27+
// Asterisk retries the unreachable PJSIP endpoint for ~6s before giving up.
28+
$sampleCDR[] = ['src_num'=>'bNum', 'dst_num'=>'offNum','duration'=>'6', 'billsec'=>'0', 'fileDuration' => '0'];
2729

2830
TestCallsBase::executeTest($sampleCDR);

tests/Calls/Scripts/TestCallsBase.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
namespace MikoPBX\Tests\Calls\Scripts;
2121
use MikoPBX\Common\Models\CallDetailRecords;
2222
use MikoPBX\Common\Models\CallDetailRecordsTmp;
23+
use MikoPBX\Common\Models\ExtensionForwardingRights;
2324
use MikoPBX\Common\Providers\CDRDatabaseProvider;
2425
use MikoPBX\Core\Asterisk\AsteriskManager;
2526
use MikoPBX\Core\System\Directories;
@@ -89,16 +90,33 @@ public static function getIdlePeers():array{
8990
return $db_data;
9091
}
9192

92-
/** Возвращает недоступные пиры */
93+
/**
94+
* Returns unreachable peers (PJSIP state != OK).
95+
* Skips peers that have any forwarding configured (forwarding / forwardingonbusy /
96+
* forwardingonunavailable). Otherwise test 09 receives an ANSWERED CDR to the
97+
* redirect target instead of the expected NO_ANSWER on the offline endpoint.
98+
*/
9399
public static function getOffPeers():array{
94100
$am = Util::getAstManager('off');
95101
$result = $am->getPjSipPeers();
102+
$forwardedExtensions = [];
103+
$fwdRows = ExtensionForwardingRights::find();
104+
foreach ($fwdRows as $row){
105+
if (!empty($row->forwarding) || !empty($row->forwardingonbusy) || !empty($row->forwardingonunavailable)){
106+
$forwardedExtensions[$row->extension] = true;
107+
}
108+
}
96109
$db_data = array();
97110
foreach ($result as $peer){
98-
if($peer['state']!== 'OK' && is_numeric($peer['id'])){
99-
$db_data[] = $peer['id'];
111+
if($peer['state'] === 'OK' || !is_numeric($peer['id'])){
112+
continue;
113+
}
114+
if(isset($forwardedExtensions[$peer['id']])){
115+
continue;
100116
}
117+
$db_data[] = $peer['id'];
101118
}
119+
sort($db_data, SORT_NUMERIC);
102120
return $db_data;
103121
}
104122

tests/Calls/db/mikopbx.db

0 Bytes
Binary file not shown.

tests/Calls/db/updateDb.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#!/usr/bin/php
22
<?php
33
require_once 'Globals.php';
4+
use MikoPBX\Common\Models\ExtensionForwardingRights;
45
use MikoPBX\Core\Asterisk\Configs\AsteriskConf;
56
use MikoPBX\Core\Asterisk\Configs\ExtensionsConf;
67
use MikoPBX\Core\Asterisk\Configs\FeaturesConf;
@@ -14,6 +15,18 @@
1415

1516
$dbUpdater = new UpdateDatabase();
1617
$dbUpdater->updateDatabaseStructure();
18+
19+
// Test 09 (call-A-to-B-attended-B-to-offNum) needs at least one offline SIP peer
20+
// with NO forwarding configured: otherwise the attended-transfer attempt is redirected
21+
// to the mobile number and the test gets ANSWERED CDR via trunk instead of NO_ANSWER.
22+
// Keep extension 504 forwarding-free regardless of how the test DB was generated.
23+
foreach (ExtensionForwardingRights::find(['extension = :ext:', 'bind' => ['ext' => '504']]) as $row) {
24+
$row->writeAttribute('forwarding', '');
25+
$row->writeAttribute('forwardingonbusy', '');
26+
$row->writeAttribute('forwardingonunavailable', '');
27+
$row->save();
28+
}
29+
1730
SIPConf::reload();;
1831
ExtensionsConf::reload();
1932
ManagerConf::reload();

tests/Calls/start.sh

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,49 @@ function cleanup() {
3838

3939
# Восстанавливаем production базу данных
4040
if [ "$didSwapDb" = "true" ] && [ -f "$dumpConfFile" ]; then
41+
echo_info "Stopping services holding the DB...";
42+
# Stop only the services that hold mikopbx.db open. NEVER call `monit stop all`:
43+
# it brings down dropbear/sshd and locks the host out of remote access.
44+
monit stop asterisk > /dev/null 2>&1 || true;
45+
monit stop php-fpm > /dev/null 2>&1 || true;
46+
47+
# Wait up to ~30s for processes to release the DB.
48+
local i;
49+
for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15; do
50+
if [ -z "$(lsof "$confFile" 2>/dev/null | awk 'NR>1')" ]; then
51+
break;
52+
fi
53+
sleep 2;
54+
done
55+
56+
# Force-kill any stragglers (PHP workers outside monit). Never touches the SSH session.
57+
local stragglers;
58+
stragglers=$(lsof "$confFile" 2>/dev/null | awk 'NR>1 {print $2}' | sort -u);
59+
if [ -n "$stragglers" ]; then
60+
echo "$stragglers" | xargs -r kill -TERM 2>/dev/null || true;
61+
sleep 2;
62+
stragglers=$(lsof "$confFile" 2>/dev/null | awk 'NR>1 {print $2}' | sort -u);
63+
[ -n "$stragglers" ] && echo "$stragglers" | xargs -r kill -KILL 2>/dev/null || true;
64+
fi
65+
4166
echo_info "Restoring production database...";
67+
# Remove stale WAL/SHM from the test DB BEFORE cp.
68+
# Otherwise SQLite will replay the old WAL on top of the restored file and the DB
69+
# ends up "malformed" — exactly the corruption we observed before this fix.
70+
rm -f "${confFile}-wal" "${confFile}-shm";
4271
cp "$dumpConfFile" "$confFile";
72+
sync;
4373

4474
if [ "${debugParams}x" != "x" ]; then
4575
export XDEBUG_CONFIG="${EMPTY}";
4676
fi;
4777
php -f "$dirName/db/updateDb.php" > /dev/null 2>/dev/null;
4878
iptables -F 2>/dev/null; iptables -X 2>/dev/null;
4979
export XDEBUG_CONFIG="${debugParams}";
80+
81+
# Bring services back via monit, named (not `start all`) — see comment above.
82+
monit start php-fpm > /dev/null 2>&1 || true;
83+
monit start asterisk > /dev/null 2>&1 || true;
5084
fi
5185
}
5286
trap cleanup EXIT INT TERM

0 commit comments

Comments
 (0)