ref(security): Add SafeDirectory to enforce path traversal checks#623
ref(security): Add SafeDirectory to enforce path traversal checks#623runningcode wants to merge 2 commits into
Conversation
ZipProvider.extract_to_temp_directory() now returns a SafeDirectory instead of a raw Path. SafeDirectory.resolve(untrusted) validates that attacker-controlled paths (manifest values, plist entries) stay within the extraction directory, replacing the manual is_safe_path() + raise pattern at each call site. This makes the safe path the default path — developers can't accidentally skip the check when joining untrusted input to an extract directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📲 Install BuildsiOS
Android
|
SafeDirectory now delegates glob, rglob, iterdir, /, exists, is_dir, relative_to, __fspath__, and common properties (name, stem, suffix, parent) to the underlying Path. This eliminates the .path accessor from all call sites. Icon parsers (IconParser, BinaryXmlDrawableParser, ProtoXmlDrawableParser) now accept only SafeDirectory instead of Path | SafeDirectory, enforcing the safety boundary at the type level. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| @@ -0,0 +1,10 @@ | |||
| class UnreasonableZipError(ValueError): | |||
There was a problem hiding this comment.
moved from zipprovider to avoid circular imports.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8a90e9f. Configure here.
| for icon_name in icon_info.primary_icon_files: | ||
| if not is_safe_path(app_bundle_path, icon_name): | ||
| raise UnsafePathError(f"Unsafe icon name in plist: {icon_name}") | ||
| app_bundle.resolve(icon_name) |
There was a problem hiding this comment.
Discarded resolve result silently performs validation
Medium Severity
app_bundle.resolve(icon_name) is called as a bare expression with its return value silently discarded. It serves as the sole path traversal validation for untrusted plist input, but it looks identical to dead code. Every other call site in this PR assigns the result (e.g., icon_path = self._extract_dir.resolve(icon_path_str) in apk.py and aab.py), making this the only place where the critical security check is invisible. A future developer or automated cleanup could remove this "unused" expression, silently disabling the path traversal protection for icon names.
Reviewed by Cursor Bugbot for commit 8a90e9f. Configure here.
1 participant
Summary
This PR makes the safe path the default path by introducing
SafeDirectory— aPath-like wrapper that validates untrusted input stays within bounds.ZipProvider.extract_to_temp_directory()returnsSafeDirectoryinstead ofPathSafeDirectory.resolve(untrusted)replaces the manualis_safe_path()+raise UnsafePathErrorpattern — developers can't accidentally skip the checkSafeDirectory.child(untrusted)creates a scoped subdirectory (e.g. AAB'sbase/dir)Pathoperations (glob,rglob,/,exists,relative_to, etc.) are delegated directly, soSafeDirectoryis a drop-in whereverPathwas used beforeIconParser,BinaryXmlDrawableParser,ProtoXmlDrawableParser) now accept onlySafeDirectory, enforcing the safety boundary at the type levelUnsafePathError/UnreasonableZipErrormoved toexceptions.pyto break a circular import🤖 Generated with Claude Code