From 069b89629e06754b68643b6561d8361a1ddb2a64 Mon Sep 17 00:00:00 2001 From: FreddleSpl0it <75116288+FreddleSpl0it@users.noreply.github.com> Date: Fri, 17 Jul 2026 20:45:46 +0200 Subject: [PATCH] [Imapsync] Use dropdown for custom params --- data/Dockerfiles/dovecot/imapsync_runner.pl | 31 +++---- data/web/api/openapi.yaml | 50 ++++++++++- data/web/inc/functions.syncjob.inc.php | 93 ++++++++------------- data/web/inc/init_db.inc.php | 26 +++++- data/web/inc/vars.inc.php | 32 ------- data/web/js/build/013-mailcow.js | 85 +++++++++++++++++++ data/web/json_api.php | 4 + data/web/lang/lang.de-de.json | 6 +- data/web/lang/lang.en-gb.json | 6 +- data/web/templates/edit/syncjob.twig | 10 ++- data/web/templates/modals/mailbox.twig | 10 ++- data/web/templates/modals/user.twig | 9 +- 12 files changed, 237 insertions(+), 125 deletions(-) diff --git a/data/Dockerfiles/dovecot/imapsync_runner.pl b/data/Dockerfiles/dovecot/imapsync_runner.pl index 7f794a111..35610c93c 100644 --- a/data/Dockerfiles/dovecot/imapsync_runner.pl +++ b/data/Dockerfiles/dovecot/imapsync_runner.pl @@ -5,6 +5,7 @@ use LockFile::Simple qw(lock trylock unlock); use Proc::ProcessTable; use Data::Dumper qw(Dumper); use IPC::Run 'run'; +use JSON::PP; use File::Temp; use Try::Tiny; use POSIX (); @@ -12,23 +13,6 @@ use sigtrap 'handler' => \&sig_handler, qw(INT TERM KILL QUIT); sub trim { my $s = shift; $s =~ s/^\s+|\s+$//g; return $s }; -sub qqw($) { - my @params = (); - my @values = split(/(?=--)/, $_[0]); - foreach my $val (@values) { - my @tmpparam = split(/ /, $val, 2); - foreach my $tmpval (@tmpparam) { - if ($tmpval ne '') { - push @params, $tmpval; - } - } - } - foreach my $val (@params) { - $val=trim($val); - } - return @params; -} - # Move a finished job to the back of the queue, keeping order_id contiguous (1..N). # Called only from the parent after reaping a child, so all rotations are serialized. sub rotate_to_back { @@ -214,7 +198,18 @@ foreach my $row (@rows) { print $passfile2 trim($master_pass) . "\n"; - my @custom_params_a = qqw($custom_params); + # Structured custom params: JSON array of {o,v}. Build one argv token per pair as "--opt=value" + # (value glued to its option) so a value can never become a separate option; passed via + # IPC::Run list form (execvp, no shell). Only allowlisted option names are stored (validated on write). + my @custom_params_a; + my $cp_pairs = eval { decode_json(defined($custom_params) && $custom_params ne '' ? $custom_params : '[]') }; + if (ref($cp_pairs) eq 'ARRAY') { + for my $p (@$cp_pairs) { + next unless ref($p) eq 'HASH' && defined $p->{o} && $p->{o} ne ''; + (my $ov = defined($p->{v}) ? $p->{v} : '') =~ s/\x00//g; + push @custom_params_a, ($ov eq '' ? "--$p->{o}" : "--$p->{o}=$ov"); + } + } my $custom_params_ref = \@custom_params_a; # Effective per-process bandwidth cap (bytes/s): per-job value, capped by the global limit. diff --git a/data/web/api/openapi.yaml b/data/web/api/openapi.yaml index 06ef29ea6..076ac3f28 100644 --- a/data/web/api/openapi.yaml +++ b/data/web/api/openapi.yaml @@ -1520,7 +1520,7 @@ paths: timeout1: "600" timeout2: "600" exclude: "(?i)spam|(?i)junk" - custom_params: "--dry" + custom_params: '[{"o":"dry","v":""}]' delete2duplicates: "1" delete1: "1" delete2: "0" @@ -1569,7 +1569,13 @@ paths: description: exclude objects (regex) type: string custom_params: - description: custom parameters + description: >- + JSON array of allowlisted imapsync options as + option/value pairs, e.g. + `[{"o":"folder","v":"INBOX"},{"o":"dry","v":""}]`. Only + option names from `GET /get/syncjob_options` are accepted; + values may contain any character and are passed as a single + argument. type: string delete2duplicates: description: delete duplicates on destination (--delete2duplicates) @@ -4063,7 +4069,13 @@ paths: etc.) type: boolean custom_params: - description: Custom parameters passed to imapsync command + description: >- + JSON array of allowlisted imapsync options as + option/value pairs, e.g. + `[{"o":"folder","v":"INBOX"},{"o":"dry","v":""}]`. Only + option names from `GET /get/syncjob_options` are + accepted; values may contain any character and are + passed as a single argument. type: string delete1: description: Delete from source when completed @@ -6216,6 +6228,38 @@ paths: flags — they are never returned over the JSON API. operationId: Get sync job sources summary: Get sync job sources + /api/v1/get/syncjob_options: + get: + parameters: + - description: e.g. api-key-string + example: api-key-string + in: header + name: X-API-Key + required: false + schema: + type: string + responses: + "401": + $ref: "#/components/responses/Unauthorized" + "200": + content: + application/json: + examples: + response: + value: + - dry + - folder + - delete2folders + - maxsize + description: OK + headers: {} + tags: + - Sync jobs + description: >- + Returns the allowlist of imapsync option names accepted in a sync job's + `custom_params`. Used by the UI to populate the option autocompletion. + operationId: Get sync job options + summary: Get sync job options "/api/v1/get/tls-policy-map/{id}": get: parameters: diff --git a/data/web/inc/functions.syncjob.inc.php b/data/web/inc/functions.syncjob.inc.php index cb8bb7129..557606723 100644 --- a/data/web/inc/functions.syncjob.inc.php +++ b/data/web/inc/functions.syncjob.inc.php @@ -360,6 +360,21 @@ function imapsync_get_settings() { return $settings; } +function imapsync_normalize_custom_params($input) { + $pairs = json_decode((string)$input, true); + if (!is_array($pairs)) return '[]'; + $allow = $GLOBALS['IMAPSYNC_OPTIONS']['whitelist']; + $out = array(); + foreach ($pairs as $p) { + if (!is_array($p)) continue; + $o = ltrim(strtolower(trim((string)($p['o'] ?? ''))), '-'); + if ($o === '') continue; + if (!in_array($o, $allow, true)) return false; // disallowed option -> reject the whole set + $out[] = array('o' => $o, 'v' => str_replace("\0", '', (string)($p['v'] ?? ''))); + } + return json_encode($out); +} + function imapsync_set_setting($name, $value) { // Admin-only writer for a global syncjob setting. global $pdo; @@ -508,36 +523,15 @@ function syncjob($_action, $_type, $_data = null, $_extra = null) { return false; } - // validate custom params - foreach (explode('-', $custom_params) as $param){ - if(empty($param)) continue; - - // extract option - if (str_contains($param, '=')) $param = explode('=', $param)[0]; - else $param = rtrim($param, ' '); - // remove first char if first char is - - if ($param[0] == '-') $param = ltrim($param, $param[0]); - - if (str_contains($param, ' ')) { - // bad char - $_SESSION['return'][] = array( - 'type' => 'danger', - 'log' => array(__FUNCTION__, $_action, $_type, $_data_log, $_attr), - 'msg' => 'bad character SPACE' - ); - return false; - } - - // check if param is whitelisted - if (!in_array(strtolower($param), $GLOBALS["IMAPSYNC_OPTIONS"]["whitelist"])){ - // bad option - $_SESSION['return'][] = array( - 'type' => 'danger', - 'log' => array(__FUNCTION__, $_action, $_type, $_data_log, $_attr), - 'msg' => 'bad option '. $param - ); - return false; - } + // Structured custom params: validate option names against the allowlist, store as JSON pairs + $custom_params = imapsync_normalize_custom_params($custom_params); + if ($custom_params === false) { + $_SESSION['return'][] = array( + 'type' => 'danger', + 'log' => array(__FUNCTION__, $_action, $_type, $_data_log, $_attr), + 'msg' => 'imapsync_custom_param_invalid' + ); + return false; } if (empty($subfolder2)) { $subfolder2 = ""; @@ -879,36 +873,15 @@ function syncjob($_action, $_type, $_data = null, $_extra = null) { $password1 = ''; } - // validate custom params - foreach (explode('-', $custom_params) as $param){ - if(empty($param)) continue; - - // extract option - if (str_contains($param, '=')) $param = explode('=', $param)[0]; - else $param = rtrim($param, ' '); - // remove first char if first char is - - if ($param[0] == '-') $param = ltrim($param, $param[0]); - - if (str_contains($param, ' ')) { - // bad char - $_SESSION['return'][] = array( - 'type' => 'danger', - 'log' => array(__FUNCTION__, $_action, $_type, $_data_log, $_attr), - 'msg' => 'bad character SPACE' - ); - return false; - } - - // check if param is whitelisted - if (!in_array(strtolower($param), $GLOBALS["IMAPSYNC_OPTIONS"]["whitelist"])){ - // bad option - $_SESSION['return'][] = array( - 'type' => 'danger', - 'log' => array(__FUNCTION__, $_action, $_type, $_data_log, $_attr), - 'msg' => 'bad option '. $param - ); - return false; - } + // Structured custom params: validate option names against the allowlist, store as JSON pairs + $custom_params = imapsync_normalize_custom_params($custom_params); + if ($custom_params === false) { + $_SESSION['return'][] = array( + 'type' => 'danger', + 'log' => array(__FUNCTION__, $_action, $_type, $_data_log, $_attr), + 'msg' => 'imapsync_custom_param_invalid' + ); + continue; } if (empty($subfolder2)) { $subfolder2 = ""; diff --git a/data/web/inc/init_db.inc.php b/data/web/inc/init_db.inc.php index 0751c511d..5c0cc8cef 100644 --- a/data/web/inc/init_db.inc.php +++ b/data/web/inc/init_db.inc.php @@ -4,7 +4,7 @@ function init_db_schema() try { global $pdo; - $db_version = "16072026_1000"; + $db_version = "17072026_1000"; $stmt = $pdo->query("SHOW TABLES LIKE 'versions'"); $num_results = count($stmt->fetchAll(PDO::FETCH_ASSOC)); @@ -886,7 +886,7 @@ function init_db_schema() "delete2" => "TINYINT(1) NOT NULL DEFAULT '0'", "automap" => "TINYINT(1) NOT NULL DEFAULT '0'", "skipcrossduplicates" => "TINYINT(1) NOT NULL DEFAULT '0'", - "custom_params" => "VARCHAR(512) NOT NULL DEFAULT ''", + "custom_params" => "TEXT", "timeout1" => "SMALLINT NOT NULL DEFAULT '600'", "timeout2" => "SMALLINT NOT NULL DEFAULT '600'", "subscribeall" => "TINYINT(1) NOT NULL DEFAULT '1'", @@ -1533,6 +1533,28 @@ function init_db_schema() $pdo->query("UPDATE `imapsync` SET `order_id` = (@r := @r + 1) ORDER BY `id`"); } + // Syncjobs: migrate custom_params from raw imapsync string to structured JSON pairs. + // Old values could not contain spaces (they were rejected), so a whitespace split is safe. + $allow = $GLOBALS['IMAPSYNC_OPTIONS']['whitelist']; + $cp_upd = $pdo->prepare("UPDATE `imapsync` SET `custom_params` = :cp WHERE `id` = :id"); + foreach ($pdo->query("SELECT `id`, `custom_params` FROM `imapsync`")->fetchAll(PDO::FETCH_ASSOC) as $r) { + $cp = trim((string)$r['custom_params']); + if ($cp !== '' && $cp[0] === '[') continue; // already migrated (JSON) + $pairs = array(); + if ($cp !== '') { + $tokens = preg_split('/\s+/', $cp, -1, PREG_SPLIT_NO_EMPTY); + for ($i = 0; $i < count($tokens); $i++) { + if (strpos($tokens[$i], '--') !== 0) continue; + $opt = ltrim($tokens[$i], '-'); $val = ''; + if (strpos($opt, '=') !== false) { list($opt, $val) = explode('=', $opt, 2); } + elseif ($i + 1 < count($tokens) && strpos($tokens[$i + 1], '--') !== 0) { $val = $tokens[++$i]; } + if (!in_array(strtolower($opt), $allow)) continue; // drop options no longer allowed + $pairs[] = array('o' => strtolower($opt), 'v' => $val); + } + } + $cp_upd->execute(array(':cp' => json_encode($pairs), ':id' => $r['id'])); + } + // Inject admin if not exists $stmt = $pdo->query("SELECT NULL FROM `admin`"); $num_results = count($stmt->fetchAll(PDO::FETCH_ASSOC)); diff --git a/data/web/inc/vars.inc.php b/data/web/inc/vars.inc.php index 44ba2a758..5902768f9 100644 --- a/data/web/inc/vars.inc.php +++ b/data/web/inc/vars.inc.php @@ -285,10 +285,7 @@ $IMAPSYNC_OPTIONS = array( 'whitelist' => array( 'abort', 'authmd51', - 'authmd52', - 'authmech2', 'authuser1', - 'authuser2', 'debug', 'debugcontent', 'debugcrossduplicates', @@ -296,20 +293,15 @@ $IMAPSYNC_OPTIONS = array( 'debugfolders', 'debugimap', 'debugimap1', - 'debugimap2', 'debugmemory', 'debugssl', 'delete1emptyfolders', 'delete2folders', 'disarmreadreceipts', 'domain1', - 'domain2', 'domino1', - 'domino2', - 'dry', 'errorsmax', 'exchange1', - 'exchange2', 'exitwhenover', 'expunge1', 'f1f2', @@ -319,7 +311,6 @@ $IMAPSYNC_OPTIONS = array( 'folderlast', 'folderrec', 'gmail1', - 'gmail2', 'idatefromheader', 'include', 'inet4', @@ -329,10 +320,7 @@ $IMAPSYNC_OPTIONS = array( 'justfoldersizes', 'justlogin', 'keepalive1', - 'keepalive2', 'log', - 'logdir', - 'logfile', 'maxbytesafter', 'maxlinelength', 'maxmessagespersecond', @@ -342,43 +330,24 @@ $IMAPSYNC_OPTIONS = array( 'minsize', 'noabletosearch', 'noabletosearch1', - 'noabletosearch2', 'noexpunge1', - 'noexpunge2', 'nofoldersizesatend', 'noid', 'nolog', 'nomixfolders', 'noresyncflags', 'nossl1', - 'nossl2', 'nosyncacls', 'notls1', - 'notls2', - 'nouidexpunge2', 'nousecache', - 'oauthaccesstoken2', - 'oauthdirect2', 'office1', - 'office2', - 'pidfile', - 'pidfilelocking', 'prefix1', - 'prefix2', - 'proxyauth1', - 'proxyauth2', 'resyncflags', 'resynclabels', 'search', 'search1', - 'search2', 'sep1', - 'sep2', - 'showpasswords', 'skipemptyfolders', - 'ssl2', - 'sslargs1', - 'sslargs2', 'subfolder1', 'subscribe', 'subscribed', @@ -389,7 +358,6 @@ $IMAPSYNC_OPTIONS = array( 'tests', 'testslive', 'testslive6', - 'tls2', 'truncmess', 'usecache', 'useheader', diff --git a/data/web/js/build/013-mailcow.js b/data/web/js/build/013-mailcow.js index 7428778a1..a5420e73e 100644 --- a/data/web/js/build/013-mailcow.js +++ b/data/web/js/build/013-mailcow.js @@ -480,7 +480,88 @@ function imapsyncSyncSourceOauthToggle($form) { $form.find('.imapsync-source-redirect-uri').val(window.location.origin + '/syncjob-oauth'); } +// Allowlisted imapsync option names for the custom-params selects. Loaded once and cached. +var imapsyncCpOptions = null; +function imapsyncCpEnsureOptions(cb) { + if (imapsyncCpOptions !== null) { cb(); return; } + $.get("/api/v1/get/syncjob_options", function(opts) { + imapsyncCpOptions = Array.isArray(opts) ? opts : []; + cb(); + }); +} + +// Append one row to an editor; option is a bootstrap-select (not free-text), value is arbitrary. +function imapsyncCpAddRow($editor, o, v) { + var $row = $( + '