fix(parser-sandbox): drop PYTHONPATH/HOME and run parser child unprivileged#2019
Open
Subramaniyajothi6 wants to merge 2 commits into
Open
fix(parser-sandbox): drop PYTHONPATH/HOME and run parser child unprivileged#2019Subramaniyajothi6 wants to merge 2 commits into
Subramaniyajothi6 wants to merge 2 commits into
Conversation
utksh1
requested changes
Jul 14, 2026
utksh1
left a comment
Owner
There was a problem hiding this comment.
Dropping to nobody can make the parser source unreadable when the backend runs as root and the parser file or its parent directory is root-private. The current tests only inspect kwargs, not an actual root-owned parser execution. Please prove the root execution path can read and run a parser safely (or stage the parser in a read-only accessible location) and add a focused regression test.
…ileged _sanitised_env() kept PYTHONPATH and HOME in the parser child's environment. An inherited PYTHONPATH pointing at an attacker-controlled directory let the "sandboxed" parser import arbitrary modules; HOME was an equivalent vector via the per-user site directory (~/.local/lib/pythonX.Y/site-packages). - Drop PYTHONPATH and HOME; set PYTHONNOUSERSITE=1 and PYTHONSAFEPATH=1 to also close the user-site and python -c CWD import vectors. Installed dependencies still resolve via the interpreter's own site-packages. - Add _privilege_drop_kwargs(): when the backend runs as root, execute the parser child as the nobody account (fallback uid/gid 65534) via subprocess user/group/extra_groups. No-op when already unprivileged or on Windows. - Apply the same drop to the unshare --user --net capability probe so a host where root can unshare but nobody cannot is detected at probe time. Flip the two helper tests that asserted PYTHONPATH/HOME are retained (they encoded the vulnerable behaviour), add hardening-flag and privilege-drop coverage, and add an end-to-end regression proving an injected PYTHONPATH cannot be imported by the sandboxed parser.
…an read it Dropping the parser child to 'nobody' when the backend runs as root broke parsing whenever parser.py (or its parent directory) was root-owned and not world-readable: the unprivileged child could not read the source and the parse failed. Reads were previously assumed to work because the tests only inspected the subprocess kwargs, never an actual root-owned execution. Stage a private copy of parser.py the drop target can read: the file is copied into a fresh temp dir, chowned to the drop uid/gid and made 0400, while the directory stays backend-owned and 0711 (traversable, not listable) so cleanup works whether or not the backend is root. The copy lives only for the child's lifetime and is removed on exit. When no privilege drop is in effect the original path is used unchanged. The drop is resolved once and reused for both staging and the child so they always target the same account. Tests: _staged_parser is a no-op without a drop and otherwise produces a faithful, 0400, drop-target-readable copy that is cleaned up; an end-to-end run under a simulated drop still parses and leaks no staging dir; and a root-gated regression reproduces the root-private parser.py condition and proves the child reads+runs it unprivileged. Addresses review feedback on utksh1#2019.
a6feffc to
37ca5dd
Compare
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
Closes #1804.
parser_sandbox.pyruns untrusted third-partyparser.pycode in a subprocess, but_sanitised_env()keptPYTHONPATHandHOMEin the child environment. An inheritedPYTHONPATHpointing at an attacker-controlled directory let the "sandboxed" parserimportarbitrary modules, andHOMEwas an equivalent vector via the per-user sitedirectory (
~/.local/lib/pythonX.Y/site-packages). There was also no privilege drop, soa backend running as root executed untrusted parser code as root.
Changes
_sanitised_env():PYTHONPATH(directsys.pathinjection) andHOME(user-siteinjection).
PYTHONNOUSERSITE=1so~/.localsite-packages is ignored even ifHOMEleaksin via another route.
PYTHONSAFEPATH=1(3.11+) so the process working directory — whichpython -cotherwise prepends to
sys.path— cannot be used to smuggle in modules.site-packagesis onsys.pathregardless ofPYTHONPATH._privilege_drop_kwargs(): when thebackend runs as root (
os.geteuid() == 0, POSIX only) the child is executed as thenobodyaccount (falling back to uid/gid65534) usingsubprocess'suser/group/extra_groupsparameters. It's a no-op when the backend is alreadyunprivileged or on Windows.
unshare --user --netcapability probe(issue Security: plugin subprocess runs with no outbound network isolation #1620) so a host where root can create user namespaces but
nobodycannot isdetected at probe time rather than failing mid-parse.
Tests
PYTHONPATH/HOMEare retained (theyencoded the vulnerable behaviour) to assert they're dropped; added coverage for the
two new hardening flags and for
_privilege_drop_kwargs()(no-op off-root / non-posix,targets
nobody, and the 65534 fallback — using an injected fakepwdmodule so itruns on non-POSIX CI too).
evilmodonPYTHONPATHandconfirms the sandboxed parser can no longer import it.
pytest testing/backend/unit/test_parser_sandbox.py testing/backend/unit/test_parser_sandbox_helpers.py→ 43 passed. Adjacent sandbox suites pass;ruff checkclean. No plugin metadata touched.Notes
subprocessraises rather than silently running the parser as root.@utksh1 please review this pr