[PATCH pmg-api/pmg-docs/pmg-gui v2 0/5] tracking-center: make input-base configurable

[PATCH pmg-api/pmg-docs/pmg-gui v2 0/5] tracking-center: make input-base configurable

[Stoiko Ivanov]

v1->v2:
* address Thomas feedback (thx!!): add patch 1/5 to pmg-api to mark
  config-options as root_only. (and the change to 1/5 to set
  'root_only' for the 'log-tracker-base' parameter
  all other patches remain as they were.

It's been a good while since I looked through the SectionConfig code - so
some feedback by someone more familiar with it would be appreciated (maybe
Max?)

minimally tested it by logging in as root@pam, and regular admin user
and trying to change the log-tracker-base option (and others).

cover-letter and discussion for v1:
https://lore.proxmox.com/pmg-devel/20260608112952.679449-1-s.ivanov@proxmox.com/T/#t


pmg-api:

Stoiko Ivanov (3):
  config: add root_only paramter option.
  config: add log-tracker-base key
  fix #3657: api: tracking center: explicitly set input base from config

 src/PMG/API2/Config.pm      | 10 +++++++++-
 src/PMG/API2/MailTracker.pm |  5 +++++
 src/PMG/Config.pm           |  7 +++++++
 3 files changed, 21 insertions(+), 1 deletion(-)


pmg-gui:

Stoiko Ivanov (1):
  fix #3657: system options: add log-tracker-base textrow

 js/SystemOptions.js | 6 ++++++
 1 file changed, 6 insertions(+)


pmg-docs:

Stoiko Ivanov (1):
  pmg-log-tracker: mention that input-base can be configured via GUI

 pmg-log-tracker.adoc | 3 +++
 1 file changed, 3 insertions(+)


Summary over all repositories:
  5 files changed, 30 insertions(+), 1 deletions(-)

-- 
Generated by murpp 0.12.0

[PATCH pmg-api v2 1/5] config: add root_only paramter option.

[Stoiko Ivanov]

this introduces a way to have some options in our pmg.conf
SectionConfig restricted to 'root@pam' only.

I skimmed the (quite improved) documentation in PVE::SectionConfig,
and did not see any blockers to extending adding other options instead
of 'optional' and 'fixed' there.

This is needed to have unified way to check if a particular option
shoudld be restricted to 'root@pam' only.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/Config.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/PMG/API2/Config.pm b/src/PMG/API2/Config.pm
index 9cce95dd..6edde9b4 100644
--- a/src/PMG/API2/Config.pm
+++ b/src/PMG/API2/Config.pm
@@ -10,6 +10,7 @@ use HTTP::Status qw(:constants);
 use Storable qw(dclone);
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::RESTHandler;
+use PVE::Exception qw(raise_perm_exc);
 use Time::HiRes qw();
 
 use PMG::Config;
@@ -194,14 +195,21 @@ my $api_update_config_section = sub {
         die "no options specified\n"
             if !$delete_str && !scalar(keys %$param);
 
+        my $plugin = PMG::Config::Base->lookup($section);
+        my $rpcenv = PMG::RESTEnvironment->get();
+        my $authuser = $rpcenv->get_user();
+
         foreach my $opt (PVE::Tools::split_list($delete_str)) {
+            my $is_root_only = $plugin->options()->{$opt}->{root_only};
+            raise_perm_exc() if ( $is_root_only && $authuser ne 'root@pam') ;
             delete $ids->{$section}->{$opt};
         }
 
-        my $plugin = PMG::Config::Base->lookup($section);
         my $config = $plugin->check_config($section, $param, 0, 1);
 
         foreach my $p (keys %$config) {
+            my $is_root_only = $plugin->options()->{$p}->{root_only};
+            raise_perm_exc() if ( $is_root_only && $authuser ne 'root@pam') ;
             $ids->{$section}->{$p} = $config->{$p};
         }
 
-- 
2.47.3

[PATCH pmg-api v2 2/5] config: add log-tracker-base key

[Stoiko Ivanov]

makes it possible to override the input-base parameter for
pmg-log-tracker - functionality there was added in:
9816d19 ("fix #3657: allow scanning a configurable rotated log series")

the option is restricted to root@pam, as suggested by Thomas, since
enabling other admin users to open arbitrary files on the system might
leak information, in case the parser in pmg-log-tracker matches
something unexpected. To err on the side of caution - restrict it to
root@pam, as we can always make it more liberal in the future, while
restricting it later could break some users workflows.

the pattern allows for a input-base with a maximal depth of 6
directories, which should cover all needs.

the default in the config is /var/log/syslog and will be explicitly
provided to pmg-log-tracker if nothing is set in pmg.conf

exposing the option in pmg.conf enables users to use a different
log location for the daily work.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Config.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
index f48c31dc..fbe2f3cb 100644
--- a/src/PMG/Config.pm
+++ b/src/PMG/Config.pm
@@ -161,6 +161,12 @@ EODESC
             maxLength => 64 * 1024,
             default => '',
         },
+        'log-tracker-base' => {
+            description => "Location of rotated mail logs, input-base argument for pmg-log-tracker",
+            type => 'string',
+            pattern => '^/([^/\0]+\/){0,6}[^/\0]+$',
+            default => '/var/log/syslog',
+        },
     };
 }
 
@@ -182,6 +188,7 @@ sub options {
         'dkim-use-domain' => { optional => 1 },
         'admin-mail-from' => { optional => 1 },
         'consent-text' => { optional => 1 },
+        'log-tracker-base' => { optional => 1, root_only => 1 },
     };
 }
 
-- 
2.47.3

[PATCH pmg-api v2 3/5] fix #3657: api: tracking center: explicitly set input base from config

[Stoiko Ivanov]

makes it possible to override the location of logs which are scanned
by pmg-log-tracker.
This should enable admins, to either point the log-tracker to another
location on disk where e.g. a rsyslog daemon combines the logs of
multiple instances, or to only log the mail-facility somewhere and
drop /var/log/syslog logging in favor of journalling.

pmg-log-tracker still uses syslog-files over the journal, as this was
far more performant when we last compared log-file reading to reading
from the journal.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/MailTracker.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/PMG/API2/MailTracker.pm b/src/PMG/API2/MailTracker.pm
index 1cc93ef2..5c175300 100644
--- a/src/PMG/API2/MailTracker.pm
+++ b/src/PMG/API2/MailTracker.pm
@@ -41,6 +41,11 @@ my $statmap = {
 my $run_pmg_log_tracker = sub {
     my ($args, $includelog) = @_;
 
+    my $pmg_cfg = PMG::Config->new();
+    my $input_base = $pmg_cfg->get('admin', 'log-tracker-base');
+
+    push @$args, '--input-base', $input_base;
+
     my $logids = {};
 
     if (defined(my $id = $includelog)) {
-- 
2.47.3

[PATCH pmg-gui v2 4/5] fix #3657: system options: add log-tracker-base textrow

[Stoiko Ivanov]

the system options panel seems like the best place to set the location
of the logs.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 js/SystemOptions.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/js/SystemOptions.js b/js/SystemOptions.js
index 92dc5d0..b7d20fd 100644
--- a/js/SystemOptions.js
+++ b/js/SystemOptions.js
@@ -91,6 +91,12 @@ Ext.define('PMG.SystemOptions', {
             renderer: Ext.htmlEncode,
         });
 
+        me.add_text_row('log-tracker-base', gettext("Maillog base name"), {
+            deleteEmpty: true,
+            defaultValue: '/var/log/syslog',
+            renderer: Ext.htmlEncode,
+        });
+
         me.add_proxy_row('http_proxy', gettext('HTTP proxy'));
 
         me.add_textareafield_row('consent-text', gettext('Consent Text'), {
-- 
2.47.3

[PATCH pmg-docs v2 5/5] pmg-log-tracker: mention that input-base can be configured via GUI

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 pmg-log-tracker.adoc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pmg-log-tracker.adoc b/pmg-log-tracker.adoc
index 60cc6b9..ba182ed 100644
--- a/pmg-log-tracker.adoc
+++ b/pmg-log-tracker.adoc
@@ -22,6 +22,9 @@ Prefer this over `-i` when the system logs mail events to a dedicated file and
 you want to cover rotated logs as well. Compression is detected per file, so
 custom logrotate compression settings are handled.
 
+You can set the `--input-base` parameter used for the Tracking Center at
+__Configuration -> Options -> Maillog base name__.
+
 Start time `-s` and end time `-e` are optional. By default the end time will be
 the current time and the start time will be 0:00 of the current day.
 
-- 
2.47.3

Re: [PATCH pmg-api v2 1/5] config: add root_only paramter option.

[Max R. Carrara]

On Tue Jun 9, 2026 at 10:04 PM CEST, Stoiko Ivanov wrote:
> this introduces a way to have some options in our pmg.conf
> SectionConfig restricted to 'root@pam' only.
>
> I skimmed the (quite improved) documentation in PVE::SectionConfig,

😇

> and did not see any blockers to extending adding other options instead
> of 'optional' and 'fixed' there.

Yeah, this should be fine, especially for such relatively small things.

The SectionConfig code doesn't really care about any other keys, i.e. it
leaves them untouched.

Also, I want to mention that it's good that it's in `options()` anyway,
because even though adding an equivalent key to the property itself in
`properties()` would've also worked, it would've nevertheless been more
tricky to handle. (We have a separate getter method for acquiring a
property's schema that you'd have to call.)

>
> This is needed to have unified way to check if a particular option
> shoudld be restricted to 'root@pam' only.

s/shoudld/should

>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/API2/Config.pm | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/PMG/API2/Config.pm b/src/PMG/API2/Config.pm
> index 9cce95dd..6edde9b4 100644
> --- a/src/PMG/API2/Config.pm
> +++ b/src/PMG/API2/Config.pm
> @@ -10,6 +10,7 @@ use HTTP::Status qw(:constants);
>  use Storable qw(dclone);
>  use PVE::JSONSchema qw(get_standard_option);
>  use PVE::RESTHandler;
> +use PVE::Exception qw(raise_perm_exc);
>  use Time::HiRes qw();
>
>  use PMG::Config;
> @@ -194,14 +195,21 @@ my $api_update_config_section = sub {
>          die "no options specified\n"
>              if !$delete_str && !scalar(keys %$param);
>
> +        my $plugin = PMG::Config::Base->lookup($section);
> +        my $rpcenv = PMG::RESTEnvironment->get();
> +        my $authuser = $rpcenv->get_user();
> +
>          foreach my $opt (PVE::Tools::split_list($delete_str)) {
> +            my $is_root_only = $plugin->options()->{$opt}->{root_only};

I would maaaybe rename the key from root_only to 'root-only' (quotes
included), since that's the newer convention we're using.
(Note that this isn't mentioned in our Perl style guide (yet?) FWICT, so
it's your call whether you want to rename it or not, tbh.)

> +            raise_perm_exc() if ( $is_root_only && $authuser ne 'root@pam') ;
>              delete $ids->{$section}->{$opt};
>          }
>
> -        my $plugin = PMG::Config::Base->lookup($section);
>          my $config = $plugin->check_config($section, $param, 0, 1);
>
>          foreach my $p (keys %$config) {
> +            my $is_root_only = $plugin->options()->{$p}->{root_only};

Here as well, if you do decide to change it.

> +            raise_perm_exc() if ( $is_root_only && $authuser ne 'root@pam') ;
>              $ids->{$section}->{$p} = $config->{$p};
>          }
>

Re: [PATCH pmg-api v2 2/5] config: add log-tracker-base key

[Max R. Carrara]

On Tue Jun 9, 2026 at 10:04 PM CEST, Stoiko Ivanov wrote:
> makes it possible to override the input-base parameter for
> pmg-log-tracker - functionality there was added in:
> 9816d19 ("fix #3657: allow scanning a configurable rotated log series")
>
> the option is restricted to root@pam, as suggested by Thomas, since
> enabling other admin users to open arbitrary files on the system might
> leak information, in case the parser in pmg-log-tracker matches
> something unexpected. To err on the side of caution - restrict it to
> root@pam, as we can always make it more liberal in the future, while
> restricting it later could break some users workflows.
>
> the pattern allows for a input-base with a maximal depth of 6
> directories, which should cover all needs.

Mmh, somewhat arbitrary, no? I don't think we should allow an infinite
amount of directories, but just to play it safe, I think we should
relax this restriction here -- perhaps a depth of 127 or 255?

>
> the default in the config is /var/log/syslog and will be explicitly
> provided to pmg-log-tracker if nothing is set in pmg.conf
>
> exposing the option in pmg.conf enables users to use a different
> log location for the daily work.
>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/Config.pm | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
> index f48c31dc..fbe2f3cb 100644
> --- a/src/PMG/Config.pm
> +++ b/src/PMG/Config.pm
> @@ -161,6 +161,12 @@ EODESC
>              maxLength => 64 * 1024,
>              default => '',
>          },
> +        'log-tracker-base' => {
> +            description => "Location of rotated mail logs, input-base argument for pmg-log-tracker",
> +            type => 'string',
> +            pattern => '^/([^/\0]+\/){0,6}[^/\0]+$',

This regex here actually gets a lot of things right, but misses one
crucial thing: It does not prevent parent directory references from
occurring inside the pattern. So, in the very unlikely scenario that
somebody is able to inject a maliciously-formed path here, they could
theoretically access arbitrary files on the system, e.g.:

  /var/log/syslog/../../../etc/pmg/pmg-csrf.key

.. would resolve to /etc/pmg/pmg-csrf.key.

Again, this is probably very unlikely to happen -- but we should
sanitize the input here nevertheless.

So, we'll need a validation sub instead of a pattern here in order to be
able to check for parent directory references [validation].

> +            default => '/var/log/syslog',
> +        },
>      };
>  }
>
> @@ -182,6 +188,7 @@ sub options {
>          'dkim-use-domain' => { optional => 1 },
>          'admin-mail-from' => { optional => 1 },
>          'consent-text' => { optional => 1 },
> +        'log-tracker-base' => { optional => 1, root_only => 1 },

If you do decide to rename it to 'root-only', don't forget this spot
here too.

>      };
>  }
>

[validation]: https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/Storage/Plugin.pm;h=4f69f9b5db69674335eb3024d61d4a3430bca1ec;hb=refs/heads/master#l331

Re: [PATCH pmg-gui v2 4/5] fix #3657: system options: add log-tracker-base textrow

[Max R. Carrara]

On Tue Jun 9, 2026 at 10:04 PM CEST, Stoiko Ivanov wrote:
> the system options panel seems like the best place to set the location
> of the logs.
>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  js/SystemOptions.js | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/js/SystemOptions.js b/js/SystemOptions.js
> index 92dc5d0..b7d20fd 100644
> --- a/js/SystemOptions.js
> +++ b/js/SystemOptions.js
> @@ -91,6 +91,12 @@ Ext.define('PMG.SystemOptions', {
>              renderer: Ext.htmlEncode,
>          });
>
> +        me.add_text_row('log-tracker-base', gettext("Maillog base name"), {
> +            deleteEmpty: true,
> +            defaultValue: '/var/log/syslog',
> +            renderer: Ext.htmlEncode,
> +        });
> +

Even though it's kind of superfluous, perhaps the config option could be
greyed out or disabled in some way if the currently logged in user isn't
root@pam? Only a small thing though -- which probably depends on
personal taste as well -- so no hard feelings if this stays as it is.

>          me.add_proxy_row('http_proxy', gettext('HTTP proxy'));
>
>          me.add_textareafield_row('consent-text', gettext('Consent Text'), {

Re: [PATCH pmg-docs v2 5/5] pmg-log-tracker: mention that input-base can be configured via GUI

[Max R. Carrara]

On Tue Jun 9, 2026 at 10:04 PM CEST, Stoiko Ivanov wrote:
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  pmg-log-tracker.adoc | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/pmg-log-tracker.adoc b/pmg-log-tracker.adoc
> index 60cc6b9..ba182ed 100644
> --- a/pmg-log-tracker.adoc
> +++ b/pmg-log-tracker.adoc
> @@ -22,6 +22,9 @@ Prefer this over `-i` when the system logs mail events to a dedicated file and
>  you want to cover rotated logs as well. Compression is detected per file, so
>  custom logrotate compression settings are handled.
>
> +You can set the `--input-base` parameter used for the Tracking Center at
> +__Configuration -> Options -> Maillog base name__.
> +

While this is fine by itself, we should probably also mention this in
4.5.2. Options and elaborate on what the config option does (not the
--input-base CLI parameter, as that is done here already). Basically
what we do for the other options in 4.5.2.

>  Start time `-s` and end time `-e` are optional. By default the end time will be
>  the current time and the start time will be 0:00 of the current day.
>

Re: [PATCH pmg-api/pmg-docs/pmg-gui v2 0/5] tracking-center: make input-base configurable

[Max R. Carrara]

On Tue Jun 9, 2026 at 10:04 PM CEST, Stoiko Ivanov wrote:
> v1->v2:
> * address Thomas feedback (thx!!): add patch 1/5 to pmg-api to mark
>   config-options as root_only. (and the change to 1/5 to set
>   'root_only' for the 'log-tracker-base' parameter
>   all other patches remain as they were.
>
> It's been a good while since I looked through the SectionConfig code - so
> some feedback by someone more familiar with it would be appreciated (maybe
> Max?)
>
> minimally tested it by logging in as root@pam, and regular admin user
> and trying to change the log-tracker-base option (and others).
>
> cover-letter and discussion for v1:
> https://lore.proxmox.com/pmg-devel/20260608112952.679449-1-s.ivanov@proxmox.com/T/#t
>
> [...]

Overall LGTM -- a few comments inline.

Works as advertised; verified that the config option can be changed by
root only and that other admin users can't change it. Also tested this
with a user with the "Helpdesk" role, just for good measure.

Overall pretty neat! Will add my trailers upon refresh.