)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"03a8ada9a0af586293ca89352d573b705b80e6ca","unresolved":true,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"There is now filtering so that /etc/ /proc/ or /dev/ files that can"},{"line_number":13,"context_line":"be read by the mistralclient user to be added to a workflow for"},{"line_number":14,"context_line":"example"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This is mostly problematic for the horizon use of mistralclient."},{"line_number":17,"context_line":"For an usual CLI use there\u0027s no priviledge escalation, but on the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"f34e05a1_1b23f994","line":14,"updated":"2021-07-14 16:20:29.000000000","message":"I don\u0027t think adding blacklist paths is not sufficient. Even after this change, you cannot prevent from leaking file contents located outside of the specified paths on a web server.","commit_id":"0b4bf6edb5cc9296b9d7b53a9e8ced18c9ef1295"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"9c12bf709ce77feef140c130a39c674e4522e633","unresolved":true,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"There is now filtering so that /etc/ /proc/ or /dev/ files that can"},{"line_number":13,"context_line":"be read by the mistralclient user to be added to a workflow for"},{"line_number":14,"context_line":"example"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This is mostly problematic for the horizon use of mistralclient."},{"line_number":17,"context_line":"For an usual CLI use there\u0027s no priviledge escalation, but on the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1eef4c39_6b2db5e6","line":14,"in_reply_to":"f34e05a1_1b23f994","updated":"2021-07-15 14:23:48.000000000","message":"I agree with Akihiro here and this change still leave risks that users can access files to /var or /usr .\n\nIMO we should completely disable the current behavior to use contents as path or url. Even usage of url is still risky because it allows users to try attacking any internal servers reachable from the node where horizon runs.\n\nI have submitted a series of changes so that mistral use definiton passed as only raw, which I believe is the safe approach here.\n\nhttps://review.opendev.org/c/openstack/python-mistralclient/+/800950\nhttps://review.opendev.org/c/openstack/mistral-dashboard/+/800952","commit_id":"0b4bf6edb5cc9296b9d7b53a9e8ced18c9ef1295"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"03a8ada9a0af586293ca89352d573b705b80e6ca","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This is mostly problematic for the horizon use of mistralclient."},{"line_number":17,"context_line":"For an usual CLI use there\u0027s no priviledge escalation, but on the"},{"line_number":18,"context_line":"case of horizon the priviledges used are the one running horizon."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I8f261854dc159098bbaec3fe84768a0372127a57"},{"line_number":21,"context_line":"closes-bug: 1931558"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"50c0db66_c209881e","line":18,"updated":"2021-07-14 16:20:29.000000000","message":"After reading this description, it seems that mistral-dashboard should prevent the usage of this feature. When misralclient is used locally (this is the primary usecase of CLI), the utility loads the file content with a local user privilege.\nHowever, in case of UI implementation, the utility is run on a web server. I think this is the root cause of the security issue.","commit_id":"0b4bf6edb5cc9296b9d7b53a9e8ced18c9ef1295"}],"mistralclient/utils.py":[{"author":{"_account_id":19134,"name":"Eyal","email":"eyalb1@gmail.com","username":"eyalb"},"change_message_id":"89bb9f8007bf26814cade5ec52f2e64fb0655cd6","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"            definition_url \u003d parse.urljoin(\u0027file:\u0027, request.pathname2url(path))"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        if definition_url.startswith(\"file:\"):"},{"line_number":88,"context_line":"            for forbidden_path in FORBIDDEN_PATHS:"},{"line_number":89,"context_line":"                if definition_url.startswith(forbidden_path):"},{"line_number":90,"context_line":"                    msg \u003d \"path {} not allowed\".format(definition_url)"},{"line_number":91,"context_line":"                    print(msg)"},{"line_number":92,"context_line":"                    raise Exception(msg)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        return request.urlopen(definition_url).read().decode(\u0027utf8\u0027)"},{"line_number":95,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":1,"id":"96e1d9f5_53749ca5","line":92,"range":{"start_line":87,"start_character":7,"end_line":92,"end_character":40},"updated":"2021-07-07 14:05:04.000000000","message":"I think its better to use https://docs.python.org/3/library/urllib.parse.html\nyou can get the schema if exists and for path handling maybe https://docs.python.org/3/library/pathlib.html#pathlib.PurePath","commit_id":"081a065b4fb920c51e4d8e3da0b2ae7680602266"},{"author":{"_account_id":15895,"name":"Adriano Petrich","email":"apetrich@redhat.com","username":"apetrich"},"change_message_id":"9ca71b0db8eafc25c6d3106e3aaa73218b598143","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"            definition_url \u003d parse.urljoin(\u0027file:\u0027, request.pathname2url(path))"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        if definition_url.startswith(\"file:\"):"},{"line_number":88,"context_line":"            for forbidden_path in FORBIDDEN_PATHS:"},{"line_number":89,"context_line":"                if definition_url.startswith(forbidden_path):"},{"line_number":90,"context_line":"                    msg \u003d \"path {} not allowed\".format(definition_url)"},{"line_number":91,"context_line":"                    print(msg)"},{"line_number":92,"context_line":"                    raise Exception(msg)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        return request.urlopen(definition_url).read().decode(\u0027utf8\u0027)"},{"line_number":95,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":1,"id":"067f8da4_04ed7c6f","line":92,"range":{"start_line":87,"start_character":7,"end_line":92,"end_character":40},"in_reply_to":"96e1d9f5_53749ca5","updated":"2021-07-07 16:49:08.000000000","message":"Sure! Good point.","commit_id":"081a065b4fb920c51e4d8e3da0b2ae7680602266"}]}