Skip to content

don't resolve the kernel's own connection file through the cwd - #1576

Open
Keshava-kesh wants to merge 1 commit into
ipython:mainfrom
Keshava-kesh:connfile-cwd-lookup
Open

Keshava-kesh wants to merge 1 commit into
ipython:mainfrom
Keshava-kesh:connfile-cwd-lookup

Conversation

@Keshava-kesh

Copy link
Copy Markdown

init_connection_file makes up kernel-.json when no connection file was passed on the command line, then hands that invented name to filefind over the current directory and connection_dir, so a file of that name sitting in the kernel's working directory wins and load_connection_file adopts what is in it, HMAC key and all five ports included. The launches that reach this are the ones without -f: ipython kernel, python -m ipykernel_launcher on its own, and every embed_kernel caller, and their working directory is frequently a checked-out project or a shared directory nobody vetted. Whoever wrote that file then holds the key the kernel signs and verifies with, which is all of the message authentication there is, so they can hand it an execute_request. I noticed it while following where the connection file actually gets created and saw that the create branch is the fallback rather than the default. Planting the file in a temp directory and calling init_connection_file confirmed it, session.key came back as the planted string and shell_port as the planted 51111. get_connection_file repeats the same cwd-first lookup, so even once the kernel owns its file in connection_dir, %connect_info and connect_qtconsole resolve the planted basename ahead of the real one and hand that out instead. The fix creates the file straight away when the name was ours to invent instead of looking it up, and puts connection_dir ahead of the working directory in get_connection_file; an explicit relative -f still resolves against the working directory the way it always did, with a regression beside the existing connection file tests in tests/test_kernelapp.py.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant