Conversation
|
Welcome! Thanks for your first pull request in this repository. A maintainer will review it soon. Please make sure:
Thanks for contributing. |
…le rollback resilience and dialog state [BS-11]
…ntegration [TW-EP-11]
Signed-off-by: Lukas Geiger <lukas@um-bruch.org>
Signed-off-by: Lukas Geiger <lukas@um-bruch.org>
lukisch
left a comment
There was a problem hiding this comment.
Zweitmodell-Review: CI grün (12/12), aber ein echter Sicherheitsbefund und starke Überlappung mit anderen PRs.
1. Befehlsinjektion beim Terminal-Start (src/core/platform_utils.py, get_terminal_command)
Der Verzeichnisname wird in Shell-Strings interpoliert:
- PowerShell:
f"Set-Location -LiteralPath '{directory}'". Ein Ordner mit'im Namen (unter Windows erlaubt, z. B.x'; calc; ') bricht das Quoting aus und führt Code aus. - cmd-Fallback:
f"cd /d {directory}"ist ungequotet.&im Ordnernamen führt zu einem zweiten Befehl.
open_terminal_in_directory setzt bereits cwd=target_dir, die Interpolation ist also überflüssig. Patch-Vorschlag:
if sys.platform.startswith("win"):
if shutil.which("wt"):
return ["wt", "-d", directory]
if shutil.which("powershell"):
return ["powershell", "-NoExit"] # Arbeitsverzeichnis kommt über Popen(cwd=...)
return ["cmd", "/K"]Dazu einen Test mit einem Ordner namens a'b&c.
2. Überlappung
#14 ist praktisch ein Obermengen-Stand von #15 (Properties-Dialog, Terminal, GC, Batch-Rename-Rollback) plus Laufwerkskapazität (drive_usage.py, gemeinsam mit #17). checksum_dialog.py, gui_gc.py, tests/conftest.py und main.py werden in #14, #15 und #17 jeweils unterschiedlich geändert. Es drohen Konflikte oder doppelte Arbeit. Bitte entscheiden, welche PRs bestehen bleiben, und die Reihenfolge festlegen.
3. Kleinigkeiten
main_window.py: zusätzliche Leerzeile direkt nachdef _open_folder(self):vor dem Docstring, ebenfalls doppelte Leerzeilen im Menüaufbau (ruff-Stil).platform_utils.py:import shutilsteht nach zwei Leerzeilen mitten im Header.
Positiv: Batch-Rename-Rollback (zweistufig, Swaps und Zyklen), negative Nummerierung und CLOCK$ sind sauber mit Tests abgedeckt. Keine Credentials oder Nutzerpfade im Diff.
Generated by Claude Code
|
Korrektur zu meinem Review vom 3.10.2026 Frühere Aussage: Punkt 1 des Reviews führt einen Sicherheitsbefund zum Terminal-Start als Befund dieses PR auf. Korrektur: Die dort genannte Funktion in Bestätigt bleibt: #15 ist Git-Vorfahre von #14 (per Hinweis zu Details: Weitere technische Einzelheiten zu diesem Punkt bitte nicht in diesem öffentlichen Thread, sondern über einen dafür geeigneten privaten Kanal klären (z. B. einen Security Advisory des Repos). Die Patch-Skizze im Review ist nicht unter Windows geprüft. Alle Entscheidungen (Basis-PR, Reihenfolge) bleiben offen. Generated by Claude Code |
Depends on #15. This branch contains the reviewed PR15 lifecycle/GC fix (
2d8120c) followed by the capacity feature (4145cc3). Base remainsmaster; merge order is #15, then #14, with the repository's independent review requirement respected.ExplorerPro shows a translated capacity bar and free, used, and total binary units for each drive in the folder sidebar. Opening or refreshing the sidebar updates the reading; unavailable drives show an explanation instead of a false zero. Long OS reads run in bounded helper processes with a ten-second timeout, and shutdown stops those helpers and drains their worker pool before Qt teardown. A pre-Qt entrypoint supports the frozen Windows executable without stdout. Sidebar selection and narrow-width layout remain intact.
Validation:
4145cc3: focused capacity, helper, GC, browser and Store regression checks: 56 passed in 56.82s, RC 0.The PR remains open for independent review. No package, build, Store submission, or release is claimed.