use nsz package for nut and Fs#12
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNSTools is refactored to delegate local filesystem, container, crypto, and utility implementations to ChangesFilesystem and Format Module Migration
Estimated code review effort: 🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@py/requirements.txt`:
- Line 1: The git dependency line for nsz
(git+https://github.com/nicoboss/nsz@c7b8d3a5617c16050966c6d9aca6a3a4b5e9f9d2)
should not be blamed for an OSV finding against requests — update
py/requirements.txt to (1) keep the nsz git URL if needed but add a short inline
note that nsz@c7b8d3a... only lists pycryptodome, zstandard, and enlighten, (2)
explicitly pin pycryptodome to a known non-vulnerable version (replace the
unpinned pycryptodome coming from nsz with pycryptodome==<safe-version> in your
requirements to override transitive unpinned use), and (3) explicitly pin
requests==<safe-version> (since its source is elsewhere) so the project’s
dependency graph is deterministically using safe releases; ensure the chosen
versions are backed by CVE/OSV fixes before committing.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fe05238c-6f81-4ae6-89a8-e7245460a30e
📒 Files selected for processing (24)
py/nstools/Fs/BaseFs.pypy/nstools/Fs/Bktr.pypy/nstools/Fs/Cnmt.pypy/nstools/Fs/File.pypy/nstools/Fs/Hfs0.pypy/nstools/Fs/Ivfc.pypy/nstools/Fs/Nacp.pypy/nstools/Fs/Nca.pypy/nstools/Fs/Nsp.pypy/nstools/Fs/Pfs0.pypy/nstools/Fs/Rom.pypy/nstools/Fs/Ticket.pypy/nstools/Fs/Type.pypy/nstools/Fs/Xci.pypy/nstools/Fs/__init__.pypy/nstools/__init__.pypy/nstools/lib/Verify.pypy/nstools/nut/Hex.pypy/nstools/nut/Keys.pypy/nstools/nut/Print.pypy/nstools/nut/Titles.pypy/nstools/nut/__init__.pypy/nstools/nut/aes128.pypy/requirements.txt
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
py/ns_verify_folder.py (1)
64-80:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPath objects are treated as strings in the scan loop and will crash verification.
itemis aPath, soitem.lower()(Line 68) raisesAttributeError, and passingitemdirectly toVerify.parse_name()(Line 80) can raiseTypeError. Keep file name/path types consistent in the loop.Suggested fix
- for item in sorted(list(Path(ipath).iterdir())): - item_path = fsPathJoin(ipath, item) - if not fsIsFile(item_path): + for item in sorted(Path(ipath).iterdir()): + if not item.is_file(): continue - if not item.lower().endswith(('.xci', '.xcz', '.nsp', '.nsz')): + item_name = item.name + if not item_name.lower().endswith(('.xci', '.xcz', '.nsp', '.nsz')): continue - files.append(item) + files.append(item_name) @@ - item_path = fsPathJoin(ipath, item) + item_path = fsPathJoin(ipath, item) @@ - data = Verify.parse_name(item) + data = Verify.parse_name(item)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@py/ns_verify_folder.py` around lines 64 - 80, The loop treats Path objects as strings which causes AttributeError/TypeError; fix by normalizing to strings early: when iterating over Path items, extract the filename (e.g., name = item.name or filename = str(item)) and use that for comparisons (filename.lower().endswith(...)), for files.append(filename) and for Verify.parse_name(filename); also ensure fsPathJoin(ipath, ...) receives the string filename (or str(item_path)) so send_hook and Verify.parse_name get consistent string paths/names instead of Path objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@py/ns_verify_folder.py`:
- Around line 49-52: The webhook POST using req_post(WHOOK_URL,
data=json_dumps(payload), headers=headers) currently has no timeout and uses a
bare except that swallows errors; update the call to include a sensible timeout
(e.g., timeout=5) and replace the bare except with explicit exception handling
for requests.exceptions.RequestException (and optionally
requests.exceptions.Timeout) around the req_post/response.raise_for_status
calls, then log or surface the exception instead of silently passing so failures
are visible (refer to req_post, WHOOK_URL, payload, and
response.raise_for_status to locate the code).
---
Outside diff comments:
In `@py/ns_verify_folder.py`:
- Around line 64-80: The loop treats Path objects as strings which causes
AttributeError/TypeError; fix by normalizing to strings early: when iterating
over Path items, extract the filename (e.g., name = item.name or filename =
str(item)) and use that for comparisons (filename.lower().endswith(...)), for
files.append(filename) and for Verify.parse_name(filename); also ensure
fsPathJoin(ipath, ...) receives the string filename (or str(item_path)) so
send_hook and Verify.parse_name get consistent string paths/names instead of
Path objects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5aa61aa8-21b5-41df-9eed-c70710ff0d20
📒 Files selected for processing (11)
py/ns_extract_hashes.pypy/ns_extract_meta.pypy/ns_verify_folder.pypy/nstools/lib/BlockDecompressorReader.pypy/nstools/lib/FsCert.pypy/nstools/lib/FsNcaMod.pypy/nstools/lib/FsTools.pypy/nstools/lib/Header.pypy/nstools/lib/PathTools.pypy/nstools/lib/Verify.pypy/nstools/lib/VerifyTools.py
💤 Files with no reviewable changes (3)
- py/nstools/lib/Header.py
- py/nstools/lib/BlockDecompressorReader.py
- py/nstools/lib/PathTools.py
✅ Files skipped from review due to trivial changes (3)
- py/ns_extract_meta.py
- py/nstools/lib/FsTools.py
- py/nstools/lib/FsCert.py
| response = req_post(WHOOK_URL, data=json_dumps(payload), headers=headers) | ||
| response.raise_for_status() | ||
| except: | ||
| pass |
There was a problem hiding this comment.
Webhook post should have a timeout and explicit exception handling.
The request can hang indefinitely without a timeout, and bare except hides transport/HTTP failures.
Suggested fix
+from requests import RequestException
@@
- response = req_post(WHOOK_URL, data=json_dumps(payload), headers=headers)
+ response = req_post(WHOOK_URL, data=json_dumps(payload), headers=headers, timeout=10)
response.raise_for_status()
- except:
- pass
+ except RequestException:
+ pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| response = req_post(WHOOK_URL, data=json_dumps(payload), headers=headers) | |
| response.raise_for_status() | |
| except: | |
| pass | |
| response = req_post(WHOOK_URL, data=json_dumps(payload), headers=headers, timeout=10) | |
| response.raise_for_status() | |
| except RequestException: | |
| pass |
🧰 Tools
🪛 Ruff (0.15.15)
[error] 49-49: Probable use of requests call without timeout
(S113)
[error] 51-51: Do not use bare except
(E722)
[error] 51-52: try-except-pass detected, consider logging the exception
(S110)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@py/ns_verify_folder.py` around lines 49 - 52, The webhook POST using
req_post(WHOOK_URL, data=json_dumps(payload), headers=headers) currently has no
timeout and uses a bare except that swallows errors; update the call to include
a sensible timeout (e.g., timeout=5) and replace the bare except with explicit
exception handling for requests.exceptions.RequestException (and optionally
requests.exceptions.Timeout) around the req_post/response.raise_for_status
calls, then log or surface the exception instead of silently passing so failures
are visible (refer to req_post, WHOOK_URL, payload, and
response.raise_for_status to locate the code).
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit
Bug Fixes
Refactor
Chores