Skip to content

Respect isolated mode when patching sys.path - #2050

Open
Karan Dhaodiyal (karandhaodiyal28-hash) wants to merge 4 commits into
microsoft:mainfrom
karandhaodiyal28-hash:fix-isolated-mode-sys-path
Open

Respect isolated mode when patching sys.path#2050
Karan Dhaodiyal (karandhaodiyal28-hash) wants to merge 4 commits into
microsoft:mainfrom
karandhaodiyal28-hash:fix-isolated-mode-sys-path

Conversation

@karandhaodiyal28-hash

Copy link
Copy Markdown

Fixes#1916

Python's isolated mode (-I) must not prepend the script directory (for a file target) or the current directory (for -m / -c) to sys.path — that's the whole point of the flag, and it exists to prevent local modules from shadowing stdlib/third-party imports. debugpy re-implements Python's startup sys.path handling in src/debugpy/server/cli.py, but did so unconditionally, so a debuggee launched under -I still got the directory injected:

$ python -I -m debugpy --listen 5678 -c "import sys; print(repr(sys.path[0]))"'' # cwd injected - isolated mode violated
$ python -I -c "import sys; print(repr(sys.path[0]))"'/usr/lib/python3.11' # plain Python, correct

The comments in those three functions already say "like Python itself does for -m/-c" — but Python itself skips the insert under -I. This guards all three sites (run_file, run_module, run_code) with sys.flags.isolated, matching CPython semantics exactly. sys.flags.isolated has existed since 3.4, and non-isolated runs are completely unaffected.

The attach-to-PID injection path also inserts a directory, but it deletes it again right after importing its helper, so it's left as-is.

Testing

Added test_isolated_mode_sys_path (parametrized over file / -m / -c) to tests/debugpy/test_cli_args.py, asserting the target directory is not prepended to sys.path[0] when the debuggee runs under -I. The three new cases fail without this change and pass with it. Verified locally on Windows / Python 3.12; test_run.py shows no new failures introduced (identical pass/fail set with and without the change).

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@karandhaodiyal28-hash

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@rchiodo

Rich Chiodo (rchiodo) commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

Comment threadsrc/debugpy/server/cli.py Outdated
# be in place before trying to use find_spec below to resolve submodules.
sys.path.insert(0, str(""))
# Add current directory to path, like Python itself does for -m, unless
# it's suppressed by isolated mode. This must be in place before trying

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning · Non-blocking recommendation

CPython suppresses the sys.path[0] prepend whenever safe_path is set, not only under isolated. -P / PYTHONSAFEPATH=1 set safe_path without setting isolated, so python -P -m debugpy ... still injects the current directory. Please broaden the gate to account for sys.flags.safe_path (with a compatibility fallback for Python <3.11), or explicitly document why this fix intentionally covers only -I.

[verified]

@rchiodoRich Chiodo (rchiodo) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@rchiodoRich Chiodo (rchiodo) added the review-auto:approved Automated review: no blocking findings (approval posted). label Jul 30, 2026
@karandhaodiyal28-hash

Copy link
Copy Markdown
Author

Thanks @rchiodo, that's a great catch — you're right that safe_path (via -P / PYTHONSAFEPATH) suppresses the prepend independently of isolated.

I've broadened the gate to cover both. All three sites (run_file, run_module, run_code) now go through a small helper:

def_skip_sys_path_prepend():
# `sys.flags.safe_path` was added in Python 3.11; fall back to False on older versions.returnsys.flags.isolatedorgetattr(sys.flags, "safe_path", False)

The getattr(..., False) keeps it safe on the pre-3.11 versions debugpy still supports. I also parametrized the regression test over both -I and -P (the -P cases are skipped below 3.11). Verified locally: with only the previous isolated gate the three -P cases fail, and they pass with this change; the full test_cli_args.py is green.

Comment threadtests/debugpy/test_cli_args.py Outdated
)

sys_path_0 = output.decode("utf-8").strip().splitlines()[-1]
assert sys_path_0 != repr(not_expected)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning · Non-blocking recommendation

Harden this subprocess-based regression test: add a timeout and preserve child stderr so a hung or failing debuggee produces a timely, diagnosable failure. The positional last-line parsing and negative-only assertion could also mask unrelated output or an incorrect replacement path; use an explicit marker and assert the expected path behavior. [verified]

@rchiodoRich Chiodo (rchiodo) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@rchiodo

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

- Add explicit timeout=30 to fail fast on Windows py312 hangs.
- Capture stderr (was DEVNULL) so failures are diagnosable.
- Add explicit pytest.fail messages on TimeoutExpired / CalledProcessError and on the equality assertion.
CopilotAI lite review requested due to automatic review settings August 6, 2026 07:12

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates debugpy’s CLI launch path handling to match CPython behavior in “safe sys.path” modes, ensuring that -I (isolated) and -P / PYTHONSAFEPATH (safe-path) do not implicitly prepend the script directory / CWD to sys.path[0].

Changes:

  • Add _skip_sys_path_prepend() and use it to guard the sys.path.insert(0, ...) behavior in run_file(), run_module(), and run_code().
  • Add a regression test covering -I and -P across file / -m / -c targets via a subprocess-invoked CLI run.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/debugpy/server/cli.pySkips sys.path prepends when sys.flags.isolated or sys.flags.safe_path indicate Python should not inject script dir / CWD.
tests/debugpy/test_cli_args.pyAdds a subprocess-based parametrized test to assert sys.path[0] is not the target directory / CWD under -I / -P.

Comment threadsrc/debugpy/server/cli.py Outdated
Comment on lines +364 to +365
dir = os.path.dirname(target)
sys.path.insert(0, dir)
@rchiodo

Copy link
Copy Markdown
Contributor

GitHub cannot anchor PR review comments to unchanged lines in the diff. Falling back to a general PR comment for tests/debugpy/test_cli_args.py:L169.

Warning · Non-blocking recommendation

assert completed.stdout checks raw bytes, but parsing strips before indexing. Whitespace-only stdout passes the assertion and then raises IndexError, losing the captured stderr diagnostics. Guard the parsed lines instead before reading lines[-1].

[verified]

@rchiodo

Copy link
Copy Markdown
Contributor

The new integration test documents occasional Windows hangs while starting a real debug server. Please consider making it deterministic by testing the sys.path branch directly or avoiding --listen.

@rchiodoRich Chiodo (rchiodo) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-auto:approvedAutomated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

debugpy ignores isolated mode (-I)

3 participants

@karandhaodiyal28-hash@rchiodo