)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f99bc3cf69ac9a4df151a5fea633e5e5411e8615","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"01829dc6_32d4d947","updated":"2024-12-17 09:53:51.000000000","message":"I don\u0027t *like* any of the patterns that I got to trigger bandit on older python, I was just curious about the effect of the shimming. I mostly wanted to be sure that I wasn\u0027t alone in noticing the lack of bandit warnings.\n\nUseful to know the FIPS jobs detect violations, and we\u0027ve talked about making at least one of them voting to get some py3 s3api tests not-skipping.","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f0affeaeceec59df84e04b77957afb40c9b5bc7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ff951275_0cdc30ee","updated":"2024-12-12 10:43:45.000000000","message":"This makes sense: better to exclude as we find it necessary than to include and grow stale.\n\nHowever, I suspect the shimming of md5 may be defeating bandit detecting the usedforsecurity keyword. That probably needs to be considered in a follow up, although I\u0027ve yet to come up with a solution other than repeating the try/import/except code in every module.\n\nPausing on a +A until we agree on plan for md5.\n\nFWIW, this is NOT detected by bandit:\n```\nmd5 \u003d hashlib.md5\nmd5(usedforsecurity\u003dTrue)\n```\n\nnor\n\n```\ntry:\n    from hashlib import md5\nexcept TypeError:\n    from swift.common.utils.base import md5_shim as md5\nmd5(usedforsecurity\u003dTrue)\n```\n\nbut this IS detected:\n\n```\nfrom swift.common.utils.base import md5_shim\ntry:\n    from hashlib import md5\nexcept TypeError:\n    md5 \u003d md5_shim\nmd5(usedforsecurity\u003dTrue)\n```","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"44bdc88fec04a6dafdfe00b23af12ac5e28e0f7f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"695ffda2_04dee4c7","in_reply_to":"ff951275_0cdc30ee","updated":"2024-12-16 19:29:18.000000000","message":"\u003e Pausing on a +A until we agree on plan for md5.\n\nAre we OK with a plan of\n\n- wait until we\u0027ve dropped support for py27, py36, py37, and py38\n- remove our shim, and just have everyone use hashlib directly again\n\n? \u0027Cause that was my plan.\n\n\u003e but this IS detected: ...\n\nBut that doesn\u0027t actually use our shim, does it? Or was there supposed to be a `md5(usedforsecurity\u003dFalse)` inside the `try:`? That feels like a fair bit of scarring to add to every file where we use `md5`...\n\nFWIW, the FIPS jobs fail if we get it wrong (since they don\u0027t allow MD5 *except* with `usedforsecurity\u003dFalse`) -- we just don\u0027t have any tests for `swift-account-audit` (!!)","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"}],"bandit.yaml":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48c71d4795fe82d50c2e852dbd3c75346397f615","unresolved":true,"context_lines":[{"line_number":84,"context_line":"# (optional) list skipped test IDs here, eg \u0027[B101, B406]\u0027:"},{"line_number":85,"context_line":"skips:"},{"line_number":86,"context_line":"  # We default to binding to all interfaces"},{"line_number":87,"context_line":"  - B104"},{"line_number":88,"context_line":"  # Yes, we sometimes catch just to quietly swallow an exception"},{"line_number":89,"context_line":"  - B110"},{"line_number":90,"context_line":"  # We use insecure randomness all over the place, because"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"03c6e3cd_1d792bfa","line":87,"updated":"2024-12-10 23:35:45.000000000","message":"We could consider adding `# nosec`s in the appropriate places, but IDK that it buys us anything -- the fact remains that we default to binding to all interfaces, and that seems unlikely to change.","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48c71d4795fe82d50c2e852dbd3c75346397f615","unresolved":true,"context_lines":[{"line_number":86,"context_line":"  # We default to binding to all interfaces"},{"line_number":87,"context_line":"  - B104"},{"line_number":88,"context_line":"  # Yes, we sometimes catch just to quietly swallow an exception"},{"line_number":89,"context_line":"  - B110"},{"line_number":90,"context_line":"  # We use insecure randomness all over the place, because"},{"line_number":91,"context_line":"  # it\u0027s exceedingly rare that we need secure randomness"},{"line_number":92,"context_line":"  - B311"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"e17ab34e_1171086a","line":89,"updated":"2024-12-10 23:35:45.000000000","message":"This is probably worth adding `# nosec`s in a follow-on; there are only 8 occurrences flagged.","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48c71d4795fe82d50c2e852dbd3c75346397f615","unresolved":true,"context_lines":[{"line_number":89,"context_line":"  - B110"},{"line_number":90,"context_line":"  # We use insecure randomness all over the place, because"},{"line_number":91,"context_line":"  # it\u0027s exceedingly rare that we need secure randomness"},{"line_number":92,"context_line":"  - B311"},{"line_number":93,"context_line":"  # We dynamically build SQL all over the place"},{"line_number":94,"context_line":"  - B608"},{"line_number":95,"context_line":"  # We often use subprocesses, and require a lot of trust in our use of them"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"e595ca42_bbfe10d1","line":92,"updated":"2024-12-10 23:35:45.000000000","message":"This one is frustrating: there\u0027s exactly one subtree (`swift/common/middleware/crypto/`) where it would be worth enabling; nowhere else actually warrants cryptographic-level randomness.","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48c71d4795fe82d50c2e852dbd3c75346397f615","unresolved":true,"context_lines":[{"line_number":91,"context_line":"  # it\u0027s exceedingly rare that we need secure randomness"},{"line_number":92,"context_line":"  - B311"},{"line_number":93,"context_line":"  # We dynamically build SQL all over the place"},{"line_number":94,"context_line":"  - B608"},{"line_number":95,"context_line":"  # We often use subprocesses, and require a lot of trust in our use of them"},{"line_number":96,"context_line":"  - B404"},{"line_number":97,"context_line":"  - B603"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"9143f0af_6c9ae219","line":94,"updated":"2024-12-10 23:35:45.000000000","message":"**40** false-positives! Ugh.","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48c71d4795fe82d50c2e852dbd3c75346397f615","unresolved":true,"context_lines":[{"line_number":96,"context_line":"  - B404"},{"line_number":97,"context_line":"  - B603"},{"line_number":98,"context_line":"  - B607"},{"line_number":99,"context_line":"  # We parse xml"},{"line_number":100,"context_line":"  - B320"},{"line_number":101,"context_line":"  - B405"},{"line_number":102,"context_line":"  - B410"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"67945d1e_d886cf11","line":99,"updated":"2024-12-10 23:35:45.000000000","message":"We should at least *consider* pulling in [defusedxml](https://pypi.org/project/defusedxml/), though.","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f0affeaeceec59df84e04b77957afb40c9b5bc7","unresolved":true,"context_lines":[{"line_number":100,"context_line":"  - B320"},{"line_number":101,"context_line":"  - B405"},{"line_number":102,"context_line":"  - B410"},{"line_number":103,"context_line":"  - B603"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"### (optional) plugin settings - some test plugins require configuration data"},{"line_number":106,"context_line":"### that may be given here, per-plugin. All bandit test plugins have a built in"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"576778fa_565117b3","line":103,"updated":"2024-12-12 10:43:45.000000000","message":"ok, there is nothing in the exclusions that we previously included","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"}],"swift/cli/account_audit.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f0affeaeceec59df84e04b77957afb40c9b5bc7","unresolved":true,"context_lines":[{"line_number":91,"context_line":"                    conn \u003d http_connect(node[\u0027ip\u0027], node[\u0027port\u0027],"},{"line_number":92,"context_line":"                                        node[\u0027device\u0027], part, \u0027GET\u0027, path, {})"},{"line_number":93,"context_line":"                    resp \u003d conn.getresponse()"},{"line_number":94,"context_line":"                    calc_hash \u003d md5(usedforsecurity\u003dFalse)"},{"line_number":95,"context_line":"                    chunk \u003d True"},{"line_number":96,"context_line":"                    while chunk:"},{"line_number":97,"context_line":"                        chunk \u003d resp.read(8192)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d9f03957_ef2f6548","line":94,"updated":"2024-12-12 10:43:45.000000000","message":"if I revert this line I cannot get bandit to fail. I suspect the shim in utils/base.p is somehow defeating bandit.\n\n```\n(pep8) vagrant@vagrant:~/swift$ python --version\nPython 3.9.21\n\n(pep8) vagrant@vagrant:~/swift$ git diff\ndiff --git a/swift/cli/account_audit.py b/swift/cli/account_audit.py\nindex b809f0c7e..42e07baee 100755\n--- a/swift/cli/account_audit.py\n+++ b/swift/cli/account_audit.py\n@@ -91,7 +91,7 @@ class Auditor(object):\n                     conn \u003d http_connect(node[\u0027ip\u0027], node[\u0027port\u0027],\n                                         node[\u0027device\u0027], part, \u0027GET\u0027, path, {})\n                     resp \u003d conn.getresponse()\n-                    calc_hash \u003d md5(usedforsecurity\u003dFalse)\n+                    calc_hash \u003d md5()\n                     chunk \u003d True\n                     while chunk:\n                         chunk \u003d resp.read(8192)\n\n(pep8) vagrant@vagrant:~/swift$ bandit -c bandit.yaml -r swift/cli/account_audit.py -n 5 -v\n[main]\tINFO\tprofile include tests: None\n[main]\tINFO\tprofile exclude tests: B311,B320,B404,B405,B410,B607,B110,B603,B608,B104\n[main]\tINFO\tcli include tests: None\n[main]\tINFO\tcli exclude tests: None\n[main]\tINFO\tusing config: bandit.yaml\n[main]\tINFO\trunning on Python 3.9.21\nRun started:2024-12-12 09:43:22.677599\nFiles in scope (1):\n\t./swift/cli/account_audit.py (score: {SEVERITY: 0, CONFIDENCE: 0})\nFiles excluded (0):\n\nTest results:\n\tNo issues identified.\n\nCode scanned:\n\tTotal lines of code: 349\n\tTotal lines skipped (#nosec): 0\n\nRun metrics:\n\tTotal issues (by severity):\n\t\tUndefined: 0\n\t\tLow: 0\n\t\tMedium: 0\n\t\tHigh: 0\n\tTotal issues (by confidence):\n\t\tUndefined: 0\n\t\tLow: 0\n\t\tMedium: 0\n\t\tHigh: 0\nFiles skipped (0):\n```\n\nIf I revert the import too then bandit detects the bad use of md5:\n\n```\n(pep8) vagrant@vagrant:~/swift$ bandit -c bandit.yaml -r swift/cli/account_audit.py -n 5 -v\n[main]\tINFO\tprofile include tests: None\n[main]\tINFO\tprofile exclude tests: B320,B608,B410,B404,B405,B110,B607,B104,B311,B603\n[main]\tINFO\tcli include tests: None\n[main]\tINFO\tcli exclude tests: None\n[main]\tINFO\tusing config: bandit.yaml\n[main]\tINFO\trunning on Python 3.9.21\nRun started:2024-12-12 09:48:39.256147\nFiles in scope (1):\n\t./swift/cli/account_audit.py (score: {SEVERITY: 10, CONFIDENCE: 10})\nFiles excluded (0):\n\nTest results:\n\u003e\u003e Issue: [B324:hashlib] Use of weak MD5 hash for security. Consider usedforsecurity\u003dFalse\n   Severity: High   Confidence: High\n   CWE: CWE-327 (https://cwe.mitre.org/data/definitions/327.html)\n   More Info: https://bandit.readthedocs.io/en/1.8.0/plugins/b324_hashlib.html\n   Location: ./swift/cli/account_audit.py:94:32\n92\t                                        node[\u0027device\u0027], part, \u0027GET\u0027, path, {})\n93\t                    resp \u003d conn.getresponse()\n94\t                    calc_hash \u003d md5()\n95\t                    chunk \u003d True\n96\t                    while chunk:\n\n--------------------------------------------------\n\nCode scanned:\n\tTotal lines of code: 349\n\tTotal lines skipped (#nosec): 0\n\nRun metrics:\n\tTotal issues (by severity):\n\t\tUndefined: 0\n\t\tLow: 0\n\t\tMedium: 0\n\t\tHigh: 1\n\tTotal issues (by confidence):\n\t\tUndefined: 0\n\t\tLow: 0\n\t\tMedium: 0\n\t\tHigh: 1\nFiles skipped (0):\n```","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"}],"swift/common/middleware/bulk.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9a2453f04976962248af11054b1f18dfa2ebcde9","unresolved":true,"context_lines":[{"line_number":197,"context_line":"import json"},{"line_number":198,"context_line":"import six"},{"line_number":199,"context_line":"import tarfile"},{"line_number":200,"context_line":"from xml.sax.saxutils import escape  # nosec B406"},{"line_number":201,"context_line":"from time import time"},{"line_number":202,"context_line":"from eventlet import sleep"},{"line_number":203,"context_line":"import zlib"}],"source_content_type":"text/x-python","patch_set":1,"id":"de10bd45_a172c9f2","line":200,"updated":"2024-12-10 23:54:57.000000000","message":"This makes for *such* a stupid message:\n```\nUsing escape to parse untrusted XML data is known to be vulnerable to XML attacks.\n```","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"}],"swift/common/swob.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f0affeaeceec59df84e04b77957afb40c9b5bc7","unresolved":true,"context_lines":[{"line_number":730,"context_line":"    \"\"\""},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"    # RFC 2616 section 2.2"},{"line_number":733,"context_line":"    token \u003d r\u0027[^()\u003c\u003e@,;:\\\"/\\[\\]?\u003d{}\\x00-\\x20\\x7f]+\u0027  # nosec B105"},{"line_number":734,"context_line":"    qdtext \u003d r\u0027[^\"]\u0027"},{"line_number":735,"context_line":"    quoted_pair \u003d r\u0027(?:\\\\.)\u0027"},{"line_number":736,"context_line":"    quoted_string \u003d r\u0027\"(?:\u0027 + qdtext + r\u0027|\u0027 + quoted_pair + r\u0027)*\"\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"cd6ec63b_0c8c08dd","line":733,"updated":"2024-12-12 10:43:45.000000000","message":"ok. B105 is hardcoded_password_string :shrug:","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"}],"swift/obj/diskfile.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9a2453f04976962248af11054b1f18dfa2ebcde9","unresolved":true,"context_lines":[{"line_number":1088,"context_line":"            results[key] \u003d join(datadir, info[\u0027filename\u0027]) if info else None"},{"line_number":1089,"context_line":""},{"line_number":1090,"context_line":"        if verify and not self._verify_ondisk_files(results, **kwargs):"},{"line_number":1091,"context_line":"            raise RuntimeError("},{"line_number":1092,"context_line":"                \"On-disk file search algorithm contract is broken: %s\""},{"line_number":1093,"context_line":"                % str(results))"},{"line_number":1094,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"eabc1d11_d5ec0de8","line":1091,"updated":"2024-12-10 23:54:57.000000000","message":"I think @alistairncoles@gmail.com had ideas about raising some flavor of `DiskFileError`","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f0affeaeceec59df84e04b77957afb40c9b5bc7","unresolved":true,"context_lines":[{"line_number":1088,"context_line":"            results[key] \u003d join(datadir, info[\u0027filename\u0027]) if info else None"},{"line_number":1089,"context_line":""},{"line_number":1090,"context_line":"        if verify and not self._verify_ondisk_files(results, **kwargs):"},{"line_number":1091,"context_line":"            raise RuntimeError("},{"line_number":1092,"context_line":"                \"On-disk file search algorithm contract is broken: %s\""},{"line_number":1093,"context_line":"                % str(results))"},{"line_number":1094,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"d5ae91d6_4e499301","line":1091,"in_reply_to":"eabc1d11_d5ec0de8","updated":"2024-12-12 10:43:45.000000000","message":"ok, RuntimeError *does* extend Exception so it is caught in the object server __call__\n\nbefore:\n\n```\ndiff --git a/swift/obj/diskfile.py b/swift/obj/diskfile.py\nindex d11e8673c..89aeff848 100644\n--- a/swift/obj/diskfile.py\n+++ b/swift/obj/diskfile.py\n@@ -1087,10 +1087,7 @@ class BaseDiskFileManager(object):\n             key \u003d info_key[:-5] + \u0027_file\u0027\n             results[key] \u003d join(datadir, info[\u0027filename\u0027]) if info else None\n\n-        if verify and not self._verify_ondisk_files(results, **kwargs):\n-            raise RuntimeError(\n-                \"On-disk file search algorithm contract is broken: %s\"\n-                % str(results))\n+        assert False\n\n         return results\n\n\n\nDec 12 03:53:29 vagrant object-server-6010: ERROR __call__ error with HEAD /sdb1/252/AUTH_test/foo/LICENSE : #012Traceback (most recent call last):#012  File \"/vagrant/swift/swift/obj/server.py\", line 1363, in __call__#012    res \u003d getattr(self, req.method)(req)#012  File \"/vagrant/swift/swift/common/utils/__init__.py\", line 1002, in _timing_stats#012    resp \u003d func(ctrl, *args, **kwargs)#012  File \"/vagrant/swift/swift/obj/server.py\", line 1189, in HEAD#012    metadata \u003d disk_file.read_metadata(current_time\u003dreq_timestamp)#012  File \"/vagrant/swift/swift/obj/diskfile.py\", line 2985, in read_metadata#012    with self.open(current_time\u003dcurrent_time):#012  File \"/vagrant/swift/swift/obj/diskfile.py\", line 2637, in open#012    file_info \u003d self._get_ondisk_files(files, self.policy)#012  File \"/vagrant/swift/swift/obj/diskfile.py\", line 3108, in _get_ondisk_files#012    self._ondisk_info \u003d self.manager.get_ondisk_files(#012  File \"/vagrant/swift/swift/obj/diskfile.py\", line 1090, in get_ondisk_files#012    assert False#012AssertionError (txn: tx4f43d2fa0c3348229bf50-00675aa489)\n```\n\nafter:\n\n```\ndiff --git a/swift/obj/diskfile.py b/swift/obj/diskfile.py\nindex d11e8673c..a70b242fa 100644\n--- a/swift/obj/diskfile.py\n+++ b/swift/obj/diskfile.py\n@@ -1087,7 +1087,7 @@ class BaseDiskFileManager(object):\n             key \u003d info_key[:-5] + \u0027_file\u0027\n             results[key] \u003d join(datadir, info[\u0027filename\u0027]) if info else None\n\n-        if verify and not self._verify_ondisk_files(results, **kwargs):\n+        if True:# and not self._verify_ondisk_files(results, **kwargs):\n             raise RuntimeError(\n                 \"On-disk file search algorithm contract is broken: %s\"\n                 % str(results))\n                 \n                 \n                 \nDec 12 03:49:32 vagrant object-server-6010: ERROR __call__ error with HEAD /sdb1/252/AUTH_test/foo/LICENSE : #012Traceback (most recent call last):#012  File \"/vagrant/swift/swift/obj/server.py\", line 1363, in __call__#012    res \u003d getattr(self, req.method)(req)#012  File \"/vagrant/swift/swift/common/utils/__init__.py\", line 1002, in _timing_stats#012    resp \u003d func(ctrl, *args, **kwargs)#012  File \"/vagrant/swift/swift/obj/server.py\", line 1189, in HEAD#012    metadata \u003d disk_file.read_metadata(current_time\u003dreq_timestamp)#012  File \"/vagrant/swift/swift/obj/diskfile.py\", line 2983, in read_metadata#012    with self.open(current_time\u003dcurrent_time):#012  File \"/vagrant/swift/swift/obj/diskfile.py\", line 2635, in open#012    file_info \u003d self._get_ondisk_files(files, self.policy)#012  File \"/vagrant/swift/swift/obj/diskfile.py\", line 3106, in _get_ondisk_files#012    self._ondisk_info \u003d self.manager.get_ondisk_files(#012  File \"/vagrant/swift/swift/obj/diskfile.py\", line 1091, in get_ondisk_files#012    raise RuntimeError(#012RuntimeError: On-disk file search algorithm contract is broken: {\u0027data_file\u0027: None, \u0027meta_file\u0027: None, \u0027ts_file\u0027: None, \u0027ctype_file\u0027: None} (txn: tx0f9fb1c1de6e41c4b94a7-00675aa39c)\n```","commit_id":"e576c5cee0240fbfa87566be906a51d9ac1613b2"}]}