fix: protect checksum workers and GUI garbage collection - #15
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>
lukisch
left a comment
There was a problem hiding this comment.
Zweitmodell-Review: CI grün (12/12), derselbe Sicherheitsbefund wie in #14.
1. Befehlsinjektion beim Terminal-Start (src/core/platform_utils.py, get_terminal_command)
Der Verzeichnisname wird in powershell -Command "Set-Location -LiteralPath '{directory}'" und cmd /K "cd /d {directory}" interpoliert. Ein Ordnername mit ' oder & (unter Windows zulässig) führt zu Codeausführung. Popen(..., cwd=target_dir) setzt das Arbeitsverzeichnis bereits, die Interpolation kann entfallen:
if shutil.which("powershell"):
return ["powershell", "-NoExit"]
return ["cmd", "/K"]Dazu ein Test mit einem Ordner a'b&c.
2. Überlappung
Der Inhalt ist (bis auf die Laufwerkskapazität) in #14 enthalten, batch_rename_service, checksum_dialog, gui_gc und properties_dialog sind in beiden PRs identisch oder fast identisch. Es sollte nur eine der beiden Varianten gemergt werden, sonst Konflikte.
Sonst: Tests für Batch-Rename-Rollback, Properties-Dialog und Terminal vorhanden. Keine Credentials oder Nutzerpfade im Diff.
Generated by Claude Code
|
Korrektur zu meinem Review vom 3.10.2026 Frühere Aussage: Der Review nennt „derselbe Sicherheitsbefund wie in #14" als Befund dieses PR. Korrektur: Die betroffene Funktion in Bestätigt bleibt: #15 hat gegenüber #14 keinen eigenen Inhalt (#15 ist Git-Vorfahre von #14). Hinweis zu Details: Weitere technische Einzelheiten bitte über einen dafür geeigneten privaten Kanal klären, nicht in diesem Thread. Entscheidungen bleiben offen. Generated by Claude Code |
|
Review (Claude Sonnet 5.5, Zweitmodell nach D-20260902-002) — Empfehlung: merge-ready Geprüft: kompletter Diff (src, CI, pyproject), Branch im isolierten Worktree.
Befunde: keine blockierenden. Hinweis: Merge-Reihenfolge: #15 → #14 → #17 (gestapelt: #14 enthält #15, #17 enthält #14+#15). Bitte als Merge-Commit (nicht Squash) mergen, dann bleiben #14/#17 ohne Rebase konfliktfrei. Danach #12, #16, #13 (brauchen nach dem Stapel einen kurzen Abgleich mit master — melden, ich erledige das). |
conftest.py: master-Fixture (GUI-GC) beibehalten, Windows-Symlink-Teardown-Schutz aus diesem PR ergaenzt; test_checksum_dialog.py auf master-Stand (qtbot, waitUntil).
…rachladen im GUI-GC-try-Block
Summary
Popen(cwd=...).Validation