)]}'
{"bin/nova-rootwrap":[{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"581f770203352f4845205824402a47c30af09d4f","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    config.read(configfile)"},{"line_number":66,"context_line":"    try:"},{"line_number":67,"context_line":"        filters_path \u003d config.get(\"DEFAULT\", \"filters_path\").split(\",\")"},{"line_number":68,"context_line":"        if config.has_option(\"DEFAULT\", \"exec_dirs\"):"},{"line_number":69,"context_line":"            exec_dirs \u003d config.get(\"DEFAULT\", \"exec_dirs\").split(\",\")"},{"line_number":70,"context_line":"        else:"},{"line_number":71,"context_line":"            exec_dirs\u003d[]"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAAMH%2F%2Fy6E%3D","line":68,"updated":"2012-11-22 16:15:26.000000000","message":"Shouldn\u0027t we just default to os.environ[\u0027PATH\u0027]\nWe probably don\u0027t even need this config var, and instead rely on standard $PATH setup for the nova services?","commit_id":"58cd6c580fbfcb3931ae31f100ad3055a79cae7c"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"56feb4d5217832f6c3bb86453b3f99858ebbeadb","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        if config.has_option(\"DEFAULT\", \"exec_dirs\"):"},{"line_number":69,"context_line":"            exec_dirs \u003d config.get(\"DEFAULT\", \"exec_dirs\").split(\",\")"},{"line_number":70,"context_line":"        else:"},{"line_number":71,"context_line":"            exec_dirs \u003d []"},{"line_number":72,"context_line":"    except ConfigParser.Error:"},{"line_number":73,"context_line":"        print \"%s: Incorrect configuration file: %s\" % (execname, configfile)"},{"line_number":74,"context_line":"        sys.exit(RC_BADCONFIG)"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAAMH%2F%2FsC0%3D","line":71,"updated":"2012-11-30 13:34:45.000000000","message":"Davanum asks a very good question\n\nOn Red Hat distros, the config file will be marked with %config(noreplace)\n\nThis means that if a user had modified the file previously, they won\u0027t have the default value of exec_dirs in the config file\n\nWe really should fall back to a sensible exec_dirs default if there is none in the config file","commit_id":"7b1c5a33f07f86f32cde06f7296d4bd0de864710"},{"author":{"_account_id":308,"name":"Thierry Carrez","email":"thierry@openstack.org","username":"ttx"},"change_message_id":"f68c83cfb729c6e2df662869044e314dcdf3cbc3","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        if config.has_option(\"DEFAULT\", \"exec_dirs\"):"},{"line_number":69,"context_line":"            exec_dirs \u003d config.get(\"DEFAULT\", \"exec_dirs\").split(\",\")"},{"line_number":70,"context_line":"        else:"},{"line_number":71,"context_line":"            exec_dirs \u003d []"},{"line_number":72,"context_line":"    except ConfigParser.Error:"},{"line_number":73,"context_line":"        print \"%s: Incorrect configuration file: %s\" % (execname, configfile)"},{"line_number":74,"context_line":"        sys.exit(RC_BADCONFIG)"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAAMH%2F%2FrD8%3D","line":71,"in_reply_to":"AAAAMH%2F%2FsC0%3D","updated":"2012-12-01 08:45:25.000000000","message":"Yes, it\u0027s a good point. exec_dirs should default to the value of PATH if not specifically mentioned. Slightly less secure (but still secure enough), and good for transition. Will do.","commit_id":"7b1c5a33f07f86f32cde06f7296d4bd0de864710"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"250fdae18584bf06915db8ef71352f1987b413a1","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            exec_dirs \u003d config.get(\"DEFAULT\", \"exec_dirs\").split(\",\")"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            # Use system PATH if exec_dirs is not specified"},{"line_number":73,"context_line":"            exec_dirs \u003d os.environ[\"PATH\"].split(\u0027:\u0027)"},{"line_number":74,"context_line":"    except ConfigParser.Error:"},{"line_number":75,"context_line":"        print \"%s: Incorrect configuration file: %s\" % (execname, configfile)"},{"line_number":76,"context_line":"        sys.exit(RC_BADCONFIG)"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAMX%2F%2F%2FdY%3D","line":73,"updated":"2012-12-03 15:16:41.000000000","message":"Using the PATH is good, at least as a fall back.\n\nFor my own reference, providing the exec_dirs config option,\nis essentially saying that, the PATH env of the nova process\nis easier to modify than the value of the exec_dirs flag var.\nIf that\u0027s really the case then having exec_dirs is warranted.","commit_id":"12e264d58f052f192f3408f5cd8637809eff085b"},{"author":{"_account_id":308,"name":"Thierry Carrez","email":"thierry@openstack.org","username":"ttx"},"change_message_id":"331f9460b0ef55fab047615e4c45b9304e39d94a","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            exec_dirs \u003d config.get(\"DEFAULT\", \"exec_dirs\").split(\",\")"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            # Use system PATH if exec_dirs is not specified"},{"line_number":73,"context_line":"            exec_dirs \u003d os.environ[\"PATH\"].split(\u0027:\u0027)"},{"line_number":74,"context_line":"    except ConfigParser.Error:"},{"line_number":75,"context_line":"        print \"%s: Incorrect configuration file: %s\" % (execname, configfile)"},{"line_number":76,"context_line":"        sys.exit(RC_BADCONFIG)"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAMX%2F%2F%2FcA%3D","line":73,"in_reply_to":"AAAAMX%2F%2F%2FdY%3D","updated":"2012-12-03 15:33:46.000000000","message":"nova-rootwrap is not using the PATH env of the nova process! It\u0027s using a PATH that was sanitized by sudo (through env_reset), which usually is the default system PATH.","commit_id":"12e264d58f052f192f3408f5cd8637809eff085b"}],"nova/rootwrap/filters.py":[{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"5fbc0c57b8ecc37ca1c9c9759466308d5ac314a3","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                expanded_path \u003d os.path.join(binary_path, self.exec_path)"},{"line_number":42,"context_line":"                if os.access(expanded_path, os.X_OK):"},{"line_number":43,"context_line":"                    self.real_exec \u003d expanded_path"},{"line_number":44,"context_line":"                    break"},{"line_number":45,"context_line":"        return self.real_exec"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def match(self, userargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2FtEI%3D","line":44,"updated":"2012-11-29 16:49:41.000000000","message":"I wondered the same thing as Kevin here\n\nWould prefer to see an exception raised here rather than trying to run \u0027\u0027 with sudo","commit_id":"1a4e1e0645a95ea5c3731011d8b6a9a4a107f42c"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"51deff591c2fafe42d665bbb5e1f81fbc99c2837","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                expanded_path \u003d os.path.join(binary_path, self.exec_path)"},{"line_number":42,"context_line":"                if os.access(expanded_path, os.X_OK):"},{"line_number":43,"context_line":"                    self.real_exec \u003d expanded_path"},{"line_number":44,"context_line":"                    break"},{"line_number":45,"context_line":"        return self.real_exec"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def match(self, userargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2FsF0%3D","line":44,"in_reply_to":"AAAAMH%2F%2Fs9Y%3D","updated":"2012-11-30 12:54:45.000000000","message":"Ok, I missed the \"or self.exec_path\" bit","commit_id":"1a4e1e0645a95ea5c3731011d8b6a9a4a107f42c"},{"author":{"_account_id":308,"name":"Thierry Carrez","email":"thierry@openstack.org","username":"ttx"},"change_message_id":"02db26a4212f91421cf990da01980a2968748278","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                expanded_path \u003d os.path.join(binary_path, self.exec_path)"},{"line_number":42,"context_line":"                if os.access(expanded_path, os.X_OK):"},{"line_number":43,"context_line":"                    self.real_exec \u003d expanded_path"},{"line_number":44,"context_line":"                    break"},{"line_number":45,"context_line":"        return self.real_exec"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def match(self, userargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2Fs9Y%3D","line":44,"in_reply_to":"AAAAMH%2F%2FtEI%3D","updated":"2012-11-29 17:45:05.000000000","message":"Note that we are *not* trying to run \u0027\u0027 through sudo (which I\u0027d agree would be bad). If get_exec() returns the empty string, then self.exec_path is used (\u0027\u0027 or self.exec_path), and that is never empty. Let me know if it\u0027s still a blocker. Otherwise I\u0027d rather fix that (raise an exception rather than try to run something we already know is not executable) as a separate bug... as it changes the current behavior.\n\nBased on your answer I\u0027ll do a direct rebase or fix the bug /and/ do the rebase.","commit_id":"1a4e1e0645a95ea5c3731011d8b6a9a4a107f42c"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"3db9ef2cefb3d929d8c97371162744be751b4107","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def get_command(self, userargs, exec_dirs\u003d[]):"},{"line_number":54,"context_line":"        \"\"\"Returns command to execute (with sudo -u if run_as !\u003d root).\"\"\""},{"line_number":55,"context_line":"        to_exec \u003d self.get_exec(exec_dirs\u003dexec_dirs) or self.exec_path"},{"line_number":56,"context_line":"        if (self.run_as !\u003d \u0027root\u0027):"},{"line_number":57,"context_line":"            # Used to run commands at lesser privileges"},{"line_number":58,"context_line":"            return [\u0027sudo\u0027, \u0027-u\u0027, self.run_as, to_exec] + userargs[1:]"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2FuFU%3D","line":55,"updated":"2012-11-28 19:56:50.000000000","message":"What happens if the executable cannot be found or isn\u0027t in fact executable?  You end up trying to execute the executable named \"\", which likely doesn\u0027t exist.  There should probably be an error check here, or a raise in get_exec()…","commit_id":"1a4e1e0645a95ea5c3731011d8b6a9a4a107f42c"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"ffca65950405f8f8da19076e95348274b72f0e9c","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def get_command(self, userargs, exec_dirs\u003d[]):"},{"line_number":54,"context_line":"        \"\"\"Returns command to execute (with sudo -u if run_as !\u003d root).\"\"\""},{"line_number":55,"context_line":"        to_exec \u003d self.get_exec(exec_dirs\u003dexec_dirs) or self.exec_path"},{"line_number":56,"context_line":"        if (self.run_as !\u003d \u0027root\u0027):"},{"line_number":57,"context_line":"            # Used to run commands at lesser privileges"},{"line_number":58,"context_line":"            return [\u0027sudo\u0027, \u0027-u\u0027, self.run_as, to_exec] + userargs[1:]"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2Ft2c%3D","line":55,"in_reply_to":"AAAAMH%2F%2Ft68%3D","updated":"2012-11-28 22:45:01.000000000","message":"OK, that sounds reasonable…","commit_id":"1a4e1e0645a95ea5c3731011d8b6a9a4a107f42c"},{"author":{"_account_id":308,"name":"Thierry Carrez","email":"thierry@openstack.org","username":"ttx"},"change_message_id":"db14751225ee340c74a07cda29e283bbeb5af5c5","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def get_command(self, userargs, exec_dirs\u003d[]):"},{"line_number":54,"context_line":"        \"\"\"Returns command to execute (with sudo -u if run_as !\u003d root).\"\"\""},{"line_number":55,"context_line":"        to_exec \u003d self.get_exec(exec_dirs\u003dexec_dirs) or self.exec_path"},{"line_number":56,"context_line":"        if (self.run_as !\u003d \u0027root\u0027):"},{"line_number":57,"context_line":"            # Used to run commands at lesser privileges"},{"line_number":58,"context_line":"            return [\u0027sudo\u0027, \u0027-u\u0027, self.run_as, to_exec] + userargs[1:]"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2Ft68%3D","line":55,"in_reply_to":"AAAAMH%2F%2FuFU%3D","updated":"2012-11-28 22:03:13.000000000","message":"Then you end up trying to execute exec_path and fail (\"no such file or directory\"). That\u0027s how the code currently works, to emulate what happened when you just used \"sudo\" to call executables. So this is just continuing to do the same.","commit_id":"1a4e1e0645a95ea5c3731011d8b6a9a4a107f42c"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"250fdae18584bf06915db8ef71352f1987b413a1","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        else:"},{"line_number":40,"context_line":"            for binary_path in exec_dirs:"},{"line_number":41,"context_line":"                expanded_path \u003d os.path.join(binary_path, self.exec_path)"},{"line_number":42,"context_line":"                if os.access(expanded_path, os.X_OK):"},{"line_number":43,"context_line":"                    self.real_exec \u003d expanded_path"},{"line_number":44,"context_line":"                    break"},{"line_number":45,"context_line":"        return self.real_exec"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAMX%2F%2F%2FgE%3D","line":42,"updated":"2012-12-03 15:16:41.000000000","message":"It\u0027s not that common, but it\u0027s possible for binaries to be not X_OK for the nova user, but be executable through sudo. For example on my system, mount.ntfs-3g is -rwxr-xr--\n\nIf you changed to using F_OK (equiv to os.path.isfile()), then I suppose there is the edge case where a non executable name could be chosen over an executable one. To avoid that, you could also check for any exec bits:\n\n    try:\n        s \u003d os.stat(path_to_test)\n        if stat.S_ISREG(s.st_mode):\n            if (s.st_mode \u0026 (stat.S_IXUSR|stat.S_IXGRP|stat.S_IXOTH)) !\u003d 0:\n                break\n    except OSError:\n        pass\n\nNote by manually testing file mode bits, you bypass additional perm checks done through access(), but TBH that\u0027s probably best done in the execution context of sudo anyway, otherwise you\u0027d be giving confusing file not found messages, when the access issue could very well be something else.","commit_id":"12e264d58f052f192f3408f5cd8637809eff085b"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"8d571f1880210f86d520a02f6719bb308e986abe","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        else:"},{"line_number":40,"context_line":"            for binary_path in exec_dirs:"},{"line_number":41,"context_line":"                expanded_path \u003d os.path.join(binary_path, self.exec_path)"},{"line_number":42,"context_line":"                if os.access(expanded_path, os.X_OK):"},{"line_number":43,"context_line":"                    self.real_exec \u003d expanded_path"},{"line_number":44,"context_line":"                    break"},{"line_number":45,"context_line":"        return self.real_exec"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAMX%2F%2F%2Fa0%3D","line":42,"in_reply_to":"AAAAMX%2F%2F%2Fb8%3D","updated":"2012-12-03 15:58:21.000000000","message":"Oh right sorry. +1 so","commit_id":"12e264d58f052f192f3408f5cd8637809eff085b"},{"author":{"_account_id":308,"name":"Thierry Carrez","email":"thierry@openstack.org","username":"ttx"},"change_message_id":"331f9460b0ef55fab047615e4c45b9304e39d94a","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        else:"},{"line_number":40,"context_line":"            for binary_path in exec_dirs:"},{"line_number":41,"context_line":"                expanded_path \u003d os.path.join(binary_path, self.exec_path)"},{"line_number":42,"context_line":"                if os.access(expanded_path, os.X_OK):"},{"line_number":43,"context_line":"                    self.real_exec \u003d expanded_path"},{"line_number":44,"context_line":"                    break"},{"line_number":45,"context_line":"        return self.real_exec"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAMX%2F%2F%2Fb8%3D","line":42,"in_reply_to":"AAAAMX%2F%2F%2FgE%3D","updated":"2012-12-03 15:33:46.000000000","message":"Remember nova-rootwrap is run as root, so that X_OK test is done as root, not as the nova user... so I think it gives accurate results ?","commit_id":"12e264d58f052f192f3408f5cd8637809eff085b"}]}