)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"d6c814b54cb3c6c8d25c887988e6612425fff6bf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"37f288d4_049953c7","updated":"2024-07-01 06:36:20.000000000","message":"so far just a POC. Still need to pass zuul and then some documentation edits are needed to address the breaking change.","commit_id":"7aa27dfd265f8f198909b568d746cc0f30ce494d"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"6da1f10e37d9ef9d2b77d0d5e04c3817ce996614","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"655441b0_a32cc984","updated":"2024-07-01 07:12:15.000000000","message":"Should we rather think of utilizing similar code as Kayobe?","commit_id":"a6c5e1501eb09c9b0be083733a5157969b48e202"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"010645506ca611afa8da71244ced280d0b7bfb54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2656f83f_e4026661","in_reply_to":"655441b0_a32cc984","updated":"2024-07-11 21:52:40.000000000","message":"Acknowledged","commit_id":"a6c5e1501eb09c9b0be083733a5157969b48e202"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"359fcc81c780a4c28d8b8db39f7008e6d93ecabd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f44d4de2_031c6bb0","updated":"2024-07-09 06:43:55.000000000","message":"I\u0027ve fixed the release notes problem locally. It looks like ansible-playbook is not not importing collections and modules and that is causing the tests to fail.","commit_id":"67761b93e9f38526da5e5aee9b515af921907484"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"65a5fc23e5d86957f17c6b659bfb76e2a1ada29b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"6a25000e_e4001c13","updated":"2024-07-15 12:20:39.000000000","message":"I\u0027m still looking into the code, but there are some things that can already get fixed I think, so I wanted to publish some comments now.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"010645506ca611afa8da71244ced280d0b7bfb54","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":9,"id":"16836fba_9158be85","updated":"2024-07-11 21:52:40.000000000","message":"Well it looks like the code-part of this is ready for review. However, I am failing linter tests because bandit is not happy. Not sure how to approach that. As advised, I took inspiration from kayobe and they use `subprocess`, which bandit seems to not like.\n\nkolla-ansible-rocky9-magnum keeps failing, but there hasn\u0027t been a successful run since 2024/07/03 so I am not sure if this is a me problem.\n\nARM64 pipeline keeps failing because of OSError, but there I am not sure why, but it also look unrelated.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"a66696234a266747f3ed724b4c87474acc28e786","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9ca5628f_a5aee724","updated":"2024-07-11 21:51:32.000000000","message":"Well it looks like the code-part of this is ready for review. However, I am failing linter tests because bandit is not happy. Not sure how to approach that. As advised, I took inspiration from kayobe and they use `subprocess`, which bandit seems to not like.\n\nkolla-ansible-rocky9-magnum keeps failing, but there hasn\u0027t been a successful run since 2024/07/03 so I am not sure if this is a me problem.\n\nARM64 pipeline keeps failing because of OSError, but there I am not sure why, but it also look unrelated.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"65a5fc23e5d86957f17c6b659bfb76e2a1ada29b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":9,"id":"29ba23d8_5e3702ec","in_reply_to":"16836fba_9158be85","updated":"2024-07-15 12:20:39.000000000","message":"regarding the bandit warning I\u0027m currently unhappy with how the helper functions work, I _think_ this could be fixed, given the code description. I\u0027ll comment specifics on the actual code.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"f6af754a8fd3399eb4e46e405c5278f3e2502bfd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"d76be35c_a29e2988","in_reply_to":"29ba23d8_5e3702ec","updated":"2024-10-25 15:48:09.000000000","message":"Done","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"2e1f1ef71443ca9dce286b204e0441fc2703f9bc","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"2f29c0cb_3b8685e4","updated":"2024-09-11 13:40:43.000000000","message":"LGTM, but any chance of running kolla-ansible on the uninstalled code in the repo (just like we could run tools/kolla-ansible deploy)? Maybe we could create some wrapper in tools/kolla-ansible place?","commit_id":"c3cc6e3e499e9d5c142f29740aeb8837c81460c6"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"3d227db918ba8baf3166c5300b27dd3440da480f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"a3826329_a161555a","updated":"2024-09-05 06:58:15.000000000","message":"another rebase","commit_id":"c3cc6e3e499e9d5c142f29740aeb8837c81460c6"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"d17ff43fefecf8505e9def231ea4aa9a88e36534","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"3755877b_496217d0","in_reply_to":"2551fc98_b126a5f9","updated":"2024-09-12 13:44:39.000000000","message":"Usually the workflow was:\ngit clone kolla-ansible\npip3 install kolla-ansible (to get all python modules installed)\nedit some role\nrun tools/kolla-ansible deploy (so you don\u0027t need to do pip3 install -U every time)\n\nI know you can fix that with editable pip install - but that was also nice","commit_id":"c3cc6e3e499e9d5c142f29740aeb8837c81460c6"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"18414014e56f6326853f56408aefe8c782f16fd7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"33ff355a_9e97fdb6","in_reply_to":"2e2b6e0c_82c5ff22","updated":"2024-10-09 12:57:12.000000000","message":"Done","commit_id":"c3cc6e3e499e9d5c142f29740aeb8837c81460c6"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"f68414cd73247393c52a656ab69a75d49bab1aae","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"2551fc98_b126a5f9","in_reply_to":"2f29c0cb_3b8685e4","updated":"2024-09-12 13:32:53.000000000","message":"I will take a look at that. Could you elaborate on what would be the use case?","commit_id":"c3cc6e3e499e9d5c142f29740aeb8837c81460c6"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"6cd905973ec99253274c92a4763489eee4a1ced2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"aceb68f4_20032d0d","in_reply_to":"3755877b_496217d0","updated":"2024-09-17 21:33:50.000000000","message":"I tried to implement a script that would do what you described and it looks like it is incompatible due to the inner workings of cliff. I am able to invoke the `kolla_ansible/cmd/kolla_ansible.py` module, however, no commands are being loaded. The only (sub)command I can user are `help` and `complete`\n\nI guess now we have to stick to `pip install -e kolla_ansible` :/","commit_id":"c3cc6e3e499e9d5c142f29740aeb8837c81460c6"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"f89865c3437a1865309f147451ed644d53eecd6f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"2e2b6e0c_82c5ff22","in_reply_to":"6a71ac42_a931f084","updated":"2024-09-26 14:16:16.000000000","message":"Done","commit_id":"c3cc6e3e499e9d5c142f29740aeb8837c81460c6"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"34dc231531a4d75ae0ff1ef5fccc9621891040c7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"6a71ac42_a931f084","in_reply_to":"aceb68f4_20032d0d","updated":"2024-09-25 13:41:01.000000000","message":"Then we need to update doc/source/reference/deployment-and-bootstrapping/bifrost.rst","commit_id":"c3cc6e3e499e9d5c142f29740aeb8837c81460c6"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"7782575abe79a5442b87232a1b0f429b86f28bf3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"428c039d_07f481ab","updated":"2024-09-27 07:22:20.000000000","message":"recheck \u0027FAILED: Instance failed to become active after resize confirm\u0027 in podman ubuntu","commit_id":"d564f16b61510edffb986b02adfed3c6d5271adf"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"f6af754a8fd3399eb4e46e405c5278f3e2502bfd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"853b35a6_c9388029","updated":"2024-10-25 15:48:09.000000000","message":"fast approving since we already flashed that out in some of marketing material and bandit/nosec is not an issue as long as we adhere to security recommendations from CWE.","commit_id":"92b50bc9873be5302f873760ce45eeb7de9fe084"},{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"28505c2106281998752775032fad453c8e3f7322","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"ac414bef_9164f6b9","updated":"2024-10-25 21:54:44.000000000","message":"recheck looks like proxysql last changes?","commit_id":"92b50bc9873be5302f873760ce45eeb7de9fe084"}],"kolla_ansible/utils.py":[{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"65a5fc23e5d86957f17c6b659bfb76e2a1ada29b","unresolved":true,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"f4740c0d_d3cfc8d7","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"updated":"2024-07-15 12:20:39.000000000","message":"this is contradicting itself - which also results in the bandit complaints:\n\nthe function name and the docstring imply, that only a single command is run, however what actually happens is that we run arbitrary lists of strings.\n\nI need to review the code more to get a grasp of the intent, so if we really want to run an arbitrary number of chained commands at the same time via a single function call or if we only use this to pass arbitrary arguments to one command.\n\nif we want to do the latter I would suggest that we should split the variables at least up into `cmd: str` and `args: list`.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"4c6325eaf7cf584e471d67d625a249c794099631","unresolved":true,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"b9d4664e_37ecb306","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"in_reply_to":"005c0487_96fb8fab","updated":"2024-08-02 14:32:22.000000000","message":"so, as far as I can see, this function is only used exactly once to call ansible-galaxy.\n\nthe problem is, that we are still passing an arbitrary number of args as strings, which still might contain new commands.\n\nthe solution should be outlined here:\n\nhttps://security.openstack.org/guidelines/dg_use-subprocess-securely.html#correct\n\nto quote:\n\n\u003e Rather than passing a string to subprocess, our function passes a list of strings. The ping program gets each argument separately (even if the argument has a space in it), so the shell does not process other commands that are provided by the user after the ping command terminates.\n\nso we should be fine if we just pass a list of strings instead.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"b638a2d6df39d32bd5b5cfbca28e87755ab6457a","unresolved":true,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"2bb22820_aa12f07d","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"in_reply_to":"190106b3_ad2ae256","updated":"2024-08-21 13:26:50.000000000","message":"the problem from bandit pov, which is a legit security concern btw, is that you pass arbitrary arguments to a command, which can be abused for command injection.\n\nmade up example in python:\n```\nimport subprocess\n\ndef run_command(user_input):\n    command \u003d f\"ls -l {user_input}\"\n    subprocess.run(command, shell\u003dTrue)\n\n# Simulate user input\nuser_input \u003d \"some_directory; echo \u0027Injected command!\u0027\"\nrun_command(user_input)\n\n```\n\none solution would be to just get rid of this function, as it\u0027s only called exactly twice and just use plain `subprocess` instead.\n\nthe other option would be to silence the error, I don\u0027t know how bandit can be configured to do so.\n\nI think it could be fine for this use case as we control which commands are run and there is not really a security concern in this case.\n\nBut we should not globally disable this warning, only for this specific occurrence.\n\nThanks.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"744350212a00593b0aab9446d3023bd719f41bcd","unresolved":true,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"420303fb_ffc7dd44","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"in_reply_to":"2bb22820_aa12f07d","updated":"2024-08-22 07:52:13.000000000","message":"I am not sure if we are on the same page. I took a look at the document you sent me and I am passing a list or arguments to subprocess and using shell\u003dFalse. Soo... that should be in line with security recommendations and not vulnerable to the shell injections you mention in your example, right? However, bandit is still not satisfied. I\u0027ve combed through the docs and using `# nosec` silences bandit errors.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"f6af754a8fd3399eb4e46e405c5278f3e2502bfd","unresolved":false,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"a77cab17_4bfdbb77","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"in_reply_to":"420303fb_ffc7dd44","updated":"2024-10-25 15:48:09.000000000","message":"I think it\u0027s fine, as long as we do things according to security recommendations - we\u0027re good.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"9720fe65702948eb0b5574e937d49b0a0939c470","unresolved":true,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"005c0487_96fb8fab","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"in_reply_to":"90954d62_abd0dcaf","updated":"2024-08-02 14:23:31.000000000","message":"for posterity, here is the error bandit throws:\n\n```\n\u003e\u003e Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input.\n   Severity: Low   Confidence: High\n   CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html)\n   More Info: https://bandit.readthedocs.io/en/1.7.9/plugins/b603_subprocess_without_shell_equals_true.html\n   Location: kolla_ansible/utils.py:174:8\n173\t    else:\n174\t        subprocess.check_call(full_cmd, **kwargs)\n```","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"7c0f2122df2e4bc776fe433c88e754e6706ca836","unresolved":true,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"190106b3_ad2ae256","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"in_reply_to":"b9d4664e_37ecb306","updated":"2024-08-07 07:34:11.000000000","message":"I\u0027m sure I see how my approach is different from the one in the example. I also pass list of strings to the subprocess.check_call function. \n\nHowever, it seems that bandit is never satisfied. Even when I use `subprocess.check_output([\"ping\", \"-c\", \"1\", \"8.8.8.8\"], shell\u003dFalse)` so without any kwargs or external variables, just to see what bandit says, it still throws the same error.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"62871fb2e6e79ac78618aba65b04134bcc025ab9","unresolved":true,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"90954d62_abd0dcaf","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"in_reply_to":"eb4684fe_64b62018","updated":"2024-07-30 16:22:47.000000000","message":"I\u0027ll take a look at bandit myself, I\u0027m not terrible familiar with it, but on the surface it seems we should be able to solve the warning somehow. Digging into it now.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"98d9eec0e159e27db1f0c6ed487822172f3c8a8f","unresolved":true,"context_lines":[{"line_number":151,"context_line":"    return {\"result\": True}"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def run_command(cmd: list, quiet: bool \u003d False,"},{"line_number":155,"context_line":"                check_output: bool \u003d False, **kwargs):"},{"line_number":156,"context_line":"    \"\"\"Run a command, checking the output."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param quiet: Redirect output to /dev/null"},{"line_number":159,"context_line":"    :param check_output: Whether to return the output of the command"},{"line_number":160,"context_line":"    :returns: The output of the command if check_output is true"},{"line_number":161,"context_line":"    \"\"\""},{"line_number":162,"context_line":"    if isinstance(cmd, str):"},{"line_number":163,"context_line":"        cmd_string \u003d cmd"},{"line_number":164,"context_line":"    else:"},{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"}],"source_content_type":"text/x-python","patch_set":9,"id":"eb4684fe_64b62018","line":165,"range":{"start_line":154,"start_character":0,"end_line":165,"end_character":34},"in_reply_to":"f4740c0d_d3cfc8d7","updated":"2024-07-19 07:15:30.000000000","message":"I\u0027ve split the command and args as you suggested and even added override of the kwargs variable, so that `shell` couldn\u0027t be set to true. However, this did not get rid of the bandit warning.\n\nI thought that bandit is \"not that smart\" and was complaining just because it saw me using those function, no?","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"65a5fc23e5d86957f17c6b659bfb76e2a1ada29b","unresolved":true,"context_lines":[{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"},{"line_number":169,"context_line":"            kwargs[\"stdout\"] \u003d devnull"},{"line_number":170,"context_line":"            kwargs[\"stderr\"] \u003d devnull"},{"line_number":171,"context_line":"            subprocess.check_call(cmd, **kwargs)"},{"line_number":172,"context_line":"    elif check_output:"},{"line_number":173,"context_line":"        return subprocess.check_output(cmd, **kwargs)"}],"source_content_type":"text/x-python","patch_set":9,"id":"2c94caf8_86b4ba2b","line":170,"range":{"start_line":168,"start_character":8,"end_line":170,"end_character":38},"updated":"2024-07-15 12:20:39.000000000","message":"this is not necessary, there is already a `subprocess` builtin for this, see:\n\nhttps://docs.python.org/3/library/subprocess.html#subprocess.DEVNULL\n\nit\u0027s even suggested in the `check_call` documentation to just use that: https://docs.python.org/3/library/subprocess.html#subprocess.check_call\n\n\u003e To suppress stdout or stderr, supply a value of DEVNULL.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"98d9eec0e159e27db1f0c6ed487822172f3c8a8f","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        cmd_string \u003d \" \".join(cmd)"},{"line_number":166,"context_line":"    LOG.debug(\"Running command: %s\", cmd_string)"},{"line_number":167,"context_line":"    if quiet:"},{"line_number":168,"context_line":"        with open(\"/dev/null\", \"w\") as devnull:"},{"line_number":169,"context_line":"            kwargs[\"stdout\"] \u003d devnull"},{"line_number":170,"context_line":"            kwargs[\"stderr\"] \u003d devnull"},{"line_number":171,"context_line":"            subprocess.check_call(cmd, **kwargs)"},{"line_number":172,"context_line":"    elif check_output:"},{"line_number":173,"context_line":"        return subprocess.check_output(cmd, **kwargs)"}],"source_content_type":"text/x-python","patch_set":9,"id":"81c2b729_0bba19e6","line":170,"range":{"start_line":168,"start_character":8,"end_line":170,"end_character":38},"in_reply_to":"2c94caf8_86b4ba2b","updated":"2024-07-19 07:15:30.000000000","message":"Done","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"65a5fc23e5d86957f17c6b659bfb76e2a1ada29b","unresolved":true,"context_lines":[{"line_number":175,"context_line":"        subprocess.check_call(cmd, **kwargs)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"def quote_and_escape(value):"},{"line_number":179,"context_line":"    \"\"\"Quote and escape a string."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    Adds enclosing single quotes to the string passed, and escapes single"},{"line_number":182,"context_line":"    quotes within the string using backslashes. This is useful for passing"},{"line_number":183,"context_line":"    \u0027extra vars\u0027 to Ansible. Without this, Ansible only uses the part of the"},{"line_number":184,"context_line":"    string up to the first whitespace."},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    :param value: the string to quote and escape."},{"line_number":187,"context_line":"    :returns: the quoted and escaped string."},{"line_number":188,"context_line":"    \"\"\""},{"line_number":189,"context_line":"    if not isinstance(value, str):"},{"line_number":190,"context_line":"        return value"},{"line_number":191,"context_line":"    return \"\u0027\" + value.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\""}],"source_content_type":"text/x-python","patch_set":9,"id":"a945fb42_eaeefa66","line":191,"range":{"start_line":178,"start_character":0,"end_line":191,"end_character":50},"updated":"2024-07-15 12:20:39.000000000","message":"This function does not do what the docstring says it will do.\n\nWhy do we return the value if value is not of type string? shouldn\u0027t we throw an error instead? The caller will assume they get an escaped string, but we don\u0027t return an escaped string for some cases. This is dangerous.\n\nAlso, the replace function does not seem correct to me, tested locally:\n\n```\n\u003e\u003e\u003e my_string\u003d\"T\u0027is is my \u0027\u0027awful\u0027\u0027 teststring\"\n\u003e\u003e\u003e print(my_string)\nT\u0027is is my \u0027\u0027awful\u0027\u0027 teststring\n\u003e\u003e\u003e print(\"\u0027\" + my_string.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\")\n\u0027T\u0027\\\u0027\u0027is is my \u0027\\\u0027\u0027\u0027\\\u0027\u0027awful\u0027\\\u0027\u0027\u0027\\\u0027\u0027 teststring\u0027\n```\n\nI\u0027d suggest the following instead:\n\n\n```suggestion\ndef quote_and_escape(value):\n    \"\"\"Quote and escape a string.\n\n    Adds enclosing single quotes to the string passed, and escapes single\n    quotes within the string using backslashes. This is useful for passing\n    \u0027extra vars\u0027 to Ansible. Without this, Ansible only uses the part of the\n    string up to the first whitespace.\n\n    :param value: the string to quote and escape.\n    :returns: the quoted and escaped string.\n    \"\"\"\n    if not isinstance(value, str):\n        return value\n    escaped_input \u003d value.replace(\"\u0027\", \"\\\\\u0027\")\n    quoted_and_escaped_input \u003d f\"\u0027{escaped_input}\u0027\"\n    return quoted_and_escaped_input\n```\n\nThis seems to be more correct in my short local testing:\n\n```\n\u003e\u003e\u003e my_string\u003d\"T\u0027is is my \u0027\u0027awful\u0027\u0027 teststring\"\n\u003e\u003e\u003e print(my_string)\nT\u0027is is my \u0027\u0027awful\u0027\u0027 teststring\n\u003e\u003e\u003e escaped_input \u003d my_string.replace(\"\u0027\", \"\\\\\u0027\")\n\u003e\u003e\u003e quoted_and_escaped_input \u003d f\"\u0027{escaped_input}\u0027\"\n\u003e\u003e\u003e print(quoted_and_escaped_input)\n\u0027T\\\u0027is is my \\\u0027\\\u0027awful\\\u0027\\\u0027 teststring\u0027\n```\n\nYou can of course reduce the number of helper variables, I just used these for better readability.","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"98d9eec0e159e27db1f0c6ed487822172f3c8a8f","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        subprocess.check_call(cmd, **kwargs)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"def quote_and_escape(value):"},{"line_number":179,"context_line":"    \"\"\"Quote and escape a string."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    Adds enclosing single quotes to the string passed, and escapes single"},{"line_number":182,"context_line":"    quotes within the string using backslashes. This is useful for passing"},{"line_number":183,"context_line":"    \u0027extra vars\u0027 to Ansible. Without this, Ansible only uses the part of the"},{"line_number":184,"context_line":"    string up to the first whitespace."},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    :param value: the string to quote and escape."},{"line_number":187,"context_line":"    :returns: the quoted and escaped string."},{"line_number":188,"context_line":"    \"\"\""},{"line_number":189,"context_line":"    if not isinstance(value, str):"},{"line_number":190,"context_line":"        return value"},{"line_number":191,"context_line":"    return \"\u0027\" + value.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\""}],"source_content_type":"text/x-python","patch_set":9,"id":"c681220a_9cb12d6c","line":191,"range":{"start_line":178,"start_character":0,"end_line":191,"end_character":50},"in_reply_to":"a945fb42_eaeefa66","updated":"2024-07-19 07:15:30.000000000","message":"My apologies. This one function slipped my attention. This was included from the original kayobe-cli project. See the links where it is used. But since k-a is not that demanding in the extra_vars space, I decided not to use it. But I forgot to remove it. :/\n\nhttps://github.com/search?q\u003drepo%3Aopenstack%2Fkayobe%20quote_and_escape\u0026type\u003dcode","commit_id":"09bbdf31e00efda176438309ddf596dc3dfea594"}]}