Repository navigation
fix(octop): add is_dir + OSError guard for agently_cli attachment folder iterdir - #2666
Closed
TrueFurina wants to merge 1 commit into
Closed
TrueFurina wants to merge 1 commit into
TrueFurina wants to merge 1 commit into
Conversation
…der iterdir The attachment-file handler in _finalize_attachment() calls list(folder.iterdir()) with zero protection - neither an is_dir() gate nor OSError handling. The folder is a temp unpack directory created from a user-supplied archive; if extraction fails (corrupt archive, partial write, concurrent cleanup), folder may not exist or not be a directory, causing FileNotFoundError or NotADirectoryError. On Windows, ACL restrictions or antivirus locks could also raise PermissionError during iterdir. Add both an is_dir gate and a try/except OSError, raising the existing ValueError on failure so the outer except Exception branch runs its cleanup (shutil.rmtree(folder)).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add is_dir() gate + ry/except OSError around older.iterdir() in �gently_cli.py attachment handler. One file, one commit.
Problem
_finalize_attachment() does list(folder.iterdir()) with zero protection. The function has an outer except Exception: that re-raises and cleans up, but the unhandled exception type (FileNotFoundError, NotADirectoryError, PermissionError) provides no semantic hint about what went wrong — and on Windows, PermissionError could originate from ACL restrictions or antivirus locks even after older was successfully created moments earlier.
The older path is a temp directory unpacked from a user-supplied attachment archive. Failure modes:
Fix
Add explicit if not folder.is_dir(): gate plus ry/except OSError around iterdir(), both raising the existing ValueError(tr('connector.agently.invalid_attachment')). This lets the outer except Exception: propagate a semantically meaningful error (and run shutil.rmtree(folder) cleanup) regardless of which filesystem error occurred.
Pattern
This fell through the R71-R72 iterdir sweep because the existing outer except Exception: masked the problem from grep-based searches. The _purge_expired_confirmations() sibling in the same file already uses is_dir() gate before older.iterdir() — this makes the two consistent.