From 7cc350b40d54f71af12deb0318c34d8e6b6ec90f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Lesimple?=
 <stephane.lesimple+bastion@ovhcloud.com>
Date: Wed, 15 Dec 2021 17:11:03 +0000
Subject: [PATCH] chore: check for spurious args in all helpers

---
 bin/helper/osh-accountAddGroupServer       | 2 ++
 bin/helper/osh-accountCreate               | 2 ++
 bin/helper/osh-accountDelete               | 2 ++
 bin/helper/osh-accountGeneratePassword     | 2 ++
 bin/helper/osh-accountGetPasswordInfo      | 2 ++
 bin/helper/osh-accountListEgressKeys       | 2 ++
 bin/helper/osh-accountListIngressKeys      | 2 ++
 bin/helper/osh-accountListPasswords        | 2 ++
 bin/helper/osh-accountMFAResetPassword     | 2 ++
 bin/helper/osh-accountMFAResetTOTP         | 2 ++
 bin/helper/osh-accountModify               | 2 ++
 bin/helper/osh-accountModifyCommand        | 2 ++
 bin/helper/osh-accountModifyPersonalAccess | 2 ++
 bin/helper/osh-accountPIV                  | 2 ++
 bin/helper/osh-accountUnexpire             | 2 ++
 bin/helper/osh-adminMaintenance            | 2 ++
 bin/helper/osh-groupAddServer              | 2 ++
 bin/helper/osh-groupAddSymlinkToAccount    | 2 ++
 bin/helper/osh-groupCreate                 | 2 ++
 bin/helper/osh-groupDelEgressKey           | 2 ++
 bin/helper/osh-groupDelete                 | 2 ++
 bin/helper/osh-groupGenerateEgressKey      | 2 ++
 bin/helper/osh-groupGeneratePassword       | 2 ++
 bin/helper/osh-groupModify                 | 2 ++
 bin/helper/osh-groupSetRole                | 2 ++
 bin/helper/osh-selfMFASetupPassword        | 2 ++
 bin/helper/osh-selfMFASetupTOTP            | 2 ++
 bin/plugin/restricted/accountCreate        | 6 +++---
 lib/perl/OVH/Bastion/Helper.pm             | 9 +++++++++
 29 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/bin/helper/osh-accountAddGroupServer b/bin/helper/osh-accountAddGroupServer
index 7c0f459..c5b180b 100755
--- a/bin/helper/osh-accountAddGroupServer
+++ b/bin/helper/osh-accountAddGroupServer
@@ -39,6 +39,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $action or not $ip or not $account or not $group) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'action' or 'ip' or 'account' or 'group'");
 }
diff --git a/bin/helper/osh-accountCreate b/bin/helper/osh-accountCreate
index ac3bb99..a74b633 100755
--- a/bin/helper/osh-accountCreate
+++ b/bin/helper/osh-accountCreate
@@ -40,6 +40,8 @@ eval {
 };
 if ($@) { die $@ }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$result) {
     local $" = ", ";
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
diff --git a/bin/helper/osh-accountDelete b/bin/helper/osh-accountDelete
index 98af9ba..7537a9f 100755
--- a/bin/helper/osh-accountDelete
+++ b/bin/helper/osh-accountDelete
@@ -35,6 +35,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account || !$type) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account' or 'type'");
 }
diff --git a/bin/helper/osh-accountGeneratePassword b/bin/helper/osh-accountGeneratePassword
index 5d04cae..876bd38 100755
--- a/bin/helper/osh-accountGeneratePassword
+++ b/bin/helper/osh-accountGeneratePassword
@@ -33,6 +33,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $size or not $account) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'size' or 'account'");
 }
diff --git a/bin/helper/osh-accountGetPasswordInfo b/bin/helper/osh-accountGetPasswordInfo
index eacd7b4..f516a11 100755
--- a/bin/helper/osh-accountGetPasswordInfo
+++ b/bin/helper/osh-accountGetPasswordInfo
@@ -33,6 +33,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account && !$all) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account' or 'all'");
 }
diff --git a/bin/helper/osh-accountListEgressKeys b/bin/helper/osh-accountListEgressKeys
index 8cdc720..3e2b73a 100755
--- a/bin/helper/osh-accountListEgressKeys
+++ b/bin/helper/osh-accountListEgressKeys
@@ -29,6 +29,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account'");
 }
diff --git a/bin/helper/osh-accountListIngressKeys b/bin/helper/osh-accountListIngressKeys
index 3b11eb4..07de24f 100755
--- a/bin/helper/osh-accountListIngressKeys
+++ b/bin/helper/osh-accountListIngressKeys
@@ -32,6 +32,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account'");
 }
diff --git a/bin/helper/osh-accountListPasswords b/bin/helper/osh-accountListPasswords
index 82f0166..7bf1663 100755
--- a/bin/helper/osh-accountListPasswords
+++ b/bin/helper/osh-accountListPasswords
@@ -29,6 +29,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account'");
 }
diff --git a/bin/helper/osh-accountMFAResetPassword b/bin/helper/osh-accountMFAResetPassword
index 4d63b3d..6568b40 100755
--- a/bin/helper/osh-accountMFAResetPassword
+++ b/bin/helper/osh-accountMFAResetPassword
@@ -30,6 +30,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $account) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account'");
 }
diff --git a/bin/helper/osh-accountMFAResetTOTP b/bin/helper/osh-accountMFAResetTOTP
index 0f68830..e8133b6 100755
--- a/bin/helper/osh-accountMFAResetTOTP
+++ b/bin/helper/osh-accountMFAResetTOTP
@@ -30,6 +30,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $account) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account'");
 }
diff --git a/bin/helper/osh-accountModify b/bin/helper/osh-accountModify
index e34d689..93908b4 100755
--- a/bin/helper/osh-accountModify
+++ b/bin/helper/osh-accountModify
@@ -34,6 +34,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account || !@modify) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account' or 'modify'");
 }
diff --git a/bin/helper/osh-accountModifyCommand b/bin/helper/osh-accountModifyCommand
index 5338ae1..72ca64a 100755
--- a/bin/helper/osh-accountModifyCommand
+++ b/bin/helper/osh-accountModifyCommand
@@ -37,6 +37,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $account or not $command or not $action) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account', 'command' or 'action'");
 }
diff --git a/bin/helper/osh-accountModifyPersonalAccess b/bin/helper/osh-accountModifyPersonalAccess
index 923a74e..9745887 100755
--- a/bin/helper/osh-accountModifyPersonalAccess
+++ b/bin/helper/osh-accountModifyPersonalAccess
@@ -50,6 +50,8 @@ eval {
 };
 if ($@) { die $@ }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$result) {
     local $" = ", ";
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
diff --git a/bin/helper/osh-accountPIV b/bin/helper/osh-accountPIV
index 986eacf..cc25f3c 100755
--- a/bin/helper/osh-accountPIV
+++ b/bin/helper/osh-accountPIV
@@ -37,6 +37,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account || !$policy || !$step) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account' or 'modify' or 'step'");
 }
diff --git a/bin/helper/osh-accountUnexpire b/bin/helper/osh-accountUnexpire
index 9e6ab0e..cf475e7 100755
--- a/bin/helper/osh-accountUnexpire
+++ b/bin/helper/osh-accountUnexpire
@@ -29,6 +29,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account'");
 }
diff --git a/bin/helper/osh-adminMaintenance b/bin/helper/osh-adminMaintenance
index 9d4689d..dc3dab9 100755
--- a/bin/helper/osh-adminMaintenance
+++ b/bin/helper/osh-adminMaintenance
@@ -32,6 +32,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $action) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'action'");
 }
diff --git a/bin/helper/osh-groupAddServer b/bin/helper/osh-groupAddServer
index c854e11..4499f4a 100755
--- a/bin/helper/osh-groupAddServer
+++ b/bin/helper/osh-groupAddServer
@@ -42,6 +42,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $ip or not $group or not $action) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'ip' or 'group' or 'action'");
 }
diff --git a/bin/helper/osh-groupAddSymlinkToAccount b/bin/helper/osh-groupAddSymlinkToAccount
index c85a80b..5156eee 100755
--- a/bin/helper/osh-groupAddSymlinkToAccount
+++ b/bin/helper/osh-groupAddSymlinkToAccount
@@ -33,6 +33,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $account or not $group or not $action) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account', 'group' or 'action'");
 }
diff --git a/bin/helper/osh-groupCreate b/bin/helper/osh-groupCreate
index b82c2ac..6009f85 100755
--- a/bin/helper/osh-groupCreate
+++ b/bin/helper/osh-groupCreate
@@ -39,6 +39,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$group || !$owner) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'group' or 'owner'");
 }
diff --git a/bin/helper/osh-groupDelEgressKey b/bin/helper/osh-groupDelEgressKey
index 6df021e..51a56cc 100755
--- a/bin/helper/osh-groupDelEgressKey
+++ b/bin/helper/osh-groupDelEgressKey
@@ -33,6 +33,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$group || !$id) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'group' or 'id'");
 }
diff --git a/bin/helper/osh-groupDelete b/bin/helper/osh-groupDelete
index 53b51e7..53d405a 100755
--- a/bin/helper/osh-groupDelete
+++ b/bin/helper/osh-groupDelete
@@ -32,6 +32,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$group) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'group'");
 }
diff --git a/bin/helper/osh-groupGenerateEgressKey b/bin/helper/osh-groupGenerateEgressKey
index d77c0a5..c5eff35 100755
--- a/bin/helper/osh-groupGenerateEgressKey
+++ b/bin/helper/osh-groupGenerateEgressKey
@@ -35,6 +35,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$size || !$algo || !$group) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'size', 'algo' or 'group'");
 }
diff --git a/bin/helper/osh-groupGeneratePassword b/bin/helper/osh-groupGeneratePassword
index d4668e8..bb8683a 100755
--- a/bin/helper/osh-groupGeneratePassword
+++ b/bin/helper/osh-groupGeneratePassword
@@ -33,6 +33,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $size or not $group) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'size' or 'group'");
 }
diff --git a/bin/helper/osh-groupModify b/bin/helper/osh-groupModify
index 1562e75..aed5274 100755
--- a/bin/helper/osh-groupModify
+++ b/bin/helper/osh-groupModify
@@ -34,6 +34,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$group) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'group'");
 }
diff --git a/bin/helper/osh-groupSetRole b/bin/helper/osh-groupSetRole
index a13547d..0b992da 100755
--- a/bin/helper/osh-groupSetRole
+++ b/bin/helper/osh-groupSetRole
@@ -44,6 +44,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 osh_debug("groupSetRole: checking preconditions");
 $fnret = OVH::Bastion::Plugin::groupSetRole::preconditions(self => $self, account => $account, group => $group, action => $action, type => $type, sudo => 1, silentoverride => 1);
 osh_debug("groupSetRole: checking preconditions result: $fnret");
diff --git a/bin/helper/osh-selfMFASetupPassword b/bin/helper/osh-selfMFASetupPassword
index f102064..8b710c2 100755
--- a/bin/helper/osh-selfMFASetupPassword
+++ b/bin/helper/osh-selfMFASetupPassword
@@ -31,6 +31,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (!$account || !defined $step) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account' or 'step'");
 }
diff --git a/bin/helper/osh-selfMFASetupTOTP b/bin/helper/osh-selfMFASetupTOTP
index 3c084be..d6b6a43 100755
--- a/bin/helper/osh-selfMFASetupTOTP
+++ b/bin/helper/osh-selfMFASetupTOTP
@@ -29,6 +29,8 @@ if (!$result) {
     HEXIT('ERR_BAD_OPTIONS', msg => "Error parsing options: @optwarns");
 }
 
+OVH::Bastion::Helper::check_spurious_args();
+
 if (not $account) {
     HEXIT('ERR_MISSING_PARAMETER', msg => "Missing argument 'account'");
 }
diff --git a/bin/plugin/restricted/accountCreate b/bin/plugin/restricted/accountCreate
index 854cd09..2009417 100755
--- a/bin/plugin/restricted/accountCreate
+++ b/bin/plugin/restricted/accountCreate
@@ -134,9 +134,9 @@ push @command, "--type", "normal";
 push @command, "--account", $account;
 push @command, "--pubKey", $pubKey if !$noKey;
 push @command, "--always-active" if $alwaysActive;
-push @command, "--comment",           $comment         if $comment;
-push @command, "--uid",               $uid             if defined $uid;
-push @command, "--osh-only",          $oshOnly         if $oshOnly;
+push @command, "--comment", $comment if $comment;
+push @command, "--uid",     $uid     if defined $uid;
+push @command, "--osh-only", if $oshOnly;
 push @command, "--max-inactive-days", $maxInactiveDays if defined $maxInactiveDays;
 push @command, "--uid-auto"      if $uidAuto;
 push @command, "--immutable-key" if $immutableKey;
diff --git a/lib/perl/OVH/Bastion/Helper.pm b/lib/perl/OVH/Bastion/Helper.pm
index f613e9b..dd91544 100644
--- a/lib/perl/OVH/Bastion/Helper.pm
+++ b/lib/perl/OVH/Bastion/Helper.pm
@@ -37,6 +37,15 @@ sub HEXIT {    ## no critic (ArgUnpacking)
     exit 0;
 }
 
+# Used after Getopt::Long::GetOptions() in each helper, to ensure there are no unparsed/spurious args
+sub check_spurious_args {
+    if (@ARGV) {
+        local $" = ", ";
+        warn_syslog("Spurious arguments on command line: @ARGV");
+        HEXIT('ERR_BAD_OPTIONS', msg => "Spurious arguments on command line: @ARGV");
+    }
+}
+
 #
 # This code has to be ran for all helpers before they attempt to do anything useful,
 # and as we're only use'd by helpers, we include it here directly on top-level.