)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"719fc5002401cd02f9cec50ffcc41d6e795b2f88","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Validate aggregate IDs before querying database"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova API\u0027s aggregate ID lookup does not require an exact match."},{"line_number":10,"context_line":"Alphanumeric strings can possibly be truncated and converted to"},{"line_number":11,"context_line":"integers incorrectly. For example send PUT request in"},{"line_number":12,"context_line":"\u0027\u003cNOVA API\u003e/os_aggregates/\u003cmodified aggregate ID\u003e\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_e4df8409","line":9,"updated":"2019-12-09 21:42:23.000000000","message":"The aggregate ID in the URL path should always be an integer. Why would someone send 13a2g152asdf?\n\nIs this a security hardening bug or working around something to enable out of tree code? If the former, rather than add a workaround option (which should always be enabled when using mysql, which is the vast majority of deployments), why not just validate that the id is an integer in the API route handler code?","commit_id":"d3ef7884871ac1c1f0461f97e73a6b62e6360008"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"1cd5410ff3e617872b67ad5dd9156c5e15e0040b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Validate aggregate IDs before querying database"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova API\u0027s aggregate ID lookup does not require an exact match."},{"line_number":10,"context_line":"Alphanumeric strings can possibly be truncated and converted to"},{"line_number":11,"context_line":"integers incorrectly. For example send PUT request in"},{"line_number":12,"context_line":"\u0027\u003cNOVA API\u003e/os_aggregates/\u003cmodified aggregate ID\u003e\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_d021d4fd","line":9,"in_reply_to":"3fa7e38b_8407b070","updated":"2019-12-10 16:35:13.000000000","message":"Done","commit_id":"d3ef7884871ac1c1f0461f97e73a6b62e6360008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"30b391b4b7cca1082233a74e11c3f4a6b41337b1","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Validate aggregate IDs before querying database"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova API\u0027s aggregate ID lookup does not require an exact match."},{"line_number":10,"context_line":"Alphanumeric strings can possibly be truncated and converted to"},{"line_number":11,"context_line":"integers incorrectly. For example send PUT request in"},{"line_number":12,"context_line":"\u0027\u003cNOVA API\u003e/os_aggregates/\u003cmodified aggregate ID\u003e\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_8407b070","line":9,"in_reply_to":"3fa7e38b_e4df8409","updated":"2019-12-09 21:45:56.000000000","message":"Here is an example that comes to mind:\n\nhttps://github.com/openstack/nova/blob/master/nova/api/openstack/compute/services.py#L245","commit_id":"d3ef7884871ac1c1f0461f97e73a6b62e6360008"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"1cd5410ff3e617872b67ad5dd9156c5e15e0040b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Validate aggregate IDs before querying database"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova API\u0027s aggregate ID lookup does not require an exact match."},{"line_number":10,"context_line":"Alphanumeric strings can possibly be truncated and converted to"},{"line_number":11,"context_line":"integers incorrectly. For example send PUT request in"},{"line_number":12,"context_line":"\u0027\u003cNOVA API\u003e/os_aggregates/\u003cmodified aggregate ID\u003e\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_f0089080","line":9,"in_reply_to":"3fa7e38b_e4df8409","updated":"2019-12-10 16:35:13.000000000","message":"Yes, one of our security checks noticed that. It\u0027s completely safe and the change is unnecessary. But probably nice to have so we would have consistency in API.","commit_id":"d3ef7884871ac1c1f0461f97e73a6b62e6360008"}],"nova/api/openstack/compute/aggregates.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"420526560cd5c885668ec409333c218f43594058","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        return agg"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":97,"context_line":"    def show(self, req, id):"},{"line_number":98,"context_line":"        \"\"\"Shows the details of an aggregate, hosts and metadata included.\"\"\""},{"line_number":99,"context_line":"        context \u003d _get_context(req)"},{"line_number":100,"context_line":"        context.can(aggr_policies.POLICY_ROOT % \u0027show\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c35eed43","line":97,"updated":"2019-12-10 20:50:01.000000000","message":"technically the same issue","commit_id":"e59660dcdca52fcb08d9f6b24601df3400def9ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"420526560cd5c885668ec409333c218f43594058","unresolved":false,"context_lines":[{"line_number":135,"context_line":"    # as this operation complete the deletion of aggregate resource and return"},{"line_number":136,"context_line":"    # no response body."},{"line_number":137,"context_line":"    @wsgi.expected_errors((400, 404))"},{"line_number":138,"context_line":"    def delete(self, req, id):"},{"line_number":139,"context_line":"        \"\"\"Removes an aggregate by id.\"\"\""},{"line_number":140,"context_line":"        context \u003d _get_context(req)"},{"line_number":141,"context_line":"        context.can(aggr_policies.POLICY_ROOT % \u0027delete\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_e370c9dc","line":138,"updated":"2019-12-10 20:50:01.000000000","message":"What about this method? If you passed a bogus id and mysql masked it so the wrong aggregate is deleted, wouldn\u0027t that be a problem?\n\nWith all of the places we could have the same problem with an invalid id that\u0027s why I suggested writing a simple decorator.","commit_id":"e59660dcdca52fcb08d9f6b24601df3400def9ad"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"16bc5e8e63fd21debcbcfd3aa72c1174b3cf5762","unresolved":false,"context_lines":[{"line_number":135,"context_line":"    # as this operation complete the deletion of aggregate resource and return"},{"line_number":136,"context_line":"    # no response body."},{"line_number":137,"context_line":"    @wsgi.expected_errors((400, 404))"},{"line_number":138,"context_line":"    def delete(self, req, id):"},{"line_number":139,"context_line":"        \"\"\"Removes an aggregate by id.\"\"\""},{"line_number":140,"context_line":"        context \u003d _get_context(req)"},{"line_number":141,"context_line":"        context.can(aggr_policies.POLICY_ROOT % \u0027delete\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_b84df399","line":138,"in_reply_to":"3fa7e38b_e370c9dc","updated":"2019-12-13 03:59:01.000000000","message":"Is this what you have in your mind? (latest PS)","commit_id":"e59660dcdca52fcb08d9f6b24601df3400def9ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"420526560cd5c885668ec409333c218f43594058","unresolved":false,"context_lines":[{"line_number":152,"context_line":"    @wsgi.expected_errors((404, 409))"},{"line_number":153,"context_line":"    @wsgi.action(\u0027add_host\u0027)"},{"line_number":154,"context_line":"    @validation.schema(aggregates.add_host)"},{"line_number":155,"context_line":"    def _add_host(self, req, id, body):"},{"line_number":156,"context_line":"        \"\"\"Adds a host to the specified aggregate.\"\"\""},{"line_number":157,"context_line":"        host \u003d body[\u0027add_host\u0027][\u0027host\u0027]"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_837b15bd","line":155,"updated":"2019-12-10 20:50:01.000000000","message":"same","commit_id":"e59660dcdca52fcb08d9f6b24601df3400def9ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"420526560cd5c885668ec409333c218f43594058","unresolved":false,"context_lines":[{"line_number":175,"context_line":"    @wsgi.expected_errors((404, 409))"},{"line_number":176,"context_line":"    @wsgi.action(\u0027remove_host\u0027)"},{"line_number":177,"context_line":"    @validation.schema(aggregates.remove_host)"},{"line_number":178,"context_line":"    def _remove_host(self, req, id, body):"},{"line_number":179,"context_line":"        \"\"\"Removes a host from the specified aggregate.\"\"\""},{"line_number":180,"context_line":"        host \u003d body[\u0027remove_host\u0027][\u0027host\u0027]"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_a37651d4","line":178,"updated":"2019-12-10 20:50:01.000000000","message":"same","commit_id":"e59660dcdca52fcb08d9f6b24601df3400def9ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"420526560cd5c885668ec409333c218f43594058","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    @wsgi.expected_errors((400, 404))"},{"line_number":205,"context_line":"    @wsgi.action(\u0027set_metadata\u0027)"},{"line_number":206,"context_line":"    @validation.schema(aggregates.set_metadata)"},{"line_number":207,"context_line":"    def _set_metadata(self, req, id, body):"},{"line_number":208,"context_line":"        \"\"\"Replaces the aggregate\u0027s existing metadata with new metadata.\"\"\""},{"line_number":209,"context_line":"        context \u003d _get_context(req)"},{"line_number":210,"context_line":"        context.can(aggr_policies.POLICY_ROOT % \u0027set_metadata\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_434bdd85","line":207,"updated":"2019-12-10 20:50:01.000000000","message":"same","commit_id":"e59660dcdca52fcb08d9f6b24601df3400def9ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"420526560cd5c885668ec409333c218f43594058","unresolved":false,"context_lines":[{"line_number":247,"context_line":"    @wsgi.response(202)"},{"line_number":248,"context_line":"    @wsgi.expected_errors((400, 404))"},{"line_number":249,"context_line":"    @validation.schema(aggregate_images.aggregate_images_v2_81)"},{"line_number":250,"context_line":"    def images(self, req, id, body):"},{"line_number":251,"context_line":"        \"\"\"Allows image cache management requests.\"\"\""},{"line_number":252,"context_line":"        context \u003d _get_context(req)"},{"line_number":253,"context_line":"        context.can(aggr_policies.NEW_POLICY_ROOT % \u0027images\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_634e1997","line":250,"updated":"2019-12-10 20:50:01.000000000","message":"same","commit_id":"e59660dcdca52fcb08d9f6b24601df3400def9ad"}],"nova/conf/workarounds.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"719fc5002401cd02f9cec50ffcc41d6e795b2f88","unresolved":false,"context_lines":[{"line_number":248,"context_line":"scheduler will only request ``PCPU``-based allocations."},{"line_number":249,"context_line":"\"\"\"),"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    cfg.BoolOpt(\"validate_aggregate_ids\","},{"line_number":252,"context_line":"        default\u003dFalse,"},{"line_number":253,"context_line":"        help\u003d\"\"\""},{"line_number":254,"context_line":"This value enables validation of aggregate IDs passed in the path. It exists to"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_04d420e9","line":251,"updated":"2019-12-09 21:42:23.000000000","message":"I really don\u0027t think a workaround option is valid/necessary for this. The aggregate ID in the URL path should always be an integer regardless of DB backend.","commit_id":"d3ef7884871ac1c1f0461f97e73a6b62e6360008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"719fc5002401cd02f9cec50ffcc41d6e795b2f88","unresolved":false,"context_lines":[{"line_number":252,"context_line":"        default\u003dFalse,"},{"line_number":253,"context_line":"        help\u003d\"\"\""},{"line_number":254,"context_line":"This value enables validation of aggregate IDs passed in the path. It exists to"},{"line_number":255,"context_line":"workaround the fact that MySQL stips letters from the end of string such as"},{"line_number":256,"context_line":"\u0027123aa\u0027 when trying to convert to integer."},{"line_number":257,"context_line":"\"\"\"),"},{"line_number":258,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_a4974c39","line":255,"range":{"start_line":255,"start_character":31,"end_line":255,"end_character":36},"updated":"2019-12-09 21:42:23.000000000","message":"strips","commit_id":"d3ef7884871ac1c1f0461f97e73a6b62e6360008"}],"nova/objects/aggregate.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"719fc5002401cd02f9cec50ffcc41d6e795b2f88","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    def get_by_id(cls, context, aggregate_id):"},{"line_number":277,"context_line":"        if CONF.workarounds.validate_aggregate_ids:"},{"line_number":278,"context_line":"            try:"},{"line_number":279,"context_line":"                int(aggregate_id)"},{"line_number":280,"context_line":"            except ValueError:"},{"line_number":281,"context_line":"                raise exception.AggregateNotFound(aggregate_id\u003daggregate_id)"},{"line_number":282,"context_line":"        db_aggregate \u003d _aggregate_get_from_db(context, aggregate_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_a4fe6c67","line":279,"updated":"2019-12-09 21:42:23.000000000","message":"It would be nice to just handle this generically in the route handler code in the API rather than down here in the object code, but I realize that might lead to some copy/paste code. Could we use a simple decorator to validate the id passed to methods like update() in the API?","commit_id":"d3ef7884871ac1c1f0461f97e73a6b62e6360008"}]}