Merge pull request #8722 from achouhan09/account-space-fix

NC | CLI | Added a validation to check if whitespace is not trimmed from string value for a flag

Author: achouhan09

src/manage_nsfs/manage_nsfs_validations.js

@@ -29,6 +29,7 @@ const notifications_util = require('../util/notifications_util');
  * @param {object} argv
  */
 async function validate_input_types(type, action, argv) {
+    validate_no_extra_args(argv._);
     validate_type_and_action(type, action);
     // when we use validate_no_extra_options we don't care about the value, only the flags
     const input_options = Object.keys(argv);
@@ -85,6 +86,35 @@ function validate_type_and_action(type, action) {
     }
 }
 
+/**
+ * validate_no_extra_args ensures that the parsed arguments contain only the expected type and action
+ * if `argv_` contains more than two elements (type and action), it indicate extra argument which may be due to:
+ *  - Leading spaces in flag values (e.g., `--new_buckets_path= abc/`)
+ *  - Additional unexpected arguments passed from the command line
+ *
+ * ### CLI Parsing Behavior:
+ * When passing arguments via CLI, if a flag's value starts with a space or consists only of whitespace,
+ * the shell may treat it differently:
+ *  - Leading spaces - might be preserved when quoted (`--new_buckets_path=" abc/"`).
+ *  - Unquoted values with spaces - may cause incorrect parsing (`--new_buckets_path= abc/` is interpreted as an empty value).
+ *  - The parsed argument structure typically follows:
+ *      ```json
+ *      { _: [ "command", "action" ], flag1: "value1", flag2: "value2", ... }
+ *      ```
+ *  - The `_` array should contain only the command and action. If additional elements appear,
+ *    it suggests an issue with flag values (e.g., an incorrectly formatted or empty value).
+ *
+ * @param {string[]} argv_
+ * @throws {ManageCLIError}
+ */
+function validate_no_extra_args(argv_) {
+    // checking if 'argv_' contains more then 2 values (i.e, type and action)
+    if (argv_.length > 2) {
+        const details = `unexpected extra arguments detected: ${JSON.stringify(argv_.slice(2))}, ensure flag values are correctly formatted.`;
+        throw_cli_error(ManageCLIError.InvalidArgument, details);
+    }
+}
+
 /**
  * validate_identifier will validate that we have the needed identifier for the command
  * @param {string} type
@@ -718,3 +748,4 @@ exports.validate_whitelist_arg = validate_whitelist_arg;
 exports.validate_whitelist_ips = validate_whitelist_ips;
 exports.validate_flags_combination = validate_flags_combination;
 exports.validate_connection_args = validate_connection_args;
+exports.validate_no_extra_args = validate_no_extra_args;

src/test/unit_tests/jest_tests/test_nc_account_cli.test.js

@@ -23,6 +23,12 @@ const timeout = 50000;
 const config = require('../../../../config');
 const config_fs_account_options = { show_secrets: true, decrypt_secret_key: true };
 
+const quoted_type = Object.freeze({
+    SPACE_ONLY: "s", // space only
+    QUOTE_SPACE: "qs", // quote then space
+    SPACE_QUOTE: "sq", // space then quote
+});
+
 // eslint-disable-next-line max-lines-per-function
 describe('manage nsfs cli account flow', () => {
     describe('cli create account', () => {
@@ -103,6 +109,33 @@ describe('manage nsfs cli account flow', () => {
             assert_account(account_symlink, account_options);
         });
 
+        it('should fail - cli create account invalid new_buckets_path (leading space in new_buckets_path)', async () => {
+            const { type, name, uid, gid, new_buckets_path } = defaults;
+            const account_options = { config_root, name, uid, gid}; // will add new_buckets_path with leading space later
+            const action = ACTIONS.ADD;
+            const command = create_command(type, action, account_options);
+            const res = await exec_manage_cli_add_leading_space_option(command, 'new_buckets_path', new_buckets_path, quoted_type.SPACE_ONLY);
+            expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidArgument.code);
+        });
+
+        it('should fail - cli create account invalid new_buckets_path (leading space in quoted new_buckets_path)', async () => {
+            const { type, name, uid, gid, new_buckets_path } = defaults;
+            const account_options = { config_root, name, uid, gid}; // will add quoted new_buckets_path with leading space later
+            const action = ACTIONS.ADD;
+            const command = create_command(type, action, account_options);
+            const res = await exec_manage_cli_add_leading_space_option(command, 'new_buckets_path', new_buckets_path, quoted_type.SPACE_QUOTE);
+            expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidArgument.code);
+        });
+
+        it('should fail - cli create account invalid new_buckets_path (quoted leading space in new_buckets_path)', async () => {
+            const { type, name, uid, gid, new_buckets_path } = defaults;
+            const account_options = { config_root, name, uid, gid}; // will add new_buckets_path with quoted leading space later
+            const action = ACTIONS.ADD;
+            const command = create_command(type, action, account_options);
+            const res = await exec_manage_cli_add_leading_space_option(command, 'new_buckets_path', new_buckets_path, quoted_type.QUOTE_SPACE);
+            expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidAccountNewBucketsPath.code); // should get this error if space is also quoted with the value
+        });
+
         it('should fail - cli update account invalid access_key - invalid size', async () => {
             const { type, secret_key, name, new_buckets_path, uid, gid } = defaults;
             const account_options = { config_root, access_key: 'abc', secret_key, name, new_buckets_path, uid, gid };
@@ -2218,6 +2251,52 @@ async function exec_manage_cli_add_empty_option(command, option) {
     return res;
 }
 
+/**
+ * exec_manage_cli_add_leading_space_option modifies the command by appending a flag with a value(quoted or non-quoted) that includes a leading space to test CLI parsing behavior
+ * @param {string} command
+ * @param {string} option
+ * @param {string} value
+ * @param {"s" | "qs" | "sq" | "default"} quoted
+ *
+ * @examples
+ * - await exec_manage_cli_add_leading_space_option("cli_command", "flag", "val", quoted_type.SPACE_ONLY);
+ *   // cli_command --flag= val  (Space before value)
+ *
+ * - await exec_manage_cli_add_leading_space_option("cli_command", "flag", "val", quoted_type.QUOTE_SPACE);
+ *   // cli_command --flag=" val"  (quote then space)
+ *
+ * - await exec_manage_cli_add_leading_space_option("cli_command", "flag", "val", quoted_type.SPACE_QUOTE);
+ *   // cli_command --flag= "val"  (space then quote)
+ *
+ * - await exec_manage_cli_add_leading_space_option("cli_command", "flag", "val");
+ *   // cli_command --flag=val  (No special formatting)
+ */
+async function exec_manage_cli_add_leading_space_option(command, option, value, quoted = "default") {
+    let changed_command;
+
+    switch (quoted) {
+        case quoted_type.SPACE_ONLY:
+            changed_command = `${command} --${option}= ${value}`;
+            break;
+        case quoted_type.QUOTE_SPACE:
+            changed_command = `${command} --${option}=" ${value}"`;
+            break;
+        case quoted_type.SPACE_QUOTE:
+            changed_command = `${command} --${option}= "${value}"`;
+            break;
+        default:
+            changed_command = `${command} --${option}=${value}`;
+    }
+
+    let res;
+    try {
+        res = await os_util.exec(changed_command, { return_stdout: true });
+    } catch (e) {
+        res = e;
+    }
+    return res;
+}
+
 /** 
  * create_command would create the string needed to run the CLI command
  * @param {string} type