install: drop the query string and fragment from a tarball URL's extraction folder name - #39011
Open
robobun wants to merge 4 commits into
Open
install: drop the query string and fragment from a tarball URL's extraction folder name#39011robobun wants to merge 4 commits into
robobun wants to merge 4 commits into
Claude / Claude Code Review
completed
Aug 15, 2026 in 15m 52s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/install/extract_tarball.rs:216-220 |
b"package" fallback for URL basenames is untested |
Annotations
Check warning on line 220 in src/install/extract_tarball.rs
claude / Claude Code Review
b"package" fallback for URL basenames is untested
The `b"package"` fallback (extract_tarball.rs:216-220) — which the PR description names as part of the fix — is not exercised by any of the 6 new test cases: each URL resolves to basename `qs-pkg-1.0.0`, which passes `is_safe_install_folder_name`, so the fallback branch never runs. Consider adding one case whose URL yields an unsafe basename (e.g. `${server.origin}/?token=abc`, whose basename after cutting the query is `127.0.0.1:PORT`), so deleting the fallback breaks a test.
Loading