From 6dc586b3c5e803c69485f6e0cef762479924b93e Mon Sep 17 00:00:00 2001 From: ljf Date: Thu, 25 Jun 2026 11:17:27 +0200 Subject: [PATCH 1/6] [fix] String started with at was interpreted as option --- moulinette/interfaces/api.py | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index b6987dd7..ed861cb1 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -227,21 +227,33 @@ def append(arg_strings, value, action): value.save(UPLOAD_DIR) if option_string is not None: arg_strings.append(option_string) - arg_strings.append(UPLOAD_DIR + "/" + value.filename) + # ==== SPACE DIRTY_HACK ===== + # In order to avoid option injection by starting a value with @ + # we add a starting space on all string values, like that argparse + # do not interpret it as an option... + # The space is remove in a post-treatment + # see https://github.com/python/cpython/issues/138950 + arg_strings.append(f" {UPLOAD_DIR}/{value.filename}") elif isinstance(value, str): + # Same space dirty hack as above + protected_value = f" {value}" if option_string is not None: arg_strings.append(option_string) # TODO: Review this fix + # FIXME What if value is an empty string ? if value: - arg_strings.append(value) + arg_strings.append(protected_value) else: - arg_strings.append(value) + arg_strings.append(protected_value) elif isinstance(value, list): if option_string is not None: arg_strings.append(option_string) for v in value: if isinstance(v, str): - arg_strings.append(v) + # Same space dirty hack as above + + protected_value = f" {v}" + arg_strings.append(protected_value) else: logger.warning( "unsupported argument value type %r " @@ -269,7 +281,13 @@ def append(arg_strings, value, action): if dest in args: arg_strings = append(arg_strings, args[dest], action) - return self._parser.parse_args(arg_strings, namespace) + # Post-treatment of the dirty hack describe above + parsed_args = self._parser.parse_args(arg_strings, namespace) + for option_string, value in vars(parsed_args).items(): + if isinstance(value, str) and option_string not in ["loginShell", "_tid"]: + # We remove the starting space we have added on all values... + setattr(parsed_args, option_string, value[1:]) + return parsed_args def _error(self, message): raise MoulinetteValidationError(message, raw_msg=True) From 508626ea934bf093134742a3b0587d36a19c90f8 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin <4533074+alexAubin@users.noreply.github.com> Date: Thu, 25 Jun 2026 12:34:52 +0200 Subject: [PATCH 2/6] Apply suggestion from @tituspijean Co-authored-by: tituspijean --- moulinette/interfaces/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index ed861cb1..2335f3ce 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -231,7 +231,7 @@ def append(arg_strings, value, action): # In order to avoid option injection by starting a value with @ # we add a starting space on all string values, like that argparse # do not interpret it as an option... - # The space is remove in a post-treatment + # The space is removed in a post-treatment # see https://github.com/python/cpython/issues/138950 arg_strings.append(f" {UPLOAD_DIR}/{value.filename}") elif isinstance(value, str): From 93ef00efabf6a8e4092bb05762d0494eb2cef574 Mon Sep 17 00:00:00 2001 From: ljf Date: Thu, 25 Jun 2026 19:08:59 +0200 Subject: [PATCH 3/6] [enh] Avoid to hardcode loginshell and _tid --- moulinette/interfaces/api.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index 2335f3ce..76bae1d2 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -283,8 +283,14 @@ def append(arg_strings, value, action): # Post-treatment of the dirty hack describe above parsed_args = self._parser.parse_args(arg_strings, namespace) - for option_string, value in vars(parsed_args).items(): - if isinstance(value, str) and option_string not in ["loginShell", "_tid"]: + parsed_args_dict = vars(parsed_args) + known_args = [action.dest for action in self._positional] + known_args += list(self._optional.keys()) + for option_string in known_args: + if option_string not in args: + continue + value = parsed_args_dict[option_string] + if isinstance(value, str) and value[0] == " ": # We remove the starting space we have added on all values... setattr(parsed_args, option_string, value[1:]) return parsed_args From 23618d910788f55f5f6bd9d2ec94e7bedee538da Mon Sep 17 00:00:00 2001 From: ljf Date: Thu, 25 Jun 2026 19:11:15 +0200 Subject: [PATCH 4/6] [enh] parse args is not post-treatment --- moulinette/interfaces/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index 76bae1d2..66e40a12 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -281,8 +281,9 @@ def append(arg_strings, value, action): if dest in args: arg_strings = append(arg_strings, args[dest], action) - # Post-treatment of the dirty hack describe above parsed_args = self._parser.parse_args(arg_strings, namespace) + + # Post-treatment of the dirty hack describe above parsed_args_dict = vars(parsed_args) known_args = [action.dest for action in self._positional] known_args += list(self._optional.keys()) From 0f8991a96afbd212b2f4fd5a424c2df049195267 Mon Sep 17 00:00:00 2001 From: ljf Date: Thu, 25 Jun 2026 19:19:45 +0200 Subject: [PATCH 5/6] [enh] Improve comments --- moulinette/interfaces/api.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index 66e40a12..f77feb93 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -146,6 +146,9 @@ def parse_accept_language(header: str) -> list[tuple[str, str | None]]: return m18n.default_locale +# FIXME This parser transform API call into CLI command and parse +# it with argparse. There are probably method to validate and ordering +# API arguments without relying on argparse cli tools class _HTTPArgumentParser: """Argument parser for HTTP requests @@ -228,9 +231,9 @@ def append(arg_strings, value, action): if option_string is not None: arg_strings.append(option_string) # ==== SPACE DIRTY_HACK ===== - # In order to avoid option injection by starting a value with @ - # we add a starting space on all string values, like that argparse - # do not interpret it as an option... + # In order to avoid option injection by starting a value with a + # prefix_chars (here @), we add a starting space on all string + # values, like that argparse do not interpret it as an option... # The space is removed in a post-treatment # see https://github.com/python/cpython/issues/138950 arg_strings.append(f" {UPLOAD_DIR}/{value.filename}") From f2539343908d860f6855ebe795109a95dbfca457 Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 8 Jul 2026 22:28:10 +0200 Subject: [PATCH 6/6] [fix] Actionmap choices broken with @ protection --- moulinette/interfaces/api.py | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index f77feb93..49c477e0 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -230,16 +230,14 @@ def append(arg_strings, value, action): value.save(UPLOAD_DIR) if option_string is not None: arg_strings.append(option_string) + arg_strings.append(f"{UPLOAD_DIR}/{value.filename}") + elif isinstance(value, str): # ==== SPACE DIRTY_HACK ===== # In order to avoid option injection by starting a value with a - # prefix_chars (here @), we add a starting space on all string - # values, like that argparse do not interpret it as an option... - # The space is removed in a post-treatment - # see https://github.com/python/cpython/issues/138950 - arg_strings.append(f" {UPLOAD_DIR}/{value.filename}") - elif isinstance(value, str): - # Same space dirty hack as above - protected_value = f" {value}" + # prefix_chars (here @), we add a starting space on string values + # starting with @, like that argparse do not interpret it as an + # option... + protected_value = f" {value}" if value.startswith("@") else value if option_string is not None: arg_strings.append(option_string) # TODO: Review this fix @@ -255,7 +253,7 @@ def append(arg_strings, value, action): if isinstance(v, str): # Same space dirty hack as above - protected_value = f" {v}" + protected_value = f" {v}" if v.startswith("@") else v arg_strings.append(protected_value) else: logger.warning( @@ -275,28 +273,29 @@ def append(arg_strings, value, action): return arg_strings # Iterate over positional arguments + known_args = [] for action in self._positional: if action.dest in args: arg_strings = append(arg_strings, args[action.dest], action) + known_args.append((action.dest, args[action.dest])) # Iterate over optional arguments for dest, action in self._optional.items(): if dest in args: arg_strings = append(arg_strings, args[dest], action) + known_args.append((dest, args[dest])) parsed_args = self._parser.parse_args(arg_strings, namespace) # Post-treatment of the dirty hack describe above parsed_args_dict = vars(parsed_args) - known_args = [action.dest for action in self._positional] - known_args += list(self._optional.keys()) - for option_string in known_args: + for option_string, value in known_args: if option_string not in args: continue - value = parsed_args_dict[option_string] - if isinstance(value, str) and value[0] == " ": - # We remove the starting space we have added on all values... - setattr(parsed_args, option_string, value[1:]) + protected_value = parsed_args_dict[option_string] + if isinstance(protected_value, str) and value.startswith("@"): + # We remove the starting space we have added on values starting with @... + setattr(parsed_args, option_string, protected_value[1:]) return parsed_args def _error(self, message):